Inject macos-background-from-layer only for compositing backgrounds - #9802
lawrencecchen wants to merge 1 commit into
Conversation
cmux unconditionally set macos-background-from-layer=true, routing default-background pixels through CoreAnimation host-layer compositing, which drifts +/-1/255 per channel vs Ghostty's stock in-Metal fill. The flag exists to unify translucent fills (#2378), so inject it only when the effective background actually composites against the host layer: background-opacity < 1, background-blur enabled, or a background image. The decision probes a finalized clone of the config (theme expansion at finalize may set background keys) and re-evaluates on every config reload, so live opacity/theme changes flip the mode. Plain opaque backgrounds now render bit-identical to stock desktop Ghostty.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughGhostty terminal background ownership now uses finalized configuration values. Host-layer rendering applies to translucent, blurred, and image-backed backgrounds. Opaque plain backgrounds retain Ghostty’s in-Metal fill. Tests cover the decision rules. ChangesTerminal background ownership
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (24 passed)
✨ 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0422832. Configure here.
| let usesLayerBackground = hostLayerBackgroundDecision(probing: config) | ||
| let renderingModeChanged = setUsesHostLayerBackground( | ||
| true, | ||
| usesLayerBackground, |
There was a problem hiding this comment.
Surface reload desyncs host layer
High Severity
loadDefaultConfigFilesWithLegacyFallback now writes the conditional app-wide usesHostLayerBackground flag even when a surface-only full reload calls it. That path installs a temporary config on one surface and intentionally leaves GhosttyApp.config unchanged, so chrome can drop the host backdrop while other surfaces still zero Metal default-background alpha via macos-background-from-layer, leaving those terminals transparent.
Reviewed by Cursor Bugbot for commit 0422832. Configure here.
cmux inline Ghostty config injection audit (D10)Every
Font resolution: cmux registers no fonts. No cmux == stock Ghostty pixel verification (D10)Setup: cmux
The base (non-abg) rows are the ones this PR changes: they route default-background pixels through the renderer background path, which previously drifted +/-1/255 under CoreAnimation compositing and now takes Ghostty's stock in-Metal fill. Translucency preserved, re-evaluated liveSame running
e2eHosted |
lawrencecchen
added
the
stale-revisit
2 participants
Part of the ghostty rendering-parity work (hq docs/ghostty-web-parity.md, D10): make cmux desktop render pixel-identically to stock desktop Ghostty for opaque (default) configs.
Problem
cmux unconditionally injected
macos-background-from-layer = true(inline config, prefixcmux-renderer-bg). That flag makes Ghostty skip its in-Metal background fill and zero the default-background alpha, so every default-background pixel is composited by CoreAnimation against a host CALayer. CA compositing drifts by +/-1/255 per channel (occasionally +/-2) versus Ghostty's stock deterministic in-Metal fill, so cmux could never bit-match stock Ghostty on default-background cells.Mechanism
The flag was added in #2378 to eliminate a transparent-window flash during sidebar toggles and to unify separate translucent fills for terminal and chrome. Both motivations only exist when the background actually composites against other layers. The injection is now conditional:
macos-background-from-layer = true(and setusesHostLayerBackground) only when the finalized config hasbackground-opacity < 1,background-blurenabled (radius or macOS glass), or abackground-image;TerminalSurfaceBackgroundFillPlanresolves to the existing.ghosttyNativeRendererowner (host layer goes clear);ghostty_config_clonebecausethemeexpansion at finalize time may set background keys, while the real config must receive the injection before finalize;loadDefaultConfigFilesWithLegacyFallback, so live opacity/blur/image/theme changes re-evaluate the decision on cmux's existing config-reload path; the minimal fallback config (invalid user config) takes the same decision.The sidebar-toggle flash cannot regress for opaque configs: during Metal layer resize lag the window backdrop behind it is the same opaque theme color.
Verification
HostLayerBackgroundDecisionTestsincmuxTests/GhosttyConfigTests.swift): opaque default stays on the stock in-Metal fill; opacity < 1, blur radius, both glass modes, and background image each flip to the host layer.Note on test commits: the decision function is new, so a red-first test commit could not compile; tests and fix land together.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
Medium Risk
Changes terminal background rendering ownership and Ghostty config injection on every init/reload; behavior is well-tested for the decision matrix but opaque vs translucent edge cases affect visible pixels and window chrome compositing.
Overview
cmux no longer always forces
macos-background-from-layer = true, so opaque default configs can match stock Ghostty’s in-Metal background fill instead of CoreAnimation compositing (which drifts ±1/255 per channel).The host CALayer path and inline
macos-background-from-layerinjection now run only when a finalized config probe shows translucent background (background-opacity < 1), blur (radius or macOS glass), or a background-image. Theme expansion is evaluated via a cloned finalized config before the real config is finalized. The same logic applies on the main config load path and the minimal fallback init path;usesHostLayerBackgroundfollows that decision.GhosttyApp.shouldUseHostLayerBackgroundis the pure rule, withHostLayerBackgroundDecisionTestscovering opaque default, translucency, blur modes, background image, and over-range opacity.Reviewed by Cursor Bugbot for commit 0422832. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Conditionally inject
macos-background-from-layerso only compositing backgrounds use the host CALayer; opaque backgrounds keep Ghostty’s in‑Metal fill for pixel-identical output to stock desktop Ghostty. This removes CoreAnimation drift on default-background cells while preserving translucency and blur behavior.macos-background-from-layeronly when background-opacity < 1, background-blur is enabled, or a background-image is set; otherwise omit and setusesHostLayerBackgroundto false.HostLayerBackgroundDecisionTestscovering opaque, translucent, blur, glass, image, and overrange opacity cases.Written for commit 0422832. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests