Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

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

Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Pankaj Bansal
+ 2d-dev, Phil.

Regards,
Pankaj Bansal

-----Original Message-----
From: Pankaj Bansal
Sent: Friday, November 3, 2017 3:11 PM
To: Sergey Bylokhov; [hidden email]
Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Hi Sergey,


XRSurfaceData, GLXSurfaceData:        On linux, only integer scale  is supported. If we give non-integer, the the uiScale is truncated to integer and used. So the ceil or round does not matter. In both XRSurfaceData and GLXSurfaceData, ceil function or direct multiplication is used arbitrarily at different places. So, we can make changes for making things more consistent, but it is not necessary as of now.
PS: There are artifacts in same test case on Linux and there is a separate bug https://bugs.openjdk.java.net/browse/JDK-8189957. But these don't seem to be result of this precision error. I will look into this bug later.

WGLSurfaceData:     In openGL, ceil function is used in WGLSurfaceData, so it will create WGLOffScreenSurfaceData and in turn create native resource. Then in DrawImage class, the clipRound is used, and these sizes are sent to native size. Then in OGLBlitTextureToSurface function inside OGLBlitLoops file, the texture coordinates are calculated by dividing the sizes sent from Java side by the native resource size. So the texture coordinates are wrong. For instance, in DrawImageBilinear, the size is 5x5 and uiScale is 2.5, so the native resource is of size, 13x13. But the dimensions sent to native are 12x12, so the textures are (0,0) and (12/13, 12/13). So artifacts in final image.

D3DSurfaceData:     In D3D, everything is similar to OpenGL, but in D3DBlitTextureToSurface function inside D3DBlitLoops file,  the calculated texture coordinates are sent to D3DDrawTextureWithHint and then data is copied from texture to surface. Here if the native resource size and dimensions sent from Java are not same, the data is copied in parts. This is somehow handling the wrong texture coordinates. So this test does not produce any artifacts in D3D pipeline. But it can in some other case. So this needs to be fixed.

GDIWindowSurfacedata: Here the volatile image creates a BufImageSurfaceData as backing resource using the ceil function and while copying from BugImageSurfaceData to a GDIWindowSurface, the Java_sun_java2d_loops_TransformHelper_Transform in native side is able to handle the difference in sizes. So the artifacts are not visible. But I think this should be fixed. So should make changes to getBackupImage function in SunVolatileImage to use clipRound instead of ceil.

BufImageSurfaceData : It uses the image raster size to create the native resource.  The raster size is already scaled according to the uiScale. So no need to change this.

So in a nutshell,
I feel we may not change the code in XRSurfaceData, GLXSurfaceData as linux only support integer scaling. We can do it later if required.
We should change code for D3DSurfaceData, GDIWindowSurfacedata and WGLSurfacedata as it may cause artifacts. I have updated the webrev for the same. Please review.

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


Regards,
Pankaj Bansal


-----Original Message-----
From: Sergey Bylokhov
Sent: Wednesday, November 1, 2017 11:46 PM
To: Pankaj Bansal; [hidden email]
Subject: Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Hi, Pankaj.
D3DSurfaceData, XRSurfaceData and probably other SurfaceData use simple multiplication or ceil(). Should we update them also? and the test to catch the errors there as well.

On 30/10/2017 03:39, Pankaj Bansal wrote:

> Hi All,
>
> Please review the fix for JDK 10.
>
> Bug:
>
> https://bugs.openjdk.java.net/browse/JDK-8159142
>
> Webrev:
>
> http://cr.openjdk.java.net/~pbansal/8159142/webrev.00/
>
> Issue:
>
> The test runs with D3D and OpenGL both. The issue is with the OpenGL
> run as there aee some visual artifacts in image when the HiDPI scale
> is set to non-integer value like 2.5.
>
> Fix:
>
> The issue is due to precision error. In WGLSurfaceData.java, the width
> and height is calculated by multiplying the width and height with scale.
> But the ceil function was being used instead of round. It should be
> using round as at most of the places, the round function is being used.
>
> Also changed the test to run at uiScale of 2.5 to catch any further errors.
>
> Regards,
>
> Pankaj Bansal
>


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

Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Pankaj Bansal
Hi Sergey,

Do these changes look good? Please give your feedback.

Regards,
Pankaj Bansal

-----Original Message-----
From: Pankaj Bansal
Sent: Wednesday, November 8, 2017 8:09 PM
To: Sergey Bylokhov; [hidden email]; [hidden email]; Philip Race
Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

+ 2d-dev, Phil.

Regards,
Pankaj Bansal

-----Original Message-----
From: Pankaj Bansal
Sent: Friday, November 3, 2017 3:11 PM
To: Sergey Bylokhov; [hidden email]
Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Hi Sergey,


XRSurfaceData, GLXSurfaceData:        On linux, only integer scale  is supported. If we give non-integer, the the uiScale is truncated to integer and used. So the ceil or round does not matter. In both XRSurfaceData and GLXSurfaceData, ceil function or direct multiplication is used arbitrarily at different places. So, we can make changes for making things more consistent, but it is not necessary as of now.
PS: There are artifacts in same test case on Linux and there is a separate bug https://bugs.openjdk.java.net/browse/JDK-8189957. But these don't seem to be result of this precision error. I will look into this bug later.

WGLSurfaceData:     In openGL, ceil function is used in WGLSurfaceData, so it will create WGLOffScreenSurfaceData and in turn create native resource. Then in DrawImage class, the clipRound is used, and these sizes are sent to native size. Then in OGLBlitTextureToSurface function inside OGLBlitLoops file, the texture coordinates are calculated by dividing the sizes sent from Java side by the native resource size. So the texture coordinates are wrong. For instance, in DrawImageBilinear, the size is 5x5 and uiScale is 2.5, so the native resource is of size, 13x13. But the dimensions sent to native are 12x12, so the textures are (0,0) and (12/13, 12/13). So artifacts in final image.

D3DSurfaceData:     In D3D, everything is similar to OpenGL, but in D3DBlitTextureToSurface function inside D3DBlitLoops file,  the calculated texture coordinates are sent to D3DDrawTextureWithHint and then data is copied from texture to surface. Here if the native resource size and dimensions sent from Java are not same, the data is copied in parts. This is somehow handling the wrong texture coordinates. So this test does not produce any artifacts in D3D pipeline. But it can in some other case. So this needs to be fixed.

GDIWindowSurfacedata: Here the volatile image creates a BufImageSurfaceData as backing resource using the ceil function and while copying from BugImageSurfaceData to a GDIWindowSurface, the Java_sun_java2d_loops_TransformHelper_Transform in native side is able to handle the difference in sizes. So the artifacts are not visible. But I think this should be fixed. So should make changes to getBackupImage function in SunVolatileImage to use clipRound instead of ceil.

