<Swing Dev> RFR: 8182043: Access to Windows Large Icons

[Alexey Ivanov]

On Mon, 28 Sep 2020 15:20:33 GMT, Alexander Zuev <kizune at openjdk.org> wrote:

> Moving review from Mercurial. See https://mail.openjdk.java.net/pipermail/awt-dev/2020-August/016078.html for previous
> iteration.

src/java.desktop/windows/native/libawt/windows/ShellFolder2.cpp line 986:

> 984:                 hIcon = hIconLow;
> 985:             } else {
> 986:                 fn_DestroyIcon((HICON)hIconLow);

I wonder why you extract two icons, you never use both. This adds a delay: only one of the extracted icons is ever
used. If the idea is to limit the size by 16, then min(16, size) passed as the icon size would do the trick.

Each time we use this function, we request two icons but we never use both of them.

Suggestion:

            if (size < 24) {
                size = 16;
            }
            hres = pIcon->Extract(szBuf, index, &hIcon, NULL, size);

And `hIconLow` can be removed.

src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java line 263:

> 261:     * a system file browser for the requested size.
> 262:     *
> 263:     * The default implementation gets information from the

Suggestion:

    * <p>The default implementation gets information from the

src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java line 264:

> 262:     *
> 263:     * The default implementation gets information from the
> 264:     * <code>ShellFolder</code> class. Whenever possible the icon

Suggestion:

    * <code>ShellFolder</code> class. Whenever possible, the icon

Do we continue using `<code>` elements? I thought that `{@code}` is the preferred way to markup class names.

src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java line 269:

> 267:     * magnification factors.
> 268:     *
> 269:     * Example: <pre>

Suggestion:

    * <p>Example: <pre>

src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java line 207:

> 205:      * Returns the icon of the specified size used to display this shell folder.
> 206:      *
> 207:      * @param size size of the icon > 0(Valid range: 1 to 256)

Suggestion:

     * @param size size of the icon > 0 (Valid range: 1 to 256)

src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java line 1035:

> 1033:                         new BufferedImage(iconSize, iconSize, BufferedImage.TYPE_INT_ARGB);
> 1034:                 img.setRGB(0, 0, iconSize, iconSize, iconBits, 0, iconSize);
> 1035:                 return img;

There are cases where the size of the buffered image is different from the requested size. It could affect the layout
because it breaks the assumption that the returned image has the requested size but it may be larger.

src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java line 1105:

> 1103:                                     bothIcons.put(getLargeIcon? SMALL_ICON_SIZE : LARGE_ICON_SIZE, newIcon2);
> 1104:                                     newIcon = new MultiResolutionIconImage(getLargeIcon ? LARGE_ICON_SIZE
> 1105:                                             : SMALL_ICON_SIZE, bothIcons);

I propose to refactor this code into a separate method which always fetches small and large icon and puts into a
corresponding cache.

However, I still think there's not much value in getting smaller icon size in addition to larger one. Or does it
provide an alternative image for the case where the system scaling is 200% and the window is moved to a monitor with
scaling of 100%?

src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java line 1163:

> 1161:
> 1162:                     multiResolutionIcon.put(s, newIcon);
> 1163:                     if (size < 8 || size > 256) {

Suggestion:

                    if (size < MIN_QUALITY_ICON || size > MAX_QUALITY_ICON) {

src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolderManager2.java line 414:

> 412:                 if (i >= 0) {
> 413:                     return Win32ShellFolder2.getShell32Icon(i,
> 414:                          key.startsWith("shell32LargeIcon ")?LARGE_ICON_SIZE : SMALL_ICON_SIZE);

Suggestion:

                         key.startsWith("shell32LargeIcon ") ? LARGE_ICON_SIZE : SMALL_ICON_SIZE);

test/jdk/javax/swing/JFileChooser/FileSystemView/SystemIconTest.java line 69:

> 67:             }
> 68:         }
> 69:     }

Does it make sense to check that the icon is a multi-resolution image with the expected sizes?

-------------

PR: https://git.openjdk.java.net/jdk/pull/380