<AWT Dev> [9] Review request for 8020443: Frame is not created on the specified GraphicsDevice with two monitors

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Thu Feb 6 06:35:14 PST 2014


The fix looks good to me too.
On 06.02.2014 18:24, Alexander Zvegintsev wrote:
> Oleg,
>
> my bad, and you are absolutely right then.
> Thanks,
>
> Alexander.
> On 02/06/2014 04:49 PM, Oleg Pekhovskiy wrote:
>> Hi Alexander,
>>
>> thank you for your comments, and let me pay attention to the wording:
>>
>> I wrote "struts" but not "coordinates of the struts".
>> I did it intentionally: here is the dump of 
>> XToolkit.getScreenInsetsManually()
>> method on the second screen:
>>
>> Root bounds: java.awt.Rectangle[x=0,y=0,width=2048,height=768]
>> Screen bounds: java.awt.Rectangle[x=1024,y=0,width=1024,height=768]
>> Window #33554540 bounds[x=1024, y=24, width=1024, height=744] 
>> struts[left=1089, right=0, top=0, bottom=0]
>> Window #31457297 bounds[x=1024, y=0, width=1024, height=24] 
>> struts[left=0, right=0, top=24, bottom=0]
>>
>> As we see struts could be 0, even if their edge is not the same as 
>> the root window edge.
>> So if we are talking about:
>> left_start_y, left_end_y, right_start_y, right_end_y, top_start_x, 
>> top_end_x, bottom_start_x
>> of _NET_WM_STRUT_PARTIAL
>> I agree with you, but if we are talking about:
>> left, right, top, bottom
>> which is the case, I don't, and treat my comments as correct.
>>
>> Thanks,
>> Oleg
>>
>> On 02/06/2014 02:34 PM, Alexander Zvegintsev wrote:
>>> Hi Oleg,
>>>
>>> I am a little concerned about this comment:
>>>> * struts*could be*relative to root window bounds, so
>>>
>>> but specification[1] clearly says:
>>>> All coordinates are root window coordinates.
>>> so I think "could be" should be replaced by "are" (there is no need 
>>> for a new webrev)
>>>
>>> Otherwise, the fix looks good to me.
>>> BTW, I like your idea with frame maximization to check insets.
>>>
>>> [1] 
>>> http://standards.freedesktop.org/wm-spec/wm-spec-latest.html#idp6337880
>>> Thanks,
>>>
>>> Alexander.
>>> On 02/06/2014 10:21 AM, Oleg Pekhovskiy wrote:
>>>> Hi Alexander,
>>>>
>>>> thanks for the comments,
>>>> please see the next version of fix:
>>>> http://cr.openjdk.java.net/~bagiras/8020443.3/
>>>>
>>>> Updates:
>>>> 1. Left usage of root window coordinates because they passed to
>>>> XToolkit.getScreenInsetsManually() in Rectangle, not just Dimension.
>>>> Added comment for struts check.
>>>>
>>>> 2. Rewrote the test to make it accurate.
>>>> Tested on Ubuntu 12.04, Solaris 11 and Oracle Linux 6.4.
>>>>
>>>> Thanks,
>>>> Oleg
>>>>
>>>> On 02/04/2014 07:50 PM, Alexander Zvegintsev wrote:
>>>>> Oleg,
>>>>>
>>>>> please see inline
>>>>>
>>>>> On 02/04/2014 05:00 PM, Oleg Pekhovskiy wrote:
>>>>>> Hi Alexander,
>>>>>>
>>>>>> thank you for the review!
>>>>>>
>>>>>> 1. The problem is that I didn't find that root window ALWAYS 
>>>>>> starts with 0, 0 in X11 docs.
>>>>>> Could you please point such statement out to avoid uncertainty?
>>>>> Unfortunately, I cannot find any document which clearly specifies 
>>>>> this. is the root of this hierarchy.
>>>>> On my understanding root window is the root of windows 
>>>>> treehierarchy and all other windows are children or descendants of it.
>>>>> So root window is the origin for all other windows and and I 
>>>>> assume that it has (0,0) coordinates always and I would
>>>>> be surprised if it not.
>>>>>
>>>>>> 2. Checking insets against screen width and height can fail too. 
>>>>>> Example:
>>>>>> 1st screen (0, 0, 1024, 768) - x, y, width, height
>>>>>> 2nd screen (1024, 0, 1920, 1080)
>>>>>> Struts for the 2nd screen could be (1074, 0, 20, 0) - left, 
>>>>>> right, top, bottom.
>>>>>> So left strut 1075 fits in 1920 but should be 50 as inset.
>>>>> Yes, my check will fail on this case.
>>>>>> PS: Just curious: you said that screen (1000, 50, 1000, 1000) is 
>>>>>> a valid case.
>>>>>> Is it valid for testing environment? As I see screens are 
>>>>>> left-top aligned.
>>>>>> It means that if two screens are located from left to right, they 
>>>>>> both have y == 0 regardless of their height.
>>>>> It is a valid case, screens can be arranged in any order, screens 
>>>>> even can be overlapped, may have empty spaces between.
>>>>> (and we definitely will not support last 2 cases).
>>>>>
>>>>> My previous example was a little abstract, here is the real one 
>>>>> with screens aligned on bottom edge:
>>>>> Screen #1 (0,0, 1920, 1080)
>>>>> Screen #2 (1920, 30, 1680, 1050) with top inset 35
>>>>>
>>>>> It is unlikely that we will meet such configuration, but who knows.
>>>>>
>>>>> Since this test will run on machines with environment close to 
>>>>> default I propose to made an assumption
>>>>> that insets are not greater that a quarter (or 1/3?) of width and 
>>>>> height (and add a comment why we do that
>>>>> and why test may fail):
>>>>>
>>>>>             if (insets.left >= bounds.width / 4
>>>>>                     || insets.right >= bounds.width / 4
>>>>>                     || insets.top >= bounds.height / 4
>>>>>                     || insets.bottom >= bounds.height / 4) {
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Alexander.
>>>>>
>>>>>
>>>>>> Usually insets are much less than dimensions of a screen and so 
>>>>>> strut minus screen edge should be less
>>>>>> than summary dimension of neighbor screens.
>>>>>> Please, correct me if I'm wrong.
>>>>>>
>>>>>> Thanks,
>>>>>> Oleg
>>>>>>
>>>>>> 04.02.2014 15:28, Alexander Zvegintsev wrote:
>>>>>>> Hi Oleg,
>>>>>>>
>>>>>>> I am just curious about adding rootBounds.x and rootBounds.y, it 
>>>>>>> looks redundant to me.
>>>>>>> Is there a case when a root window have coordinates with 
>>>>>>> non-zero values?
>>>>>>>
>>>>>>> MultiScreenInsetsTest.java:
>>>>>>>>    62             if ((bounds.x != 0 && insets.left >= bounds.x)
>>>>>>>>    63                 || (bounds.y != 0 && insets.top >= 
>>>>>>>> bounds.y)) {
>>>>>>> This check will fail for screen with 
>>>>>>> [x=1000,y=50,width=1000,height=1000] bounds and top inset 100,
>>>>>>> but it is a valid case.
>>>>>>>
>>>>>>> I think we should check insets against screen width and height:
>>>>>>>             if (insets.left >= bounds.width
>>>>>>>                     || insets.right >= bounds.width
>>>>>>>                     || insets.top >= bounds.height
>>>>>>>                     || insets.bottom >= bounds.height) {
>>>>>>>
>>>>>>>
>>>>>>> Otherwise fix looks good to me.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Alexander.
>>>>>>> On 02/04/2014 10:26 AM, Oleg Pekhovskiy wrote:
>>>>>>>> Hi All,
>>>>>>>>
>>>>>>>> please review the second version of fix:
>>>>>>>> http://cr.openjdk.java.net/~bagiras/8020443.2/
>>>>>>>>
>>>>>>>> It turned out that struts must be checked and corrected from 
>>>>>>>> all sides
>>>>>>>> to become the proper screen insets.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Oleg
>>>>>>>>
>>>>>>>> On 01/21/2014 06:31 PM, Oleg Pekhovskiy wrote:
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> please review the fix
>>>>>>>>> http://cr.openjdk.java.net/~bagiras/8020443.1/
>>>>>>>>> for
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8020443
>>>>>>>>>
>>>>>>>>> Referring to the standards, we must calculate insets the 
>>>>>>>>> special way for Xinerama:
>>>>>>>>> http://standards.freedesktop.org/wm-spec/1.3/ar01s05.html
>>>>>>>>>
>>>>>>>>> _NET_WM_STRUT_PARTIAL
>>>>>>>>> "The start and end values associated with each strut allow 
>>>>>>>>> areas to be reserved which do not span the entire width or 
>>>>>>>>> height of the screen. Struts MUST be specified in root window 
>>>>>>>>> coordinates, that is, they are /not/ relative to the edges of 
>>>>>>>>> any view port or Xinerama monitor."
>>>>>>>>>
>>>>>>>>> So the fix checks if the insets have absolute values and if so 
>>>>>>>>> makes them relative to each screen.
>>>>>>>>> The issue occurred when the Frame was created with the 
>>>>>>>>> location by default.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Oleg
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>


-- 
Best regards, Sergey.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20140206/304a1a97/attachment.html 


More information about the awt-dev mailing list