Skip to content

Commit 3267df7

Browse files
angusbezzinaclaude
andcommitted
fix(overlay): re-assert the clamped panel frame after AppKit's parent-follow
THE vanishing-toolbar root cause, measured against a live host rather than reasoned about. AppKit repositions a CHILD window to preserve its offset from its parent, and it does so AFTER the didMove notification the controller reacts to. So placement computed the correct visible-frame-clamped rect, applied it, and was then silently dragged back a runloop turn later. With forensics on: host=(221,-200,1291,889) computed=(1272,60,240,104) afterSet=(1272,60,240,104) next-turn panel=(1272,-200,240,104) clobbered=true On a host whose bottom hangs below the display -- a tall scrollable window, which is what 'on scrollable screens' meant -- that parks the pill under the Dock or off the display entirely. Verified end to end: moving the host down 260pt now leaves the pill at y=818 (on screen) instead of 1078 (off it). Also fixes the scroll hand-off. Forwarding the NSEvent object into the host's view tree engaged NSScrollView's responsive-scroll event tracking against event.window -- the PANEL -- and that cross-window tracking wedged the panel's event delivery and display: the overlay stopped rendering AND stopped hit-testing while its window sat there, which is why a plain mouse wheel never triggered it but a trackpad always did. The panel now drives the enclosing scroller's clip directly by the deltas; the event never crosses. Both mutation-verified. Panel forensics kept behind ANNOTKIT_PANEL_FORENSICS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 8d3091b commit 3267df7

4 files changed

Lines changed: 515 additions & 65 deletions

File tree

Sources/AnnotKit/macOS/OverlayController.swift

