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.
wlroots < 0.17 didn't allow to reliably check the source surface of
an xdg activation request as it reset the surface to NULL when it
was destroyed before the token was used. This happens regularly for
notifications for example. Thus we treated the token as valid even
without checking for the source surface.
wlroots 0.17 added a new_token signal where we can attach information
to the existing token which we can then use when evaluating activation
requests. This patch implements that check.
See https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/3514
which added support on the wlroots side.
We now re-run popup positioning (for both xdg-shell and layer-shell
popups) when the "reposition" event is received. This allows popups that
change size (such as qmpanel's applications menu) to be positioned
correctly.
xdg-shell v3 also gives the compositor some additional "hints" for popup
positioning (reactive, parent_size, and parent_configure_serial) which
are available but we don't make use of currently.
Before this patch, labwc would happily kill itself when the user
called the `Kill` action when any xwayland view had focus.
The reason this happened was that wlroots creates the xwayland
wayland client via socketpair() and thus a lookup of the pid
of the socket connection would return the pid of labwc itself.
This patch fixes that by implementing different pid lookup
mechanisms based on the view implementation backend.
Fixes: #1739
When growing or shrinking a view by snapping to an edge, a client may
ignore the requested size and instead keep its original size or
substitute a different (possibly constrained) size. In this case, the
view may not actually contact the snapped edge, and a subsequent snap
attempt will just keep re-trying (and failing) to contact the same ege.
To mitigate this, remember the last-snapped view, snapping direction and
offset of the snapping edge in snap.c; when re-attempting a snap for the
same view in the same direction, ignore the edge that was last "hit", to
allow snapping to progress beyond the problematic edge.
Move/resize requests from xwayland views and xdg toplevels should be
ignored when the view is not pressed.
This is relevant for touchpad taps with <tapAndDrag> disabled.
When the user taps the client surface (e.g. chromium and mpv) with the
setting above, libinput sends button press & release signals so quickly
that the compositor receives move/resize request from the client AFTER
the button release signal is processed, so `interactive_finish()` is
never called.
...and notify the client of the preferred output scale when doing so.
This should allow clients to better determine an optimal size if they
are initially configured (unmapped) with zero size.
In particular, this fixes an issue with foot:
https://codeberg.org/dnkl/foot/issues/1579
Applications may respond to pending resize requests either by ignoring
them or substituting alternative sizes (for example, when mpv constrains
resizes to keep its aspect ratio fixed). In these cases, view->pending
will fall out of sync with the actual view geometry. This will cause
problems when subsequent operations (e.g., MoveToEdge) use the pending
geometry to decide where to place the window.
To fix this, reset view->pending to be equal view->current when either:
1. The requested size change has been commited, to the scene graph, and
no subsequent changes are pending; or
2. The requested size change has been ignored by the client.
Need to handle new unified mapping, where mapping is attached to the
wlr_surface objects instead of their parents. Also, most of them require
a new associate event for xsurface objects, their surface member will be
NULL before this event is received.
Refactored by jlindgren:
- add struct mappable
- unify map/unmap logic
Qt applications occasionally fail to call set_window_geometry after a
configure request, but do correctly update the actual surface extent.
This results in a mismatch between the window decorations (which follow
the logical geometry) and the visual size of the client area. As a
workaround, try to detect this case and ignore the out-of-date window
geometry.
Fixes: #1194
This is a useful (if lesser-known) feature of at least a few popular X11
window managers, for example Openbox and XFWM4. Typically right-click on
the maximize button toggles horizontal maximize, while middle-click
toggles vertical maximize.
Support in labwc uses the same configuration syntax as Openbox, where the
Maximize/ToggleMaximize actions have an optional "direction" argument:
horizontal, vertical, or both (default). The default mouse bindings match
the XFWM4 defaults (not sure what Openbox has by default).
Most of the external protocols still assume "maximized" is a Boolean,
which is no longer true internally. For the sake of the outside world,
a view is only "maximized" if maximized in both directions.
Internally, I've taken the following approach:
- SSD code decorates the view as "maximized" (i.e. hiding borders) only
if maximized in both directions.
- Layout code (interactive move/resize, tiling, etc.) generally treats
the view as "maximized" (with the restrictions that entails) if
maximized in either direction. For example, moving a vertically-
maximized view first restores the natural geometry (this differs from
Openbox, which instead allows the view to move only horizontally.)
v2: use enum view_axis for view->maximized
v3:
- update docs
- allow resizing if partly maximized
- add TODOs & corrections noted by Consolatis
The logic was the same for xdg-shell and xwayland views, so move it from
the view->impl layer out to the view_move_to_front/back() functions.
view->impl->move_to_front/back() still exist for now, in case we want to
add xdg/xwayland-specific logic in future, but they now move only one
view and not sub-views.
This makes the code a bit more readable IMHO (and forces us to be
consistent with event handler function names).
Adjust scripts/checkpatch.pl to not complain.
When a parent view has multiple sub-views (dialogs) visible, focusing
one sub-view ought to raise it above the others. This doesn't currently
happen -- focusing a sub-view raises the whole group of views together,
but has no effect on the relative stacking order between them.
This seems like a simple oversight in xdg/xwayland_view_move_to_front()
that's pretty easy to fix.
Add FIXMEs to deduplicate this logic in future.
Tested with HomeBank: the Import dialog pops up an additional Open File
dialog, which before this change appears behind the Import dialog (and
clicking on it does not raise it to the front). After this change, the
Open File dialog appears in front as expected.
The unmap() handlers should only call desktop_focus_topmost_view() if
the unmapped view was the focused view. Unmapping a view that was not
focused should not change the focus.
I expect this rarely had any effect in practice; it would only matter in
a focus-follows-mouse config where some view other than the one on top
was focused. But it still seems better to fix.
Rather than repeating the logic in two places, create a small
view_impl_unmap() helper. Perhaps more common "unmap" logic could be
moved there in future.
XWayland views can self-declare that they don't want keyboard focus via
the ICCCM WM_HINTS property. Most of the logic is already in place to
avoid giving focus to such views (e.g. taskbars).
Add a couple of missing pieces to make this work:
- Hook up view_isfocusable() to look at WM_HINTS for XWayland views
- Adjust desktop_focus_topmost_mapped_view() to skip unfocusable views
Make desktop_focus_view() always switch to the workspace containing the
view being focused. It doesn't make much sense for an invisible view to
have the keyboard focus.
Also add an optional "raise" parameter to desktop_focus_view(). This
allows the common pattern of desktop_focus_view() + view_move_to_front()
to be reduced to a single function call.
Our current approach to handling the focused/active view is a bit
confusing. In particular, it's hard to be sure when server->focused_view
is or isn't in sync with the real wlroots keyboard focus.
Try to clean things up a bit. In particular:
- Add comments to server->focused_view and desktop_focused_view() to
clarify that they should match, but it's not guaranteed.
- desktop_focused_view() now prints a warning if it detects that
server->focused_view is out of sync. We should keep an eye out for
this warning, and if we see it, try to figure out why it happened.
- For consistency, use only "focus/defocus" as the verbs in function
names rather than "activate". This is a bit arbitrary, but the idea is
that focus is the primary action while the active/inactive state is a
side effect.
- view_focus/defocus() replace view_set_activated() and now update both
focus and active/inactive state, to try to keep them in sync.
- Add comments at view_focus/defocus() to warn against calling them
directly (we should generally call the desktop.c functions).
- desktop_focus_view(NULL) is now forbidden and is no longer handled as
a special case to clear the focus. This was (at least to me) a
surprising behavior and caused trouble when working on another change.
- To maintain existing behavior, desktop_focus_topmost_mapped_view() now
explicitly clears the focus if there are no mapped views.
There should be no behavioral change here.
When a view is destroyed (including override_redirect in the xwayland
case), the view_destroy() handler is called which checks for a currently
open A-Tab window switcher and causes an update there to remove the
destroying view from the list. Before view_destroy() is called, both
xwayland and xdg handlers reset the xdg_surface / xwayland_surface.
The window switcher update then creates a list of all windows which do
not have the 'skipWindowSwitcher' window rule property set. If there is
at least one 'matchOnce' window rule configured, this also tries to get
string properties of the destroying view which already had their
xdg_surface / xwayland_surface reset and thus run into an assert.
This patch fixes that so that the string_prop() handlers always return
an empty string in those cases rather than running into the assert.
For a more in-depth analyses and alternative solutions see the linked
issue.
Fixes#1082
...to share common code with minimize_sub_views()
Also, fix a bug in the move-to-back functions to move the window
hierarchy in the right order.
Helped-by: @Consolatis
Minimize the whole view-hierarchy from top to bottom regardless of which
one in the hierarchy requested the minimize. For example, if an 'About' or
'Open File' dialog is minimized, its toplevel is minimized also, and vice
versa.
For reference:
- This is consistent with in openbox, where child views (dialogs) can be
minimized, but when doing so the parent is also minimized.
- In mutter these types of dialogs cannot be minimized (via client-menu or
otherwise).
- In both openbox and mutter, when a toplevel window is minimized any open
children are also minimized.
...so that other window cannot be positioned between modal dialogs and
their parent windows. This is consistent with Gtk3 and Qt5 applications on
mutter and openbox.
This makes explicit the subtle behavioral difference between
xwayland_view_unmap() and handle_unmap().
With this change, the XDG and XWayland versions of handle_map/unmap()
are now identical, which will make further refactoring possible.
Two types of window rules are supported, actions and properties. They are
defined as shown below.
<windowRules>
<!-- Action -->
<windowRule identifier="some-application">
<action name="Maximize"/>
</windowRule>
<!-- Property -->
<windowRule identifier="foo*" serverDecoration="yes|no"/>
</windowRules>
Rules are applied if windows match the criteria defined by the
'identifier' attribute which relates to app_id for native Wayland windows
and WM_CLASS for XWayland clients.
Matching against patterns with '*' (wildcard) and '?' (joker) is
supported.
Add 'serverDecoration' property.
