Hello Pankaj I imported your latest changes and checked the build with the test file. The test now passes with uiScale mentioned in the test case.
The changes look good. Thanks Have a good day Prahalad -----Original Message----- From: Sergey Bylokhov Sent: Saturday, November 18, 2017 3:46 AM To: Pankaj Bansal; Prahalad Kumar Narayanan; 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 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; > 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 > > 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; 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 > > 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. > -- Best regards, Sergey.