Lines changed: 107 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ public final class OverlayController: NSObject {
4545
/// or two later without posting `didMove`/`didResize`; comparing against this
4646
/// lets the post-attach settle poll tell when the frame has stabilized.
4747
private var lastSyncedHostFrame: NSRect = .null
48+
/// The clamped frame the panel is SUPPOSED to occupy, re-asserted after AppKit's
49+
/// parent-follow repositioning. `.null` until the first sync.
50+
private var desiredPanelFrame: NSRect = .null
4851
/// Bumped on every attach/unmount so an in-flight settle poll for a previous
4952
/// host stops instead of re-syncing against a stale (or detached) window.
5053
private var settleGeneration = 0
@@ -361,7 +364,22 @@ public final class OverlayController: NSObject {
361364
private func syncFrameAndOrigin() {
362365
guard let panel, let host else { return }
363366
let panelFrame = frame(for: session.mode, on: host)
367+
desiredPanelFrame = panelFrame
364368
panel.setFrame(panelFrame, display: true)
369+
// AppKit repositions a CHILD window to follow its parent, and it does so AFTER
370+
// the `didMove` notification we are reacting to — so the clamped frame we just
371+
// applied is silently dragged back to the host's own corner a runloop turn
372+
// later. On a host whose bottom hangs below the display that puts the pill
373+
// off-screen: the toolbar "disappears", exactly as reported, with placement
374+
// that computed the right answer and a panel that no longer sits there.
375+
//
376+
// Measured, not assumed — with forensics on:
377+
// computed=(1272, 60, 240, 104) afterSet=(1272, 60, 240, 104)
378+
// next-turn panel=(1272, -200, 240, 104) clobbered=true
379+
//
380+
// Re-assert once AppKit has finished. `enforcePanelFrame()` is a no-op when it
381+
// left us alone, so the common case costs one runloop hop and nothing else.
382+
DispatchQueue.main.async { [weak self] in self?.enforcePanelFrame() }
365383
// Primary display = the origin/menu-bar screen, NOT NSScreen.main (the
366384
// active screen), which was the single-display bug.
367385
let primaryHeight = NSScreen.screens.first?.frame.height ?? 0
@@ -396,6 +414,21 @@ public final class OverlayController: NSObject {
396414
hostingView?.rootView = makeRootView()
397415
}
398416

417+
/// Put the panel back where placement said it belongs, if AppKit moved it.
418+
///
419+
/// Child windows are repositioned by AppKit to preserve their offset from the
420+
/// parent, which undoes the visible-frame clamp on every host move. Comparing
421+
/// before setting keeps this free when nothing fought us, and keeps it from
422+
/// looping: a `setFrame` to the frame the window already has posts no move.
423+
private func enforcePanelFrame() {
424+
guard let panel, desiredPanelFrame != .null, panel.frame != desiredPanelFrame else { return }
425+
if KeyablePanel.forensics {
426+
FileHandle.standardError.write(Data(
427+
"[sync] re-asserting clamped frame: \(panel.frame) -> \(desiredPanelFrame)\n".utf8))
428+
}
429+
panel.setFrame(desiredPanelFrame, display: true)
430+
}
431+
399432
private func frame(for mode: AnnotationSession.Mode, on host: NSWindow) -> NSRect {
400433
OverlayPlacement.panelFrame(
401434
for: mode,
@@ -475,6 +508,40 @@ enum OverlayPlacement {
475508
final class KeyablePanel: NSPanel {
476509
override var canBecomeKey: Bool { true }
477510

511+
/// Forensics for "the pill vanished": every path AppKit can take to hide a
512+
/// window funnels through `orderWindow`/`setIsVisible`/`close`, so logging the
513+
/// call stack at each one names the culprit instead of leaving a symptom.
514+
/// Opt-in via `ANNOTKIT_PANEL_FORENSICS=1`; costs one env lookup otherwise.
515+
static let forensics = ProcessInfo.processInfo.environment["ANNOTKIT_PANEL_FORENSICS"] == "1"
516+
517+
private func forensic(_ what: String) {
518+
guard Self.forensics else { return }
519+
FileHandle.standardError.write(Data("""
520+
[panel-forensics] \(what) visible=\(isVisible) frame=\(frame) parent=\(parent.map { "\($0.title)" } ?? "nil")
521+
\(Thread.callStackSymbols.prefix(14).joined(separator: "\n"))\n\n
522+
""".utf8))
523+
}
524+
525+
override func order(_ place: NSWindow.OrderingMode, relativeTo otherWin: Int) {
526+
if place == .out { forensic("order(.out)") }
527+
super.order(place, relativeTo: otherWin)
528+
}
529+
530+
override func orderOut(_ sender: Any?) {
531+
forensic("orderOut(sender: \(sender.map { String(describing: type(of: $0)) } ?? "nil"))")
532+
super.orderOut(sender)
533+
}
534+
535+
override func setIsVisible(_ flag: Bool) {
536+
if !flag { forensic("setIsVisible(false)") }
537+
super.setIsVisible(flag)
538+
}
539+
540+
override func close() {
541+
forensic("close()")
542+
super.close()
543+
}
544+
478545
/// Hand an unconsumed scroll down to the host window.
479546
///
480547
/// Measured, not assumed: while annotating this panel covers the host's whole frame
@@ -495,19 +562,47 @@ final class KeyablePanel: NSPanel {
495562
// clamped to the visible region, so handing it over unconverted would scroll
496563
// whatever sits at the wrong point (the wrong scroller, in a window with two).
497564
let hostPoint = host.convertPoint(fromScreen: convertPoint(toScreen: event.locationInWindow))
498-
// Falls back to the content view when the point is over no host view at all
499-
// (the title-bar strip, or a host smaller than the panel): a wheel that hit
500-
// AnnotKit must never simply vanish. `NSView`'s default `scrollWheel` walks the
501-
// event UP to the enclosing scroller from wherever it lands, so aiming at the
502-
// deepest view under the pointer is enough — no scroll-view search here.
565+
// Hit-test from the window's ROOT view (the content view's superview, AppKit's
566+
// border/theme frame), not from the content view: `hitTest(_:)` takes a point
567+
// in the receiver's SUPERVIEW space, and the border view is flipped — so
568+
// handing window-base coordinates to `contentView.hitTest` mirrors the y and
569+
// misses everything, silently. The root view's "superview space" is defined
570+
// as window base, which is exactly what `convertPoint(fromScreen:)` yields.
571+
let root = host.contentView?.superview ?? host.contentView
572+
guard let target = root?.hitTest(hostPoint) ?? host.contentView else { return }
573+
574+
// Scroll the host's scroller DIRECTLY by the event's deltas. The event object
575+
// itself must NEVER cross into the host's view tree: an earlier version did
576+
// `targetView.scrollWheel(with: event)`, and for a TRACKPAD stream — phase
577+
// `began`/`changed`/`ended` plus momentum, which is every real-world scroll —
578+
// NSScrollView's responsive scrolling responds to `began` by engaging an
579+
// event-tracking loop against `event.window`. That window is THIS PANEL, not
580+
// the scroll view's own, and the cross-window tracking never terminates:
581+
// it wedged the panel's event delivery and display, so the overlay silently
582+
// stopped rendering AND stopped hit-testing while its window sat there —
583+
// observed as "the toolbar vanishes after I scroll, then hover on and off it",
584+
// with a plain mouse wheel (no phases) never triggering it. Reproduced
585+
// against a live host and pinned by the probe's scroll phase.
503586
//
504-
// The event itself is passed along unchanged, so the target reads a
505-
// `locationInWindow` that is still PANEL-local (an `NSEvent`'s location cannot
506-
// be rewritten). Scroll handling uses the DELTAS, and the location's one real
507-
// job — choosing the target — is done above, so this costs nothing short of a
508-
// host view that positions something off the wheel's own coordinates.
509-
510-
(host.contentView?.hitTest(hostPoint) ?? host.contentView)?.scrollWheel(with: event)
587+
// Driving the clip view by deltas keeps everything inside the HOST's own
588+
// machinery, no event identity involved. Momentum events still arrive here
589+
// carrying deltas, so inertia is preserved; only the edge rubber-band is
590+
// lost, because `constrainBoundsRect` clamps at the document bounds.
591+
var view: NSView? = target
592+
while let current = view, !(current is NSScrollView) { view = current.superview }
593+
guard let scrollView = view as? NSScrollView else { return }
594+
595+
let clip = scrollView.contentView
596+
// Non-precise deltas (an external mouse wheel) are in LINES; convert to
597+
// points the same way NSScrollView itself does.
598+
let scale: CGFloat = event.hasPreciseScrollingDeltas ? 1 : scrollView.verticalLineScroll
599+
var origin = clip.bounds.origin
600+
origin.x -= event.scrollingDeltaX * scale
601+
// A flipped clip (the AppKit default for scroll content) grows y downward, so
602+
// natural-scroll deltas subtract; an unflipped one is the mirror.
603+
origin.y += (clip.isFlipped ? -1 : 1) * event.scrollingDeltaY * scale
604+
clip.scroll(to: clip.constrainBoundsRect(NSRect(origin: origin, size: clip.bounds.size)).origin)
605+
scrollView.reflectScrolledClipView(clip)
511606
}
512607
}
513608
#endif

0 commit comments

Comments
 (0)