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

[Alexey Ivanov]

Hi Semyon,

On 26/09/2017 21:58, Semyon Sadetsky wrote:
> Hi Alexey,
>
> On 9/26/2017 12:29 PM, Alexey Ivanov wrote:
>> Hi Semyon,
>>
>> ShellFolder2.cpp
>> 944 pIcon->Extract(szBuf, index, &hIcon, *0*, sz);
>> I think 0 should really be |NULL|.
> Ok, but both represent the null pointer in Win32.

Yes, I know. Yet NULL makes it clear that it's a null-pointer rather 
than an index, size…
It's still zero in the latest review.

>
>> The result of the call is ignored now. Is it intentional?
> Yes, it has been working the same way before the fix. The function 
> returns the Icon reference which is 0 in case of OS API call error.
>
>>
>> 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?
> Ok.
>>
>> |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)|?
> It has additional difference because of query for a fixed icon size 
> from another API call.  It's better to leave it as it is.

Okay.


Regards,
Alexey

>
> http://cr.openjdk.java.net/~ssadetsky/8182043/webrev.02/
>
> --Semyon
>>
>>
>> Regards,
>> Alexey
>>
>> On 22/09/2017 22:05, Semyon Sadetsky wrote:
>>>
>>> <SNIP>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20170928/1ae71498/attachment.html>