8287743: javax/swing/text/CSSBorder/6796710/bug6796710.java failed #9011

prsadhuk · 2022-06-03T09:46:11Z

Test is failing in iMac CI systems owing to color difference of 1

x 0 y 0 rgb1: fff0f0f0 rgb2: fff0eff0

Added minor color tolerance check. CI testing is green

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8287743: javax/swing/text/CSSBorder/6796710/bug6796710.java failed

Reviewers

Alexey Ivanov (@aivanov-jdk - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/9011/head:pull/9011
$ git checkout pull/9011

Update a local copy of the PR:
$ git checkout pull/9011
$ git pull https://git.openjdk.org/jdk pull/9011/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 9011

View PR using the GUI difftool:
$ git pr show -t 9011

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/9011.diff

bridgekeeper · 2022-06-03T09:47:11Z

👋 Welcome back psadhukhan! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2022-06-03T09:50:44Z

@prsadhuk The following label will be automatically applied to this pull request:

client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2022-06-03T09:54:48Z

Webrevs

mrserb · 2022-06-04T21:03:12Z

This test does not check the resulted color against the golden value. It validates the same area of the window which is captured twice, so even monitor color profile cannot affect it. Why do we have a difference of 1? Is it an issue in the rendering or robot?

prsadhuk · 2022-06-06T15:40:53Z

Used Robot.createMultiResolutionScreenCapture instead of Robot.createScreenCapture to get proper image variant for the iMac system.
Several iterations of the test in intended system is ok.

mrserb · 2022-06-06T21:11:03Z

test/jdk/javax/swing/text/CSSBorder/6796710/bug6796710.java

-        return robot.createScreenCapture(rect);
+        MultiResolutionImage img = robot.createMultiResolutionScreenCapture(rect);
+        return (BufferedImage)img.getResolutionVariant(rect.width, rect.height);
+        //return robot.createScreenCapture(rect);


Isn't the old and new code should get the same result? The old createScreenCapture always creates an image of the low dpi quality. The createMultiResolutionScreenCapture creates two images low/hi dpi, but since you request the same size "rect" the low dpi image is returned, no?

Maybe it's different in iMac systems..I dont have access to this system to analyse further nor do I have much knowledge on this 2D/awt multiresolution image side of things..

If I remember correctly, MRI will contain only one image captured by Robot, so it would return the only image it stores, which would not be scaled down if High DPI is in effect.

However, the test is supposed to run with UIScale=1, so this should make any difference.

On the HiDPI screen that MRI will have two images, and my expectation is that the image requested by the updated test and the image used before the fix should be the same.

How to find those 2 bufferedimages returned by createMultiResolutionScreenCapture and createScreenCapture are same or not apart from pixel check (which shows obviously they are not same and color differ by 1)?

Probably we can apply the defensive fix of color tolerance as per 1st iteration ?

It still fails once or twice with createMultiResolutionScreenCapture if the test iteration is increased, so I think color tolerance, even if defensive, is the one to go for.

On the HiDPI screen that MRI will have two images, and my expectation is that the image requested by the updated test and the image used before the fix should be the same.

In this case, yes, the image will be the same as before: a scaled down bitmap you get with regular createScreenCapture.

How to find those 2 bufferedimages returned by createMultiResolutionScreenCapture and createScreenCapture are same or not apart from pixel check (which shows obviously they are not same and color differ by 1)?

You can get the largest variant, and compare colors there. Yet the size of the two images will be different, most likely.

It still fails once or twice with createMultiResolutionScreenCapture if the test iteration is increased, so I think color tolerance, even if defensive, is the one to go for.

So we return back to the initial question, why the code which capture same window rendered in same color in the same location produce different results? Why the tolerance is needed, Is it possible that the robot or rendering generate some garbage or override/read something wrong in memory?

aivanov-jdk

Shall the copyright year be updated?

test/jdk/javax/swing/text/CSSBorder/6796710/bug6796710.java

openjdk · 2022-06-10T12:35:27Z

@prsadhuk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8287743: javax/swing/text/CSSBorder/6796710/bug6796710.java failed

Reviewed-by: aivanov

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 321 new commits pushed to the master branch:

5d0e8b6: 8288203: runtime/ClassUnload/UnloadTestWithVerifyDuringGC.java fails with release VMs
975316e: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows
0901548: 8283724: Incorrect description for jtreg-failure-handler option
dae4c49: 8286197: C2: Optimize MemorySegment shape in int loop
94b473e: 8280454: G1: ClassLoaderData verification keeps CLDs live that causes problems with VerifyDuringGC during Remark
900d967: 8287924: Avoid redundant HashMap.containsKey call in EnvHelp.mapToHashtable
d482d7f: 8286160: (fs) Files.exists returns unexpected results with C:\pagefile.sys because it's not readable
edff51e: 8284858: Start of release updates for JDK 20
2671443: 8286171: HttpClient/2 : Expect:100-Continue blocks indefinitely when response is not 100
59b0de6: 8288048: Build failure with GCC 6 after JDK-8286562
... and 311 more: https://git.openjdk.org/jdk/compare/40e99a19f20dde0916684f93c17c51c7c5de109a...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

test/jdk/javax/swing/text/CSSBorder/6796710/bug6796710.java

prsadhuk · 2022-06-10T13:35:39Z

/integrate

openjdk · 2022-06-10T13:37:34Z

Going to push as commit fcb35ed.
Since your change was applied there have been 322 commits pushed to the master branch:

bdd64d6: 8288181: AArch64: clean up out-of-date comments
5d0e8b6: 8288203: runtime/ClassUnload/UnloadTestWithVerifyDuringGC.java fails with release VMs
975316e: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows
0901548: 8283724: Incorrect description for jtreg-failure-handler option
dae4c49: 8286197: C2: Optimize MemorySegment shape in int loop
94b473e: 8280454: G1: ClassLoaderData verification keeps CLDs live that causes problems with VerifyDuringGC during Remark
900d967: 8287924: Avoid redundant HashMap.containsKey call in EnvHelp.mapToHashtable
d482d7f: 8286160: (fs) Files.exists returns unexpected results with C:\pagefile.sys because it's not readable
edff51e: 8284858: Start of release updates for JDK 20
2671443: 8286171: HttpClient/2 : Expect:100-Continue blocks indefinitely when response is not 100
... and 312 more: https://git.openjdk.org/jdk/compare/40e99a19f20dde0916684f93c17c51c7c5de109a...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-06-10T13:37:51Z

@prsadhuk Pushed as commit fcb35ed.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8287743: javax/swing/text/CSSBorder/6796710/bug6796710.java failed

37b97a3

openjdk bot added the rfr Pull request is ready for review label Jun 3, 2022

openjdk bot added the client client-libs-dev@openjdk.org label Jun 3, 2022

Use RObot.createMultiResoutionCapture

dcc0858

mrserb reviewed Jun 6, 2022

View reviewed changes

Add color tolerance fix

afd6fb9

aivanov-jdk approved these changes Jun 10, 2022

View reviewed changes

test/jdk/javax/swing/text/CSSBorder/6796710/bug6796710.java Outdated Show resolved Hide resolved

test/jdk/javax/swing/text/CSSBorder/6796710/bug6796710.java Outdated Show resolved Hide resolved

test/jdk/javax/swing/text/CSSBorder/6796710/bug6796710.java Outdated Show resolved Hide resolved

openjdk bot added the ready Pull request is ready to be integrated label Jun 10, 2022

prsadhuk added 2 commits June 10, 2022 13:43

Fix review comments

33e0613

Fix review comments

51c37d9

aivanov-jdk approved these changes Jun 10, 2022

View reviewed changes

test/jdk/javax/swing/text/CSSBorder/6796710/bug6796710.java Outdated Show resolved Hide resolved

test/jdk/javax/swing/text/CSSBorder/6796710/bug6796710.java Outdated Show resolved Hide resolved

Fix review comments

c702892

aivanov-jdk approved these changes Jun 10, 2022

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Jun 10, 2022

openjdk bot closed this Jun 10, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jun 10, 2022

prsadhuk deleted the JDK-8287743 branch June 10, 2022 14:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8287743: javax/swing/text/CSSBorder/6796710/bug6796710.java failed #9011

8287743: javax/swing/text/CSSBorder/6796710/bug6796710.java failed #9011

prsadhuk commented Jun 3, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jun 3, 2022

openjdk bot commented Jun 3, 2022

mlbridge bot commented Jun 3, 2022 •

edited

Loading

mrserb commented Jun 4, 2022

prsadhuk commented Jun 6, 2022

mrserb Jun 6, 2022

prsadhuk Jun 7, 2022

aivanov-jdk Jun 9, 2022 •

edited

Loading

mrserb Jun 10, 2022

prsadhuk Jun 10, 2022

prsadhuk Jun 10, 2022

prsadhuk Jun 10, 2022

aivanov-jdk Jun 10, 2022

mrserb Jun 13, 2022

aivanov-jdk left a comment

openjdk bot commented Jun 10, 2022 •

edited

Loading

prsadhuk commented Jun 10, 2022

openjdk bot commented Jun 10, 2022

openjdk bot commented Jun 10, 2022

8287743: javax/swing/text/CSSBorder/6796710/bug6796710.java failed #9011

8287743: javax/swing/text/CSSBorder/6796710/bug6796710.java failed #9011

Conversation

prsadhuk commented Jun 3, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Jun 3, 2022

openjdk bot commented Jun 3, 2022

mlbridge bot commented Jun 3, 2022 • edited Loading

Webrevs

mrserb commented Jun 4, 2022

prsadhuk commented Jun 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aivanov-jdk Jun 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aivanov-jdk left a comment

Choose a reason for hiding this comment

openjdk bot commented Jun 10, 2022 • edited Loading

prsadhuk commented Jun 10, 2022

openjdk bot commented Jun 10, 2022

openjdk bot commented Jun 10, 2022

prsadhuk commented Jun 3, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Jun 3, 2022 •

edited

Loading

aivanov-jdk Jun 9, 2022 •

edited

Loading

openjdk bot commented Jun 10, 2022 •

edited

Loading