BufImageSurfaceData : It uses the image raster size to create the native resource.  The raster size is already scaled according to the uiScale. So no need to change this.

So in a nutshell,
I feel we may not change the code in XRSurfaceData, GLXSurfaceData as linux only support integer scaling. We can do it later if required.
We should change code for D3DSurfaceData, GDIWindowSurfacedata and WGLSurfacedata as it may cause artifacts. I have updated the webrev for the same. Please review.

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


Regards,
Pankaj Bansal


-----Original Message-----
From: Sergey Bylokhov
Sent: Wednesday, November 1, 2017 11:46 PM
To: Pankaj Bansal; [hidden email]
Subject: Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Hi, Pankaj.
D3DSurfaceData, XRSurfaceData and probably other SurfaceData use simple multiplication or ceil(). Should we update them also? and the test to catch the errors there as well.

On 30/10/2017 03:39, Pankaj Bansal wrote:

> Hi All,
>
> Please review the fix for JDK 10.
>
> Bug:
>
> https://bugs.openjdk.java.net/browse/JDK-8159142
>
> Webrev:
>
> http://cr.openjdk.java.net/~pbansal/8159142/webrev.00/
>
> Issue:
>
> The test runs with D3D and OpenGL both. The issue is with the OpenGL
> run as there aee some visual artifacts in image when the HiDPI scale
> is set to non-integer value like 2.5.
>
> Fix:
>
> The issue is due to precision error. In WGLSurfaceData.java, the width
> and height is calculated by multiplying the width and height with scale.
> But the ceil function was being used instead of round. It should be
> using round as at most of the places, the round function is being used.
>
> Also changed the test to run at uiScale of 2.5 to catch any further errors.
>
> Regards,
>
> Pankaj Bansal
>


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

Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Phil Race
Wasn't it suggested to have the Linux code stop truncating the uiScale if it was
explicitly set to a non-integer value ? Or at least have another property to tell it
not to truncate. This would facilitate ongoing testing of fixing Linux so we can support fractional values.

To test the software pipeline it may be that there should be one more line added to these
to disable the default accelerated pipeline
 29  * @run main/othervm -Dsun.java2d.uiScale=2.5 DrawImageBilinear
  30  * @run main/othervm -Dsun.java2d.uiScale=2.5 -Dsun.java2d.opengl=True -Dsun.java2d.opengl.fbobject=true DrawImageBilinear

@run main/othervm -Dsun.java2d.uiScale=2.5 -Dsun.java2d.3d=false -Dsun.java2d.xrender=false

That will ensure that on Windows we test the GDI pipeline and on Linux we test the X11 pipeline

-phil.
On 11/09/2017 08:43 AM, Pankaj Bansal wrote:
Hi Sergey,

Do these changes look good? Please give your feedback.

Regards,
Pankaj Bansal

-----Original Message-----
From: Pankaj Bansal 
Sent: Wednesday, November 8, 2017 8:09 PM
To: Sergey Bylokhov; [hidden email]; [hidden email]; Philip Race
Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

+ 2d-dev, Phil.

Regards,
Pankaj Bansal

-----Original Message-----
From: Pankaj Bansal
Sent: Friday, November 3, 2017 3:11 PM
To: Sergey Bylokhov; [hidden email]
Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Hi Sergey,


XRSurfaceData, GLXSurfaceData:        On linux, only integer scale  is supported. If we give non-integer, the the uiScale is truncated to integer and used. So the ceil or round does not matter. In both XRSurfaceData and GLXSurfaceData, ceil function or direct multiplication is used arbitrarily at different places. So, we can make changes for making things more consistent, but it is not necessary as of now. 
PS: There are artifacts in same test case on Linux and there is a separate bug https://bugs.openjdk.java.net/browse/JDK-8189957. But these don't seem to be result of this precision error. I will look into this bug later.

WGLSurfaceData:     In openGL, ceil function is used in WGLSurfaceData, so it will create WGLOffScreenSurfaceData and in turn create native resource. Then in DrawImage class, the clipRound is used, and these sizes are sent to native size. Then in OGLBlitTextureToSurface function inside OGLBlitLoops file, the texture coordinates are calculated by dividing the sizes sent from Java side by the native resource size. So the texture coordinates are wrong. For instance, in DrawImageBilinear, the size is 5x5 and uiScale is 2.5, so the native resource is of size, 13x13. But the dimensions sent to native are 12x12, so the textures are (0,0) and (12/13, 12/13). So artifacts in final image. 

D3DSurfaceData:     In D3D, everything is similar to OpenGL, but in D3DBlitTextureToSurface function inside D3DBlitLoops file,  the calculated texture coordinates are sent to D3DDrawTextureWithHint and then data is copied from texture to surface. Here if the native resource size and dimensions sent from Java are not same, the data is copied in parts. This is somehow handling the wrong texture coordinates. So this test does not produce any artifacts in D3D pipeline. But it can in some other case. So this needs to be fixed.

GDIWindowSurfacedata: Here the volatile image creates a BufImageSurfaceData as backing resource using the ceil function and while copying from BugImageSurfaceData to a GDIWindowSurface, the Java_sun_java2d_loops_TransformHelper_Transform in native side is able to handle the difference in sizes. So the artifacts are not visible. But I think this should be fixed. So should make changes to getBackupImage function in SunVolatileImage to use clipRound instead of ceil. 

BufImageSurfaceData : It uses the image raster size to create the native resource.  The raster size is already scaled according to the uiScale. So no need to change this.

So in a nutshell,
I feel we may not change the code in XRSurfaceData, GLXSurfaceData as linux only support integer scaling. We can do it later if required.
We should change code for D3DSurfaceData, GDIWindowSurfacedata and WGLSurfacedata as it may cause artifacts. I have updated the webrev for the same. Please review.

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


Regards,
Pankaj Bansal


-----Original Message-----
From: Sergey Bylokhov
Sent: Wednesday, November 1, 2017 11:46 PM
To: Pankaj Bansal; [hidden email]
Subject: Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Hi, Pankaj.
D3DSurfaceData, XRSurfaceData and probably other SurfaceData use simple multiplication or ceil(). Should we update them also? and the test to catch the errors there as well.

On 30/10/2017 03:39, Pankaj Bansal wrote:
Hi All,

Please review the fix for JDK 10.

Bug:

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

Webrev:

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

Issue:

The test runs with D3D and OpenGL both. The issue is with the OpenGL 
run as there aee some visual artifacts in image when the HiDPI scale 
is set to non-integer value like 2.5.

Fix:

The issue is due to precision error. In WGLSurfaceData.java, the width 
and height is calculated by multiplying the width and height with scale.
But the ceil function was being used instead of round. It should be 
using round as at most of the places, the round function is being used.

Also changed the test to run at uiScale of 2.5 to catch any further errors.

Regards,

Pankaj Bansal


--
Best regards, Sergey.

Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Pankaj Bansal

