interactive: Refactor natural_geometry/tiled state handling

Currently, snapping to a screen edge and then snapping to maximize
results in both the natural_geometry and tiled state of the view
getting messed up. After unmaximize, the view ends up in a weird
state (tiled location but natural/untiled size).

There are also a couple of sketchy things going on in the code:

- interactive_begin() pokes its own values into view->natural_geometry
  to force view_maximize() to set a particular geometry.

- interactive_end() "fixes" view->natural_geometry after calling
  view_maximize() to save the original geometry from the start of the
  interactive move/resize.

To fix all this:

- Adjust/expand the API of view.c so that the interactive.c can
  avoid this "back door" of overwriting view->natural_geometry
  directly.

- Save the natural geometry and the tiled state of the view in
  interactive_begin() when starting to move the view.  When done,
  interactive_end() will update the tiled state if appropriate but
  *not* overwrite the natural geometry.

diff --git a/include/labwc.h b/include/labwc.h
index c16cc036f4bbc2b993c5192db59573eb8bc07146..f5bf413401e512229cac99059a6cd3cb58d70d60 100644 (file)
--- a/include/labwc.h
+++ b/include/labwc.h
@@ -480,8 +480,11 @@ void view_moved(struct view *view);
 void view_minimize(struct view *view, bool minimized);
 /* view_wlr_output - return the output that a view is mostly on */
 struct wlr_output *view_wlr_output(struct view *view);
+void view_store_natural_geometry(struct view *view);
 void view_center(struct view *view);
-void view_maximize(struct view *view, bool maximize);
+void view_restore_to(struct view *view, struct wlr_box geometry);
+void view_maximize(struct view *view, bool maximize,
+       bool store_natural_geometry);
 void view_set_fullscreen(struct view *view, bool fullscreen,
        struct wlr_output *wlr_output);
 void view_toggle_maximize(struct view *view);
@@ -492,7 +495,8 @@ void view_toggle_fullscreen(struct view *view);
 void view_adjust_for_layout_change(struct view *view);
 void view_discover_output(struct view *view);
 void view_move_to_edge(struct view *view, const char *direction);
-void view_snap_to_edge(struct view *view, const char *direction);
+void view_snap_to_edge(struct view *view, const char *direction,
+       bool store_natural_geometry);
 const char *view_get_string_prop(struct view *view, const char *prop);
 void view_update_title(struct view *view);
 void view_update_app_id(struct view *view);
@@ -562,7 +566,8 @@ void seat_reset_pressed(struct seat *seat);
 
 void interactive_begin(struct view *view, enum input_mode mode,
                      uint32_t edges);
-void interactive_end(struct view *view);
+void interactive_finish(struct view *view);
+void interactive_cancel(struct view *view);
 
 void output_init(struct server *server);
 void output_manager_init(struct server *server);

diff --git a/src/action.c b/src/action.c
index 6ead08735b5527a15d27e5473967af517729d0e4..e1d8778d8a5d0aa0c64e837d5344607a67bfad94 100644 (file)
--- a/src/action.c
+++ b/src/action.c
@@ -254,7 +254,8 @@ actions_run(struct view *activator, struct server *server,
                        break;
                case ACTION_TYPE_SNAP_TO_EDGE:
                        if (arg) {
-                               view_snap_to_edge(view, action_str_from_arg(arg));
+                               view_snap_to_edge(view, action_str_from_arg(arg),
+                                       /*store_natural_geometry*/ true);
                        } else {
                                wlr_log(WLR_ERROR, "Missing argument for SnapToEdge");
                        }

diff --git a/src/cursor.c b/src/cursor.c
index 888b032702c7c5ce6f55c3417a6334c064409aea..b81583c9a8ed2372d4dbb3171a97381b0aaac9e2 100644 (file)
--- a/src/cursor.c
+++ b/src/cursor.c
@@ -848,7 +848,7 @@ cursor_button_release(struct seat *seat, struct wlr_pointer_button_event *event)
 
        if (server->input_mode != LAB_INPUT_STATE_PASSTHROUGH) {
                /* Exit interactive move/resize mode */
-               interactive_end(server->grabbed_view);
+               interactive_finish(server->grabbed_view);
                return;
        }
 

diff --git a/src/foreign.c b/src/foreign.c
index 658dca32d8c4cf4c4f84414f8e293ef7adb3ffe9..915520b6821e07d92b962aaf26295828cce3695d 100644 (file)
--- a/src/foreign.c
+++ b/src/foreign.c
@@ -20,7 +20,8 @@ handle_toplevel_handle_request_maximize(struct wl_listener *listener, void *data
                toplevel_handle_request_maximize);
        struct wlr_foreign_toplevel_handle_v1_maximized_event *event = data;
        if (view) {
-               view_maximize(view, event->maximized);
+               view_maximize(view, event->maximized,
+                       /*store_natural_geometry*/ true);
        }
 }
 

