Skip to content
Merged
22 changes: 22 additions & 0 deletions Sources/SnapshotPreviewsCore/AppKitRenderingStrategy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,23 @@ final class AppKitContainer: NSHostingController<EmergeModifierView>, ScrollExpa
}
var heightAnchor: NSLayoutConstraint?
var previousHeight: CGFloat?
var pendingContentSizeRetries: Int = 0
var lastObservedContentHeight: CGFloat?
var pendingIntrinsicSizeRetries: Int = 0
var lastObservedIntrinsicSize: CGSize?

var hostFittingSize: CGSize? {
// NSView's `fittingSize` is the AppKit equivalent of UIKit's
// systemLayoutSizeFitting(compressedSize) and honors active layout
// constraints — same role in the settle loop.
view.fittingSize
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AppKit hostFittingSize override prevents intended immediate completion

High Severity

AppKitContainer overrides hostFittingSize to return view.fittingSize (non-nil, typically non-zero), but its setNeedsAnotherLayoutPass() is a no-op (protocol default). The comment on lines 130–134 states the settle loop "falls through to immediate completion," which only works if hostFittingSize returns nil (the protocol extension default). With a real value returned, the non-scroll settle loop defers on its first observation (since lastObservedIntrinsicSize is nil ≠ fittingSize) and calls the no-op, so updateScrollViewHeight() may never be re-invoked — hanging the rendered callback indefinitely. The same issue affects the scroll path's new pendingContentSizeRetries guard.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 79608ca. Configure here.


// No setNeedsAnotherLayoutPass override — AppKit isn't exercised by our
// CI (we run iOS simulator only), so we accept the protocol's no-op default
// and the AppKit settle loop falls through to immediate completion. Avoids
// having to maintain an AppKit-correct re-entry mechanism for code we
// don't actually run.

public var rendered: ((EmergeRenderingMode?, Float?, Bool?, Bool?) -> Void)? {
didSet { didCall = false }
Expand Down Expand Up @@ -143,6 +160,11 @@ final class AppKitContainer: NSHostingController<EmergeModifierView>, ScrollExpa
widthAnchor?.isActive = false
heightAnchor = nil
widthAnchor = nil
previousHeight = nil
pendingContentSizeRetries = 0
lastObservedContentHeight = nil
pendingIntrinsicSizeRetries = 0
lastObservedIntrinsicSize = nil
}

public func setupView(layout: PreviewLayout) {
Expand Down
52 changes: 52 additions & 0 deletions Sources/SnapshotPreviewsCore/ExpandingViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ public final class ExpandingViewController: UIHostingController<EmergeModifierVi

private var didCall = false
var previousHeight: CGFloat?
var pendingContentSizeRetries: Int = 0
var lastObservedContentHeight: CGFloat?
var pendingIntrinsicSizeRetries: Int = 0
var lastObservedIntrinsicSize: CGSize?

var heightAnchor: NSLayoutConstraint?
private var widthAnchor: NSLayoutConstraint?
Expand All @@ -43,6 +47,15 @@ public final class ExpandingViewController: UIHostingController<EmergeModifierVi
}
view.translatesAutoresizingMaskIntoConstraints = false
view.backgroundColor = .clear
// Pin safe-area-derived layout to a stable, zeroed value up front.
// UIHostingController otherwise resolves these insets across multiple
// layout passes (the host's safeAreaInsets propagate from the window /
// scene asynchronously), which translates the rendered content
// vertically run-to-run on layouts that read directionalLayoutMargins
// (e.g. screens with manual top/bottom padding via UILayoutGuide.
// safeAreaLayoutGuide). Zeroing them eliminates that source of drift.
view.insetsLayoutMarginsFromSafeArea = false
view.directionalLayoutMargins = .zero
}

@MainActor required dynamic init?(coder aDecoder: NSCoder) {
Expand All @@ -55,6 +68,23 @@ public final class ExpandingViewController: UIHostingController<EmergeModifierVi
heightAnchor = nil
widthAnchor = nil
previousHeight = nil
pendingContentSizeRetries = 0
lastObservedContentHeight = nil
pendingIntrinsicSizeRetries = 0
lastObservedIntrinsicSize = nil
didAppear = false
}

var hostFittingSize: CGSize? {
// systemLayoutSizeFitting honors the active height/width anchors, which
// makes it the canonical "what would this view want to render at" value
// we're racing to stabilize. UILayoutFittingCompressedSize asks for the
// smallest size satisfying constraints, matching the snapshot intent.
view.systemLayoutSizeFitting(UIView.layoutFittingCompressedSize)
}

func setNeedsAnotherLayoutPass() {
view.setNeedsLayout()
}

public func setupView(layout: PreviewLayout) {
Expand Down Expand Up @@ -82,8 +112,30 @@ public final class ExpandingViewController: UIHostingController<EmergeModifierVi
stopAndResetTimer()
}

// Tracks whether viewDidAppear has fired. Layout passes that fire BEFORE
// the view appears can read a transient view tree — e.g. a SwiftUI body
// that returns EmptyView while @State is still nil and only mutates to its
// final value inside viewDidAppear (`SelfRewardClaimViewController` was the
// canary case: width collapsed 1418 → 786 in some captures because the
// a11y legend depended on content the body wasn't yet rendering). Settling
// on those passes locks in the wrong dimensions. Gating the settle loop
// on this flag means we only commit after viewDidAppear has fired and any
// state changes it triggers have had a chance to propagate.
private var didAppear = false

