7190978: javax/swing/JComponent/7154030/bug7154030.java fails on mac #955

prsadhuk · 2020-10-30T11:42:53Z

Please review a test fix for an issue seen where robot image capture is wrong if screen scale factor is more, which could result in button or frame to move out of captured dimension of 300,300.
Fixed by setting uiScale=1 in the test which can still reproduce JDK-7154030.
Mach5 job has been run for several iterations in all platforms. Link in JBS.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Testing

	Linux x64	Linux x86	Windows x64	macOS x64
Build	✔️ (5/5 passed)	✔️ (2/2 passed)	✔️ (2/2 passed)	✔️ (2/2 passed)
Test (tier1)	✔️ (9/9 passed)	✔️ (9/9 passed)	✔️ (9/9 passed)	✔️ (9/9 passed)

Issue

JDK-7190978: javax/swing/JComponent/7154030/bug7154030.java fails on mac

Reviewers

Sergey Bylokhov (@mrserb - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/955/head:pull/955
$ git checkout pull/955

bridgekeeper · 2020-10-30T11:44:31Z

👋 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 · 2020-10-30T11:46:31Z

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

swing

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 · 2020-10-30T11:51:56Z

Webrevs

mrserb · 2020-10-31T03:19:40Z

test/jdk/javax/swing/JComponent/7154030/bug7154030.java

@@ -50,7 +52,7 @@
 * @library /lib/client/
 * @build Util
 * @build ExtendedRobot
- * @run main bug7154030
+ * @run main/othervm -Dsun.java2d.uiScale=1 bug7154030


The "sun.java2d.uiScale" does not affect the sizes of the frame on macOS, Is it possible that it was reported on HiDPI screen macOS?

It was reported in macmini (probably connect to hidpi screen) and iMac (I guess having hidpi screen).
Also, I have seen mach5 job failing on windows too.
I have posted failing mach5 jobs in JBS.

Then please double-check that the test will pass after the fix on HiDPI screen. Not sure that -Dsun.java2d.uiScale=1 affects such usecase.

Since mach5 systems are mostly macmini, where it initially failed, and the resultant test is passing on them, I would hope they will pass on hidpi screen they are connected to.

The mach5 macOS test systems currently have ZERO hidpi monitors. I think they are all 1920x1080 non-retina.
They only ever had at most one and that was because it was accidentally connected that way after a lab move.

prsadhuk · 2020-11-03T16:32:24Z

Removed uiScale and move frame from (0,0) location to about mid-screen and made sure robot screenCapture also captures the frame from that location. mach5 iterations is green.

mrserb · 2020-11-03T18:38:15Z

test/jdk/javax/swing/JComponent/7154030/bug7154030.java

+                    Dimension screenSize = Toolkit.getDefaultToolkit().getScreenSize();
+                    locx = screenSize.width/2;
+                    locy = screenSize.height/2;
+                    frame.setLocation(locx, locy);


To move it to the center you can use, setLocationRelativeTo(null) and you will need to get the actual bounds of the window(locx and locy might be ignored/tweaked by the OS)

I know setLocationRelativeTo(null) will move to center of screen but I needed the coordinate for robot interaction.
The locx,locy calculation to move to center of screen is done intentionally as we need to use the same coordinate in robot to capture the screen

The locx,locy calculation to move to center of screen is done intentionally as we need to use the same coordinate in robot to capture the screen

You can use locx/loxy intentionally to move the window, but you can't use it in a robot because of:

you will need to get the actual bounds of the window(locx and locy might be ignored/tweaked by the OS)

Modified to use bounds.

GC.getBounds() and Toolkit.getScreenSize() return the same bounds, but the bounds of the screen, not the window bounds.

I guess we need the screen bounds only, no? I guess by window bounds, u mean frame bounds which is anyway set to 300,300 which I did not change.
If not, can you say which API to use?

Used window bounds...Please let me know if any other comments.

mrserb · 2020-11-07T09:02:02Z

test/jdk/javax/swing/JComponent/7154030/bug7154030.java

                    frame.setVisible(true);
                    frame.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);
                }
            });

-            robot.waitForIdle(500);
-            imageInit = robot.createScreenCapture(new Rectangle(0, 0, 300, 300));


The usage of size 300x300 has the same issues as locx/locy before. It is part of the window bounds.

but 300x300 is set in frame setSize..if I use frame getBounds instead of already set 300x300, that will not be right, according to me...What if getBounds() has some bug (in some platform) and return some other width/height (say 1 pixel less, we might have similar bug in 8196465) that what is set in setSize, then the test might pass even though robot is capturing wrong bounds.

but 300x300 is set in frame setSize..if I use frame getBounds instead of already set 300x300, that will not be right, according to me...

The test should set the size 300,300 to the frame and then request and use the real bounds of the frame.

What if getBounds() has some bug (in some platform) and return some other width/height (say 1 pixel less, we might have similar bug in 8196465) that what is set in setSize, then the test might pass even though robot is capturing wrong bounds.

The getBounds() simply returns the size which was set by the native system, so is it a bug in getBounds() or the real bounds of the frame "say 1 pixel less"?

Any further feedback?

openjdk · 2020-11-11T01:40:41Z

@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:

7190978: javax/swing/JComponent/7154030/bug7154030.java fails on mac

Reviewed-by: serb

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 201 new commits pushed to the master branch:

