xdg: handle initially maximized xdg-shell views better

Currently, initially maximized (or fullscreen) xdg-shell views exhibit
one of two issues:

 - some (e.g. GTK and Qt apps) paint an initial frame un-maximized
   (before the "map" event) and only maximize in a later commit

 - others (e.g. foot) maximize immediately without flicker, but never
   store a valid natural size, so we end up using a fallback (640x480)

Under KWin, neither of these issues occur, so I looked into what labwc
is doing wrong. It seems that:

 - wlroots internally sends an initial configure event with a size of
   0x0 to all xdg-shell views. This requests the client to set its own
   preferred (a.k.a. natural) size.

 - For an initially maximized/fullscreen view, the initial configure
   event should contain the maximized/fullscreen size rather than 0x0.
   In labwc, this means we have to call wlr_xdg_toplevel_set_size()
   earlier, i.e. from the new_surface event. Tracing with WAYLAND_DEBUG
   shows that the initial configure event now has the correct geometry,
   matching KWin behavior. With this change, GTK and Qt apps no longer
   paint an incorrect un-maximized frame.

 - However, this means that all xdg-shell views now suffer from the same
   issue as foot, where we never receive a commit with the un-maximized
   (natural) geometry. The correct way to get the natural geometry seems
   to be to wait until we want to un-maximize, and send a configure
   event of 0x0 at that point.

Sending a configure event of 0x0 when un-maximizing is a bit annoying as
it breaks some assumptions in labwc code. In particular:

 - view->natural_geometry may now be unknown (0x0), requiring various
   wlr_box_empty() checks sprinkled around. I added these in all the
   obvious places, but there could be some code paths that I missed.

 - Positioning the newly un-maximized view within view_maximize() no
   longer works since we don't know the natural size. Instead we have to
   run the positioning logic from the surface commit handler. This
   results in some extra complexity, especially for interactive move.
   See the new do_late_positioning() function in xdg.c.

Some TODOs/FIXMEs (non-blocking in my opinion):

 - The view_wants_decorations() check is now duplicated in both the
   new_surface and map event handlers. I'm not sure if this is necessary
   but it seemed like the safest approach for now. More testing would be
   nice, particularly with various combinations of config and client SSD
   preferences.

 - Aside from the interactive move case, the "late positioning" logic
   always centers the view when un-maximizing, and does not invoke any
   of the smart placement logic. If we want to invoke smart placement
   here, I'd appreciate someone with more knowledge of that code to take
   a look and figure out how to do that correctly.
This commit is contained in:
John Lindgren 2024-07-02 07:24:29 -04:00 committed by Johan Malm
parent e26ec472b2
commit 3be8fe25f3
6 changed files with 148 additions and 41 deletions

View file

@ -451,6 +451,7 @@ void view_moved(struct view *view);
void view_minimize(struct view *view, bool minimized);
bool view_compute_centered_position(struct view *view,
const struct wlr_box *ref, int w, int h, int *x, int *y);
void view_set_fallback_natural_geometry(struct view *view);
void view_store_natural_geometry(struct view *view);
/**