We were only using it to allow quick bitset comparisons of sets of
outputs (such as view->outputs). We can maintain our own bit IDs for
this purpose and avoid using the private wlroots field.
Note: from my reading of wlr_scene_output_create(), it appears to
always take the lowest unused index, resulting in aggressive re-use of
index values when outputs are disconnected and reconnected. I've tried
to make re-use as infrequent as possible. This could theoretically
reduce the chance of a mix-up in view_update_outputs(), although I'm
not aware of any practical scenario where it matters.
v2: prevent adding more than 64 outputs
Map/unmap logic is currently re-used for minimize/unminimize, but lots
of it doesn't actually apply in that case. This is both confusing and
creates some extra complexity, such as:
- extra "client_request" parameter to unmap(), in which case it has to
still do some cleanup even if view->mapped is already false
- various "view->mapped || view->minimized" checks when we really just
mean "is the view mapped"
To clean this all up, let's put the logic that really is common into
a new view_update_visiblity() function, and stop using map/unmap for
minimize/unminimize.
Note that this changes the meaning of "view->mapped", which used to
mean "mapped and not minimized" but now really just means "mapped".
I left some "view->mapped" conditions as-is (rather than changing to
"view->mapped && !view->minimized") where it seemed to make sense.
v2: add view_update_visibility() as suggested by tokyo4j
The previous "minimal fix" (5148c2aa31) worked but was a bit of a
hack, as it basically un-minimized and then immediately minimized the
view again at map. It's not actually too difficult to make the map
handlers aware of minimized views, eliminating the need for the hack.
Note: this depends on the previous commit ("xwayland: connect commit
and surface_destroy handlers together") otherwise the xwayland map
handler registers the commit handler twice, leading to a crash.
Factor out set_surface() which consolidates connecting/disconnecting
the wlr_surface event listeners in one place.
In theory, this means we can receive commit events for minimized views.
However, with a test app that resizes itself, I didn't see any change,
i.e. the commits still don't come through until un-minimize. It's
possible they are being filtered at wlroots or protocol level.
Also remove an old, semi-related TODO from view.c.
It's possible for a fullscreen xwayland view to be unmapped without
being destroyed. In this case, we need to update top layer visibility,
otherwise panels and the like will remain hidden.
Since unmap is always called before destroy, it's sufficient to do the
update only in view_impl_unmap() and not in view_destroy().
Adaptive sync logic needs work still, but I tried to minimize changes
to it since I don't have hardware to test it.
In 2ac4811, I was missing that windows can be tiled to "center".
As a result, after executing
`<action name="SnapToEdge" combined="yes" direction="left" />` against a
center-tiled window, `view->tiled` is set to `CENTER|LEFT`.
This patch adds `combine` argument to (Toggle)SnapToEdge actions.
This allows to snap a window to e.g. up-left by running two actions:
- `<action name="SnapToEdge" direction="left" combine="yes" />`
- `<action name="SnapToEdge" direction="up" combine="yes" />`
Then running `<action name="SnapToEdge" direction="down" combine="yes" />`
snaps it to left again. This behavior is almost the same as KWin, except
that snapping a up-right-tiled window to right doesn't move it to the
right-adjacent output, but makes it right-tiled first.
Before this commit, <else> branch was always executed with
monitor="current", monitor="left" or monitor="right" queries.
For example:
<action name="If">
<query monitor="current" />
<then>
<action />
</then>
<else>
<action />
</else>
</action>
view_restore_to() (which is just set_maximized() + view_move_resize())
hasn't aged well and doesn't line up with typical usage anymore:
- it's missing view_set_untiled(), which has to be called separately
- it always forces view_move_resize() even when that's not needed
- it doesn't allow un-maximizing only one axis (see next commit)
- the fullscreen check is unnecessary (already checked in callers)
Eliminate it and just expose view_set_maximized() instead.
No functional change intended in this commit.
`update_last_layout_geometry()` stores `view->natural_geometry` in
`view->last_layout_geometry`, but it's empty for initially-maximized
windows, so their positions were not restored after outputs are
unplugged and plugged (also when VT switching in wlroots 0.19.0).
This commit sets the fallback natural geometry (at the center of the
output) so that initially-maximized windows reappears in the same output.
Now it's not very reasonable to determine the default window width based
on the titlebar geometry, as the titlebar can be shrunk to 1px.
Let's use the fixed value of 100px for simplification.
- add LAB_WINDOW_TYPE_INVALID in place of literal -1
- document more clearly that enum lab_view_criteria is a bitset
- other one-off replacements of integer values/types for consistency
Note: variables of type enum lab_view_criteria are already used
extensively throughout the code to contain combinations of the declared
enum values. I am not introducing any new usage here, just changing the
single uint32_t to be consistent with all the other usages.
I like the new common/edge.h. I don't like how inconsistently we use it.
Current situation:
- enum wlr_edges and wlr_direction are designed to be used as bitset,
and are defined compatibly
- enum lab_edge is *also* designed to be used as bitset, but
incompatible with the others (LEFT/RIGHT come before UP/DOWN)
- we use an inconsistent mix of all three *AND* uint32_t (usually with
the WLR_EDGE constants rather than the LAB_EDGE constants), and
convert between them on an ad-hoc basis, sometimes implicitly
Let's clean this up:
- reorder enum lab_edge to be compatible with the two wlr enums
(check this by static_assert)
- use TOP/BOTTOM naming rather than UP/DOWN (matches wlr_edges)
- add constants for the remaining possible combinations of the 4 edges
- use lab_edge for all internal edge/direction fields, consistently
- add lab_edge_is_cardinal() as a sanity check before casting to
enum wlr_direction, and then eliminate all of direction.c/h
Instead of "enum wlr_edges direction", we now have
"enum lab_edge direction" which is not that much better. At least we
are now clear that we're overloading one enum with two meanings.
In 943f5751, I initialized heap-allocated `view_query` used for
`If` actions with `decoration=LAB_SSD_MODE_INVALID`, but I forgot to do
that for stack-allocated `view_query` used for window rules.
When implementing single-axis maximize some time ago, I made the
simplifying assumption that a view couldn't be resized while maximized
(even in only one axis). And indeed for compositor-initiated resize,
we always unmaximize the view first.
However, I didn't account for the client resizing the non-maximized
axis, which we can't (and shouldn't) prevent. When this happens, we
should also update the natural geometry of that single axis so that we
don't undo the resize when un-maximizing.
P.S. xdg-shell clients resizing the *maximized* axis is still an
unsolved problem, exacerbated by the fact that xdg-shell protocol
doesn't allow clients to even know about single-axis maximize.
P.P.S. the view_invalidate_last_layout_geometry() logic may need
similar updates, I'm not sure.
Currently, the dependencies between foreign-toplevel[-internal],
ext-foreign, and wlr-foreign are cyclical and a bit complex.
I suggest we reorganize it into a simpler hierarchy:
foreign-toplevel/
-> foreign.c/h
-> (depends on) ext-foreign.c/h
-> (depends on) wlr-foreign.c/h
The refactored code is smaller and (IMO) easier to follow.
In detail:
- Add include/foreign-toplevel folder mirroring src/foreign-toplevel
- Split foreign-toplevel-internal.h to ext-foreign.h and wlr-foreign.h
- Eliminate ext-/wlr-foreign.c -> foreign.c reverse dependencies
(including internal signals and foreign_request* functions)
- Make struct foreign_toplevel private to foreign.c
Lightly tested with qmpanel (which uses wlr-foreign-toplevel).
v2: reorder foreign-toplevel internal API funcs
If they are not empty, then we are headed for use-after-free shortly.
An assert() failure is easier to debug than UAF, so let's fail early.
Inspired by:
8f56f7ca43
In addition to <snapping><range>, <snapping><cornerRange> configures the
distance from the screen corner to trigger quater window snapping.
Also, new values "up-left", "up-right", "down-left" and "down-right" are
allowed for <action name="(Toggle)SnapToEdge" direction="[value]"> and
<query tiled="[value]">.
This is a common practice in C projects, which simply enforces that
each header must compile cleanly without implicit dependencies on
other headers (see also the previous commit).
In 9e3785f8cd, a heuristic was added to assume that NORMAL and DIALOG
window types were always focusable. (This was before we had the "offer
focus" mechanism in place.)
However, we should still call wlr_xwayland_surface_offer_focus() for
these views, in case they actually don't want focus. (This is uncommon
but has recently been seen with WeChat popups, which have both NORMAL
and UTILITY type.)
To make this possible, refine view_wants_focus() to return either
LIKELY or UNLIKELY for Globally Active input windows. This decouples
the question of "should we try to focus this view" from the actual
mechanism used to do so.
