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

diff --git a/include/view.h b/include/view.h
index 2888512fc0e34edfa1e77f45c1902054fd14aea6..46a8e29f4033382be2ab0fd6e8b053f2df0eb978 100644 (file)
--- a/include/view.h
+++ b/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);
-       /*
-        * 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 (*unmap)(struct view *view);
        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);

diff --git a/src/desktop.c b/src/desktop.c
index d805eae3b9858da93fd41820127a86cb32a27d57..76c3275e1e0633eda93716f5c5ef3ad868eb3c7b 100644 (file)
--- a/src/desktop.c
+++ b/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;
                }
        }

diff --git a/src/view-impl-common.c b/src/view-impl-common.c
index fdef56012a1cdf447c913cd1afe483c437ef107f..cf23a798f38bbda02c0461a027ca29d2ec570760 100644 (file)
--- a/src/view-impl-common.c
+++ b/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 */
-       if (!view->minimized) {
-               desktop_focus_view(view, /*raise*/ true);
-       }
+       view_update_visibility(view);
+
        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;
-       /*
-        * 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);
+       view_update_visibility(view);
 
        /*
-        * 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().
+        * Destroy the foreign toplevel handle so the unmapped view
+        * doesn't show up in panels and the like.
         */
-       if (view->fullscreen) {
-               output_set_has_fullscreen_view(view->output, false);
+       if (view->foreign_toplevel) {
+               foreign_toplevel_destroy(view->foreign_toplevel);
+               view->foreign_toplevel = NULL;
        }
 }
 

diff --git a/src/view.c b/src/view.c
index efad9d83c9865061c9c0058ee99d8076e23350f9..8a0d3cafb7a094cfc818cf46f0c82e44441ae12b 100644 (file)
--- a/src/view.c
+++ b/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->impl->unmap(view, /* client_request */ false);
-       } else {
-               view->impl->map(view);
-       }
+       view_update_visibility(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)
 {

diff --git a/src/xdg.c b/src/xdg.c
index 5c878ccdbca6bc2b1a83455321afdba7263f6a08..48179a6decc0b05321c960b2fa18d4f17e1eb7f6 100644 (file)
--- a/src/xdg.c
+++ b/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));
        }
 
-       /*
-        * 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);
-       }
+       view->mapped = 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

diff --git a/src/xwayland.c b/src/xwayland.c
index 1dda710b237d090b19a74b16a8d14aa23691ef8e..ed974a33d36e9bb0809d06c4e90d910a7db0254f 100644 (file)
--- a/src/xwayland.c
+++ b/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);
 
-       /*
-        * 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);
-       }
+       view->mapped = 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) {