XDG WindowGeometry
Open, Needs TriagePublic

Description

Somewhat big task, so probably needs some tracking.

The easiest way to map this to keep compatibility and do this safely is to keep AbstractClient::geometry() the same untouched true canonical source of the buffer location in compositor space.

Instead we should consider consider windowGeometry to be a set of margins to apply to/from this. Similar to the existing AbstractClient::borders.

AbstractClient can gain a realGeometry / setRealGeometry which add/subtract margins as appropriate.

(ideally we need a better name but both "Window Geometry" and "Client Geometry" are terms that are taken that also mean removing AbstractClient::borders )

Then most porting is just a case of switching methods.

SubTasks

  • Send correct configure event so resizing works (done)
  • Set correct popup location
  • Handle window margins changing (i.e if margins change top left of the real window should stay the same
  • Adjust placement.cpp to use other method
  • Adjust rules
  • Adjust snapping
  • Etc.
  • Done!
zzag added a subscriber: zzag.Apr 30 2019, 8:57 PM

On re-reading I didn't give a very good rationale of why I suggest this approach as opposed to the obvious other solution.

We could just set the AbstractClient::geometry to the Xdg windowGeometry fixing all window management in one go, and then draw the buffer at a different size in scene. The problem with that is we also have to adjust the entirety of the input stack.

Ultimately you still end up with two concepts of geometry. I'd rather adjust all the window management, because if it does go wrong, you can just move the window slightly. If a path isn't ported it's no worse than the current state. If rendering or input goes wrong everything explodes.

zzag added a comment.EditedMay 8 2019, 2:43 PM

As I said previously, I'd prefer a solution where Toplevel::geometry() is logical geometry of the client. This way we don't have to adjust lots of code and if we implement this in more or less "generic" way then it should be quite easy to add support for a bit controversial _GTK_FRAME_EXTENTS (distros could patch kwin for example). xdg-surface::set_window_geometry and _GTK_FRAME_EXTENTS look quite similar, except that the former provides us absolute values.

In general, I think it would be worth to analyze and come up with a set of geometries that would seem sensible as well reduce the number of unnecessary changes in KWin core. Currently I see three distinct geometries: the one that used in window management operations (e.g. resize, move, etc), input geometry, and render geometry (used during rendering operations to build window quads, etc). Though it's worth to mention that at this moment of writing KWin has much more different kinds of geometry.

Definitely it may require some refactoring in KWin core.

Currently I see three distinct geometries: the one that used in window management operations (e.g. resize, move, etc), input geometry, and render geometry (used during rendering operations to build window quads, etc).

Yep. Fully 100% with you there. We're both describing the same thing.
++

The difference of opinion is do we write code patching rendering and input or write code patching the window management.

One is smaller, but also a lot more risky and requires everything to be done at once.
Particularly if we did support X as that has numerous paths that write back geometry to places.

zzag added a comment.May 8 2019, 8:44 PM

The difference of opinion is do we write code patching rendering and input or write code patching the window management.

Please don't forget that there are tiling scripts in the wild. Do they have to deal with "client frames"?

Sure, they need to use the "window geometry". That doesn't really have a huge impact on anything though.

zzag moved this task from Backlog to Work In Progress on the KWin board.
romangg moved this task from Work In Progress to Under Review on the KWin board.Oct 21 2019, 8:22 AM
zzag added a comment.Oct 22 2019, 11:17 AM

Alright, let's go quickly over the proposed solution...

The problem with the window geometry is that it and the geometry of the main surface are loosely connected. The window geometry can be completely inside the main surface, but it can also cover a rectangle which is larger than the one occupied by the main surface (because a sub-surface is sticking a little bit outside of the main surface and no window geometry was set explicitly). Thus, Toplevel::geometry() is not suitable anymore. We need something else.

My patch series introduces two geometries - frame and buffer. The frame geometry specifies visible bounds of the client from the user's perspective. This geometry doesn't include server-side and client-side drop-shadows. The buffer geometry specifies a rectangle on the screen occupied by the x11 pixmap or the main surface.

The nicest thing about the buffer geometry is that it makes mapping global screen coordinates to surface-local coordinates very simple. It's just a matter of subtracting the top-left corner of the buffer geometry!

Things like moving or resizing must be done in terms of the frame geometry (not buffer geometry!). Ideally, the buffer geometry must be used only during rendering.

However, my patch series doesn't fix all problems. We still need to fix Toplevel::inputGeometry() and Toplevel::visibleRect().

  • In most cases, Toplevel::inputGeometry() is practically the same as the frame geometry. That won't fly with client-side decorated clients;
  • Toplevel::visibleRect() must include sub-surfaces.

In addition to that, the overall geometry stuff in XdgShellClient is lame. I plan to rewrite it. The biggest issue I see at the moment is that kwayland wrappers hide way too much stuff. Thus, I can't move window geometry handling down to xdg_surface level, where it belongs. Though we could implement some bits of window geometry in kwayland, but I think it will go against the purpose of kwayland. KWayland provides wrappers for wayland protocols, it's not a library to build wayland compositors.

zzag added a comment.Oct 22 2019, 11:24 AM

In addition to the above, notice that KWin must put server-side decoration around the window geometry, not the main surface.

zzag moved this task from Under Review to Done on the KWin board.Wed, Nov 27, 2:00 PM
zzag moved this task from Work In Progress to Done on the Plasma on Wayland board.