#990 closed defect (fixed)
refactor window geometry code
Reported by: | Antoine Martin | Owned by: | Antoine Martin |
---|---|---|---|
Priority: | critical | Milestone: | 0.16 |
Component: | server | Version: | 0.15.x |
Keywords: | Cc: |
Description
Split from #907.
There are too many places recording the same information, all with slightly different intents: the X11 window itself (and its parents..), the window model classes, the desktop manager, (and of course: the client)...
Also relevant: the ownership information, initial window position and size hints, multiple clients (#41), etc..
This is a blocker for a number of tickets: #846, #988, #881 - and probably many others.
Change History (9)
comment:1 Changed 5 years ago by
Status: | new → assigned |
---|
comment:2 Changed 5 years ago by
More notes:
- r11016 moves
size-hints
to window model - r11020 removes
user-friendly-size
(not used) - r11017 removes
get_position
and r11031 removesactual-size
, we now use the geometry attribute instead - removing
get_dimensions
would be too hard for now - and as of r11031 it's implemented just once in core, cleanly actual-size
should always be the same as the corral window size - the only external caller is the x11 server's_window_resized_signaled
which could access the corral window if needed- new / improved tests: r11023, r11028
- fixes: r11019, r11029, r11025, r11024 (scaling related, found during window move testing)
- debug logging added or improved: r11030, r11027, r11019, r11026, r11025, r11022, r11021
requested-size
andrequested-position
are set from the window's initial geometry (queried viaXGetGeometry
), we update it when receiving a configure request on the child window (when the application requests it), but not when we handle _NET_MOVERESIZE_WINDOW messages. They are only used for the geometry when: there is no owner, or when the configure request omits some values. This looks fine.- the "ownership" stuff could be re-used for better multi-client support: the one that moved or focused the window last would become the owner,
maybe_recalculate_geometry_for
already handles most of this - when we get a configure notify for the corral window, shouldn't we verify the position part of the geometry is correct? and update it if not?
Remaining tasks:
- remove border from geometry (unused)
- remove the
do_get_property_geometry
ugly accessor method, it should never be used if we keep the geometry up to date, and useupdateprop
to fire geometry notifications? - add some logging to verify all geometry attributes make sense and are consistent with their intended purpose, test with OR windows, windows that trigger move, resize, etc
- remove the need to know which type of window you are dealing with to know its real position on screen: go through the desktop manager for all, and expose via xpra info
comment:3 Changed 5 years ago by
More updates, see:
- r11035: doc update
- r11051: big, see commit message
- r11052: better debugging
- r11053: better configure request handling
- r11054: simplify code
- r11055 + r11056: turn "geometry" into a regular gobject property
- r11057: window message handling fixes
Note: this removes the border attribute from geometry, which we never really used anyway.
Things that will need checking:
- iconification / resizing loops, #790
comment:4 Changed 5 years ago by
Owner: | changed from Antoine Martin to alas |
---|---|
Status: | assigned → new |
Summary: the window geometry code is cleaner and more consistent. It is now handled as a regular property.
The current geometry is always accessible with:
$ xpra info | grep -e "window.*geometry=" window[1].client-geometry=(370, 132, 499, 316) window[1].geometry=(370, 132, 499, 316)
(the client-geometry is for non-OR windows and shows where the window is mapped for this particular client - which should be the same as the server's geometry)
This the same geometry which is also used with xpra screenshot and with the sync-xvfb option (#988).
These changes need to be tested with most of the window test apps to make sure this has not caused any regressions.
Will follow up in #41, #846 and #881.
comment:5 Changed 5 years ago by
Owner: | changed from alas to Antoine Martin |
---|
Tested a Fedora 21 trunk r11118 Server from a Fedora 20 trunk r11118 Client:
Played around with the python test scripts for a while
- All working as expected....or at least identical to running it locally
- a couple spawned a weird secondary panel in addition to the main window...but they did that running locally so I guess it was expected. They behaved nicely, it was just unexpected
- No noticeable regressions
Passing back to you for now. Pass it back to us (either afarr or myself) if you need some more testing, as always.
comment:6 Changed 5 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:9 Changed 3 months ago by
this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/990
The basics:
geometry
property of all window models claims to be current (border-corrected, relative to parent) coordinates (x, y, w, h) for the window, it is actually retrieved via:do_get_property_geometry
returns the value of the_geometry
attribute, if it is still set to None this method will first attempt to populate it by calling XGetWindowAttributes and use the first 5 attributes from theXWindowAttributes
structure:do_xpra_configure_event
is the handler for ConfigureNotify X11 events, in the base class implementation it will simply update the window geometry with the event data and then fire the "geometry" notify signalget_position
andget_dimensions
are just shorthand versions for accessing parts of the geometryThe real trickery starts in WindowModel:
_NET_MOVERESIZE_WINDOW
messages cause the window geometry to get updated, after sanitizing the event data with the size-hints, we call XConfigureWindow followed by a ConfigureNotifysize-hints
should be moved here - only used here and doesn't make sense for OR windows... (what about "strut" - does that make any sense for OR windows?)actual-size
should be the same as size portion of the geometry?requested-position
andrequested-size
from the data we get back from calling XGetGeometrydo_xpra_configure_event
is a little bit different and callsresize_corral_window
which may or may not updateactual-size
anduser-friendly-size
and "geometry"do_child_configure_request_event
is the handler for ConfigureRequest Events and will update the geometry using the event's attribute, for the attributes which aren't set it will fallback to usingrequested-position
andrequested-size
(is that right??) - the actual updating is done via_update_client_geometry
And the real ugly parts:
ownership_election
probably deserves a entire wiki page to itself... it may reparent the window to the parking window, call_update_client_geometry
, raise the corral window, etc..maybe_recalculate_geometry_for
is similar: checks for ownership may call_update_client_geometry
_update_client_geometry
will call_do_update_client_geometry
with values it gets from the owner (if there is one), or fromrequested-position
andrequested-size
_do_update_client_geometry
does geometry calculations (honour "size-hints", etc) and then does the configure + notifyThe WM object:
do_child_configure_request_event
does a configure + notify for windows that don't have a model yetThe WorldWindow is just about GTK / X11 focus hell. Not very relevant here.
The DesktopManager is a hidden gtk widget which sits between out server class and the window models, deal with the ownership election business.
Each window is also recorded there using a (very confusingly named) model containing the following attributes:
maybe_recalculate_geometry_for
The Source and WindowSource try very hard not to deal with any geometry stuff, for 2 reasons:
The only call made is to
get_dimensions
so we know the size of the window, andget_image
to get the window pixels.The server is where some of the ugliest code lives:
Maybe we should keep the desktop manager, but have one per client with some shared data structures? (for things like resize counter?) Maybe it should handle OR windows too? (and just delegate those calls so we don't have this ugly if/else mess)
Maybe start by exposing as much as we can via xpra info.