From a3c041bcb4de25fbbfdbba0ddcb2f49106ce6957 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 8 Aug 2025 15:03:28 -0700 Subject: [PATCH] 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 ... }