diff --git a/src/interactive.c b/src/interactive.c
index 99622fc1a017e08b0afcd4c198aeee3e1122fdd8..2691cccd2449b8e225cc780f74ee540ac42f463f 100644 (file)
--- a/src/interactive.c
+++ b/src/interactive.c
@@ -17,124 +17,154 @@ max_move_scale(double pos_cursor, double pos_current,
 void
 interactive_begin(struct view *view, enum input_mode mode, uint32_t edges)
 {
-       if (mode == LAB_INPUT_STATE_MOVE && view->fullscreen) {
-               /**
-                * We don't allow moving fullscreen windows.
-                *
-                * If you think there is a good reason to allow it
-                * feel free to open an issue explaining your use-case.
-                */
-               return;
-       }
-       if (mode == LAB_INPUT_STATE_RESIZE
-                       && (view->fullscreen || view->maximized)) {
-               /* We don't allow resizing while in maximized or fullscreen state */
-               return;
-       }
-
        /*
         * This function sets up an interactive move or resize operation, where
         * the compositor stops propagating pointer events to clients and
         * instead consumes them itself, to move or resize windows.
         */
-       struct seat *seat = &view->server->seat;
        struct server *server = view->server;
-       server->grabbed_view = view;
-       server->input_mode = mode;
+       struct seat *seat = &server->seat;
+       struct wlr_box geometry = {
+               .x = view->x,
+               .y = view->y,
+               .width = view->w,
+               .height = view->h
+       };
+
+       switch (mode) {
+       case LAB_INPUT_STATE_MOVE:
+               if (view->fullscreen) {
+                       /**
+                        * We don't allow moving fullscreen windows.
+                        *
+                        * If you think there is a good reason to allow
+                        * it, feel free to open an issue explaining
+                        * your use-case.
+                        */
+                       return;
+               }
+               if (view->maximized || view->tiled) {
+                       /*
+                        * Un-maximize and restore natural width/height.
+                        * Don't reset tiled state yet since we may want
+                        * to keep it (in the snap-to-maximize case).
+                        */
+                       geometry = view->natural_geometry;
+                       geometry.x = max_move_scale(seat->cursor->x, view->x,
+                               view->w, geometry.width);
+                       geometry.y = max_move_scale(seat->cursor->y, view->y,
+                               view->h, geometry.height);
+                       view_restore_to(view, geometry);
+               } else {
+                       /* Store natural geometry at start of move */
+                       view_store_natural_geometry(view);
+               }
+               cursor_set(seat, LAB_CURSOR_GRAB);
+               break;
+       case LAB_INPUT_STATE_RESIZE:
+               if (view->maximized || view->fullscreen) {
+                       /*
+                        * We don't allow resizing while maximized or
+                        * fullscreen.
+                        */
+                       return;
+               }
+               /*
+                * Reset tiled state but keep the same geometry as the
+                * starting point for the resize.
+                */
+               view->tiled = 0;
+               cursor_set(seat, cursor_get_from_edge(edges));
+               break;
+       default:
+               /* Should not be reached */
+               return;
+       }
 
+       server->input_mode = mode;
+       server->grabbed_view = view;
        /* Remember view and cursor positions at start of move/resize */
        server->grab_x = seat->cursor->x;
        server->grab_y = seat->cursor->y;
-       server->grab_box.x = view->x;
-       server->grab_box.y = view->y;
-       server->grab_box.width = view->w;
-       server->grab_box.height = view->h;
+       server->grab_box = geometry;
        server->resize_edges = edges;
+}
 
-       if (view->maximized || view->tiled) {
-               if (mode == LAB_INPUT_STATE_MOVE) {
-                       /* Exit maximized or tiled mode */
-                       int new_x = max_move_scale(view->server->seat.cursor->x,
-                               view->x, view->w, view->natural_geometry.width);
-                       int new_y = max_move_scale(view->server->seat.cursor->y,
-                               view->y, view->h, view->natural_geometry.height);
-                       view->natural_geometry.x = new_x;
-                       view->natural_geometry.y = new_y;
-                       if (view->maximized) {
-                               view_maximize(view, false);
-                       }
-                       if (view->tiled) {
-                               view_move_resize(view, view->natural_geometry);
-                       }
-                       /**
-                        * view_maximize() / view_move_resize() indirectly calls
-                        * view->impl->configure which is async but we are using
-                        * the current values in server->grab_box. We pretend the
-                        * configure already happened by setting them manually.
-                        */
-                       server->grab_box.x = new_x;
-                       server->grab_box.y = new_y;
-                       server->grab_box.width = view->natural_geometry.width;
-                       server->grab_box.height = view->natural_geometry.height;
-               }
+/* Returns true if view was snapped to any edge */
+static bool
+snap_to_edge(struct view *view)
+{
+       int snap_range = rc.snap_edge_range;
+       if (!snap_range) {
+               return false;
        }
 
-       /* Moving or resizing always resets tiled state */
-       view->tiled = 0;
+       /* Translate into output local coordinates */
+       double cursor_x = view->server->seat.cursor->x;
+       double cursor_y = view->server->seat.cursor->y;
+       wlr_output_layout_output_coords(view->server->output_layout,
+               view->output->wlr_output, &cursor_x, &cursor_y);
 
-       switch (mode) {
-       case LAB_INPUT_STATE_MOVE:
-               cursor_set(&server->seat, LAB_CURSOR_GRAB);
-               break;
-       case LAB_INPUT_STATE_RESIZE:
-               cursor_set(&server->seat, cursor_get_from_edge(edges));
-               break;
-       default:
-               break;
+       /*
+        * Don't store natural geometry here (it was
+        * stored already in interactive_begin())
+        */
+       struct wlr_box *area = &view->output->usable_area;
+       if (cursor_x <= area->x + snap_range) {
+               view_snap_to_edge(view, "left",
+                       /*store_natural_geometry*/ false);
+       } else if (cursor_x >= area->x + area->width - snap_range) {
+               view_snap_to_edge(view, "right",
+                       /*store_natural_geometry*/ false);
+       } else if (cursor_y <= area->y + snap_range) {
+               if (rc.snap_top_maximize) {
+                       view_maximize(view, true,
+                               /*store_natural_geometry*/ false);
+               } else {
+                       view_snap_to_edge(view, "up",
+                               /*store_natural_geometry*/ false);
+               }
+       } else if (cursor_y >= area->y + area->height - snap_range) {
+               view_snap_to_edge(view, "down",
+                       /*store_natural_geometry*/ false);
+       } else {
+               /* Not close to any edge */
+               return false;
        }
+
+       return true;
 }
 
 void
-interactive_end(struct view *view)
+interactive_finish(struct view *view)
 {
        if (view->server->grabbed_view == view) {
-               bool should_snap = view->server->input_mode == LAB_INPUT_STATE_MOVE
-                        && rc.snap_edge_range;
+               enum input_mode mode = view->server->input_mode;
                view->server->input_mode = LAB_INPUT_STATE_PASSTHROUGH;
                view->server->grabbed_view = NULL;
-               if (should_snap) {
-                       int snap_range = rc.snap_edge_range;
-                       struct wlr_box *area = &view->output->usable_area;
-
-                       /* Translate into output local coordinates */
-                       double cursor_x = view->server->seat.cursor->x;
-                       double cursor_y = view->server->seat.cursor->y;
-                       wlr_output_layout_output_coords(view->server->output_layout,
-                               view->output->wlr_output, &cursor_x, &cursor_y);
-
-                       if (cursor_x <= area->x + snap_range) {
-                               view_snap_to_edge(view, "left");
-                       } else if (cursor_x >= area->x + area->width - snap_range) {
-                               view_snap_to_edge(view, "right");
-                       } else if (cursor_y <= area->y + snap_range) {
-                               if (rc.snap_top_maximize) {
-                                       view_maximize(view, true);
-                                       /*
-                                        * When unmaximizing later on restore
-                                        * original position
-                                        */
-                                       view->natural_geometry.x =
-                                               view->server->grab_box.x;
-                                       view->natural_geometry.y =
-                                               view->server->grab_box.y;
-                               } else {
-                                       view_snap_to_edge(view, "up");
-                               }
-                       } else if (cursor_y >= area->y + area->height - snap_range) {
-                               view_snap_to_edge(view, "down");
+               if (mode == LAB_INPUT_STATE_MOVE) {
+                       if (!snap_to_edge(view)) {
+                               /* Reset tiled state if not snapped */
+                               view->tiled = 0;
                        }
                }
                /* Update focus/cursor image */
                cursor_update_focus(view->server);
        }
 }
+
+/*
+ * Cancels interative move/resize without changing the state of the of
+ * the view in any way. This may leave the tiled state inconsistent with
+ * the actual geometry of the view.
+ */
+void
+interactive_cancel(struct view *view)
+{
+       if (view->server->grabbed_view == view) {
+               view->server->input_mode = LAB_INPUT_STATE_PASSTHROUGH;
+               view->server->grabbed_view = NULL;
+               /* Update focus/cursor image */
+               cursor_update_focus(view->server);
+       }
+}

diff --git a/src/view.c b/src/view.c
index 0f3ef3c645d98f3818b40d0d362560bee4aa52b2..61408b0ee81327cb31e8c696bebf66f9005a56dc 100644 (file)
--- a/src/view.c
+++ b/src/view.c
@@ -308,7 +308,7 @@ set_fallback_geometry(struct view *view)
 #undef LAB_FALLBACK_WIDTH
 #undef LAB_FALLBACK_HEIGHT
 
-static void
+void
 view_store_natural_geometry(struct view *view)
 {
        /**
@@ -423,8 +423,37 @@ view_apply_unmaximized_geometry(struct view *view)
        }
 }
 
+static void
+set_maximized(struct view *view, bool maximized)
+{
+       if (view->impl->maximize) {
+               view->impl->maximize(view, maximized);
+       }
+       if (view->toplevel_handle) {
+               wlr_foreign_toplevel_handle_v1_set_maximized(
+                       view->toplevel_handle, maximized);
+       }
+       view->maximized = maximized;
+}
+
+/*
+ * Un-maximize view and move it to specific geometry. Does not reset
+ * tiled state (set view->tiled = 0 manually if you want that).
+ */
 void
-view_maximize(struct view *view, bool maximize)
+view_restore_to(struct view *view, struct wlr_box geometry)
+{
+       if (view->fullscreen) {
+               return;
+       }
+       if (view->maximized) {
+               set_maximized(view, false);
+       }
+       view_move_resize(view, geometry);
+}
+
+void
+view_maximize(struct view *view, bool maximize, bool store_natural_geometry)
 {
        if (view->maximized == maximize) {
                return;
@@ -432,20 +461,18 @@ view_maximize(struct view *view, bool maximize)
        if (view->fullscreen) {
                return;
        }
-       if (view->impl->maximize) {
-               view->impl->maximize(view, maximize);
-       }
-       if (view->toplevel_handle) {
-               wlr_foreign_toplevel_handle_v1_set_maximized(
-                       view->toplevel_handle, maximize);
-       }
+       set_maximized(view, maximize);
        if (maximize) {
-               interactive_end(view);
-               if (!view->tiled) {
+               /*
+                * Maximize via keybind or client request cancels
+                * interactive move/resize since we can't move/resize
+                * a maximized view.
+                */
+               interactive_cancel(view);
+               if (!view->tiled && store_natural_geometry) {
                        view_store_natural_geometry(view);
                }
                view_apply_maximized_geometry(view);
-               view->maximized = true;
        } else {
                /* unmaximize */
                if (view->tiled) {
@@ -453,14 +480,14 @@ view_maximize(struct view *view, bool maximize)
                } else {
                        view_apply_unmaximized_geometry(view);
                }
-               view->maximized = false;
        }
 }
 
 void
 view_toggle_maximize(struct view *view)
 {
-       view_maximize(view, !view->maximized);
+       view_maximize(view, !view->maximized,
+               /*store_natural_geometry*/ true);
 }
 
 void
@@ -526,7 +553,12 @@ view_set_fullscreen(struct view *view, bool fullscreen,
                wlr_output = view_wlr_output(view);
        }
        if (fullscreen) {
-               interactive_end(view);
+               /*
+                * Fullscreen via keybind or client request cancels
+                * interactive move/resize since we can't move/resize
+                * a fullscreen view.
+                */
+               interactive_cancel(view);
                if (!view->maximized && !view->tiled) {
                        view_store_natural_geometry(view);
                }
@@ -697,7 +729,8 @@ view_edge_parse(const char *direction)
 }
 
 void
-view_snap_to_edge(struct view *view, const char *direction)
+view_snap_to_edge(struct view *view, const char *direction,
+               bool store_natural_geometry)
 {
        if (!view) {
                wlr_log(WLR_ERROR, "no view");
@@ -754,8 +787,8 @@ view_snap_to_edge(struct view *view, const char *direction)
 
        if (view->maximized) {
                /* Unmaximize + keep using existing natural_geometry */
-               view_maximize(view, false);
-       } else if (!view->tiled) {
+               view_maximize(view, false, /*store_natural_geometry*/ false);
+       } else if (!view->tiled && store_natural_geometry) {
                /* store current geometry as new natural_geometry */
                view_store_natural_geometry(view);
        }

diff --git a/src/xdg.c b/src/xdg.c
index 1d8815c3d1664d2d4fd6051dc6f33890569fea26..f3e28d3ee44e3e0ed3302be68ab2bfb2a1e25e86 100644 (file)
--- a/src/xdg.c
+++ b/src/xdg.c
@@ -144,7 +144,8 @@ static void
 handle_request_maximize(struct wl_listener *listener, void *data)
 {
        struct view *view = wl_container_of(listener, view, request_maximize);
-       view_maximize(view, view->xdg_surface->toplevel->requested.maximized);
+       view_maximize(view, view->xdg_surface->toplevel->requested.maximized,
+               /*store_natural_geometry*/ true);
 }
 
 static void
@@ -310,7 +311,8 @@ xdg_toplevel_view_map(struct view *view)
                        view_set_fullscreen(view, true,
                                requested->fullscreen_output);
                } else if (!view->maximized && requested->maximized) {
-                       view_maximize(view, true);
+                       view_maximize(view, true,
+                               /*store_natural_geometry*/ true);
                }
 
                view_moved(view);