[10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

Pankaj Bansal

Hi All,

 

Please review the fix for JDK 10.

 

Bug:

https://bugs.openjdk.java.net/browse/JDK-8164811

 

Webrev:

 http://cr.openjdk.java.net/~pbansal/8164811/webrev.00/

 

Issue:

The tests given in the bug were failing when run with OpenGL and GDI.  This bug is related to https://bugs.openjdk.java.net/browse/JDK-8189257 which states that that HIDPI does not work with swing components when Translucent window is used. Because of which all the tests in the bug were failing.

 

Fix:

The TranslucentWindowPainter class was creating BufferedImage for OpenGL (when forceOpt is false) and GDI pipeline, but it is not considering the device HiDPI scale. There is no way to create a scaled BufferedImage because of which the scale value in BufImgSurfaceData is always 1. Made changes to store graphics config in Buffered image, so that the BufImgSurfaceManager can create BufImgSurfaceData with scale set properly.

 

This fix also fixes  https://bugs.openjdk.java.net/browse/JDK-8189257

 

Regards,

Pankaj Bansal

 

Reply | Threaded
Open this post in threaded view
|

Re: [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

Sergey Bylokhov
Hi, Pankaj.
The BufferedImage class is a raster in the memory which should be
unrelated to any grpahics config on the system. So this class should not
be changed, but instead you should create a correct BI-as a back buffer
which takes into account the size of the window and the scale of the GC.

On 31/10/2017 04:59, Pankaj Bansal wrote:

> Hi All,
>
> Please review the fix for JDK 10.
>
> Bug:
>
> https://bugs.openjdk.java.net/browse/JDK-8164811
>
> Webrev:
>
> http://cr.openjdk.java.net/~pbansal/8164811/webrev.00/
>
> Issue:
>
> The tests given in the bug were failing when run with OpenGL and GDI.  
> This bug is related to https://bugs.openjdk.java.net/browse/JDK-8189257 
> which states that that HIDPI does not work with swing components when
> Translucent window is used. Because of which all the tests in the bug
> were failing.
>
> Fix:
>
> The TranslucentWindowPainter class was creating BufferedImage for OpenGL
> (when /forceOpt/ is false) and GDI pipeline, but it is not considering
> the device HiDPI scale. There is no way to create a scaled BufferedImage
> because of which the scale value in BufImgSurfaceData is always 1. Made
> changes to store graphics config in Buffered image, so that the
> BufImgSurfaceManager can create BufImgSurfaceData with scale set properly.
>
> This fix also fixes https://bugs.openjdk.java.net/browse/JDK-8189257
>
> Regards,
>
> Pankaj Bansal
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

Pankaj Bansal
Hi Sergey,

I have created the BufferedImage of correct size considering the window dimension and graphics context scale.

I changed BufferedImage just to store graphicsConfig, so that it can be used by BufImgSurfaceManager while creating BufImgSurfaceData. The scale in BufImgSurfaceData is used to create config while creating the SunGraphics2D through getDefaultTransform function. Now when the TranslucentWindowPainter create scaled BufferedImage and then creates Graphics2D using that, the Graphics2D has transform scale as (1,1) as the BufImgSurfaceData has scale (1,1). Then this Graphics2D is used to update the Window in  updateWindow in TranslucentWindowPainter. As the transform is identity, none of the Swing Components which are added to the window are scaled.
So I modified the BufferedImage to store graphicsConfig which can then be used by BugImgSurfaceManager to create BugImgSurfaceData with correct scale values and hence correct transform value in SunGraphics2D.

If this is not the correct way, I have one more way of doing this by transforming the SunGraphics2D object with transform of peer. So the SunGraphics2D will have correct transform  and objects are properly scaled. I have updated the webrev for the same. Please have a look. Also if this is also not the correct way, can you please suggest a way I can do this?

Webrev:
http://cr.openjdk.java.net/~pbansal/8164811/webrev.01/


Regards,
Pankaj Bansal



-----Original Message-----
From: Sergey Bylokhov
Sent: Friday, November 3, 2017 5:34 AM
To: Pankaj Bansal; Prem Kumar Balakrishnan ([hidden email]); [hidden email]; [hidden email]
Subject: Re: [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

Hi, Pankaj.
The BufferedImage class is a raster in the memory which should be unrelated to any grpahics config on the system. So this class should not be changed, but instead you should create a correct BI-as a back buffer which takes into account the size of the window and the scale of the GC.

On 31/10/2017 04:59, Pankaj Bansal wrote:

> Hi All,
>
> Please review the fix for JDK 10.
>
> Bug:
>
> https://bugs.openjdk.java.net/browse/JDK-8164811
>
> Webrev:
>
> http://cr.openjdk.java.net/~pbansal/8164811/webrev.00/
>
> Issue:
>
> The tests given in the bug were failing when run with OpenGL and GDI.  
> This bug is related to
> https://bugs.openjdk.java.net/browse/JDK-8189257
> which states that that HIDPI does not work with swing components when
> Translucent window is used. Because of which all the tests in the bug
> were failing.
>
> Fix:
>
> The TranslucentWindowPainter class was creating BufferedImage for
> OpenGL (when /forceOpt/ is false) and GDI pipeline, but it is not
> considering the device HiDPI scale. There is no way to create a scaled
> BufferedImage because of which the scale value in BufImgSurfaceData is
> always 1. Made changes to store graphics config in Buffered image, so
> that the BufImgSurfaceManager can create BufImgSurfaceData with scale set properly.
>
> This fix also fixes https://bugs.openjdk.java.net/browse/JDK-8189257
>
> Regards,
>
> Pankaj Bansal
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

Phil Race
Adding new API to BufferedImage was definitely NOT the way to approach this.
Adding new public API we need to support forever to fix an internal
implementation bug
on one platform in a very atypical scenario is the wrong trade-off.

And anyway as Sergey said BI should have no dependency or relationship
on a BI

TranslucentWindowPainter already had the GraphicsConfiguration anyway so it does not seem necessary
and you seem to be more along the right lines with this fix but I'd like to try it ..

Couple of questions :
Why are none of the failing tests referenced with an updated reg-test @bug tag to indicate this
fix corrects them ?

You say it is observable with GDI .. the bug report refers only to OpenGL.
So is it both ?

If so you don't need to enable the flaky OGL pipeline to verify this .. but then
I am left puzzled since Intel cards don't use D3D so I'd expect more reports.

So does it reproduce with just
-Dsun.java2d.d3d=false

??

-phil.



On 11/3/17, 6:44 AM, Pankaj Bansal wrote:

> Hi Sergey,
>
> I have created the BufferedImage of correct size considering the window dimension and graphics context scale.
>
> I changed BufferedImage just to store graphicsConfig, so that it can be used by BufImgSurfaceManager while creating BufImgSurfaceData. The scale in BufImgSurfaceData is used to create config while creating the SunGraphics2D through getDefaultTransform function. Now when the TranslucentWindowPainter create scaled BufferedImage and then creates Graphics2D using that, the Graphics2D has transform scale as (1,1) as the BufImgSurfaceData has scale (1,1). Then this Graphics2D is used to update the Window in  updateWindow in TranslucentWindowPainter. As the transform is identity, none of the Swing Components which are added to the window are scaled.
> So I modified the BufferedImage to store graphicsConfig which can then be used by BugImgSurfaceManager to create BugImgSurfaceData with correct scale values and hence correct transform value in SunGraphics2D.
>
> If this is not the correct way, I have one more way of doing this by transforming the SunGraphics2D object with transform of peer. So the SunGraphics2D will have correct transform  and objects are properly scaled. I have updated the webrev for the same. Please have a look. Also if this is also not the correct way, can you please suggest a way I can do this?
>
> Webrev:
> http://cr.openjdk.java.net/~pbansal/8164811/webrev.01/
>
>
> Regards,
> Pankaj Bansal
>
>
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Friday, November 3, 2017 5:34 AM
> To: Pankaj Bansal; Prem Kumar Balakrishnan ([hidden email]); [hidden email]; [hidden email]
> Subject: Re: [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering
>
> Hi, Pankaj.
> The BufferedImage class is a raster in the memory which should be unrelated to any grpahics config on the system. So this class should not be changed, but instead you should create a correct BI-as a back buffer which takes into account the size of the window and the scale of the GC.
>
> On 31/10/2017 04:59, Pankaj Bansal wrote:
>> Hi All,
>>
>> Please review the fix for JDK 10.
>>
>> Bug:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8164811
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~pbansal/8164811/webrev.00/
>>
>> Issue:
>>
>> The tests given in the bug were failing when run with OpenGL and GDI.
>> This bug is related to
>> https://bugs.openjdk.java.net/browse/JDK-8189257
>> which states that that HIDPI does not work with swing components when
>> Translucent window is used. Because of which all the tests in the bug
>> were failing.
>>
>> Fix:
>>
>> The TranslucentWindowPainter class was creating BufferedImage for
>> OpenGL (when /forceOpt/ is false) and GDI pipeline, but it is not
>> considering the device HiDPI scale. There is no way to create a scaled
>> BufferedImage because of which the scale value in BufImgSurfaceData is
>> always 1. Made changes to store graphics config in Buffered image, so
>> that the BufImgSurfaceManager can create BufImgSurfaceData with scale set properly.
>>
>> This fix also fixes https://bugs.openjdk.java.net/browse/JDK-8189257
>>
>> Regards,
>>
>> Pankaj Bansal
>>
>
> --
> Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

Pankaj Bansal
Hi Phil,

<< Adding new API to BufferedImage was definitely NOT the way to approach this.
Yes, I understand now. I have to add a API in TranslucantPainter.

<<Why are none of the failing tests referenced with an updated reg-test @bug tag to indicate this fix corrects them ?
I have updated the webrev for this.

<<You say it is observable with GDI .. the bug report refers only to OpenGL. So is it both ?
Yes, the issue is reproducible with both GDI and OpenGL. The D3D one was fixed in http://hg.openjdk.java.net/jdk9/client/jdk/rev/14918637b76e.

<<So does it reproduce with just -Dsun.java2d.d3d=false
Yes the issue is reproducible by setting uiScale > 1.0 and -Dsun.java2d.d3d=false. The issue is also reproducible by just setting uiScale without  -Dsun.java2d.d3d=false, as it runs the GDI because D3D is disabled on Intel cards as mentioned in D3DBadHardware.h.
I tested this on a machine which supports D3D pipeline. It fails with uiScale > 1.0 with -Dsun.java2d.d3d=false. With just scale being set, it passes as D3D is used as default pipeline on this machine.

Webrev:
http://cr.openjdk.java.net/~pbansal/8164811/webrev.02/


Regards,
Pankaj Bansal


-----Original Message-----
From: Philip Race
Sent: Saturday, November 4, 2017 1:23 AM
To: Pankaj Bansal
Cc: Sergey Bylokhov; Prem Balakrishnan; [hidden email]; [hidden email]
Subject: Re: [OpenJDK 2D-Dev] [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

Adding new API to BufferedImage was definitely NOT the way to approach this.
Adding new public API we need to support forever to fix an internal implementation bug on one platform in a very atypical scenario is the wrong trade-off.

And anyway as Sergey said BI should have no dependency or relationship on a BI

TranslucentWindowPainter already had the GraphicsConfiguration anyway so it does not seem necessary and you seem to be more along the right lines with this fix but I'd like to try it ..

Couple of questions :
Why are none of the failing tests referenced with an updated reg-test @bug tag to indicate this fix corrects them ?

You say it is observable with GDI .. the bug report refers only to OpenGL.
So is it both ?

If so you don't need to enable the flaky OGL pipeline to verify this .. but then I am left puzzled since Intel cards don't use D3D so I'd expect more reports.

So does it reproduce with just
-Dsun.java2d.d3d=false

??

-phil.



On 11/3/17, 6:44 AM, Pankaj Bansal wrote:

> Hi Sergey,
>
> I have created the BufferedImage of correct size considering the window dimension and graphics context scale.
>
> I changed BufferedImage just to store graphicsConfig, so that it can be used by BufImgSurfaceManager while creating BufImgSurfaceData. The scale in BufImgSurfaceData is used to create config while creating the SunGraphics2D through getDefaultTransform function. Now when the TranslucentWindowPainter create scaled BufferedImage and then creates Graphics2D using that, the Graphics2D has transform scale as (1,1) as the BufImgSurfaceData has scale (1,1). Then this Graphics2D is used to update the Window in  updateWindow in TranslucentWindowPainter. As the transform is identity, none of the Swing Components which are added to the window are scaled.
> So I modified the BufferedImage to store graphicsConfig which can then be used by BugImgSurfaceManager to create BugImgSurfaceData with correct scale values and hence correct transform value in SunGraphics2D.
>
> If this is not the correct way, I have one more way of doing this by transforming the SunGraphics2D object with transform of peer. So the SunGraphics2D will have correct transform  and objects are properly scaled. I have updated the webrev for the same. Please have a look. Also if this is also not the correct way, can you please suggest a way I can do this?
>
> Webrev:
> http://cr.openjdk.java.net/~pbansal/8164811/webrev.01/
>
>
> Regards,
> Pankaj Bansal
>
>
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Friday, November 3, 2017 5:34 AM
> To: Pankaj Bansal; Prem Kumar Balakrishnan
> ([hidden email]); [hidden email];
> [hidden email]
> Subject: Re: [10] Review Request JDK - 8164811 : [hidpi]Tests fail
> with OpenGL Rendering
>
> Hi, Pankaj.
> The BufferedImage class is a raster in the memory which should be unrelated to any grpahics config on the system. So this class should not be changed, but instead you should create a correct BI-as a back buffer which takes into account the size of the window and the scale of the GC.
>
> On 31/10/2017 04:59, Pankaj Bansal wrote:
>> Hi All,
>>
>> Please review the fix for JDK 10.
>>
>> Bug:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8164811
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~pbansal/8164811/webrev.00/
>>
>> Issue:
>>
>> The tests given in the bug were failing when run with OpenGL and GDI.
>> This bug is related to
>> https://bugs.openjdk.java.net/browse/JDK-8189257
>> which states that that HIDPI does not work with swing components when
>> Translucent window is used. Because of which all the tests in the bug
>> were failing.
>>
>> Fix:
>>
>> The TranslucentWindowPainter class was creating BufferedImage for
>> OpenGL (when /forceOpt/ is false) and GDI pipeline, but it is not
>> considering the device HiDPI scale. There is no way to create a
>> scaled BufferedImage because of which the scale value in
>> BufImgSurfaceData is always 1. Made changes to store graphics config
>> in Buffered image, so that the BufImgSurfaceManager can create BufImgSurfaceData with scale set properly.
>>
>> This fix also fixes https://bugs.openjdk.java.net/browse/JDK-8189257
>>
>> Regards,
>>
>> Pankaj Bansal
>>
>
> --
> Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

Sergey Bylokhov
Hi, Pankaj.
This version will creates back-buffer every time the paint/update is
fail, but before the fix it was created only once before painting.

On 06/11/2017 04:23, Pankaj Bansal wrote:
> I tested this on a machine which supports D3D pipeline. It fails with uiScale > 1.0 with -Dsun.java2d.d3d=false. With just scale being set, it passes as D3D is used as default pipeline on this machine.
>
> Webrev:
> http://cr.openjdk.java.net/~pbansal/8164811/webrev.02/


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

Pankaj Bansal
Hi Sergey,

<< This version will creates back-buffer every time the paint/update is fail, but before the fix it was created only once before painting.
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.

Please let me know if you are talking about something else or please elaborate a bit more about the issue.

Regards,
Pankaj Bansal


-----Original Message-----
From: Sergey Bylokhov
Sent: Tuesday, November 7, 2017 2:26 AM
To: Pankaj Bansal; Philip Race; Prem Balakrishnan
Cc: [hidden email]; [hidden email]
Subject: Re: [OpenJDK 2D-Dev] [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

Hi, Pankaj.
This version will creates back-buffer every time the paint/update is fail, but before the fix it was created only once before painting.

On 06/11/2017 04:23, Pankaj Bansal wrote:
> I tested this on a machine which supports D3D pipeline. It fails with uiScale > 1.0 with -Dsun.java2d.d3d=false. With just scale being set, it passes as D3D is used as default pipeline on this machine.
>
> Webrev:
> http://cr.openjdk.java.net/~pbansal/8164811/webrev.02/


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

Sergey Bylokhov
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.
Reply | Threaded
Open this post in threaded view
|

Re: [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

Pankaj Bansal
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: [hidden email]; [hidden email]
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.
Reply | Threaded
Open this post in threaded view
|

Re: [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

Sergey Bylokhov
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: [hidden email]; [hidden email]
> 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.
Reply | Threaded
Open this post in threaded view
|

Re: [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering

Prahalad kumar Narayanan
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: [hidden email]; [hidden email]
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: [hidden email]; [hidden email]
> 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.