RFR: 8160755: bug6492108.java test fails with exception Image comparison failed at (0, 0) for image 4 in GTK L&F [v5]

[Alexey Ivanov]

On Mon, 3 Jun 2024 05:52:31 GMT, Abhishek Kumar <abhiscxk at openjdk.org> wrote:

>> bug6492108.java test always fails in GTK L&F in single as well as dual screen linux machines. Since this test was not marked as "_headful_" in it's initial version, it never failed but after the fix of [JDK-8287051](https://bugs.openjdk.org/browse/JDK-8287051) this test was problem listed as it always
>> failed which is captured in the JBS.
>> The reason of failure is the pixel color mismatch between JEditorPane and JTextArea/JTextPane which is caused by the JEditorPane's default opaque value which is false for GTK3 at [L767](https://github.com/kumarabhi006/jdk/blob/c642f44bbe1e4cdbc23496a34ddaae30990ce7c0/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java#L767).
>> In initial load JEditorPane, JTextArea and JTextPane components are opaque at [L718](https://github.com/kumarabhi006/jdk/blame/6c7656678916ff3f5c9fc70efcbb69ce76801458/jdk/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java#L718) but after the fix for [JDK-8145547](https://bugs.openjdk.org/browse/JDK-8145547) the implementation was changed to provide conditional support for GTK3 on linux where few components like Editor Pane, Formatted text Field, Password Field etc are opaque only if the  [GTK version is not 3](https://github.com/kumarabhi006/jdk/blame/73d2181d56063f6015e4fc42e130591bee39bc36/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java#L746C21-L746C21).
>> JTextPane's issue was observed by [JDK-8218479](https://bugs.openjdk.org/browse/JDK-8218479) and then the default opacity is set to true for JTextPane [irrespective of GTK version](https://github.com/kumarabhi006/jdk/blame/c642f44bbe1e4cdbc23496a34ddaae30990ce7c0/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java#L750C16-L750C16).
>> Extending the fix in isOpaque() method in GTKStyle.java for JEditorPane similar to JTextPane resolves the issue.
>> 
>> Test verified in both single and dual screen linux machines.
>
> Abhishek Kumar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Code formatting update

Changes requested by aivanov (Reviewer).

src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java line 1:

> 1: /*

You should update the copyright year.

test/jdk/com/sun/java/swing/plaf/gtk/bug6492108.java line 64:

> 62:         } catch (Exception e) {
> 63:             throw new SkippedException("GTK LAF is not supported on this system;" +
> 64:                     " test passes");

Suggestion:

            throw new SkippedException("GTK LAF is not supported on this system");

I'd remove the additional message. I believe it was `println` before.

test/jdk/com/sun/java/swing/plaf/gtk/bug6492108.java line 116:

> 114:     }
> 115: 
> 116:     private void onBackgroundThread20() {

This method should run on EDT because it accesses the state of the components.

test/jdk/com/sun/java/swing/plaf/gtk/bug6492108.java line 124:

> 122:             Point loc = ref.getLocationOnScreen();
> 123:             Rectangle refRect =
> 124:                 new Rectangle(loc.x, loc.y, ref.getWidth(), ref.getHeight());

Suggestion:

            Rectangle refRect = new Rectangle(loc, ref.getSize());

test/jdk/com/sun/java/swing/plaf/gtk/bug6492108.java line 133:

> 131:                 Rectangle testRect =
> 132:                     new Rectangle(loc.x, loc.y,
> 133:                                   test.getWidth(), test.getHeight());

Suggestion:

                loc = test.getLocationOnScreen();
                Rectangle testRect = new Rectangle(test.getLocationOnScreen(),
                                                   test.getSize());

Choose one style and adjust both statements, `refRect` and `testRect`.

test/jdk/com/sun/java/swing/plaf/gtk/bug6492108.java line 136:

> 134:                 BufferedImage testimg = robot.createScreenCapture(testRect);
> 135: 
> 136:                 if (refimg.getWidth() != testimg.getWidth()

I suggest moving the part that compares the images into a helper method. Alternatively, you can use [`Util.compareBufferedImages`](https://github.com/openjdk/jdk/blob/8d3de45f4dfd60dc4e2f210cb0c085fcf6efb8e2/test/jdk/javax/swing/regtesthelpers/Util.java#L59-L80).

test/jdk/com/sun/java/swing/plaf/gtk/bug6492108.java line 137:

> 135: 
> 136:                 if (refimg.getWidth() != testimg.getWidth()
> 137:                    || refimg.getHeight() != testimg.getHeight())

Suggestion:

                if (refimg.getWidth() != testimg.getWidth()
                    || refimg.getHeight() != testimg.getHeight())

One more space, it should be aligned to the inside of the parenthesis.

test/jdk/com/sun/java/swing/plaf/gtk/bug6492108.java line 148:

> 146:                         if (refPixel != testPixel) {
> 147:                             fail("Image comparison failed at (" +
> 148:                                  x + "," + y + ") for image " + index);

Adding `count` and `k` to diagnostic message could help identifying the failing case quicker.

-------------

PR Review: https://git.openjdk.org/jdk/pull/19381#pullrequestreview-2096399951
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626048836
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626007721
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626017821
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626017048
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626024722
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626046506
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626023890
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626048214