Re: RFR: 8160755: bug6492108.java test fails with exception Image comparison failed at (0, 0) for image 4 in GTK L [v7]
On Tue, 4 Jun 2024 15:37:00 GMT, Abhishek Kumar wrote: >> bug6492108.java test always fails in GTK L 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: > > copyright year update LGTM. - Marked as reviewed by tr (Committer). PR Review: https://git.openjdk.org/jdk/pull/19381#pullrequestreview-2098039565
Re: RFR: JDK-8333360 : PrintNullString.java doesn't use float arguments
On Tue, 4 Jun 2024 12:07:40 GMT, Renjith Kannath Pariyangad
wrote:
> Hi Reviewers,
> I have updated the test case with passing float value for evaluation and a
> typo. Please review and let me know your suggestions if any.
test/jdk/java/awt/print/PrinterJob/PrintNullString.java line 172:
> 170:
> 171: try {
> 172: g2d.drawString(emptyIterator, 20.0f, 180.0f);
Does the line below also need to change ?
`g2d.drawString("FAILURE: No IAE for empty iterator, float", 20.0f 180.0f);`
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19540#discussion_r1626943802
Re: RFR: 8160755: bug6492108.java test fails with exception Image comparison failed at (0, 0) for image 4 in GTK L [v7]
On Tue, 4 Jun 2024 15:37:00 GMT, Abhishek Kumar wrote: >> bug6492108.java test always fails in GTK L 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: > > copyright year update Ran bug6492108.java with changes. LGTM. - Marked as reviewed by dnguyen (Committer). PR Review: https://git.openjdk.org/jdk/pull/19381#pullrequestreview-2096958201
Re: RFR: 8332103: since-checker - Add missing @ since tags to java.desktop [v2]
On Mon, 3 Jun 2024 23:33:53 GMT, Nizar Benalla wrote: > method: void java.awt.geom.Path2D.Double.trimToSize(): `@since` version is 9 > instead of 10 > method: void java.awt.geom.Path2D.Float.trimToSize(): `@since` version is 9 > instead of 10 In JDK 10, a new method `trimToSize` was added to Path2D. It is marked with `@since 10`. `Path2D.Float` and `Path2D.Double` override the method or rather implement it. Does this require an explicit `@since` tag? I'm unsure about it. - PR Comment: https://git.openjdk.org/jdk/pull/19192#issuecomment-2147928711
Re: RFR: 8332103: since-checker - Add missing @ since tags to java.desktop [v2]
On Wed, 15 May 2024 03:38:29 GMT, Nizar Benalla wrote:
>> If you're currently reviewing this PR, thank you!
>> Most fixes here are according to the reports by the since checker tool in
>> #18934 and are pretty simple.
>>
>> To make reviewing easier
>> - `BasicSliderUI` has the constructor `public BasicSliderUI(JSlider b)` for
>> a long time so the default constructor (without parameters) didn't exist
>> until JDK 16
>>
>> For the `package-info` files, it is pretty hard to find source code of JDK
>> 1-5 so I used the `grep` command to find the oldest instance of an `@since`
>> in those packages.
>>
>> I found instances of `@since 1.1` in the other packages but
>> `javax/swing/plaf/synth/package-info.java` might be worth checking as most
>> classes there had no `@since`.
>
> Nizar Benalla has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Swing was added in JDK 1.2
src/java.desktop/share/classes/java/awt/geom/Path2D.java line 297:
> 295: /**
> 296: * @since 10
> 297: */
Not sure it's required…
If it is, you should also add explicit `{@inheritDoc}`:
Suggestion:
/**
* {@inheritDoc}
*
* @since 10
*/
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19192#discussion_r1626298196
Re: RFR: 8332103: since-checker - Add missing @ since tags to java.desktop [v2]
On Tue, 4 Jun 2024 00:02:56 GMT, Nizar Benalla wrote: >> It seems that BasicSliderUI() was added by the mistake? it was not mentioned >> in the bug report...Seems it is too late to delete it? > > I'm sorry but `method: void javax.swing.plaf.basic.BasicSliderUI.()` > refers to the constructor, > as I use [this > method](https://docs.oracle.com/en/java/javase/22/docs/api/java.compiler/javax/lang/model/element/ExecutableElement.html#getSimpleName()) > to get a method's name. > > I am saying that there was no default constructor before JDK 16 as it doesn't > appear in the compiler's historical data until then and therefore warrants an > `@since` > > I am stealing my colleagues words but here is the general rule for when we > want to add an `@since` until we publish a doc with rules for `@since` > >> As a practical rule for deciding whether any declaration is new or not, >> imagine writing a test program that refers to the most specific form of the >> declaration. If that test program does not compile on JDK version N-1 and >> does compile on version N, then it warrants having `@since N`. Put another >> way, `@since N` should identify the first release in which the declaration >> can be used in the given form > It seems that BasicSliderUI() was added by the mistake? it was not mentioned > in the bug report...Seems it is too late to delete it? I agree. It shouldn't have been added. Instead of adding `@since`, the constructor should be removed. It requires a CSR. The longer it exists, the more chances there are that it's used. - PR Review Comment: https://git.openjdk.org/jdk/pull/19192#discussion_r1626272799
Re: RFR: 8160755: bug6492108.java test fails with exception Image comparison failed at (0, 0) for image 4 in GTK L [v7]
On Tue, 4 Jun 2024 15:37:00 GMT, Abhishek Kumar wrote: >> bug6492108.java test always fails in GTK L 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: > > copyright year update Marked as reviewed by aivanov (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19381#pullrequestreview-2096810770
Re: RFR: 8160755: bug6492108.java test fails with exception Image comparison failed at (0, 0) for image 4 in GTK L [v5]
On Tue, 4 Jun 2024 15:21:30 GMT, Alexey Ivanov wrote: >> Updated. > > I can't see it in the PR. Didn't push? Oh my bad, I missed to add that changes. Pushed now. - PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626228348
Re: RFR: 8160755: bug6492108.java test fails with exception Image comparison failed at (0, 0) for image 4 in GTK L [v7]
> bug6492108.java test always fails in GTK L 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: copyright year update - Changes: - all: https://git.openjdk.org/jdk/pull/19381/files - new: https://git.openjdk.org/jdk/pull/19381/files/0afa8d87..649c3ad8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19381=06 - incr: https://webrevs.openjdk.org/?repo=jdk=19381=05-06 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19381.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19381/head:pull/19381 PR: https://git.openjdk.org/jdk/pull/19381
Re: RFR: 8160755: bug6492108.java test fails with exception Image comparison failed at (0, 0) for image 4 in GTK L [v5]
On Tue, 4 Jun 2024 14:46:37 GMT, Abhishek Kumar wrote:
>> src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java
>> line 1:
>>
>>> 1: /*
>>
>> You should update the copyright year.
>
> Updated.
I can't see it in the PR. Didn't push?
>> 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.
>
> Don't find adding `k` to diagnostic message useful.. failure message is more
> relevant now about the images which are compared.
The images themselves are more useful for sure. Thanks!
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626209941
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626214172
Re: RFR: 8160755: bug6492108.java test fails with exception Image comparison failed at (0, 0) for image 4 in GTK L [v5]
On Tue, 4 Jun 2024 13:45:44 GMT, Alexey Ivanov wrote:
>> Abhishek Kumar has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Code formatting update
>
> src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java line
> 1:
>
>> 1: /*
>
> You should update the copyright year.
Updated.
> 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.
Updated.
> 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.
Updated to run this on EDT.
> 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`.
Updated to use same style for both places.
> 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).
Ok.. Updated
[Util.compareBufferedImages](https://github.com/openjdk/jdk/blob/8d3de45f4dfd60dc4e2f210cb0c085fcf6efb8e2/test/jdk/javax/swing/regtesthelpers/Util.java#L59-L80)
to compare images.
> 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.
Used Util to compare images, so these lines are removed now.
> 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.
Don't find adding `k` to diagnostic message useful.. failure message is more
relevant now about the images which are compared.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626152744
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626146072
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626148142
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626150340
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626150884
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626149792
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626152392
Re: RFR: 8160755: bug6492108.java test fails with exception Image comparison failed at (0, 0) for image 4 in GTK L [v6]
> bug6492108.java test always fails in GTK L 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: Review comment update - Changes: - all: https://git.openjdk.org/jdk/pull/19381/files - new: https://git.openjdk.org/jdk/pull/19381/files/f94e4b0c..0afa8d87 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19381=05 - incr: https://webrevs.openjdk.org/?repo=jdk=19381=04-05 Stats: 32 lines in 1 file changed: 4 ins; 14 del; 14 mod Patch: https://git.openjdk.org/jdk/pull/19381.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19381/head:pull/19381 PR: https://git.openjdk.org/jdk/pull/19381
Re: RFR: 8160755: bug6492108.java test fails with exception Image comparison failed at (0, 0) for image 4 in GTK L [v3]
On Mon, 3 Jun 2024 05:48:18 GMT, Abhishek Kumar wrote: >> Previous formatting was less confusing and aligned with Java Coding Style >> Guidelines. >> >> The parameters to the method are aligned to the opening parenthesis; the >> `throws` clause is not part of the parameters and it's placed on another >> line according to rules of wrapping lines (it should've used >> double-indentation of 8 spaces rather than 4 spaces to indicate a >> continuation line). > > Reverted back to previous formatting with double-indentation. Hopefully this > should be ok now. This formatting looks good to me. - PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1626008412
Re: RFR: 8160755: bug6492108.java test fails with exception Image comparison failed at (0, 0) for image 4 in GTK L [v5]
On Mon, 3 Jun 2024 05:52:31 GMT, Abhishek Kumar wrote:
>> bug6492108.java test always fails in GTK L 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 (" +
Re: RFR: JDK-8333360 : PrintNullString.java doesn't use float arguments
On Tue, 4 Jun 2024 12:07:40 GMT, Renjith Kannath Pariyangad wrote: > Hi Reviewers, > I have updated the test case with passing float value for evaluation and a > typo. Please review and let me know your suggestions if any. Marked as reviewed by aivanov (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19540#pullrequestreview-2096332476
RFR: JDK-8333360 : PrintNullString.java doesn't use float arguments
Hi Reviewers, I have updated the test case with passing float value for evaluation and a typo. Please review and let me know your suggestions if any. - Commit messages: - JDK-860 : PrintNullString.java doesn't use float arguments Changes: https://git.openjdk.org/jdk/pull/19540/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19540=00 Issue: https://bugs.openjdk.org/browse/JDK-860 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19540.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19540/head:pull/19540 PR: https://git.openjdk.org/jdk/pull/19540
Re: RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized [v2]
On Mon, 3 Jun 2024 06:07:37 GMT, Prasanta Sadhukhan
wrote:
>> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 1031:
>>
>>> 1029: (p.x >= (frame.origin.x + contentRect.size.width -
>>> 3)) ||
>>> 1030: (fabs(frame.origin.x - p.x) < 3) ||
>>> 1031: (fabs(frame.origin.y - p.y) < 3)) {
>>
>> Does `fabs` use float? It makes code more compact but it may be imprecise
>> and less performant than integer arithmetics.
>
> Yes..https://man7.org/linux/man-pages/man3/fabs.3.html and [CGFLoat
> ](https://developer.apple.com/documentation/corefoundation/cgfloat?language=objc)
> can be double/float and fabs can accomodate both, in this case it uses
> double...We have tested on 3 different systems without any issue so would
> keep it for now until we find any adverse effect and also it is better to
> accommodate double in hi-dpi environment which is the case in OSX
I see, the coordinates are floats, then it's perfectly fine. I assumed the
coordinates were integers.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1625831262