5de99da: 8237495: Java MIDI fails with a dereferenced memory error when asked to send a raw 0xF7
be63525: 8211999: Window positioning bugs due to overlapping GraphicsDevice bounds (Windows/HiDPI)
0a41ca6: 8254354: Add a withInvokeExactBehavior() VarHandle combinator
d6f1463: 8233332: Need to create exploded tests covering all forms of modules
f2a0bf3: 8256017: Remove unused elapsedTimer constructor
7d4e86b: 8138588: VerifyMergedCPBytecodes option cleanup needed
a7f4691: 8244088: [Regression] Switch of Gnome theme ends up in deadlocked UI
bd3e65b: 8256052: Remove unused allocation type from fieldInfo
6d8acd2: 8256066: Tests use deprecated TestNG API that is no longer available in new versions
643969a: 8255822: Zero: improve build-time JVMTI handling
... and 191 more: https://git.openjdk.java.net/jdk/compare/caec8d2208fea0ddd4387a4530b4914645482377...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.

prsadhuk · 2020-11-11T08:08:03Z

/integrate

openjdk · 2020-11-11T08:09:00Z

@prsadhuk Since your change was applied there have been 203 commits pushed to the master branch:

35284e4: 8255916: [macos] javax/swing/JInternalFrame/6647340/bug6647340.java timed out
8638cd9: 8255625: AArch64: Implement Base64.encodeBlock accelerator/intrinsic
5de99da: 8237495: Java MIDI fails with a dereferenced memory error when asked to send a raw 0xF7
be63525: 8211999: Window positioning bugs due to overlapping GraphicsDevice bounds (Windows/HiDPI)
0a41ca6: 8254354: Add a withInvokeExactBehavior() VarHandle combinator
d6f1463: 8233332: Need to create exploded tests covering all forms of modules
f2a0bf3: 8256017: Remove unused elapsedTimer constructor
7d4e86b: 8138588: VerifyMergedCPBytecodes option cleanup needed
a7f4691: 8244088: [Regression] Switch of Gnome theme ends up in deadlocked UI
bd3e65b: 8256052: Remove unused allocation type from fieldInfo
... and 193 more: https://git.openjdk.java.net/jdk/compare/caec8d2208fea0ddd4387a4530b4914645482377...master

Your commit was automatically rebased without conflicts.

Pushed as commit 5181f9c.

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

Reviewed-by: serb

prsadhuk added 2 commits October 30, 2020 09:53

Initial fix

62fbb21

7190978: javax/swing/JComponent/7154030/bug7154030.java fails on mac

f3f9f9f

openjdk bot added the rfr Pull request is ready for review label Oct 30, 2020

openjdk bot added the swing client-libs-dev@openjdk.org label Oct 30, 2020

mrserb reviewed Oct 31, 2020

View reviewed changes

Remove uiScale

b6a90f1

mrserb reviewed Nov 3, 2020

View reviewed changes

prsadhuk added 2 commits November 4, 2020 14:33

use getBounds()

cf936ae

Used window getbounds

63209dd

mrserb reviewed Nov 7, 2020

View reviewed changes

Address review comment

f5fd716

mrserb approved these changes Nov 11, 2020

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Nov 11, 2020

openjdk bot closed this Nov 11, 2020

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Nov 11, 2020

prsadhuk deleted the JDK-7190978 branch November 11, 2020 08:08

openjdk-notifier bot referenced this pull request Nov 11, 2020

7190978: javax/swing/JComponent/7154030/bug7154030.java fails on mac

5181f9c

Reviewed-by: serb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

7190978: javax/swing/JComponent/7154030/bug7154030.java fails on mac #955

7190978: javax/swing/JComponent/7154030/bug7154030.java fails on mac #955

prsadhuk commented Oct 30, 2020 •

edited by openjdk bot

bridgekeeper bot commented Oct 30, 2020

openjdk bot commented Oct 30, 2020

mlbridge bot commented Oct 30, 2020 •

edited

mrserb Oct 31, 2020

prsadhuk Oct 31, 2020

mrserb Nov 2, 2020

prsadhuk Nov 2, 2020

prrace Nov 2, 2020

prsadhuk commented Nov 3, 2020

mrserb Nov 3, 2020

prsadhuk Nov 4, 2020

mrserb Nov 4, 2020

prsadhuk Nov 4, 2020

mrserb Nov 5, 2020

prsadhuk Nov 5, 2020

prsadhuk Nov 6, 2020

mrserb Nov 7, 2020

prsadhuk Nov 7, 2020

mrserb Nov 7, 2020

prsadhuk Nov 10, 2020

openjdk bot commented Nov 11, 2020 •

edited

prsadhuk commented Nov 11, 2020

openjdk bot commented Nov 11, 2020

7190978: javax/swing/JComponent/7154030/bug7154030.java fails on mac #955

7190978: javax/swing/JComponent/7154030/bug7154030.java fails on mac #955

Conversation

prsadhuk commented Oct 30, 2020 • edited by openjdk bot

Progress

Testing

Issue

Reviewers

Download

bridgekeeper bot commented Oct 30, 2020

openjdk bot commented Oct 30, 2020

mlbridge bot commented Oct 30, 2020 • edited

Webrevs

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

prsadhuk commented Nov 3, 2020

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

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

openjdk bot commented Nov 11, 2020 • edited

prsadhuk commented Nov 11, 2020

openjdk bot commented Nov 11, 2020

prsadhuk commented Oct 30, 2020 •

edited by openjdk bot

mlbridge bot commented Oct 30, 2020 •

edited

openjdk bot commented Nov 11, 2020 •

edited