Skip to content

REOPEN-F7: a hidden sidebar icon button makes its image when it first shows - #33112

Merged
lawrencecchen merged 8 commits into
feat-cmux-nextfrom
fix/reopen-f7-lazy-button-image
Nov 3, 2026
Merged

lawrencecchen merged 8 commits into
feat-cmux-nextfrom
fix/reopen-f7-lazy-button-image

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

REOPEN-F7 (first window frame after a reopen, target p95 < 300 ms on the realistic launch bench). Profile 8 (xctrace, cmux19, job 72f71795e8b896e2f54c5ad4, 4 warm realistic reopens, both earlier fixes in) puts SidebarIconButton.init at 9-12 ms (profiled) of the window build before the first frame. renderIcon is about 4 ms of that, and the buttons' own layout another 2 ms.

Cause: each workspace row makes 5 icon buttons (close, audio, pin, tree fold, more) that stay hidden until the pointer is on the row. Each button made its template image (an NSImage with a bitmap representation per scale) at init.

Fix (1): renderIcon returns while the button is hidden or in a hidden row. It runs when the button shows (viewDidUnhide, which AppKit also sends when an ancestor unhides), joins a window, lays out, or is asked its intrinsic size. An icon, label or symbol change on a hidden button draws when it shows. A shown button has its image before it draws, so the look does not change (Sidebar-Look: no visual change). The shared glyph bitmaps from #32481 do not change.

Fix (2), from review at bc142a2 (P3): the row's Unpin button was not hidden at init, so it made its image when the row joined the window, before the row's first layout hid it. It now starts hidden, as the close, audio and tree fold buttons; it shows only through layoutPin / setHoverShown, which unhides a hidden view the same way. The More button is an OverflowButton, not changed here. So the saving is the 4 SidebarIconButtons per row (close, audio, pin, tree fold), not 5.

Gain

Gain: 108-112 fewer icon images per launch (4 hidden icon buttons per workspace row: close, audio, pin, tree fold; 24-25 rows plus 3 spare rows in the realistic launch, from debug.sidebar_perf row_views_made). Count tests: SidebarIconButtonLazyImageTests red on base (steps d90f890c, 9ffa7f7a), green on head (step 9d21cdb9). Time gain 1-4 ms main-thread CPU, under the bench's noise; no regression (A/B cmux21, 20 launches per arm). Accepted under the HOT PATH AUDIT rule (handoff/hot-path-audit/README.md item 3: a deterministic count when a time is too noisy).

Testing

Review

Review subagent: SHIP at 8a8ba37 (comment 6601535817); the earlier SHIP at bc142a2 (comment 6600179225) found the pin P3, fixed at 8a8ba37.

Observed / Not run

  • Observed: the fleet runs above.
  • Not run: the sidebar budgets suite (split-spring.sh --suite=budgets) runs on cmuxs-Mac-mini, which has the LaunchServices fault (STATUS 7775), so its timings are not valid now. No gallery capture: no look change.

Changelog

none

…OPEN-F7, red)

A reopened window's sidebar makes 5 icon buttons per workspace row,
hidden until hover, and each made its icon image at init: about 4 ms of
the profiled window build before the first frame (profile 8,
SidebarIconButton.renderIcon). These tests fail at the base: a hidden
button, or one in a hidden row, already has its image.
…t at init (REOPEN-F7)

Each workspace row makes 5 icon buttons (close, audio, pin, tree fold,
more) that stay hidden until the pointer is on the row; each made its
template image (an NSImage with a bitmap per scale) at init. Profile 8
(cmux19, job 72f71795): SidebarIconButton.renderIcon about 4 ms and the
buttons' own layout 2 ms of the window build before the first frame.

renderIcon now returns while the button is hidden or in a hidden row.
It runs when the button shows (viewDidUnhide, also sent for an
ancestor), joins a window, lays out, or is asked its intrinsic size. An
icon, label or symbol change on a hidden button draws when it shows.
The look does not change: a shown button has its image before it draws.
The shared glyph bitmaps (#32481) stay as they are.

Sidebar-Look: no visual change
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 3, 2026
@cursor

cursor Bot commented Nov 3, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 8a8ba37: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr33112 -f ref=fix/reopen-f7-lazy-button-image -f platform=macos -f backend_mode=local

web and size checks: green

Shipped in:

… placeholder

The green step 6bc3ff4c failed: a hidden button's image read as a
1 x 1 NSImage, the same object in every test. AppKit answers a button
that has no image of its own with a shared placeholder, so image == nil
never held. The test now counts an image as the icon only when it is
at least 2 points wide.

Sidebar-Look: no visual change
@blacksmith-sh

blacksmith-sh Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Found 1 test failure on Blacksmith runners:

Failure

Test View Logs
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)/
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)
View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

…wn state (green step 1844f51d)

