On Thu, 31 Aug 2023 14:03:28 GMT, Alexey Ivanov <[email protected]> wrote:
>> test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java line 57:
>>
>>> 55: public static final String INSTRUCTIONS = "INSTRUCTIONS:\n\n"
>>> 56: + "Verify that high resolution system icons are used for
>>> JCheckBoxMenuItem and JRadioButtonMenuItem on HiDPI displays.\n"
>>> 57: + "If the display does not support HiDPI mode press PASS.\n"
>>
>> We can verify this condition by something like this:
>>
>> AffineTransform transform =
>> GraphicsEnvironment.getLocalGraphicsEnvironment()
>>
>> .getDefaultScreenDevice()
>>
>> .getDefaultConfiguration()
>>
>> .getDefaultTransform();
>> if (!(transform.getScaleX() > 1.0 && transform.getScaleY() > 1.0)) {
>> throw new SkippedException("This test is for High DPI displays
>> only");
>> }
>>
>>
>> Then the line can be removed. On the other hand, the second case is
>> applicable even if the main display isn't a High DPI one.
>>
>> What do others think?
>>
>> To use `SkippedException`, add the following lines to the tags:
>>
>> * @library /test/lib
>> * @build jtreg.SkippedException
>>
>> and import `jtreg.SkippedException`. See
>> [TaskbarPositionTest.java](https://github.com/openjdk/jdk/blob/master/test/jdk/javax/swing/Popup/TaskbarPositionTest.java)
>> for an example.
>
>> @aivanov-jdk I tried it on my High DPI Windows monitor but with scale set to
>> 100% and that threw the skipped exception. I know by default we always have
>> some scale setting on Windows for High DPI Monitors, but do we expect such a
>> scenario? , just making sure before we add this.
>
> It's expected, isn't it? High DPI basically refers to the scale > 100%. You,
> as the user, are free to set the scale to 100% even though Windows recommends
> 150% or higher; just as you're free to set the scale to a value higher than
> the recommended one.
>
> Whether we want it or not is up for discussion. That's what I meant by
>
>> On the other hand, the second case is applicable even if the main display
>> isn't a High DPI one.
>
> Originally, the test was intended to run on *Retina displays **only*** which
> correspond to the scale of 200% in Windows environment.
>
> You and I used this test to verify rendering of the check or radio marks when
> you worked on [JDK-8294427](https://bugs.openjdk.org/browse/JDK-8294427).
> With that fix, a new scenario was added to the test: change the scale of the
> monitor or move the window to another monitor with a different scale and
> verify that the check or radio marks are rendered crisply.
>
> If the instructions are written as they are now, we can verify whether the
> main display is in High DPI mode or not and skip the test accordingly.
> Otherwise, the user needs to click Pass which gives the wrong idea that the
> test passed, but the test wasn't run as intended.
>
> Then, on a Windows system, it's possible to change the scale even if the
> current one is 100%.
>
> On the other hand, spelling all the cases in the instructions could make them
> unclear, which means we lose the test altogether.
>
> Any other opinions? What do you think, @honkar-jdk?
@aivanov-jdk @rajamah Both your points seem to be valid. On one hand the
condition might not cover all the cases as stated by @rajamah. When we have
dual-monitor setup, the main or default one needs to be a High DPI one to
proceed with the testing, correct?
But then again adding the condition that @aivanov-jdk suggested ensures that
the user has the correct display setting to proceed with the test.
Perhaps adding a note at the end of the test instructions about HiDPI and
single/multi-screen requirements can add clarity along with the condition?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15441#discussion_r1312079236
