Fix transparent background flash during sidebar toggle - #2378
Conversation
Move terminal background rendering from the Metal GPU pass to a CALayer (backgroundView). The GPU bg_color pass is disabled via a new Ghostty config flag (macos-background-from-layer). The CALayer resizes instantly with its parent NSView, eliminating the 3-5 frame gap where the desktop was visible through the transparent window during sidebar toggles and layout transitions. Also simplifies the titlebar and sidebar opacity formulas since there is now a single background layer instead of two stacked semi-transparent layers.
|
@codex review |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughReworks background sourcing so macOS CALayers provide titlebar, sidebar, and terminal backgrounds: injects Changes
Sequence Diagram(s)sequenceDiagram
participant App as App (ContentView)
participant Ghostty as Ghostty Config
participant Renderer as Metal Renderer
participant Layer as Terminal CALayer
App->>Ghostty: initialize/fallback config (inject macos-background-from-layer = true)
Ghostty-->>App: finalized config (flag present)
App->>Layer: set layer.backgroundColor & layer.isOpaque from configured color/opacity
App->>Renderer: begin frame (renderer reads config)
Renderer->>Renderer: if flag true -> set bg_color[3] = 0
Renderer-->>Layer: draw frame with transparent GPU background
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
1 issue found across 3 files
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:7304">
P2: Unconditional translucent panel fill assumes layer-bg mode globally, but fallback config path skips that flag and can reintroduce alpha stacking.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Greptile SummaryThis PR fixes a 3–5 frame "flash of desktop" that appeared during sidebar toggles when
The architecture is sound and the visual changes (opacity values for titlebar/sidebar) are intentionally different from before — users on non-opaque configs will see slightly lower effective opacity in the titlebar/sidebar area, matching the terminal itself more accurately.
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Desktop as Desktop/Wallpaper
participant Window as NSWindow (transparent)
participant BG as backgroundView (CALayer)
participant Metal as GhosttyNSView (Metal/GPU)
Note over BG,Metal: Before this PR (transparent mode)
Desktop->>Window: visible through clear backgroundView
Metal-->>Window: GPU renders semi-transparent bg (3-5 frame delay during layout)
Note over Desktop,Metal: ⚠️ Flash: bg clear for 3-5 frames during sidebar toggle
Note over BG,Metal: After this PR
BG->>Window: always fills with terminalColor @ configured opacity (instant)
Metal-->>Window: GPU renders ONLY text & per-cell colors (bg pass disabled via macos-background-from-layer=true)
Note over Desktop,Metal: ✅ No flash: backgroundView always covers the area instantly
|
| private static func panelBackgroundFillColor(for terminalBackgroundColor: NSColor) -> NSColor { | ||
| // The Ghostty renderer already draws translucent terminal backgrounds. If we paint an | ||
| // additional translucent layer here, alpha stacks and appears effectively opaque. | ||
| terminalBackgroundColor.alphaComponent < 0.999 ? .clear : terminalBackgroundColor | ||
| // The Ghostty renderer's GPU bg pass is disabled (macos-background-from-layer), | ||
| // so this CALayer is the sole provider of the terminal background color. | ||
| // It resizes instantly with its parent, eliminating the flash of desktop | ||
| // that would otherwise appear during sidebar toggles and layout transitions. | ||
| terminalBackgroundColor | ||
| } |
There was a problem hiding this comment.
panelBackgroundFillColor is now a trivial identity wrapper
After the change, panelBackgroundFillColor(for:) simply returns its input unchanged. The two call sites (init and setBackgroundColor) pass the terminal color with alpha already embedded. Since there's no longer any conditional logic, the function adds indirection without transformation. Consider inlining it at its two call sites for clarity, or keeping the function but renaming it to signal its purpose (e.g., backgroundLayerColor(for:)) so future readers aren't surprised by the lack of transformation.
| private static func panelBackgroundFillColor(for terminalBackgroundColor: NSColor) -> NSColor { | |
| // The Ghostty renderer already draws translucent terminal backgrounds. If we paint an | |
| // additional translucent layer here, alpha stacks and appears effectively opaque. | |
| terminalBackgroundColor.alphaComponent < 0.999 ? .clear : terminalBackgroundColor | |
| // The Ghostty renderer's GPU bg pass is disabled (macos-background-from-layer), | |
| // so this CALayer is the sole provider of the terminal background color. | |
| // It resizes instantly with its parent, eliminating the flash of desktop | |
| // that would otherwise appear during sidebar toggles and layout transitions. | |
| terminalBackgroundColor | |
| } | |
| private static func panelBackgroundFillColor(for terminalBackgroundColor: NSColor) -> NSColor { | |
| // The Ghostty renderer's GPU bg pass is disabled (macos-background-from-layer), | |
| // so this CALayer is the sole provider of the terminal background color. | |
| // It resizes instantly with its parent, eliminating the flash of desktop | |
| // that would otherwise appear during sidebar toggles and layout transitions. | |
| return terminalBackgroundColor | |
| } |
(No functional change — just noting the wrapper can be inlined if desired.)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…apper - Inject macos-background-from-layer in the fallback config path too, preventing alpha double-stacking when user config is invalid - Inline panelBackgroundFillColor (now an identity function) at its two call sites and remove the wrapper
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docs/ghostty-fork.md (1)
107-115: Add the Ghostty commit SHA for this fork patch.Using only
Branch: feat-layer-bgmakes rebases/debugging harder; please include the exact commit ID (like other sections) so the patch is fully traceable.Based on learnings: Ghostty submodule changes must be committed in the
ghosttysubmodule anddocs/ghostty-fork.mdshould stay current with fork changes/conflict notes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ghostty`:
- Line 1: Add a checksum entry for the new ghostty submodule commit
41e796064e89eacabdf3a6729475e250a5518e7a to the ghosttykit checksums file so the
download script can resolve it: update scripts/ghosttykit-checksums.txt by
appending the mapping that pairs the commit hash
41e796064e89eacabdf3a6729475e250a5518e7a with the corresponding prebuilt archive
checksum (the value used by download-prebuilt-ghosttykit.sh), ensuring the
format matches the existing entries so download-prebuilt-ghosttykit.sh will
recognize and validate the new commit.
- Line 1: The submodule pointer was updated to commit
41e796064e89eacabdf3a6729475e250a5518e7a but that commit is not on the remote
manaflow-ai/ghostty main branch; push the local commit
41e796064e89eacabdf3a6729475e250a5518e7a to the manaflow-ai/ghostty main branch,
then update the submodule pointer in this repo to the pushed commit; finally,
verify and update docs/ghostty-fork.md to record the fork changes or any
merge/conflict notes related to this update so the submodule change is
documented.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 1392-1400: The blur radius isn't cleared when switching back to an
opaque background; update the background update paths (e.g.
GhosttyApp.applyBackgroundToKeyWindow() and
GhosttyNSView.applyWindowBackgroundIfActive()) to always call
cmuxApplyBackgroundBlur(to: radius:) on every background change and pass radius
0 whenever the effective opacity is >= 1.0 or the configured blur radius is 0
(i.e. when blur is disabled), ensuring blur is explicitly cleared even on the
opaque branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 96ce9d4a-515e-46a1-ae1f-1d7ebf9dc75e
📒 Files selected for processing (4)
Sources/ContentView.swiftSources/GhosttyTerminalView.swiftdocs/ghostty-fork.mdghostty
| // cmux provides the terminal background via backgroundView (CALayer) | ||
| // instead of the GPU full-screen bg pass, so the layer can provide | ||
| // instant coverage during sidebar toggle and other layout transitions. | ||
| loadInlineGhosttyConfig( | ||
| "macos-background-from-layer = true", | ||
| into: config, | ||
| prefix: "cmux-layer-bg", | ||
| logLabel: "layer background" | ||
| ) |
There was a problem hiding this comment.
Reset compositor blur when this path goes back to opaque.
Turning on macos-background-from-layer makes the host layer/window path responsible for the visible background, but GhosttyApp.applyBackgroundToKeyWindow() and GhosttyNSView.applyWindowBackgroundIfActive() still only touch the blur setter on the transparent branch. If the user moves from translucent back to opaque, the old blur radius can stay latched and bleed into the supposedly solid background. Please clear blur on every background update and pass 0 when opacity is opaque or blur is disabled.
Based on learnings: In Sources/GhosttyTerminalView.swift, CGS window background blur is stateful. Always call cmuxApplyBackgroundBlur(to: NSWindow, radius: Int) on background updates and pass radius 0 when blur should be disabled (opacity >= 1.0 or configured radius == 0).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 1392 - 1400, The blur radius
isn't cleared when switching back to an opaque background; update the background
update paths (e.g. GhosttyApp.applyBackgroundToKeyWindow() and
GhosttyNSView.applyWindowBackgroundIfActive()) to always call
cmuxApplyBackgroundBlur(to: radius:) on every background change and pass radius
0 whenever the effective opacity is >= 1.0 or the configured blur radius is 0
(i.e. when blur is disabled), ensuring blur is explicitly cleared even on the
opaque branch.
The bg_color uniform alpha is still zeroed for cell compositing (so transparent cells pass through to the CALayer), but the fullscreen background fill draw step is now explicitly skipped instead of relying on alpha=0 as a no-op.
) * Fix transparent background flash during sidebar toggle Move terminal background rendering from the Metal GPU pass to a CALayer (backgroundView). The GPU bg_color pass is disabled via a new Ghostty config flag (macos-background-from-layer). The CALayer resizes instantly with its parent NSView, eliminating the 3-5 frame gap where the desktop was visible through the transparent window during sidebar toggles and layout transitions. Also simplifies the titlebar and sidebar opacity formulas since there is now a single background layer instead of two stacked semi-transparent layers. * Document macos-background-from-layer fork change * Pin GhosttyKit checksum for macos-background-from-layer * Address review feedback: fix fallback config path, inline identity wrapper - Inject macos-background-from-layer in the fallback config path too, preventing alpha double-stacking when user config is invalid - Inline panelBackgroundFillColor (now an identity function) at its two call sites and remove the wrapper * Address adversarial review: skip fullscreen bg draw call explicitly The bg_color uniform alpha is still zeroed for cell compositing (so transparent cells pass through to the CALayer), but the fullscreen background fill draw step is now explicitly skipped instead of relying on alpha=0 as a no-op. * Pin GhosttyKit checksum for bg draw-call skip --------- Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Summary
backgroundView), which resizes instantly with its parent NSViewmacos-background-from-layerconfig flag to Ghostty fork that setsbg_coloralpha to 0, letting the host layer provide the background without alpha double-stackingThe root cause: during sidebar toggle, there's a 3-5 frame gap between SwiftUI layout expansion and Metal re-render. With
backgroundViewclear in transparent mode, the desktop was visible through the near-clear window background. NowbackgroundViewalways provides the terminal color, and the GPU bg pass is disabled.Test plan
background-opacity < 1(e.g. 0.5, 0.8). Verify no flash of desktop/wallpaperSummary by cubic
Fixes the transparent background flash during sidebar toggles by moving the terminal background to a CALayer that resizes instantly. Disables Ghostty’s fullscreen GPU background fill via
macos-background-from-layerand uses the configured alpha for the titlebar and sidebar.Bug Fixes
Refactors
backgroundView(CALayer) and injectmacos-background-from-layer = trueinto both normal and fallback inline configs; Ghostty fork explicitly skips the fullscreen background draw while keeping cell compositing pass-through.ghosttysubmodule, document the new flag, and pinGhosttyKitchecksums (including the bg draw-call skip).Written for commit 8965e94. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores