XdgToplevel: handle configure timeout gracefully

Currently configure timeouts hit the "client is buggy and initiated size
change while tiled or fullscreen" code path even if the client is not in
fact buggy. This causes state to get out of sync between river and the
client, and is highly visible as the borders drawn by river no longer
align with the buffer dimensions committed by the client.

This commit fixes this by tracking acks/commits in response to
configures even after a timeout and properly integrating them with the
transaction system.

river/Root.zig

@@ -620,7 +620,7 @@ fn commitTransaction(root: *Self) void {
             view.tree.node.reparent(root.hidden.tree);
             view.popup_tree.node.reparent(root.hidden.tree);
 
-            view.updateCurrent();
+            view.commitTransaction();
         }
     }
 
@@ -657,7 +657,7 @@ fn commitTransaction(root: *Self) void {
                 }
             }
 
-            view.updateCurrent();
+            view.commitTransaction();
 
             const enabled = view.current.tags & output.current.tags != 0;
             view.tree.node.setEnabled(enabled);

river/View.zig

@@ -272,26 +272,32 @@ pub fn resizeUpdatePosition(view: *Self, width: i32, height: i32) void {
     }
 }
 
-pub fn updateCurrent(view: *Self) void {
-    const config = &server.config;
+pub fn commitTransaction(view: *Self) void {
+    view.foreign_toplevel_handle.update();
+
+    // Tag and output changes must be applied immediately even if the configure sequence times out.
+    // This allows Root.commitTransaction() to rely on the fact that all tag and output changes
+    // are applied directly by this function.
+    view.current.tags = view.inflight.tags;
+    view.current.output = view.inflight.output;
+
+    view.dropSavedSurfaceTree();
 
     switch (view.impl) {
         .xdg_toplevel => |*xdg_toplevel| {
             switch (xdg_toplevel.configure_state) {
-                // If the configure timed out, don't update current to dimensions
-                // that have not been committed by the client.
-                .inflight, .acked => {
-                    if (view.inflight.resizing) {
-                        view.resizeUpdatePosition(view.current.box.width, view.current.box.height);
-                    }
-                    view.inflight.box.width = view.current.box.width;
-                    view.inflight.box.height = view.current.box.height;
-                    view.pending.box.width = view.current.box.width;
-                    view.pending.box.height = view.current.box.height;
+                .inflight => |serial| {
+                    xdg_toplevel.configure_state = .{ .timed_out = serial };
                 },
-                .idle, .committed => {},
+                .acked => {
+                    xdg_toplevel.configure_state = .timed_out_acked;
+                },
+                .idle, .committed => {
+                    xdg_toplevel.configure_state = .idle;
+                    view.updateCurrent();
+                },
+                .timed_out, .timed_out_acked => unreachable,
             }
-            xdg_toplevel.configure_state = .idle;
         },
         .xwayland_view => |xwayland_view| {
             if (view.inflight.resizing) {
@@ -300,18 +306,24 @@ pub fn updateCurrent(view: *Self) void {
                     xwayland_view.xwayland_surface.height,
                 );
             }
+
             view.inflight.box.width = xwayland_view.xwayland_surface.width;
             view.inflight.box.height = xwayland_view.xwayland_surface.height;
             view.pending.box.width = xwayland_view.xwayland_surface.width;
             view.pending.box.height = xwayland_view.xwayland_surface.height;
+
+            view.updateCurrent();
         },
         .none => {},
     }
+}
 
-    view.foreign_toplevel_handle.update();
+pub fn updateCurrent(view: *Self) void {
+    // Applied already in View.commitTransaction()
+    assert(view.current.tags == view.inflight.tags);
+    assert(view.current.output == view.inflight.output);
 
     view.current = view.inflight;
-    view.dropSavedSurfaceTree();
 
     const box = &view.current.box;
     view.tree.node.setPosition(box.x, box.y);
@@ -344,6 +356,7 @@ pub fn updateCurrent(view: *Self) void {
     }
 
     {
+        const config = &server.config;
         const border_width: c_int = config.border_width;
         const border_color = blk: {
             if (view.current.urgent) break :blk &config.border_color_urgent;

river/XdgToplevel.zig

@@ -52,6 +52,10 @@ configure_state: union(enum) {
     acked,
     /// A configure was acked and the surface was committed.
     committed,
+    /// A configure was sent but not acked before the transaction timed out.
+    timed_out: u32,
+    /// A configure was sent and acked but not committed before the transaction timed out.
+    timed_out_acked,
 } = .idle,
 
 // Listeners that are always active over the view's lifetime
@@ -109,7 +113,10 @@ pub fn create(xdg_toplevel: *wlr.XdgToplevel) error{OutOfMemory}!void {
 
 /// Send a configure event, applying the inflight state of the view.
 pub fn configure(self: *Self) bool {
-    assert(self.configure_state == .idle);
+    switch (self.configure_state) {
+        .idle, .timed_out, .timed_out_acked => {},
+        .inflight, .acked, .committed => unreachable,
+    }
 
     const inflight = &self.view.inflight;
     const current = &self.view.current;
@@ -289,7 +296,10 @@ fn handleAckConfigure(
         .inflight => |serial| if (acked_configure.serial == serial) {
             self.configure_state = .acked;
         },
-        .acked, .idle, .committed => {},
+        .timed_out => |serial| if (acked_configure.serial == serial) {
+            self.configure_state = .timed_out_acked;
+        },
+        .acked, .idle, .committed, .timed_out_acked => {},
     }
 }
 
@@ -311,7 +321,7 @@ fn handleCommit(listener: *wl.Listener(*wlr.Surface), _: *wlr.Surface) void {
     self.xdg_toplevel.base.getGeometry(&self.geometry);
 
     switch (self.configure_state) {
-        .idle, .committed => {
+        .idle, .committed, .timed_out => {
             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;
@@ -337,9 +347,7 @@ fn handleCommit(listener: *wl.Listener(*wlr.Surface), _: *wlr.Surface) void {
         // 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 = .committed;
-
+        inline .acked, .timed_out_acked => |_, tag| {
             if (view.inflight.resizing) {
                 view.resizeUpdatePosition(self.geometry.width, self.geometry.height);
             }
@@ -349,7 +357,17 @@ fn handleCommit(listener: *wl.Listener(*wlr.Surface), _: *wlr.Surface) void {
             view.pending.box.width = self.geometry.width;
             view.pending.box.height = self.geometry.height;
 
-            server.root.notifyConfigured();
+            switch (tag) {
+                .acked => {
+                    self.configure_state = .committed;
+                    server.root.notifyConfigured();
+                },
+                .timed_out_acked => {
+                    self.configure_state = .idle;
+                    view.updateCurrent();
+                },
+                else => unreachable,
+            }
         },
     }
 }