aShownButtonHasItsImageInAWindowAndWithoutOne failed at d567840: a
button outside a window, asked its intrinsic size, kept AppKit's 1 x 1
placeholder image, because the check was image == nil and a button
with no image of its own answers that placeholder. A stack or a
constraint reading that size would size the button 1 x 1. The check is
now renderedSize == 0 (no icon made yet).

Sidebar-Look: no visual change
@cursor

cursor Bot commented Nov 3, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (subagent opus): SHIP at bc142a2

  • Every path that shows a button makes its image before it draws. viewDidUnhide fires each time isHiddenOrHasHiddenAncestor goes from true to false, so it covers the button's own unhide, an ancestor's unhide (setHoverShown, the pool's dequeue, spare rows) and both unhide orders. Joining a window while visible renders in viewDidMoveToWindow. An alpha-0 button that is not hidden renders when it joins the window. No change from the base there.
  • The hidden guard returns before renderedSize is read or written. A density or tunable change, or an icon, label or symbol change made while the button is hidden, is applied by the size compare on unhide. Nothing is lost.
  • No caller reads a SidebarIconButton's .image. Row, card and header layouts set frames and do not use intrinsic size. The emoji button in the stack view stays visible and gets its image from the intrinsicContentSize override. Snapshot and gallery paths draw in-window, where hidden views do not draw. VoiceOver uses setAccessibilityLabel, which init still sets.
  • The red/green logic is sound. On the base, 3 of 4 tests fail (test 3 passes there, as it should) and all pass at the head.
  • P3 (perf claim only): pinButton is not hidden at init, so it still makes its image when its row joins the window, before layout hides it. moreButton is an OverflowButton, which this PR does not change. Only close, audio and tree fold get the full saving. The pending A/B will show the real gain.
  • P3: the intrinsicContentSize getter sets image once, which invalidates the intrinsic size from inside a size read. This is harmless, but it is a side effect in a getter.

…h no image (review P3, red)

Review of #33112 at bc142a2 (P3): the row's Unpin button was not
hidden at init, so it made its image when the row joined the window,
before the row's first layout hid it. Fails at bc142a2 on the pin.

Sidebar-Look: no visual change
It shows only on hover of an expanded pinned row (layoutPin through
setHoverShown, which unhides a hidden view the same way), and the row's
first layout hid it anyway. Hidden at init, it makes no image before
the first frame, as the close, audio and tree fold buttons.

Sidebar-Look: no visual change
@cursor

cursor Bot commented Nov 3, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (subagent opus): SHIP at 8a8ba37

  • New since bc142a2: only pinButton.isHidden = true in WorkspaceRowView.init plus the test aNewRowsHoverControlsStartHidden. SidebarIconButton.swift and the rest of the PR are unchanged from the reviewed SHIP (merge-base 8f963a6, 3 files).
  • Show paths still work: layoutPin -> setHoverShown treats a hidden pin as appears (alpha 1 when not fading, a fade-in from 0 when fading, the same as close/more), and viewDidUnhide makes the image. The frame is set after that in both close-spot branches. placeCloseSpot's offset only moves frames. prepareForReuse does not touch the pin. The tests that expect a visible pin (SidebarSearchColumnTests, SidebarGridAlignmentTests, SidebarTrailingColumn*) lay out first.
  • No visual change: before, the row's first layout already hid an unshown pin, and its frame was .zero until then. Now it starts in that hidden state, so the person sees nothing different.
  • P3 (no action): the new test checks a row that is not in a window, so it proves the start state, not "no image when the row joins a window". The red/green on the pin's start state (9ffa7f7a / 9d21cdb9) matches the change. I did not compile or run anything; I read the code and used the author's fleet results.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 3, 2026
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 3, 2026 14:32 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 3, 2026 14:32 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 3, 2026 14:32 Inactive
@lawrencecchen
lawrencecchen merged commit 3b4a7b7 into feat-cmux-next Nov 3, 2026
142 of 147 checks passed
@lawrencecchen
lawrencecchen deleted the fix/reopen-f7-lazy-button-image branch November 3, 2026 14:43
lawrencecchen added a commit that referenced this pull request Nov 3, 2026
… first shows (stacked on #33112) (#33113)

* test(sidebar): a hidden icon button makes no image until it shows (REOPEN-F7, red)

A reopened window's sidebar makes 5 icon buttons per workspace row,
hidden until hover, and each made its icon image at init: about 4 ms of
the profiled window build before the first frame (profile 8,
SidebarIconButton.renderIcon). These tests fail at the base: a hidden
button, or one in a hidden row, already has its image.

* perf(sidebar): an icon button makes its image when it first shows, not at init (REOPEN-F7)

Each workspace row makes 5 icon buttons (close, audio, pin, tree fold,
more) that stay hidden until the pointer is on the row; each made its
template image (an NSImage with a bitmap per scale) at init. Profile 8
(cmux19, job 72f71795): SidebarIconButton.renderIcon about 4 ms and the
buttons' own layout 2 ms of the window build before the first frame.

renderIcon now returns while the button is hidden or in a hidden row.
It runs when the button shows (viewDidUnhide, also sent for an
ancestor), joins a window, lays out, or is asked its intrinsic size. An
icon, label or symbol change on a hidden button draws when it shows.
The look does not change: a shown button has its image before it draws.
The shared glyph bitmaps (#32481) stay as they are.

Sidebar-Look: no visual change

* test(sidebar): a hidden icon button registers no tooltip until it shows (REOPEN-F7, red)

Each workspace row's hover-only buttons registered their themed tooltip
(a tracker in the tooltip table, a tracking area) at init, before the
first window frame: about 1 ms profiled (profile 8). Fails at the base:
a hidden button already has its tracker.

* perf(sidebar): an icon button registers its tooltip when it first shows (REOPEN-F7)

Each workspace row's hover-only icon buttons put a tracker in the
tooltip table and a tracking area on themselves at init, before the
first window frame (TooltipTracker.setWhole, about 1 ms profiled in
profile 8), though a hidden button cannot be hovered.

applyToolTip registers the label as the themed tooltip once the button
shows (unhide, also of an ancestor, or joining a window). Until then it
sets only the VoiceOver help, so a hidden button still has its help.
A tooltip already registered follows a label change at once.

Sidebar-Look: no visual change

* test(sidebar): the lazy-image test reads the icon, not AppKit's 1 x 1 placeholder

The green step 6bc3ff4c failed: a hidden button's image read as a
1 x 1 NSImage, the same object in every test. AppKit answers a button
that has no image of its own with a shared placeholder, so image == nil
never held. The test now counts an image as the icon only when it is
at least 2 points wide.

Sidebar-Look: no visual change

* fix(sidebar): a button asked its size makes its icon, read from its own state (green step 1844f51d)

aShownButtonHasItsImageInAWindowAndWithoutOne failed at d567840: a
button outside a window, asked its intrinsic size, kept AppKit's 1 x 1
placeholder image, because the check was image == nil and a button
with no image of its own answers that placeholder. A stack or a
constraint reading that size would size the button 1 x 1. The check is
now renderedSize == 0 (no icon made yet).

Sidebar-Look: no visual change

* test(sidebar): a shortcut set before a button shows is kept; the tooltip reads back with no window (review P1, red)

Review of #33113 at e2c9ba9: showPriority sets the Priority bell's
themedToolTipShortcut on a sidebar that may have no window yet. With
the deferred registration, TooltipTracker.setWhole(nil, shortcut:)
returned early and the shortcut was lost (SidebarIconMeaningTests.
thePriorityBellNamesItsDotAndKeepsTheShortcutApart fails too). These
tests fail at e2c9ba9. The earlier tests now check the tooltip
tracking area, not the tracker table: the deferred part is the hover
tracking, not the text.

Sidebar-Look: no visual change

* fix(sidebar): keep a hidden button's tooltip text and shortcut; defer only its tracking area (review P1)

Review of #33113 at e2c9ba9 (P1): showPriority sets the Priority
bell's themedToolTipShortcut on a sidebar that may have no window yet.
The deferred registration made TooltipTracker.setWhole(nil, shortcut:)
return early, so the bell lost its shortcut, and themedToolTip read nil
before the button showed (SidebarIconMeaningTests.thePriorityBell...).

Now the tracker always records the text and the shortcut. Only the
hover tracking area waits: NSView.defersThemedToolTipTracking (opt-in;
only SidebarIconButton sets it, the 127 other themedToolTip users do
not change) keeps the area off while the view is hidden or has no
window, and themedToolTipCanShow() adds it when the button shows
(viewDidUnhide, viewDidMoveToWindow, and now viewDidMoveToSuperview,
the review's P3: a move from a hidden to a shown superview in the same
window). VoiceOver's help is set with the text, as before.

Sidebar-Look: no visual change

* test(sidebar): a new workspace row's hover controls start hidden, with no image (review P3, red)

Review of #33112 at bc142a2 (P3): the row's Unpin button was not
hidden at init, so it made its image when the row joined the window,
before the row's first layout hid it. Fails at bc142a2 on the pin.

Sidebar-Look: no visual change

* perf(sidebar): a workspace row's Unpin button starts hidden (review P3)

It shows only on hover of an expanded pinned row (layoutPin through
setHoverShown, which unhides a hidden view the same way), and the row's
first layout hid it anyway. Hidden at init, it makes no image before
the first frame, as the close, audio and tree fold buttons.

Sidebar-Look: no visual change

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 55c077e1 Deployed Nov 3, 2026 by cmux-bot[bot] via wait for same-key tree publisher #106480
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant