Hi, Alexander.

Some generic questions about the general approach:
 - Did you experiment using LookupIconIdFromDirectoryEx? This method was 
referred in the previous email.
 - Did you check how the performance will be affected if we always load a few 
icons, all icons, or if we try to load them on
   the fly when the user request some size from the MRI?
 - As far as I understand to access the list of icons the user will need to use 
instanceof MRI, can we improve it somehow?
   Probably add example to the javadoc, and to the test?
 - What about returning the array of icons, will that solution be 
simpler/faster then using MRI or not?
 - Did you try to merge implementation of 
getSystemIcon(File)/getSystemIcon(File, size)?
 - I think we need to improve the spec for the new method:
   * "width and height of the icon in pixels to be scaled" -  what does it mean "to 
be scaled", is
     the returned icon will always have this size in pixels?
   * "Whenever possible the icon returned will be a multi-resolution icon image
      with maximum quality icon provided by the system and the size stored
      reflecting the size requested by the user. That will allow nice scaling
      with differen magnification factors." Does our implementation follow this 
spec?

On 6/9/20 2:24 pm, Alexander Zuev wrote:
   while generally agree i'm not sure that we need to populate ALL of the 
resolutions for every sing> icon we request. So my idea that you can see in the 
new webrev is to create the true MRI which will
hold all standatd resolutions within the range of half of requested icon size 
and double of it. That would
cover magnification factors from 50% to 200%. I tested it and the icons in the 
standard JFioleChooser looks
pretty good on display with different magnification factors.

 - Looks like in the fix we always use the "size" passed by the user when we 
call getShell32Icon() so we
always fetch the same icons from the native?
 - The change in the makeIcon reverts back the fix for JDK-8151385, can you 
please confirm that those fix
   still works as expected?
 - The new parameter "size" in the makeIcon(long hIcon, int size) is unused.

--
Best regards, Sergey.

Reply via email to