Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linux: Window getPosition returning incorrect position #1228

Closed
JonesBlunt opened this issue May 7, 2017 · 7 comments
Closed

Linux: Window getPosition returning incorrect position #1228

JonesBlunt opened this issue May 7, 2017 · 7 comments

Comments

@JonesBlunt
Copy link

JonesBlunt commented May 7, 2017

Related to #346, #372

Testing on Ubuntu/GNOME 3 - getPosition() output differs quite drastically depending on if window does/doesn't have decorations

New to X11, though it seems XTranslateCoordinates() is meant to use (0, 0) for src_x/y as they are relative to the source window origin. Works on my build minus the decorations discussed in #372, though that's good for me.

https://gist.github.com/JonesBlunt/7c396e2341b69362f0d940c1b955e0e0/revisions

@eXpl0it3r
Copy link
Member

Looks like this issue got missed somehow. We like to discuss potential issues on the forum first, as in many/most cases things turn out to be either a user error or some weird setup. See our contribution guidelines.

Can you provide some examples of what decorations you use and what the differences are and any special window manager settings you use.

@llongi
Copy link
Contributor

llongi commented Aug 17, 2017

I'm having a similar problem. I'm running the XMonad WM, tested on Gentoo, Fedora 25 and Fedora 26.
SetPosition() works fine, but getPosition() always returns double the expected value, as if it's adding itself. X11 utils like xwininfo report the correct position, so I don't think this is a WM or X11 bug.
So I added a few print statements to getPosition(), and it seems like localX and localY from XGetGeometry() already contain the position, which then XTranslateCoordinates() uses as offset. Just using XTranslateCoordinates() with 0/0 as source X,Y fixes this in all my tests, see https://gist.github.com/llongi/23e326ac46eed275a6be6ff5b32a5298 for a patch (that also removes XGetGeometry(), since we can get the right root window by using DefaultRootWindow(m_display) as it's done in other parts of the X11 SFML code). This is the same as GLFW does (https://github.com/glfw/glfw/blob/master/src/x11_window.c#L1982), they also had a lot of discussions and testing in glfw/glfw#800 and in the end this was the most consistent behavior, so that getPosition() and setPosition() return the same value.

@llongi
Copy link
Contributor

llongi commented Aug 17, 2017

Small test program here: https://gist.github.com/llongi/e02ea9d824572a0854a032c10b35d136 it just creates a 100x100 window, puts it at position 100,100 and on close prints the position as returned by getPosition(), which should be, in a perfect world, 100,100 again.
So with the change discussed in this bug, it is 100,100 on XMonad, and I just tested Gnome 3 too, there it is 100,137 (the +37 comes from the thickness of the toolbar).
Basically the behavior as I can see it is that with the change, X/Y as returned by getPosition() correspond to "Absolute upper-left X/Y" as displayed by xwininfo. Without the change, as the code is now, the returned X/Y are a sum of "Absolute upper-left X/Y" and "Relative upper-left X/Y", which is in all cases I've tested an (even more) incorrect number.
In Gnome 3 for example I get X,Y of 110,182 currently, which has no bearing on reality whatsoever: it's not what I passed via setPosition(), and it isn't even the top-left corner of the render-window if you adjust for the toolbar, it's somewhere inside the render area, which is clearly incorrect, see attached image:
sfml_gnome3_pos_test

So far in summary:

  • setPosition() always works fine, puts window upper-left corner at given absolute position
  • getPosition() as is current returns bogus values on Gnome 3, Xmonad (Sum of absolute and relative positions as reported by xwininfo)
  • getPosition() after patch reports better values (Absolute position as reported by xwininfo), it doesn't consider decorations while setPosition() does, so there's still some dissonance there, but it's better than the current completely bogus values.

Just FYI, my use case is an application that can create multiple windows to show different visualizations of scientific data, and I want to be able to save how the user arranges those windows and present him with them at the same place next time he starts the program.

@llongi
Copy link
Contributor

llongi commented Aug 18, 2017

I've spent a few hours digging into this and my latest attempt is at:
https://gist.github.com/llongi/6d85632e3a681d789e82486b713af631
This now takes into account X11 border widths (if set), as well as tries to remove decorations, by getting the info from XGetGeometry(), when they do exist (re-parenting WMs).

So far it works well everywhere I tried (Xmonad, Gnome Xorg, Gnome Wayland), on Gnome it just reports 99,99 because it can't take the 1x1 window border drawn by Gnome into account, X11 simply doesn't know about it and there's no way to find out (Gnome doesn't set X11 border width, just shifts the parent negatively by the border width and then adds that to the difference between parent and window... what an inconsistent mess this all is...).

I believe the above approach is the best that can be done in a generic way using the information available via X11/Xlib.

One other approach I thought of was to setPosition to something known, getPosition() and then calculate the difference and apply that as offset on subsequent getPosition() calls. But that doesn't consider cases like changing the WM theme while your application is running. I experimented with ConfigureNotify X11 events to detect such changes, it's possible, but it just grows into a gigantic hack.

