diff --git a/clients/AGENTS.md b/clients/AGENTS.md index 5e75824fc42..14b8d46e206 100644 --- a/clients/AGENTS.md +++ b/clients/AGENTS.md @@ -197,6 +197,7 @@ Prefer migrating the parent to `@Observable` so the bridge becomes unnecessary ( - **Scope observation narrowly.** Only observe the specific properties a view needs. Prefer granular `@Observable` properties or `withObservationTracking` over observing an entire store that publishes on unrelated changes. - **Enable non-contiguous layout on TextKit 1 stacks in `NSViewRepresentable`.** When constructing an `NSLayoutManager` for an `NSTextView` hosted via `NSViewRepresentable`, set `layoutManager.allowsNonContiguousLayout = true`. The default (`false`) forces full-document glyph layout from character 0 on the main thread whenever a glyph range is queried — which happens the first time the text view is attached to its hosting view (`setDocumentView:` / `addSubview:` → `_setSuperview:` → `setNeedsDisplayInRect:` → `_glyphRangeForBoundingRect:`). Reference: [`NSLayoutManager.allowsNonContiguousLayout`](https://developer.apple.com/documentation/appkit/nslayoutmanager/allowsnoncontiguouslayout). **Exception — streaming text with a separate measurement stack:** when the representable measures height via a sibling TextKit stack that always fully lays out every glyph (e.g. `ensureLayout(for:)` + `usedRect(for:)`), the measured frame assumes the render stack will also lay out every glyph in that rect. Non-contiguous layout leaves glyphs pending until the view scrolls or draws them, which races with streaming updates — producing a gap (render paints a smaller laid-out region inside the correct frame) and then overlap (the next sibling is placed via the measured frame and the lazy glyphs later paint outside it). In that case set `allowsNonContiguousLayout = false` and document why. See `clients/shared/DesignSystem/Components/Display/SelectableTextView.swift` (lines 180–205) for the canonical example; `VCodeView` / `HighlightedTextView` still opt into non-contiguous layout independently because they do not share this measurement model. - **Prefer line-count math over `ensureLayout(for:)` for `sizeThatFits` when line wrapping is disabled and line height is pinned.** If the TextKit 1 stack uses an unbounded-width `NSTextContainer` (so lines never wrap) and the paragraph style pins `minimumLineHeight == maximumLineHeight`, the total height is exactly `lineCount * lineHeight + insets` — no glyph layout required. This avoids an O(glyph count) main-thread layout pass that SwiftUI would otherwise re-run on every cell during `LazyVStack` layout. Reference: [`NSLayoutManager.defaultLineHeight(for:)`](https://developer.apple.com/documentation/appkit/nslayoutmanager/defaultlineheight(for:)). +- **Decouple `NSTextContainer` from the view frame when an `NSViewRepresentable` hands SwiftUI a precomputed size.** If the caller applies `.frame(width:height:)` from a static measurement helper (`measureSize`), set [`NSTextContainer.widthTracksTextView`](https://developer.apple.com/documentation/uikit/nstextcontainer/widthtrackstextview) to `false`, size the container explicitly from the same `maxWidth` used to measure, and propagate `maxWidth` changes onto `containerSize` in `updateNSView`. Do not pair this path with `autoresizingMask = [.width]`. The default (`widthTracksTextView = true`, autoresize on) causes every SwiftUI-driven `setFrameSize` — window resize, eager `VStack` measurement — to forward onto the layout manager, triggering `_fillLayoutHoleForCharacterRange`: a synchronous main-thread relayout that is O(glyph count). Reference: [`NSTextView.isHorizontallyResizable`](https://developer.apple.com/documentation/appkit/nstextview/ishorizontallyresizable). - **Memoize `sizeThatFits` (and sibling measurement calls) in `NSViewRepresentable` views placed in lazy containers.** Any `NSViewRepresentable` that hosts a TextKit stack (`NSLayoutManager` + `NSTextContainer`) and can appear inside a `LazyVStack` / `LazyHStack` / `List` cell must cache its last measurement. [`NSLayoutManager.ensureLayout(for:)`](https://developer.apple.com/documentation/uikit/nslayoutmanager/ensurelayout(for:)) is O(glyph count) and runs synchronously on the main thread, and SwiftUI calls [`sizeThatFits(_:nsView:context:)`](https://developer.apple.com/documentation/swiftui/nsviewrepresentable/sizethatfits(_:nsview:context:)) on every visible cell on every layout pass (scroll, resize, streaming edit). Cache at minimum on `(textStorage.length, proposalWidth)`; for attribute-sensitive paths (static measurement helpers, highlighted content) key on the `NSAttributedString` itself and fall through to [`NSAttributedString.isEqual(_:)`](https://developer.apple.com/documentation/foundation/nsattributedstring/isequal(_:)) for identity — `NSAttributedString.hash` alone admits collisions and surfaces as wrong-height cells. Invalidate at every text-storage mutation site (text apply, edit callbacks, async highlight/formatting completions). Reference: `SelectableTextView`. When line wrapping is disabled and line height is pinned (as in `VCodeView.VCodeTextView` and `HighlightedTextView.CodeTextView`), prefer the line-count math above over a cached `ensureLayout` call.
Per-entity observation and dictionary patterns diff --git a/clients/macos/vellum-assistant/Features/Chat/MarkdownSegmentView.swift b/clients/macos/vellum-assistant/Features/Chat/MarkdownSegmentView.swift index 0ecd9c74351..21588685628 100644 --- a/clients/macos/vellum-assistant/Features/Chat/MarkdownSegmentView.swift +++ b/clients/macos/vellum-assistant/Features/Chat/MarkdownSegmentView.swift @@ -194,7 +194,7 @@ struct MarkdownSegmentView: View, Equatable { VSelectableTextView( attributedString: measurement.nsAttributedString, - maxWidth: markdownView.maxContentWidth, + maxWidth: measurement.effectiveMaxWidth, lineSpacing: 4, tintColor: NSColor(markdownView.tintColor), useExternalSizing: true @@ -410,6 +410,11 @@ struct MarkdownSegmentView: View, Equatable { let nsAttributedString: NSAttributedString let size: CGSize let hasUnresolvedEmphasis: Bool + /// The wrap width used to produce `size`. Callers rendering with + /// `VSelectableTextView(useExternalSizing: true)` must pass this as + /// `maxWidth` so the live text container wraps at the same width + /// that measurement assumed. + let effectiveMaxWidth: CGFloat } /// Computes or retrieves from cache the `(NSAttributedString, CGSize)` pair @@ -455,7 +460,8 @@ struct MarkdownSegmentView: View, Equatable { return SelectableRunMeasurementResult( nsAttributedString: cached.nsAttributedString, size: cached.size, - hasUnresolvedEmphasis: false + hasUnresolvedEmphasis: false, + effectiveMaxWidth: effectiveMaxWidth ) } @@ -500,7 +506,8 @@ struct MarkdownSegmentView: View, Equatable { return SelectableRunMeasurementResult( nsAttributedString: nsAttributed, size: size, - hasUnresolvedEmphasis: hasUnresolvedEmphasis + hasUnresolvedEmphasis: hasUnresolvedEmphasis, + effectiveMaxWidth: effectiveMaxWidth ) } diff --git a/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift b/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift index c3392911410..6c3cfda6048 100644 --- a/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift +++ b/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift @@ -32,6 +32,12 @@ private final class SelectableNSTextView: NSTextView { /// view during the layout pass. This avoids an O(N) layout measurement /// cascade through nested `StackLayout.sizeThatFits` calls. /// +/// **Contract for `useExternalSizing: true`:** `maxWidth` MUST be non-nil +/// and equal to the value passed to `measureSize`. The live text container +/// is sized from `maxWidth`; if it diverges from the measurement width the +/// rendered wrap geometry will not match the precomputed frame, producing +/// horizontal clipping or stale heights. +/// /// For low-instance-count scenarios (e.g., a single thinking block), /// leave `useExternalSizing` at its default (`false`) and let /// `sizeThatFits` compute the size normally. @@ -204,11 +210,30 @@ public struct VSelectableTextView: NSViewRepresentable { // VCodeView / HighlightedTextView still opt in to non-contiguous // layout independently for their own scroll-attachment perf fix. textStorage.addLayoutManager(layoutManager) + + // Two container sizing modes: + // + // - `useExternalSizing: true` — the caller precomputes size via + // `measureSize` and applies `.frame(width:height:)`. The container + // is decoupled from the view frame so that `setFrameSize` cannot + // forward a width change onto the layout manager and trigger + // `_fillLayoutHoleForCharacterRange`, an O(glyph-count) main-thread + // relayout. Container width is sized explicitly from `maxWidth` + // here, and propagated in `updateNSView` when `maxWidth` changes. + // + // - `useExternalSizing: false` — `sizeThatFits` drives layout and + // the container tracks the view frame, matching NSTextView's + // default. Used by the design system gallery only. + // + // Reference: NSTextContainer.widthTracksTextView + // https://developer.apple.com/documentation/uikit/nstextcontainer/widthtrackstextview + let initialContainerWidth = useExternalSizing ? (maxWidth ?? CGFloat.greatestFiniteMagnitude) : 0 let textContainer = NSTextContainer(size: NSSize( - width: 0, + width: initialContainerWidth, height: CGFloat.greatestFiniteMagnitude )) - textContainer.widthTracksTextView = true + textContainer.widthTracksTextView = !useExternalSizing + textContainer.heightTracksTextView = false textContainer.lineFragmentPadding = 0 layoutManager.addTextContainer(textContainer) @@ -220,8 +245,22 @@ public struct VSelectableTextView: NSViewRepresentable { textView.backgroundColor = .clear textView.drawsBackground = false textView.isVerticallyResizable = true - textView.isHorizontallyResizable = false - textView.autoresizingMask = [.width] + // Allow the text view to grow independently of its frame when + // `useExternalSizing` is true. This reinforces the container + // decoupling above: if `setFrameSize` is invoked with a width + // smaller than the current content, AppKit will not shrink the + // container and trigger a re-layout of all glyphs. + textView.isHorizontallyResizable = useExternalSizing + if useExternalSizing { + textView.maxSize = NSSize( + width: CGFloat.greatestFiniteMagnitude, + height: CGFloat.greatestFiniteMagnitude + ) + } else { + // Match NSTextView's default autoresizing so the container + // width tracks the view frame in the Gallery preview path. + textView.autoresizingMask = [.width] + } textView.textContainerInset = .zero textView.delegate = context.coordinator @@ -232,12 +271,29 @@ public struct VSelectableTextView: NSViewRepresentable { .cursor: NSCursor.pointingHand, ] + context.coordinator.lastContainerWidth = useExternalSizing ? maxWidth : nil context.coordinator.applyAttributedString(attributedString, lineSpacing: lineSpacing, to: textView) return textView } public func updateNSView(_ textView: NSTextView, context: Context) { let coordinator = context.coordinator + + // Propagate container width changes before applying any text update. + // On the external-sizing path, the container is decoupled from the + // view frame, so a new `maxWidth` (e.g. from a window resize) must + // be written onto the container explicitly. Skip when unchanged so + // we do not perturb the current layout. + if useExternalSizing, + let textContainer = textView.textContainer, + coordinator.lastContainerWidth != maxWidth { + textContainer.containerSize = NSSize( + width: maxWidth ?? CGFloat.greatestFiniteMagnitude, + height: CGFloat.greatestFiniteMagnitude + ) + coordinator.lastContainerWidth = maxWidth + } + guard coordinator.lastAttributedString != attributedString || coordinator.lastLineSpacing != lineSpacing else { return } if useExternalSizing { @@ -326,6 +382,12 @@ public struct VSelectableTextView: NSViewRepresentable { var lastMeasuredWidth: CGFloat = -1 var lastMeasuredSize: CGSize = .zero + // Width currently written onto the text container on the external- + // sizing path. `updateNSView` propagates changes when this drifts + // from the incoming `maxWidth`, so the container stays decoupled + // from the view frame without re-sizing on every SwiftUI update. + var lastContainerWidth: CGFloat? + func reset() { lastAttributedString = nil lastLineSpacing = 0 @@ -333,6 +395,7 @@ public struct VSelectableTextView: NSViewRepresentable { pendingLineSpacing = nil pendingTextView = nil hasScheduledApply = false + lastContainerWidth = nil invalidateMeasurementCache() }