Skip to content

Commit 8d3091b

Browse files
angusbezzinaclaude
andcommitted
fix(overlay): stop the hover-driven relayout storm that hid the pill
Two multipliers behind 'the menu disappears when I hover over it and then off it, on scrollable screens'. AXIntrospection.appElement() WROTE AXEnhancedUserInterface on every call, and the catcher calls it from the hover hit-test at up to 60Hz. That attribute announces an assistive client, and AppKit answers by re-evaluating and relaying out its windows -- so a moving pointer drove a resize storm in the host. Hovering ONTO the pill stops the writes (the pill consumes hover, so the catcher sees .ended and queries nothing) and moving OFF restarts them, which is exactly the reported trigger. Scrollable screens surfaced it first because a large scroll view is a large AX tree, so materialising it costs a real layout pass. It is now set once per process and the element is cached. Each resulting resize then pushed a fresh SwiftUI root view, tearing the pill down and rebuilding it mid-hover. syncFrameAndOrigin() now skips that when neither axOrigin nor surfaceSize changed, while still recording the host frame for the settle poll and still rebuilding on a REAL change. Both pinned by mutation-verified tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 3cb5eee commit 8d3091b

3 files changed

Lines changed: 155 additions & 5 deletions

File tree

Sources/AnnotKit/macOS/AXIntrospection.swift

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,39 @@ enum AXIntrospection {
9797

9898
// MARK: - Application element
9999

100-
/// Our own application AX element, with `AXEnhancedUserInterface` set so
101-
/// AppKit/SwiftUI materializes the full semantic tree for us. Self-query
102-
/// needs no accessibility-trust prompt.
100+
/// The process's application AX element, cached, with `AXEnhancedUserInterface`
101+
/// set EXACTLY ONCE so AppKit/SwiftUI materializes the full semantic tree for us.
102+
/// Self-query needs no accessibility-trust prompt.
103+
///
104+
/// Setting that attribute once is load-bearing, not an optimization. It was
105+
/// previously re-set on every call — and this is called from the HOVER hit-test,
106+
/// which the catcher runs at up to 60Hz while the pointer moves. Writing
107+
/// `AXEnhancedUserInterface` tells AppKit an assistive client just attached, and
108+
/// AppKit responds by re-evaluating (and, on SwiftUI hosts, relaying out) its
109+
/// windows; doing that 60 times a second drove a resize storm in the host, each
110+
/// resize firing `didResize` -> `syncFrameAndOrigin()` -> a fresh SwiftUI root
111+
/// view, which is what made the toolbar pill visibly vanish.
112+
///
113+
/// The reported trigger pinpointed it: hovering ONTO the pill stops the storm
114+
/// (the pill consumes hover, so the catcher sees `.ended` and queries nothing)
115+
/// and moving OFF it restarts them. It showed up on scrollable screens because a
116+
/// large scroll view is a large AX tree, so materializing it is far more likely
117+
/// to cost a real layout pass.
118+
///
119+
/// The element itself is stable for the life of the process, so caching it also
120+
/// drops an `AXUIElementCreateApplication` per query.
121+
private static var cachedAppElement: AXUIElement?
122+
/// How many times `AXEnhancedUserInterface` has been written. Must never exceed 1;
123+
/// the probe asserts it across a hover storm so the regression cannot come back
124+
/// silently.
125+
private(set) static var enhancedUserInterfaceWrites = 0
126+
103127
private static func appElement() -> AXUIElement {
128+
if let cachedAppElement { return cachedAppElement }
104129
let app = AXUIElementCreateApplication(ProcessInfo.processInfo.processIdentifier)
105130
AXUIElementSetAttributeValue(app, "AXEnhancedUserInterface" as CFString, kCFBooleanTrue)
131+
enhancedUserInterfaceWrites += 1
132+
cachedAppElement = app
106133
return app
107134
}
108135

Sources/AnnotKit/macOS/OverlayController.swift

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ public final class OverlayController: NSObject {
3535
private var axOrigin: CGPoint = .zero
3636
/// Panel-local size, for clamping the composer inside the visible region.
3737
private var surfaceSize: CGSize = .zero
38+
/// How many times a fresh SwiftUI root view has been pushed into the hosting view.
39+
/// Rebuilding it mid-hover is what makes the pill flicker or vanish, so the probe
40+
/// asserts a storm of redundant geometry notifications pushes none.
41+
private(set) var rootViewPushes = 0
3842

3943
/// Host frame captured at the last geometry sync. A SwiftUI/content-sized host
4044
/// can attach at its PRE-LAYOUT frame and grow to its final size a runloop turn
@@ -374,9 +378,21 @@ public final class OverlayController: NSObject {
374378
// the VISIBLE region rather than inside a window that runs off the display.
375379
// Clamping the TOP (a host tucked under the menu bar) genuinely does move the
376380
// origin, and that is the case a host-derived origin breaks silently.
377-
axOrigin = ScreenSpace.windowAXOrigin(cocoaFrame: panelFrame, primaryHeight: primaryHeight)
378-
surfaceSize = panelFrame.size
381+
let newAXOrigin = ScreenSpace.windowAXOrigin(cocoaFrame: panelFrame, primaryHeight: primaryHeight)
382+
// Always record the host frame, even on the early-out below: the settle poll
383+
// decides "has the host stopped growing" by comparing against this, so leaving
384+
// it stale would keep the poll re-syncing a window that has already settled.
379385
lastSyncedHostFrame = host.frame
386+
// Push a new SwiftUI root view ONLY when something it renders from actually
387+
// changed. `syncFrameAndOrigin()` runs on every move/resize/screen-parameter
388+
// notification, and a host that emits a burst of them (a relayout storm, a live
389+
// resize drag) would otherwise replace the root view on each one — tearing down
390+
// and rebuilding the pill mid-hover, which reads as the toolbar flickering or
391+
// vanishing. Cheap guard, and it makes a redundant notification free.
392+
guard newAXOrigin != axOrigin || panelFrame.size != surfaceSize else { return }
393+
axOrigin = newAXOrigin
394+
surfaceSize = panelFrame.size
395+
rootViewPushes += 1
380396
hostingView?.rootView = makeRootView()
381397
}
382398

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
#if os(macOS)
2+
import AppKit
3+
import CoreGraphics
4+
import Foundation
5+
import XCTest
6+
@testable import AnnotKit
7+
8+
/// The vanishing-pill regression, reported as: "the menu disappears when I hover over
9+
/// it and then off it, on scrollable screens."
10+
///
11+
/// Two multipliers, both fixed, both pinned here:
12+
///
13+
/// 1. `AXIntrospection.appElement()` WROTE `AXEnhancedUserInterface` on every call,
14+
/// and the catcher calls it from the hover hit-test at up to 60Hz. That attribute
15+
/// announces an assistive client; AppKit answers by re-evaluating and relaying out
16+
/// its windows, so a moving pointer drove a resize storm in the host.
17+
/// 2. Every resulting resize pushed a FRESH SwiftUI root view, tearing the pill down
18+
/// and rebuilding it mid-hover.
19+
///
20+
/// The reported trigger is what identified it: hovering ONTO the pill stops the
21+
/// writes — the pill consumes hover, so the catcher sees `.ended` and queries nothing
22+
/// — and moving OFF restarts them. Scrollable screens showed it first because a large
23+
/// scroll view is a large AX tree, so materializing it costs a real layout pass.
24+
@MainActor
25+
final class HoverStormTests: XCTestCase {
26+
/// A hover storm must write the AX attribute ZERO extra times.
27+
///
28+
/// `snapshot()` goes through the same `appElement()` every hover hit-test uses, so
29+
/// this exercises the real path without needing a live window. The first call in
30+
/// the process may legitimately write once; what must never happen again is a
31+
/// second write, let alone one per query.
32+
func testHoverStormDoesNotRewriteTheEnhancedUIAttribute() {
33+
_ = NSApplication.shared
34+
// Prime the cache so this test is independent of whichever test ran first.
35+
_ = AXIntrospection.snapshot()
36+
let writesAfterPriming = AXIntrospection.enhancedUserInterfaceWrites
37+
XCTAssertLessThanOrEqual(writesAfterPriming, 1, "the attribute is set once per process, not per query")
38+
39+
// ~3 seconds of real 60Hz hovering.
40+
for _ in 0 ..< 200 { _ = AXIntrospection.snapshot() }
41+
42+
XCTAssertEqual(AXIntrospection.enhancedUserInterfaceWrites, writesAfterPriming,
43+
"200 queries wrote AXEnhancedUserInterface again — this is the resize storm that ate the pill")
44+
}
45+
46+
/// Redundant geometry notifications must not rebuild the overlay's SwiftUI root.
47+
///
48+
/// A relayout storm arrives as a burst of `didResize`/`didMove`. None of them
49+
/// change the panel's geometry, so none may tear down and rebuild the pill — that
50+
/// rebuild is what the user SEES as the toolbar vanishing.
51+
func testRedundantGeometryNotificationsDoNotRebuildTheOverlay() {
52+
_ = NSApplication.shared
53+
let host = NSWindow(contentRect: NSRect(x: 200, y: 200, width: 640, height: 480),
54+
styleMask: [.titled, .resizable], backing: .buffered, defer: false)
55+
host.orderFront(nil)
56+
defer { host.orderOut(nil) }
57+
58+
let controller = OverlayController(
59+
session: AnnotationSession(source: MacElementSource(), sink: NotesFileSink(path: "/dev/null"))
60+
)
61+
controller.mount(on: host)
62+
defer { controller.unmount() }
63+
64+
let pushesBefore = controller.rootViewPushes
65+
let frameBefore = host.childWindows?.first?.frame
66+
67+
for _ in 0 ..< 50 {
68+
NotificationCenter.default.post(name: NSWindow.didResizeNotification, object: host)
69+
NotificationCenter.default.post(name: NSWindow.didMoveNotification, object: host)
70+
}
71+
72+
XCTAssertEqual(controller.rootViewPushes, pushesBefore,
73+
"100 redundant notifications rebuilt the pill — mid-hover that reads as it vanishing")
74+
XCTAssertEqual(host.childWindows?.first?.frame, frameBefore, "and the panel must not have moved")
75+
}
76+
77+
/// The guard must skip REDUNDANT syncs, not all of them.
78+
///
79+
/// An idempotence check that silently stopped syncing would be a far worse bug
80+
/// than the flicker it fixes: the panel would drift away from a host that really
81+
/// did move.
82+
func testARealResizeStillRebuildsTheOverlay() {
83+
_ = NSApplication.shared
84+
let host = NSWindow(contentRect: NSRect(x: 200, y: 200, width: 640, height: 480),
85+
styleMask: [.titled, .resizable], backing: .buffered, defer: false)
86+
host.orderFront(nil)
87+
defer { host.orderOut(nil) }
88+
89+
let controller = OverlayController(
90+
session: AnnotationSession(source: MacElementSource(), sink: NotesFileSink(path: "/dev/null"))
91+
)
92+
controller.mount(on: host)
93+
defer { controller.unmount() }
94+
95+
let pushesBefore = controller.rootViewPushes
96+
// MOVE the host rather than growing it. In idle mode the panel is a fixed-size
97+
// corner pinned to the host's BOTTOM edge, so growing the window upward leaves
98+
// the panel exactly where it was — the guard correctly skips that, and a test
99+
// built on it would be asserting the guard is broken.
100+
host.setFrameOrigin(NSPoint(x: host.frame.minX + 140, y: host.frame.minY + 90))
101+
NotificationCenter.default.post(name: NSWindow.didMoveNotification, object: host)
102+
103+
XCTAssertGreaterThan(controller.rootViewPushes, pushesBefore,
104+
"a real resize must still push a new root view, or the overlay stops tracking its host")
105+
}
106+
}
107+
#endif

0 commit comments

Comments
 (0)