perf: cache portal split-divider regions off the hover/keyboard hot path (#18, upstream #6587)#30
Merged
Merged
Conversation
…racode worktree]
…flow-ai#6587 review) The adversarial review found a stale-cache bug: the cache was invalidated only on the host view's frame hooks (setFrameSize/setFrameOrigin/viewDidMoveToWindow), but adding/removing a split pane divides space WITHIN the host's fixed boundary — the host frame doesn't change, so none of those fire and the cache held stale divider regions. Result: a newly-added divider was ungrabbable (cursor stayed arrow, clicks passed through to the surface) until something else resized the host. Fix (both WindowTerminalHostView/WindowTerminalPortal and the Browser equivalents): - resetCursorRects() now drops `cachedDividerRegions` before re-warming, so the cache is fresh per cursor-rect cycle (not per pointer event — perf preserved). - synchronizeAllEntriesFromExternalGeometryChange() (driven by NSSplitView.didResizeSubviewsNotification) now calls invalidateCursorRects(for:) on the host, forcing resetCursorRects → cache drop, even when the host frame is unchanged. So a split add/remove makes the new divider immediately grabbable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #18 (upstream cmux manaflow-ai#6587). Portal host views recursively walked the entire AppKit view tree to find split-divider regions on every cursorUpdate/mouseMoved/hitTest (and, for the browser portal, every keyboard event too) — a typing-latency hot path.
Fix: cache the divider regions (
cachedDividerRegions), invalidate on geometry change, and add the missingisPointerEventguard toBrowserWindowPortal.hitTestso keyboard events skip the work entirely.Adversarial review caught a stale-cache bug (and it's fixed here): a split add/remove divides space within the host's fixed frame, so the frame hooks never fire and the cache went stale → the new divider was ungrabbable. Now
resetCursorRects()drops the cache before re-warming, and the split-change sync path (NSSplitView.didResizeSubviewsNotification) forcesinvalidateCursorRects, so a new divider is immediately grabbable. Cache is per-cursor-rect-cycle, so the per-event walk is still eliminated.Triaged + implemented + adversarially reviewed via ultracode; the review found and the fix addresses the stale-cache regression.