<Swing Dev> <AWT Dev> [10] Review request for 8182043: Access to Windows Large Icons

[Alexey Ivanov]

Hi Semyon,

On 06/10/2017 21:33, Semyon Sadetsky wrote:
> Hi Alexey,
>
> On 10/06/2017 11:42 AM, Alexey Ivanov wrote:
>> Hi Semyon, Sergey,
>>
>> On 30/09/2017 00:08, Semyon Sadetsky wrote:
>>> On 9/29/2017 3:15 PM, Sergey Bylokhov wrote:
>>>> On 9/29/17 12:39, Semyon Sadetsky wrote:
>>>>>>> Why 128 pixels? Windows shell usually provides icons up to 256 
>>>>>>> pixels, for example there are 256×256 icons for folders and 
>>>>>>> generic file type.
>>>>>>
>>>>>> It is limitation of our implementation:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8151385
>>>>>> http://mail.openjdk.java.net/pipermail/awt-dev/2016-March/010777.html 
>>>>>>
>>>>>>
>>>>> Sergey, it is not clear how those links are related to the icon 
>>>>> size returned by Windows?
>>>>
>>>> It was a fix where the MAX_ICON_SIZE=128 was added.
>>> Actually it limits nothing. We told about the Extract call which may 
>>> return any size.
>>
>> Yes, it does. It limits the size of the returned icon to 128×128.
>> I guess if a larger icon is requested, then we'll get a distorted image.
> This is artificial limitation when the image is transfered from native 
> to java. WinAPI may return bigger images without any issues.

Yes, I understand it.
Yet, it's still a limitation. With the current implementation, the 
caller of the API cannot get icon larger than 128×128.

>>>>>> As far as I understand the bug above, it is possible that OS 
>>>>>> returns some other size.
>>>>> You've probably didn't understand what Alexey meant. The Extract 
>>>>> call may return any size you request (it does scaling internally 
>>>>> if there are no suitable image) > But the bug above is about 
>>>>> queering the fixed size
>>>>> (small or long) which size is determined by OS shell according to 
>>>>> the current scale. For those fixed sizes we use SHGetFileInfo not 
>>>>> the Extract.
>>>>
>>>> And every time we will try to make an icon it will be limited to 
>>>> 128x128. But it is not critical.
>>>>
>>>> The issue is that this api, as you said, will depends from some 
>>>> general "current scale". which is unrelated to the transform of the 
>>>> screen in java.
>>>>
>>>> If the user will want to use FILE_ICON_LARGE, then to work properly 
>>>> he will need to use this code every time in the the paint():
>>>> Icon icon = getSystemIcon(file, FILE_ICON_LARGE);
>>>> Icon hicon = getSystemIcon(file, 
>>>> icon.getIconWidth()*currentScreenScale);
>>> This is just wrong. The first line is the correct one for both HiDPI 
>>> and nonHiDPI. If you want to have icons like in native apps. For 
>>> custom behavior - please use the second line.
>>>
>>
>> Why is it wrong?
>> getSystemIcon(file) requests FILE_ICON_SMALL from the OS, then all 
>> Java has to paint at any DPI scale is 16×16 icon.
>>
>> Or am I missing anything?
> Sorry, I did not get how is the small icon related to the code above. 
> Probably we understood it differently because the explanation is not  
> the best. My interpretation is:
> For the low DPI screen one should use icon=getSystemIcon(file, 
> FILE_ICON_LARGE) when the window is moved to hiDPI screen the 
> hicon=getSystemIcon(file, icon.getIconWidth()*currentScreenScale) 
> should be used. And this approach is wrong.
>
> The primary purpose of the current fix is to fix the compatibility 
> issue we got when we closed shell folder API in 9.
> The user code which doesn't work in 9 should not be changed in the way 
> proposed by Sergey. This code should be updated to use 
> getSystemIcon(file, FILE_ICON_LARGE) instead of closed getIcon(true) 
> and getSystemIcon(file, FILE_ICON_SMALL) instead of closed 
> getIcon(false).
> The newly written code may use getSystemIcon(file, 
> FILE_ICON_LARGE/SMALL) to get the icon in the native platform which 
> determines its size at the current DPI (DPI-unaware usage)
> or getSystemIcon(file, size) to have any custom size which can be 
> multiplied by DPI. I see no reason to use both approaches 
> simultaneously  at any circumstances.
>
> --Semyon

Thank you Semyon for your explanation.

I think I understand it better now. In either case, Swing components 
should use the size in component coordinates. In case of JFileChooser 
file view, it's FILE_ICON_SMALL.

I didn't expect native OS, Windows in my case, to adjust icon resolution 
automatically to account for HiDPI scaling because documentation for 
IExtractIcon::Extract [1] does not mention that such scaling is performed.

However, my testing [2] with opening JFileChooser in SwingDemo shows 
that the scaling is performed, at least in some cases.


Overall, the fix looks fine. It resolves the stated problem that is it 
provides access to larger icons via public API.

The issues with the current icon size limitation to 128 and with HiDPI 
support, if any, can be resolved later under separate bug ids.

Does it sound sensible?


Regards,
Alexey

[1] 
https://msdn.microsoft.com/en-us/library/windows/desktop/bb761850%28v=vs.85%29.aspx
[2] http://mail.openjdk.java.net/pipermail/awt-dev/2017-October/013181.html