...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
The fix caused a couple of issues:
1. Ignoring client configure requests caused some clients to hang
and not repaint correctly. We are supposed to synthesize a
ConfigureNotify event when ignore/override a client configure
request, but this isn't possible with current wlroots.
2. Setting view->natural_geometry from client configure requests
resulted in overwriting good values with bad in some cases (e.g.
with tiled xfce4-terminal in xwayland mode).
For now, revert the fix. This does allow clients to mess with view
positioning for maximized/fullscreen/tiled views, but right now the
alternatives seem worse.
The original specific issue (VLC undoing its fullscreen geometry)
is arguably a bug in VLC anyway.
This reverts commit 09599861ac.
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.
- 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().
- Move xwayland-specific struct definitions to new xwayland.h header
- Move xwayland_move_sub_views_to_front() from desktop.c to xwayland.c
- Split out xwayland_server_init/finish() from server_init/finish()
- Rename new_xwayland_surface -> xwayland_new_surface and
xwayland_surface_new() -> handle_new_surface() for consistency
- Add "mapped" argument to xwayland_unmanaged_create() so that we can
make unmanaged_handle_map() private to xwayland-unmanaged.c
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).
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.
Each XWayland view is paired with a particular wlr_xwayland_surface and
its lifetime is tied to that surface. This condition in handle_map():
if (xsurface != view->xwayland_surface)
could never be true since the view is only registered to receive the
"map" signal from view->xwayland_surface, and no other. So the code
updating view->xwayland_surface in handle_map() was dead.
So let's clean things up a little:
- Remove the dead code
- Add some comments, and slightly rearrange code to match
- Add/update assert()s in signal handlers for consistency
- Pass xsurface as <data> when calling handle_unmap() and
handle_destroy() explicitly, to be consistent
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().
When a view is destroyed labwc calls interactive_end(view) which
may reposition the view which is partly destroyed and doesn't own
any surface anymore. To prevent this scenario from happening don't
call interactive_end() at all and just reset server->grabbed_view
and server->input_mode directly.
Before this patch, the bug could be reproduced by:
- xcalc &
- sleep 5; killall xcalc
- move the xcalc window completely to one of the edges
The change in src/xwayland.c is not required for this bug
to be fixed but may prevent something similar in the future.
Set node position in the configure/set_geometry handlers when moving a
window in response to a client request.
Steps to reproduce weird positioning fixed by this patch:
1. Start leafpad
2. Open Help->About
3. Move the dialog
4. Close the dialog
5. Open it again
6. Drag it and observe a jump in position
There is also an xwayland PyQt5 script in PR #428 which demonstrates
jumpy position.
- Add missing call to wlr_scene_node_set_position() in
unmanaged_handle_commit() -- this fixes moving unmanaged XWayland
windows.
- Update view->pending_move_resize when we receive a configure request
for a managed XWayland surface -- this fixes moving managed but
undecorated XWayland windows.
- Also update view->pending_move_resize when we move a surface while a
configure request is already pending -- this fixes a discrepancy
between displayed and actual position for XWayland windows that try to
set their own initial position, but then get overridden by labwc's
positioning.
Moving undecorated XWayland windows is still really glitchy -- it appears
that an XWayland window gets sent incorrect mouse motion coordinates when
there is a pending configure request moving the window itself.
Some xwayland clients leave unmapped child views around, typically when a
dialog window is closed. Although handle_destroy() is not called for
these, we have to call foreign-toplevel-destroy to avoid panels and the
like incorrecly showing them.
Move SSD related map() work into the !view->been_mapped branch,
similar to src/xdg.c.
Without this patch following series of events restores the initial
state of the SSD when activating the view:
- spawn xcalc
- observe it shows the SSD
- toggleDecorations to hide the SSD (via keybinding or window menu)
- minimize xcalc (via some panel or A-Space)
- activate xcalc (via some panel or A-Tab)
- observe the SSD is back visible
This is needed to allow X11 applications to create surfaces as
non-override_redirect and then change them to override_redirect later
Without this gitk-menus and rofi are treated as xwayland-views with
associated server-side-decoration and forced positioning.
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.
