view: combine natural_geometry and last_layout_geometry

The logic around last_layout_geometry seems unnecessarily complicated.
As far as I can see, it's basically just a copy of the natural_geometry
with some special semantics, specifically:

1. it's copied *from* the natural geometry (or equivalently, the
   pending geometry if floating) at the first layout change after a
   user-initiated move or resize, and

2. it's copied *back to* the natural geometry at subsequent layout
   changes and used as the "original" position to restore.

I don't see the point of the copying back and forth. Unless I'm missing
something, we can achieve the same semantics in a lot less code by just
updating/using the natural_geometry itself at the appropriate times.
The only additional data needed is a single bool to track whether a
layout change has occurred since the last user-initiated move/resize.

No functional change intended, but I may be missing some subtleties as
I found the existing code (and comments) quite difficult to follow.

include/view.h

@@ -209,17 +209,16 @@ struct view {
 	/*
 	 * Saved geometry which will be restored when the view returns
 	 * to normal/floating state after being maximized/fullscreen/
-	 * tiled. Values are undefined/out-of-date when the view is not
+	 * tiled. The natural geometry is also saved prior to relocating
-	 * maximized/fullscreen/tiled.
+	 * the view due to an output layout change, so that the view can
+	 * be restored to its original location later.
 	 */
 	struct wlr_box natural_geometry;
 	/*
-	 * Whenever an output layout change triggers a view relocation, the
+	 * True if the most recent move/resize of the view was due to a
-	 * last pending position (or natural geometry) will be saved so the
+	 * layout change. False if it was due to user-initiated action.
-	 * view may be restored to its original location on a subsequent layout
-	 * change.
 	 */
-	struct wlr_box last_layout_geometry;
+	bool adjusted_for_layout_change;
 
 	/* used by xdg-shell views */
 	uint32_t pending_configure_serial;
@@ -533,7 +532,6 @@ bool view_titlebar_visible(struct view *view);
 void view_set_ssd_mode(struct view *view, enum lab_ssd_mode mode);
 void view_set_decorations(struct view *view, enum lab_ssd_mode mode, bool force_ssd);
 void view_toggle_fullscreen(struct view *view);
-void view_invalidate_last_layout_geometry(struct view *view);
 void view_adjust_for_layout_change(struct view *view);
 void view_move_to_edge(struct view *view, enum lab_edge direction, bool snap_to_windows);
 void view_grow_to_edge(struct view *view, enum lab_edge direction);

src/interactive.c

@@ -92,9 +92,6 @@ interactive_begin(struct view *view, enum input_mode mode, enum lab_edge edges)
 
 		/* Store natural geometry at start of move */
 		view_store_natural_geometry(view);
-		if (view_is_floating(view)) {
-			view_invalidate_last_layout_geometry(view);
-		}
 
 		/* Prevent region snapping when just moving via A-Left mousebind */
 		seat->region_prevent_snap = keyboard_get_all_modifiers(seat);
@@ -111,12 +108,6 @@ interactive_begin(struct view *view, enum input_mode mode, enum lab_edge edges)
 			return;
 		}
 
-		/*
-		 * Resizing overrides any attempt to restore window
-		 * geometries altered by layout changes.
-		 */
-		view_invalidate_last_layout_geometry(view);
-
 		/*
 		 * If tiled or maximized in only one direction, reset
 		 * tiled state and un-maximize the relevant axes, but

src/view.c

@@ -588,6 +588,13 @@ view_move_resize(struct view *view, struct wlr_box geo)
 	if (view->impl->configure) {
 		view->impl->configure(view, geo);
 	}
+	/*
+	 * Assume by default that the move/resize was user-initiated
+	 * (rather than due to output layout change) and clear the flag.
+	 * For the few cases where it is due to layout change, the flag
+	 * is set to true afterward by view_adjust_for_layout_change().
+	 */
+	view->adjusted_for_layout_change = false;
 }
 
 void
@@ -1405,9 +1412,6 @@ view_maximize(struct view *view, enum view_axis axis)
 		 * a maximized view.
 		 */
 		interactive_cancel(view);
-		if (store_natural_geometry && view_is_floating(view)) {
-			view_invalidate_last_layout_geometry(view);
-		}
 	}
 
 	/*
@@ -1695,7 +1699,6 @@ view_set_fullscreen(struct view *view, bool fullscreen)
 		 */
 		interactive_cancel(view);
 		view_store_natural_geometry(view);
-		view_invalidate_last_layout_geometry(view);
 	}
 
 	set_fullscreen(view, fullscreen);
@@ -1713,134 +1716,53 @@ view_set_fullscreen(struct view *view, bool fullscreen)
 	cursor_update_focus(view->server);
 }
 
