Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8252133: The java/awt/GraphicsDevice/DisplayModes/CycleDMImage.java fails if metal pipeline is active #554

Closed
wants to merge 7 commits into from

Conversation

@mrserb
Copy link
Member

@mrserb mrserb commented Oct 8, 2020

This bug easily reproduced by the test in question on the dual video card systems
when the metal pipeline is active. But it is possible to reproduce it in the OGL
pipeline as well, but it is required some additional steps.

Problem description:
Our CGraphicsEnvironment maintains the list of active graphics devices. The one
important feature of this CGraphicsEnvironment is to invalidate the old devices and
map them to the new devices. For example, if the user got a reference to the device,
and this device was removed then this reference will refer to the main screen.

The problem in the current implementation arise when the system has two video cards:
1 The user get some GraphicsDevice
2 The user sets the full-screen window for this device
3 The user change screen resolution for this device
4 The resolution of the screen is not changed ->> BUG.

The problem is that somewhere after step 1 or 2 and before step 3 the macOS decided
to switch to the discrete video card, but it does not report the old device(integrated VC)
as removed, because actually no screens were removed.

Since it was not reported as removed we did not invalidate it and did not map it to the
new device ->> request to change the screen resolution at step 3 send to some non existed
deviceID.

As a fix I suggest to change this current logic:

  • Invalidate devices reported by macOS as removed
  • Initialize the main screen
  • Initialize all NEW screens

To this logic:

  • Ignore devices reported by the macOS as removed
  • Initialize the main screen
  • Initialize all NEW screens
  • Check that the main device is in the list of all NEW devices
  • Invalidate all OLD devices which are not in the list of NEW devices

The old review request:
https://mail.openjdk.java.net/pipermail/2d-dev/2020-August/011011.html


Progress

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

Testing

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

Issue

  • JDK-8252133: The java/awt/GraphicsDevice/DisplayModes/CycleDMImage.java fails if metal pipeline is active

Reviewers

Download

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

@mrserb
Copy link
Member Author

@mrserb mrserb commented Oct 8, 2020

/cc lanai

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Oct 8, 2020

👋 Welcome back serb! 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
Copy link

@openjdk openjdk bot commented Oct 8, 2020

@mrserb The label lanai is not a valid label. These labels are valid:

  • serviceability
  • hotspot
  • sound
  • hotspot-compiler
  • kulla
  • i18n
  • shenandoah
  • jdk
  • javadoc
  • 2d
  • security
  • swing
  • hotspot-runtime
  • jmx
  • build
  • nio
  • beans
  • core-libs
  • compiler
  • net
  • hotspot-gc
  • hotspot-jfr
  • awt
@openjdk
Copy link

@openjdk openjdk bot commented Oct 8, 2020

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

  • awt

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.

@openjdk openjdk bot added the awt label Oct 8, 2020
@mrserb
Copy link
Member Author

@mrserb mrserb commented Oct 8, 2020

/cc 2d

@mrserb
Copy link
Member Author

@mrserb mrserb commented Oct 8, 2020

/label remove awt

@openjdk openjdk bot added the 2d label Oct 8, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Oct 8, 2020

@mrserb
The 2d label was successfully added.

@openjdk openjdk bot removed the awt label Oct 8, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Oct 8, 2020

@mrserb
The awt label was successfully removed.

@mrserb mrserb marked this pull request as ready for review Oct 8, 2020
@openjdk openjdk bot added the rfr label Oct 8, 2020
@mlbridge
Copy link

@mlbridge mlbridge bot commented Oct 8, 2020

@mrserb mrserb marked this pull request as draft Oct 8, 2020
@openjdk openjdk bot removed the rfr label Oct 8, 2020
Sergey Bylokhov and others added 3 commits Oct 9, 2020
Make sure the main display is in the list of all displays.
Prevent duplicate main screen if the switch has happened.
@mrserb mrserb marked this pull request as ready for review Oct 11, 2020
@openjdk openjdk bot added the rfr label Oct 11, 2020
@prrace
prrace approved these changes Oct 19, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Oct 19, 2020

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

8252133: The java/awt/GraphicsDevice/DisplayModes/CycleDMImage.java fails if metal pipeline is active

Reviewed-by: prr

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

  • 294e070: 8254998: C2: assert(!n->as_Loop()->is_transformed_long_loop()) failure with -XX:StressLongCountedLoop=1
  • 3ee0380: 8234393: [macos] printing ignores printer tray
  • 3f9c8a3: 8254966: Remove unused code from Matcher
  • 21e67e5: 8225790: Two NestedDialogs tests fail on Ubuntu
  • 017d151: 8254842: [JVMCI] copy thread name when attaching libgraal thread to HotSpot
  • 5d1397f: 8254282: Add Linux x86_32 builds to submit workflow
  • 7ba6a6b: 8251158: Implementation of JEP 387: Elastic Metaspace
  • 5fedfa7: 8251271: C2: Compile::_for_igvn list is corrupted after RenumberLiveNodes
  • 98ec4a6: 8254805: compiler/debug/TestStressCM.java is still failing
  • 355f44d: 8254994: [x86] C1 StubAssembler::call_RT, "call_offset might not be initialized"
  • ... and 154 more: https://git.openjdk.java.net/jdk/compare/ced46b19f70382d50a45fb54a58bcb9f742fb008...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.

@openjdk openjdk bot added the ready label Oct 19, 2020
: new CGraphicsDevice(id));
}
// fetch the main display again, the old value might be outdated
mainDisplayID = getMainDisplayID();

This comment has been minimized.

@azuev-java

azuev-java Oct 20, 2020
Member

What action since line 162 can trigger changing the getMainDisplayID() outcome?

This comment has been minimized.

@mrserb

mrserb Oct 21, 2020
Author Member

If the main screen is not in the list of screens means that the system monitors were changed in some way after this method was called and we will get one more notification later.

@mrserb
Copy link
Member Author

@mrserb mrserb commented Oct 21, 2020

/integrate

@openjdk openjdk bot closed this Oct 21, 2020
@openjdk openjdk bot added integrated and removed ready rfr labels Oct 21, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Oct 21, 2020

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

Your commit was automatically rebased without conflicts.

Pushed as commit e5870cf.

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

@mrserb mrserb deleted the mrserb:JDK-8252133 branch Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants