Skip to content

fix: prevent terminal flickering during layout changes (e.g., new-workspace) #1689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
BillionClaw wants to merge 1 commit into from
Closed

fix: prevent terminal flickering during layout changes (e.g., new-workspace) #1689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Sources/ContentView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2044,6 +2044,9 @@ struct ContentView: View {
)
}
)
// Stable identity prevents view recreation when mountedWorkspaces changes,
// avoiding terminal flicker during workspace layout transitions (e.g., new-workspace).
.id(tab.id)
.opacity(workspaceRenderOpacity)
.allowsHitTesting(isSelectedWorkspace)
.accessibilityHidden(!isRenderedVisible)
Expand Down
21 changes: 20 additions & 1 deletion Sources/GhosttyTerminalView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3953,6 +3953,9 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
applySurfaceColorScheme(force: !isSameSurface || !isAlreadyAttached)
}

/// Tracks the last window this view was attached to for optimizing re-parenting within the same window.
private var lastAttachedWindow: NSWindow?

override func viewDidMoveToWindow() {
super.viewDidMoveToWindow()
if let windowObserver {
Expand All @@ -3966,7 +3969,15 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
"pending=\(String(format: "%.1fx%.1f", pendingSurfaceSize?.width ?? 0, pendingSurfaceSize?.height ?? 0))"
)
#endif
guard let window else { return }
guard let window else {
// View is being detached from a window - clear the tracking reference
lastAttachedWindow = nil
return
Comment on lines +3974 to +3975

@cubic-dev-ai cubic-dev-ai Bot Mar 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Same-window reparent detection is neutralized by clearing lastAttachedWindow on transient detach, so the anti-flicker fast path often won’t trigger.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/GhosttyTerminalView.swift, line 3974:

<comment>Same-window reparent detection is neutralized by clearing `lastAttachedWindow` on transient detach, so the anti-flicker fast path often won’t trigger.</comment>

<file context>
@@ -3966,7 +3969,15 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
-        guard let window else { return }
+        guard let window else {
+            // View is being detached from a window - clear the tracking reference
+            lastAttachedWindow = nil
+            return
+        }
</file context>
Suggested change
lastAttachedWindow = nil
return
// Keep the previous window reference so a transient detach/reattach
// can still be recognized as same-window reparenting.
return
Fix with Cubic

}

// Check if this is a re-parenting within the same window (no actual window change)
let isReparentingWithinSameWindow = lastAttachedWindow === window
lastAttachedWindow = window
Comment on lines +3972 to +3980

@coderabbitai coderabbitai Bot Mar 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's see the complete viewDidMoveToWindow implementation and surrounding context
cd /tmp && git clone --depth 1 https://github.com/manaflow-ai/cmux.git 2>&1 | grep -E "(Cloning|done)" && cd cmux

Repository: manaflow-ai/cmux

Length of output: 617

🏁 Script executed:

# Get full context around the changed lines (3956-4015)
sed -n '3950,4020p' Sources/GhosttyTerminalView.swift | cat -n

Repository: manaflow-ai/cmux

Length of output: 3489

🏁 Script executed:

# Look for the fast-path branch that should be triggered (IME invalidation only)
sed -n '4000,4020p' Sources/GhosttyTerminalView.swift | cat -n

Repository: manaflow-ai/cmux

Length of output: 1120

🏁 Script executed:

# Search for where viewDidMoveToWindow is called during workspace transitions
rg -A 5 -B 5 "new-workspace\|workspace.*switch\|layout.*change" Sources/AppDelegate.swift Sources/Workspace.swift | head -100

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Check if weak references are used in similar patterns elsewhere in this file
rg "weak var.*NSWindow" Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Look at the complete property declarations around line 3956
sed -n '3950,3965p' Sources/GhosttyTerminalView.swift | cat -n

Repository: manaflow-ai/cmux

Length of output: 724

🏁 Script executed:

# Search for workspace switch and panel reparenting logic
rg -B 3 -A 10 "func reconcilePanelPortalVisibilityForCurrentLayout\|func.*reparent\|new-workspace" Sources/Workspace.swift | head -80

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Look for panel view reparenting during layout changes
rg -B 2 -A 8 "removeFromSuperview\|addSubview" Sources/Workspace.swift | grep -A 8 -B 2 "panel\|surface\|terminal" | head -100

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Check what happens to GhosttySurfaceScrollView during workspace layout switches
rg -B 3 -A 5 "GhosttySurfaceScrollView" Sources/Workspace.swift | head -60

Repository: manaflow-ai/cmux

Length of output: 2179

🏁 Script executed:

# Check NSView reparenting patterns that could trigger viewDidMoveToWindow
rg "removeFromSuperview\|addSubview" Sources/Workspace.swift Sources/ContentView.swift | head -30

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Search for any discussion or tracking of flicker issues during workspace changes
rg -B 2 -A 2 "flicker\|reparent.*window" Sources/GhosttyTerminalView.swift | head -50

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Search for panel view hierarchy manipulation in ContentView (SwiftUI)
rg -B 3 -A 10 "if.*workspace\|if.*layout\|if panels" Sources/ContentView.swift | head -100

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Look for panel content/hosting view updates
rg -B 2 -A 8 "\.panel\|panelContainers\|hostingController" Sources/Workspace.swift | head -80

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Check if GhosttySurfaceScrollView is recreated or reused
rg "GhosttySurfaceScrollView()" Sources/ | head -20

Repository: manaflow-ai/cmux

Length of output: 2230

🏁 Script executed:

# Search for actual issue/discussion in comments about workspace flicker
rg -i "workspace.*flicker\|layout.*flicker\|same.*window.*reparent" Sources/ --type swift | head -20

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Check the actual call chain: what triggers viewDidMoveToWindow during workspace ops
rg "new-workspace\|selectWorkspace\|activateWorkspace" Sources/AppDelegate.swift | head -10

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Look at what Workspace.reconcile does (mentioned in learnings)
rg -B 5 -A 15 "func reconcile" Sources/Workspace.swift | head -100

Repository: manaflow-ai/cmux

Length of output: 3714

🏁 Script executed:

# Search for TerminalWindowPortal.detach which might trigger viewDidMoveToWindow
rg -B 5 -A 15 "static func detach\|func.*detach.*hostedView" Sources/TerminalWindowPortal.swift | head -80

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Check what bind/detach do with view hierarchy
rg -B 3 -A 12 "func bind.*hostedView\|hostedView\.removeFromSuperview\|hostView\.addSubview" Sources/TerminalWindowPortal.swift | head -120

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Check if new-workspace command triggers reconcile operations
rg -B 5 -A 10 "new-workspace.*handleCustomCommand\|v1NewWorkspace\|newWorkspace" Sources/AppDelegate.swift | head -80

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Look at the actual detach implementation in TerminalWindowPortal
sed -n '1,600p' Sources/TerminalWindowPortal.swift | grep -n -A 20 "static func detach"

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Check if there's a bind/detach cycle during layout reconciliation
rg -B 3 -A 8 "detach.*hostedView\|bind.*hostedView" Sources/Workspace.swift | head -100

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Search more broadly for panel repositioning during layout changes
rg "setVisibleInUI\|reconcile.*visibility\|renderedVisiblePanelIds" Sources/Workspace.swift -A 5 | head -100

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Look at the complete TerminalPanel definition to understand panel hosting
rg -B 3 -A 20 "class TerminalPanel\|struct TerminalPanel" Sources/Workspace.swift | head -80

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Check if panels are recreated or just hidden/shown during workspace switches
rg "panels\[" Sources/Workspace.swift | head -30

Repository: manaflow-ai/cmux

Length of output: 1694

🏁 Script executed:

# Search in ContentView for panel view updates during workspace transitions
rg -B 3 -A 8 "\.workspace\|workspace.panels" Sources/ContentView.swift | head -100

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Look for what actually causes viewDidMoveToWindow during normal terminal operations
rg "viewDidMoveToWindow\|window change" Sources/GhosttyTerminalView.swift -B 2 -A 2

Repository: manaflow-ai/cmux

Length of output: 42

🏁 Script executed:

# Check git log for recent changes to GhosttyTerminalView around viewDidMoveToWindow
git log --oneline -20 -- Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 144

🏁 Script executed:

# Get the full details of the commit that added this fix
git show aeab5e0 --stat

Repository: manaflow-ai/cmux

Length of output: 50373

🏁 Script executed:

# Get the actual diff of the problematic code
git show aeab5e0 | head -200

Repository: manaflow-ai/cmux

Length of output: 8547

🏁 Script executed:

# Look at what the commit message says about the problem
git show aeab5e0 | grep -A 20 "^    "

Repository: manaflow-ai/cmux

Length of output: 1393

🏁 Script executed:

# Check if there are any related issues or discussion in recent PRs
git log --oneline --all --grep="flicker\|reparent" | head -10

Repository: manaflow-ai/cmux

Length of output: 726

Same-window reparent detection is reset too early

On line 25, clearing lastAttachedWindow during detach masks a nil→same-window reattach, so line 30 evaluates false and still runs the expensive path (the flicker scenario this PR targets).

💡 Proposed fix 
-    private var lastAttachedWindow: NSWindow?
+    private weak var lastAttachedWindow: NSWindow?
@@
-        guard let window else {
-            // View is being detached from a window - clear the tracking reference
-            lastAttachedWindow = nil
-            return
-        }
+        guard let window else {
+            // Keep the previous window reference so nil -> same-window reattach
+            // can still be detected as same-window reparenting.
+            return
+        }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Sources/GhosttyTerminalView.swift` around lines 3972 - 3980, The detach
branch currently clears lastAttachedWindow too early, causing a nil→same-window
reattach to be misdetected; remove the assignment that sets lastAttachedWindow =
nil inside the guard-else detach path and only update lastAttachedWindow when a
non-nil window is present (keep the existing comparison let
isReparentingWithinSameWindow = lastAttachedWindow === window and the later
lastAttachedWindow = window), so same-window reparenting is detected correctly
by the isReparentingWithinSameWindow check.


// If the surface creation was deferred while detached, create/attach it now.
terminalSurface?.attachToView(self)
Expand Down Expand Up @@ -3995,6 +4006,14 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
ghostty_surface_set_display_id(surface, displayID)
}

// Skip expensive layout and surface updates when re-parenting within the same window
// to avoid terminal flicker during workspace layout changes (e.g., new-workspace).
if isReparentingWithinSameWindow {
// Still invalidate text input coordinates as the view position may have changed
invalidateTextInputCoordinates()
return
}

// Recompute from current bounds after layout. Pending size is only a fallback
// when we don't have usable bounds (e.g. detached/off-window transitions).
superview?.layoutSubtreeIfNeeded()
Expand Down