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; awt-...@openjdk.java.net; 2d-dev@openjdk.java.net; 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; awt-...@openjdk.java.net;
> 2d-dev@openjdk.java.net; 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; awt-...@openjdk.java.net
> 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; awt-...@openjdk.java.net
> 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.
