From ae5dc3a4fb67db8689a858ed6d020b1b43b8e41b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 7 Aug 2025 10:32:34 -0700 Subject: [PATCH 01/18] apprt/gtk-ng: split tree new split actions --- src/apprt/gtk-ng/class/split_tree.zig | 160 ++++++++++++++++++++++++++ src/apprt/gtk-ng/class/tab.zig | 36 ++---- src/apprt/gtk-ng/ui/1.2/surface.blp | 8 +- 3 files changed, 175 insertions(+), 29 deletions(-) diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index 2e2af118a..4e7e55f00 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -1,6 +1,7 @@ const std = @import("std"); const build_config = @import("../../../build_config.zig"); const assert = std.debug.assert; +const Allocator = std.mem.Allocator; const adw = @import("adw"); const gio = @import("gio"); const glib = @import("glib"); @@ -36,6 +37,30 @@ pub const SplitTree = extern struct { }); pub const properties = struct { + /// The active surface is the surface that should be receiving all + /// surface-targeted actions. This is usually the focused surface, + /// but may also not be focused if the user has selected a non-surface + /// widget. + pub const @"active-surface" = struct { + pub const name = "active-surface"; + const impl = gobject.ext.defineProperty( + name, + Self, + ?*Surface, + .{ + .nick = "Active Surface", + .blurb = "The currently active surface.", + .accessor = gobject.ext.typedAccessor( + Self, + ?*Surface, + .{ + .getter = getActiveSurface, + }, + ), + }, + ); + }; + pub const @"has-surfaces" = struct { pub const name = "has-surfaces"; const impl = gobject.ext.defineProperty( @@ -98,11 +123,132 @@ pub const SplitTree = extern struct { fn init(self: *Self, _: *Class) callconv(.c) void { gtk.Widget.initTemplate(self.as(gtk.Widget)); + + // Initialize our actions + self.initActions(); + } + + fn initActions(self: *Self) void { + // The set of actions. Each action has (in order): + // [0] The action name + // [1] The callback function + // [2] The glib.VariantType of the parameter + // + // For action names: + // https://docs.gtk.org/gio/type_func.Action.name_is_valid.html + const actions = .{ + // All of these will eventually take a target surface parameter. + // For now all our targets originate from the focused surface. + .{ "new-left", actionNew, null }, + .{ "new-right", actionNew, null }, + .{ "new-up", actionNew, null }, + .{ "new-down", actionNew, null }, + }; + + // We need to collect our actions into a group since we're just + // a plain widget that doesn't implement ActionGroup directly. + const group = gio.SimpleActionGroup.new(); + errdefer group.unref(); + const map = group.as(gio.ActionMap); + inline for (actions) |entry| { + const action = gio.SimpleAction.new( + entry[0], + entry[2], + ); + defer action.unref(); + _ = gio.SimpleAction.signals.activate.connect( + action, + *Self, + entry[1], + self, + .{}, + ); + map.addAction(action.as(gio.Action)); + } + + self.as(gtk.Widget).insertActionGroup( + "split-tree", + group.as(gio.ActionGroup), + ); + } + + /// Create a new split in the given direction from the currently + /// active surface. + /// + /// If the tree is empty this will create a new tree with a new surface + /// and ignore the direction. + /// + /// The parent will be used as the parent of the surface regardless of + /// if that parent is in this split tree or not. This allows inheriting + /// surface properties from anywhere. + pub fn newSplit( + self: *Self, + direction: Surface.Tree.Split.Direction, + parent_: ?*Surface, + ) Allocator.Error!void { + const alloc = Application.default().allocator(); + + // Create our new surface. + const surface: *Surface = .new(); + defer surface.unref(); + _ = surface.refSink(); + + // Inherit properly if we were asked to. + if (parent_) |p| { + if (p.core()) |core| { + surface.setParent(core); + } + } + + // Create our tree + var single_tree = try Surface.Tree.init(alloc, surface); + defer single_tree.deinit(); + + // If we have no tree yet, then this becomes our tree and we're done. + const old_tree = self.getTree() orelse { + self.setTree(&single_tree); + return; + }; + + // The handle we create the split relative to. Today this is the active + // surface but this might be the handle of the given parent if we want. + const handle = self.getActiveSurfaceHandle() orelse 0; + + // Create our split! + var new_tree = try old_tree.split( + alloc, + handle, + direction, + &single_tree, + ); + defer new_tree.deinit(); + self.setTree(&new_tree); + + // Focus our new surface + surface.grabFocus(); } //--------------------------------------------------------------- // Properties + /// Get the currently active surface. See the "active-surface" property. + /// This does not ref the value. + pub fn getActiveSurface(self: *Self) ?*Surface { + const tree = self.getTree() orelse return null; + const handle = self.getActiveSurfaceHandle() orelse return null; + return tree.nodes[handle].leaf; + } + + fn getActiveSurfaceHandle(self: *Self) ?Surface.Tree.Node.Handle { + const tree = self.getTree() orelse return null; + var it = tree.iterator(); + while (it.next()) |entry| { + if (entry.view.getFocused()) return entry.handle; + } + + return null; + } + pub fn getHasSurfaces(self: *Self) bool { const tree: *const Surface.Tree = self.private().tree orelse &.empty; return !tree.isEmpty(); @@ -185,6 +331,20 @@ pub const SplitTree = extern struct { //--------------------------------------------------------------- // Signal handlers + pub fn actionNew( + _: *gio.SimpleAction, + parameter_: ?*glib.Variant, + self: *Self, + ) callconv(.c) void { + _ = parameter_; + self.newSplit( + .right, + self.getActiveSurface(), + ) catch |err| { + log.warn("new split failed error={}", .{err}); + }; + } + fn propTree( self: *Self, _: *gobject.ParamSpec, diff --git a/src/apprt/gtk-ng/class/tab.zig b/src/apprt/gtk-ng/class/tab.zig index a5c088d15..4b75701bf 100644 --- a/src/apprt/gtk-ng/class/tab.zig +++ b/src/apprt/gtk-ng/class/tab.zig @@ -36,7 +36,7 @@ pub const Tab = extern struct { }); pub const properties = struct { - /// The active surface is the focus that should be receiving all + /// The active surface is the surface that should be receiving all /// surface-targeted actions. This is usually the focused surface, /// but may also not be focused if the user has selected a non-surface /// widget. @@ -164,23 +164,15 @@ pub const Tab = extern struct { .{}, ); - // A tab always starts with a single surface. - const surface: *Surface = .new(); - defer surface.unref(); - _ = surface.refSink(); - const alloc = Application.default().allocator(); - if (Surface.Tree.init(alloc, surface)) |tree| { - priv.split_tree.setTree(&tree); - - // Hacky because we need a non-const result. - var mut = tree; - mut.deinit(); - } else |_| { - // TODO: We should make our "no surfaces" state more aesthetically - // pleasing and show something like an "Oops, something went wrong" - // message. For now, this is incredibly unlikely. - @panic("oom"); - } + // Create our initial surface in the split tree. + priv.split_tree.newSplit(.right, null) catch |err| switch (err) { + error.OutOfMemory => { + // TODO: We should make our "no surfaces" state more aesthetically + // pleasing and show something like an "Oops, something went wrong" + // message. For now, this is incredibly unlikely. + @panic("oom"); + }, + }; } fn connectSurfaceHandlers( @@ -232,13 +224,7 @@ pub const Tab = extern struct { /// Get the currently active surface. See the "active-surface" property. /// This does not ref the value. pub fn getActiveSurface(self: *Self) ?*Surface { - const tree = self.getSurfaceTree() orelse return null; - var it = tree.iterator(); - while (it.next()) |entry| { - if (entry.view.getFocused()) return entry.view; - } - - return null; + return self.getSplitTree().getActiveSurface(); } /// Get the surface tree of this tab. diff --git a/src/apprt/gtk-ng/ui/1.2/surface.blp b/src/apprt/gtk-ng/ui/1.2/surface.blp index e671a0d82..23499c7f3 100644 --- a/src/apprt/gtk-ng/ui/1.2/surface.blp +++ b/src/apprt/gtk-ng/ui/1.2/surface.blp @@ -172,22 +172,22 @@ menu context_menu_model { item { label: _("Split Up"); - action: "win.split-up"; + action: "split-tree.new-up"; } item { label: _("Split Down"); - action: "win.split-down"; + action: "split-tree.new-down"; } item { label: _("Split Left"); - action: "win.split-left"; + action: "split-tree.new-left"; } item { label: _("Split Right"); - action: "win.split-right"; + action: "split-tree.new-right"; } } From 75dd8e46b52bf0f9b3afd6475f37ecbcce6bfe97 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 7 Aug 2025 10:51:07 -0700 Subject: [PATCH 02/18] datastruct: fix split tree debug log rounding --- src/apprt/gtk-ng/class/split_tree.zig | 6 +++ src/datastruct/split_tree.zig | 57 +++++++++++++++++++++------ 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index 4e7e55f00..3d28d9e18 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -222,6 +222,12 @@ pub const SplitTree = extern struct { &single_tree, ); defer new_tree.deinit(); + log.debug( + "new split at={} direction={} old_tree={} new_tree={}", + .{ handle, direction, old_tree, &new_tree }, + ); + + // Replace our tree self.setTree(&new_tree); // Focus our new surface diff --git a/src/datastruct/split_tree.zig b/src/datastruct/split_tree.zig index 68a7c09e7..61e04e6f6 100644 --- a/src/datastruct/split_tree.zig +++ b/src/datastruct/split_tree.zig @@ -610,6 +610,8 @@ pub fn SplitTree(comptime V: type) type { // the width/height based on node 0. const grid = grid: { // Get our initial width/height. Each leaf is 1x1 in this. + // We round up for this because partial widths/heights should + // take up an extra cell. var width: usize = @intFromFloat(@ceil(sp.slots[0].width)); var height: usize = @intFromFloat(@ceil(sp.slots[0].height)); @@ -637,10 +639,10 @@ pub fn SplitTree(comptime V: type) type { .split => continue, } - var x: usize = @intFromFloat(@ceil(slot.x)); - var y: usize = @intFromFloat(@ceil(slot.y)); - var width: usize = @intFromFloat(@ceil(slot.width)); - var height: usize = @intFromFloat(@ceil(slot.height)); + var x: usize = @intFromFloat(@floor(slot.x)); + var y: usize = @intFromFloat(@floor(slot.y)); + var width: usize = @intFromFloat(@max(@floor(slot.width), 1)); + var height: usize = @intFromFloat(@max(@floor(slot.height), 1)); x *= cell_width; y *= cell_height; width *= cell_width; @@ -806,13 +808,13 @@ test "SplitTree: single node" { test "SplitTree: split horizontal" { 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 t3 = try t1.split( alloc, 0, // at root @@ -821,14 +823,45 @@ test "SplitTree: split horizontal" { ); defer t3.deinit(); - const str = try std.fmt.allocPrint(alloc, "{}", .{t3}); - defer alloc.free(str); - try testing.expectEqualStrings(str, - \\+---++---+ - \\| A || B | - \\+---++---+ - \\ + { + const str = try std.fmt.allocPrint(alloc, "{}", .{t3}); + defer alloc.free(str); + try testing.expectEqualStrings(str, + \\+---++---+ + \\| A || B | + \\+---++---+ + \\ + ); + } + + var vC: TestTree.View = .{ .label = "C" }; + var tC: TestTree = try .init(alloc, &vC); + defer tC.deinit(); + + // Split right at B + var it = t3.iterator(); + var t4 = try t3.split( + alloc, + while (it.next()) |entry| { + if (std.mem.eql(u8, entry.view.label, "B")) { + break entry.handle; + } + } else return error.NotFound, + .right, + &tC, ); + defer t4.deinit(); + + { + const str = try std.fmt.allocPrint(alloc, "{}", .{t4}); + defer alloc.free(str); + try testing.expectEqualStrings(str, + \\+---++---++---+ + \\| A || B || C | + \\+---++---++---+ + \\ + ); + } } test "SplitTree: split vertical" { From fbe28477ff409e65ea367986d7daecd056106013 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 7 Aug 2025 11:41:15 -0700 Subject: [PATCH 03/18] datastruct: fix split tree ascii diagram --- src/apprt/gtk-ng/class/split_tree.zig | 2 +- src/datastruct/split_tree.zig | 173 +++++++++++++++++++++----- 2 files changed, 143 insertions(+), 32 deletions(-) diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index 3d28d9e18..7172c931b 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -379,7 +379,7 @@ pub const SplitTree = extern struct { ) *gtk.Widget { switch (tree.nodes[current]) { .leaf => |v| { - // We have to setup our signal handlers. + v.as(gtk.Widget).unparent(); return v.as(gtk.Widget); }, diff --git a/src/datastruct/split_tree.zig b/src/datastruct/split_tree.zig index 61e04e6f6..cab296081 100644 --- a/src/datastruct/split_tree.zig +++ b/src/datastruct/split_tree.zig @@ -409,22 +409,7 @@ pub fn SplitTree(comptime V: type) type { assert(reffed == nodes.len - 1); } - /// Spatial representation of the split tree. This can be used to - /// better understand the layout of the tree in a 2D space. - /// - /// The bounds of the representation are always based on each split - /// being exactly 1 unit wide and high. The x and y coordinates - /// are offsets into that space. This means that the spatial - /// representation is a normalized representation of the actual - /// space. - /// - /// The top-left corner of the tree is always (0, 0). - /// - /// We use a normalized form because we can calculate it without - /// accessing to the actual rendered view sizes. These actual sizes - /// may not be available at various times because GUI toolkits often - /// only make them available once they're part of a widget tree and - /// a SplitTree can represent views that aren't currently visible. + /// Spatial representation of the split tree. See spatial. pub const Spatial = struct { /// The slots of the spatial representation in the same order /// as the tree it was created from. @@ -445,8 +430,22 @@ pub fn SplitTree(comptime V: type) type { } }; - /// Returns the spatial representation of this tree. See Spatial - /// for more details. + /// Spatial representation of the split tree. This can be used to + /// better understand the layout of the tree in a 2D space. + /// + /// The bounds of the representation are always based on each split + /// being exactly 1 unit wide and high. The x and y coordinates + /// are offsets into that space. This means that the spatial + /// representation is a normalized representation of the actual + /// space. + /// + /// The top-left corner of the tree is always (0, 0). + /// + /// We use a normalized form because we can calculate it without + /// accessing to the actual rendered view sizes. These actual sizes + /// may not be available at various times because GUI toolkits often + /// only make them available once they're part of a widget tree and + /// a SplitTree can represent views that aren't currently visible. pub fn spatial( self: *const Self, alloc: Allocator, @@ -549,14 +548,20 @@ pub fn SplitTree(comptime V: type) type { }; } - /// Format the tree in a human-readable format. + /// Format the tree in a human-readable format. By default this will + /// output a diagram followed by a textual representation. This can + /// be controlled via the formatting string: + /// + /// - `diagram` - Output a diagram of the split tree only. + /// - `text` - Output a textual representation of the split tree only. + /// - Empty - Output both a diagram and a textual representation. + /// pub fn format( self: *const Self, comptime fmt: []const u8, options: std.fmt.FormatOptions, writer: anytype, ) !void { - _ = fmt; _ = options; if (self.nodes.len == 0) { @@ -564,6 +569,48 @@ pub fn SplitTree(comptime V: type) type { return; } + if (std.mem.eql(u8, fmt, "diagram")) { + self.formatDiagram(writer) catch + try writer.writeAll("failed to draw split tree diagram"); + } else if (std.mem.eql(u8, fmt, "text")) { + try self.formatText(writer, 0, 0); + } else if (fmt.len == 0) { + self.formatDiagram(writer) catch {}; + try self.formatText(writer, 0, 0); + } else { + return error.InvalidFormat; + } + } + + fn formatText( + self: *const Self, + writer: anytype, + current: Node.Handle, + depth: usize, + ) !void { + for (0..depth) |_| try writer.writeAll(" "); + + switch (self.nodes[current]) { + .leaf => |v| if (@hasDecl(View, "splitTreeLabel")) + try writer.print("leaf: {s}\n", .{v.splitTreeLabel()}) + else + try writer.print("leaf: {d}\n", .{current}), + + .split => |s| { + try writer.print("split (layout: {s}, ratio: {d:.2})\n", .{ + @tagName(s.layout), + s.ratio, + }); + try self.formatText(writer, s.left, depth + 1); + try self.formatText(writer, s.right, depth + 1); + }, + } + } + + fn formatDiagram( + self: *const Self, + writer: anytype, + ) !void { // Use our arena's GPA to allocate some intermediate memory. // Requiring allocation for formatting is nasty but this is really // only used for debugging and testing and shouldn't hit OOM @@ -573,7 +620,29 @@ pub fn SplitTree(comptime V: type) type { const alloc = arena.allocator(); // Get our spatial representation. - const sp = try self.spatial(alloc); + const sp = spatial: { + const sp = try self.spatial(alloc); + + // Scale our spatial representation to have minimum width/height 1. + var min_w: f16 = 1; + var min_h: f16 = 1; + for (sp.slots) |slot| { + min_w = @min(min_w, slot.width); + min_h = @min(min_h, slot.height); + } + + const ratio_w: f16 = 1 / min_w; + const ratio_h: f16 = 1 / min_h; + const slots = try alloc.dupe(Spatial.Slot, sp.slots); + for (slots) |*slot| { + slot.x *= ratio_w; + slot.y *= ratio_h; + slot.width *= ratio_w; + slot.height *= ratio_h; + } + + break :spatial .{ .slots = slots }; + }; // The width we need for the largest label. const max_label_width: usize = max_label_width: { @@ -795,7 +864,7 @@ test "SplitTree: single node" { var t: TestTree = try .init(alloc, &v); defer t.deinit(); - const str = try std.fmt.allocPrint(alloc, "{}", .{t}); + const str = try std.fmt.allocPrint(alloc, "{diagram}", .{t}); defer alloc.free(str); try testing.expectEqualStrings(str, \\+---+ @@ -830,15 +899,17 @@ test "SplitTree: split horizontal" { \\+---++---+ \\| A || B | \\+---++---+ + \\split (layout: horizontal, ratio: 0.50) + \\ leaf: A + \\ leaf: B \\ ); } + // Split right at B var vC: TestTree.View = .{ .label = "C" }; var tC: TestTree = try .init(alloc, &vC); defer tC.deinit(); - - // Split right at B var it = t3.iterator(); var t4 = try t3.split( alloc, @@ -856,12 +927,52 @@ test "SplitTree: split horizontal" { const str = try std.fmt.allocPrint(alloc, "{}", .{t4}); defer alloc.free(str); try testing.expectEqualStrings(str, - \\+---++---++---+ - \\| A || B || C | - \\+---++---++---+ + \\+--------++---++---+ + \\| A || B || C | + \\+--------++---++---+ + \\split (layout: horizontal, ratio: 0.50) + \\ leaf: A + \\ split (layout: horizontal, ratio: 0.50) + \\ leaf: B + \\ leaf: C \\ ); } + + // Split right at C + var vD: TestTree.View = .{ .label = "D" }; + var tD: TestTree = try .init(alloc, &vD); + defer tD.deinit(); + it = t4.iterator(); + var t5 = try t4.split( + alloc, + while (it.next()) |entry| { + if (std.mem.eql(u8, entry.view.label, "C")) { + break entry.handle; + } + } else return error.NotFound, + .right, + &tD, + ); + defer t5.deinit(); + + { + const str = try std.fmt.allocPrint(alloc, "{}", .{t5}); + defer alloc.free(str); + try testing.expectEqualStrings( + \\+------------------++--------++---++---+ + \\| A || B || C || D | + \\+------------------++--------++---++---+ + \\split (layout: horizontal, ratio: 0.50) + \\ leaf: A + \\ split (layout: horizontal, ratio: 0.50) + \\ leaf: B + \\ split (layout: horizontal, ratio: 0.50) + \\ leaf: C + \\ leaf: D + \\ + , str); + } } test "SplitTree: split vertical" { @@ -883,7 +994,7 @@ test "SplitTree: split vertical" { ); defer t3.deinit(); - const str = try std.fmt.allocPrint(alloc, "{}", .{t3}); + const str = try std.fmt.allocPrint(alloc, "{diagram}", .{t3}); defer alloc.free(str); try testing.expectEqualStrings(str, \\+---+ @@ -926,7 +1037,7 @@ test "SplitTree: remove leaf" { ); defer t4.deinit(); - const str = try std.fmt.allocPrint(alloc, "{}", .{t4}); + const str = try std.fmt.allocPrint(alloc, "{diagram}", .{t4}); defer alloc.free(str); try testing.expectEqualStrings(str, \\+---+ @@ -969,7 +1080,7 @@ test "SplitTree: split twice, remove intermediary" { defer split2.deinit(); { - const str = try std.fmt.allocPrint(alloc, "{}", .{split2}); + const str = try std.fmt.allocPrint(alloc, "{diagram}", .{split2}); defer alloc.free(str); try testing.expectEqualStrings(str, \\+---++---+ @@ -995,7 +1106,7 @@ test "SplitTree: split twice, remove intermediary" { defer split3.deinit(); { - const str = try std.fmt.allocPrint(alloc, "{}", .{split3}); + const str = try std.fmt.allocPrint(alloc, "{diagram}", .{split3}); defer alloc.free(str); try testing.expectEqualStrings(str, \\+---+ From 517f17995c7ab8447433216442f9bccc6e4e87e9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 8 Aug 2025 14:35:34 -0700 Subject: [PATCH 04/18] apprt/gtk-ng: rebuild the widget tree on an idle callback --- src/apprt/gtk-ng/class/split_tree.zig | 56 +++++++++++++++++++++------ 1 file changed, 44 insertions(+), 12 deletions(-) diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index 7172c931b..fb228cf17 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -118,6 +118,10 @@ pub const SplitTree = extern struct { // Template bindings tree_bin: *adw.Bin, + /// The source that we use to rebuild the tree. This is also + /// used to debounce updates. + rebuild_source: ?c_uint = null, + pub var offset: c_int = 0; }; @@ -310,6 +314,14 @@ pub const SplitTree = extern struct { // Virtual methods fn dispose(self: *Self) callconv(.c) void { + const priv = self.private(); + if (priv.rebuild_source) |v| { + if (glib.Source.remove(v) == 0) { + log.warn("unable to remove rebuild source", .{}); + } + priv.rebuild_source = null; + } + gtk.Widget.disposeTemplate( self.as(gtk.Widget), getGObjectType(), @@ -357,16 +369,40 @@ pub const SplitTree = extern struct { _: ?*anyopaque, ) callconv(.c) void { const priv = self.private(); - const tree: *const Surface.Tree = self.private().tree orelse &.empty; - // Reset our widget tree. + // We need to reset our tree and create the new widget hierarchy + // on two separate event loop ticks to allow GTK to properly relayout + // our widgets. + // + // Doing this all at once will cause strange rendering glitches, + // the glarea to be gone forever (but not deallocated), etc. I think + // this is probably a bug in GTK we can minimize and report later. + // + // Using an idle callback also allows us to debounce updates. priv.tree_bin.setChild(null); + if (priv.rebuild_source == null) priv.rebuild_source = glib.idleAdd( + onRebuild, + self, + ); + + // Dependent properties + self.as(gobject.Object).notifyByPspec(properties.@"has-surfaces".impl.param_spec); + } + + fn onRebuild(ud: ?*anyopaque) callconv(.c) c_int { + const self: *Self = @ptrCast(@alignCast(ud orelse return 0)); + + // Always mark our rebuild source as null since we're done. + const priv = self.private(); + priv.rebuild_source = null; + + // Rebuild our tree + const tree: *const Surface.Tree = self.private().tree orelse &.empty; if (!tree.isEmpty()) { priv.tree_bin.setChild(buildTree(tree, 0)); } - // Dependent properties - self.as(gobject.Object).notifyByPspec(properties.@"has-surfaces".impl.param_spec); + return 0; } /// Builds the widget tree associated with a surface split tree. @@ -377,13 +413,9 @@ pub const SplitTree = extern struct { tree: *const Surface.Tree, current: Surface.Tree.Node.Handle, ) *gtk.Widget { - switch (tree.nodes[current]) { - .leaf => |v| { - v.as(gtk.Widget).unparent(); - return v.as(gtk.Widget); - }, - - .split => |s| return gobject.ext.newInstance( + return switch (tree.nodes[current]) { + .leaf => |v| v.as(gtk.Widget), + .split => |s| gobject.ext.newInstance( gtk.Paned, .{ .orientation = @as(gtk.Orientation, switch (s.layout) { @@ -395,7 +427,7 @@ pub const SplitTree = extern struct { // TODO: position/ratio }, ).as(gtk.Widget), - } + }; } //--------------------------------------------------------------- From a3c041bcb4de25fbbfdbba0ddcb2f49106ce6957 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 8 Aug 2025 15:03:28 -0700 Subject: [PATCH 05/18] apprt/gtk-ng: keep track of last focused surface --- src/apprt/gtk-ng/class/application.zig | 1 + src/apprt/gtk-ng/class/split_tree.zig | 76 ++++++++++++++++++++++++++ valgrind.supp | 36 +++++++++++- 3 files changed, 110 insertions(+), 3 deletions(-) diff --git a/src/apprt/gtk-ng/class/application.zig b/src/apprt/gtk-ng/class/application.zig index 9c0a821cd..226d7c56b 100644 --- a/src/apprt/gtk-ng/class/application.zig +++ b/src/apprt/gtk-ng/class/application.zig @@ -1257,6 +1257,7 @@ pub const Application = extern struct { diag.close(); diag.unref(); // strong ref from get() } + priv.config_errors_dialog.set(null); if (priv.signal_source) |v| { if (glib.Source.remove(v) == 0) { log.warn("unable to remove signal source", .{}); diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index fb228cf17..22cae7ee2 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -17,6 +17,7 @@ const adw_version = @import("../adw_version.zig"); const ext = @import("../ext.zig"); const gresource = @import("../build/gresource.zig"); const Common = @import("../class.zig").Common; +const WeakRef = @import("../weak_ref.zig").WeakRef; const Config = @import("config.zig").Config; const Application = @import("application.zig").Application; const CloseConfirmationDialog = @import("close_confirmation_dialog.zig").CloseConfirmationDialog; @@ -118,6 +119,10 @@ pub const SplitTree = extern struct { // Template bindings tree_bin: *adw.Bin, + /// Last focused surface in the tree. We need this to handle various + /// tree change states. + last_focused: WeakRef(Surface) = .{}, + /// The source that we use to rebuild the tree. This is also /// used to debounce updates. rebuild_source: ?c_uint = null, @@ -208,6 +213,13 @@ pub const SplitTree = extern struct { var single_tree = try Surface.Tree.init(alloc, surface); defer single_tree.deinit(); + // We want to move our focus to the new surface no matter what. + // But we need to be careful to restore state if we fail. + const old_last_focused = self.private().last_focused.get(); + defer if (old_last_focused) |v| v.unref(); // unref strong ref from get + self.private().last_focused.set(surface); + errdefer self.private().last_focused.set(old_last_focused); + // If we have no tree yet, then this becomes our tree and we're done. const old_tree = self.getTree() orelse { self.setTree(&single_tree); @@ -238,6 +250,38 @@ pub const SplitTree = extern struct { surface.grabFocus(); } + fn disconnectSurfaceHandlers(self: *Self) void { + const tree = self.getTree() orelse return; + var it = tree.iterator(); + while (it.next()) |entry| { + const surface = entry.view; + _ = gobject.signalHandlersDisconnectMatched( + surface.as(gobject.Object), + .{ .data = true }, + 0, + 0, + null, + null, + self, + ); + } + } + + fn connectSurfaceHandlers(self: *Self) void { + const tree = self.getTree() orelse return; + var it = tree.iterator(); + while (it.next()) |entry| { + const surface = entry.view; + _ = gobject.Object.signals.notify.connect( + surface, + *Self, + propSurfaceFocused, + self, + .{ .detail = "focused" }, + ); + } + } + //--------------------------------------------------------------- // Properties @@ -259,6 +303,15 @@ pub const SplitTree = extern struct { return null; } + /// Returns the last focused surface in the tree. + pub fn getLastFocusedSurface(self: *Self) ?*Surface { + const surface = self.private().last_focused.get() orelse return null; + // We unref because get() refs the surface. We don't use the weakref + // in a multi-threaded context so this is safe. + surface.unref(); + return surface; + } + pub fn getHasSurfaces(self: *Self) bool { const tree: *const Surface.Tree = self.private().tree orelse &.empty; return !tree.isEmpty(); @@ -285,12 +338,14 @@ pub const SplitTree = extern struct { ); if (priv.tree) |old_tree| { + self.disconnectSurfaceHandlers(); ext.boxedFree(Surface.Tree, old_tree); priv.tree = null; } if (tree) |new_tree| { priv.tree = ext.boxedCopy(Surface.Tree, new_tree); + self.connectSurfaceHandlers(); } self.as(gobject.Object).notifyByPspec(properties.tree.impl.param_spec); @@ -315,6 +370,7 @@ pub const SplitTree = extern struct { fn dispose(self: *Self) callconv(.c) void { const priv = self.private(); + priv.last_focused.set(null); if (priv.rebuild_source) |v| { if (glib.Source.remove(v) == 0) { log.warn("unable to remove rebuild source", .{}); @@ -363,6 +419,18 @@ pub const SplitTree = extern struct { }; } + fn propSurfaceFocused( + surface: *Surface, + _: *gobject.ParamSpec, + self: *Self, + ) callconv(.c) void { + // We never CLEAR our last_focused because the property is specifically + // the last focused surface. We let the weakref clear itself when + // the surface is destroyed. + if (!surface.getFocused()) return; + self.private().last_focused.set(surface); + } + fn propTree( self: *Self, _: *gobject.ParamSpec, @@ -402,6 +470,14 @@ pub const SplitTree = extern struct { priv.tree_bin.setChild(buildTree(tree, 0)); } + // If we have a last focused surface, we need to refocus it, because + // during the frame between setting the bin to null and rebuilding, + // GTK will reset our focus state (as it should!) + if (priv.last_focused.get()) |v| { + defer v.unref(); + v.grabFocus(); + } + return 0; } diff --git a/valgrind.supp b/valgrind.supp index 0ce99e34a..45a9c3ea7 100644 --- a/valgrind.supp +++ b/valgrind.supp @@ -45,6 +45,38 @@ ... } +# Reproduction: +# +# 1. Launch Ghostty +# 2. Split Right +# 3. Hit "X" to close +{ + GTK CSS Node State + Memcheck:Leak + match-leak-kinds: possible + fun:malloc + fun:g_malloc + fun:g_memdup2 + fun:gtk_css_node_declaration_set_state + fun:gtk_css_node_set_state + fun:gtk_widget_propagate_state + fun:gtk_widget_update_state_flags + fun:gtk_main_do_event + fun:surface_event + fun:_gdk_marshal_BOOLEAN__POINTERv + fun:gdk_surface_event_marshallerv + fun:_g_closure_invoke_va + fun:signal_emit_valist_unlocked + fun:g_signal_emit_valist + fun:g_signal_emit + fun:gdk_surface_handle_event + fun:gdk_wayland_event_source_dispatch + fun:g_main_context_dispatch_unlocked + fun:g_main_context_iterate_unlocked.isra.0 + fun:g_main_context_iteration + ... +} + { GTK CSS Provider Leak Memcheck:Leak @@ -516,9 +548,7 @@ pango font map Memcheck:Leak match-leak-kinds: possible - fun:calloc - fun:g_malloc0 - fun:g_rc_box_alloc_full + ... fun:pango_fc_font_map_load_fontset ... } From e396d9d78d651a5dd1ad60fcb283c4e7adca8952 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 8 Aug 2025 15:43:05 -0700 Subject: [PATCH 06/18] apprt/gtk-ng: setup gtk paned listeners to set position --- src/apprt/gtk-ng/build/gresource.zig | 1 + src/apprt/gtk-ng/class/split_tree.zig | 283 ++++++++++++++++++- src/apprt/gtk-ng/ui/1.5/split-tree-split.blp | 16 ++ 3 files changed, 288 insertions(+), 12 deletions(-) create mode 100644 src/apprt/gtk-ng/ui/1.5/split-tree-split.blp diff --git a/src/apprt/gtk-ng/build/gresource.zig b/src/apprt/gtk-ng/build/gresource.zig index 0f7237331..0818a98f6 100644 --- a/src/apprt/gtk-ng/build/gresource.zig +++ b/src/apprt/gtk-ng/build/gresource.zig @@ -41,6 +41,7 @@ pub const blueprints: []const Blueprint = &.{ .{ .major = 1, .minor = 3, .name = "debug-warning" }, .{ .major = 1, .minor = 2, .name = "resize-overlay" }, .{ .major = 1, .minor = 5, .name = "split-tree" }, + .{ .major = 1, .minor = 5, .name = "split-tree-split" }, .{ .major = 1, .minor = 2, .name = "surface" }, .{ .major = 1, .minor = 3, .name = "surface-child-exited" }, .{ .major = 1, .minor = 5, .name = "tab" }, diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index 22cae7ee2..d30b8ce4d 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -467,7 +467,7 @@ pub const SplitTree = extern struct { // Rebuild our tree const tree: *const Surface.Tree = self.private().tree orelse &.empty; if (!tree.isEmpty()) { - priv.tree_bin.setChild(buildTree(tree, 0)); + priv.tree_bin.setChild(self.buildTree(tree, 0)); } // If we have a last focused surface, we need to refocus it, because @@ -486,22 +486,17 @@ pub const SplitTree = extern struct { /// The final returned widget is expected to be a floating reference, /// ready to be attached to a parent widget. fn buildTree( + self: *Self, tree: *const Surface.Tree, current: Surface.Tree.Node.Handle, ) *gtk.Widget { return switch (tree.nodes[current]) { .leaf => |v| v.as(gtk.Widget), - .split => |s| gobject.ext.newInstance( - gtk.Paned, - .{ - .orientation = @as(gtk.Orientation, switch (s.layout) { - .horizontal => .horizontal, - .vertical => .vertical, - }), - .@"start-child" = buildTree(tree, s.left), - .@"end-child" = buildTree(tree, s.right), - // TODO: position/ratio - }, + .split => |s| SplitTreeSplit.new( + current, + &s, + self.buildTree(tree, s.left), + self.buildTree(tree, s.right), ).as(gtk.Widget), }; } @@ -556,3 +551,267 @@ pub const SplitTree = extern struct { pub const bindTemplateCallback = C.Class.bindTemplateCallback; }; }; + +/// This is an internal-only widget that represents a split in the +/// split tree. This is a wrapper around gtk.Paned that allows us to handle +/// ratio (0 to 1) based positioning of the split, and also allows us to +/// write back the updated ratio to the split tree when the user manually +/// adjusts the split position. +/// +/// Since this is internal, it expects to be nested within a SplitTree and +/// will use `getAncestor` to find the SplitTree it belongs to. +/// +/// This is an _immutable_ widget. It isn't meant to be updated after +/// creation. As such, there are no properties or APIs to change the split, +/// access the paned, etc. +const SplitTreeSplit = extern struct { + const Self = @This(); + parent_instance: Parent, + pub const Parent = adw.Bin; + pub const getGObjectType = gobject.ext.defineClass(Self, .{ + .name = "GhosttySplitTreeSplit", + .instanceInit = &init, + .classInit = &Class.init, + .parent_class = &Class.parent, + .private = .{ .Type = Private, .offset = &Private.offset }, + }); + + const Private = struct { + /// The handle of the node in the tree that this split represents. + /// Assumed to be correct. + handle: Surface.Tree.Node.Handle, + + /// Source to handle repositioning the split when properties change. + idle: ?c_uint = null, + + // Template bindings + paned: *gtk.Paned, + + pub var offset: c_int = 0; + }; + + /// Create a new split. + /// + /// The reason we don't use GObject properties here is because this is + /// an immutable widget and we don't want to deal with the overhead of + /// all the boilerplate for properties, signals, bindings, etc. + pub fn new( + handle: Surface.Tree.Node.Handle, + split: *const Surface.Tree.Split, + start_child: *gtk.Widget, + end_child: *gtk.Widget, + ) *Self { + const self = gobject.ext.newInstance(Self, .{}); + const priv = self.private(); + priv.handle = handle; + + // Setup our paned fields + const paned = priv.paned; + paned.setStartChild(start_child); + paned.setEndChild(end_child); + paned.as(gtk.Orientable).setOrientation(switch (split.layout) { + .horizontal => .horizontal, + .vertical => .vertical, + }); + + // Signals and so on are setup in the template. + + return self; + } + + fn init(self: *Self, _: *Class) callconv(.c) void { + gtk.Widget.initTemplate(self.as(gtk.Widget)); + } + + fn refresh(self: *Self) void { + const priv = self.private(); + if (priv.idle == null) priv.idle = glib.idleAdd( + onIdle, + self, + ); + } + + fn onIdle(ud: ?*anyopaque) callconv(.c) c_int { + const self: *Self = @ptrCast(@alignCast(ud orelse return 0)); + const priv = self.private(); + const paned = priv.paned; + + // Our idle source is always over + priv.idle = null; + + // Get our split. This is the most dangerous part of this entire + // widget. We assume that this widget is always a child of a + // SplitTree, we assume that our handle is valid, and we assume + // the handle is always a split node. + const split: *Surface.Tree.Split = split: { + const split_tree = ext.getAncestor( + SplitTree, + self.as(gtk.Widget), + ) orelse return 0; + const tree = split_tree.getTree() orelse return 0; + // TODO: fix this constCast + break :split @constCast(&tree.nodes[priv.handle].split); + }; + + // Current, min, and max positions as pixels. + const pos = paned.getPosition(); + const min = min: { + var val = gobject.ext.Value.new(c_int); + defer val.unset(); + gobject.Object.getProperty( + paned.as(gobject.Object), + "min-position", + &val, + ); + break :min gobject.ext.Value.get(&val, c_int); + }; + const max = max: { + var val = gobject.ext.Value.new(c_int); + defer val.unset(); + gobject.Object.getProperty( + paned.as(gobject.Object), + "max-position", + &val, + ); + break :max gobject.ext.Value.get(&val, c_int); + }; + + // We don't actually use min, but we don't expect this to ever + // be non-zero, so let's add an assert to ensure that. + assert(min == 0); + + // If our max is zero then we can't do any math. I don't know + // if this is possible but I suspect it can be if you make a nested + // split completely minimized. + if (max == 0) return 0; + + // Determine our current ratio. + const current_ratio: f64 = ratio: { + const pos_f64: f64 = @floatFromInt(pos); + const max_f64: f64 = @floatFromInt(max); + break :ratio pos_f64 / max_f64; + }; + const desired_ratio: f64 = @floatCast(split.ratio); + + // If our ratio is close enough to our desired ratio, then + // we ignore the update. This is to avoid constant split updates + // for lossy floating point math. + if (std.math.approxEqAbs( + f64, + current_ratio, + desired_ratio, + 0.001, + )) { + return 0; + } + + // If we're out of bounds, then we need to either set the position + // to what we expect OR update our expected ratio. + + const desired_pos: c_int = desired_pos: { + const max_f64: f64 = @floatFromInt(max); + break :desired_pos @intFromFloat(@round(max_f64 * desired_ratio)); + }; + paned.setPosition(desired_pos); + + log.warn("DESIRED={} CURRENT={}", .{ desired_ratio, current_ratio }); + + return 0; + } + + //--------------------------------------------------------------- + // Signal handlers + + fn propPosition( + _: *gtk.Paned, + _: *gobject.ParamSpec, + self: *Self, + ) callconv(.c) void { + self.refresh(); + } + + fn propMaxPosition( + _: *gtk.Paned, + _: *gobject.ParamSpec, + self: *Self, + ) callconv(.c) void { + self.refresh(); + } + + fn propMinPosition( + _: *gtk.Paned, + _: *gobject.ParamSpec, + self: *Self, + ) callconv(.c) void { + self.refresh(); + } + + //--------------------------------------------------------------- + // Virtual methods + + fn dispose(self: *Self) callconv(.c) void { + const priv = self.private(); + if (priv.idle) |v| { + if (glib.Source.remove(v) == 0) { + log.warn("unable to remove idle source", .{}); + } + priv.idle = null; + } + + gtk.Widget.disposeTemplate( + self.as(gtk.Widget), + getGObjectType(), + ); + + gobject.Object.virtual_methods.dispose.call( + Class.parent, + self.as(Parent), + ); + } + + fn finalize(self: *Self) callconv(.c) void { + gobject.Object.virtual_methods.finalize.call( + Class.parent, + self.as(Parent), + ); + } + + const C = Common(Self, Private); + pub const as = C.as; + pub const ref = C.ref; + pub const unref = C.unref; + const private = C.private; + + pub const Class = extern struct { + parent_class: Parent.Class, + var parent: *Parent.Class = undefined; + pub const Instance = Self; + + fn init(class: *Class) callconv(.c) void { + gtk.Widget.Class.setTemplateFromResource( + class.as(gtk.Widget.Class), + comptime gresource.blueprint(.{ + .major = 1, + .minor = 5, + .name = "split-tree-split", + }), + ); + + // Bindings + class.bindTemplateChildPrivate("paned", .{}); + + // Template Callbacks + class.bindTemplateCallback("notify_max_position", &propMaxPosition); + class.bindTemplateCallback("notify_min_position", &propMinPosition); + class.bindTemplateCallback("notify_position", &propPosition); + + // Virtual methods + gobject.Object.virtual_methods.dispose.implement(class, &dispose); + gobject.Object.virtual_methods.finalize.implement(class, &finalize); + } + + pub const as = C.Class.as; + pub const bindTemplateChildPrivate = C.Class.bindTemplateChildPrivate; + pub const bindTemplateCallback = C.Class.bindTemplateCallback; + }; +}; diff --git a/src/apprt/gtk-ng/ui/1.5/split-tree-split.blp b/src/apprt/gtk-ng/ui/1.5/split-tree-split.blp new file mode 100644 index 000000000..aa194e8e8 --- /dev/null +++ b/src/apprt/gtk-ng/ui/1.5/split-tree-split.blp @@ -0,0 +1,16 @@ +using Gtk 4.0; +using Adw 1; + +template $GhosttySplitTreeSplit: Adw.Bin { + // The double-nesting is required due to a GTK bug where you can't + // bind the first child of a builder layout. If you do, you get a double + // dispose. Easiest way to see that is simply remove this and see the + // GTK critical errors (and sometimes crashes). + Adw.Bin { + Paned paned { + notify::max-position => $notify_max_position(); + notify::min-position => $notify_min_position(); + notify::position => $notify_position(); + } + } +} From 34be4de018011daa946a5fd12833b30ede3fad0b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 9 Aug 2025 12:19:03 -0700 Subject: [PATCH 07/18] apprt/gtk-ng: write back split ratio to tree --- src/apprt/gtk-ng/class/split_tree.zig | 43 +++++++++++++++++---------- src/datastruct/split_tree.zig | 21 +++++++++++++ 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index d30b8ce4d..cd03297b4 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -643,15 +643,12 @@ const SplitTreeSplit = extern struct { // widget. We assume that this widget is always a child of a // SplitTree, we assume that our handle is valid, and we assume // the handle is always a split node. - const split: *Surface.Tree.Split = split: { - const split_tree = ext.getAncestor( - SplitTree, - self.as(gtk.Widget), - ) orelse return 0; - const tree = split_tree.getTree() orelse return 0; - // TODO: fix this constCast - break :split @constCast(&tree.nodes[priv.handle].split); - }; + const split_tree = ext.getAncestor( + SplitTree, + self.as(gtk.Widget), + ) orelse return 0; + const tree = split_tree.getTree() orelse return 0; + const split: *const Surface.Tree.Split = &tree.nodes[priv.handle].split; // Current, min, and max positions as pixels. const pos = paned.getPosition(); @@ -675,6 +672,16 @@ const SplitTreeSplit = extern struct { ); break :max gobject.ext.Value.get(&val, c_int); }; + const pos_set: bool = max: { + var val = gobject.ext.Value.new(c_int); + defer val.unset(); + gobject.Object.getProperty( + paned.as(gobject.Object), + "position-set", + &val, + ); + break :max gobject.ext.Value.get(&val, c_int) != 0; + }; // We don't actually use min, but we don't expect this to ever // be non-zero, so let's add an assert to ensure that. @@ -708,13 +715,19 @@ const SplitTreeSplit = extern struct { // If we're out of bounds, then we need to either set the position // to what we expect OR update our expected ratio. - const desired_pos: c_int = desired_pos: { - const max_f64: f64 = @floatFromInt(max); - break :desired_pos @intFromFloat(@round(max_f64 * desired_ratio)); - }; - paned.setPosition(desired_pos); + // If we've never set the position, then we set it to the desired. + if (!pos_set) { + const desired_pos: c_int = desired_pos: { + const max_f64: f64 = @floatFromInt(max); + break :desired_pos @intFromFloat(@round(max_f64 * desired_ratio)); + }; + paned.setPosition(desired_pos); + return 0; + } - log.warn("DESIRED={} CURRENT={}", .{ desired_ratio, current_ratio }); + // If we've set the position, then this is a manual human update + // and we need to write our update back to the tree. + tree.resizeInPlace(priv.handle, @floatCast(current_ratio)); return 0; } diff --git a/src/datastruct/split_tree.zig b/src/datastruct/split_tree.zig index cab296081..d0233986a 100644 --- a/src/datastruct/split_tree.zig +++ b/src/datastruct/split_tree.zig @@ -174,6 +174,27 @@ pub fn SplitTree(comptime V: type) type { } }; + /// Resize the given node in place. The node MUST be a split (asserted). + /// + /// In general, this is an immutable data structure so this is + /// heavily discouraged. However, this is provided for convenience + /// and performance reasons where its very important for GUIs to + /// update the ratio during a live resize than to redraw the entire + /// widget tree. + pub fn resizeInPlace( + self: *Self, + at: Node.Handle, + ratio: f16, + ) void { + // Let's talk about this constCast. Our member are const but + // we actually always own their memory. We don't want consumers + // who directly access the nodes to be able to modify them + // (without nasty stuff like this), but given this is internal + // usage its perfectly fine to modify the node in-place. + const s: *Split = @constCast(&self.nodes[at].split); + s.ratio = ratio; + } + /// Insert another tree into this tree at the given node in the /// specified direction. The other tree will be inserted in the /// new direction. For example, if the direction is "right" then From 9ad92d2c3d6c3a80b7abc2dbe9c6d7d478f38d06 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 9 Aug 2025 12:21:24 -0700 Subject: [PATCH 08/18] apprt/gtk-ng: proper split operations --- src/apprt/gtk-ng/class/split_tree.zig | 52 ++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 5 deletions(-) diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index cd03297b4..bc6bc6c3c 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -148,10 +148,10 @@ pub const SplitTree = extern struct { const actions = .{ // All of these will eventually take a target surface parameter. // For now all our targets originate from the focused surface. - .{ "new-left", actionNew, null }, - .{ "new-right", actionNew, null }, - .{ "new-up", actionNew, null }, - .{ "new-down", actionNew, null }, + .{ "new-left", actionNewLeft, null }, + .{ "new-right", actionNewRight, null }, + .{ "new-up", actionNewUp, null }, + .{ "new-down", actionNewDown, null }, }; // We need to collect our actions into a group since we're just @@ -405,7 +405,21 @@ pub const SplitTree = extern struct { //--------------------------------------------------------------- // Signal handlers - pub fn actionNew( + pub fn actionNewLeft( + _: *gio.SimpleAction, + parameter_: ?*glib.Variant, + self: *Self, + ) callconv(.c) void { + _ = parameter_; + self.newSplit( + .left, + self.getActiveSurface(), + ) catch |err| { + log.warn("new split failed error={}", .{err}); + }; + } + + pub fn actionNewRight( _: *gio.SimpleAction, parameter_: ?*glib.Variant, self: *Self, @@ -419,6 +433,34 @@ pub const SplitTree = extern struct { }; } + pub fn actionNewUp( + _: *gio.SimpleAction, + parameter_: ?*glib.Variant, + self: *Self, + ) callconv(.c) void { + _ = parameter_; + self.newSplit( + .up, + self.getActiveSurface(), + ) catch |err| { + log.warn("new split failed error={}", .{err}); + }; + } + + pub fn actionNewDown( + _: *gio.SimpleAction, + parameter_: ?*glib.Variant, + self: *Self, + ) callconv(.c) void { + _ = parameter_; + self.newSplit( + .down, + self.getActiveSurface(), + ) catch |err| { + log.warn("new split failed error={}", .{err}); + }; + } + fn propSurfaceFocused( surface: *Surface, _: *gobject.ParamSpec, From a28d6734679286ce99a037c765512eb69de086f0 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 9 Aug 2025 12:24:40 -0700 Subject: [PATCH 09/18] update supps --- valgrind.supp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/valgrind.supp b/valgrind.supp index 45a9c3ea7..cf82b7c2a 100644 --- a/valgrind.supp +++ b/valgrind.supp @@ -70,10 +70,6 @@ fun:g_signal_emit_valist fun:g_signal_emit fun:gdk_surface_handle_event - fun:gdk_wayland_event_source_dispatch - fun:g_main_context_dispatch_unlocked - fun:g_main_context_iterate_unlocked.isra.0 - fun:g_main_context_iteration ... } From 8232cf33b4420b9ec8a4d066723ae1a5059e3ccd Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 9 Aug 2025 12:36:24 -0700 Subject: [PATCH 10/18] apprt/gtk-ng: surface close in split tree --- src/apprt/gtk-ng/class/split_tree.zig | 46 +++++++++++++++++++++++++++ src/apprt/gtk-ng/class/tab.zig | 43 ++++++++----------------- src/datastruct/split_tree.zig | 3 ++ 3 files changed, 62 insertions(+), 30 deletions(-) diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index bc6bc6c3c..ae945027e 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -272,6 +272,13 @@ pub const SplitTree = extern struct { var it = tree.iterator(); while (it.next()) |entry| { const surface = entry.view; + _ = Surface.signals.@"close-request".connect( + surface, + *Self, + surfaceCloseRequest, + self, + .{}, + ); _ = gobject.Object.signals.notify.connect( surface, *Self, @@ -461,6 +468,45 @@ pub const SplitTree = extern struct { }; } + fn surfaceCloseRequest( + surface: *Surface, + scope: *const Surface.CloseScope, + self: *Self, + ) callconv(.c) void { + switch (scope.*) { + // Handled upstream... this will probably go away for widget + // actions eventually. + .window, .tab => return, + + // Remove the surface from the tree. + .surface => { + // TODO: close confirmation + + // Find the surface in the tree. + const tree = self.getTree() orelse return; + const handle: Surface.Tree.Node.Handle = handle: { + var it = tree.iterator(); + while (it.next()) |entry| { + if (entry.view == surface) break :handle entry.handle; + } + + return; + }; + + // Remove it from the tree. + var new_tree = tree.remove( + Application.default().allocator(), + handle, + ) catch |err| { + log.warn("unable to remove surface from tree: {}", .{err}); + return; + }; + defer new_tree.deinit(); + self.setTree(&new_tree); + }, + } + } + fn propSurfaceFocused( surface: *Surface, _: *gobject.ParamSpec, diff --git a/src/apprt/gtk-ng/class/tab.zig b/src/apprt/gtk-ng/class/tab.zig index 4b75701bf..964f6fc3e 100644 --- a/src/apprt/gtk-ng/class/tab.zig +++ b/src/apprt/gtk-ng/class/tab.zig @@ -182,13 +182,6 @@ pub const Tab = extern struct { var it = tree.iterator(); while (it.next()) |entry| { const surface = entry.view; - _ = Surface.signals.@"close-request".connect( - surface, - *Self, - surfaceCloseRequest, - self, - .{}, - ); _ = gobject.Object.signals.notify.connect( surface, *Self, @@ -285,27 +278,6 @@ pub const Tab = extern struct { //--------------------------------------------------------------- // Signal handlers - fn surfaceCloseRequest( - _: *Surface, - scope: *const Surface.CloseScope, - self: *Self, - ) callconv(.c) void { - switch (scope.*) { - // Handled upstream... we don't control our window close. - .window => return, - - // Presently both the same, results in the tab closing. - .surface, .tab => { - signals.@"close-request".impl.emit( - self, - null, - .{}, - null, - ); - }, - } - } - fn splitTreeChanged( _: *SplitTree, old_tree: ?*const Surface.Tree, @@ -316,9 +288,20 @@ pub const Tab = extern struct { self.disconnectSurfaceHandlers(tree); } - if (new_tree) |tree| { - self.connectSurfaceHandlers(tree); + // If our tree is empty we close the tab. + const tree: *const Surface.Tree = new_tree orelse &.empty; + if (tree.isEmpty()) { + signals.@"close-request".impl.emit( + self, + null, + .{}, + null, + ); + return; } + + // Non-empty tree, connect handlers we care about. + self.connectSurfaceHandlers(tree); } fn propSplitTree( diff --git a/src/datastruct/split_tree.zig b/src/datastruct/split_tree.zig index d0233986a..a1f2de035 100644 --- a/src/datastruct/split_tree.zig +++ b/src/datastruct/split_tree.zig @@ -119,6 +119,9 @@ pub fn SplitTree(comptime V: type) type { /// Clone this tree, returning a new tree with the same nodes. pub fn clone(self: *const Self, gpa: Allocator) Allocator.Error!Self { + // If we're empty then return an empty tree. + if (self.isEmpty()) return .empty; + // Create a new arena allocator for the clone. var arena = ArenaAllocator.init(gpa); errdefer arena.deinit(); From ec293c1fd0eeebe7ce72e62c088199937f375877 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 9 Aug 2025 13:49:25 -0700 Subject: [PATCH 11/18] apprt/gtk-ng: active surface hookups --- src/apprt/gtk-ng/class/split_tree.zig | 10 ++++- src/apprt/gtk-ng/class/tab.zig | 54 +-------------------------- src/apprt/gtk-ng/ui/1.5/tab.blp | 2 +- src/datastruct/split_tree.zig | 18 +++++++++ 4 files changed, 29 insertions(+), 55 deletions(-) diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index ae945027e..b0c1a34a3 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -49,8 +49,6 @@ pub const SplitTree = extern struct { Self, ?*Surface, .{ - .nick = "Active Surface", - .blurb = "The currently active surface.", .accessor = gobject.ext.typedAccessor( Self, ?*Surface, @@ -481,6 +479,7 @@ pub const SplitTree = extern struct { // Remove the surface from the tree. .surface => { // TODO: close confirmation + // TODO: invalid free on final close // Find the surface in the tree. const tree = self.getTree() orelse return; @@ -517,6 +516,9 @@ pub const SplitTree = extern struct { // the surface is destroyed. if (!surface.getFocused()) return; self.private().last_focused.set(surface); + + // Our active surface probably changed + self.as(gobject.Object).notifyByPspec(properties.@"active-surface".impl.param_spec); } fn propTree( @@ -566,6 +568,9 @@ pub const SplitTree = extern struct { v.grabFocus(); } + // Our active surface may have changed + self.as(gobject.Object).notifyByPspec(properties.@"active-surface".impl.param_spec); + return 0; } @@ -616,6 +621,7 @@ pub const SplitTree = extern struct { // Properties gobject.ext.registerProperties(class, &.{ + properties.@"active-surface".impl, properties.@"has-surfaces".impl, properties.tree.impl, }); diff --git a/src/apprt/gtk-ng/class/tab.zig b/src/apprt/gtk-ng/class/tab.zig index 964f6fc3e..051807071 100644 --- a/src/apprt/gtk-ng/class/tab.zig +++ b/src/apprt/gtk-ng/class/tab.zig @@ -175,42 +175,6 @@ pub const Tab = extern struct { }; } - fn connectSurfaceHandlers( - self: *Self, - tree: *const Surface.Tree, - ) void { - var it = tree.iterator(); - while (it.next()) |entry| { - const surface = entry.view; - _ = gobject.Object.signals.notify.connect( - surface, - *Self, - propSurfaceFocused, - self, - .{ .detail = "focused" }, - ); - } - } - - fn disconnectSurfaceHandlers( - self: *Self, - tree: *const Surface.Tree, - ) void { - var it = tree.iterator(); - while (it.next()) |entry| { - const surface = entry.view; - _ = gobject.signalHandlersDisconnectMatched( - surface.as(gobject.Object), - .{ .data = true }, - 0, - 0, - null, - null, - self, - ); - } - } - //--------------------------------------------------------------- // Properties @@ -280,14 +244,10 @@ pub const Tab = extern struct { fn splitTreeChanged( _: *SplitTree, - old_tree: ?*const Surface.Tree, + _: ?*const Surface.Tree, new_tree: ?*const Surface.Tree, self: *Self, ) callconv(.c) void { - if (old_tree) |tree| { - self.disconnectSurfaceHandlers(tree); - } - // If our tree is empty we close the tab. const tree: *const Surface.Tree = new_tree orelse &.empty; if (tree.isEmpty()) { @@ -299,9 +259,6 @@ pub const Tab = extern struct { ); return; } - - // Non-empty tree, connect handlers we care about. - self.connectSurfaceHandlers(tree); } fn propSplitTree( @@ -313,7 +270,7 @@ pub const Tab = extern struct { } fn propActiveSurface( - _: *Self, + _: *SplitTree, _: *gobject.ParamSpec, self: *Self, ) callconv(.c) void { @@ -322,14 +279,7 @@ pub const Tab = extern struct { if (self.getActiveSurface()) |surface| { priv.surface_bindings.setSource(surface.as(gobject.Object)); } - } - fn propSurfaceFocused( - surface: *Surface, - _: *gobject.ParamSpec, - self: *Self, - ) callconv(.c) void { - if (!surface.getFocused()) return; self.as(gobject.Object).notifyByPspec(properties.@"active-surface".impl.param_spec); } diff --git a/src/apprt/gtk-ng/ui/1.5/tab.blp b/src/apprt/gtk-ng/ui/1.5/tab.blp index 61f106ce1..70e4f709b 100644 --- a/src/apprt/gtk-ng/ui/1.5/tab.blp +++ b/src/apprt/gtk-ng/ui/1.5/tab.blp @@ -5,12 +5,12 @@ template $GhosttyTab: Box { "tab", ] - notify::active-surface => $notify_active_surface(); orientation: vertical; hexpand: true; vexpand: true; $GhosttySplitTree split_tree { + notify::active-surface => $notify_active_surface(); notify::tree => $notify_tree(); changed => $tree_changed(); } diff --git a/src/datastruct/split_tree.zig b/src/datastruct/split_tree.zig index a1f2de035..48b887707 100644 --- a/src/datastruct/split_tree.zig +++ b/src/datastruct/split_tree.zig @@ -1151,3 +1151,21 @@ test "SplitTree: split twice, remove intermediary" { t.deinit(); } } + +test "SplitTree: clone empty tree" { + const testing = std.testing; + const alloc = testing.allocator; + var t: TestTree = .empty; + defer t.deinit(); + + var t2 = try t.clone(alloc); + defer t2.deinit(); + + { + const str = try std.fmt.allocPrint(alloc, "{}", .{t2}); + defer alloc.free(str); + try testing.expectEqualStrings(str, + \\empty + ); + } +} From aed6a3a343fc9621c74a10fceee530740ec8b183 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 9 Aug 2025 14:19:42 -0700 Subject: [PATCH 12/18] apprt/gtk-ng: clean up some changed handlers --- src/apprt/gtk-ng/class/split_tree.zig | 13 ++++++++++++- src/apprt/gtk-ng/class/tab.zig | 18 +++++------------- src/apprt/gtk-ng/ui/1.5/tab.blp | 1 - src/datastruct/split_tree.zig | 6 ++++-- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index b0c1a34a3..1cfaeb669 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -330,9 +330,18 @@ pub const SplitTree = extern struct { /// Set the tree data model that we're showing in this widget. This /// will clone the given tree. - pub fn setTree(self: *Self, tree: ?*const Surface.Tree) void { + pub fn setTree(self: *Self, tree_: ?*const Surface.Tree) void { const priv = self.private(); + // We always normalize our tree parameter so that empty trees + // become null so that we don't have to deal with callers being + // confused about that. + const tree: ?*const Surface.Tree = tree: { + const tree = tree_ orelse break :tree null; + if (tree.isEmpty()) break :tree null; + break :tree tree; + }; + // Emit the signal so that handlers can witness both the before and // after values of the tree. signals.changed.impl.emit( @@ -349,6 +358,8 @@ pub const SplitTree = extern struct { } if (tree) |new_tree| { + assert(priv.tree == null); + assert(!new_tree.isEmpty()); priv.tree = ext.boxedCopy(Surface.Tree, new_tree); self.connectSurfaceHandlers(); } diff --git a/src/apprt/gtk-ng/class/tab.zig b/src/apprt/gtk-ng/class/tab.zig index 051807071..428ce72d6 100644 --- a/src/apprt/gtk-ng/class/tab.zig +++ b/src/apprt/gtk-ng/class/tab.zig @@ -242,14 +242,15 @@ pub const Tab = extern struct { //--------------------------------------------------------------- // Signal handlers - fn splitTreeChanged( + fn propSplitTree( _: *SplitTree, - _: ?*const Surface.Tree, - new_tree: ?*const Surface.Tree, + _: *gobject.ParamSpec, self: *Self, ) callconv(.c) void { + self.as(gobject.Object).notifyByPspec(properties.@"surface-tree".impl.param_spec); + // If our tree is empty we close the tab. - const tree: *const Surface.Tree = new_tree orelse &.empty; + const tree: *const Surface.Tree = self.getSurfaceTree() orelse &.empty; if (tree.isEmpty()) { signals.@"close-request".impl.emit( self, @@ -261,14 +262,6 @@ pub const Tab = extern struct { } } - fn propSplitTree( - _: *SplitTree, - _: *gobject.ParamSpec, - self: *Self, - ) callconv(.c) void { - self.as(gobject.Object).notifyByPspec(properties.@"surface-tree".impl.param_spec); - } - fn propActiveSurface( _: *SplitTree, _: *gobject.ParamSpec, @@ -318,7 +311,6 @@ pub const Tab = extern struct { class.bindTemplateChildPrivate("split_tree", .{}); // Template Callbacks - class.bindTemplateCallback("tree_changed", &splitTreeChanged); class.bindTemplateCallback("notify_active_surface", &propActiveSurface); class.bindTemplateCallback("notify_tree", &propSplitTree); diff --git a/src/apprt/gtk-ng/ui/1.5/tab.blp b/src/apprt/gtk-ng/ui/1.5/tab.blp index 70e4f709b..4cb47487d 100644 --- a/src/apprt/gtk-ng/ui/1.5/tab.blp +++ b/src/apprt/gtk-ng/ui/1.5/tab.blp @@ -12,6 +12,5 @@ template $GhosttyTab: Box { $GhosttySplitTree split_tree { notify::active-surface => $notify_active_surface(); notify::tree => $notify_tree(); - changed => $tree_changed(); } } diff --git a/src/datastruct/split_tree.zig b/src/datastruct/split_tree.zig index 48b887707..14ef6370e 100644 --- a/src/datastruct/split_tree.zig +++ b/src/datastruct/split_tree.zig @@ -826,8 +826,10 @@ pub fn SplitTree(comptime V: type) type { .copy = &struct { fn copy(self: *Self) callconv(.c) *Self { const ptr = @import("glib").ext.create(Self); - const alloc = self.arena.child_allocator; - ptr.* = self.clone(alloc) catch @panic("oom"); + ptr.* = if (self.nodes.len == 0) + .empty + else + self.clone(self.arena.child_allocator) catch @panic("oom"); return ptr; } }.copy, From e682e99bf5934d5f1031966a07a21245e066fbc4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 9 Aug 2025 14:26:57 -0700 Subject: [PATCH 13/18] apprt/gtk-ng: hook up win split actions --- src/apprt/gtk-ng/class/application.zig | 25 +++++++++++++++++- src/apprt/gtk-ng/class/split_tree.zig | 1 - src/apprt/gtk-ng/class/window.zig | 36 ++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/apprt/gtk-ng/class/application.zig b/src/apprt/gtk-ng/class/application.zig index 226d7c56b..523a98033 100644 --- a/src/apprt/gtk-ng/class/application.zig +++ b/src/apprt/gtk-ng/class/application.zig @@ -562,6 +562,8 @@ pub const Application = extern struct { .move_tab => return Action.moveTab(target, value), + .new_split => return Action.newSplit(target, value), + .new_tab => return Action.newTab(target), .new_window => try Action.newWindow( @@ -611,7 +613,6 @@ pub const Application = extern struct { .prompt_title, .inspector, // TODO: splits - .new_split, .resize_split, .equalize_splits, .goto_split, @@ -1747,6 +1748,28 @@ const Action = struct { } } + pub fn newSplit( + target: apprt.Target, + direction: apprt.action.SplitDirection, + ) bool { + switch (target) { + .app => { + log.warn("new split to app is unexpected", .{}); + return false; + }, + + .surface => |core| { + const surface = core.rt_surface.surface; + return surface.as(gtk.Widget).activateAction(switch (direction) { + .right => "split-tree.new-right", + .left => "split-tree.new-left", + .down => "split-tree.new-down", + .up => "split-tree.new-up", + }, null) != 0; + }, + } + } + pub fn newTab(target: apprt.Target) bool { switch (target) { .app => { diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index 1cfaeb669..62caf7108 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -490,7 +490,6 @@ pub const SplitTree = extern struct { // Remove the surface from the tree. .surface => { // TODO: close confirmation - // TODO: invalid free on final close // Find the surface in the tree. const tree = self.getTree() orelse return; diff --git a/src/apprt/gtk-ng/class/window.zig b/src/apprt/gtk-ng/class/window.zig index 739405961..f3e8ee129 100644 --- a/src/apprt/gtk-ng/class/window.zig +++ b/src/apprt/gtk-ng/class/window.zig @@ -335,6 +335,10 @@ pub const Window = extern struct { .{ "close-tab", actionCloseTab, null }, .{ "new-tab", actionNewTab, null }, .{ "new-window", actionNewWindow, null }, + .{ "split-right", actionSplitRight, null }, + .{ "split-left", actionSplitLeft, null }, + .{ "split-up", actionSplitUp, null }, + .{ "split-down", actionSplitDown, null }, .{ "copy", actionCopy, null }, .{ "paste", actionPaste, null }, .{ "reset", actionReset, null }, @@ -1650,6 +1654,38 @@ pub const Window = extern struct { self.performBindingAction(.new_tab); } + fn actionSplitRight( + _: *gio.SimpleAction, + _: ?*glib.Variant, + self: *Window, + ) callconv(.c) void { + self.performBindingAction(.{ .new_split = .right }); + } + + fn actionSplitLeft( + _: *gio.SimpleAction, + _: ?*glib.Variant, + self: *Window, + ) callconv(.c) void { + self.performBindingAction(.{ .new_split = .left }); + } + + fn actionSplitUp( + _: *gio.SimpleAction, + _: ?*glib.Variant, + self: *Window, + ) callconv(.c) void { + self.performBindingAction(.{ .new_split = .up }); + } + + fn actionSplitDown( + _: *gio.SimpleAction, + _: ?*glib.Variant, + self: *Window, + ) callconv(.c) void { + self.performBindingAction(.{ .new_split = .down }); + } + fn actionCopy( _: *gio.SimpleAction, _: ?*glib.Variant, From b1da644b62a780bb0f2624f57cb3344473eb6220 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 9 Aug 2025 14:43:34 -0700 Subject: [PATCH 14/18] apprt/gtk-ng: unnecessary grab focus --- src/apprt/gtk-ng/class/split_tree.zig | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index 62caf7108..145730989 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -243,9 +243,6 @@ pub const SplitTree = extern struct { // Replace our tree self.setTree(&new_tree); - - // Focus our new surface - surface.grabFocus(); } fn disconnectSurfaceHandlers(self: *Self) void { From 46560d00182c174002ce608563a5a163e9cb7f9d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 10 Aug 2025 07:30:06 -0700 Subject: [PATCH 15/18] apprt/gtk-ng: wait for unparent to rebuild split tree --- src/apprt/gtk-ng/class/split_tree.zig | 96 ++++++++++++++++++++++++--- 1 file changed, 85 insertions(+), 11 deletions(-) diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index 145730989..e547fa6d7 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -125,6 +125,11 @@ pub const SplitTree = extern struct { /// used to debounce updates. rebuild_source: ?c_uint = null, + /// Tracks whether we want a rebuild to happen at the next tick + /// that our surface tree has no surfaces with parents. See the + /// propTree function for a lot more details. + rebuild_pending: bool, + pub var offset: c_int = 0; }; @@ -281,6 +286,13 @@ pub const SplitTree = extern struct { self, .{ .detail = "focused" }, ); + _ = gobject.Object.signals.notify.connect( + surface.as(gtk.Widget), + *Self, + propSurfaceParent, + self, + .{ .detail = "parent" }, + ); } } @@ -314,6 +326,20 @@ pub const SplitTree = extern struct { return surface; } + /// Returns whether any of the surfaces in the tree have a parent. + /// This is important because we can only rebuild the widget tree + /// when every surface has no parent. + fn getTreeHasParents(self: *Self) bool { + const tree: *const Surface.Tree = self.getTree() orelse &.empty; + var it = tree.iterator(); + while (it.next()) |entry| { + const surface = entry.view; + if (surface.as(gtk.Widget).getParent() != null) return true; + } + + return false; + } + pub fn getHasSurfaces(self: *Self) bool { const tree: *const Surface.Tree = self.private().tree orelse &.empty; return !tree.isEmpty(); @@ -528,6 +554,27 @@ pub const SplitTree = extern struct { self.as(gobject.Object).notifyByPspec(properties.@"active-surface".impl.param_spec); } + fn propSurfaceParent( + _: *gtk.Widget, + _: *gobject.ParamSpec, + self: *Self, + ) callconv(.c) void { + const priv = self.private(); + + // If we're not waiting to rebuild then ignore this. + if (!priv.rebuild_pending) return; + + // If any parents still exist in our tree then don't do anything. + if (self.getTreeHasParents()) return; + + // Schedule the rebuild. Note, I tried to do this immediately (not + // on an idle tick) and it didn't work and had obvious rendering + // glitches. Something to look into in the future. + assert(priv.rebuild_source == null); + priv.rebuild_pending = false; + priv.rebuild_source = glib.idleAdd(onRebuild, self); + } + fn propTree( self: *Self, _: *gobject.ParamSpec, @@ -535,20 +582,43 @@ pub const SplitTree = extern struct { ) callconv(.c) void { const priv = self.private(); - // We need to reset our tree and create the new widget hierarchy - // on two separate event loop ticks to allow GTK to properly relayout - // our widgets. + // If we were planning a rebuild, always remove that so we can + // start from a clean slate. + if (priv.rebuild_source) |v| { + if (glib.Source.remove(v) == 0) { + log.warn("unable to remove rebuild source", .{}); + } + priv.rebuild_source = null; + } + + // We need to wait for all our previous surfaces to lose their + // parent before adding them to a new one. I'm not sure if its a GTK + // bug, but manually forcing an unparent of all prior surfaces AND + // adding them to a new parent in the same tick causes the GLArea + // to break (it seems). I didn't investigate too deeply. // - // Doing this all at once will cause strange rendering glitches, - // the glarea to be gone forever (but not deallocated), etc. I think - // this is probably a bug in GTK we can minimize and report later. + // Note, we also can't just defer to an idle tick (via idleAdd) because + // sometimes it takes more than one tick for all our surfaces to + // lose their parent. // - // Using an idle callback also allows us to debounce updates. + // To work around this issue, if we have any surfaces that have + // a parent, we set the build pending flag and wait for the tree + // to be fully parent-free before building. + priv.rebuild_pending = self.getTreeHasParents(); + + // Reset our prior bin. This will force all prior surfaces to + // unparent... eventually. priv.tree_bin.setChild(null); - if (priv.rebuild_source == null) priv.rebuild_source = glib.idleAdd( - onRebuild, - self, - ); + + // If none of the surfaces we plan on drawing require an unparent + // then we can setup our tree immediately. Otherwise, it'll happen + // via the `propSurfaceParent` callback. + if (!priv.rebuild_pending and priv.rebuild_source == null) { + priv.rebuild_source = glib.idleAdd( + onRebuild, + self, + ); + } // Dependent properties self.as(gobject.Object).notifyByPspec(properties.@"has-surfaces".impl.param_spec); @@ -561,6 +631,10 @@ pub const SplitTree = extern struct { const priv = self.private(); priv.rebuild_source = null; + // Prior to rebuilding the tree, our surface tree must be + // comprised of fully orphaned surfaces. + assert(!self.getTreeHasParents()); + // Rebuild our tree const tree: *const Surface.Tree = self.private().tree orelse &.empty; if (!tree.isEmpty()) { From 441af8389b9227eb74fd75e89dd7f14ebce6a69c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 10 Aug 2025 13:05:17 -0700 Subject: [PATCH 16/18] apprt/gtk-ng: split separator styling --- src/apprt/gtk-ng/css/style-dark.css | 5 +++++ src/apprt/gtk-ng/css/style.css | 23 ++++++++++++++++++++ src/apprt/gtk-ng/ui/1.5/split-tree-split.blp | 4 ++++ 3 files changed, 32 insertions(+) diff --git a/src/apprt/gtk-ng/css/style-dark.css b/src/apprt/gtk-ng/css/style-dark.css index a9aa2dcc0..f13b4f4f0 100644 --- a/src/apprt/gtk-ng/css/style-dark.css +++ b/src/apprt/gtk-ng/css/style-dark.css @@ -1,3 +1,8 @@ .transparent { background-color: transparent; } + +.window .split paned > separator { + background-color: rgba(36, 36, 36, 1); + background-clip: content-box; +} diff --git a/src/apprt/gtk-ng/css/style.css b/src/apprt/gtk-ng/css/style.css index 970c91b03..a1a425f66 100644 --- a/src/apprt/gtk-ng/css/style.css +++ b/src/apprt/gtk-ng/css/style.css @@ -114,3 +114,26 @@ label.resize-overlay { margin-left: 4px; margin-right: 8px; } + +/* + * Splits + */ + +.window .split paned > separator { + background-color: rgba(250, 250, 250, 1); + background-clip: content-box; + + /* This works around the oversized drag area for the right side of GtkPaned. + * + * Upstream Gtk issue: + * https://gitlab.gnome.org/GNOME/gtk/-/issues/4484#note_2362002 + * + * Ghostty issue: + * https://github.com/ghostty-org/ghostty/issues/3020 + * + * Without this, it's not possible to select the first character on the + * right-hand side of a split. + */ + margin: 0; + padding: 0; +} diff --git a/src/apprt/gtk-ng/ui/1.5/split-tree-split.blp b/src/apprt/gtk-ng/ui/1.5/split-tree-split.blp index aa194e8e8..182919f4e 100644 --- a/src/apprt/gtk-ng/ui/1.5/split-tree-split.blp +++ b/src/apprt/gtk-ng/ui/1.5/split-tree-split.blp @@ -2,6 +2,10 @@ using Gtk 4.0; using Adw 1; template $GhosttySplitTreeSplit: Adw.Bin { + styles [ + "split", + ] + // The double-nesting is required due to a GTK bug where you can't // bind the first child of a builder layout. If you do, you get a double // dispose. Easiest way to see that is simply remove this and see the From ca4e38ff03b8e45edce0b3817c678ead264a40d1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 10 Aug 2025 13:35:26 -0700 Subject: [PATCH 17/18] apprt/gtk-ng: split close confirmation --- .../class/close_confirmation_dialog.zig | 4 + src/apprt/gtk-ng/class/split_tree.zig | 96 ++++++++++++++----- 2 files changed, 75 insertions(+), 25 deletions(-) diff --git a/src/apprt/gtk-ng/class/close_confirmation_dialog.zig b/src/apprt/gtk-ng/class/close_confirmation_dialog.zig index 210533c1c..3debafbb5 100644 --- a/src/apprt/gtk-ng/class/close_confirmation_dialog.zig +++ b/src/apprt/gtk-ng/class/close_confirmation_dialog.zig @@ -133,6 +133,7 @@ pub const CloseConfirmationDialog = extern struct { const C = Common(Self, Private); pub const as = C.as; pub const ref = C.ref; + pub const refSink = C.refSink; pub const unref = C.unref; const private = C.private; @@ -179,12 +180,14 @@ pub const Target = enum(c_int) { app, tab, window, + surface, pub fn title(self: Target) [*:0]const u8 { return switch (self) { .app => i18n._("Quit Ghostty?"), .tab => i18n._("Close Tab?"), .window => i18n._("Close Window?"), + .surface => i18n._("Close Split?"), }; } @@ -193,6 +196,7 @@ pub const Target = enum(c_int) { .app => i18n._("All terminal sessions will be terminated."), .tab => i18n._("All terminal sessions in this tab will be terminated."), .window => i18n._("All terminal sessions in this window will be terminated."), + .surface => i18n._("The currently running process in this split will be terminated."), }; } diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index e547fa6d7..b70f0e2f8 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -130,6 +130,10 @@ pub const SplitTree = extern struct { /// propTree function for a lot more details. rebuild_pending: bool, + /// Used to store state about a pending surface close for the + /// close dialog. + pending_close: ?Surface.Tree.Node.Handle, + pub var offset: c_int = 0; }; @@ -138,6 +142,10 @@ pub const SplitTree = extern struct { // Initialize our actions self.initActions(); + + // Initialize some basic state + const priv = self.private(); + priv.pending_close = null; } fn initActions(self: *Self) void { @@ -511,32 +519,70 @@ pub const SplitTree = extern struct { .window, .tab => return, // Remove the surface from the tree. - .surface => { - // TODO: close confirmation - - // Find the surface in the tree. - const tree = self.getTree() orelse return; - const handle: Surface.Tree.Node.Handle = handle: { - var it = tree.iterator(); - while (it.next()) |entry| { - if (entry.view == surface) break :handle entry.handle; - } - - return; - }; - - // Remove it from the tree. - var new_tree = tree.remove( - Application.default().allocator(), - handle, - ) catch |err| { - log.warn("unable to remove surface from tree: {}", .{err}); - return; - }; - defer new_tree.deinit(); - self.setTree(&new_tree); - }, + .surface => {}, } + + const core = surface.core() orelse return; + + // Reset our pending close state + const priv = self.private(); + priv.pending_close = null; + + // Find the surface in the tree to verify this is valid and + // set our pending close handle. + priv.pending_close = handle: { + const tree = self.getTree() orelse return; + var it = tree.iterator(); + while (it.next()) |entry| { + if (entry.view == surface) { + break :handle entry.handle; + } + } + + return; + }; + + // If we don't need to confirm then just close immediately. + if (!core.needsConfirmQuit()) { + closeConfirmationClose( + null, + self, + ); + return; + } + + // Show a confirmation dialog + const dialog: *CloseConfirmationDialog = .new(.surface); + _ = CloseConfirmationDialog.signals.@"close-request".connect( + dialog, + *Self, + closeConfirmationClose, + self, + .{}, + ); + dialog.present(self.as(gtk.Widget)); + } + + fn closeConfirmationClose( + _: ?*CloseConfirmationDialog, + self: *Self, + ) callconv(.c) void { + // Get the handle we're closing + const priv = self.private(); + const handle = priv.pending_close orelse return; + priv.pending_close = null; + + // Remove it from the tree. + const old_tree = self.getTree() orelse return; + var new_tree = old_tree.remove( + Application.default().allocator(), + handle, + ) catch |err| { + log.warn("unable to remove surface from tree: {}", .{err}); + return; + }; + defer new_tree.deinit(); + self.setTree(&new_tree); } fn propSurfaceFocused( From aba5a34335406e620645035b9eb7cbb21126e751 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Sun, 10 Aug 2025 21:29:46 -0500 Subject: [PATCH 18/18] gtk-ng: sync action accelerators for split-tree --- src/apprt/gtk-ng/class/application.zig | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/apprt/gtk-ng/class/application.zig b/src/apprt/gtk-ng/class/application.zig index 523a98033..7dde0fa93 100644 --- a/src/apprt/gtk-ng/class/application.zig +++ b/src/apprt/gtk-ng/class/application.zig @@ -882,6 +882,10 @@ pub const Application = extern struct { self.syncActionAccelerator("win.reset", .{ .reset = {} }); self.syncActionAccelerator("win.clear", .{ .clear_screen = {} }); self.syncActionAccelerator("win.prompt-title", .{ .prompt_surface_title = {} }); + self.syncActionAccelerator("split-tree.new-left", .{ .new_split = .left }); + self.syncActionAccelerator("split-tree.new-right", .{ .new_split = .right }); + self.syncActionAccelerator("split-tree.new-up", .{ .new_split = .up }); + self.syncActionAccelerator("split-tree.new-down", .{ .new_split = .down }); } fn syncActionAccelerator(