Repository navigation
perf(layout): one stationary tracking area hit-tests the dividers for hover and the resize cursor; no window region rebuild per spring frame - #34589
Conversation
…: cursor rects invalidated as dividers move); DividerHandleView.cursor(atLocal:)
… pointer event, not from cursor rects Moving dividers (every frame of a spring or a drag) and the cursor clip applyPresentation sets on them every frame made AppKit invalidate their cursor rects, and each invalidation rebuilt the window's structural regions in that display cycle: drag margins and a constraint min-size walk of every view (-[NSWindow _resetDragMarginsIfNeeded]: 129 of 996 busy main-thread ms of a STRESS divider drag, fleet job 02225774; 206 invalidations in 36 frames of a 12-pane spring, job 8d0c5e73). Chrome sets the cursor from the hit test of each mouse event, never from rects kept per frame; the divider now does the same: cursorUpdate and mouseMoved in its existing tracking area pick the resize cursor inside its clip (cursor(atLocal:)), the arrow when the pointer leaves the clip inside the area. No cursor rects, no invalidation on a move, an axis change or a new clip. Red: DividerCursorNoRegionRebuildTests (6998b42).
…-divider-cursor-hit-test
…t set their cursor from a hit test The first change took the divider's cursor rects away, but AppKit invalidates the cursor rects of every view whose frame moves, rects or not: 240 divider invalidations in 36 spring frames remained (fleet step b971c2f84d5c01b4c8bb186e), each one scheduling a rebuild of the window's structural regions. HitTestCursorView (CmuxNextDesign) marks a view that adds no cursor rects and sets its cursor from a hit test of the pointer event (as Chrome); ShellWindow.invalidateCursorRects(for:) returns at once for it, as it already does for a held invalidation. DividerHandleView adopts it. The test's clip check now uses the flipped handle's coordinates.
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); NEW: green on the base c084488; do not merge until green; see lane-rules Shipped in:
|
|
Review (hpa-frames-reviewer-8): FIX at 07e7820 Correctness first. The handle-side change (cursor from 1. Blocking: the guard may stop AppKit from moving the handle's tracking area (the resize cursor could go missing after a divider moves).
2. The other cursor cases look OK (not blocking):
3. Test honesty: the spy swizzles 4. Layering: a marker in CmuxNextDesign, adopted in Layout and checked in App, is fine. The guard is limited to views that adopt the protocol, and today only perf claim: none (count tests only) |
|
Author (hpa-frames): I accept the FIX. The window guard drops AppKit's The next design removes the guard and needs no AppKit bookkeeping skipped:
Before it is ready: (a) the invalidation count without a guard on a display worker (a view with no areas may get no invalidation at all; this must be measured, not assumed), and (b) a real-HID hover journey: move a divider, point at its new place and expect the resize cursor, then point at its old place and expect no resize cursor. |
…vider-cursor-hit-test
…e dividers for hover and the resize cursor A divider's cursor rects and its own tracking area moved with its frame, and AppKit rebuilt the window's structural regions (drag margins, a constraint min-size walk of every view) for each move: 206-240 invalidations in the 36 frames of a 12-pane spring (jobs 8d0c5e73, b971c2f8), 14.6 % of a STRESS divider drag's busy main thread (job 02225774). A divider with neither gets 0 (job 5146dc44). The handles now have no tracking area and no cursor rects. The screen view, which does not move, owns one tracking area; its enter, exit and move events, and every presentation pass under a still pointer, hit-test the current handle frames (ScreenDividers.refreshHover) for the hover and the resize cursor (ScreenDividerPointer), as Chrome sets the cursor from the hit test of each mouse event. No AppKit call is skipped: this takes back the window guard of the previous head, which review found could leave a moved handle's tracking area stale. Tests: DividerCursorNoRegionRebuildTests counts invalidations over a spring (display worker) and checks the cursor leaves a divider's old place and follows it to the new one under a still pointer.
…builds) For the real-pointer hover check of the divider cursor (a journey moves the real pointer to a divider's new and old places and reads which cursor shows).
…tRootView.debugDividerCenter)
… cursor object per axis NSEvent.trackingArea is valid only for enter, exit and cursor-update events: the screen view read it on mouse moves to tell its own moves from moves passed up its responder chain. The area's owner is now ScreenDividerPointer (an NSResponder), so its events come only there and the screen view keeps no mouse overrides. On leaving a divider the view under the pointer gets its cursor rects read again (one invalidation for that one change), not a cursorUpdate call with a move event.
|
Found 6 test failures on Blacksmith runners: Failures
|
…esize cursor back Review FIX (independent reviewer, cf29a8c): a screen hidden by a workspace switch under a still pointer on its divider kept the resize cursor (a hidden view gets no tracking events and its handles did not move). viewDidHide and leaving the window give it back, applyScreenFrames refreshes the divider hover of a screen that slid, faded or hid, and a faded-out screen is not hoverable. The cursor hand-back reads the pointer the hover hit test read (debug.mouse's too). The test-only cursor(atLocal:) is gone; the tests also check the cursor the app shows.
…kspace switch with the pointer on the divider (reviewer's evidence for #34589)
…n turn before the spring frames
…s resize cursor; debug.cursor reports the hover The real-pointer journey px-divider-hover (build 34fc0b3e, cmux22) read the I-beam with the pointer on a column edge. AppKit sends cursorUpdate to the view under the pointer when the pointer crosses any cursor area (a terminal's I-beam rect beside the handle); the handle had dropped its answer with its cursor rects. It answers again inside its clip (no tracking area of its own, so nothing is invalidated when it moves). debug.cursor also reports the key window's hover pointer and each screen's hovered divider and resize cursor, so a real-pointer run tells a missed event from a lost race.
…nswer, and the dividers whose frames hold the pointer
…a window (red: px-divider-hover showed no hover event)
… a window; a handle's cursorUpdate cursor is given back too px-divider-hover on build 0ebd88ef (cmux24): the pointer sat on a column edge (debug.cursor: inside the handle's clip, our window topmost) and no screen had the divider hovered, while the control build of feat (f6f59043, the handle's own tracking area) did. AppKit calls updateTrackingAreas when tracking areas need recomputing, not for a view that has none, so the screen's area may never have been added. viewDidMoveToWindow adds it. The same runs kept the resize cursor after a workspace switch: a handle's cursorUpdate set it with the screen not knowing; the screen now records it and gives it back on hide.
|
Review (independent-reviewer): SHIP at cdb8ef3 Condition from the reviewer and the lead: a real-pointer check before landing. The automated journey px-divider-hover cannot pass until the hid_inject PostEvent grant (STATUS 5064). Substitute the reviewer accepts: a hand check on fleet build e66921083bd588e9b50a07af (tag hpa-divider-hover-v6, head cdb8ef3) with debug.cursor read at each step: terminal to divider (resize cursor), divider to terminal (I-beam), a spring moves the divider from a still pointer (I-beam), a workspace switch with the pointer on a divider (no resize cursor), a drag past the pane minimum (resize cursor until release). The PR stays DRAFT, with no ready-to-land label, until that check is done or the lead decides to land without it. |
|
This PR added |
…me table (#35440) Two cmux-next checks have been red on feat-cmux-next since this morning: - Web bundle key inputs: #35205 made webviews/src/ui/tooltipTokens.ts import schemas/design-scale/design-scale.json, which the bundle key hashes but the publish workflow's push paths and cmux_next_route.py BUNDLE_INPUTS did not list. Both lists now name it. - Default cursor: #34589 added NSCursor+DebugName.swift, a DEBUG-only table that names the standard cursors (pointingHand among them) for debug.cursor. It sets no cursor; it goes on the allowlist with that reason, like DebugTerminalLinks.swift. Agent-Session: https://cmux.com/deeplink/agent?id=aid-d750cf10922c785efa64 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…eView) (#35424) * base red: record NSImageView viewDidUnhide and NSView cursorUpdate in the selector table The cmux-next crash-class job (check-objc-selectors) was red on feat-cmux-next with "probe needed" for two new super calls: - ThemedImageView.viewDidUnhide (CmuxNextDesign/ThemedLabel.swift:92), from 3cd55d9 - DividerHandleView.cursorUpdate(with:) (CmuxNextLayout/Views/DividerHandleView.swift:189), from #34589 Both selectors exist (viewDidUnhide on NSView, cursorUpdate: on NSResponder), so the code is right and only the table was stale. Table written by the fleet probe, not by hand: cmux-ci run --class light --script scripts/cmux-next/check-objc-selectors.sh --ref a727959 --arg=--probe --arg=--write (step ba81ce229443823aee642f21 on aws-m4pro-4, macOS 27.0.1, artifact sha256:8e0f4184...afaa). Red: CI runs 41832548265 and 41831415899, 2 failures. Green: python3 -I scripts/cmux-next/check-objc-selectors.py -> 0 failure(s). Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> * base red: the selector table's macOS 26.5.2 answers for every key (fleet probe) The same fleet probe on a macOS 26 worker: cmux-ci run --class light --script scripts/cmux-next/check-objc-selectors.sh --ref 485483c --target-worker aws-m4pro-5 --arg=--probe --arg=--write (step 96532333b1474648ee23d712, macOS 26.5.2, artifact sha256:785e7a9e...2a23). It adds 26.5.2 to the two new keys and to 60 keys that only macOS 27 had recorded; every answer is true, so no call is held to macOS 27 only. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Summary
A divider set its resize cursor with AppKit cursor rects and had its own tracking area. AppKit moves a view's cursor rects and tracking areas with its frame, and a divider moves in every frame of a spring or a drag. Each move made AppKit rebuild the window's structural regions in that display cycle: the drag margins and a constraint min-size walk of every view (
-[NSWindow _resetDragMarginsIfNeeded]). That walk took 129 of 996 busy main-thread ms of a STRESS divider drag (fleet job 02225774068545169e3b1901). A probe counted 206 divider invalidations in the 36 frames of a 12-pane spring with cursor rects (job 8d0c5e7398e6433bcc620037), 240 with only the handle's own tracking area (job b971c2f84d5c01b4c8bb186e), and 0 with neither (job 5146dc44b2a16965266b6aeb). The floor is 0: no window edge moves.Chrome sets the cursor from the hit test of each mouse event and keeps no rects per frame. This PR does the same, and skips no AppKit call:
DividerHandleViewhas no cursor rects and no tracking area.setAxisandcursorClipinvalidate nothing.ScreenContentView, which does not move, has one tracking area, added when it joins a window. Its ownerScreenDividerPointer(anNSResponder, so its events never mix with the screen's responder chain) gets enter, exit and move events. They, and each presentation pass under a still pointer, hit-test the current handle frames (ScreenDividers.refreshHover, which already decided hover). The same hit sets the hover and the resize cursor. When the pointer leaves a divider, the view under it gets its cursor rects read again, once.cursorUpdateto the view under the pointer when the pointer crosses a neighbor's cursor area (a terminal's I-beam rect). The handle answers it with its resize cursor inside its clip, as its cursor rect did.viewDidHide,applyScreenFrames, leaving the window).debug.cursor(DEBUG and bench builds) names the cursor the app shows (NSCursor.debugName), for the real-pointer journey below. Thedebug.mousedivider lookup moved toCmuxNextLayout(LayoutRootView.debugDividerCenter); the App target is 1 line smaller.An earlier head of this PR skipped
NSWindow.invalidateCursorRects(for:)for the dividers. Review found it could leave a moved divider's tracking area stale; this head takes that back.HOT PATH AUDIT frames (handoff/hot-path-audit/frames/AUDIT.md, paths
motion-frameanddrag-frame). PL-motion-core was told in STATUS 10236 that I take this change.Testing
CMUX_TEST_REQUIRE_GUI=1, proof-step.py):scripts/ci/package-test-lane.sh suite Packages/macOS/CmuxNext DividerCursorNoRegionRebuildTests.aScreenThatHidesUnderThePointerGivesTheResizeCursorBack(the hidden screen kept the resize cursor).theScreenHasItsDividerTrackingAreaInAWindow(0 areas).DividerCursorNoRegionRebuildTests(0 divider invalidations in 36 spring frames; 2 other invalidations in the window), andDividerCursorUnderDockTests,DividerCatcherMotionTests,DividerDragFollowsPointerTests,DividerHandleLineTests,DividerGlowBandTests.px-divider-hover(branch perf/hpa-frames-px-divider-hover, on the PIXEL PERFECT kit), 7 rounds on cmux22 and cmux24. It found 2 defects that the unit tests missed (no answer tocursorUpdate; no tracking area until a layout change), both fixed above with tests. It cannot pass yet: CuaDrivermove_cursorwarps the pointer and posts no mouse-moved event, and the injector that posts real events (hid_inject) has no PostEvent grant on the minis (grant item STATUS 5064). Control: the same warp on feat (build f6f59043) also showed the I-beam at the divider's new place.Observed / Not run
PaneHostView.setFrameSizestill dirties tracking areas; that is a separate change.Performance
perf claim: none measured in time. The count test proves 206-240 divider invalidations become 0 per spring.
Review (independent-reviewer): SHIP at cdb8ef3. Its three DEBUG-only notes on
debug.cursor(a cache write on read, a no-op convert, keys of two layout roots) go to a follow-up PR.Changelog
none
No capture: the cursor looks the same; only how it is set changes.