Skip to content

Commit f6352de

Browse files
authored
perf: cache portal split-divider regions off the hover/keyboard hot path (#18, upstream manaflow-ai#6587) (#30)
* fix: portal-hittest-cache (issue manaflow-ai#6587) [salvaged from ultracode worktree] * fix: invalidate divider-region cache on split structure changes (manaflow-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.
1 parent 903d53d commit f6352de

2 files changed

Lines changed: 125 additions & 73 deletions

File tree

Sources/BrowserWindowPortal.swift

Lines changed: 78 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,9 @@ final class WindowBrowserHostView: NSView {
276276
private struct DividerRegion {
277277
let rectInWindow: NSRect
278278
let isVertical: Bool
279+
/// True when the split view that owns this divider is a descendant of the
280+
/// portal host view (i.e. an inspector/internal split, not an app-layout split).
281+
let isInHostedContent: Bool
279282
}
280283

281284
private struct DividerHit {
@@ -325,6 +328,9 @@ final class WindowBrowserHostView: NSView {
325328
private var activeDividerCursorKind: DividerCursorKind?
326329
private var hostedInspectorDividerDrag: HostedInspectorDividerDragState?
327330
private var lastHostedInspectorLayoutBoundsSize: NSSize?
331+
// PERF: Cache split-divider regions to avoid recursive view-tree walk on every
332+
// pointer event. Invalidated on any geometry change.
333+
private var cachedDividerRegions: [DividerRegion]?
328334

329335
deinit {
330336
if let trackingArea {
@@ -376,6 +382,7 @@ final class WindowBrowserHostView: NSView {
376382

377383
override func viewDidMoveToWindow() {
378384
super.viewDidMoveToWindow()
385+
cachedDividerRegions = nil
379386
if window == nil {
380387
clearActiveDividerCursor(restoreArrow: false)
381388
}
@@ -384,11 +391,13 @@ final class WindowBrowserHostView: NSView {
384391

385392
override func setFrameSize(_ newSize: NSSize) {
386393
super.setFrameSize(newSize)
394+
cachedDividerRegions = nil
387395
window?.invalidateCursorRects(for: self)
388396
}
389397

390398
override func setFrameOrigin(_ newOrigin: NSPoint) {
391399
super.setFrameOrigin(newOrigin)
400+
cachedDividerRegions = nil
392401
window?.invalidateCursorRects(for: self)
393402
}
394403

@@ -419,9 +428,14 @@ final class WindowBrowserHostView: NSView {
419428

420429
override func resetCursorRects() {
421430
super.resetCursorRects()
422-
guard let rootView = dividerSearchRootView() else { return }
423-
var regions: [DividerRegion] = []
424-
Self.collectSplitDividerRegions(in: rootView, into: &regions)
431+
// A split add/remove can change divider geometry without changing the host
432+
// frame, so frame-hook invalidation alone is insufficient (#6587 review).
433+
// Drop the cache here so the warm below reflects current structure;
434+
// resetCursorRects is driven by invalidateCursorRects, not per pointer event.
435+
cachedDividerRegions = nil
436+
guard window != nil else { return }
437+
// Warms the cache. Subsequent pointer events avoid the recursive walk.
438+
let regions = dividerRegions()
425439
let expansion: CGFloat = 4
426440
for region in regions {
427441
var rectInHost = convert(region.rectInWindow, from: nil)
@@ -468,6 +482,27 @@ final class WindowBrowserHostView: NSView {
468482
}
469483

470484
override func hitTest(_ point: NSPoint) -> NSView? {
485+
// PERF: hitTest is called on EVERY event including keyboard. Keep non-pointer
486+
// path minimal. Mirror the isPointerEvent guard from WindowTerminalHostView.
487+
let currentEvent = NSApp.currentEvent
488+
let isPointerEvent: Bool
489+
switch currentEvent?.type {
490+
case .mouseMoved, .mouseEntered, .mouseExited,
491+
.leftMouseDown, .leftMouseUp, .leftMouseDragged,
492+
.rightMouseDown, .rightMouseUp, .rightMouseDragged,
493+
.otherMouseDown, .otherMouseUp, .otherMouseDragged,
494+
.scrollWheel, .cursorUpdate:
495+
isPointerEvent = true
496+
default:
497+
isPointerEvent = false
498+
}
499+
500+
if !isPointerEvent {
501+
// Non-pointer event: skip divider/drag routing, just do standard hit testing.
502+
let hitView = super.hitTest(point)
503+
return hitView === self ? nil : hitView
504+
}
505+
471506
let dividerHit = splitDividerHit(at: point)
472507
let hostedInspectorHit = dividerHit == nil ? hostedInspectorDividerHit(at: point) : nil
473508
updateDividerCursor(at: point, dividerHit: dividerHit, hostedInspectorHit: hostedInspectorHit)
@@ -825,11 +860,36 @@ final class WindowBrowserHostView: NSView {
825860
}
826861
}
827862

863+
// PERF: Returns the cached divider regions (including isInHostedContent), computing
864+
// and caching on first call. The cache is invalidated by setFrameSize / setFrameOrigin /
865+
// viewDidMoveToWindow. Do NOT call from non-pointer-event paths.
866+
private func dividerRegions() -> [DividerRegion] {
867+
if let cached = cachedDividerRegions { return cached }
868+
guard let rootView = dividerSearchRootView() else {
869+
cachedDividerRegions = []
870+
return []
871+
}
872+
var regions: [DividerRegion] = []
873+
Self.collectSplitDividerRegions(in: rootView, hostView: self, into: &regions)
874+
cachedDividerRegions = regions
875+
return regions
876+
}
877+
828878
private func splitDividerHit(at point: NSPoint) -> DividerHit? {
829879
guard window != nil else { return nil }
830880
let windowPoint = convert(point, to: nil)
831-
guard let rootView = dividerSearchRootView() else { return nil }
832-
return Self.dividerHit(at: windowPoint, in: rootView, hostView: self)
881+
let expansion: CGFloat = 5
882+
for region in dividerRegions() {
883+
// Mirror the original dividerHit expansion: expand in all directions.
884+
let expanded = region.rectInWindow.insetBy(dx: -expansion, dy: -expansion)
885+
if expanded.contains(windowPoint) {
886+
return DividerHit(
887+
kind: region.isVertical ? .vertical : .horizontal,
888+
isInHostedContent: region.isInHostedContent
889+
)
890+
}
891+
}
892+
return nil
833893
}
834894

835895
private func dividerSearchRootView() -> NSView? {
@@ -1084,65 +1144,6 @@ final class WindowBrowserHostView: NSView {
10841144
#endif
10851145
return (pageFrame, inspectorFrame)
10861146
}
1087-
private static func dividerHit(
1088-
at windowPoint: NSPoint,
1089-
in view: NSView,
1090-
hostView: WindowBrowserHostView
1091-
) -> DividerHit? {
1092-
guard !view.isHidden else { return nil }
1093-
1094-
if let splitView = view as? NSSplitView {
1095-
let pointInSplit = splitView.convert(windowPoint, from: nil)
1096-
if splitView.bounds.contains(pointInSplit) {
1097-
let expansion: CGFloat = 5
1098-
let dividerCount = max(0, splitView.arrangedSubviews.count - 1)
1099-
for dividerIndex in 0..<dividerCount {
1100-
let first = splitView.arrangedSubviews[dividerIndex].frame
1101-
let second = splitView.arrangedSubviews[dividerIndex + 1].frame
1102-
let thickness = splitView.dividerThickness
1103-
let dividerRect: NSRect
1104-
if splitView.isVertical {
1105-
// Keep divider hit-testing active even when one side is nearly collapsed,
1106-
// so users can drag the divider back out from the border.
1107-
// But ignore transient states where both panes are effectively 0-width.
1108-
guard first.width > 1 || second.width > 1 else { continue }
1109-
let x = max(0, first.maxX)
1110-
dividerRect = NSRect(
1111-
x: x,
1112-
y: 0,
1113-
width: thickness,
1114-
height: splitView.bounds.height
1115-
)
1116-
} else {
1117-
// Same behavior for horizontal splits with a near-zero-height pane.
1118-
guard first.height > 1 || second.height > 1 else { continue }
1119-
let y = max(0, first.maxY)
1120-
dividerRect = NSRect(
1121-
x: 0,
1122-
y: y,
1123-
width: splitView.bounds.width,
1124-
height: thickness
1125-
)
1126-
}
1127-
let expanded = dividerRect.insetBy(dx: -expansion, dy: -expansion)
1128-
if expanded.contains(pointInSplit) {
1129-
return DividerHit(
1130-
kind: splitView.isVertical ? .vertical : .horizontal,
1131-
isInHostedContent: splitView.isDescendant(of: hostView)
1132-
)
1133-
}
1134-
}
1135-
}
1136-
}
1137-
1138-
for subview in view.subviews.reversed() {
1139-
if let hit = dividerHit(at: windowPoint, in: subview, hostView: hostView) {
1140-
return hit
1141-
}
1142-
}
1143-
1144-
return nil
1145-
}
11461147

11471148
private static func verticalOverlap(between lhs: NSRect, and rhs: NSRect) -> CGFloat {
11481149
max(0, min(lhs.maxY, rhs.maxY) - max(lhs.minY, rhs.minY))
@@ -1187,7 +1188,11 @@ final class WindowBrowserHostView: NSView {
11871188
view.frame.height > 1
11881189
}
11891190

1190-
private static func collectSplitDividerRegions(in view: NSView, into result: inout [DividerRegion]) {
1191+
private static func collectSplitDividerRegions(
1192+
in view: NSView,
1193+
hostView: WindowBrowserHostView,
1194+
into result: inout [DividerRegion]
1195+
) {
11911196
guard !view.isHidden else { return }
11921197

11931198
if let splitView = view as? NSSplitView {
@@ -1211,14 +1216,15 @@ final class WindowBrowserHostView: NSView {
12111216
result.append(
12121217
DividerRegion(
12131218
rectInWindow: dividerRectInWindow,
1214-
isVertical: splitView.isVertical
1219+
isVertical: splitView.isVertical,
1220+
isInHostedContent: splitView.isDescendant(of: hostView)
12151221
)
12161222
)
12171223
}
12181224
}
12191225

12201226
for subview in view.subviews {
1221-
collectSplitDividerRegions(in: subview, into: &result)
1227+
collectSplitDividerRegions(in: subview, hostView: hostView, into: &result)
12221228
}
12231229
}
12241230

@@ -2324,6 +2330,11 @@ final class WindowBrowserPortal: NSObject {
23242330
reason: "externalGeometry"
23252331
)
23262332
}
2333+
// Split add/remove fires this (via NSSplitView.didResizeSubviewsNotification)
2334+
// without changing the host frame, so force a cursor-rect rebuild → the host
2335+
// drops its stale divider-region cache (see resetCursorRects) and the new
2336+
// divider is grabbable (#6587 review).
2337+
hostView.window?.invalidateCursorRects(for: hostView)
23272338
}
23282339

23292340
@discardableResult

Sources/TerminalWindowPortal.swift

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ final class WindowTerminalHostView: NSView {
5050
private var sidebarDividerMissCount = 0
5151
private var trackingArea: NSTrackingArea?
5252
private var activeDividerCursorKind: DividerCursorKind?
53+
// PERF: Cache split-divider regions to avoid recursive view-tree walk on every
54+
// pointer event. Invalidated on any geometry change.
55+
private var cachedDividerRegions: [DividerRegion]?
5356
#if DEBUG
5457
private var lastDragRouteSignature: String?
5558
#endif
@@ -63,6 +66,7 @@ final class WindowTerminalHostView: NSView {
6366

6467
override func viewDidMoveToWindow() {
6568
super.viewDidMoveToWindow()
69+
cachedDividerRegions = nil
6670
if window == nil {
6771
clearActiveDividerCursor(restoreArrow: false)
6872
}
@@ -71,19 +75,28 @@ final class WindowTerminalHostView: NSView {
7175

7276
override func setFrameSize(_ newSize: NSSize) {
7377
super.setFrameSize(newSize)
78+
cachedDividerRegions = nil
7479
window?.invalidateCursorRects(for: self)
7580
}
7681

7782
override func setFrameOrigin(_ newOrigin: NSPoint) {
7883
super.setFrameOrigin(newOrigin)
84+
cachedDividerRegions = nil
7985
window?.invalidateCursorRects(for: self)
8086
}
8187

8288
override func resetCursorRects() {
8389
super.resetCursorRects()
84-
guard let window, let rootView = window.contentView else { return }
85-
var regions: [DividerRegion] = []
86-
Self.collectSplitDividerRegions(in: rootView, into: &regions)
90+
// A split add/remove can change divider geometry without changing the host
91+
// frame, so the frame-hook invalidation alone is insufficient (#6587 review).
92+
// Drop the cache here so the warm below always reflects current structure;
93+
// resetCursorRects is driven by invalidateCursorRects, not per pointer event,
94+
// so this keeps the cache fresh per cursor-rect cycle without re-walking the
95+
// tree on every hover.
96+
cachedDividerRegions = nil
97+
guard window != nil else { return }
98+
// Warms the cache. Subsequent pointer events avoid the recursive walk.
99+
let regions = dividerRegions()
87100
let expansion: CGFloat = 4
88101
for region in regions {
89102
var rectInHost = convert(region.rectInWindow, from: nil)
@@ -275,11 +288,33 @@ final class WindowTerminalHostView: NSView {
275288
}
276289
}
277290

291+
// PERF: Returns the cached divider regions, computing and caching on first call.
292+
// The cache is invalidated by setFrameSize / setFrameOrigin / viewDidMoveToWindow.
293+
// Do NOT call from non-pointer-event paths.
294+
private func dividerRegions() -> [DividerRegion] {
295+
if let cached = cachedDividerRegions { return cached }
296+
guard let rootView = window?.contentView else {
297+
cachedDividerRegions = []
298+
return []
299+
}
300+
var regions: [DividerRegion] = []
301+
Self.collectSplitDividerRegions(in: rootView, into: &regions)
302+
cachedDividerRegions = regions
303+
return regions
304+
}
305+
278306
private func splitDividerCursorKind(at point: NSPoint) -> DividerCursorKind? {
279-
guard let window else { return nil }
307+
guard window != nil else { return nil }
280308
let windowPoint = convert(point, to: nil)
281-
guard let rootView = window.contentView else { return nil }
282-
return Self.dividerCursorKind(at: windowPoint, in: rootView)
309+
let expansion: CGFloat = 5
310+
for region in dividerRegions() {
311+
// Mirror the original dividerCursorKind expansion: expand in all directions.
312+
let expanded = region.rectInWindow.insetBy(dx: -expansion, dy: -expansion)
313+
if expanded.contains(windowPoint) {
314+
return region.isVertical ? .vertical : .horizontal
315+
}
316+
}
317+
return nil
283318
}
284319

285320
static func hasSplitDivider(atScreenPoint screenPoint: NSPoint, in window: NSWindow) -> Bool {
@@ -774,6 +809,12 @@ final class WindowTerminalPortal: NSObject {
774809
synchronizeLayoutHierarchy()
775810
synchronizeAllHostedViews(excluding: nil)
776811
reconcileVisibleHostedViewsAfterGeometrySync(reason: "portal.externalGeometrySync")
812+
// This fires on NSSplitView.didResizeSubviewsNotification (split add/remove),
813+
// which can change divider geometry without changing the host frame — so the
814+
// host's frame hooks never run. Force a cursor-rect rebuild so the host drops
815+
// its stale divider-region cache (see resetCursorRects) and the new divider is
816+
// grabbable (#6587 review).
817+
hostView.window?.invalidateCursorRects(for: hostView)
777818
}
778819

779820
private func ensureDividerOverlayOnTop() {

0 commit comments

Comments
 (0)