Hi Phil,

 

I think we discussed that there are many artifacts in both Windows and Linux and we should try to fix one at a time. So we should try to remove all artifacts from Windows first and then  move on to something else.

Also, the linux contains artifacts even without non-integer scaling https://bugs.openjdk.java.net/browse/JDK-8189957. So I think we should first try to solve these issues first in linux before moving ahead. So I will fix these issues when I work on the listed bug.

 

So for time being, I am working on the current bug to remove issue on Windows and want a feedback on the changes done for the same.

I have changed the test and added a @run to run with -Dsun.java2d.d3d=false. I have tested this on my machine (Intel chipset) and another machine which supports D3D. It passes on all.

 

Webrev:

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

 

Regards,

Pankaj Bansal

 

From: Phil Race
Sent: Friday, November 10, 2017 3:26 AM
To: Pankaj Bansal; Sergey Bylokhov; [hidden email]; [hidden email]
Subject: Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

 

Wasn't it suggested to have the Linux code stop truncating the uiScale if it was
explicitly set to a non-integer value ? Or at least have another property to tell it
not to truncate. This would facilitate ongoing testing of fixing Linux so we can support fractional values.

To test the software pipeline it may be that there should be one more line added to these
to disable the default accelerated pipeline

 29  * @run main/othervm -Dsun.java2d.uiScale=2.5 DrawImageBilinear
  30  * @run main/othervm -Dsun.java2d.uiScale=2.5 -Dsun.java2d.opengl=True -Dsun.java2d.opengl.fbobject=true DrawImageBilinear
 
@run main/othervm -Dsun.java2d.uiScale=2.5 -Dsun.java2d.3d=false -Dsun.java2d.xrender=false
 
That will ensure that on Windows we test the GDI pipeline and on Linux we test the X11 pipeline
 
-phil.

On 11/09/2017 08:43 AM, Pankaj Bansal wrote:

Hi Sergey,
 
Do these changes look good? Please give your feedback.
 
Regards,
Pankaj Bansal
 
-----Original Message-----
From: Pankaj Bansal 
Sent: Wednesday, November 8, 2017 8:09 PM
To: Sergey Bylokhov; [hidden email]; [hidden email]; Philip Race
Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
 
+ 2d-dev, Phil.
 
Regards,
Pankaj Bansal
 
-----Original Message-----
From: Pankaj Bansal
Sent: Friday, November 3, 2017 3:11 PM
To: Sergey Bylokhov; [hidden email]
Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
 
Hi Sergey,
 
 
XRSurfaceData, GLXSurfaceData:        On linux, only integer scale  is supported. If we give non-integer, the the uiScale is truncated to integer and used. So the ceil or round does not matter. In both XRSurfaceData and GLXSurfaceData, ceil function or direct multiplication is used arbitrarily at different places. So, we can make changes for making things more consistent, but it is not necessary as of now. 
PS: There are artifacts in same test case on Linux and there is a separate bug https://bugs.openjdk.java.net/browse/JDK-8189957. But these don't seem to be result of this precision error. I will look into this bug later.
 
WGLSurfaceData:     In openGL, ceil function is used in WGLSurfaceData, so it will create WGLOffScreenSurfaceData and in turn create native resource. Then in DrawImage class, the clipRound is used, and these sizes are sent to native size. Then in OGLBlitTextureToSurface function inside OGLBlitLoops file, the texture coordinates are calculated by dividing the sizes sent from Java side by the native resource size. So the texture coordinates are wrong. For instance, in DrawImageBilinear, the size is 5x5 and uiScale is 2.5, so the native resource is of size, 13x13. But the dimensions sent to native are 12x12, so the textures are (0,0) and (12/13, 12/13). So artifacts in final image. 
 
D3DSurfaceData:     In D3D, everything is similar to OpenGL, but in D3DBlitTextureToSurface function inside D3DBlitLoops file,  the calculated texture coordinates are sent to D3DDrawTextureWithHint and then data is copied from texture to surface. Here if the native resource size and dimensions sent from Java are not same, the data is copied in parts. This is somehow handling the wrong texture coordinates. So this test does not produce any artifacts in D3D pipeline. But it can in some other case. So this needs to be fixed.
 
GDIWindowSurfacedata: Here the volatile image creates a BufImageSurfaceData as backing resource using the ceil function and while copying from BugImageSurfaceData to a GDIWindowSurface, the Java_sun_java2d_loops_TransformHelper_Transform in native side is able to handle the difference in sizes. So the artifacts are not visible. But I think this should be fixed. So should make changes to getBackupImage function in SunVolatileImage to use clipRound instead of ceil. 
 
BufImageSurfaceData : It uses the image raster size to create the native resource.  The raster size is already scaled according to the uiScale. So no need to change this.
 
So in a nutshell,
I feel we may not change the code in XRSurfaceData, GLXSurfaceData as linux only support integer scaling. We can do it later if required.
We should change code for D3DSurfaceData, GDIWindowSurfacedata and WGLSurfacedata as it may cause artifacts. I have updated the webrev for the same. Please review.
 
Webrev:
http://cr.openjdk.java.net/~pbansal/8159142/webrev.01/
 
 
Regards,
Pankaj Bansal
 
 
-----Original Message-----
From: Sergey Bylokhov
Sent: Wednesday, November 1, 2017 11:46 PM
To: Pankaj Bansal; [hidden email]
Subject: Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
 
Hi, Pankaj.
D3DSurfaceData, XRSurfaceData and probably other SurfaceData use simple multiplication or ceil(). Should we update them also? and the test to catch the errors there as well.
 
On 30/10/2017 03:39, Pankaj Bansal wrote:
Hi All,
 
Please review the fix for JDK 10.
 
Bug:
 
https://bugs.openjdk.java.net/browse/JDK-8159142
 
Webrev:
 
http://cr.openjdk.java.net/~pbansal/8159142/webrev.00/
 
Issue:
 
The test runs with D3D and OpenGL both. The issue is with the OpenGL 
run as there aee some visual artifacts in image when the HiDPI scale 
is set to non-integer value like 2.5.
 
Fix:
 
The issue is due to precision error. In WGLSurfaceData.java, the width 
and height is calculated by multiplying the width and height with scale.
But the ceil function was being used instead of round. It should be 
using round as at most of the places, the round function is being used.
 
Also changed the test to run at uiScale of 2.5 to catch any further errors.
 
Regards,
 
Pankaj Bansal
 
 
 
--
Best regards, Sergey.

 

Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Sergey Bylokhov
In reply to this post by Pankaj Bansal
I have only one comment:
@run main/othervm DrawImageBilinear
Should we preserve this tag and run the test in the common-default config?

On 09/11/2017 08:43, Pankaj Bansal wrote:

