Investigate ghostty issue 8208 using gh CLI

Medium
Unlisted
Use `gh` to look at https://github.com/ghostty-org/ghostty/issues/8208 and determine a fix.
Ran 1 command (1 cancelled)
Don't run full tests

Implemented a concrete fix for issue #8208 after inspecting it with gh: the GTK split tree no longer clears to an empty state while waiting for asynchronous unparenting, and it now reuses leaf widgets across rebuilds to avoid the flicker path.

  1. In src/apprt/gtk/class/split_tree.zig, I removed the rebuild_pending/parent-notification waiting flow and switched propTree to a debounced idle rebuild that keeps the existing tree attached until replacement time.
  2. In onRebuild, I added tracking for reused widgets and changed rebuild to atomically replace tree_bin with the newly built root (or null only for empty trees), instead of forcing an intermediate blank frame.
  3. In buildTree, leaf nodes now try to reuse an existing SurfaceScrolledWindow ancestor for each surface, detaching and reparenting that widget rather than recreating it every change; this avoids the problematic full surface reparent churn that caused visible flicker.
  4. I also removed now-obsolete parent-change wiring and helper code tied to the old pending-rebuild mechanism in connectSurfaceHandlers/private state.

Validation run:

  1. zig build passed.
  2. I did not run zig build test (full suite), per your request.
Please explain the concrete fix here in more detail so I can better understand as I review

What Changed, Concretely

  1. The rebuild trigger in split_tree.zig was changed so we no longer clear tree_bin immediately when tree changes. Instead, we debounce to one idle callback and keep the old widget tree visible until rebuild happens.
  2. The old “wait for every surface to become parentless” mechanism was removed from SplitTree private state and signal wiring, including rebuild_pending, propSurfaceParent, and parent-notify hookups.
  3. In onRebuild, rebuild now happens in one pass, and the bin child is swapped atomically to the new root widget tree.
  4. In buildTree, leaf handling now tries to reuse the existing SurfaceScrolledWindow widget for each surface, instead of creating a new wrapper every time.
  5. The new detachWidget helper safely detaches reused widgets from their old parent (GtkPaned or AdwBin) before attaching them into the new split tree.
  6. Reused widgets are temporarily ref-count protected in onRebuild so removing them from old parents cannot destroy them mid-rebuild.

Why This Reduces Flicker

  1. The previous flow had an intentional “empty state” window: clear old tree now, rebuild later after unparent settles. That empty window is what users were seeing as flicker.
  2. The new flow keeps old content on screen until replacement, so there is no extra frame where the split area is blank.
  3. The key behavior change is that we now reparent wrapper widgets (SurfaceScrolledWindow) rather than repeatedly rebuilding wrappers around surfaces. This avoids the fragile surface reparent timing that previously required multi-tick waiting.

Mental Model While Reviewing

  1. Data model changes (new split, close split, resize, equalize) still call setTree exactly as before.
  2. setTree still emits notify::tree, which now just schedules one rebuild idle.
  3. Rebuild walks the new tree and, for each leaf, either reuses existing UI widget or creates a new one if it’s a genuinely new surface.
  4. Once the new root is built, tree_bin.setChild(new_root) swaps it in one step.
  5. Focus restore logic remains in place after swap, so active-surface behavior stays stable.

Verification Scope

  1. I only ran zig build (passes).
  2. I did not run full tests, per your instruction.
Use `jj desc` to write a commit message explaining the fix. Copy the style of other recent fixes by mitchellh

