On Tue, 11 May 2021 10:53:06 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Alexander Zuev has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains five additional
>> commits since the last revision:
>>
>> - Small fixes
>> Added testing for MultiResolutionImage
>> - Merge remote-tracking branch 'origin/master' into JDK-8182043
>> - Update src/java.desktop/windows/native/libawt/windows/ShellFolder2.cpp
>>
>> Select one icon at a time.
>>
>> Co-authored-by: Alexey Ivanov
>> <70774172+aivanov-...@users.noreply.github.com>
>> - Small fixes
>> Remived size parameter from makeIcon
>> Removed useVGAColors usage as irrelevant since it is ignored on all
>> supported platforms now
>> - 8182043: Access to Windows Large Icons
>> Fix updated based on the previous review.
>
> src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java line
> 1192:
>
>> 1190: */
>> 1191: static Image getShell32Icon(int iconID, int size) {
>> 1192: long hIcon = getIconResource("shell32.dll", iconID, size,
>> size);
>
> It's outside the scope for this code review but still, should
> `getIconResource` free the loaded library? With each call, the library gets
> loaded but it's never freed and thus it's never unloaded even if it's not
> needed any more.
I will create a separate bug to track this - i do not know if library loading
gets cached on some level and what impact on performance will we have if we
release it every call. But i will check it out separately.
> src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java line
> 1196:
>
>> 1194: Image icon = makeIcon(hIcon);
>> 1195: disposeIcon(hIcon);
>> 1196: return icon;
>
> Shall it not be wrapped into multi-resolution image if the `size` is
> different from the actual size of the icon?
> Previously, it was inside `makeIcon`.
Wherever it is necessary down the line we are wrapping the result in
multi-resolution and if we wrap it here too we will have multi-resolution icon
that contains multi-resolution images as a resolution variant. Unfortunately
AWT can't handle painting of such abomination.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2875
