Re: Review Request JDK-8159168 [hidpi] Window.setShape() works incorrectly on HiDPI

[Semyon Sadetsky]

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@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@openjdk.java.net <mailto:swing-dev@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@openjdk.java.net
        <mailto:swing-dev@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@openjdk.java.net
            <mailto:swing-dev@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