Updated with jj desc in Mitchell-style scope format.

  1. New subject: apprt/gtk: reduce split-tree flicker by reusing leaf widgets (#8208)
  2. Body explains the old blank-frame path (tree_bin cleared while waiting for parentless surfaces), the new atomic-swap rebuild approach, and leaf widget reuse via detach/reparent.
  3. Current working-copy commit is now dae0787f with that description.
Why is detachWidget required in the way it is? Why can't we just unparent? Please also add comments to the code itself explaining this

Why detachWidget Uses Parent APIs

detachWidget in split_tree.zig is written this way because plain widget.unparent() is a low-level fallback, not the normal container-removal path.

  1. GtkPaned and AdwBin maintain their own internal child-slot state (start-child, end-child, child) and property notifications.
  2. Their set*Child(null) APIs update those internal pointers and emit the expected container-side updates.
  3. If we only call unparent() directly, the child may detach but the container can be left with stale slot state/invariants, which is exactly what we want to avoid while aggressively reparenting during split rebuilds.
  4. That’s why we do typed removal first (paned.setStartChild(null), paned.setEndChild(null), bin.setChild(null)), and only use widget.unparent() as a fallback for unexpected parent types.

Comments Added

I added explicit comments in detachWidget explaining this rationale inline, including why parent-specific APIs are preferred and why unparent() is only a fallback.

zig fmt and zig build both pass after this comment update.

Is there a better way to handle the append failure in buildTree? Can we fallback to the prior approach or something so we don't have a dangling ref?

Yes — there’s a better approach than handling append failure, and I implemented it.

  1. I removed the ArrayListUnmanaged.append tracking path entirely from buildTree/onRebuild, so there is no allocation failure point there anymore.
  2. buildTree now returns a small result struct (widget + release_ref) in BuildTreeResult.
  3. For reused leaf widgets, we still take a temporary ref before detaching, but now we mark release_ref = true and release it deterministically after the widget is attached to its new parent in the same recursion flow in buildTree.
  4. Root-level attach does the same in onRebuild: set child first, then release temporary ref if needed.

So instead of “fallback to prior approach on append failure,” the fix is to eliminate that failure mode entirely. This avoids dangling refs and avoids the old leak log path.

I also kept/additionally clarified code comments where this matters: reference semantics in buildTree and parent-specific detach behavior in detachWidget.

Validation run: zig fmt src/apprt/gtk/class/split_tree.zig and zig build both pass.

I'm getting this on graceful exit after closing all splits: thread 40148 panic: reached unreachable code /nix/store/b4ycvfb7zv8k401161d5grv436aczdh5-zig-0.15.2/lib/std/debug.zig:559:14: 0x2a91b5f in assert (ghostty) if (!ok) unreachable; // assertion failure ^ /home/mitchellh/Documents/ghostty/src/App.zig:114:11: 0x2cdb46f in deinit (ghostty) assert(self.font_grid_set.count() == 0); ^ /home/mitchellh/Documents/ghostty/src/App.zig:120:16: 0x2c50f47 in destroy (ghostty) self.deinit(); ^ /home/mitchellh/Documents/ghostty/src/main_ghostty.zig:98:22: 0x2c50ae3 in main (ghostty) defer app.destroy(); ^ /nix/store/b4ycvfb7zv8k401161d5grv436aczdh5-zig-0.15.2/lib/std/start.zig:627:37: 0x2c512c7 in main (ghostty) const result = root.main() catch |err| { ^ ???:?:?: 0xffff88f362b3 in ??? (libc.so.6) Unwind information for `libc.so.6:0xffff88f362b3` was not available, trace may be incomplete ???:?:?: 0xffff88f36397 in ??? (libc.so.6) ???:?:?: 0x275a66f in ??? (???) run

Root cause was in the new debounced rebuild behavior: when the split tree became empty, we were still waiting for an idle callback before clearing the old widget tree.

That meant the last Surface could stay alive if shutdown happened before the idle ran, which leaves entries in font_grid_set and trips the assert you hit in src/App.zig.

I patched propTree in split_tree.zig to special-case empty/null trees:

  1. If priv.tree == null, call priv.tree_bin.setChild(null) immediately.
  2. Notify dependent properties and return without scheduling idle rebuild.
  3. Keep the debounced idle path only for non-empty tree-to-tree transitions.

That preserves the anti-flicker behavior for normal split updates, but ensures teardown is immediate when the last split closes, so shutdown doesn’t retain a stale surface.

Validation run: zig build passed.

I did some manual cleanups, do you see any further issues related to the changes?

I’ll inspect the current working diff and re-check the split-tree rebuild/lifetime paths for subtle regressions (especially ref ownership and shutdown ordering), then I’ll report any concrete issues I find.

Ran 6 searches, read 7 files, ran 1 command

Findings

  1. No further correctness issues stood out in the current version of src/apprt/gtk/class/split_tree.zig after re-checking the updated rebuild flow in propTree, ref handoff in onRebuild/buildTree, and parent-specific detach logic in detachWidget.

  2. The main risk area (temporary refs during reuse) now looks sound: refs are acquired in initReused, and deterministically released via deinit after the widget is attached to the new parent.

  3. The shutdown regression you hit is addressed by the empty-tree fast path in [propTree](file:///home/mitchellh/Documents/ghostty/src/apprt/gtk/class/split_tree.zig#