Skip to content

Keep the browse scroll view out of fullscreen transitions - #3

Open
Puellaquae wants to merge 1 commit into
Force1ess:mainfrom
Puellaquae:fix/fullscreen-display-cycle-livelock
Open

Puellaquae wants to merge 1 commit into
Force1ess:mainfrom
Puellaquae:fix/fullscreen-display-cycle-livelock

Conversation

@Puellaquae

Copy link
Copy Markdown

Summary

  • Startup fullscreen aborts the process with EXC_BREAKPOINT; the uncaught exception is NSGenericException: The window has been marked as needing another Display Window pass, but it has already had more Display Window passes than there are views in the window.
  • That is an AppKit display-cycle livelock, not a one-off re-entrancy. During the fullscreen animation every display pass re-runs HostingScrollView.updateContext, which resizes the document view; NSClipView._reflectDocumentViewFrameChange re-clamps the scroll origin, which marks vibrancy needing an update, which asks for another pass. The next pass hit-tests (-[NSTrackingAreaAKManager setCursorForMouseLocation:]), and SwiftUI's NSHostingView.hitTest forces a synchronous AttributeGraph update — back to the resize. AppKit gives up once passes exceed the view count (observed: 3246).
  • This PR keeps the browse ScrollView out of the view tree for the duration of a fullscreen transition, so the loop has no receiver. NSHostingView.sizingOptions = [] in 启动崩溃 #2 removes one input to the loop; this removes the step that re-enters it, so the two are complementary rather than alternative.

Changes

  • AppModel.windowIsTransitioning (published) plus a pre-set at the app's own toggleFullScreen call sites.
  • ApplicationLifecycle.observeFullScreenTransition: willEnter/willExit set the gate synchronously (queue: nil + MainActor.assumeIsolated); didEnter/didExit only arm the release, which happens after two NSWindow.didUpdateNotification passes so the content is not re-inserted inside the animation's own display cycles.
  • RootView renders the browse ScrollView only when the gate is closed.

Evidence

Configuration Result
main before the gate, launch with fullscreen crash in 285-865 ms, 3/3 DMG build 106 and 3/4 local builds
--windowed (no transition) no crash
transition entered from a settled app no crash
gate with asynchronous flag delivery still 5/6 crashed — the flag landed one runloop turn late
gate with synchronous delivery + delayed release no crash, verified by hand with the pointer inside the window

Test plan

  • Launch normally (no --windowed) with the pointer resting inside the window and moving during the animation; repeat ~5 cold starts and confirm no MTV-*.ips in ~/Library/Logs/DiagnosticReports
  • Exit fullscreen with Esc / Ctrl-Cmd-F and re-enter from the settings row; content must return at the new size
  • Confirm the browse list is scrolled to the focused row after the transition (the gate tears the scroll view down and rebuilds it)
  • A/B against main alone: if sizingOptions = [] already prevents the loop in your environment, say so and the gate can be dropped

Known gap

Thirteen other ScrollView sites (SMB, metadata, tracks, About, panels) are not gated. They need the same three conditions — live resize, pending invalidations, and a hit test — so opening a panel and then toggling fullscreen is still exposed. Extending the gate to a shared view modifier is the obvious follow-up.

AppKit aborts a window's display cycle when a SwiftUI scroll view keeps resizing
its document view during a fullscreen animation: every pass marks vibrancy as
needing an update, which asks for another pass, and the cycle ends in
NSGenericException once there have been more passes than the window has views.
The launch transition hits it whenever the pointer is inside the window and the
library is still filling in, so the scroll view is absent until the transition's
own display cycles are over.

Notification delivery is synchronous, and the gate lifts only after two window
updates, because both edges have to land outside the animation's display passes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants