<Swing Dev> [12] JDK-8182043: Access to Windows Large Icons

[Prasanta Sadhukhan]

Hi Shashi,

The previous review thread is too long so probably need the original 
reviewer to take a look.
But here are my 2 cents

  * Can you please explain how the new API is supposed to be used? It
    says "Scaled icon for a file, directory, or folder as it would be
    displayed ina system file browser"

                 getSystemIcon(File f, int width, int height)
             If the specified width, height is not available, then it 
will be scaled to available icon size, is it? I do not think the javadoc 
captures the essence of the api thoroughly.
             For example, if width/height asked for is 100 and we have 
icon of 96/96 and 128/128, then what it will return, the closest size 
96/96 or the next positive one 128/128.

  * As per fix of /JDK-8151385
    <https://bugs.openjdk.java.net/browse/JDK-8151385>/,
    /MAX_ICON_SIZE=128 /was added so even if you accept width/height as
    256 (by your check it will still restrict icon size to 128)...

287 if((width > 256 || width < 1) || (height > 256 || height < 1)) {
288 System.err.println("Warning: Icon scaling may be distorted");
289 throw new IllegalArgumentException("unexpected icon scaling size");
290 }

             so maybe either increase MAX_ICON_SIZE or adjust your check 
accordingly.

  * We normally do not throw unchecked exception (RuntimeException) from
    public API, so you should consider throwing checked exception instead.

  * Also, you need to change javadoc to have @throws instead of @exception

  * Also, it was asked, IIUC that the public API used
    MutliResolutionImage and uses |getResolutionVariants
    <https://docs.oracle.com/javase/10/docs/api/java/awt/image/MultiResolutionImage.html#getResolutionVariants%28%29>()
    to select appropriate icon image or let user select it (for case
    mentioned above, if we are not sure ourselves)|
  *

    scaleIconImage, scaleIcon uses lot of common code so we can have a
    common method

  * makeIcon(long hIcon, int bsize).. I guess parameter name bsize does
    not convey meaning, is it "bestsize"?
  *

    int size = getLargeIcon ? 32 : 16; and also in
    Win32ShellFolderManager2. java probably you can have constants for
    these 2 number

  *

    Win32ShellFolderManager2 1118 return getShell32Icon(4, size);
    1119                     } else {
    1120 return getShell32Icon(1, size);

    You should use constant to be more readable FILE_ICON_ID =
    1;FOLDER_ICON_ID = 4;

Regards
Prasanta
||
On 7/20/2018 12:02 PM, Shashidhara Veerabhadraiah wrote:
>
> Hi All, Please review a fix for the below bug.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8182043
>
> Webrev: http://cr.openjdk.java.net/~sveerabhadra/8182043/webrev.00/ 
> <http://cr.openjdk.java.net/%7Esveerabhadra/8182043/webrev.00/>
>
> History: This fix is derived from an earlier fix proposed under this 
> mail thread: 
> http://mail.openjdk.java.net/pipermail/awt-dev/2017-September/013016.html. 
> Thanks to Semyon for that trial and part of this solution is continued 
> from where it was left off.
>
> Solution details: Large system icons were not able to be extracted 
> because of sun package internal classes are not exposed anymore. This 
> change adds a new api to get access to those icons.
>
> Thanks and regards,
>
> Shashi
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20180720/edf5dcd0/attachment.html>