...to increase xwayland and xdg-shell encapsulation and to avoid passing a
function pointer as an argument in `xwayland_move_sub_views_to_front()`
which is inconsistent with labwc design patterns.
Rename view-impl.c to view-impl-common.c
Move function declarations that are common to view-implementations from
view.h into view-impl-common.h
This fixes an issue with havoc not having a valid size on map().
Investigation showed that xdg_surface->current.geometry is set only by
the xdg_surface::set_geometry protocol message, which is optional. If
set_geometry is not called, then we are supposed to compute the size
from the surface buffer(s). wlr_xdg_surface_get_geometry() already
accounts for this, so we just need to use wlr_xdg_surface_get_geometry()
instead of reading xdg_surface->current.geometry directly.
A fullscreen view currently has its output specified twice by:
- struct output *output
- struct wlr_output *fullscreen
view->fullscreen may also become a dangling pointer if the output is
disconnected, because view_on_output_destroy() clears view->output but
not view->fullscreen.
To eliminate the redundancy and the dangling pointer, let's change
view->fullscreen to a Boolean and rely on view->output to specify the
output.
Along the way, change a few related usages of struct wlr_output to
struct output as well.
No functional change intended.
v2: Don't allow entering fullscreen on disabled output (makes
conditions for entering/leaving fullscreen symmetric)
v3: Use output_is_usable() helper
view->impl->move() is a specific case of view->impl->configure().
To reduce code duplication, we can use view->impl->configure() for
pure moves (without resize) as well.
xwayland's move() function also possibly contained a race condition
when there was a pending resize, as it used the current surface
width/height rather than the pending width/height. This is fixed.
Before this patch, configuring a surface with a new size,
immediately followed up by one or more view_move() calls
would move the surface to the new coordinates immediately
without waiting for the resize. This caused visual glitches
when for example dragging a maximized window: the position
would change but the size was still that of a maximized
window.
This patch fixes that by just ignoring view_move() requests
(but still updating view->pending) if there is a configure
request pending. Once the client commit comes in the new
size will be applied as usual.
It was not working before because in the case of wayland
we are only dealing with sizes as wayland has no notion
of a global position. A wayland client would thus not
necessarily respond to a configure request which sets
the same size again. This causes us to also not apply
a new position set in view->pending because there may
be no commit from the client in those cases.
We previously worked around this issue in some parts
of the code to check our new sizes against the pending
ones and if they were the same we would call view_move
instead. That had two issues:
- Not all parts of the code did that which could end up
delaying the positioning either to the next completely
unrelated xdg commit event or to the next view_move call
- The code started to repeat itself, e.g. the same condition
with calls to either view_move or view_move_resize based
on the result
This patch fixes it by doing the check in the xdg configure
handler instead. Xwayland is unaffected by this issue as we
are always configuring a xwayland client with both, position
and size.
This PR allows applications to activate themselves *if they provide
a valid xdg_activation token* (e.g. raise to the top and get keyboard
focus).
These tokens are given out by the xdg_activation protocol implemented
by wlroots and can be configured by the client requesting the token
in three ways:
- an "empty" token
(apparently used to tell the compositor about "urgency")
- seat / input serial attached
- surface attached
Wlroots makes sure that
- If the client attached the seat / input serial: those two are valid.
- If the client attached a surface: that it has keyboard focus at the
point where the request is finalized. There is a patch [1] pending
for backport to wlroots 0.16 that also allows valid tokens when the
supplied surface had cursor focus.
- a token is only valid for 30 seconds after being given out
The token can then be used by the client or given to other clients by
unspecified means (e.g. via environment variable or dbus) which then
may use the token on their own surface and request activation.
We only handle the actual request activation part:
- If the seat is set on the token we know wlroots validated seat and
input serial
- Thus, if no seat is set we deny the activation request so we don't
have windows suddenly popping up and taking over the keyboard focus
(focus stealing prevention)
- We should also check for the surface being set but we can't do that
with wlroots 0.16 as it will reset the surface to `NULL` when it is
destroyed (which is something that usually happens for
notifications). Once we move to wlroots 0.17.x we can add the
missing surface check because it provides a `new_token` signal.
We can use it to attach further details to the token which are then
verified later when we decide if we allow the activate request or
not.
With this PR in place the following setup should activate windows:
- launching an URL in foot should activate the target application if
it is already running, foot requests a proper token and then sets it
as `XDG_ACTIVATION_TOKEN` environment var before spawning `xdg-open`
- clicking on a `mako` notification with a `default` action defined
should request a proper token which is then given to the application
starting the notification and can thus be used to activate itself
This protocol is still very much in the process of being
implemented / finalized all over the place (e.g. GTK / QT / Firefox /
notification daemons, ..) but we should do our part and remove labwc
from the puzzle of potential issues causing this not to work.
[1] f6008ffff4)
- Don't overwrite pending size in map() if it was already set
- Don't reposition view in map() if maximized/fullscreen
Also, as future-proofing in case we one day allow initially-tiled views,
replace explicit maximized/fullscreen checks with view_is_floating().
If xdg_toplevel_view_move() is called when a resize is pending
(e.g. after xdg_toplevel_view_resize() but before handle_commit()),
the newer x/y coordinates passed to move() should take precendence
over the older pending_move_resize.x/y coordinates.
This is consistent with the logic used in xwayland.c's move().
struct view's pending_move_resize.update_x/update_y flags appear to
be redundant, since we can easily determine whether x/y have been
update via a simple comparison in handle_commit().
The only corner case I can think of where this change might affect
behavior, is if xdg_toplevel_view_move() is called while a resize
is still pending (e.g. after xdg_toplevel_view_configure() but
before handle_commit()). This corner case will be addressed in the
following commit.
these should only be set on commit. doing so before then confuses
code that expects these fields to be in sync with the scene tree,
such as `handle_commit`.
these were only being set so that `view_center` could read them to
compute a centered position, so instead we can simply forward the
values directly to `view_compute_centered_position` and `view_move`.
`view->surface` had not been set to a non-`NULL` value here, so it
was not yet appropriate to access its `resource` member. instead,
use the resource from the argument xdg_surface.
furthermore, we need to free the newly-allocated `xdg_toplevel_view`
before error return.
- Store a pointer to the `struct view` in `struct ssd`
- Pass `struct ssd *` instead of `struct view *` to ssd functions
- Add `ssd_get_margin()` convenience function
Add xdg_surface_from_view() + xwayland_surface_from_view() accessors
that assert() the view is of the expected type before returning.
Fix a real bug in xdg.c parent_of() that dereferenced
`view->xdg_surface->toplevel` without first checking `view->type`.
The goal of the new accessors is to catch similar bugs in future.
IMHO it encourages better design (by making dependencies more obvious)
to have source file/header file pairs like view.c/view.h, rather than a
monolithic header like labwc.h with everything in it.
I don't think we need to break up all of labwc.h at once, but maybe we
can start pulling it apart bit by bit as it's convenient.
Also:
- Move "struct border" to ssd.h so that view.h can use it without pulling
in all of labwc.h.
- Add a missing required #include within scaled_font_buffer.h (forward
declaration of "struct font" is not enough).
Currently, snapping to a screen edge and then snapping to maximize
results in both the natural_geometry and tiled state of the view
getting messed up. After unmaximize, the view ends up in a weird
state (tiled location but natural/untiled size).
There are also a couple of sketchy things going on in the code:
- interactive_begin() pokes its own values into view->natural_geometry
to force view_maximize() to set a particular geometry.
- interactive_end() "fixes" view->natural_geometry after calling
view_maximize() to save the original geometry from the start of the
interactive move/resize.
To fix all this:
- Adjust/expand the API of view.c so that the interactive.c can
avoid this "back door" of overwriting view->natural_geometry
directly.
- Save the natural geometry and the tiled state of the view in
interactive_begin() when starting to move the view. When done,
interactive_end() will update the tiled state if appropriate but
*not* overwrite the natural geometry.
map() in xwayland.c called ssd_create() but did not call
view_apply_maximized_geometry() afterward, resulting in the
decorations being displayed off-screen.
Rather than calling view_apply_maximized_geometry() in more places,
let's reuse the existing call in view_set_decorations(), and extend
ssd_update_geometry() to call ssd_create() when needed.
At least for XWayland surfaces, handle_commit() is not always
called after map(), and as a result, the scene-graph node is never
positioned.
Not sure 100% if the same can occur with XDG surfaces, but the
extra view_moved() call should be harmless, so add it there too
for consistency.
For a move-only client configure request, treat it similarly to
view_move() by updating the scene-graph immediately, rather than waiting
for handle_commit(). Move-and-resize requests are handled the same as
before.
This (mostly?) fixes the glitchiness that was noticeable when dragging
an undecorated XWayland window (e.g. Audacious in Winamp mode).
Also:
- Reduce some code duplication in handle_request_configure() by
simply calling configure(), as suggested by @johanmalm in #428.
- Factor out common logic after a move and/or resize into view_moved().
...on first map so that view->unmaximized_geometry is known even if an
xdg-shell application started in maximized mode.
Fixes issue #305
Reported-by: @01micko
Consolidates all of the view destruction code for xwl + xdg into one function.
Fixes several notable bugs along the way:
- Fixes a crash when alt tabbing when a selected view gets destroyed.
- Fixes the OSD not updating to reflect a view has been destroyed.
Commit 08c537e ("xwayland: Honor size increments from
WM_SIZE_HINTS") adjusted only the window width/height according
to the size hints. If resizing from the top or left edge of the
window, we also need to adjust the window position to keep the
bottom or right edge from jumping around.
Support identification of wlr_scene_node role to enable simplification
of codebase including the avoidance of iterating over lists of
layer-surface, menuitems, and so on.
Use node-descriptors for xdg toplevels and popups
