On Fri, 19 Apr 2024 19:19:26 GMT, Rajat Mahajan <rmaha...@openjdk.org> wrote:
>> Getting a theme for particular dpi failed in windows L&F during print test. >> Before [JDK-8294427](https://bugs.openjdk.org/browse/JDK-8294427) fix, theme >> was independent of DPI. After the fix >> (https://github.com/openjdk/jdk/commit/a63afa4aa62863d1a199a0fb7d2f56ff8fcd04fd) >> getting a theme in Windows L&F became dependent on DPI. Now it works fine >> in paint to a frame or window but for printing the DPI is computed with >> scaling factor which might not be w.r.t set of connected monitors and hence >> error is returned according to [this from windows >> document](https://learn.microsoft.com/en-us/windows/win32/api/uxtheme/nf-uxtheme-openthemedatafordpi). >> >> The suggested fix is to handle printer cases with default dpi since in error >> condition 0 is returned with `openThemeImpl(widget, dpi)`. The fix seems to >> be working fine without causing any regression (Tested in CI systems and >> also the affected tests). > > src/java.desktop/windows/classes/sun/awt/windows/ThemeReader.java line 120: > >> 118: // this should be called only with writeLock held >> 119: private static Long getThemeImpl(String widget, int dpi) { >> 120: if (openThemeImpl(widget, dpi) == null || openThemeImpl(widget, >> dpi) == 0) { > > I think it would be cleaner and better if this is handled in > `openThemeImpl()` itself. We can put a check for NULL `theme `being returned > and return the one based on `defaultDPI`, if that is the case. I agree. The current approach calls `openThemeImpl` three times: once or twice with the passed `dpi` and then once with `defaultDPI` as the fallback. Moreover, it *leaks theme handles* if `openThemeImpl` returns non-zero value and it does so repeatedly even when there's a cached theme handle. Handling the failure inside `openThemeImpl` seems a better option. Even though it results in a higher memory usage. I believe `openTheme` returns 0 because the DPI of a printer is higher than that of screens, and Windows doesn't support such DPIs. The problem is likely reproducible when you print other components such as buttons. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18706#discussion_r1572847417