public override func viewDidLayoutSubviews() {
super.viewDidLayoutSubviews()
if didAppear {
updateScrollViewHeight()
}
}

public override func viewDidAppear(_ animated: Bool) {
super.viewDidAppear(animated)
didAppear = true
// Kick off the first settle attempt now that any viewDidAppear-driven
// state mutations have had their chance to fire. The retry-on-instability
// loop takes over from here via setNeedsLayout → next viewDidLayoutSubviews.
updateScrollViewHeight()
}

Expand Down
72 changes: 72 additions & 0 deletions Sources/SnapshotPreviewsCore/ScrollExpansion.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,46 @@ protocol ScrollExpansionProviding: AnyObject, FirstScrollViewProviding {
var previousHeight: CGFloat? { get set }
var heightAnchor: NSLayoutConstraint? { get }
var supportsExpansion: Bool { get }
// Tracks how many times we've deferred completion waiting for the inner
// scroll view's contentSize to stabilize. iOS 18 reordered hosting
// controller layout passes so the inner UIKit layout (where contentSize
// is assigned) can lag — and may land at a partial value before the
// final one — without this counter we'd race into complete() at the
// wrong height.
var pendingContentSizeRetries: Int { get set }
// Last contentSize observation. Used to detect stabilization: we only
// commit when two consecutive passes report the same height.
var lastObservedContentHeight: CGFloat? { get set }
// Tracks how many times we've deferred completion waiting for the host's
// intrinsic-content-size / fitting size to stabilize on the non-scroll
// path. Same iOS 18 hosting-controller pass-reordering issue as the scroll
// counter, but for views that don't have an inner UIScrollView (e.g. a
// hosting controller wrapping a fixed-layout SwiftUI view).
var pendingIntrinsicSizeRetries: Int { get set }
// Last intrinsic-size observation. Used to detect stabilization: we only
// commit when two consecutive passes report the same fitting size.
var lastObservedIntrinsicSize: CGSize? { get set }
// The host's currently-resolved fitting size. Implementations return
// `view.systemLayoutSizeFitting(...)` (UIKit) or
// `view.fittingSize` (AppKit). Returning nil signals "no useful intrinsic
// size to wait on" — settle loop falls through to immediate completion.
var hostFittingSize: CGSize? { get }
// Asks the host to schedule another layout pass. UIKit hosts implement
// this as view.setNeedsLayout(); AppKit / unsupported platforms can no-op.
func setNeedsAnotherLayoutPass()
}

extension ScrollExpansionProviding {
func setNeedsAnotherLayoutPass() {}
Comment thread
cursor[bot] marked this conversation as resolved.

var hostFittingSize: CGSize? { nil }

func updateHeight(_ complete: (() -> Void)) {
// Cap on contentSize-not-stabilized retries before we give up and complete
// anyway. Prevents an infinite wait on a genuinely empty / animating view.
let maxPendingContentSizeRetries = 10
// Symmetric cap for the intrinsic-size settle loop.
let maxPendingIntrinsicSizeRetries = 10
// If heightAnchor isn't set, this was a fixed size and we don't expand the scroll view
guard let heightAnchor else {
complete()
Expand All @@ -42,6 +78,21 @@ extension ScrollExpansionProviding {
let supportsExpansion = supportsExpansion
let scrollView = firstScrollView
if let scrollView, supportsExpansion {
// Stabilization: scroll content can land at a partial height before its
// final size when SwiftUI hosting / UIKit child layout completes in
// multiple passes. We retry until we see the same non-zero contentHeight
// on two consecutive passes — a stable observation — before committing.
// Without this, we capture at intermediate layout states and produce
// run-to-run dimension drift on iOS 18 / Apple Silicon.
let currentContentHeight = scrollView.contentHeight
guard previousHeight != nil
|| (currentContentHeight > 0 && lastObservedContentHeight == currentContentHeight)
|| pendingContentSizeRetries >= maxPendingContentSizeRetries else {
lastObservedContentHeight = currentContentHeight
pendingContentSizeRetries += 1
setNeedsAnotherLayoutPass()
return
}
Comment thread
cursor[bot] marked this conversation as resolved.
let diff = Int(scrollView.contentHeight - scrollView.visibleContentHeight)
if abs(diff) > 0 {
if previousHeight != nil || diff > 0 {
Expand All @@ -62,6 +113,27 @@ extension ScrollExpansionProviding {
complete()
}
} else {
// Non-scroll path. UIHostingController's two-pass layout can also drop
// intrinsic content size into the host view *after* the first
// viewDidLayoutSubviews — same race as the scroll-view path, but
// observed via systemLayoutSizeFitting / fittingSize instead of
// contentSize. Wait until two consecutive passes report the same
// fitting size before committing.
guard let fittingSize = hostFittingSize,
fittingSize.width > 0 || fittingSize.height > 0 else {
// Either the host doesn't expose a fitting size, or it returned
// (noIntrinsicMetric, noIntrinsicMetric)-equivalent zeros — there's
// nothing meaningful to wait on, complete immediately.
complete()
return
}
guard lastObservedIntrinsicSize == fittingSize
|| pendingIntrinsicSizeRetries >= maxPendingIntrinsicSizeRetries else {
lastObservedIntrinsicSize = fittingSize
pendingIntrinsicSizeRetries += 1
setNeedsAnotherLayoutPass()
return
}
complete()
}
}
Expand Down