Fix blank terminal renders after workspace switches#1964
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
To use Codex here, create a Codex account and connect to github. |
📝 WalkthroughWalkthroughThe change adds explicit occlusion control to terminal surfaces based on portal visibility state transitions. When a portal's visibility changes, the code now calls Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes stale blank/white terminal surfaces that appear after switching workspaces by driving Ghostty's native occlusion API ( Key changes:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant WS as Workspace.swift
participant ScrollView as GhosttySurfaceScrollView
participant NSView as GhosttyNSView
participant Surface as TerminalSurface
Note over WS,Surface: Workspace switch (hide)
WS->>ScrollView: setVisibleInUI(false)
ScrollView->>NSView: setVisibleInUI(false)
NSView-->>NSView: visibleInUI = false
ScrollView->>ScrollView: isHidden = true
alt wasVisible != visible AND lastRequested != visible
ScrollView->>ScrollView: lastRequestedPortalOcclusionVisible = false
ScrollView->>Surface: setOcclusion(false)
Surface-->>Surface: ghostty_surface_set_occlusion(false)<br/>(pause rendering)
end
Note over WS,Surface: Workspace switch (show)
WS->>ScrollView: setVisibleInUI(true)
ScrollView->>NSView: setVisibleInUI(true)
NSView-->>NSView: visibleInUI = true
ScrollView->>ScrollView: isHidden = false
alt wasVisible != visible AND lastRequested != visible
ScrollView->>ScrollView: lastRequestedPortalOcclusionVisible = true
ScrollView->>Surface: setOcclusion(true)
Surface-->>Surface: ghostty_surface_set_occlusion(true)<br/>(resume + queue redraw → fixes blank surface)
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if wasVisible != visible, lastRequestedPortalOcclusionVisible != visible { | ||
| lastRequestedPortalOcclusionVisible = visible | ||
| surfaceView.terminalSurface?.setOcclusion(visible) | ||
| } |
There was a problem hiding this comment.
lastRequestedPortalOcclusionVisible is updated before the optional-chained setOcclusion call. If terminalSurface is nil at the time of the transition (e.g. the surface hasn't been attached yet), the flag is set to visible but Ghostty is never actually notified. A subsequent call with the same visible value will then be suppressed by the guard, leaving the surface's occlusion state out of sync with the recorded intent.
In practice this is unlikely to be hit on the normal workspace-switch path (surfaces are attached before workspaces are shown/hidden), but the pattern can be made robust by only recording the intent after a successful call:
| if wasVisible != visible, lastRequestedPortalOcclusionVisible != visible { | |
| lastRequestedPortalOcclusionVisible = visible | |
| surfaceView.terminalSurface?.setOcclusion(visible) | |
| } | |
| if wasVisible != visible { | |
| let surface = surfaceView.terminalSurface | |
| if lastRequestedPortalOcclusionVisible != visible, surface != nil { | |
| lastRequestedPortalOcclusionVisible = visible | |
| surface?.setOcclusion(visible) | |
| } | |
| } |
Alternatively, unconditionally updating the flag and adding a comment noting the nil-surface race is also acceptable if the surface lifecycle guarantees absence of the edge case.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/GhosttyTerminalView.swift`:
- Line 6595: The cached occlusion flag lastRequestedPortalOcclusionVisible on
GhosttyTerminalView can become stale across runtime surface (ghostty_surface_t)
lifecycles causing missed hidden→visible transitions; update the logic so that
when createSurface(for:) finishes (or whenever the runtime surface is
rebound/recreated, e.g. after forceRefreshSurface()) you either reset
lastRequestedPortalOcclusionVisible or replay/apply the cached occlusion against
the newly created surface so the new surface sees the correct transition; modify
createSurface(for:) (and the surface-binding/rebind path) to clear or
re-evaluate lastRequestedPortalOcclusionVisible and call the same code path used
by setVisibleInUI(_:) to apply the occlusion to the fresh surface (or switch the
dedupe key to be per-current-surface rather than per-view).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ef13150d-9b40-4831-baba-c885156bf236
📒 Files selected for processing (1)
Sources/GhosttyTerminalView.swift
| private static let scrollToBottomThreshold: CGFloat = 5.0 | ||
| private var isActive = true | ||
| private var lastFocusRefreshAt: CFTimeInterval = 0 | ||
| private var lastRequestedPortalOcclusionVisible: Bool? |
There was a problem hiding this comment.
Replay occlusion when the runtime surface changes.
lastRequestedPortalOcclusionVisible lives on the hosted view, but the actual ghostty_surface_t has its own lifecycle. If setVisibleInUI(_:) runs before createSurface(for:) finishes, or after the runtime surface is torn down/recreated, this cache can flip even though no occlusion was applied to the new surface. The next restore then sends only true, so the replacement surface never sees the hidden→visible transition this fix is relying on. Reset/replay the cached occlusion when a runtime surface is created/rebound, or key the dedupe off the current runtime surface instead of the view.
Based on learnings: "after calling view.forceRefreshSurface(), re-read self.surface because the surface may be torn down or reparented."
Also applies to: 7915-7918
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` at line 6595, The cached occlusion flag
lastRequestedPortalOcclusionVisible on GhosttyTerminalView can become stale
across runtime surface (ghostty_surface_t) lifecycles causing missed
hidden→visible transitions; update the logic so that when createSurface(for:)
finishes (or whenever the runtime surface is rebound/recreated, e.g. after
forceRefreshSurface()) you either reset lastRequestedPortalOcclusionVisible or
replay/apply the cached occlusion against the newly created surface so the new
surface sees the correct transition; modify createSurface(for:) (and the
surface-binding/rebind path) to clear or re-evaluate
lastRequestedPortalOcclusionVisible and call the same code path used by
setVisibleInUI(_:) to apply the occlusion to the fresh surface (or switch the
dedupe key to be per-current-surface rather than per-view).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/GhosttyTerminalView.swift">
<violation number="1" location="Sources/GhosttyTerminalView.swift:7915">
P1: Occlusion state is cached before confirming a live terminal surface, which can drop the occlusion transition and leave Ghostty visibility state out of sync.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if wasVisible != visible, lastRequestedPortalOcclusionVisible != visible { | ||
| lastRequestedPortalOcclusionVisible = visible | ||
| surfaceView.terminalSurface?.setOcclusion(visible) |
There was a problem hiding this comment.
P1: Occlusion state is cached before confirming a live terminal surface, which can drop the occlusion transition and leave Ghostty visibility state out of sync.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/GhosttyTerminalView.swift, line 7915:
<comment>Occlusion state is cached before confirming a live terminal surface, which can drop the occlusion transition and leave Ghostty visibility state out of sync.</comment>
<file context>
@@ -7909,6 +7912,10 @@ final class GhosttySurfaceScrollView: NSView {
let wasVisible = surfaceView.isVisibleInUI
surfaceView.setVisibleInUI(visible)
isHidden = !visible
+ if wasVisible != visible, lastRequestedPortalOcclusionVisible != visible {
+ lastRequestedPortalOcclusionVisible = visible
+ surfaceView.terminalSurface?.setOcclusion(visible)
</file context>
| if wasVisible != visible, lastRequestedPortalOcclusionVisible != visible { | |
| lastRequestedPortalOcclusionVisible = visible | |
| surfaceView.terminalSurface?.setOcclusion(visible) | |
| if lastRequestedPortalOcclusionVisible != visible, | |
| let terminalSurface = surfaceView.terminalSurface { | |
| terminalSurface.setOcclusion(visible) | |
| lastRequestedPortalOcclusionVisible = visible | |
| } |
|
Reproducing on the latest build. The original fix may have regressed, or this
Symptom: Switching between two already-existing workspaces via Shift+F12 Repro:
Notes:
|
2 participants
Closes #1789
Summary
Testing
./scripts/reload.sh --tag issue-1789-v2.cmuxCLI to switch workspaces back and forth to exercise the workspace handoff path.Demo Video
Review Trigger (Copy/Paste as PR comment)
Checklist
Summary by cubic
Fixes blank/white terminal surfaces after switching workspaces by driving Ghostty occlusion from portal visibility transitions and relying on the native pause/resume render path. This removes the need for forced surface refreshes and ensures a redraw when a workspace becomes visible.
terminalSurface?.setOcclusion(visible)on portal visibility changes so hidden surfaces pause and resume with a redraw.lastRequestedPortalOcclusionVisible; keep view visibility for focus gating only.Written for commit aa24f12. Summary will update on new commits.
Summary by CodeRabbit