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