In any case I believe the current approach is clearly wrong, from the XGetGeometry() docs: "Return the x and y coordinates that define the location of the drawable. For a window, these coordinates specify the upper-left outer corner relative to its parent's origin."; so using them as source coordinates in XTranslateCoordinates(window, root) is incorrect, as it gets the absolute position of the window respective to the root, and then adds the parent-window difference on top. That can't work, and brutally breaks down on non-reparenting WMs (there the parent is the root, so you add the amount to itself). If you had used negative xLocal/yLocal, that would have been more correct, at least on re-parenting WMs. Would still break on non-reparenting ones (result would be 0,0).
I hope my analysis is understandable. :-) Thanks and have a nice week!

@llongi
Copy link
Contributor

llongi commented Aug 20, 2017

And I believed wrong, one can do much better thanks to Extended Window
Manager Hints (EWMH). Welcome to patch V3, the fruit of a Saturday
spent looking at the behavior of over 20 Linux window managers. :-)

Link: https://gist.github.com/llongi/c993113f0fc31db638af22a3c4784563

The below table should prove that the current code is often completely
incorrect (only on Enlightenment and FVWM it returns sane results), and
that the V3 patch fixes things (for the most part, see comments).

Now, we first look at the WM and get a couple of rare exceptions that give
the correct value out of the way, then we look at the EWMH _NET_FRAME_EXTENTS
property, and if that fails we fall back to getting the geometry of the
uppermost parent. The code should be clean and well commented.

The following table documents the results of testing setPosition(100, 100)
followed by getPosition() on various WMs in their default configuration, all
tested on Fedora 26 unless otherwise noted:

EWMH WMs (do define Frame Extents)

WM Name and Version Current Patched Notes
Unity / Compiz 0.9.12 100x128 100x100 Ubuntu 16.04 LTS
Compiz 0.8.14 212x254 100x100
Fluxbox 1.3.7 101x149 100x100
KDE / Kwin 5.10.4 104x129 100x100
Metacity 3.24.1 126x197 100x100
Gnome / Mutter 3.24.4 101x175 100x100
Xfce4 / Xfwm 4.12.4 106x158 100x100
WindowMaker 0.95.7 101x145 100x100
awesome 4.2 101x141 100x100
LXDE / Openbox 3.6.1 104x146 100x100
MATE / Marco 1.18 112x154 100x100
Cinnamon / Muffin 3.4.1 100x154 100x100
Enlightenment 0.21.8 100x100 100x100 [1]
FVWM 2.6.7 100x100 100x100 [2]

[1], [2]: these two WMs place the actual render window at position 100x100,
not the top-left corner of the "whole window" (the one with the decorations).
As such, their reported frame extents getting substracted does indeed tell
you where the top-left corner of the "whole window" is, but it then differs
from what was given to setPosition(). At least they give a correct absolute
value for the window right away, and define a 'windowManagerName', so working
around them is very easy and clean.

Non-EWMH WMs (Frame Extents not defined)

WM Name and Version Current Patched Notes
Xmonad 0.12 201x201 100x100 Gentoo
Xmonad 0.13 201x201 100x100
Blackbox 0.70.1 101x122 100x100
IceWM 1.3.8 102x122 100x100
dwm 6.1 201x201 100x100
Ratpoison 1.4.8 2459x1239 1229x619 [3]
i3 4.13 102x100 100x100 [4]
twm 1.0.9 100x182 98x98 [5]

[3]: auto-centers window ignoring any setPosition(), though before the patch
the position of the centered window was wrong, with the patch at least now the
correct position of the centered window is returned.
[4]: puts the actual render window at 100x100, the parent window with the
borders is shifted back, and we get the coordinates of that one. The actual
size of the full decorations is not reported anywhere either. We can easily
work around this exactly like in [1], [2].
[5]: the parent window with decorations also has a 2px border, and is shifted
back by that 2px border, so the actual top-left corner is indeed at 98x98.
This can't be worked around (no WM name), but the value is much more sane now.

I'm personally pretty happy with the code, it often just works by going the
EWMH Frame Extents route, and if not the X11 parent-relative-to-root fallback
is the most generic fallback possible. Only 3 cases out of 21 tested I had to
handle as an exception (and an easy one to boot), the other 18 just work.
I'm sure it's possible some WMs exist out there that do even crazier things,
or you may be able to configure them in such a way that they report bogus
values, but at some point you have to draw the line. :-)

I believe this code is a clear improvement on the current one, easy to
understand and easy to maintain. I do offer myself for follow up questions
and fixes as needed, and I can say I've learned a lot today.
If you need any testing done regarding Linux WMs, I do now have a VirtualBox
VM with all of the above installed, so just shout out.
Thanks and have a nice week!

@eXpl0it3r
Copy link
Member

Thanks for all the work you've put into this! The various WMs on Linux is a bit of a nightmare to get things right.

Feel free to provide a pull request with your changes, that way we can make the review easier and run it through the CI system.

eXpl0it3r pushed a commit that referenced this issue Sep 5, 2017
…sults on Linux depending on the used WM,

as well as not returning values that are in-sync with what was given to setPosition(x, y).
@eXpl0it3r eXpl0it3r added this to the 2.5 milestone Sep 5, 2017
@eXpl0it3r
Copy link
Member

Merged in 58b7c2c with PR #1266

@eXpl0it3r eXpl0it3r added this to Ready in SFML 2.5.0 Sep 5, 2017
@eXpl0it3r eXpl0it3r moved this from Ready to Merged / Superseded in SFML 2.5.0 Sep 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
SFML 2.5.0
  
Merged / Superseded
Development

No branches or pull requests

4 participants