-static bool
-last_layout_geometry_is_valid(struct view *view)
-{
-	return view->last_layout_geometry.width > 0
-		&& view->last_layout_geometry.height > 0;
-}
-
-static void
-update_last_layout_geometry(struct view *view)
-{
-	/*
-	 * Only update an invalid last-layout geometry to prevent a series of
-	 * successive layout changes from continually replacing the "preferred"
-	 * location with whatever location the view currently holds. The
-	 * "preferred" location should be whatever state was set by user
-	 * interaction, not automatic responses to layout changes.
-	 */
-	if (last_layout_geometry_is_valid(view)) {
-		return;
-	}
-
-	if (view_is_floating(view)) {
-		view->last_layout_geometry = view->pending;
-	} else {
-		view->last_layout_geometry = view->natural_geometry;
-	}
-}
-
-static bool
-apply_last_layout_geometry(struct view *view, bool force_update)
-{
-	/* Only apply a valid last-layout geometry */
-	if (!last_layout_geometry_is_valid(view)) {
-		return false;
-	}
-
-	/*
-	 * Unless forced, the last-layout geometry is only applied
-	 * when the relevant view geometry is distinct.
-	 */
-	if (!force_update) {
-		struct wlr_box *relevant = view_is_floating(view) ?
-			&view->pending : &view->natural_geometry;
-
-		if (wlr_box_equal(relevant, &view->last_layout_geometry)) {
-			return false;
-		}
-	}
-
-	view->natural_geometry = view->last_layout_geometry;
-	adjust_floating_geometry(view, &view->natural_geometry,
-		/* midpoint_visibility */ true);
-	return true;
-}
-
-void
-view_invalidate_last_layout_geometry(struct view *view)
-{
-	assert(view);
-	view->last_layout_geometry.width = 0;
-	view->last_layout_geometry.height = 0;
-}
-
 void
 view_adjust_for_layout_change(struct view *view)
 {
 	assert(view);
 
 	bool is_floating = view_is_floating(view);
-	bool use_natural = false;
+	bool adjusted = view->adjusted_for_layout_change;
-
-	if (!output_is_usable(view->output)) {
-		/* A view losing an output should have a last-layout geometry */
-		update_last_layout_geometry(view);
-	}
-
-	/* Capture a pointer to the last-layout geometry (only if valid) */
-	struct wlr_box *last_geometry = NULL;
-	if (last_layout_geometry_is_valid(view)) {
-		last_geometry = &view->last_layout_geometry;
-	}
 
 	/*
 	 * Check if an output change is required:
 	 * - Floating views are always mapped to the nearest output
 	 * - Any view without a usable output needs to be repositioned
-	 * - Any view with a valid last-layout geometry might be better
+	 * - Any fullscreen/tiled/maximized view which was previously
-	 *   positioned on another output
+	 *   handled through this path (i.e. previously lost its output)
+	 *   is also checked again. For these, the logic is not quite
+	 *   right since the output is chosen based on the natural
+	 *   (floating) geometry, but it's better than nothing.
 	 */
-	if (is_floating || last_geometry || !output_is_usable(view->output)) {
+	if (is_floating || adjusted || !output_is_usable(view->output)) {
-		/* Move the view to an appropriate output, if needed */
-		bool output_changed = view_discover_output(view, last_geometry);
-
 		/*
-		 * Try to apply the last-layout to the natural geometry
+		 * Save natural geometry if this is the first layout
-		 * (adjusting to ensure that it fits on the screen). This is
+		 * change since a user-initiated move/resize. Do not
-		 * forced if the output has changed, but will be done
+		 * save it again for a subsequent layout change, since
-		 * opportunistically even on the same output if the last-layout
+		 * the point is to be able to restore to the original
-		 * geometry is different from the view's governing geometry.
+		 * location after multiple layout changes (e.g. output
+		 * disconnected and then reconnected).
 		 */
-		if (apply_last_layout_geometry(view, output_changed)) {
+		if (!adjusted) {
-			use_natural = true;
+			view_store_natural_geometry(view);
+			adjusted = true;
 		}
 
-		/*
+		view_discover_output(view, &view->natural_geometry);
-		 * Whether or not the view has moved, the layout has changed.
-		 * Ensure that the view now has a valid last-layout geometry.
-		 */
-		update_last_layout_geometry(view);
 	}
 
-	if (!is_floating) {
+	if (is_floating) {
-		view_apply_special_geometry(view);
-	} else if (use_natural) {
 		/*
 		 * Move the window to its natural location, because
 		 * we are trying to restore a prior layout.
 		 */
 		view_apply_natural_geometry(view);
 	} else {
-		/* Otherwise, just ensure the view is on screen. */
+		view_apply_special_geometry(view);
-		struct wlr_box geometry = view->pending;
-		if (adjust_floating_geometry(view, &geometry,
-					/* midpoint_visibility */ true)) {
-			view_move_resize(view, geometry);
-		}
 	}
 
+	/* Set/reset flag after view_move_resize(), which clears it */
+	view->adjusted_for_layout_change = adjusted;
 	view_update_outputs(view);
 }
 
@@ -2130,7 +2052,6 @@ view_snap_to_edge(struct view *view, enum lab_edge edge,
 	} else if (store_natural_geometry) {
 		/* store current geometry as new natural_geometry */
 		view_store_natural_geometry(view);
-		view_invalidate_last_layout_geometry(view);
 	}
 	view_set_untiled(view);
 	view_set_output(view, output);
@@ -2164,7 +2085,6 @@ view_snap_to_region(struct view *view, struct region *region)
 	} else if (store_natural_geometry) {
 		/* store current geometry as new natural_geometry */
 		view_store_natural_geometry(view);
-		view_invalidate_last_layout_geometry(view);
 	}
 	view_set_untiled(view);
 	view->tiled_region = region;
@@ -2177,7 +2097,6 @@ view_move_to_output(struct view *view, struct output *output)
 {
 	assert(view);
 
-	view_invalidate_last_layout_geometry(view);
 	view_set_output(view, output);
 	if (view_is_floating(view)) {
 		struct wlr_box output_area = output_usable_area_in_layout_coords(output);