xwayland: notify correct window stacking order to xwayland

Before this commit, when a normal window is raised, xwayland thought it's
above always-on-top (AOT) windows even though it's actually below AOT
windows in the scene. This means mouse scroll events may be unexpectedly
sent to normal windows below AOT windows even when the cursor is hovering
over a AOT window.

So this commit fixes it by notifying the correct stacking order (where AOT
windows are placed above normal windows) to xwayland every time the
stacking order is updated.

Other benefits of this commit are:
- It makes the code more readable and predictable by aggregating logic
  about stacking order management in xwayland (e.g. shaded windows or
  windows in other workspaces should be notified to xwayland as being
  placed at the bottom).
- As server->last_raised_view is removed in the previous commit, we were
  notifying the stacking order to xwayland every time a window with dialog
  windows is clicked (not when clicking a topmost window without dialogs,
  due to some optimization in wlroots). This commit fixes this by caching
  the window stacking order in xwayland_view->stacking_order and notifying
  it to xwayland only when it's updated.

include/view-impl-common.h

@@ -10,8 +10,6 @@
 
 struct view;
 
-void view_impl_move_to_front(struct view *view);
-void view_impl_move_to_back(struct view *view);
 void view_impl_map(struct view *view);
 void view_impl_unmap(struct view *view);
 

include/view.h