> Hi Sergey,
>
> Do these changes look good? Please give your feedback.
>
> Regards,
> Pankaj Bansal
>
> -----Original Message-----
> From: Pankaj Bansal
> Sent: Wednesday, November 8, 2017 8:09 PM
> To: Sergey Bylokhov; [hidden email]; [hidden email]; Philip Race
> Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> + 2d-dev, Phil.
>
> Regards,
> Pankaj Bansal
>
> -----Original Message-----
> From: Pankaj Bansal
> Sent: Friday, November 3, 2017 3:11 PM
> To: Sergey Bylokhov; [hidden email]
> Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> Hi Sergey,
>
>
> XRSurfaceData, GLXSurfaceData:        On linux, only integer scale  is supported. If we give non-integer, the the uiScale is truncated to integer and used. So the ceil or round does not matter. In both XRSurfaceData and GLXSurfaceData, ceil function or direct multiplication is used arbitrarily at different places. So, we can make changes for making things more consistent, but it is not necessary as of now.
> PS: There are artifacts in same test case on Linux and there is a separate bug https://bugs.openjdk.java.net/browse/JDK-8189957. But these don't seem to be result of this precision error. I will look into this bug later.
>
> WGLSurfaceData:     In openGL, ceil function is used in WGLSurfaceData, so it will create WGLOffScreenSurfaceData and in turn create native resource. Then in DrawImage class, the clipRound is used, and these sizes are sent to native size. Then in OGLBlitTextureToSurface function inside OGLBlitLoops file, the texture coordinates are calculated by dividing the sizes sent from Java side by the native resource size. So the texture coordinates are wrong. For instance, in DrawImageBilinear, the size is 5x5 and uiScale is 2.5, so the native resource is of size, 13x13. But the dimensions sent to native are 12x12, so the textures are (0,0) and (12/13, 12/13). So artifacts in final image.
>
> D3DSurfaceData:     In D3D, everything is similar to OpenGL, but in D3DBlitTextureToSurface function inside D3DBlitLoops file,  the calculated texture coordinates are sent to D3DDrawTextureWithHint and then data is copied from texture to surface. Here if the native resource size and dimensions sent from Java are not same, the data is copied in parts. This is somehow handling the wrong texture coordinates. So this test does not produce any artifacts in D3D pipeline. But it can in some other case. So this needs to be fixed.
>
> GDIWindowSurfacedata: Here the volatile image creates a BufImageSurfaceData as backing resource using the ceil function and while copying from BugImageSurfaceData to a GDIWindowSurface, the Java_sun_java2d_loops_TransformHelper_Transform in native side is able to handle the difference in sizes. So the artifacts are not visible. But I think this should be fixed. So should make changes to getBackupImage function in SunVolatileImage to use clipRound instead of ceil.
>
> BufImageSurfaceData : It uses the image raster size to create the native resource.  The raster size is already scaled according to the uiScale. So no need to change this.
>
> So in a nutshell,
> I feel we may not change the code in XRSurfaceData, GLXSurfaceData as linux only support integer scaling. We can do it later if required.
> We should change code for D3DSurfaceData, GDIWindowSurfacedata and WGLSurfacedata as it may cause artifacts. I have updated the webrev for the same. Please review.
>
> Webrev:
> http://cr.openjdk.java.net/~pbansal/8159142/webrev.01/
>
>
> Regards,
> Pankaj Bansal
>
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Wednesday, November 1, 2017 11:46 PM
> To: Pankaj Bansal; [hidden email]
> Subject: Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> Hi, Pankaj.
> D3DSurfaceData, XRSurfaceData and probably other SurfaceData use simple multiplication or ceil(). Should we update them also? and the test to catch the errors there as well.
>
> On 30/10/2017 03:39, Pankaj Bansal wrote:
>> Hi All,
>>
>> Please review the fix for JDK 10.
>>
>> Bug:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8159142
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~pbansal/8159142/webrev.00/
>>
>> Issue:
>>
>> The test runs with D3D and OpenGL both. The issue is with the OpenGL
>> run as there aee some visual artifacts in image when the HiDPI scale
>> is set to non-integer value like 2.5.
>>
>> Fix:
>>
>> The issue is due to precision error. In WGLSurfaceData.java, the width
>> and height is calculated by multiplying the width and height with scale.
>> But the ceil function was being used instead of round. It should be
>> using round as at most of the places, the round function is being used.
>>
>> Also changed the test to run at uiScale of 2.5 to catch any further errors.
>>
>> Regards,
>>
>> Pankaj Bansal
>>
>
>
> --
> Best regards, Sergey.
>


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

Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Prahalad kumar Narayanan
Hello Pankaj

Good day to you.

I imported your changes and checked the build with the JTreg test file.
    . I tried random uiScales using -Dsun.java2d.uiScale and by adjusting DPI scale on Windows 10.
          . The test failed at uiScale set at 1.75 on all the pipelines- D3D, OpenGL and GDI.

    . As you explained offline, this could be due to Robot running on HiDPI tests. Kindly check this out.
          . Sergey's suggestion to include "@run main/othervm DrawImageBilinear" will enable the test-case use the DPI scale as available on host machine.
          . Hence any uiScale (1.75) is possible & makes test vulnerable for failures.

I 've not reviewed the code yet. I 'm working on a similar D3D bug and will need some time to review code.
If other reviewers approve of the change, kindly proceed with the check-in.

Thank you
Have a good day

Prahalad

-----Original Message-----
From: Sergey Bylokhov
Sent: Tuesday, November 14, 2017 3:57 AM
To: Pankaj Bansal; [hidden email]; [hidden email]; Philip Race
Subject: Re: [OpenJDK 2D-Dev] <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

I have only one comment:
@run main/othervm DrawImageBilinear
Should we preserve this tag and run the test in the common-default config?

On 09/11/2017 08:43, Pankaj Bansal wrote:

