apprt/gtk-ng: `goto_split` (including spatial navigation for the first time for our GTK backend) (#8210)

This continues #8202 by fixing two of the known issues: `goto_split` key
binds work and closing a split moves focus to the proper place.

A big improvement in this PR is that for the first time ever in our GTK
backend, the up/down/left/right `goto_split` bindings **use spatial
navigation.** "Spatial navigation" means that the direction to move
focus is done based on the nearest split _visually_ from the current
split, rather than via a tree traversal. We did this on macOS a couple
months ago, with a lot more details there: #7523

Similar to macOS, the spatial navigation is currently based on top-left
corner. Now that our split tree is implemented in Zig though it should
be a lot easier for us to work in the current cursor position as the
reference point.

~~🚧 TODO: Going to add some unit tests for the spatial navigation before
merge.~~

src/apprt/gtk-ng/class/application.zig

@@ -34,6 +34,7 @@ const Common = @import("../class.zig").Common;
 const WeakRef = @import("../weak_ref.zig").WeakRef;
 const Config = @import("config.zig").Config;
 const Surface = @import("surface.zig").Surface;
+const SplitTree = @import("split_tree.zig").SplitTree;
 const Window = @import("window.zig").Window;
 const CloseConfirmationDialog = @import("close_confirmation_dialog.zig").CloseConfirmationDialog;
 const ConfigErrorsDialog = @import("config_errors_dialog.zig").ConfigErrorsDialog;
@@ -552,6 +553,8 @@ pub const Application = extern struct {
 
             .desktop_notification => Action.desktopNotification(self, target, value),
 
+            .goto_split => return Action.gotoSplit(target, value),
+
             .goto_tab => return Action.gotoTab(target, value),
 
             .initial_size => return Action.initialSize(target, value),
@@ -615,7 +618,6 @@ pub const Application = extern struct {
             // TODO: splits
             .resize_split,
             .equalize_splits,
-            .goto_split,
             .toggle_split_zoom,
             => {
                 log.warn("unimplemented action={}", .{action});
@@ -1650,6 +1652,38 @@ const Action = struct {
         gio_app.sendNotification(n.body, notification);
     }
 
+    pub fn gotoSplit(
+        target: apprt.Target,
+        to: apprt.action.GotoSplit,
+    ) bool {
+        switch (target) {
+            .app => return false,
+            .surface => |core| {
+                // Design note: we can't use widget actions here because
+                // we need to know whether there is a goto target for returning
+                // the proper perform result (boolean).
+
+                const surface = core.rt_surface.surface;
+                const tree = ext.getAncestor(
+                    SplitTree,
+                    surface.as(gtk.Widget),
+                ) orelse {
+                    log.warn("surface is not in a split tree, ignoring goto_split", .{});
+                    return false;
+                };
+
+                return tree.goto(switch (to) {
+                    .previous => .previous_wrapped,
+                    .next => .next_wrapped,
+                    .up => .{ .spatial = .up },
+                    .down => .{ .spatial = .down },
+                    .left => .{ .spatial = .left },
+                    .right => .{ .spatial = .right },
+                });
+            },
+        }
+    }
+
     pub fn gotoTab(
         target: apprt.Target,
         tab: apprt.action.GotoTab,

src/apprt/gtk-ng/class/split_tree.zig

@@ -246,6 +246,7 @@ pub const SplitTree = extern struct {
             alloc,
             handle,
             direction,
+            0.5, // Always split equally for new splits
             &single_tree,
         );
         defer new_tree.deinit();
@@ -258,6 +259,34 @@ pub const SplitTree = extern struct {
         self.setTree(&new_tree);
     }
 
+    /// Move focus from the currently focused surface to the given
+    /// direction. Returns true if focus switched to a new surface.
+    pub fn goto(self: *Self, to: Surface.Tree.Goto) bool {
+        const tree = self.getTree() orelse return false;
+        const active = self.getActiveSurfaceHandle() orelse return false;
+        const target = if (tree.goto(
+            Application.default().allocator(),
+            active,
+            to,
+        )) |handle_|
+            handle_ orelse return false
+        else |err| switch (err) {
+            // Nothing we can do in this scenario. This is highly unlikely
+            // since split trees don't use that much memory. The application
+            // is probably about to crash in other ways.
+            error.OutOfMemory => return false,
+        };
+
+        // If we aren't changing targets then we did nothing.
+        if (active == target) return false;
+
+        // Get the surface at the target location and grab focus.
+        const surface = tree.nodes[target].leaf;
+        surface.grabFocus();
+
+        return true;
+    }
+
     fn disconnectSurfaceHandlers(self: *Self) void {
         const tree = self.getTree() orelse return;
         var it = tree.iterator();
@@ -572,8 +601,25 @@ pub const SplitTree = extern struct {
         const handle = priv.pending_close orelse return;
         priv.pending_close = null;
 
-        // Remove it from the tree.
+        // Figure out our next focus target. The next focus target is
+        // always the "previous" surface unless we're the leftmost then
+        // its the next.
         const old_tree = self.getTree() orelse return;
+        const next_focus: ?*Surface = next_focus: {
+            const alloc = Application.default().allocator();
+            const next_handle: Surface.Tree.Node.Handle =
+                (old_tree.goto(alloc, handle, .previous) catch null) orelse
+                (old_tree.goto(alloc, handle, .next) catch null) orelse
+                break :next_focus null;
+            if (next_handle == handle) break :next_focus null;
+
+            // Note: we don't need to ref this or anything because its
+            // guaranteed to remain in the new tree since its not part
+            // of the handle we're removing.
+            break :next_focus old_tree.nodes[next_handle].leaf;
+        };
+
+        // Remove it from the tree.
         var new_tree = old_tree.remove(
             Application.default().allocator(),
             handle,
@@ -583,6 +629,10 @@ pub const SplitTree = extern struct {
         };
         defer new_tree.deinit();
         self.setTree(&new_tree);
+
+        // Grab focus. We have to set this on the "last focused" because our
+        // focus will be set when the tree is redrawn.
+        if (next_focus) |v| priv.last_focused.set(v);
     }
 
     fn propSurfaceFocused(

src/datastruct/split_tree.zig

@@ -152,16 +152,16 @@ pub fn SplitTree(comptime V: type) type {
             return .{ .nodes = self.nodes };
         }
 
+        pub const ViewEntry = struct {
+            handle: Node.Handle,
+            view: *View,
+        };
+
         pub const Iterator = struct {
             i: Node.Handle = 0,
             nodes: []const Node,
 
-            pub const Entry = struct {
+            pub fn next(self: *Iterator) ?ViewEntry {
-                handle: Node.Handle,
-                view: *View,
-            };
-
-            pub fn next(self: *Iterator) ?Entry {
                 // If we have no nodes, return null.
                 if (self.i >= self.nodes.len) return null;
 
@@ -177,6 +177,214 @@ pub fn SplitTree(comptime V: type) type {
             }
         };
 
+        pub const Goto = union(enum) {
+            /// Previous view, null if we're the first view.
+            previous,
+
+            /// Next view, null if we're the last view.
+            next,
+
+            /// Previous view, but wrapped around to the last view. May
+            /// return the same view if this is the first view.
+            previous_wrapped,
+
+            /// Next view, but wrapped around to the first view. May return
+            /// the same view if this is the last view.
+            next_wrapped,
+
+            /// A spatial direction. "Spatial" means that the direction is
+            /// based on the nearest surface in the given direction visually
+            /// as the surfaces are laid out on a 2D grid.
+            spatial: Spatial.Direction,
+        };
+
+        /// Goto a view from a certain point in the split tree. Returns null
+        /// if the direction results in no visitable view.
+        ///
+        /// Allocator is only used for temporary state for spatial navigation.
+        pub fn goto(
+            self: *const Self,
+            alloc: Allocator,
+            from: Node.Handle,
+            to: Goto,
+        ) Allocator.Error!?Node.Handle {
+            return switch (to) {
+                .previous => self.previous(from),
+                .next => self.next(from),
+                .previous_wrapped => self.previous(from) orelse self.deepest(.right, 0),
+                .next_wrapped => self.next(from) orelse self.deepest(.left, 0),
+                .spatial => |d| spatial: {
+                    // Get our spatial representation.
+                    var sp = try self.spatial(alloc);
+                    defer sp.deinit(alloc);
+                    break :spatial self.nearest(sp, from, d);
+                },
+            };
+        }
+
+        pub const Side = enum { left, right };
+
+        /// Returns the deepest view in the tree in the given direction.
+        /// This can be used to find the leftmost/rightmost surface within
+        /// a given split structure.
+        pub fn deepest(
+            self: *const Self,
+            side: Side,
+            from: Node.Handle,
+        ) Node.Handle {
+            var current: Node.Handle = from;
+            while (true) {
+                switch (self.nodes[current]) {
+                    .leaf => return current,
+                    .split => |s| current = switch (side) {
+                        .left => s.left,
+                        .right => s.right,
+                    },
+                }
+            }
+        }
+
+        /// Returns the previous view from the given node handle (which itself
+        /// doesn't need to be a view). If there is no previous (this is the
+        /// most previous view) then this will return null.
+        ///
+        /// "Previous" is defined as the previous node in an in-order
+        /// traversal of the tree. This isn't a perfect definition and we
+        /// may want to change this to something that better matches a
+        /// spatial view of the tree later.
+        fn previous(self: *const Self, from: Node.Handle) ?Node.Handle {
+            return switch (self.previousBacktrack(from, 0)) {
+                .result => |v| v,
+                .backtrack, .deadend => null,
+            };
+        }
+
+        /// Same as `previous`, but returns the next view instead.
+        fn next(self: *const Self, from: Node.Handle) ?Node.Handle {
+            return switch (self.nextBacktrack(from, 0)) {
+                .result => |v| v,
+                .backtrack, .deadend => null,
+            };
+        }
+
+        // Design note: we use a recursive backtracking search because
+        // split trees are never that deep, so we can abuse the stack as
+        // a safe allocator (stack overflow unlikely unless the kernel is
+        // tuned in some really weird way).
+        const Backtrack = union(enum) {
+            deadend,
+            backtrack,
+            result: Node.Handle,
+        };
+
+        fn previousBacktrack(
+            self: *const Self,
+            from: Node.Handle,
+            current: Node.Handle,
+        ) Backtrack {
+            // If we reached the point that we're trying to find the previous
+            // value of, then we need to backtrack from here.
+            if (from == current) return .backtrack;
+
+            return switch (self.nodes[current]) {
+                // If we hit a leaf that isn't our target, then deadend.
+                .leaf => .deadend,
+
+                .split => |s| switch (self.previousBacktrack(from, s.left)) {
+                    .result => |v| .{ .result = v },
+
+                    // Backtrack from the left means we have to continue
+                    // backtracking because we can't see what's before the left.
+                    .backtrack => .backtrack,
+
+                    // If we hit a deadend on the left then let's move right.
+                    .deadend => switch (self.previousBacktrack(from, s.right)) {
+                        .result => |v| .{ .result = v },
+
+                        // Deadend means its not in this split at all since
+                        // we already tracked the left.
+                        .deadend => .deadend,
+
+                        // Backtrack means that its in our left view because
+                        // we can see the immediate previous and there MUST
+                        // be leaves (we can't have split-only leaves).
+                        .backtrack => .{ .result = self.deepest(.right, s.left) },
+                    },
+                },
+            };
+        }
+
+        // See previousBacktrack for detailed comments. This is a mirror
+        // of that.
+        fn nextBacktrack(
+            self: *const Self,
+            from: Node.Handle,
+            current: Node.Handle,
+        ) Backtrack {
+            if (from == current) return .backtrack;
+            return switch (self.nodes[current]) {
+                .leaf => .deadend,
+                .split => |s| switch (self.nextBacktrack(from, s.right)) {
+                    .result => |v| .{ .result = v },
+                    .backtrack => .backtrack,
+                    .deadend => switch (self.nextBacktrack(from, s.left)) {
+                        .result => |v| .{ .result = v },
+                        .deadend => .deadend,
+                        .backtrack => .{ .result = self.deepest(.left, s.right) },
+                    },
+                },
+            };
+        }
+
+        /// Returns the nearest leaf node (view) in the given direction.
+        fn nearest(
+            self: *const Self,
+            sp: Spatial,
+            from: Node.Handle,
+            direction: Spatial.Direction,
+        ) ?Node.Handle {
+            const target = sp.slots[from];
+
+            var result: ?struct {
+                handle: Node.Handle,
+                distance: f16,
+            } = null;
+            for (sp.slots, 0..) |slot, handle| {
+                // Never match ourself
+                if (handle == from) continue;
+
+                // Only match leaves
+                switch (self.nodes[handle]) {
+                    .leaf => {},
+                    .split => continue,
+                }
+
+                // Ensure it is in the proper direction
+                if (!switch (direction) {
+                    .left => slot.maxX() <= target.x,
+                    .right => slot.x >= target.maxX(),
+                    .up => slot.maxY() <= target.y,
+                    .down => slot.y >= target.maxY(),
+                }) continue;
+
+                // Track our distance
+                const dx = slot.x - target.x;
+                const dy = slot.y - target.y;
+                const distance = @sqrt(dx * dx + dy * dy);
+
+                // If we have a nearest it must be closer.
+                if (result) |n| {
+                    if (distance >= n.distance) continue;
+                }
+                result = .{
+                    .handle = @intCast(handle),
+                    .distance = distance,
+                };
+            }
+
+            return if (result) |n| n.handle else null;
+        }
+
         /// Resize the given node in place. The node MUST be a split (asserted).
         ///
         /// In general, this is an immutable data structure so this is
@@ -211,6 +419,7 @@ pub fn SplitTree(comptime V: type) type {
             gpa: Allocator,
             at: Node.Handle,
             direction: Split.Direction,
+            ratio: f16,
             insert: *const Self,
         ) Allocator.Error!Self {
             // The new arena for our new tree.
@@ -255,7 +464,7 @@ pub fn SplitTree(comptime V: type) type {
             nodes[nodes.len - 1] = nodes[at];
             nodes[at] = .{ .split = .{
                 .layout = layout,
-                .ratio = 0.5,
+                .ratio = ratio,
                 .left = @intCast(if (left) self.nodes.len else nodes.len - 1),
                 .right = @intCast(if (left) nodes.len - 1 else self.nodes.len),
             } };
@@ -441,14 +650,24 @@ pub fn SplitTree(comptime V: type) type {
 
             pub const empty: Spatial = .{ .slots = &.{} };
 
+            pub const Direction = enum { left, right, down, up };
+
             const Slot = struct {
                 x: f16,
                 y: f16,
                 width: f16,
                 height: f16,
+
+                fn maxX(self: *const Slot) f16 {
+                    return self.x + self.width;
+                }
+
+                fn maxY(self: *const Slot) f16 {
+                    return self.y + self.height;
+                }
             };
 
-            pub fn deinit(self: *const Spatial, alloc: Allocator) void {
+            pub fn deinit(self: *Spatial, alloc: Allocator) void {
                 alloc.free(self.slots);
                 self.* = undefined;
             }
@@ -779,6 +998,12 @@ pub fn SplitTree(comptime V: type) type {
 
             // Output every row
             for (grid) |row| {
+                // We currently have a bug in our height calculation that
+                // results in trailing blank lines. Ignore those. We should
+                // really fix our height calculation instead. If someone wants
+                // to do that just remove this line and see the tests that fail
+                // and go from there.
+                if (row[0] == ' ') break;
                 try writer.writeAll(row);
             }
         }
@@ -914,6 +1139,7 @@ test "SplitTree: split horizontal" {
         alloc,
         0, // at root
         .right, // split right
+        0.5,
         &t2, // insert t2
     );
     defer t3.deinit();
@@ -945,6 +1171,7 @@ test "SplitTree: split horizontal" {
             }
         } else return error.NotFound,
         .right,
+        0.5,
         &tC,
     );
     defer t4.deinit();
@@ -978,6 +1205,7 @@ test "SplitTree: split horizontal" {
             }
         } else return error.NotFound,
         .right,
+        0.5,
         &tD,
     );
     defer t5.deinit();
@@ -999,6 +1227,66 @@ test "SplitTree: split horizontal" {
             \\
         , str);
     }
+
+    // Find "previous" from D back.
+    {
+        var current: u8 = 'D';
+        while (current != 'A') : (current -= 1) {
+            it = t5.iterator();
+            const handle = t5.previous(
+                while (it.next()) |entry| {
+                    if (std.mem.eql(u8, entry.view.label, &.{current})) {
+                        break entry.handle;
+                    }
+                } else return error.NotFound,
+            ).?;
+
+            const entry = t5.nodes[handle].leaf;
+            try testing.expectEqualStrings(
+                entry.label,
+                &.{current - 1},
+            );
+        }
+
+        it = t5.iterator();
+        try testing.expect(t5.previous(
+            while (it.next()) |entry| {
+                if (std.mem.eql(u8, entry.view.label, &.{current})) {
+                    break entry.handle;
+                }
+            } else return error.NotFound,
+        ) == null);
+    }
+
+    // Find "next" from A forward.
+    {
+        var current: u8 = 'A';
+        while (current != 'D') : (current += 1) {
+            it = t5.iterator();
+            const handle = t5.next(
+                while (it.next()) |entry| {
+                    if (std.mem.eql(u8, entry.view.label, &.{current})) {
+                        break entry.handle;
+                    }
+                } else return error.NotFound,
+            ).?;
+
+            const entry = t5.nodes[handle].leaf;
+            try testing.expectEqualStrings(
+                entry.label,
+                &.{current + 1},
+            );
+        }
+
+        it = t5.iterator();
+        try testing.expect(t5.next(
+            while (it.next()) |entry| {
+                if (std.mem.eql(u8, entry.view.label, &.{current})) {
+                    break entry.handle;
+                }
+            } else return error.NotFound,
+        ) == null);
+    }
 }
 
 test "SplitTree: split vertical" {
@@ -1016,6 +1304,7 @@ test "SplitTree: split vertical" {
         alloc,
         0, // at root
         .down, // split down
+        0.5,
         &t2, // insert t2
     );
     defer t3.deinit();
@@ -1047,6 +1336,7 @@ test "SplitTree: remove leaf" {
         alloc,
         0, // at root
         .right, // split right
+        0.5,
         &t2, // insert t2
     );
     defer t3.deinit();
@@ -1092,6 +1382,7 @@ test "SplitTree: split twice, remove intermediary" {
         alloc,
         0, // at root
         .right, // split right
+        0.5,
         &t2, // insert t2
     );
     defer split1.deinit();
@@ -1101,6 +1392,7 @@ test "SplitTree: split twice, remove intermediary" {
         alloc,
         0, // at root
         .down, // split down
+        0.5,
         &t3, // insert t3
     );
     defer split2.deinit();
@@ -1154,6 +1446,125 @@ test "SplitTree: split twice, remove intermediary" {
     }
 }
 
+test "SplitTree: spatial goto" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var v1: TestTree.View = .{ .label = "A" };
+    var t1: TestTree = try .init(alloc, &v1);
+    defer t1.deinit();
+    var v2: TestTree.View = .{ .label = "B" };
+    var t2: TestTree = try .init(alloc, &v2);
+    defer t2.deinit();
+    var v3: TestTree.View = .{ .label = "C" };
+    var t3: TestTree = try .init(alloc, &v3);
+    defer t3.deinit();
+    var v4: TestTree.View = .{ .label = "D" };
+    var t4: TestTree = try .init(alloc, &v4);
+    defer t4.deinit();
+
+    // A | B horizontal
+    var splitAB = try t1.split(
+        alloc,
+        0, // at root
+        .right, // split right
+        0.5,
+        &t2, // insert t2
+    );
+    defer splitAB.deinit();
+
+    // A | C vertical
+    var splitAC = try splitAB.split(
+        alloc,
+        at: {
+            var it = splitAB.iterator();
+            break :at while (it.next()) |entry| {
+                if (std.mem.eql(u8, entry.view.label, "A")) {
+                    break entry.handle;
+                }
+            } else return error.NotFound;
+        },
+        .down, // split down
+        0.8,
+        &t3, // insert t3
+    );
+    defer splitAC.deinit();
+
+    // B | D vertical
+    var splitBD = try splitAC.split(
+        alloc,
+        at: {
+            var it = splitAB.iterator();
+            break :at while (it.next()) |entry| {
+                if (std.mem.eql(u8, entry.view.label, "B")) {
+                    break entry.handle;
+                }
+            } else return error.NotFound;
+        },
+        .down, // split down
+        0.3,
+        &t4, // insert t4
+    );
+    defer splitBD.deinit();
+    const split = splitBD;
+
+    {
+        const str = try std.fmt.allocPrint(alloc, "{diagram}", .{split});
+        defer alloc.free(str);
+        try testing.expectEqualStrings(str,
+            \\+---++---+
+            \\|   || B |
+            \\|   |+---+
+            \\|   |+---+
+            \\| A ||   |
+            \\|   ||   |
+            \\|   ||   |
+            \\|   || D |
+            \\+---+|   |
+            \\+---+|   |
+            \\| C ||   |
+            \\+---++---+
+            \\
+        );
+    }
+
+    // Spatial C => right
+    {
+        const target = (try split.goto(
+            alloc,
+            from: {
+                var it = split.iterator();
+                break :from while (it.next()) |entry| {
+                    if (std.mem.eql(u8, entry.view.label, "C")) {
+                        break entry.handle;
+                    }
+                } else return error.NotFound;
+            },
+            .{ .spatial = .right },
+        )).?;
+        const view = split.nodes[target].leaf;
+        try testing.expectEqualStrings(view.label, "D");
+    }
+
+    // Spatial D => left
+    {
+        const target = (try split.goto(
+            alloc,
+            from: {
+                var it = split.iterator();
+                break :from while (it.next()) |entry| {
+                    if (std.mem.eql(u8, entry.view.label, "D")) {
+                        break entry.handle;
+                    }
+                } else return error.NotFound;
+            },
+            .{ .spatial = .left },
+        )).?;
+        const view = split.nodes[target].leaf;
+        try testing.expectEqualStrings("A", view.label);
+    }
+}
+
 test "SplitTree: clone empty tree" {
     const testing = std.testing;
     const alloc = testing.allocator;