On Fri, 6 May 2022 17:59:39 GMT, Harshitha Onkar <d...@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
