<AWT Dev> [9] Review Request: 7124417 [macosx] failure in Window.initGC on Mac with monitor sleeping

[Sergey Bylokhov]

On 3/19/14 11:50 PM, Petr Pchelko wrote:
> Hello, Sergey.
>
> After Anthony have asked all the possible questions I’m left with the only option:
> The fix looks good to me.
>
> BTW: I see that there’s one more bug in this method: https://bugs.openjdk.java.net/browse/JDK-8036927 Is it also resolved by this fix?
I hope so, but since I cannot reproduce this issue, I don't want to 
close it right now.
>
> With best regards. Petr.
>
> 19 марта 2014 г., в 11:34 после полудня, Anthony Petrov <anthony.petrov at oracle.com> написал(а):
>
>> All right, if you say this additional filtering is required for correct results and tests confirm that, let it be.
>>
>> I'm fine with the fix then.
>>
>> --
>> best regards,
>> Anthony
>>
>> On 3/19/2014 11:29 PM, Sergey Bylokhov wrote:
>>> On 3/19/14 10:43 PM, Anthony Petrov wrote:
>>>> Why would there be a regression? Displays using hardware mirroring are
>>>> never included in the list by either CGGetActiveDisplayList or
>>>> CGGetOnlineDisplayList. So in this case the results should be the same
>>>> independent from the function used.
>>>>
>>>> Similarly, when using software mirroring, both functions return all
>>>> the mirrored displays. And again, there wouldn't be a regression from
>>>> just using another function.
>>>>
>>>> Please clarify what regression you're talking about.
>>> On my system they return different results. In the mirroring
>>> CGGetActiveDisplayList returns only 1 device, but CGGetOnlineDisplayList
>>> returns 2.
>>>> --
>>>> best regards,
>>>> Anthony
>>>>
>>>> On 3/19/2014 10:02 PM, Sergey Bylokhov wrote:
>>>>> On 3/19/14 9:35 PM, Anthony Petrov wrote:
>>>>>> On 3/19/2014 9:29 PM, Sergey Bylokhov wrote:
>>>>>>> On 3/19/14 9:20 PM, Anthony Petrov wrote:
>>>>>>>> 1. Previously we didn't exclude mirrored displays from the list. So
>>>>>>>> the new code that you're adding at lines 84-92 might be better
>>>>>>>> integrated as a separate fix.
>>>>>>> Before the fix we use CGGetActiveDisplayList, which returns only
>>>>>>> active
>>>>>>> device w/o device in sleep or mirror state. The new function
>>>>>>> CGGetOnlineDisplayList return all of them and we should filter out
>>>>>>> mirror device.
>>>>>> The specification of CGGetActiveDisplayList suggests that in the case
>>>>>> of software mirroring the returned list includes all mirror devices.
>>>>>> Therefore I still think that this is a separate bug.
>>>>> You are right! It seems that on my system always a hardware mirroring is
>>>>> used. But I am not sure that it is better to fix it in separate CR. If I
>>>>> remove filtering then I'll introduce a new regression for hard
>>>>> mirroring. Actually the fix is targeted to fix the problem in graphics
>>>>> initialization and it fix that, No?
>>>>>>
>>>>>>>> 2. Before the fix, the list of display devices returned from
>>>>>>>> getDisplayIDs was in the same order as reported by the native system.
>>>>>>>> After your fix the order is reversed. For instance, if previously the
>>>>>>>> main display was always at index [0], not it's always at the tail of
>>>>>>>> the array. This may or may not be a problem for Java applications
>>>>>>>> (likely not a serious problem). However, keeping the same order
>>>>>>>> between Java and native APIs might be a good thing for debugging
>>>>>>>> purposes. Would it be possible to modify the loop at lines 103-106 to
>>>>>>>> preserve the order?
>>>>>>> It is not necessary, because we store them in the Map on java side
>>>>>>> anyway.
>>>>>> Thanks for the clarification. This part looks fine then.
>>>>>>
>>>>>> --
>>>>>> best regards,
>>>>>> Anthony
>>>>>>
>>>>>>>> --
>>>>>>>> best regards,
>>>>>>>> Anthony
>>>>>>>>
>>>>>>>> On 3/19/2014 7:27 PM, Sergey Bylokhov wrote:
>>>>>>>>> Hello.
>>>>>>>>> Please review the fix for jdk 9.
>>>>>>>>> After the fix we use screen in sleep state as normal screen,
>>>>>>>>> where we
>>>>>>>>> can create new windows, etc.
>>>>>>>>> Disadvantage is that Robot still does not work on such devices.
>>>>>>>>>
>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-7124417
>>>>>>>>> Webrev can be found at:
>>>>>>>>> http://cr.openjdk.java.net/~serb/7124417/webrev.01
>>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Best regards, Sergey.
>>>>>>>
>>>>>
>>>


-- 
Best regards, Sergey.