[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [12] Review Request: 8211992 GraphicsConfiguration.getDevice().getDisplayMode()
From:       Phil Race <philip.race () oracle ! com>
Date:       2018-10-31 17:54:53
Message-ID: 28b4bad3-ee4b-b0a7-b54c-0e2d3ce54e9c () oracle ! com
[Download RAW message or body]

OK. +1.

-phil.

On 10/31/18 10:26 AM, Sergey Bylokhov wrote:
> 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.
>>>
>>
>
>

[prev in list] [next in list] [prev in thread] [next in thread]