RFR: JDK-8255439: System Tray icons get corrupted when windows scaling changes [v2]
Kevin Rushforth
kcr at openjdk.java.net
Sat May 7 13:04:49 UTC 2022
On Fri, 6 May 2022 17:59:39 GMT, Harshitha Onkar <duke at openjdk.java.net> wrote:
>> In Windows, when desktop scaling is changed the tray icons was distorted/blurred a bit each time scaling changes.
>>
>> With the proposed fix, the tray icon scales according to on-the-fly DPI scale settings. A test case has been added which adds a MRI icon to system tray, to observe the icon scaling when DPI is changed. Since the scale cannot be programmatically changed (for dynamic on-the-fly scale changes), I have used a manual test case to test this scenario.
>>
>> When DPI changes usually two messages are sent by windows -
>>
>> - [WM_DPICHANGED](https://docs.microsoft.com/en-us/windows/win32/hidpi/wm-dpichanged)
>> - [WMPOSCHANGING](https://docs.microsoft.com/en-us/windows/win32/winmsg/wm-windowposchanging)
>>
>> I'm triggering an update on tray icons on receiving WMPOSCHANGING msg through the Tray icon's Window Procedure. Triggering an update on WM_DPICHANGED was still causing the icons to be distorted, hence WMPOSCHANGING is being used as the message to trigger the update.
>
> Harshitha Onkar has updated the pull request incrementally with one additional commit since the last revision:
>
> resized instruction window and formatted line lengths
The fix looks good to me, but someone more familiar with the code will need to review it. I can confirm that the new test program fails without the fix and passes with the fix. One problem with the test is that it doesn't exit for me, regardless of whether the test passes or fails (or times out).
To partially answer Sergey's question, the problem with just using `WM_DPICHANGED` to trigger the update is that the scale of the default screen transform used by `WTrayIconPeer::updateNativeImage` hasn't yet been updated to reflect that new DPI scale. I don't know _why_ that is the case here, but it is. I do know that in JavaFX we stopped relying on `WM_DPICHANGED` for anything related to HiDPI several years ago when fixing some multi-monitor HiDPI issues in [JDK-8146920](https://bugs.openjdk.java.net/browse/JDK-8146920).
To show that triggering the update on `WM_DPICHANGED` is insufficient, I applied the patch from this PR, added some debugging output, and modified the fix to also update the image on a `WM_DPICHANGED` event. Here is the output when I start the program with a screen scale of 1.25 and then change it to 1.5:
WM_WINDOWPOSCHANGING
AwtTrayIcon::UpdateTrayIconHandler
SystemTray::updateTrayIcons
WTrayIconPeer::updateNativeImage : scale = (1.25, 1.25)
WM_DPICHANGED
AwtTrayIcon::UpdateTrayIconHandler
SystemTray::updateTrayIcons
WM_WINDOWPOSCHANGING
AwtTrayIcon::UpdateTrayIconHandler
SystemTray::updateTrayIcons
WTrayIconPeer::updateNativeImage : scale = (1.25, 1.25) <----- This is from the WM_DPICHANGED event
WTrayIconPeer::updateNativeImage : scale = (1.25, 1.25)
WM_WINDOWPOSCHANGING
AwtTrayIcon::UpdateTrayIconHandler
SystemTray::updateTrayIcons
WTrayIconPeer::updateNativeImage : scale = (1.5, 1.5) <----- Transform has now been updated
WM_WINDOWPOSCHANGING
AwtTrayIcon::UpdateTrayIconHandler
SystemTray::updateTrayIcons
WTrayIconPeer::updateNativeImage : scale = (1.5, 1.5)
WM_WINDOWPOSCHANGING
AwtTrayIcon::UpdateTrayIconHandler
SystemTray::updateTrayIcons
WTrayIconPeer::updateNativeImage : scale = (1.5, 1.5)
So I think the fix using `WM_WINDOWPOSCHANGING` to trigger the update is good. I left a couple minor comments inline.
src/java.desktop/share/classes/java/awt/SystemTray.java line 294:
> 292: SwingUtilities.invokeLater(()->{
> 293: TrayIcon[] trayIconList = null;
> 294: trayIconList = systemTray.getTrayIcons();
Minor: you can combine this into a single statement (no need to set it to `null` first).
src/java.desktop/share/classes/java/awt/SystemTray.java line 300:
> 298: return;
> 299: }
> 300: for (TrayIcon trayIcon: trayIconList) {
Minor: we usually put a space both before and after the `:` when used as a for-each operator.
src/java.desktop/windows/native/libawt/windows/awt_TrayIcon.cpp line 227:
> 225: void AwtTrayIcon::UpdateTrayIconHandler()
> 226: {
> 227: jmethodID updateTrayFn;
Minor: indentation not right
-------------
PR: https://git.openjdk.java.net/jdk/pull/8441
More information about the client-libs-dev
mailing list