On Fri, 19 Apr 2024 19:46:01 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> 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.
Yes, `openTheme()` actually returns 0. Hence I used `null` && `0` checker.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18706#discussion_r1574224390