@@ -142,9 +142,6 @@ struct view_impl {
 	void (*unmap)(struct view *view, bool client_request);
 	void (*maximize)(struct view *view, bool maximize);
 	void (*minimize)(struct view *view, bool minimize);
-	void (*move_to_front)(struct view *view);
-	void (*move_to_back)(struct view *view);
-	void (*shade)(struct view *view, bool shaded);
 	struct view *(*get_root)(struct view *self);
 	void (*append_children)(struct view *self, struct wl_array *children);
 	struct view_size_hints (*get_size_hints)(struct view *self);

include/xwayland.h

@@ -59,6 +59,9 @@ struct xwayland_view {
 	struct view base;
 	struct wlr_xwayland_surface *xwayland_surface;
 
+	/* Used to detect stacking order updates */
+	int stacking_order;
+
 	/* Events unique to XWayland views */
 	struct wl_listener associate;
 	struct wl_listener dissociate;

src/view-impl-common.c

@@ -2,29 +2,12 @@
 /* view-impl-common.c: common code for shell view->impl functions */
 #include <stdio.h>
 #include <strings.h>
-#include "common/list.h"
 #include "foreign-toplevel.h"
 #include "labwc.h"
 #include "view.h"
 #include "view-impl-common.h"
 #include "window-rules.h"
 
-void
-view_impl_move_to_front(struct view *view)
-{
-	wl_list_remove(&view->link);
-	wl_list_insert(&view->server->views, &view->link);
-	wlr_scene_node_raise_to_top(&view->scene_tree->node);
-}
-
-void
-view_impl_move_to_back(struct view *view)
-{
-	wl_list_remove(&view->link);
-	wl_list_append(&view->server->views, &view->link);
-	wlr_scene_node_lower_to_bottom(&view->scene_tree->node);
-}
-
 void
 view_impl_map(struct view *view)
 {

src/view.c

@@ -5,6 +5,7 @@
 #include <wlr/types/wlr_output_layout.h>
 #include <wlr/types/wlr_security_context_v1.h>
 #include "common/box.h"
+#include "common/list.h"
 #include "common/macros.h"
 #include "common/match.h"
 #include "common/mem.h"
@@ -2248,17 +2249,17 @@ for_each_subview(struct view *view, void (*action)(struct view *))
 static void
 move_to_front(struct view *view)
 {
-	if (view->impl->move_to_front) {
-		view->impl->move_to_front(view);
-	}
+	wl_list_remove(&view->link);
+	wl_list_insert(&view->server->views, &view->link);
+	wlr_scene_node_raise_to_top(&view->scene_tree->node);
 }
 
 static void
 move_to_back(struct view *view)
 {
-	if (view->impl->move_to_back) {
-		view->impl->move_to_back(view);
-	}
+	wl_list_remove(&view->link);
+	wl_list_append(&view->server->views, &view->link);
+	wlr_scene_node_lower_to_bottom(&view->scene_tree->node);
 }
 
 /*
@@ -2282,6 +2283,12 @@ view_move_to_front(struct view *view)
 		move_to_front(view);
 	}
 
+#if HAVE_XWAYLAND
+	if (view->type == LAB_XWAYLAND_VIEW) {
+		xwayland_adjust_stacking_order(view->server);
+	}
+#endif
+
 	cursor_update_focus(view->server);
 	desktop_update_top_layer_visibility(view->server);
 }
@@ -2296,6 +2303,12 @@ view_move_to_back(struct view *view)
 	for_each_subview(root, move_to_back);
 	move_to_back(root);
 
+#if HAVE_XWAYLAND
+	if (view->type == LAB_XWAYLAND_VIEW) {
+		xwayland_adjust_stacking_order(view->server);
+	}
+#endif
+
 	cursor_update_focus(view->server);
 	desktop_update_top_layer_visibility(view->server);
 }
@@ -2468,9 +2481,11 @@ view_set_shade(struct view *view, bool shaded)
 	ssd_enable_shade(view->ssd, view->shaded);
 	wlr_scene_node_set_enabled(&view->content_tree->node, !view->shaded);
 
-	if (view->impl->shade) {
-		view->impl->shade(view, shaded);
+#if HAVE_XWAYLAND
+	if (view->type == LAB_XWAYLAND_VIEW) {
+		xwayland_adjust_stacking_order(view->server);
 	}
+#endif
 }
 
 void

src/xdg.c

@@ -798,8 +798,6 @@ static const struct view_impl xdg_toplevel_view_impl = {
 	.unmap = xdg_toplevel_view_unmap,
 	.maximize = xdg_toplevel_view_maximize,
 	.minimize = xdg_toplevel_view_minimize,
-	.move_to_front = view_impl_move_to_front,
-	.move_to_back = view_impl_move_to_back,
 	.get_root = xdg_toplevel_view_get_root,
 	.append_children = xdg_toplevel_view_append_children,
 	.get_size_hints = xdg_toplevel_view_get_size_hints,

src/xwayland.c

@@ -814,40 +814,6 @@ xwayland_view_minimize(struct view *view, bool minimized)
 		minimized);
 }
 
-static void
-xwayland_view_move_to_front(struct view *view)
-{
-	view_impl_move_to_front(view);
-
-	if (view->shaded) {
-		/*
-		 * Ensure that we don't raise a shaded window
-		 * to the front which then steals mouse events.
-		 */
-		return;
-	}
-
-	/*
-	 * Update XWayland stacking order.
-	 *
-	 * FIXME: it would be better to restack above the next lower
-	 * view, rather than on top of all other surfaces. Restacking
-	 * the unmanaged surfaces afterward is ugly and still doesn't
-	 * account for always-on-top views.
-	 */
-	wlr_xwayland_surface_restack(xwayland_surface_from_view(view),
-		NULL, XCB_STACK_MODE_ABOVE);
-}
-
-static void
-xwayland_view_move_to_back(struct view *view)
-{
-	view_impl_move_to_back(view);
-	/* Update XWayland stacking order */
-	wlr_xwayland_surface_restack(xwayland_surface_from_view(view),
-		NULL, XCB_STACK_MODE_BELOW);
-}
-
 static struct view *
 xwayland_view_get_root(struct view *view)
 {
@@ -927,20 +893,6 @@ xwayland_view_get_pid(struct view *view)
 	return xwayland_surface->pid;
 }
 
-static void
-xwayland_view_shade(struct view *view, bool shaded)
-{
-	assert(view);
-
-	/* Ensure that clicks on some xwayland surface don't end up on the shaded one */
-	if (shaded) {
-		wlr_xwayland_surface_restack(xwayland_surface_from_view(view),
-			NULL, XCB_STACK_MODE_BELOW);
-	} else {
-		xwayland_adjust_stacking_order(view->server);
-	}
-}
-
 static const struct view_impl xwayland_view_impl = {
 	.configure = xwayland_view_configure,
 	.close = xwayland_view_close,
@@ -951,9 +903,6 @@ static const struct view_impl xwayland_view_impl = {
 	.unmap = xwayland_view_unmap,
 	.maximize = xwayland_view_maximize,
 	.minimize = xwayland_view_minimize,
-	.move_to_front = xwayland_view_move_to_front,
-	.move_to_back = xwayland_view_move_to_back,
-	.shade = xwayland_view_shade,
 	.get_root = xwayland_view_get_root,
 	.append_children = xwayland_view_append_children,
 	.get_size_hints = xwayland_view_get_size_hints,
@@ -974,6 +923,9 @@ xwayland_view_create(struct server *server,
 	view->type = LAB_XWAYLAND_VIEW;
 	view->impl = &xwayland_view_impl;
 
+	/* Set to -1 so we always restack the view on map */
+	xwayland_view->stacking_order = -1;
+
 	/*
 	 * Set two-way view <-> xsurface association.  Usually the association
 	 * remains until the xsurface is destroyed (which also destroys the
@@ -1143,44 +1095,94 @@ xwayland_server_init(struct server *server, struct wlr_compositor *compositor)
 	}
 }
 
-/*
- * Until we expose the workspaces to xwayland we need a way to
- * ensure that xwayland views on the current workspace are always
- * stacked above xwayland views on other workspaces.
- *
- * If we fail to do so, issues arise in scenarios where we change
- * the mouse focus but do not change the (xwayland) stacking order.
- *
- * Reproducer:
- * - open at least two xwayland windows which allow scrolling
- *   (some X11 terminal with 'man man' for example)
- * - switch to another workspace, open another xwayland
- *   window which allows scrolling and maximize it
- * - switch back to the previous workspace with the two windows
- * - move the mouse to the xwayland window that does *not* have focus
- * - start scrolling
- * - all scroll events should end up on the maximized window on the other workspace
- */
+enum xwayland_view_layer {
+	XWAYLAND_VIEW_HIDDEN,
+	XWAYLAND_VIEW_BOTTOM,
+	XWAYLAND_VIEW_NORMAL,
+	XWAYLAND_VIEW_TOP,
+};
+
+static enum xwayland_view_layer
+get_layer(struct view *view)
+{
+	if (view->workspace != view->server->workspaces.current) {
+		/*
+		 * Until we expose the workspaces to xwayland we need a way to
+		 * ensure that xwayland views on the current workspace are
+		 * always stacked above xwayland views on other workspaces.
+		 *
+		 * If we fail to do so, issues arise in scenarios where we
+		 * change the mouse focus but do not change the (xwayland)
+		 * stacking order.
+		 *
+		 * Reproducer:
+		 * - open at least two xwayland windows which allow scrolling
+		 *   (e.g. urxvt with 'man man')
+		 * - switch to another workspace, open another xwayland window
+		 *   which allows scrolling and maximize it
+		 * - switch back to the previous workspace with the two windows
+		 * - move the mouse to the xwayland window that does *not* have
+		 *   focus
+		 * - start scrolling
+		 * - all scroll events should end up on the maximized window on
+		 *   the other workspace
+		 */
+		return XWAYLAND_VIEW_HIDDEN;
+	} else if (view->shaded) {
+		/*
+		 * Ensure that we don't raise a shaded window to the front
+		 * which then steals mouse events.
+		 */
+		return XWAYLAND_VIEW_HIDDEN;
+	} else if (view_is_always_on_bottom(view)) {
+		return XWAYLAND_VIEW_BOTTOM;
+	} else if (view_is_always_on_top(view)) {
+		return XWAYLAND_VIEW_TOP;
+	} else {
+		return XWAYLAND_VIEW_NORMAL;
+	}
+}
+
 void
 xwayland_adjust_stacking_order(struct server *server)
 {
-	struct view **view;
-	struct wl_array views;
-
-	wl_array_init(&views);
-	view_array_append(server, &views, LAB_VIEW_CRITERIA_ALWAYS_ON_TOP);
-	view_array_append(server, &views, LAB_VIEW_CRITERIA_CURRENT_WORKSPACE
-		| LAB_VIEW_CRITERIA_NO_ALWAYS_ON_TOP);
-
-	/*
-	 * view_array_append() provides top-most windows
-	 * first so we simply reverse the iteration here
-	 */
-	wl_array_for_each_reverse(view, &views) {
-		view_move_to_front(*view);
+	if (!server->xwayland) {
+		/* This happens when windows are unmapped on exit */
+		return;
 	}
 
-	wl_array_release(&views);
+	int stacking_order = 0;
+	bool update = false;
+
+	/*
+	 * Iterate over the windows from bottom to top and notify their
+	 * stacking order to xwayland if we detect updates in it. Note
+	 * that server->views are sorted from top to bottom but doesn't
+	 * consider always-on-{top,bottom} windows.
+	 */
+	for (enum xwayland_view_layer layer = XWAYLAND_VIEW_HIDDEN;
+			layer <= XWAYLAND_VIEW_TOP; layer++) {
+		struct view *view;
+		wl_list_for_each_reverse(view, &server->views, link) {
+			if (view->type != LAB_XWAYLAND_VIEW
+					|| layer != get_layer(view)) {
+				continue;
+			}
+
+			struct xwayland_view *xwayland_view =
+				xwayland_view_from_view(view);
+
+			/* On detecting update, restack all the views above it */
+			update |= xwayland_view->stacking_order != stacking_order;
+			if (update) {
+				wlr_xwayland_surface_restack(
+					xwayland_view->xwayland_surface,
+					NULL, XCB_STACK_MODE_ABOVE);
+			}
+			xwayland_view->stacking_order = stacking_order;
+			stacking_order++;
+		}
+	}
 }
 
 void