> Hi Sergey,
>
> Do these changes look good? Please give your feedback.
>
> Regards,
> Pankaj Bansal
>
> -----Original Message-----
> From: Pankaj Bansal
> Sent: Wednesday, November 8, 2017 8:09 PM
> To: Sergey Bylokhov; [hidden email];
> [hidden email]; Philip Race
> Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible
> artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> + 2d-dev, Phil.
>
> Regards,
> Pankaj Bansal
>
> -----Original Message-----
> From: Pankaj Bansal
> Sent: Friday, November 3, 2017 3:11 PM
> To: Sergey Bylokhov; [hidden email]
> Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible
> artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> Hi Sergey,
>
>
> XRSurfaceData, GLXSurfaceData:        On linux, only integer scale  is supported. If we give non-integer, the the uiScale is truncated to integer and used. So the ceil or round does not matter. In both XRSurfaceData and GLXSurfaceData, ceil function or direct multiplication is used arbitrarily at different places. So, we can make changes for making things more consistent, but it is not necessary as of now.
> PS: There are artifacts in same test case on Linux and there is a separate bug https://bugs.openjdk.java.net/browse/JDK-8189957. But these don't seem to be result of this precision error. I will look into this bug later.
>
> WGLSurfaceData:     In openGL, ceil function is used in WGLSurfaceData, so it will create WGLOffScreenSurfaceData and in turn create native resource. Then in DrawImage class, the clipRound is used, and these sizes are sent to native size. Then in OGLBlitTextureToSurface function inside OGLBlitLoops file, the texture coordinates are calculated by dividing the sizes sent from Java side by the native resource size. So the texture coordinates are wrong. For instance, in DrawImageBilinear, the size is 5x5 and uiScale is 2.5, so the native resource is of size, 13x13. But the dimensions sent to native are 12x12, so the textures are (0,0) and (12/13, 12/13). So artifacts in final image.
>
> D3DSurfaceData:     In D3D, everything is similar to OpenGL, but in D3DBlitTextureToSurface function inside D3DBlitLoops file,  the calculated texture coordinates are sent to D3DDrawTextureWithHint and then data is copied from texture to surface. Here if the native resource size and dimensions sent from Java are not same, the data is copied in parts. This is somehow handling the wrong texture coordinates. So this test does not produce any artifacts in D3D pipeline. But it can in some other case. So this needs to be fixed.
>
> GDIWindowSurfacedata: Here the volatile image creates a BufImageSurfaceData as backing resource using the ceil function and while copying from BugImageSurfaceData to a GDIWindowSurface, the Java_sun_java2d_loops_TransformHelper_Transform in native side is able to handle the difference in sizes. So the artifacts are not visible. But I think this should be fixed. So should make changes to getBackupImage function in SunVolatileImage to use clipRound instead of ceil.
>
> BufImageSurfaceData : It uses the image raster size to create the native resource.  The raster size is already scaled according to the uiScale. So no need to change this.
>
> So in a nutshell,
> I feel we may not change the code in XRSurfaceData, GLXSurfaceData as linux only support integer scaling. We can do it later if required.
> We should change code for D3DSurfaceData, GDIWindowSurfacedata and WGLSurfacedata as it may cause artifacts. I have updated the webrev for the same. Please review.
>
> Webrev:
> http://cr.openjdk.java.net/~pbansal/8159142/webrev.01/
>
>
> Regards,
> Pankaj Bansal
>
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Wednesday, November 1, 2017 11:46 PM
> To: Pankaj Bansal; [hidden email]
> Subject: Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible
> artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> Hi, Pankaj.
> D3DSurfaceData, XRSurfaceData and probably other SurfaceData use simple multiplication or ceil(). Should we update them also? and the test to catch the errors there as well.
>
> On 30/10/2017 03:39, Pankaj Bansal wrote:
>> Hi All,
>>
>> Please review the fix for JDK 10.
>>
>> Bug:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8159142
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~pbansal/8159142/webrev.00/
>>
>> Issue:
>>
>> The test runs with D3D and OpenGL both. The issue is with the OpenGL
>> run as there aee some visual artifacts in image when the HiDPI scale
>> is set to non-integer value like 2.5.
>>
>> Fix:
>>
>> The issue is due to precision error. In WGLSurfaceData.java, the
>> width and height is calculated by multiplying the width and height with scale.
>> But the ceil function was being used instead of round. It should be
>> using round as at most of the places, the round function is being used.
>>
>> Also changed the test to run at uiScale of 2.5 to catch any further errors.
>>
>> Regards,
>>
>> Pankaj Bansal
>>
>
>
> --
> Best regards, Sergey.
>


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

Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Pankaj Bansal
Hi Prahalad,

Thanks for the review.

Yes, the test fails at 1.75 scale value both before and after the fix on GDI, OpenGL and D3D pipelines with exceptions similar to this.
Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: Test failed at x=10 y=10 (expected=0xffff0000 actual=0xffff6060)
 
In this bug, I was working to remove the artifact in the image as shown in the image attached in the JBS. The issue is reproducible at scale 2.5 and similar on only OpenGL pipeline. I mostly worked to remove the artifact at mentioned scale. The proposed fix removes that artifact from the image.

I am not very sure if it’s a Robot issue or not, but I have some reasons to believe that. I will work on it and let you know.

Regards,
Pankaj Bansal

-----Original Message-----
From: Prahalad Kumar Narayanan
Sent: Tuesday, November 14, 2017 3:15 PM
To: Sergey Bylokhov; Pankaj Bansal; [hidden email]; [hidden email]; Philip Race
Subject: RE: [OpenJDK 2D-Dev] <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Hello Pankaj

Good day to you.

I imported your changes and checked the build with the JTreg test file.
    . I tried random uiScales using -Dsun.java2d.uiScale and by adjusting DPI scale on Windows 10.
          . The test failed at uiScale set at 1.75 on all the pipelines- D3D, OpenGL and GDI.

    . As you explained offline, this could be due to Robot running on HiDPI tests. Kindly check this out.
          . Sergey's suggestion to include "@run main/othervm DrawImageBilinear" will enable the test-case use the DPI scale as available on host machine.
          . Hence any uiScale (1.75) is possible & makes test vulnerable for failures.

I 've not reviewed the code yet. I 'm working on a similar D3D bug and will need some time to review code.
If other reviewers approve of the change, kindly proceed with the check-in.

Thank you
Have a good day

Prahalad

-----Original Message-----
From: Sergey Bylokhov
Sent: Tuesday, November 14, 2017 3:57 AM
To: Pankaj Bansal; [hidden email]; [hidden email]; Philip Race
Subject: Re: [OpenJDK 2D-Dev] <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

I have only one comment:
@run main/othervm DrawImageBilinear
Should we preserve this tag and run the test in the common-default config?

On 09/11/2017 08:43, Pankaj Bansal wrote:

