Skip to content

Pre-composite backdrop#86

Merged
lawrencecchen merged 1 commit intomainfrom
hstack-sibling-buttons
Mar 30, 2026
Merged

Pre-composite backdrop#86
lawrencecchen merged 1 commit intomainfrom
hstack-sibling-buttons

Conversation

@lawrencecchen
Copy link
Copy Markdown

@lawrencecchen lawrencecchen commented Mar 30, 2026

Summary by cubic

Pre-composites the tab bar pane background over the window background and uses the flat color for the split-button backdrop. This removes double-compositing mismatches so the overlay matches the bar fill in both focused and unfocused states.

Written for commit 07e8fa0. Summary will update on new commits.

@lawrencecchen lawrencecchen merged commit 082fd92 into main Mar 30, 2026
1 check passed
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 30, 2026

Warning

Rate limit exceeded

@lawrencecchen has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 26 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 13 minutes and 26 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 33e772ac-3568-4ce7-ac9a-bd564a5be1e2

📥 Commits

Reviewing files that changed from the base of the PR and between 21626d5 and 07e8fa0.

📒 Files selected for processing (1)
  • Sources/Bonsplit/Internal/Views/TabBarView.swift
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hstack-sibling-buttons

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 30, 2026

Greptile Summary

This PR refactors the pre-compositing logic for the split-button backdrop from an inline closure in the view body into a reusable private static func precompositedPaneBackground(for:focused:) -> NSColor. The intent — flattening the translucent pane chrome over the window background to avoid double-compositing — is preserved. However, the extracted function inadvertently changes the color space of the returned NSColor from sRGB (original: NSColor(sRGBRed:…)) to calibrated RGB (new: NSColor(red:…)), even though the blended component values are computed in the sRGB space.

  • The refactor improves readability and reusability of the compositing math.
  • Regression: Line 546 uses NSColor(red:green:blue:alpha:) (creates an NSCalibratedRGBColorSpace color) instead of NSColor(sRGBRed:green:blue:alpha:) (sRGB). Because r, g, b are computed as sRGB values, wrapping them in a calibratedRGB color is semantically incorrect and partially defeats the purpose of this PR.

Confidence Score: 3/5

  • Not safe to merge as-is — the color-space regression partially reintroduces the compositing mismatch this PR intends to fix.
  • There is one clear P1 regression: the extracted static function uses NSColor(red:green:blue:alpha:) (calibratedRGB) instead of NSColor(sRGBRed:green:blue:alpha:) (sRGB). The blended values are sRGB-space, so packaging them as calibratedRGB produces a subtly wrong color, undoing the core goal of the PR. The fix is a one-word change on a single line.
  • Sources/Bonsplit/Internal/Views/TabBarView.swift — line 546, the NSColor initializer.

Important Files Changed

Filename Overview
Sources/Bonsplit/Internal/Views/TabBarView.swift Refactors the inline pre-compositing closure into a reusable static func precompositedPaneBackground(for:focused:), but introduces a regression: NSColor(red:green:blue:alpha:) (calibratedRGB) is used instead of the original NSColor(sRGBRed:green:blue:alpha:) (sRGB), mismatching the color space of the blended components.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["View body: showSplitButtons == true"] --> B["Call Self.precompositedPaneBackground(for: appearance, focused: isFocused)"]
    B --> C["Get chrome = TabBarColors.nsColorPaneBackground(for: appearance)"]
    B --> D["Get winBg = NSColor.windowBackgroundColor"]
    C & D --> E{"Convert both to .sRGB"}
    E -- "conversion fails" --> F["Return chrome.withAlphaComponent(1.0)"]
    E -- "success: fg, bk" --> G["Compute alpha 'a'\n(focused ? fg.alpha : fg.alpha × 0.95)"]
    G --> H["Blend: r/g/b = fg×a + bk×(1−a)"]
    H --> I["⚠️ NSColor(red:green:blue:alpha:)\ncalibratedRGB — should be sRGBRed:"]
    I --> J["Color(nsColor:) → SwiftUI Color 'bg'"]
    J --> K["LinearGradient + Rectangle backdrop\nusing 'bg'"]
    K --> L["ZStack: backdrop + splitButtons overlay"]
Loading

Reviews (1): Last reviewed commit: "Pre-composite pane bg over window bg for..." | Re-trigger Greptile

let r = fg.redComponent * a + bk.redComponent * oneMinusA
let g = fg.greenComponent * a + bk.greenComponent * oneMinusA
let b = fg.blueComponent * a + bk.blueComponent * oneMinusA
return NSColor(red: r, green: g, blue: b, alpha: 1.0)
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 Wrong NSColor initializer changes color space from sRGB to calibratedRGB

The extracted function blends components after converting both inputs to .sRGB (lines 537–538), so r, g, and b are sRGB-space values. The original inline code used NSColor(sRGBRed:green:blue:alpha:) to correctly wrap those values back into the sRGB color space.

The new code uses NSColor(red:green:blue:alpha:), which creates an NSCalibratedRGBColorSpace color — a different color space. The same numeric component values interpreted in calibrated RGB will render at a slightly different color than intended, reintroducing the compositing mismatch the PR aims to fix.

Suggested change
return NSColor(red: r, green: g, blue: b, alpha: 1.0)
return NSColor(sRGBRed: r, green: g, blue: b, alpha: 1.0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant