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

[Alexey Ivanov]

On Mon, 8 Mar 2021 13:22:07 GMT, Alexander Zuev <kizune at openjdk.org> wrote:

> Fix updated after first round of review.

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

> 1042:                         new BufferedImage(iconSize, iconSize, BufferedImage.TYPE_INT_ARGB);
> 1043:                 img.setRGB(0, 0, iconSize, iconSize, iconBits, 0, iconSize);
> 1044:                 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. (Or is it no longer possible?) I think it should be wrapped into `MultiResolutionIconImage` in this case.

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

> 1112:                                     bothIcons.put(getLargeIcon? SMALL_ICON_SIZE : LARGE_ICON_SIZE, newIcon2);
> 1113:                                     newIcon = new MultiResolutionIconImage(getLargeIcon ? LARGE_ICON_SIZE
> 1114:                                             : SMALL_ICON_SIZE, bothIcons);

I still propose to refactor this code to make it clearer. The code always returns two icons: _small + large_ or _large + small_ — combined in `MultiResolutionIconImage`.

You can get `smallIcon` and `largeIcon` and then wrap them into `MultiResolutionIconImage` in the correct order and store each icon in the cache.

My opinion hasn't changed here.

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 1195:

> 1193:      */
> 1194:     static Image getShell32Icon(int iconID, int size) {
> 1195:         boolean useVGAColors = true; // Will be ignored on XP and later

I cannot see where `useVGAColors` is used. Since the supported OS are later than XP, it can be removed, can't it?

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

> 981:                 size = 16;
> 982:             }
> 983:             hres = pIcon->Extract(szBuf, index, &hIcon, NULL, (16 << 16) + size);

Suggestion:

            hres = pIcon->Extract(szBuf, index, &hIcon, NULL, size);
Now you request only one icon.

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

> 62:             }
> 63: 
> 64:             if (icon.getIconWidth() != size) {

Does it make sense to check that the icon is a multi-resolution image with the expected sizes?
We know for sure `explorer.exe` contains the entire set of icons.

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

> 262:     * <p>
> 263:     * The default implementation gets information from the
> 264:     * <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/windows/classes/sun/awt/shell/Win32ShellFolder2.java line 1112:

> 1110:                                     Map<Integer, Image> bothIcons = new HashMap<>(2);
> 1111:                                     bothIcons.put(getLargeIcon? LARGE_ICON_SIZE : SMALL_ICON_SIZE, newIcon);
> 1112:                                     bothIcons.put(getLargeIcon? SMALL_ICON_SIZE : LARGE_ICON_SIZE, newIcon2);

Suggestion:

                                    bothIcons.put(getLargeIcon ? LARGE_ICON_SIZE : SMALL_ICON_SIZE, newIcon);
                                    bothIcons.put(getLargeIcon ? SMALL_ICON_SIZE : LARGE_ICON_SIZE, newIcon2);

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

> 1144:             }
> 1145:             Map<Integer, Image> multiResolutionIcon = new HashMap<>();
> 1146:             int start = size > MAX_QUALITY_ICON ? ICON_RESOLUTIONS.length - 1 : 0;

Does it make sense to always start at zero?
The icons of smaller size will never be used, will they?
Thus it's safe to start at the index which corresponds to the requested size if `size` matches, or the index such as `ICON_RESOLUTIONS[index] < size && ICON_RESOLUTIONS[index + 1] > size`.

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

> 1147:             int increment = size > MAX_QUALITY_ICON ? -1 : 1;
> 1148:             int end = size > MAX_QUALITY_ICON ? -1 : ICON_RESOLUTIONS.length;
> 1149:             for (int i = start; i != end; i+=increment) {

Suggestion:

            for (int i = start; i != end; i += increment) {

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

> 1420:             int w = (int) width;
> 1421:             int retindex = 0;
> 1422:             for (Integer i: resolutionVariants.keySet()) {

Suggestion:

            for (Integer i : resolutionVariants.keySet()) {

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

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