view: separate (un)minimize and (un)map logic

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

include/view.h

@@ -110,13 +110,7 @@ struct view_impl {
 	void (*set_activated)(struct view *view, bool activated);
 	void (*set_fullscreen)(struct view *view, bool fullscreen);
 	void (*notify_tiled)(struct view *view);
-	/*
+	void (*unmap)(struct view *view);
-	 * client_request is true if the client unmapped its own
-	 * surface; false if we are just minimizing the view. The two
-	 * cases are similar but have subtle differences (e.g., when
-	 * minimizing we don't destroy the foreign toplevel handle).
-	 */
-	void (*unmap)(struct view *view, bool client_request);
 	void (*maximize)(struct view *view, enum view_axis maximized);
 	void (*minimize)(struct view *view, bool minimize);
 	struct view *(*get_parent)(struct view *self);
@@ -591,6 +585,7 @@ void view_adjust_size(struct view *view, int *w, int *h);
 void view_evacuate_region(struct view *view);
 void view_on_output_destroy(struct view *view);
 void view_connect_map(struct view *view, struct wlr_surface *surface);
+void view_update_visibility(struct view *view);
 
 void view_init(struct view *view);
 void view_destroy(struct view *view);

src/desktop.c

@@ -145,7 +145,7 @@ desktop_topmost_focusable_view(struct server *server)
 			continue;
 		}
 		view = node_view_from_node(node);
-		if (view->mapped && view_is_focusable(view)) {
+		if (view_is_focusable(view) && !view->minimized) {
 			return view;
 		}
 	}

src/view-impl-common.c

@@ -3,7 +3,6 @@
 #include "view-impl-common.h"
 #include "foreign-toplevel/foreign.h"
 #include "labwc.h"
-#include "output.h"
 #include "view.h"
 #include "window-rules.h"
 
@@ -28,10 +27,8 @@ view_impl_init_foreign_toplevel(struct view *view)
 void
 view_impl_map(struct view *view)
 {
-	/* Leave minimized, if minimized before map */
+	view_update_visibility(view);
-	if (!view->minimized) {
+
-		desktop_focus_view(view, /*raise*/ true);
-	}
 	if (!view->been_mapped) {
 		window_rules_apply(view, LAB_WINDOW_RULE_EVENT_ON_FIRST_MAP);
 	}
@@ -49,12 +46,6 @@ view_impl_map(struct view *view)
 		}
 	}
 
-	/*
-	 * Some clients (e.g. Steam's Big Picture Mode window) request
-	 * fullscreen before mapping.
-	 */
-	desktop_update_top_layer_visibility(view->server);
-
 	wlr_log(WLR_DEBUG, "[map] identifier=%s, title=%s",
 		view->app_id, view->title);
 }
@@ -62,30 +53,15 @@ view_impl_map(struct view *view)
 void
 view_impl_unmap(struct view *view)
 {
-	struct server *server = view->server;
+	view_update_visibility(view);
-	/*
-	 * When exiting an xwayland application with multiple views
-	 * mapped, a race condition can occur: after the topmost view
-	 * is unmapped, the next view under it is offered focus, but is
-	 * also unmapped before accepting focus (so server->active_view
-	 * remains NULL). To avoid being left with no active view at
-	 * all, check for that case also.
-	 */
-	if (view == server->active_view || !server->active_view) {
-		desktop_focus_topmost_view(server);
-	}
-
-	desktop_update_top_layer_visibility(view->server);
 
 	/*
-	 * We may need to disable adaptive sync if view was fullscreen.
+	 * Destroy the foreign toplevel handle so the unmapped view
-	 *
+	 * doesn't show up in panels and the like.
-	 * FIXME: this logic doesn't account for multiple fullscreen
-	 * views. It should probably be combined with the existing
-	 * logic in desktop_update_top_layer_visibility().
 	 */
-	if (view->fullscreen) {
+	if (view->foreign_toplevel) {
-		output_set_has_fullscreen_view(view->output, false);
+		foreign_toplevel_destroy(view->foreign_toplevel);
+		view->foreign_toplevel = NULL;
 	}
 }
 

src/view.c

@@ -423,7 +423,7 @@ view_is_focusable(struct view *view)
 	switch (view_wants_focus(view)) {
 	case VIEW_WANTS_FOCUS_ALWAYS:
 	case VIEW_WANTS_FOCUS_LIKELY:
-		return (view->mapped || view->minimized);
+		return view->mapped;
 	default:
 		return false;
 	}
@@ -786,11 +786,7 @@ _minimize(struct view *view, bool minimized)
 	view->minimized = minimized;
 	wl_signal_emit_mutable(&view->events.minimized, NULL);
 
-	if (minimized) {
+	view_update_visibility(view);
-		view->impl->unmap(view, /* client_request */ false);
-	} else {
-		view->impl->map(view);
-	}
 }
 
 static void
