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? With best regards. Petr. 19 марта 2014 г., в 11:34 после полудня, Anthony Petrov <anthony.pet...@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. >>>>>> >>>> >>>> >> >>
