<AWT Dev> <Swing Dev> [10] Review request for 8182043: Access to Windows Large Icons
Alexey Ivanov
alexey.ivanov at oracle.com
Tue Sep 26 19:29:06 UTC 2017
Hi Semyon,
ShellFolder2.cpp
944 pIcon->Extract(szBuf, index, &hIcon, *0*, sz);
I think 0 should really be |NULL|.
The result of the call is ignored now. Is it intentional?
Win32ShellFolder2.java
1010 private static Image makeIcon(long hIcon, int bsize) {
|bsize| was called |baseSize| previously, and it conveyed the meaning
better, didn't it?
1043 if(hIcon <= 0) {
1044 if (isDirectory()) {
1045 return getShell32Icon(4, size);
1046 } else {
1047 return getShell32Icon(1, size);
1048 }
I guess I understand what hides behind 4 and 1: generic folder and
generic file icon ids. Would declaring these numbers as constants
improve readability?
|getIcon(int size)| and |getIcon(boolean getLargeIcon)| are somewhat
similar. The latter provides additional caching. Can it be simplified to
re-use portions of new |getIcon(int size)|?
Regards,
Alexey
On 22/09/2017 22:05, Semyon Sadetsky wrote:
>
> Hi Alexey,
>
> On 09/22/2017 01:01 PM, Alexey Ivanov wrote:
>> Hi Semyon,
>>
>> On 22/09/2017 20:06, Semyon Sadetsky wrote:
>>>
>>> Hi Alexey,
>>>
>>> On 09/22/2017 10:53 AM, Alexey Ivanov wrote:
>>>> Hi Semyon,
>>>>
>>>> On 22/09/2017 18:29, Semyon Sadetsky wrote:
>>>>> Hi Alexey,
>>>>>
>>>>> On 09/22/2017 09:43 AM, Alexey Ivanov wrote:
>>>>>> Hi Semyon,
>>>>>>
>>>>>> On 22/09/2017 17:13, Semyon Sadetsky wrote:
>>>>>>> Hi Alexey,
>>>>>>>
>>>>>>> Thank you for your exact clarification.
>>>>>>>
>>>>>>> On 09/22/2017 04:22 AM, Alexey Ivanov wrote:
>>>>>>>> <SNIP>
>>>>>>>>
>>>>>>>> As for FILE_ICON_SMALL and FILE_ICON_LARGE, I'd suggest using
>>>>>>>> Windows API to retrieve the recommended size for small and
>>>>>>>> large icon size rather than defaulting to 16×16 and 32×32. If
>>>>>>>> HiDPI is in effect, the icons must be larger.
>>>>>>> I also found this as most suitable approach for the moment.
>>>>>>> Later this may be changed, for example, if Swing JFC is
>>>>>>> re-factored to support shell determined icon sizes at HiDPI.
>>>>>>
>>>>>> Swing UI scales to accommodate HiDPI settings. If fonts are
>>>>>> larger then icons should be larger too. Otherwise icons are too
>>>>>> small compared to surrounding text.
>>>>>>
>>>>>> Anyway it could be postponed to a later fix.
>>>>>>
>>>>>> Does it make sense to declare the standard sizes of 16×16 and
>>>>>> 32×32 as constants at least in Java sources? This way, it would
>>>>>> be easier to find the places in code where a change is necessary.
>>>>> This topic requires more investigations. At first, we need to keep
>>>>> the API cross-platform and this requires comparing all supported
>>>>> platforms in details. At the second, even for the existing windows
>>>>> implementation there is an ambiguity in icons sizes received form
>>>>> the OS shell. Windows platform has number of predefined constants
>>>>> to query icon sizes (small, large, extra large, jumbo...) but
>>>>> their actual size may differ depending on the shell preferences.
>>>>> Those icon sizes may be changed by Windows registry settings and
>>>>> may depend on the hi-res scale. I did several experiments and
>>>>> found that depending on the way of desktop scaling in Windows 10
>>>>> (it has two ways the new one and the old) at the same scale 2x the
>>>>> returned large icon, for example, may be 32 or 64 pixels in size
>>>>> (this was the root cause of the 8151385 bug).
>>>>> I would postpone digging in this direction because we are not
>>>>> planning to update Swing JFC dialog for better HiDPI view in the
>>>>> nearest future. Also,we don't have statistics how users may use
>>>>> the API. Since that, the most flexible API that leaves to the user
>>>>> the decision about icon size to query seems more preferable.
>>>>
>>>> I totally agree with your points. And the new API provides means
>>>> for app developer to choose better-suited size for icons.
>>>>
>>>> What about using constants, private ones, for the two standard
>>>> sizes instead of using “magic” numbers?
>>> Which constants do you mean? The next for large and small sizes?
>>>
>>> public static final int FILE_ICON_SMALL = -1;
>>> public static final int FILE_ICON_LARGE = -2;
>>> they cannot be private because they are part of the new API that is
>>> requested in the bug. The bug asks enabling the query for the large
>>> icon that ShellFolder had been providing before. The bug itself is
>>> not related to HiDPI. The possibility to get arbitrary icon sizes
>>> was added because after 9 this may be in demand for HiDPI UIs.
>>
>> I'm talking about these two numbers in Win32ShellFolder2.java as an
>> example:
>>
>> public Image getIcon(final boolean getLargeIcon) {
>> int size = getLargeIcon ? *32* : *16*;
>>
>> Does it make sense to declare constants for the size of 16 and 32. So
>> that the places where they're used are more easily identified if
>> someone decides to update the code later for supporting system icon size.
> I updated the fix.
>
> http://cr.openjdk.java.net/~ssadetsky/8182043/webrev.01/
>
> --Semyon
>>
>>
>> Regards,
>> Alexey
>>
>>>
>>> --Semyon
>>>
>>>> Other than that, the fix looks good to me.
>>>>
>>>>
>>>> Regards,
>>>> Alexey
>>>>
>>>>>
>>>>>
>>>>> --Semyon
>>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> Alexey
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --Semyon
>>>>>>>>
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Alexey
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> <SNIP>
>>>>>>>>>>>
>>>>>>>>>>>>> On 9/13/17 11:01, Semyon Sadetsky wrote:
>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Please review fix for JDK10 (the changes involve AWT and
>>>>>>>>>>>>>> Swing):
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8182043
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> webrev:
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ssadetsky/8182043/webrev.00/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The fix opens the part of the ShellFolder API for
>>>>>>>>>>>>>> getting system icons which was decided to be left closed
>>>>>>>>>>>>>> during the 8081722 enhancement review in 9.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Also the fix extends the API by adding possibility to
>>>>>>>>>>>>>> query file icons of arbitrary size and implements this
>>>>>>>>>>>>>> extension for Windows platform.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20170926/8a60833f/attachment.html>
More information about the awt-dev
mailing list