@@ -841,11 +837,6 @@ view_minimize(struct view *view, bool minimized)
 	struct view *root = view_get_root(view);
 	_minimize(root, minimized);
 	minimize_sub_views(root, minimized);
-
-	/* Enable top-layer when full-screen views are minimized */
-	if (view->fullscreen && view->output) {
-		desktop_update_top_layer_visibility(view->server);
-	}
 }
 
 bool
@@ -2465,7 +2456,7 @@ static void
 handle_unmap(struct wl_listener *listener, void *data)
 {
 	struct view *view = wl_container_of(listener, view, mappable.unmap);
-	view->impl->unmap(view, /* client_request */ true);
+	view->impl->unmap(view);
 }
 
 void
@@ -2475,6 +2466,58 @@ view_connect_map(struct view *view, struct wlr_surface *surface)
 	mappable_connect(&view->mappable, surface, handle_map, handle_unmap);
 }
 
+/* Used in both (un)map and (un)minimize */
+void
+view_update_visibility(struct view *view)
+{
+	bool visible = view->mapped && !view->minimized;
+	if (visible == view->scene_tree->node.enabled) {
+		return;
+	}
+
+	wlr_scene_node_set_enabled(&view->scene_tree->node, visible);
+	struct server *server = view->server;
+
+	if (visible) {
+		desktop_focus_view(view, /*raise*/ true);
+	} else {
+		/*
+		 * When exiting an xwayland application with multiple
+		 * views mapped, a race condition can occur: after the
+		 * topmost view is unmapped, the next view under it is
+		 * offered focus, but is also unmapped before accepting
+		 * focus (so server->active_view remains NULL). To avoid
+		 * being left with no active view at all, check for that
+		 * case also.
+		 */
+		if (view == server->active_view || !server->active_view) {
+			desktop_focus_topmost_view(server);
+		}
+	}
+
+	/*
+	 * Show top layer when a fullscreen view is hidden.
+	 * Hide it if a fullscreen view is shown (or uncovered).
+	 */
+	desktop_update_top_layer_visibility(server);
+
+	/*
+	 * We may need to disable adaptive sync if view was fullscreen.
+	 *
+	 * FIXME: this logic doesn't account for multiple fullscreen
+	 * views. It should probably be combined with the existing
+	 * logic in desktop_update_top_layer_visibility().
+	 */
+	if (view->fullscreen && !visible) {
+		output_set_has_fullscreen_view(view->output, false);
+	}
+
+	/* Update usable area to account for XWayland "struts" (panels) */
+	if (view_has_strut_partial(view)) {
+		output_update_all_usable_areas(server, false);
+	}
+}
+
 void
 view_set_shade(struct view *view, bool shaded)
 {

src/xdg.c

@@ -614,7 +614,7 @@ xdg_toplevel_view_append_children(struct view *self, struct wl_array *children)
 		if (view->type != LAB_XDG_SHELL_VIEW) {
 			continue;
 		}
-		if (!view->mapped && !view->minimized) {
+		if (!view->mapped) {
 			continue;
 		}
 		if (top_parent_of(view) != toplevel) {
@@ -757,15 +757,7 @@ xdg_toplevel_view_map(struct view *view)
 		view_set_output(view, output_nearest_to_cursor(view->server));
 	}
 
-	/*
+	view->mapped = true;
-	 * For initially minimized views, we do not set view->mapped
-	 * nor enable the scene node. All other map logic (positioning,
-	 * creating foreign toplevel, etc.) happens as normal.
-	 */
-	if (!view->minimized) {
-		view->mapped = true;
-		wlr_scene_node_set_enabled(&view->scene_tree->node, true);
-	}
 
 	if (!view->foreign_toplevel) {
 		view_impl_init_foreign_toplevel(view);
@@ -815,23 +807,12 @@ xdg_toplevel_view_map(struct view *view)
 }
 
 static void
-xdg_toplevel_view_unmap(struct view *view, bool client_request)
+xdg_toplevel_view_unmap(struct view *view)
 {
 	if (view->mapped) {
 		view->mapped = false;
-		wlr_scene_node_set_enabled(&view->scene_tree->node, false);
 		view_impl_unmap(view);
 	}
-
-	/*
-	 * If the view was explicitly unmapped by the client (rather
-	 * than just minimized), destroy the foreign toplevel handle so
-	 * the unmapped view doesn't show up in panels and the like.
-	 */
-	if (client_request && view->foreign_toplevel) {
-		foreign_toplevel_destroy(view->foreign_toplevel);
-		view->foreign_toplevel = NULL;
-	}
 }
 
 static pid_t

src/xwayland.c

@@ -39,7 +39,7 @@ static_assert(ARRAY_SIZE(atom_names) == ATOM_COUNT, "atom names out of sync");
 static xcb_atom_t atoms[ATOM_COUNT] = {0};
 
 static void set_surface(struct view *view, struct wlr_surface *surface);
-static void xwayland_view_unmap(struct view *view, bool client_request);
+static void xwayland_view_unmap(struct view *view);
 
 static struct xwayland_view *
 xwayland_view_from_view(struct view *view)
@@ -557,7 +557,7 @@ handle_set_override_redirect(struct wl_listener *listener, void *data)
 	struct server *server = view->server;
 	bool mapped = xsurface->surface && xsurface->surface->mapped;
 	if (mapped) {
-		xwayland_view_unmap(view, /* client_request */ true);
+		xwayland_view_unmap(view);
 	}
 	handle_destroy(&view->destroy, xsurface);
 	/* view is invalid after this point */
@@ -813,15 +813,7 @@ xwayland_view_map(struct view *view)
 	 */
 	handle_map_request(&xwayland_view->map_request, NULL);
 
-	/*
+	view->mapped = true;
-	 * For initially minimized views, we do not set view->mapped
-	 * nor enable the scene node. All other map logic (positioning,
-	 * creating foreign toplevel, etc.) happens as normal.
-	 */
-	if (!view->minimized) {
-		view->mapped = true;
-		wlr_scene_node_set_enabled(&view->scene_tree->node, true);
-	}
 
 	if (view->surface != xwayland_surface->surface) {
 		set_surface(view, xwayland_surface->surface);
@@ -877,38 +869,16 @@ xwayland_view_map(struct view *view)
 
 	view_impl_map(view);
 	view->been_mapped = true;
-
-	/* Update usable area to account for XWayland "struts" (panels) */
-	if (xwayland_surface->strut_partial) {
-		output_update_all_usable_areas(view->server, false);
-	}
 }
 
 static void
-xwayland_view_unmap(struct view *view, bool client_request)
+xwayland_view_unmap(struct view *view)
 {
 	if (!view->mapped) {
-		goto out;
+		return;
 	}
 	view->mapped = false;
-	wlr_scene_node_set_enabled(&view->scene_tree->node, false);
 	view_impl_unmap(view);
-
-	/* Update usable area to account for XWayland "struts" (panels) */
-	if (xwayland_surface_from_view(view)->strut_partial) {
-		output_update_all_usable_areas(view->server, false);
-	}
-
-	/*
-	 * If the view was explicitly unmapped by the client (rather
-	 * than just minimized), destroy the foreign toplevel handle so
-	 * the unmapped view doesn't show up in panels and the like.
-	 */
-out:
-	if (client_request && view->foreign_toplevel) {
-		foreign_toplevel_destroy(view->foreign_toplevel);
-		view->foreign_toplevel = NULL;
-	}
 }
 
 static void
@@ -959,7 +929,7 @@ xwayland_view_append_children(struct view *self, struct wl_array *children)
 		if (!view->surface) {
 			continue;
 		}
-		if (!view->mapped && !view->minimized) {
+		if (!view->mapped) {
 			continue;
 		}
 		if (top_parent_of(view) != surface) {