> Hi Sergey,
>
> Do these changes look good? Please give your feedback.
>
> Regards,
> Pankaj Bansal
>
> -----Original Message-----
> From: Pankaj Bansal
> Sent: Wednesday, November 8, 2017 8:09 PM
> To: Sergey Bylokhov; [hidden email];
> [hidden email]; Philip Race
> Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible
> artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> + 2d-dev, Phil.
>
> Regards,
> Pankaj Bansal
>
> -----Original Message-----
> From: Pankaj Bansal
> Sent: Friday, November 3, 2017 3:11 PM
> To: Sergey Bylokhov; [hidden email]
> Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible
> artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> Hi Sergey,
>
>
> XRSurfaceData, GLXSurfaceData:        On linux, only integer scale  is supported. If we give non-integer, the the uiScale is truncated to integer and used. So the ceil or round does not matter. In both XRSurfaceData and GLXSurfaceData, ceil function or direct multiplication is used arbitrarily at different places. So, we can make changes for making things more consistent, but it is not necessary as of now.
> PS: There are artifacts in same test case on Linux and there is a separate bug https://bugs.openjdk.java.net/browse/JDK-8189957. But these don't seem to be result of this precision error. I will look into this bug later.
>
> WGLSurfaceData:     In openGL, ceil function is used in WGLSurfaceData, so it will create WGLOffScreenSurfaceData and in turn create native resource. Then in DrawImage class, the clipRound is used, and these sizes are sent to native size. Then in OGLBlitTextureToSurface function inside OGLBlitLoops file, the texture coordinates are calculated by dividing the sizes sent from Java side by the native resource size. So the texture coordinates are wrong. For instance, in DrawImageBilinear, the size is 5x5 and uiScale is 2.5, so the native resource is of size, 13x13. But the dimensions sent to native are 12x12, so the textures are (0,0) and (12/13, 12/13). So artifacts in final image.
>
> D3DSurfaceData:     In D3D, everything is similar to OpenGL, but in D3DBlitTextureToSurface function inside D3DBlitLoops file,  the calculated texture coordinates are sent to D3DDrawTextureWithHint and then data is copied from texture to surface. Here if the native resource size and dimensions sent from Java are not same, the data is copied in parts. This is somehow handling the wrong texture coordinates. So this test does not produce any artifacts in D3D pipeline. But it can in some other case. So this needs to be fixed.
>
> GDIWindowSurfacedata: Here the volatile image creates a BufImageSurfaceData as backing resource using the ceil function and while copying from BugImageSurfaceData to a GDIWindowSurface, the Java_sun_java2d_loops_TransformHelper_Transform in native side is able to handle the difference in sizes. So the artifacts are not visible. But I think this should be fixed. So should make changes to getBackupImage function in SunVolatileImage to use clipRound instead of ceil.
>
> BufImageSurfaceData : It uses the image raster size to create the native resource.  The raster size is already scaled according to the uiScale. So no need to change this.
>
> So in a nutshell,
> I feel we may not change the code in XRSurfaceData, GLXSurfaceData as linux only support integer scaling. We can do it later if required.
> We should change code for D3DSurfaceData, GDIWindowSurfacedata and WGLSurfacedata as it may cause artifacts. I have updated the webrev for the same. Please review.
>
> Webrev:
> http://cr.openjdk.java.net/~pbansal/8159142/webrev.01/
>
>
> Regards,
> Pankaj Bansal
>
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Wednesday, November 1, 2017 11:46 PM
> To: Pankaj Bansal; [hidden email]
> Subject: Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible
> artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> Hi, Pankaj.
> D3DSurfaceData, XRSurfaceData and probably other SurfaceData use simple multiplication or ceil(). Should we update them also? and the test to catch the errors there as well.
>
> On 30/10/2017 03:39, Pankaj Bansal wrote:
>> Hi All,
>>
>> Please review the fix for JDK 10.
>>
>> Bug:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8159142
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~pbansal/8159142/webrev.00/
>>
>> Issue:
>>
>> The test runs with D3D and OpenGL both. The issue is with the OpenGL
>> run as there aee some visual artifacts in image when the HiDPI scale
>> is set to non-integer value like 2.5.
>>
>> Fix:
>>
>> The issue is due to precision error. In WGLSurfaceData.java, the
>> width and height is calculated by multiplying the width and height with scale.
>> But the ceil function was being used instead of round. It should be
>> using round as at most of the places, the round function is being used.
>>
>> Also changed the test to run at uiScale of 2.5 to catch any further errors.
>>
>> Regards,
>>
>> Pankaj Bansal
>>
>
>
> --
> Best regards, Sergey.
>


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

Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Pankaj Bansal
Hi Sergey,

As discussed, I have created a new issue for test failure at HiDPI scale of 1.75 https://bugs.openjdk.java.net/browse/JDK-8191406
I have also mentioned that we need to write a new test case also for this.
<<test fails at 1.75 scale value both before and after the fix on GDI, OpenGL and D3D pipelines with exceptions similar to this.
<<Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: Test failed at x=10 y=10 (expected=0xffff0000 actual=0xffff6060)


As discussed I think we should keep this now, so that the test passes irrespective of default uiScale of system. We will update this later when the issue at HiDPI 1.75 is fixed. I have updated the test to run with all GDI, OpenGL and D3D, so we can catch any artifacts if any are there. Please let me know if this looks ok.
Webrev: http://cr.openjdk.java.net/~pbansal/8159142/webrev.04/
<<I have only one comment:
<<@run main/othervm DrawImageBilinear
<<Should we preserve this tag and run the test in the common-default config?


Regards,
Pankaj Bansal

-----Original Message-----
From: Pankaj Bansal
Sent: Tuesday, November 14, 2017 6:15 PM
To: Prahalad Kumar Narayanan; Sergey Bylokhov; [hidden email]; [hidden email]; Philip Race
Subject: RE: [OpenJDK 2D-Dev] <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Hi Prahalad,

Thanks for the review.

Yes, the test fails at 1.75 scale value both before and after the fix on GDI, OpenGL and D3D pipelines with exceptions similar to this.
Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: Test failed at x=10 y=10 (expected=0xffff0000 actual=0xffff6060)
 
In this bug, I was working to remove the artifact in the image as shown in the image attached in the JBS. The issue is reproducible at scale 2.5 and similar on only OpenGL pipeline. I mostly worked to remove the artifact at mentioned scale. The proposed fix removes that artifact from the image.

I am not very sure if it’s a Robot issue or not, but I have some reasons to believe that. I will work on it and let you know.

Regards,
Pankaj Bansal

-----Original Message-----
From: Prahalad Kumar Narayanan
Sent: Tuesday, November 14, 2017 3:15 PM
To: Sergey Bylokhov; Pankaj Bansal; [hidden email]; [hidden email]; Philip Race
Subject: RE: [OpenJDK 2D-Dev] <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Hello Pankaj

Good day to you.

I imported your changes and checked the build with the JTreg test file.
    . I tried random uiScales using -Dsun.java2d.uiScale and by adjusting DPI scale on Windows 10.
          . The test failed at uiScale set at 1.75 on all the pipelines- D3D, OpenGL and GDI.

    . As you explained offline, this could be due to Robot running on HiDPI tests. Kindly check this out.
          . Sergey's suggestion to include "@run main/othervm DrawImageBilinear" will enable the test-case use the DPI scale as available on host machine.
          . Hence any uiScale (1.75) is possible & makes test vulnerable for failures.

I 've not reviewed the code yet. I 'm working on a similar D3D bug and will need some time to review code.
If other reviewers approve of the change, kindly proceed with the check-in.

Thank you
Have a good day

Prahalad

-----Original Message-----
From: Sergey Bylokhov
Sent: Tuesday, November 14, 2017 3:57 AM
To: Pankaj Bansal; [hidden email]; [hidden email]; Philip Race
Subject: Re: [OpenJDK 2D-Dev] <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

