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

Semyon Sadetsky semyon.sadetsky at oracle.com
Mon Jul 11 09:13:26 UTC 2016


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/ 
> <http://cr.openjdk.java.net/%7Erchamyal/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/
>     <http://cr.openjdk.java.net/%7Erchamyal/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 <mailto: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/
>         <http://cr.openjdk.java.net/%7Erchamyal/8159168/webrev.02/>
>
>         Regards,
>
>         Rajeev Chamyal
>
>         *From:*Alexandr Scherbatiy
>         *Sent:* 05 July 2016 11:38
>         *To:* Rajeev Chamyal; swing-dev at openjdk.java.net
>         <mailto: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/
>             <http://cr.openjdk.java.net/%7Erchamyal/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
>             <mailto: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/2f41e46c/attachment.html>


More information about the swing-dev mailing list