Add adaptive and configurable terminal backgrounds - #178
Conversation
github-actions
Bot
added
area:ui
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds terminal background source preferences, custom color overrides, IOSurface-based adaptive background detection, stabilized pane/window presentation, contrast-aware theming, lifecycle callbacks, and tests for color conversion, detection, stabilization, and theme behavior. ChangesAdaptive terminal backgrounds
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TerminalPane
participant GhosttyTerminalNSView
participant AdaptiveTerminalChrome
participant AdaptiveTerminalBackgroundDetector
participant GhosttyApp
TerminalPane->>AdaptiveTerminalChrome: Report focus and render events
GhosttyTerminalNSView->>AdaptiveTerminalChrome: Report background and reset events
AdaptiveTerminalChrome->>AdaptiveTerminalBackgroundDetector: Sample visible terminal surface
AdaptiveTerminalBackgroundDetector-->>AdaptiveTerminalChrome: Return dominant color
AdaptiveTerminalChrome->>GhosttyTerminalNSView: Present pane background
AdaptiveTerminalChrome->>GhosttyApp: Adopt window background
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Window-state benchmark
Reported value is the median of 3×10s windows per state (splitting the window and taking the median keeps one co-scheduled spike from skewing a state); CPU % is the process CPU-time delta over a window. Runs land on different shared runners, so treat small deltas as noise — 🔺/🔻 marks changes ≥25% that also clear the metric's absolute noise floor (CPU % ≥0.5, Memory (RSS MB) ≥25, CPU ms/s ≥5, Wakeups/s ≥50); CPU deltas off a noise-dominated baseline aren't flagged (CPU % baseline ≥1.5, CPU ms/s baseline ≥15). Flagged changes add the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Macterm/Model/SplitNode.swift`:
- Around line 342-345: Route adaptive pane-color changes through AppState
instead of directly mutating Pane.adaptiveBackgroundColor. Add an AppState
mutation API for updating the relevant pane, then access AppState in the
TerminalPane view via `@Environment`(AppState.self) and invoke that API from the
callback.
In `@MactermTests/Ghostty/AdaptiveTerminalBackgroundTests.swift`:
- Line 6: Annotate the AdaptiveTerminalBackgroundTests test struct with
`@MainActor`, preserving its existing testable import and test implementation.
🪄 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 Plus
Run ID: 6818a05a-2e13-40bb-8296-1864954c11b3
📒 Files selected for processing (21)
Macterm/App/MactermApp.swiftMacterm/App/Preferences.swiftMacterm/Config/MactermConfig.swiftMacterm/Config/TerminalBackgroundOverride.swiftMacterm/Ghostty/AdaptiveTerminalBackground.swiftMacterm/Ghostty/AdaptiveTerminalChrome.swiftMacterm/Ghostty/GhosttyApp.swiftMacterm/Ghostty/GhosttyCallbacks.swiftMacterm/Ghostty/Theme.swiftMacterm/Ghostty/ThemeColorMath.swiftMacterm/Model/SplitNode.swiftMacterm/Settings/SettingsView.swiftMacterm/Views/MainWindow.swiftMacterm/Views/SplitTreeView.swiftMacterm/Views/Terminal/GhosttyTerminalNSView.swiftMacterm/Views/TerminalPane.swiftMacterm/Views/WindowAppearance.swiftMactermTests/Config/TerminalBackgroundOverrideTests.swiftMactermTests/Ghostty/AdaptiveTerminalBackgroundTests.swiftMactermTests/Ghostty/ThemeColorMathTests.swiftproject.yml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
MactermTests/App/AppStateTests.swift (1)
62-75: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover project scoping and the nil-reset path.
The test only uses an unknown pane ID within the same project. Add a valid pane ID with a different
projectID, plus a valid-panenilupdate, to protect both ownership isolation and clearing behavior.Suggested test extension
state.setAdaptiveBackgroundColor(color, paneID: pane.id, projectID: project.id) `#expect`(pane.adaptiveBackgroundColor == color) + let otherProject = seedProject(state, name: "other", path: "/tmp/other") + state.setAdaptiveBackgroundColor(nil, paneID: pane.id, projectID: otherProject.id) + `#expect`(pane.adaptiveBackgroundColor == color) + state.setAdaptiveBackgroundColor(nil, paneID: UUID(), projectID: project.id) `#expect`(pane.adaptiveBackgroundColor == color) + + state.setAdaptiveBackgroundColor(nil, paneID: pane.id, projectID: project.id) + `#expect`(pane.adaptiveBackgroundColor == nil)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@MactermTests/App/AppStateTests.swift` around lines 62 - 75, The adaptiveBackgroundColor_updatesOwnedPaneOnly test should also cover project ownership and clearing. Add a valid pane ID belonging to a different project and verify the original pane remains unchanged, then call setAdaptiveBackgroundColor with the original pane ID and project ID and nil and verify its adaptiveBackgroundColor is cleared.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@MactermTests/App/AppStateTests.swift`:
- Around line 62-75: The adaptiveBackgroundColor_updatesOwnedPaneOnly test
should also cover project ownership and clearing. Add a valid pane ID belonging
to a different project and verify the original pane remains unchanged, then call
setAdaptiveBackgroundColor with the original pane ID and project ID and nil and
verify its adaptiveBackgroundColor is cleared.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 69dde426-a777-4b3f-83a4-5a41b2d22542
📒 Files selected for processing (7)
Macterm/App/AppState.swiftMacterm/Views/MainWindow.swiftMacterm/Views/QuickTerminal.swiftMacterm/Views/SplitTreeView.swiftMacterm/Views/TerminalPane.swiftMactermTests/App/AppStateTests.swiftMactermTests/Ghostty/AdaptiveTerminalBackgroundTests.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- MactermTests/Ghostty/AdaptiveTerminalBackgroundTests.swift
- Macterm/Views/TerminalPane.swift
github-actions
Bot
added
the
benchmark:improvement
|
@OpenSource03 Wow this is impressive. Thanks for your work! I applied some fixes for issues I encountered while testing this locally:
|
Switching to a tab with a running TUI flashed the configured theme: pruneState wiped the pane's remembered color, fill, and stabilizer on occlusion, so every revisit restarted detection through the 0.25s verification pass (or the 0.5s monitor when focus fired before window attachment). Remembered colors and fills now survive occlusion and seed fresh stabilizers, so a revisited tab presents immediately while a TUI that exited off-screen still clears via two observations. On translucent windows, a lone TUI pane also showed a see-through seam: single-pane tabs only got the window-wide tint, which follows windowOpacity, next to the TUI's opaque pixels. Every detected pane now gets the same opaque fill splits already had; the window tint rule is unchanged.
Feeding the adaptive tint into .preferredColorScheme made the app-wide scheme flap on every tint adopt/clear whenever a TUI's background sat on the opposite side of the WCAG light/dark split. That setting is scene-level - it also governs the Settings window - and the runtime churn destabilized SwiftUI window management: the closed Settings window reopened on every app activation, and the WindowGroup window lost the cached identity that gates hotkeys (Cmd+W closed the whole window, Cmd+D went dead). The scheme now derives from the config background only, keeping the WCAG-contrast classification. Window-scoped adaptation (nsBg tint, nsFg contrast) is untouched.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
macterm | 5f6c8bf | Commit Preview URL Branch Preview URL |
Jul 21 2026, 08:42 AM |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@MactermTests/Ghostty/ThemeTests.swift`:
- Around line 18-19: Update the test around
GhosttyApp.shared.adoptAdaptiveBackgroundColor to capture the existing
adaptiveBackgroundColor before applying opposite, then restore the captured
value in defer instead of always passing nil. Preserve the test’s temporary
override while restoring process-wide state exactly as it was.
🪄 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 Plus
Run ID: 5a50c140-4474-4f90-ab4e-8812f6b4f754
📒 Files selected for processing (6)
Macterm/App/MactermApp.swiftMacterm/Ghostty/AdaptiveTerminalBackground.swiftMacterm/Ghostty/AdaptiveTerminalChrome.swiftMacterm/Ghostty/Theme.swiftMactermTests/Ghostty/AdaptiveTerminalBackgroundTests.swiftMactermTests/Ghostty/ThemeTests.swift
🚧 Files skipped from review as they are similar to previous changes (4)
- Macterm/Ghostty/AdaptiveTerminalBackground.swift
- Macterm/Ghostty/Theme.swift
- MactermTests/Ghostty/AdaptiveTerminalBackgroundTests.swift
- Macterm/Ghostty/AdaptiveTerminalChrome.swift
|
@OpenSource03 The improvements are applied but I want to discuss whether we need the background override setting. Ghostty already has a background setting in its config, and I want to avoid having duplicate responsibilities between ghostty and macterm as much as possible. Is there a reason you wanted the override setting? |
github-actions
Bot
removed
the
benchmark:improvement
I could see that from the way it was handled originally, yet I would, personally, much more prefer to have a color picker UI than having to edit Ghostty config. I did deliberately leave option to use Ghostty config, but I find it much easier to edit stuff via UI than via some config file, specially when it comes to colors. Additionally, if we go with such logic, isn't the background blur something Ghostty controls too? |
|
@OpenSource03 Yes Macterm does have its own opacity and blur settings which could seem redundant with ghostty config. But those are slightly different from ghostty's other appearance settings, because opacity and blur of the terminal surface needed to be always overridden to zero, to completely show through the app window's opacity and blur. Otherwise, the terminal panes' appearance can diverge from rest of the app's. There are a few more settings ignored from ghostty config (documented here) because Macterm explicitly overrides. Since ghostty config doesn't control these, I added them in the settings UI. When it comes to background color or any other color customization (e.g. foreground, selection-background, selection-foreground, etc.), Macterm still respects ghostty config for them. So unlike window opacity/blur, creating UI to edit those settings introduces two distinct ways to achieve the same thing. |
|
@OpenSource03 I'm happy to discuss more about the settings. But either way, I think it would have to be a separate discussion from this PR, as this mainly implements appearance adapting to fullscreen TUI. Would you be okay with removing the settings change in this PR and continue discussing in another? |
Hey @thdxg, sorry for the late reply. Yes, I can see your point, and it does make sense. I do also see benefit of having the override option with a color picker, but I will leave it up to you to decide. I will remove the color picker from this PR probably later today or tomorrow. |
Ghostty already owns background (and every other color) via its own config, so a Macterm-side override created a second way to set the same thing. Unlike window opacity/blur — which Macterm must pin to zero and therefore has to expose itself — nothing here requires Macterm to take ownership, so the setting is dropped and this PR stays focused on appearance adapting to full-screen TUIs. Removes the background source picker, the custom color picker and its persisted preference, the generated background/foreground override lines, and the debounced config reload that existed only to smooth color-picker drags. Adaptive detection, presentation, and chrome theming are untouched.
|
@thdxg Done! :) |
There was a problem hiding this comment.
@OpenSource03 Thanks! I've left some comments on the diff.
| return | ||
| } | ||
| guard adaptiveBackgroundColor != nil || color != nil else { return } | ||
| adaptiveBackgroundColor = color |
There was a problem hiding this comment.
Blocking: SwiftUI chrome has no observation path to the tint.
Compare adoptResolvedColors a few lines above — it does configVersion += 1 and posts. This only posts.
The notification reaches AppKit observers (WindowAppearance.sync, quick-terminal blur), but SwiftUI chrome reads MactermTheme.bg/fg/bgWithOpacity, which are plain static var computed accessors carrying no observation of their own. AppColorScheme is the only thing establishing that dependency, via _ = GhosttyApp.shared.configVersion.
adaptiveBackgroundColor is observable (not @ObservationIgnored), but nothing in the SwiftUI tree reads it — and MactermApp.swift now carries a comment saying it deliberately does not track it. So the sidebar and palette keep the old theme color until some unrelated event bumps configVersion. The screenshots showing the sidebar adapting suggest it currently works only because render activity keeps firing other config changes, i.e. accidentally.
Either bump configVersion here, or have something in the tree read adaptiveBackgroundColor. Worth saying which you intend, since the existing comment argues against the latter.
| return false | ||
| } | ||
|
|
||
| let candidate: NSColor? = if let surface = view.layer?.contents as? IOSurface { |
There was a problem hiding this comment.
Blocking: this reads a libghostty implementation detail, not a contract.
Macterm hands the raw NSView pointer to ghostty_surface_new (GhosttyTerminalNSView.swift:230) and libghostty creates and owns the layer. This reaches into that layer's contents and assumes an IOSurface in BGRA8. Citing "Ghostty's Metal Target.zig" is honest, but it's referencing upstream Zig internals from Swift across a C ABI that makes no such promise.
Given this repo's history — the fork-sync breakage and the #112 reflow overflow — an upstream change to the drawable path or pixel format makes this silently stop working, or worse, misinterpret bytes. The test suite won't catch it either, since it builds its own synthetic IOSurfaceCreate.
Minimum fix: guard the pixel format with IOSurfaceGetPixelFormat(surface) against BGRA rather than relying only on bytesPerElement >= 4, and record the fragility in CLAUDE.md's Known Limitations.
Better: drop pixel sampling for now and ship the OSC 11 path alone — see the summary comment.
| sampleTimer = timer | ||
| } | ||
|
|
||
| private func startMonitoring() { |
There was a problem hiding this comment.
Blocking: this timer never stops while the feature is enabled.
startMonitoring() guards on monitorTimer == nil and is called from preferenceDidEnable, focusDidChange, and terminalDidRender — but nothing tears it down except preferenceDidDisable. So once enabled, the app samples every visible pane twice per second forever, including a fully idle app with no TUI running. Each sample locks the IOSurface and reads ~4000 pixels per pane, contending with the renderer on its own live surface.
This contradicts the design this codebase already settled on for exactly this problem: PollCadence bursts, backs off, and stops entirely when nothing is on screen, specifically to avoid polling a static app.
It's also likely to move the workload-focused/workload-unfocused CPU and wakeups/s metrics in the benchmark harness that just landed (#183) — but since the feature is off by default the benchmark won't flag it, so the regression ships invisibly for anyone who enables it.
Suggest mirroring PollCadence: stop after N consecutive no-change samples, resume on a render or focus event.
| if #available(macOS 26.0, *) { | ||
| guard let glass = existingGlass(in: window) else { return } | ||
| glass.updateKeyStatus(window.isKeyWindow, backgroundColor: GhosttyApp.shared.backgroundColor) | ||
| glass.updateKeyStatus(window.isKeyWindow, backgroundColor: MactermTheme.nsBg) |
There was a problem hiding this comment.
Blocking (inconsistency): the glass tint helper still uses the heuristic this PR replaces.
This call now correctly reads MactermTheme.nsBg, but the helper it feeds (WindowAppearance.swift:175-176) computes isLightColor and adjustingSaturation from a plain luminance > 0.5 threshold — the exact heuristic this PR replaces elsewhere with WCAG contrast via ThemeColorMath.prefersDarkForeground.
So with a vivid TUI background, the glass overlay opacity is chosen by the misclassifying branch while nsFg uses the correct one. That's visible on precisely the saturated red/magenta cases semanticForegroundUsesContrastRatherThanRawBrightness was added to cover. Route isLightColor through the new contrast math too.
Separately: syncKeyStatus fires on windowDidBecomeKey/windowDidResignKey, which .mactermConfigDidChange does not trigger — worth confirming the tint is picked up on a plain tint change with no focus event, given the configVersion gap noted on GhosttyApp.swift.
| return true | ||
| } | ||
|
|
||
| private func effectiveCandidate(_ color: NSColor?) -> NSColor? { |
There was a problem hiding this comment.
Wrong baseline on split themes.
The 0.04 threshold suppresses colors too close to the configured background — but GhosttyApp.shared.backgroundColor is app-global and, per its own doc comment, "always collapse[s] a split to its light side" for theme = light:X,dark:Y. That's exactly why effectiveBackgroundColor/resolvedColors exist.
So on a split theme the baseline can be the wrong side, and a TUI whose background matches the dark configured background won't be suppressed. Compare against the resolved color instead.
Also: no test covers this suppression threshold.
| guard coverage >= minimumCoverage else { return nil } | ||
|
|
||
| return Match( | ||
| red: UInt8(winner.red / winner.count), |
There was a problem hiding this comment.
Trapping conversion guarded only by a non-obvious invariant.
winner.red / winner.count is Int division, so it truncates before the UInt8 conversion (minor, ±1). The bigger issue is that UInt8(_:) traps on out-of-range. It can't overflow today — every summand is ≤255 and the divisor is the count — but that's a subtle invariant protecting a crashing conversion in a path that reads renderer memory.
UInt8(clamping:) costs nothing and removes the reasoning burden entirely.
| /// lone pane. The window-wide tint follows the user's window opacity, so | ||
| /// on a translucent window it alone would leave a visible seam between | ||
| /// the TUI's opaque pixels and the pane padding around them. | ||
| static func paneColors(for paneColors: [NSColor?]) -> [NSColor?] { |
There was a problem hiding this comment.
This abstraction is an identity function.
paneColors(for:) returns its argument unchanged, and windowColor is a one-line ternary. Two of the new tests assert the behavior of an identity function, which doesn't buy much.
The doc comment explaining why pane colors pass through unchanged (the opaque-fill-vs-translucent-window seam) is genuinely valuable — I'd fold both functions into the call site in refreshPresentation and keep that comment there.
| final class AdaptiveTerminalChrome { | ||
| static let shared = AdaptiveTerminalChrome() | ||
|
|
||
| private var stabilizers: [ObjectIdentifier: AdaptiveTerminalBackgroundStabilizer] = [:] |
There was a problem hiding this comment.
ObjectIdentifier keys can be reused after dealloc.
Views are weakly held in liveViews, so a view can dealloc and a fresh one land on the same address, matching a stale stabilizer. pruneState runs every sample so the window is small, but the pane's UUID is stable, already threaded through this code, and would remove the hazard entirely.
|
|
||
| var body: some View { | ||
| Form { | ||
| Section("Terminal") { |
There was a problem hiding this comment.
Placing this above Section("Window") makes an off-by-default experimental feature the first thing in Appearance settings. Suggest moving it below Window.
The label also reads as an OS full-screen-mode setting rather than "full-screen TUI" — something like "Match terminal app backgrounds" is less ambiguous.
| Preferences.shared.adaptiveTerminalChromeEnabled = enabled | ||
| } | ||
| Text("Matches the whole window for a single pane; in a split, only each full-screen app's pane changes color.") | ||
| .font(.system(size: 11)) |
There was a problem hiding this comment.
Hardcoded font size — worth checking whether the other Settings descriptions use .caption/.footnote, and matching them.
|
@OpenSource03 I can fix conflicts and address the comments I made if you're okay with it! |
Hey! Sorry, I was on vacation for the past time, so I did not have time to go properly through those. I will handle your comments tomorrow or the day after. If you want to go over it yourself and ship earlier, feel free to let me know, it also works for me. |
|
@OpenSource03 No worries! No pressure on hurrying up. I'll leave it up to you! |
2 participants
What
Adds an opt-in Appearance setting that matches full-screen terminal apps such as OpenCode: a single pane can tint the whole window, while split panes adapt independently and leave the surrounding chrome unchanged.
Why
Full-screen TUIs can paint a background that clashes with transparent terminal padding, the sidebar, titlebar, and Liquid Glass.
Screenshots
Properly adapts to split view
With only single pane, whole app adapts to the TUI color
With feature off
How
Verified
Notes for reviewers
The feature is off by default. IOSurface is linked explicitly because sampling reads the renderer-owned BGRA8 surface directly; it does not use screen capture or request additional permissions.
Summary by CodeRabbit
Summary
New Features
Bug Fixes
Tests