I have only one comment:
@run main/othervm DrawImageBilinear
Should we preserve this tag and run the test in the common-default config?

On 09/11/2017 08:43, Pankaj Bansal wrote:

> Hi Sergey,
>
> Do these changes look good? Please give your feedback.
>
> Regards,
> Pankaj Bansal
>
> -----Original Message-----
> From: Pankaj Bansal
> Sent: Wednesday, November 8, 2017 8:09 PM
> To: Sergey Bylokhov; [hidden email];
> [hidden email]; Philip Race
> Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible
> artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> + 2d-dev, Phil.
>
> Regards,
> Pankaj Bansal
>
> -----Original Message-----
> From: Pankaj Bansal
> Sent: Friday, November 3, 2017 3:11 PM
> To: Sergey Bylokhov; [hidden email]
> Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible
> artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> Hi Sergey,
>
>
> XRSurfaceData, GLXSurfaceData:        On linux, only integer scale  is supported. If we give non-integer, the the uiScale is truncated to integer and used. So the ceil or round does not matter. In both XRSurfaceData and GLXSurfaceData, ceil function or direct multiplication is used arbitrarily at different places. So, we can make changes for making things more consistent, but it is not necessary as of now.
> PS: There are artifacts in same test case on Linux and there is a separate bug https://bugs.openjdk.java.net/browse/JDK-8189957. But these don't seem to be result of this precision error. I will look into this bug later.
>
> WGLSurfaceData:     In openGL, ceil function is used in WGLSurfaceData, so it will create WGLOffScreenSurfaceData and in turn create native resource. Then in DrawImage class, the clipRound is used, and these sizes are sent to native size. Then in OGLBlitTextureToSurface function inside OGLBlitLoops file, the texture coordinates are calculated by dividing the sizes sent from Java side by the native resource size. So the texture coordinates are wrong. For instance, in DrawImageBilinear, the size is 5x5 and uiScale is 2.5, so the native resource is of size, 13x13. But the dimensions sent to native are 12x12, so the textures are (0,0) and (12/13, 12/13). So artifacts in final image.
>
> D3DSurfaceData:     In D3D, everything is similar to OpenGL, but in D3DBlitTextureToSurface function inside D3DBlitLoops file,  the calculated texture coordinates are sent to D3DDrawTextureWithHint and then data is copied from texture to surface. Here if the native resource size and dimensions sent from Java are not same, the data is copied in parts. This is somehow handling the wrong texture coordinates. So this test does not produce any artifacts in D3D pipeline. But it can in some other case. So this needs to be fixed.
>
> GDIWindowSurfacedata: Here the volatile image creates a BufImageSurfaceData as backing resource using the ceil function and while copying from BugImageSurfaceData to a GDIWindowSurface, the Java_sun_java2d_loops_TransformHelper_Transform in native side is able to handle the difference in sizes. So the artifacts are not visible. But I think this should be fixed. So should make changes to getBackupImage function in SunVolatileImage to use clipRound instead of ceil.
>
> BufImageSurfaceData : It uses the image raster size to create the native resource.  The raster size is already scaled according to the uiScale. So no need to change this.
>
> So in a nutshell,
> I feel we may not change the code in XRSurfaceData, GLXSurfaceData as linux only support integer scaling. We can do it later if required.
> We should change code for D3DSurfaceData, GDIWindowSurfacedata and WGLSurfacedata as it may cause artifacts. I have updated the webrev for the same. Please review.
>
> Webrev:
> http://cr.openjdk.java.net/~pbansal/8159142/webrev.01/
>
>
> Regards,
> Pankaj Bansal
>
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Wednesday, November 1, 2017 11:46 PM
> To: Pankaj Bansal; [hidden email]
> Subject: Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible
> artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> Hi, Pankaj.
> D3DSurfaceData, XRSurfaceData and probably other SurfaceData use simple multiplication or ceil(). Should we update them also? and the test to catch the errors there as well.
>
> On 30/10/2017 03:39, Pankaj Bansal wrote:
>> Hi All,
>>
>> Please review the fix for JDK 10.
>>
>> Bug:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8159142
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~pbansal/8159142/webrev.00/
>>
>> Issue:
>>
>> The test runs with D3D and OpenGL both. The issue is with the OpenGL
>> run as there aee some visual artifacts in image when the HiDPI scale
>> is set to non-integer value like 2.5.
>>
>> Fix:
>>
>> The issue is due to precision error. In WGLSurfaceData.java, the
>> width and height is calculated by multiplying the width and height with scale.
>> But the ceil function was being used instead of round. It should be
>> using round as at most of the places, the round function is being used.
>>
>> Also changed the test to run at uiScale of 2.5 to catch any further errors.
>>
>> Regards,
>>
>> Pankaj Bansal
>>
>
>
> --
> Best regards, Sergey.
>


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

Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java

Sergey Bylokhov
Looks fine. Thank you.

On 16/11/2017 01:53, Pankaj Bansal wrote:

> Hi Sergey,
>
> As discussed, I have created a new issue for test failure at HiDPI scale of 1.75 https://bugs.openjdk.java.net/browse/JDK-8191406
> I have also mentioned that we need to write a new test case also for this.
> <<test fails at 1.75 scale value both before and after the fix on GDI, OpenGL and D3D pipelines with exceptions similar to this.
> <<Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: Test failed at x=10 y=10 (expected=0xffff0000 actual=0xffff6060)
>
>
> As discussed I think we should keep this now, so that the test passes irrespective of default uiScale of system. We will update this later when the issue at HiDPI 1.75 is fixed. I have updated the test to run with all GDI, OpenGL and D3D, so we can catch any artifacts if any are there. Please let me know if this looks ok.
> Webrev: http://cr.openjdk.java.net/~pbansal/8159142/webrev.04/
> <<I have only one comment:
> <<@run main/othervm DrawImageBilinear
> <<Should we preserve this tag and run the test in the common-default config?
>
>
> Regards,
> Pankaj Bansal
>
> -----Original Message-----
> From: Pankaj Bansal
> Sent: Tuesday, November 14, 2017 6:15 PM
> To: Prahalad Kumar Narayanan; Sergey Bylokhov; [hidden email]; [hidden email]; Philip Race
> Subject: RE: [OpenJDK 2D-Dev] <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> Hi Prahalad,
>
> Thanks for the review.
>
> Yes, the test fails at 1.75 scale value both before and after the fix on GDI, OpenGL and D3D pipelines with exceptions similar to this.
> Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: Test failed at x=10 y=10 (expected=0xffff0000 actual=0xffff6060)
>  
> In this bug, I was working to remove the artifact in the image as shown in the image attached in the JBS. The issue is reproducible at scale 2.5 and similar on only OpenGL pipeline. I mostly worked to remove the artifact at mentioned scale. The proposed fix removes that artifact from the image.
>
> I am not very sure if it’s a Robot issue or not, but I have some reasons to believe that. I will work on it and let you know.
>
> Regards,
> Pankaj Bansal
>
> -----Original Message-----
> From: Prahalad Kumar Narayanan
> Sent: Tuesday, November 14, 2017 3:15 PM
> To: Sergey Bylokhov; Pankaj Bansal; [hidden email]; [hidden email]; Philip Race
> Subject: RE: [OpenJDK 2D-Dev] <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> Hello Pankaj
>
> Good day to you.
>
> I imported your changes and checked the build with the JTreg test file.
>      . I tried random uiScales using -Dsun.java2d.uiScale and by adjusting DPI scale on Windows 10.
>            . The test failed at uiScale set at 1.75 on all the pipelines- D3D, OpenGL and GDI.
>
>      . As you explained offline, this could be due to Robot running on HiDPI tests. Kindly check this out.
>            . Sergey's suggestion to include "@run main/othervm DrawImageBilinear" will enable the test-case use the DPI scale as available on host machine.
>            . Hence any uiScale (1.75) is possible & makes test vulnerable for failures.
>
> I 've not reviewed the code yet. I 'm working on a similar D3D bug and will need some time to review code.
> If other reviewers approve of the change, kindly proceed with the check-in.
>
> Thank you
> Have a good day
>
> Prahalad
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Tuesday, November 14, 2017 3:57 AM
> To: Pankaj Bansal; [hidden email]; [hidden email]; Philip Race
> Subject: Re: [OpenJDK 2D-Dev] <AWT Dev> [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>
> I have only one comment:
> @run main/othervm DrawImageBilinear
> Should we preserve this tag and run the test in the common-default config?
>
> On 09/11/2017 08:43, Pankaj Bansal wrote:
>> Hi Sergey,
>>
>> Do these changes look good? Please give your feedback.
>>
>> Regards,
>> Pankaj Bansal
>>
>> -----Original Message-----
>> From: Pankaj Bansal
>> Sent: Wednesday, November 8, 2017 8:09 PM
>> To: Sergey Bylokhov; [hidden email];
>> [hidden email]; Philip Race
>> Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible
>> artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>>
>> + 2d-dev, Phil.
>>
>> Regards,
>> Pankaj Bansal
>>
>> -----Original Message-----
>> From: Pankaj Bansal
>> Sent: Friday, November 3, 2017 3:11 PM
>> To: Sergey Bylokhov; [hidden email]
>> Subject: RE: <AWT Dev> [10] Review Request: JDK-8159142 : Visible
>> artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>>
>> Hi Sergey,
>>
>>
>> XRSurfaceData, GLXSurfaceData:        On linux, only integer scale  is supported. If we give non-integer, the the uiScale is truncated to integer and used. So the ceil or round does not matter. In both XRSurfaceData and GLXSurfaceData, ceil function or direct multiplication is used arbitrarily at different places. So, we can make changes for making things more consistent, but it is not necessary as of now.
>> PS: There are artifacts in same test case on Linux and there is a separate bug https://bugs.openjdk.java.net/browse/JDK-8189957. But these don't seem to be result of this precision error. I will look into this bug later.
>>
>> WGLSurfaceData:     In openGL, ceil function is used in WGLSurfaceData, so it will create WGLOffScreenSurfaceData and in turn create native resource. Then in DrawImage class, the clipRound is used, and these sizes are sent to native size. Then in OGLBlitTextureToSurface function inside OGLBlitLoops file, the texture coordinates are calculated by dividing the sizes sent from Java side by the native resource size. So the texture coordinates are wrong. For instance, in DrawImageBilinear, the size is 5x5 and uiScale is 2.5, so the native resource is of size, 13x13. But the dimensions sent to native are 12x12, so the textures are (0,0) and (12/13, 12/13). So artifacts in final image.
>>
>> D3DSurfaceData:     In D3D, everything is similar to OpenGL, but in D3DBlitTextureToSurface function inside D3DBlitLoops file,  the calculated texture coordinates are sent to D3DDrawTextureWithHint and then data is copied from texture to surface. Here if the native resource size and dimensions sent from Java are not same, the data is copied in parts. This is somehow handling the wrong texture coordinates. So this test does not produce any artifacts in D3D pipeline. But it can in some other case. So this needs to be fixed.
>>
>> GDIWindowSurfacedata: Here the volatile image creates a BufImageSurfaceData as backing resource using the ceil function and while copying from BugImageSurfaceData to a GDIWindowSurface, the Java_sun_java2d_loops_TransformHelper_Transform in native side is able to handle the difference in sizes. So the artifacts are not visible. But I think this should be fixed. So should make changes to getBackupImage function in SunVolatileImage to use clipRound instead of ceil.
>>
>> BufImageSurfaceData : It uses the image raster size to create the native resource.  The raster size is already scaled according to the uiScale. So no need to change this.
>>
>> So in a nutshell,
>> I feel we may not change the code in XRSurfaceData, GLXSurfaceData as linux only support integer scaling. We can do it later if required.
>> We should change code for D3DSurfaceData, GDIWindowSurfacedata and WGLSurfacedata as it may cause artifacts. I have updated the webrev for the same. Please review.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~pbansal/8159142/webrev.01/
>>
>>
>> Regards,
>> Pankaj Bansal
>>
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Wednesday, November 1, 2017 11:46 PM
>> To: Pankaj Bansal; [hidden email]
>> Subject: Re: <AWT Dev> [10] Review Request: JDK-8159142 : Visible
>> artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
>>
>> Hi, Pankaj.
>> D3DSurfaceData, XRSurfaceData and probably other SurfaceData use simple multiplication or ceil(). Should we update them also? and the test to catch the errors there as well.
>>
>> On 30/10/2017 03:39, Pankaj Bansal wrote:
>>> Hi All,
>>>
>>> Please review the fix for JDK 10.
>>>
>>> Bug:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8159142
>>>
>>> Webrev:
>>>
>>> http://cr.openjdk.java.net/~pbansal/8159142/webrev.00/
>>>
>>> Issue:
>>>
>>> The test runs with D3D and OpenGL both. The issue is with the OpenGL
>>> run as there aee some visual artifacts in image when the HiDPI scale
>>> is set to non-integer value like 2.5.
>>>
>>> Fix:
>>>
>>> The issue is due to precision error. In WGLSurfaceData.java, the
>>> width and height is calculated by multiplying the width and height with scale.
>>> But the ceil function was being used instead of round. It should be
>>> using round as at most of the places, the round function is being used.
>>>
>>> Also changed the test to run at uiScale of 2.5 to catch any further errors.
>>>
>>> Regards,
>>>
>>> Pankaj Bansal
>>>
>>
>>
>> --
>> Best regards, Sergey.
>>
>
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.