[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering
From:       Prahalad Kumar Narayanan <prahalad.kumar.narayanan () oracle ! com>
Date:       2017-11-08 4:05:33
Message-ID: 009f9dfe-5cc1-4973-9d64-783a56bc6f10 () default
[Download RAW message or body]

Hello Pankaj

The changes look fine to me. 
The back buffer -BufferedImage here is scaled and Graphics2D corresponding to the \
back buffer is appropriately transformed.

Just a minor correction. Doesn't require another webrev:
. Kindly edit multi-line comments at Line: 254 to reflect the coding guideline
/*
 * Start of multi-line comments
 */

Thanks
Have a good day

Prahalad
 
-----Original Message-----
From: Sergey Bylokhov 
Sent: Wednesday, November 08, 2017 2:04 AM
To: Pankaj Bansal; Philip Race; Prem Balakrishnan
Cc: 2d-dev@openjdk.java.net; swing-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [10] Review Request JDK - 8164811 : [hidpi]Tests fail \
with OpenGL Rendering

Looks fine, thank you!

On 07/11/2017 05:09, Pankaj Bansal wrote:
> Hi Sergey,
> 
> << One more thing which can improve the fix is to add "uiScale > 1.0" as an \
> additional @run step to the tests, so these tests will fail before the fix even on \
> common lowdpi(100%) systems. I have updated the webrev for the changes.
> 
> I will like to point out that, there are 9 test cases mentioned in the bug. But \
> only 5 fail without fix.  4 tests pass because of following reasons. 
> 1. SetShapeAndClickSwing.java,  TranslucentJComboBox.java,  
> TranslucentWindowClickSwing.java These tests don't use the TranslucentWindowPainter \
> as in these tests, the PerPixelTransparency which means the alpha of the window is \
> set 1.0. Instead these tests change the window Opacity which is handled \
> differently. So in these tests are scaled properly and don't fail without the fix. 
> 2.  PerPixelTranslucentSwing.java
> This test passes as there is a problem in the way test is written. It is trying to \
> check the transparency of window by comparing the robot picked color with \
> Foreground color. But with HIDPI, the color value is picked from wrong location, \
> but still the test passes as the picked value is not equal to the Foreground color. \
> So we should check the color value with Background color. I was not very sure if I \
> should fix this in this webrev, so I have created a separate issue for this. \
> https://bugs.openjdk.java.net/browse/JDK-8190861. I will fix this test differently. \
>  So I have added the additional @run with uiScale in 5 tests that fail 
> without fix and PerPixelTranslucentSwing.java as this is a test 
> problem and this test fails when corrected
> 
> Webrev:
> http://cr.openjdk.java.net/~pbansal/8164811/webrev.03/
> 
> 
> Regards,
> Pankaj Bansal
> 
> 
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Tuesday, November 7, 2017 1:45 PM
> To: Pankaj Bansal; Philip Race; Prem Balakrishnan
> Cc: swing-dev@openjdk.java.net; 2d-dev@openjdk.java.net
> Subject: Re: [OpenJDK 2D-Dev] [10] Review Request JDK - 8164811 : 
> [hidpi]Tests fail with OpenGL Rendering
> 
> Thanks for clarification. Looks fine.
> One more thing which can improve the fix is to add "uiScale > 1.0" as an additional \
> @run step to the tests, so these tests will fail before the fix even on common \
> lowdpi(100%) systems. 
> On 06/11/2017 23:20, Pankaj Bansal wrote:
> > I am assuming you are talking about changes in UpdateWindow function in \
> > TranslucentPainter. I think it is doing exactly what was happening before the \
> > fix. It called getBackBuffers and saved it in bb initially and then,  If the \
> > update fails, "done" will be false and it was calling getBackBuffers (true) to \
> > save the buffer in bb everytime. So it was calling getBackBuffers() everytime the \
> > update fails. Also the createBackBuffers function is creating backbuffer only \
> > once and then just returning it if the width and height remains same. So when I \
> > call update(getBackBuffers(false)), it will just return the backBuffers, not \
> > create it. So this change has added a call to getBackBuffers, but without it, I \
> > will have to add a check to see if the Image is BufferedImage or VolatileImage to \
> > decide to transform the Graphics or not.


--
Best regards, Sergey.


[prev in list] [next in list] [prev in thread] [next in thread]