<Swing Dev> Review Request JDK-8159168 [hidpi] Window.setShape() works incorrectly on HiDPI

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 7/11/2016 12:13 PM, Semyon Sadetsky wrote:
>
> Looks good to me.
>
> --Semyon
>
> On 7/11/2016 12:08 PM, Rajeev Chamyal wrote:
>>
>> Hello Semyon,
>>
>> Please review the updated webrev as per review comments.
>>
>> http://cr.openjdk.java.net/~rchamyal/8159168/webrev.04/
>>
>> Regards,
>>
>> Rajeev Chamyal
>>
>> *From:*Semyon Sadetsky
>> *Sent:* 11 July 2016 14:29
>> *To:* Rajeev Chamyal; Alexander Scherbatiy; 
>> swing-dev at openjdk.java.net; Sergey Bylokhov
>> *Subject:* Re: <Swing Dev> Review Request JDK-8159168 [hidpi] 
>> Window.setShape() works incorrectly on HiDPI
>>
>> On 7/11/2016 11:10 AM, Rajeev Chamyal wrote:
>>
>>     Hello Semyon,
>>
>>     Thanks for the review. Yes, mouse move is not required I have
>>     removed it.
>>
>>     Please review the updated webrev.
>>
>>     http://cr.openjdk.java.net/~rchamyal/8159168/webrev.03/
>>
>> Thanks.
>> I also cannot see the reason to wait 1 second in line 59.
>> It seems tx variable from line 53 is never used.
>>
>> --Semyon
>>
>>     Regards,
>>
>>     Rajeev Chamyal
>>
>>     *From:*Semyon Sadetsky
>>     *Sent:* 11 July 2016 12:30
>>     *To:* Rajeev Chamyal; Alexander Scherbatiy;
>>     swing-dev at openjdk.java.net; Sergey Bylokhov
>>     *Subject:* Re: <Swing Dev> Review Request JDK-8159168 [hidpi]
>>     Window.setShape() works incorrectly on HiDPI
>>
>>     Hi Rajeev,
>>
>>     The fix itself looks good. I only did not get for what purpose
>>     mouse pointer is moved in the test?
>>
>>     --Semyon
>>
>>     On 7/5/2016 9:16 AM, Rajeev Chamyal wrote:
>>
>>         Hello Alexandr,
>>
>>         Please review updated webrev.
>>
>>         http://cr.openjdk.java.net/~rchamyal/8159168/webrev.02/
>>
>>         Regards,
>>
>>         Rajeev Chamyal
>>
>>         *From:*Alexandr Scherbatiy
>>         *Sent:* 05 July 2016 11:38
>>         *To:* Rajeev Chamyal; swing-dev at openjdk.java.net; Sergey Bylokhov
>>         *Subject:* Re: <Swing Dev> Review Request JDK-8159168 [hidpi]
>>         Window.setShape() works incorrectly on HiDPI
>>
>>         On 7/5/2016 8:25 AM, Rajeev Chamyal wrote:
>>
>>
>>
>>             Hello Alexandr,
>>
>>             Thanks for the review.
>>
>>             As per windows specification X & Y scale are always equal
>>             that’s why I have put scaleX == scaleY check.
>>
>>             But it may change in future so I have removed this check.
>>
>>             http://cr.openjdk.java.net/~rchamyal/8159168/webrev.01/
>>
>>
>>         1135             if (scaleX != 1 && scaleY != 1) {
>>
>>           It is better to use 'or' operator because the shape should
>>         be scaled when on of the scales is differ from 1.
>>
>>         Thanks,
>>         Alexandr.
>>
>>
>>
>>
>>             Regards,
>>
>>             Rajeev Chamyal
>>
>>             *From:*Alexandr Scherbatiy
>>             *Sent:* 04 July 2016 18:03
>>             *To:* Rajeev Chamyal; swing-dev at openjdk.java.net; Sergey
>>             Bylokhov
>>             *Subject:* Re: <Swing Dev> Review Request JDK-8159168
>>             [hidpi] Window.setShape() works incorrectly on HiDPI
>>
>>             On 7/4/2016 3:09 PM, Rajeev Chamyal wrote:
>>
>>
>>
>>
>>                 Hello All,
>>
>>                 Please review the following webrev.
>>
>>                 Bug: https://bugs.openjdk.java.net/browse/JDK-8159168
>>
>>                 Webrev:
>>                 http://cr.openjdk.java.net/~rchamyal/8159168/webrev.00/
>>                 <http://cr.openjdk.java.net/%7Erchamyal/8159168/webrev.00/>
>>
>>
>>                 Issue: In HiDPI screen shape set through
>>                 window::setShape API is not scaled based on system scale.
>>
>>                 Fix:. Updated the WComponentPeer::applyShape to
>>                 update shape based on system scale.
>>
>>             1131 double scaleX =
>>             winGraphicsConfig.getDefaultTransform().getScaleX();
>>             1132             double scaleY =
>>             winGraphicsConfig.getDefaultTransform().getScaleY();
>>
>>              The getDefaultTransform() is called twice which leads
>>             that AffineTransform object is created two times
>>             1133             if (scaleX != 1 && scaleY != 1 && scaleX
>>             == scaleY) {
>>
>>                Is the check scaleX == scaleY really necessary here?
>>
>>                Is it possible to make the test automated? Just run it
>>             with option "@run main/othervm -Dsun.java2d.uiScale=2
>>             TestName" and check the area where the shape is drawn?
>>
>>               Thanks,
>>               Alexandr.
>>
>>
>>
>>
>>                 Regards,
>>
>>                 Rajeev Chamyal
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160711/5f660983/attachment.html>