On 30/10/2018 09:59, Phil Race wrote:
Looks reasonable. A test is difficult, but I presume you did some testing.
Can you report the scenarios you tested and on which OS version ?
eg,
Did you try remove/add/remove, or add/remove/add ?
Did you try wake from sleep ?
Did you try removing a monitor whilst it was sleeping and then waking ?
I was not able to reproduce this bug on the multimonitor config,
but in a single monitor the bug is reproduced when I switch
the user while the program is executed.
For the test purpose and to prove that the updated methods will not crash,
I have hardcoded the wrong display id at the end of the constructor
of the GraphicsDevice, and run our tests.
All whilst a Java app was running of course - on the monitor being removed.
-phil.
On 10/15/18 6:11 PM, Sergey Bylokhov wrote:
Hello.
Please review the fix for jdk 12.
Bug: https://bugs.openjdk.java.net/browse/JDK-8211992
Webrev: http://cr.openjdk.java.net/~serb/8211992/webrev.00
Short description:
The root cause is how we use CoreGraphics display ID. This identifier can
become non-valid at any time therefore methods, which is using this id should
be ready to it. And this bug found a few places which does not take care about
the rule above.
Long description:
In the CGraphicsDevice class we maintain the ID for the native screen. This
ID usually changed when the user adds/removes monitors to the system. Since
using invalid id can cause NULL result in the native code we tries to
workaround it by two steps:
Step (1) If the monitor is removed from the system, then we reassign its
displayid to the primary screen. So if the user will holds the reference to
this GDevice, it will use the MainDisplay. But note that if the user will
remove the main screen again then the old screen, which were removed
previously, will not be updated(but will use an invalid displayid).
Step (2) Some of the native methods in CGraphicsDevice class are ready to
NULL and provide some meaningful defaults, like 72 dpi. But some others methods
are not ready for that, like nativeGetDisplayMode/etc.
Fix description:
- I have minimized/dropped the usage of displayid outside of CGraphicsDevice
class, in CRobot it was unused, and the code from CGraphicsConfig.m was moved
to CGraphicsDevice.m.
- In CGraphicsEnvironment class we now maintain the list of old devices, which
is updated for all _displayReconfiguration events. This is the same
implementation as on windows in Win32GraphicsEnvironment.java
- I have added some default values to all native methods also. Just to cover
the rare case when we call the method and displayid became invalid at the same
moment.
--
Best regards, Sergey.
