From a5fcbfaf720fc6c2486d850012215102e90e04bf Mon Sep 17 00:00:00 2001 From: Johan Malm Date: Thu, 7 Mar 2024 19:15:02 +0000 Subject: [PATCH] cursor: process layer subsurfaces in `cursor_button_press()` ...to give keyboard focus to layer-shell clients if exclusive or on-demand interactivity is set, so that menu popups can be navigated with the keyboard. This still only works if the client is in top (or overlay) layers. Support for bottom and background to be done as a separate patch set. Revert 06b19f0 to process layer-shell subsurfaces in `cursor_button_press()`, but only when their parent layer-shell surface has keyboard interactivity. Fix bug in `get_cursor_context()` which resulted in layer-surfaces not being detected correctly. Background: Commit 06b19f0 (issue #1131) disabled processing of layer-shell subsurfaces in cursor_button_press() because when pressing a task in Waybar (Gtk panel using layer-shell subsurfaces) the foreign-toplevel minimize-raise action did not work correctly as the action logic relied on the recipient window being activated and by clicking on the panel, the panel itself was both surface-focusd and activated (and thus the window de-activated). The un-intended consequence was that by not responding to layer-subsurface cursor buttons presses, layer-shell clients (such as panels) were not given keyboard focus if they indeed wanted it by setting exclusive or on-demand keyboard interactivity. The good news is that that following @jlindgren90's refactoring (various) the only place where we call `view_set_actived()` is in `focus_change_notify()` in `seat.c` and we now only do it for views (bb8f0bc). Another side-effect (positive) of 06b19f0 was that a Waybar dnd bug was fixed (pointer-serial-number validation failure). Have tested with sfwbar, waybar and tint (test-panel) the following results: - Minimize-raise works even when on-demand keyboard interactivity is set - Keyboard interactivity is given popup-menus (sfwbar and tint) when the panels are in the top layer (support for bottom will be as a separate patch set) - Waybar dnd still works (even when hard-coding keyboard-interactivity) References: - https://github.com/labwc/labwc/commit/bb8f0bc960dca192b8579d67297c0586ec20bfe0 - https://github.com/labwc/labwc/blob/40ce95a68cf19796dd67b0527fddfdbe46181805/src/seat.c#L481-L483 - https://github.com/labwc/labwc/blob/40ce95a68cf19796dd67b0527fddfdbe46181805/src/dnd.c#L24 - https://github.com/johanmalm/tint Fixes: #1572 --- include/common/surface-helpers.h | 16 ++++++++++ src/common/meson.build | 1 + src/common/surface-helpers.c | 28 ++++++++++++++++++ src/desktop.c | 51 +++++++++++--------------------- src/input/cursor.c | 15 ++++++---- 5 files changed, 72 insertions(+), 39 deletions(-) create mode 100644 include/common/surface-helpers.h create mode 100644 src/common/surface-helpers.c diff --git a/include/common/surface-helpers.h b/include/common/surface-helpers.h new file mode 100644 index 00000000..1dd7fd74 --- /dev/null +++ b/include/common/surface-helpers.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef LABWC_SURFACE_HELPERS_H +#define LABWC_SURFACE_HELPERS_H + +struct wlr_surface; +struct wlr_layer_surface_v1; + +/** + * subsurface_parent_layer() - Get wlr_layer_surface from layer-subsurface + * @wlr_surface: The wlr_surface of the wlr_subsurface for which to get the + * layer-surface. + */ +struct wlr_layer_surface_v1 *subsurface_parent_layer( + struct wlr_surface *wlr_surface); + +#endif /* LABWC_SURFACE_HELPERS_H */ diff --git a/src/common/meson.build b/src/common/meson.build index 72ad6f2b..6a90f9b3 100644 --- a/src/common/meson.build +++ b/src/common/meson.build @@ -13,6 +13,7 @@ labwc_sources += files( 'scaled_font_buffer.c', 'scaled_scene_buffer.c', 'scene-helpers.c', + 'surface-helpers.c', 'spawn.c', 'string-helpers.c', ) diff --git a/src/common/surface-helpers.c b/src/common/surface-helpers.c new file mode 100644 index 00000000..76102e8b --- /dev/null +++ b/src/common/surface-helpers.c @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include +#include +#include +#include "common/surface-helpers.h" + +struct wlr_layer_surface_v1 * +subsurface_parent_layer(struct wlr_surface *wlr_surface) +{ + struct wlr_subsurface *subsurface = + wlr_subsurface_try_from_wlr_surface(wlr_surface); + if (!subsurface) { + wlr_log(WLR_DEBUG, "surface %p is not subsurface", subsurface); + return NULL; + } + struct wlr_surface *parent = subsurface->parent; + if (!parent) { + wlr_log(WLR_DEBUG, "subsurface %p has no parent", subsurface); + return NULL; + } + struct wlr_layer_surface_v1 *wlr_layer_surface = + wlr_layer_surface_v1_try_from_wlr_surface(parent); + if (wlr_layer_surface) { + return wlr_layer_surface; + } + /* Recurse in case there are nested sub-surfaces */ + return subsurface_parent_layer(parent); +} diff --git a/src/desktop.c b/src/desktop.c index 412de004..4aca14b8 100644 --- a/src/desktop.c +++ b/src/desktop.c @@ -2,6 +2,7 @@ #include "config.h" #include #include "common/scene-helpers.h" +#include "common/surface-helpers.h" #include "dnd.h" #include "labwc.h" #include "layers.h" @@ -255,21 +256,6 @@ get_surface_from_layer_node(struct wlr_scene_node *node) return NULL; } -static bool -is_layer_descendant(struct wlr_scene_node *node) -{ - goto start; - while (node) { - struct node_descriptor *desc = node->data; - if (desc && desc->type == LAB_NODE_DESC_LAYER_SURFACE) { - return true; - } -start: - node = node->parent ? &node->parent->node : NULL; - } - return false; -} - /* TODO: make this less big and scary */ struct cursor_context get_cursor_context(struct server *server) @@ -354,24 +340,23 @@ get_cursor_context(struct server *server) /* Edge-case nodes without node-descriptors */ if (node->type == WLR_SCENE_NODE_BUFFER) { struct wlr_surface *surface = lab_wlr_surface_from_node(node); - if (surface) { - if (wlr_layer_surface_v1_try_from_wlr_surface(surface)) { - ret.type = LAB_SSD_LAYER_SURFACE; - } - if (is_layer_descendant(node)) { - /* - * layer-shell subsurfaces need to be - * able to receive pointer actions. - * - * Test by running - * `gtk-layer-demo -k exclusive`, then - * open the 'set margin' dialog and try - * setting the margin with the pointer. - */ - ret.surface = surface; - ret.type = LAB_SSD_LAYER_SUBSURFACE; - return ret; - } + + /* + * Handle layer-shell subsurfaces + * + * These don't have node-descriptors, but need to be + * able to receive pointer actions so we have to process + * them here. + * + * Test by running `gtk-layer-demo -k exclusive`, then + * open the 'set margin' dialog and try setting the + * margin with the pointer. + */ + if (surface && wlr_subsurface_try_from_wlr_surface(surface) + && subsurface_parent_layer(surface)) { + ret.surface = surface; + ret.type = LAB_SSD_LAYER_SUBSURFACE; + return ret; } } diff --git a/src/input/cursor.c b/src/input/cursor.c index b06a9ca4..fd7d0e61 100644 --- a/src/input/cursor.c +++ b/src/input/cursor.c @@ -11,6 +11,7 @@ #include "common/macros.h" #include "common/mem.h" #include "common/scene-helpers.h" +#include "common/surface-helpers.h" #include "config/mousebind.h" #include "dnd.h" #include "idle.h" @@ -965,11 +966,19 @@ cursor_button_press(struct seat *seat, uint32_t button, * the Focus action (used for normal views) does not work. */ if (ctx.type == LAB_SSD_LAYER_SURFACE) { + wlr_log(WLR_DEBUG, "press on layer-surface"); struct wlr_layer_surface_v1 *layer = wlr_layer_surface_v1_try_from_wlr_surface(ctx.surface); if (layer && layer->current.keyboard_interactive) { seat_set_focus_layer(seat, layer); } + } else if (ctx.type == LAB_SSD_LAYER_SUBSURFACE) { + wlr_log(WLR_DEBUG, "press on layer-subsurface"); + struct wlr_layer_surface_v1 *layer = + subsurface_parent_layer(ctx.surface); + if (layer && layer->current.keyboard_interactive) { + seat_set_focus_layer(seat, layer); + } #ifdef HAVE_XWAYLAND } else if (ctx.type == LAB_SSD_UNMANAGED) { desktop_focus_view_or_surface(seat, NULL, ctx.surface, @@ -977,12 +986,6 @@ cursor_button_press(struct seat *seat, uint32_t button, #endif } - /* - * TODO: We may need to handle press on layer-shell subsurfaces here, - * but need to check keyboard interactivity before focusing them - * otherwise we break waybar. See issue #1131 - */ - if (ctx.type != LAB_SSD_CLIENT && ctx.type != LAB_SSD_LAYER_SUBSURFACE && wlr_seat_pointer_has_grab(seat->seat)) { /* -- 2.52.0