Investigate ghostty issue 8208 using gh CLI
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.
- In src/apprt/gtk/class/split_tree.zig, I removed the
rebuild_pending/parent-notification waiting flow and switchedpropTreeto a debounced idle rebuild that keeps the existing tree attached until replacement time. - In onRebuild, I added tracking for reused widgets and changed rebuild to atomically replace
tree_binwith the newly built root (ornullonly for empty trees), instead of forcing an intermediate blank frame. - In buildTree, leaf nodes now try to reuse an existing
SurfaceScrolledWindowancestor 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. - I also removed now-obsolete parent-change wiring and helper code tied to the old pending-rebuild mechanism in connectSurfaceHandlers/private state.
Validation run:
zig buildpassed.- I did not run
zig build test(full suite), per your request.
What Changed, Concretely
- The rebuild trigger in split_tree.zig was changed so we no longer clear
tree_binimmediately whentreechanges. Instead, we debounce to one idle callback and keep the old widget tree visible until rebuild happens. - 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. - In onRebuild, rebuild now happens in one pass, and the bin child is swapped atomically to the new root widget tree.
- In buildTree, leaf handling now tries to reuse the existing
SurfaceScrolledWindowwidget for each surface, instead of creating a new wrapper every time. - The new detachWidget helper safely detaches reused widgets from their old parent (
GtkPanedorAdwBin) before attaching them into the new split tree. - Reused widgets are temporarily ref-count protected in onRebuild so removing them from old parents cannot destroy them mid-rebuild.
Why This Reduces Flicker
- 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.
- The new flow keeps old content on screen until replacement, so there is no extra frame where the split area is blank.
- 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
- Data model changes (
new split,close split,resize,equalize) still callsetTreeexactly as before. setTreestill emitsnotify::tree, which now just schedules one rebuild idle.- 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.
- Once the new root is built,
tree_bin.setChild(new_root)swaps it in one step. - Focus restore logic remains in place after swap, so active-surface behavior stays stable.
Verification Scope
- I only ran
zig build(passes). - I did not run full tests, per your instruction.
Updated with jj desc in Mitchell-style scope format.
- New subject:
apprt/gtk: reduce split-tree flicker by reusing leaf widgets (#8208) - Body explains the old blank-frame path (
tree_bincleared while waiting for parentless surfaces), the new atomic-swap rebuild approach, and leaf widget reuse via detach/reparent. - Current working-copy commit is now
dae0787fwith that description.
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.
GtkPanedandAdwBinmaintain their own internal child-slot state (start-child,end-child,child) and property notifications.- Their
set*Child(null)APIs update those internal pointers and emit the expected container-side updates. - 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. - That’s why we do typed removal first (
paned.setStartChild(null),paned.setEndChild(null),bin.setChild(null)), and only usewidget.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.
Yes — there’s a better approach than handling append failure, and I implemented it.
- I removed the
ArrayListUnmanaged.appendtracking path entirely frombuildTree/onRebuild, so there is no allocation failure point there anymore. buildTreenow returns a small result struct (widget+release_ref) inBuildTreeResult.- For reused leaf widgets, we still take a temporary ref before detaching, but now we mark
release_ref = trueand release it deterministically after the widget is attached to its new parent in the same recursion flow inbuildTree. - 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.
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:
- If
priv.tree == null, callpriv.tree_bin.setChild(null)immediately. - Notify dependent properties and return without scheduling idle rebuild.
- 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’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.
Findings
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.
The main risk area (temporary refs during reuse) now looks sound: refs are acquired in
initReused, and deterministically released viadeinitafter the widget is attached to the new parent.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#