Re: [10] Review request for 8182043: Access to Windows Large Icons

2017-09-28 Thread Sergey Bylokhov
On 9/28/17 10:57, Semyon Sadetsky wrote: Small and large don't have any special meanings for HiDPI. They are some conditional sizes established by the native platform for the current screen resolution. The question what is the current screens resolution when you have two screens? We should

Re: [10] Review request for 8182043: Access to Windows Large Icons

2017-09-28 Thread Philip Race
We definitely can't backport an API change. Reading the bug report it seems they have been doing this by using internal APIs. For JDK 9, since illegal-access is allowed by default, they can continue to do that, so perhaps we can just not worry about the backport and solve this only for 10 +

Re: [10] Review request for 8182043: Access to Windows Large Icons

2017-09-28 Thread Semyon Sadetsky
On 9/28/2017 10:51 AM, Philip Race wrote: If this is up for consideration for backporting - as appears to be the case - then I think you should post an updated webrev. Not sure that we can backport it because this would change the API. --Semyon -phil.

Re: [10] Review request for 8182043: Access to Windows Large Icons

2017-09-28 Thread Semyon Sadetsky
Hi Sergey, On 9/27/2017 12:22 PM, Sergey Bylokhov wrote: On 9/26/17 14:37, Semyon Sadetsky wrote: This means that on HiDPI screen the FILE_ICON_LARGE works in similar way as FILE_ICON_SMALL on non-HiDPI screen, and the meaning of the FILE_ICON_SMALL on HiDPI is unclear, because it is half of

Re: [10] Review request for 8182043: Access to Windows Large Icons

2017-09-28 Thread Philip Race
If this is up for consideration for backporting - as appears to be the case - then I think you should post an updated webrev. -phil. On 9/28/17, 10:41 AM, Semyon Sadetsky wrote: Hi Alexey, On 9/28/2017 7:28 AM, Alexey Ivanov wrote: I think 0 should really be |NULL|. Ok, but both represent

Re: [10] Review request for 8182043: Access to Windows Large Icons

2017-09-28 Thread Alexey Ivanov
Sure! -- Alexey On 28/09/2017 18:41, Semyon Sadetsky wrote: Hi Alexey, On 9/28/2017 7:28 AM, Alexey Ivanov wrote: 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…

Re: [10] Review request for 8182043: Access to Windows Large Icons

2017-09-28 Thread Semyon Sadetsky
Hi Alexey, On 9/28/2017 7:28 AM, Alexey Ivanov wrote: 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. Agh... I will fix it

Re: [10] Review request for 8155197: Focus transition issue

2017-09-28 Thread Semyon Sadetsky
Hi Dmitry and Alexey, On 9/28/2017 8:14 AM, Dmitry Markov wrote: I have updated the regression test for the fix. The new version is located at http://cr.openjdk.java.net/~dmarkov/8155197/webrev.01/ Now the test is failed on the build

Re: [10] Review request for 8155197: Focus transition issue

2017-09-28 Thread Dmitry Markov
I have updated the regression test for the fix. The new version is located at http://cr.openjdk.java.net/~dmarkov/8155197/webrev.01/ Now the test is failed on the build without fix and passed on the build with the fix. Tested on Mac and Windows. Many thanks to Alexey for his help with testing

Re: [10] Review request for 8182043: Access to Windows Large Icons

2017-09-28 Thread 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, , *0*, sz); I think 0 should really be |NULL|. Ok, but both represent the null pointer in Win32. Yes, I know. Yet NULL