On Wed, 24 May 2023 18:55:35 GMT, Rajat Mahajan <rmaha...@openjdk.org> wrote:
>> Problem:
>>
>> Check boxes and radio buttons in Windows Look-and-Feel have rendering issues
>> when window is moved from display with one scale to display with a different
>> scale on a multi-monitor setup:
>>
>> - Scrawly ticks in checkboxes;
>> - Wrong circle relations in selected radio buttons.
>>
>> Root-cause:
>> We open theme on AWT Toolkit Window which always has Primary Monitor DPI.
>> Due to this when the app window goes to Secondary Screen with different DPI
>> UI buttons
>> appear incorrectly rendered.
>> Following is a list proposed changes to fix this issue.
>>
>>
>> Proposed Fix with Summary of changes:
>>
>> 1. Open a new Theme Handle per the DPI of the Screen where the App window is.
>> --> This makes sure we get the correct size for UI buttons based on the DPI
>> of the screen where the app.
>> window is.
>>
>> 2. GetPartSize() of icons returns size based on standard size = 96 DPI.
>> --> This change makes sure that the default size of UI buttons is 96 since
>> we use this on Java side to layout UI.
>>
>> 3. Rect size for icons in native paintBackground() function is fetched from
>> Windows that specific DPI.
>> -->This makes sure that the UI buttons aren't stretched because the size
>> calculated on Java side is different from what Windows returns. Thus UI
>> buttons are scaled correctly once we get their size back from Windows.
>>
>> 4. Adjust width and the height of the resolution variant image so that for
>> scaling values of 1.25 , 2.25 , and such we always floor, while for 1.5,
>> 1.75, 2.5, 2.75 , and such we always ceil.
>> --> This helps make sure that for .25s scaling we get better rendering.
>> This is because when we go from Double to Int for pixel rendering we
>> sometimes need to floor or ceil to get crisp rendering.
>>
>> As of now with these changes the rendering is crisp and good for all scaling
>> factors with the exception .25s wherein some small artifact is seen
>> sometimes in rendering of buttons but is still much better compared to what
>> we have now.
>>
>>
>> Testing:
>>
>> Tested locally on my Windows machine with a 2 monitor setup 125%, 150%,
>> 175%, 225% scaling values and on mach5.
>>
>> ___________________________________
>> Monitor 1 | Monitor 2
>> (Primary) |
>> |
>> 125% | 175%
>> 150% | 175%
>> 150% | 225%
>> 175% | 175%
>> 175% | 150%
>> 175% | 225%
>> _____________________ |_____________
>>
>> Also tested on setup with scaling values of up-to 350%.
>
> Rajat Mahajan has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update
> src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/XPStyle.java
>
> Co-authored-by: Andrey Turbanov <turban...@gmail.com>
src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/XPStyle.java
line 60:
> 58:
> 59: import java.util.HashMap;
> 60: import javax.swing.AbstractButton;
I find it very uncommon to have blank line not between `java.*` and `javax.*`
imports. I suggest combining `HashMap` with other `java.*` and adding a blank
line after `HashMap`.
src/java.desktop/windows/classes/sun/awt/windows/ThemeReader.java line 106:
> 104: }
> 105:
> 106: dpiAwareWidgetToTheme.computeIfAbsent(dpi, key->new
> HashMap<>()).put(widget, theme);
Suggestion:
dpiAwareWidgetToTheme.computeIfAbsent(dpi, key -> new
HashMap<>()).put(widget, theme);
There are usually spaces around `->` in lambda expressions.
src/java.desktop/windows/classes/sun/awt/windows/ThemeReader.java line 125:
> 123: // Close old themes.
> 124: if (!dpiAwareWidgetToTheme.isEmpty()) {
> 125: for (Map <String, Long> dpiVal:
> dpiAwareWidgetToTheme.values()) {
Suggestion:
for (Map<String, Long> dpiVal :
dpiAwareWidgetToTheme.values()) {
No space between type and its type arguments; spaces around `:` in for-each
loop.
src/java.desktop/windows/classes/sun/awt/windows/ThemeReader.java line 145:
> 143:
> 144: if (dpiAwareWidgetToTheme.get(dpi) != null)
> 145: {
Suggestion:
if (dpiAwareWidgetToTheme.get(dpi) != null) {
For consistent Java style.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13701#discussion_r1204694047
PR Review Comment: https://git.openjdk.org/jdk/pull/13701#discussion_r1204707624
PR Review Comment: https://git.openjdk.org/jdk/pull/13701#discussion_r1204711820
PR Review Comment: https://git.openjdk.org/jdk/pull/13701#discussion_r1204713180
