View: rework configure abstraction

- Move the decision whether a configure should be tracked or not into
the xdg toplevel/xwayland code.

- Only track configures for xdg toplevels with the transaction system
if the dimensions of the view are affected.

4 files changed, 110 insertions, 105 deletions

diff --git a/river/Root.zig b/river/Root.zig
index 7514217..49aa9db 100644
--- a/river/Root.zig
+++ b/river/Root.zig
@@ -497,15 +497,10 @@ fn sendConfigures(root: *Self) void {
             // This can happen if a view is unmapped while a layout demand including it is inflight
             if (!view.mapped) continue;
 
-            if (view.needsConfigure()) {
-                view.configure();
-
-                // We don't give a damn about frame perfection for xwayland views
-                if (!build_options.xwayland or view.impl != .xwayland_view) {
-                    root.inflight_configures += 1;
-                    view.saveSurfaceTree();
-                    view.sendFrameDone();
-                }
+            if (view.configure()) {
+                root.inflight_configures += 1;
+                view.saveSurfaceTree();
+                view.sendFrameDone();
             }
         }
     }
@@ -583,8 +578,6 @@ fn commitTransaction(root: *Self) void {
         while (focus_stack_it.next()) |view| {
             assert(view.inflight.output == output);
 
-            view.inflight_serial = null;
-
             if (view.current.output != view.inflight.output or
                 (output.current.fullscreen == view and output.inflight.fullscreen != view))
             {
diff --git a/river/View.zig b/river/View.zig
index 967fadb..3c783a7 100644
--- a/river/View.zig
+++ b/river/View.zig
@@ -139,9 +139,6 @@ inflight_wm_stack_link: wl.list.Link,
 
 current: State = .{},
 
-/// The serial sent with the currently inflight configure event
-inflight_serial: ?u32 = null,
-
 /// The floating dimensions the view, saved so that they can be restored if the
 /// view returns to floating mode.
 float_box: wlr.Box = undefined,
@@ -224,6 +221,9 @@ pub fn updateCurrent(view: *Self) void {
 
     view.current = view.inflight;
     view.dropSavedSurfaceTree();
+    if (view.impl == .xdg_toplevel) {
+        view.impl.xdg_toplevel.configure_state = .idle;
+    }
 
     const color = blk: {
         if (view.current.urgent) break :blk &config.border_color_urgent;
@@ -259,22 +259,16 @@ pub fn updateCurrent(view: *Self) void {
     view.borders.bottom.setColor(color);
 }
 
-pub fn needsConfigure(self: Self) bool {
-    assert(self.mapped);
-    return switch (self.impl) {
-        .xdg_toplevel => |xdg_toplevel| xdg_toplevel.needsConfigure(),
-        .xwayland_view => |xwayland_view| xwayland_view.needsConfigure(),
-    };
-}
-
-pub fn configure(self: *Self) void {
+/// Returns true if the configure should be waited for by the transaction system.
+pub fn configure(self: *Self) bool {
     assert(self.mapped and !self.destroying);
     switch (self.impl) {
-        .xdg_toplevel => |*xdg_toplevel| xdg_toplevel.configure(),
+        .xdg_toplevel => |*xdg_toplevel| return xdg_toplevel.configure(),
         .xwayland_view => |*xwayland_view| {
             // TODO(zig): remove this uneeded if statement
             // https://github.com/ziglang/zig/issues/13655
-            if (build_options.xwayland) xwayland_view.configure();
+            if (build_options.xwayland) return xwayland_view.configure();
+            unreachable;
         },
     }
 }
diff --git a/river/XdgToplevel.zig b/river/XdgToplevel.zig
index 9aa68bd..64229ad 100644
--- a/river/XdgToplevel.zig
+++ b/river/XdgToplevel.zig
@@ -17,6 +17,7 @@
 const Self = @This();
 
 const std = @import("std");
+const assert = std.debug.assert;
 const math = std.math;
 const wlr = @import("wlroots");
 const wl = @import("wayland").server.wl;
@@ -39,8 +40,14 @@ xdg_toplevel: *wlr.XdgToplevel,
 /// Initialized on map
 geometry: wlr.Box = undefined,
 
-/// Set to true when the client acks the configure with serial View.inflight_serial.
-acked_inflight_serial: bool = false,
+configure_state: union(enum) {
+    /// No configure has been sent since the last configure was acked.
+    idle,
+    /// A configure was sent with the given serial but has not yet been acked.
+    inflight: u32,
+    /// A configure was acked but the surface has not yet been committed.
+    acked,
+} = .idle,
 
 // Listeners that are always active over the view's lifetime
 destroy: wl.Listener(void) = wl.Listener(void).init(handleDestroy),
@@ -83,43 +90,52 @@ pub fn create(xdg_toplevel: *wlr.XdgToplevel) error{OutOfMemory}!void {
     _ = xdg_toplevel.setWmCapabilities(.{ .fullscreen = true });
 }
 
-/// Returns true if a configure must be sent to ensure that the inflight
-/// dimensions are applied.
-pub fn needsConfigure(self: Self) bool {
-    const view = self.view;
+/// Send a configure event, applying the inflight state of the view.
+pub fn configure(self: *Self) bool {
+    assert(self.configure_state == .idle);
+
+    const inflight = &self.view.inflight;
+    const current = &self.view.current;
 
     // We avoid a special case for newly mapped views which we have not yet
     // configured by setting the current width/height to the initial width/height
     // of the view in handleMap().
-    return view.inflight.box.width != view.current.box.width or
-        view.inflight.box.height != view.current.box.height or
-        (view.inflight.focus != 0) != (view.current.focus != 0) or
-        (view.inflight.output != null and view.inflight.output.?.inflight.fullscreen == view) !=
-        (view.current.output != null and view.current.output.?.current.fullscreen == view) or
-        view.inflight.borders != view.current.borders or
-        view.inflight.resizing != view.current.resizing;
-}
-
-/// Send a configure event, applying the inflight state of the view.
-pub fn configure(self: *Self) void {
-    const state = &self.view.inflight;
-
-    self.view.inflight_serial = self.xdg_toplevel.setSize(state.box.width, state.box.height);
+    if (inflight.box.width == current.box.width and
+        inflight.box.height == current.box.height and
+        (inflight.focus != 0) == (current.focus != 0) and
+        (inflight.output != null and inflight.output.?.inflight.fullscreen == self.view) ==
+        (current.output != null and current.output.?.current.fullscreen == self.view) and
+        inflight.borders == current.borders and
+        inflight.resizing == current.resizing)
+    {
+        return false;
+    }
 
-    _ = self.xdg_toplevel.setActivated(state.focus != 0);
+    _ = self.xdg_toplevel.setActivated(inflight.focus != 0);
 
-    const fullscreen = state.output != null and state.output.?.inflight.fullscreen == self.view;
+    const fullscreen = inflight.output != null and inflight.output.?.inflight.fullscreen == self.view;
     _ = self.xdg_toplevel.setFullscreen(fullscreen);
 
-    if (state.borders) {
+    if (inflight.borders) {
         _ = self.xdg_toplevel.setTiled(.{ .top = true, .bottom = true, .left = true, .right = true });
     } else {
         _ = self.xdg_toplevel.setTiled(.{ .top = false, .bottom = false, .left = false, .right = false });
     }
 
-    _ = self.xdg_toplevel.setResizing(state.resizing);
+    _ = self.xdg_toplevel.setResizing(inflight.resizing);
 
-    self.acked_inflight_serial = false;
+    // Only track configures with the transaction system if they affect the dimensions of the view.
+    if (inflight.box.width == current.box.width and
+        inflight.box.height == current.box.height)
+    {
+        return false;
+    }
+
+    self.configure_state = .{
+        .inflight = self.xdg_toplevel.setSize(inflight.box.width, inflight.box.height),
+    };
+
+    return true;
 }
 
 pub fn rootSurface(self: Self) *wlr.Surface {
@@ -240,10 +256,11 @@ fn handleAckConfigure(
     acked_configure: *wlr.XdgSurface.Configure,
 ) void {
     const self = @fieldParentPtr(Self, "ack_configure", listener);
-    if (self.view.inflight_serial) |serial| {
-        if (serial == acked_configure.serial) {
-            self.acked_inflight_serial = true;
-        }
+    switch (self.configure_state) {
+        .inflight => |serial| if (acked_configure.serial == serial) {
+            self.configure_state = .acked;
+        },
+        .acked, .idle => {},
     }
 }
 
@@ -263,35 +280,35 @@ fn handleCommit(listener: *wl.Listener(*wlr.Surface), _: *wlr.Surface) void {
 
     const old_geometry = self.geometry;
     self.xdg_toplevel.base.getGeometry(&self.geometry);
-    const size_changed = self.geometry.width != old_geometry.width or
-        self.geometry.height != old_geometry.height;
 
-    if (view.inflight_serial != null) {
-        if (self.acked_inflight_serial) {
-            view.inflight_serial = null;
+    switch (self.configure_state) {
+        .idle => {
+            const size_changed = self.geometry.width != old_geometry.width or
+                self.geometry.height != old_geometry.height;
+            const no_layout = view.current.output != null and view.current.output.?.layout == null;
+
+            if (size_changed and (view.current.float or no_layout) and !view.current.fullscreen) {
+                log.info(
+                    "client initiated size change: {}x{} -> {}x{}",
+                    .{ old_geometry.width, old_geometry.height, self.geometry.width, self.geometry.height },
+                );
+
+                view.current.box.width = self.geometry.width;
+                view.current.box.height = self.geometry.height;
+                view.pending.box.width = self.geometry.width;
+                view.pending.box.height = self.geometry.height;
+                server.root.applyPending();
+            }
+        },
+        // If the client has not yet acked our configure, we need to send a
+        // frame done event so that it commits another buffer. These
+        // buffers won't be rendered since we are still rendering our
+        // stashed buffer from when the transaction started.
+        .inflight => view.sendFrameDone(),
+        .acked => {
+            self.configure_state = .idle;
             server.root.notifyConfigured();
-        } else {
-            // If the client has not yet acked our configure, we need to send a
-            // frame done event so that it commits another buffer. These
-            // buffers won't be rendered since we are still rendering our
-            // stashed buffer from when the transaction started.
-            view.sendFrameDone();
-        }
-    } else if (size_changed and !view.current.fullscreen and
-        (view.current.float or view.current.output == null or view.current.output.?.layout == null))
-    {
-        log.info(
-            "client initiated size change: {}x{} -> {}x{}",
-            .{ old_geometry.width, old_geometry.height, self.geometry.width, self.geometry.height },
-        );
-
-        // If the client has decided to resize itself and the view is floating,
-        // then respect that resize.
-        view.current.box.width = self.geometry.width;
-        view.current.box.height = self.geometry.height;
-        view.pending.box.width = self.geometry.width;
-        view.pending.box.height = self.geometry.height;
-        server.root.applyPending();
+        },
     }
 }
 
diff --git a/river/XwaylandView.zig b/river/XwaylandView.zig
index 4befec0..b715abd 100644
--- a/river/XwaylandView.zig
+++ b/river/XwaylandView.zig
@@ -79,38 +79,39 @@ pub fn create(xwayland_surface: *wlr.XwaylandSurface) error{OutOfMemory}!void {
     }
 }
 
-pub fn needsConfigure(self: Self) bool {
+/// Always returns false as we do not care about frame perfection for Xwayland views.
+pub fn configure(self: Self) bool {
     const output = self.view.inflight.output orelse return false;
-    var output_box: wlr.Box = undefined;
-    server.root.output_layout.getBox(output.wlr_output, &output_box);
-
-    const view = self.view;
-    return self.xwayland_surface.x != view.inflight.box.x + output_box.x or
-        self.xwayland_surface.y != view.inflight.box.y + output_box.y or
-        self.xwayland_surface.width != view.inflight.box.width or
-        self.xwayland_surface.height != view.inflight.box.height or
-        (view.inflight.focus != 0) != (view.current.focus != 0) or
-        (view.inflight.output != null and view.inflight.output.?.inflight.fullscreen == view) !=
-        (view.current.output != null and view.current.output.?.current.fullscreen == view);
-}
 
-pub fn configure(self: Self) void {
-    const output = self.view.inflight.output orelse return;
     var output_box: wlr.Box = undefined;
     server.root.output_layout.getBox(output.wlr_output, &output_box);
 
-    const state = &self.view.inflight;
+    const inflight = &self.view.inflight;
+    const current = &self.view.current;
+
+    if (self.xwayland_surface.x == inflight.box.x + output_box.x and
+        self.xwayland_surface.y == inflight.box.y + output_box.y and
+        self.xwayland_surface.width == inflight.box.width and
+        self.xwayland_surface.height == inflight.box.height and
+        (inflight.focus != 0) == (current.focus != 0) and
+        (output.inflight.fullscreen == self.view) ==
+        (current.output != null and current.output.?.current.fullscreen == self.view))
+    {
+        return false;
+    }
+
     self.xwayland_surface.configure(
-        @intCast(i16, state.box.x + output_box.x),
-        @intCast(i16, state.box.y + output_box.y),
-        @intCast(u16, state.box.width),
-        @intCast(u16, state.box.height),
+        @intCast(i16, inflight.box.x + output_box.x),
+        @intCast(i16, inflight.box.y + output_box.y),
+        @intCast(u16, inflight.box.width),
+        @intCast(u16, inflight.box.height),
     );
 
-    self.setActivated(state.focus != 0);
+    self.setActivated(inflight.focus != 0);
+
+    self.xwayland_surface.setFullscreen(output.inflight.fullscreen == self.view);
 
-    const fullscreen = state.output != null and state.output.?.inflight.fullscreen == self.view;
-    self.xwayland_surface.setFullscreen(fullscreen);
+    return false;
 }
 
 pub fn rootSurface(self: Self) *wlr.Surface {
@@ -237,7 +238,7 @@ fn handleRequestConfigure(
         self.view.pending.box.width = event.width;
         self.view.pending.box.height = event.height;
     }
-    self.configure();
+    server.root.applyPending();
 }
 
 fn handleSetOverrideRedirect(