diff --git a/clients/AGENTS.md b/clients/AGENTS.md index c493b0c63a4..0b34d2d9316 100644 --- a/clients/AGENTS.md +++ b/clients/AGENTS.md @@ -195,7 +195,9 @@ Prefer migrating the parent to `@Observable` so the bridge becomes unnecessary ( - **Lazy containers for large collections.** Use `LazyVStack`, `LazyHStack`, `LazyVGrid` instead of eager equivalents when the item count is unbounded or large. In particular, avoid `VStack`/`HStack` inside a `ScrollView` for large or unbounded data-driven lists — eager loading kills scroll performance. Small, fixed-size lists where visibility-sensitive logic (e.g., `onAppear` pagination triggers) matters may use eager containers intentionally. - **Keep modifier chains lean.** Every SwiftUI modifier wraps its content in a new view value. Long chains of redundant or duplicated modifiers deepen the view tree and increase diffing cost. Group related modifiers, remove redundant ones (e.g., don't set `.font` on every row when the parent already sets it), and extract heavily-modified subtrees into standalone `@ViewBuilder` methods. - **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. -- **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`, `HighlightedTextView.CodeTextView`, `VCodeView.VCodeTextView`. +- **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). +- **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:)). +- **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 @@ -448,7 +450,8 @@ Swift's type checker has quadratic complexity with chained view modifiers. Compl | `Timer.publish` / `Timer.scheduledTimer` for UI updates | Timer continues firing when the view is off-screen, wastes energy, and requires manual lifecycle management (invalidate, cancellable storage) | Use `TimelineView(.periodic(from: .now, by: interval))` for fixed-rate progress displays or `TimelineView(.animation)` for frame-rate animations. TimelineView auto-pauses when the view is off-screen and requires no manual teardown. | | `DispatchQueue.main.sync` from `@MainActor` or main thread | Deadlocks. Ref: [Apple — DispatchQueue.sync](https://developer.apple.com/documentation/dispatch/dispatchqueue/sync(execute:)-3gef0) | Use `Thread.isMainThread` guard, or `await MainActor.run {}` from async contexts. Prefer thread-safe APIs that don't need the main thread. | | CPU-bound work inside `@MainActor` type without offloading | Blocks UI (JSON decode, image resize, compression). Ref: [WWDC25 — Embracing Swift concurrency](https://developer.apple.com/videos/play/wwdc2025/268/) | Offload the expensive call via `Task.detached`. Keep the type on `@MainActor`. See § "@MainActor Isolation Boundaries". | -| Unguarded `NSLayoutManager.ensureLayout(for:)` in `NSViewRepresentable.sizeThatFits` inside `LazyVStack`/`List` cells | SwiftUI calls `sizeThatFits` on every visible cell on every layout pass; `ensureLayout` is O(glyphs) on the main thread. Caches keyed only on text content (not width) silently skip on same-text-new-width queries, running layout again. | Cache last measurement on `(textStorage.length, proposalWidth)` at minimum; use `NSAttributedString.isEqual(_:)` when attributes matter. Invalidate at every text-storage mutation site. See § "View Bodies and Rendering". | +| Unguarded `NSLayoutManager.ensureLayout(for:)` in `NSViewRepresentable.sizeThatFits` inside `LazyVStack`/`List` cells | SwiftUI calls `sizeThatFits` on every visible cell on every layout pass; `ensureLayout` is O(glyphs) on the main thread. Caches keyed only on text content (not width) silently skip on same-text-new-width queries, running layout again. | Cache last measurement on `(textStorage.length, proposalWidth)` at minimum; use `NSAttributedString.isEqual(_:)` when attributes matter. Invalidate at every text-storage mutation site. When line wrapping is off and line height is pinned, use `lineCount * lineHeight + insets` directly. See § "View Bodies and Rendering". | +| Default `NSLayoutManager.allowsNonContiguousLayout` (`false`) on TextKit 1 stacks in `NSViewRepresentable` | The first time the `NSTextView` is attached to its hosting view, AppKit's `setNeedsDisplayInRect:` asks the layout manager for a glyph range, which forces full-document layout from character 0 on the main thread. Multi-second hangs on large documents. | Set `layoutManager.allowsNonContiguousLayout = true` when constructing the TextKit 1 stack. See § "View Bodies and Rendering". |
diff --git a/clients/macos/vellum-assistant/Features/Chat/ComposerTextEditor.swift b/clients/macos/vellum-assistant/Features/Chat/ComposerTextEditor.swift index fcfdf10251c..6f568b1f7e0 100644 --- a/clients/macos/vellum-assistant/Features/Chat/ComposerTextEditor.swift +++ b/clients/macos/vellum-assistant/Features/Chat/ComposerTextEditor.swift @@ -102,6 +102,11 @@ struct ComposerTextEditor: NSViewRepresentable { // Reference: https://developer.apple.com/documentation/appkit/nstextview/1449309-layoutmanager let textStorage = NSTextStorage() let layoutManager = NSLayoutManager() + // Confine glyph generation to the requested bounding rect so that + // attaching the text view to its scroll view does not force full + // document layout on the main thread. + // https://developer.apple.com/documentation/appkit/nslayoutmanager/allowsnoncontiguouslayout + layoutManager.allowsNonContiguousLayout = true textStorage.addLayoutManager(layoutManager) let textContainer = NSTextContainer(size: NSSize( width: 0, diff --git a/clients/macos/vellum-assistant/Features/MainWindow/Panels/HighlightedTextView.swift b/clients/macos/vellum-assistant/Features/MainWindow/Panels/HighlightedTextView.swift index 3c0b93252ee..d28adddc8d5 100644 --- a/clients/macos/vellum-assistant/Features/MainWindow/Panels/HighlightedTextView.swift +++ b/clients/macos/vellum-assistant/Features/MainWindow/Panels/HighlightedTextView.swift @@ -222,6 +222,11 @@ private struct CodeTextView: NSViewRepresentable { // TextKit 1 stack — matches the gutter's NSLayoutManager.defaultLineHeight computation let textStorage = NSTextStorage() let layoutManager = NSLayoutManager() + // Confine glyph generation to the requested bounding rect so that + // attaching the text view to its scroll view does not force full + // document layout on the main thread. + // https://developer.apple.com/documentation/appkit/nslayoutmanager/allowsnoncontiguouslayout + layoutManager.allowsNonContiguousLayout = true textStorage.addLayoutManager(layoutManager) let textContainer = NSTextContainer(size: NSSize( width: CGFloat.greatestFiniteMagnitude, @@ -271,6 +276,7 @@ private struct CodeTextView: NSViewRepresentable { textView.onEscape = onEscape textView.onCommandF = onCommandF textView.string = text + context.coordinator.lineCount = VCodeView.countLines(in: text) let scrollView = VCodeHorizontalScrollView() scrollView.documentView = textView @@ -287,7 +293,7 @@ private struct CodeTextView: NSViewRepresentable { context.coordinator.parent = self guard let textView = scrollView.documentView as? CodeNSTextView else { return } if textView.string != text { - context.coordinator.invalidateMeasurementCache() + context.coordinator.lineCount = VCodeView.countLines(in: text) let selectedRanges = textView.selectedRanges textView.string = text let length = (text as NSString).length @@ -310,33 +316,20 @@ private struct CodeTextView: NSViewRepresentable { nsView: VCodeHorizontalScrollView, context: Context ) -> CGSize? { - guard let textView = nsView.documentView as? CodeNSTextView, - let layoutManager = textView.layoutManager, - let textContainer = textView.textContainer, - let textStorage = textView.textStorage else { return nil } + guard let textView = nsView.documentView as? CodeNSTextView else { return nil } let width = proposal.width ?? 400 - // SwiftUI calls `sizeThatFits` for every cell on every `LazyVStack` - // layout pass. Returning the cached size when - // `(textStorage.length, width)` matches the last measurement avoids - // rerunning `ensureLayout`, which is O(n) in glyph count. - // `updateNSView` invalidates on text replacement and `textDidChange` - // invalidates on user edits. - let coordinator = context.coordinator - let length = textStorage.length - if length == coordinator.lastMeasuredLength, - width == coordinator.lastMeasuredWidth { - return CGSize(width: width, height: coordinator.lastMeasuredHeight) - } - - layoutManager.ensureLayout(for: textContainer) - let usedRect = layoutManager.usedRect(for: textContainer) - let height = usedRect.height + textView.textContainerInset.height * 2 - - coordinator.lastMeasuredLength = length - coordinator.lastMeasuredWidth = width - coordinator.lastMeasuredHeight = height + // Height is derived directly from line count and the pinned + // per-line height, bypassing `NSLayoutManager.ensureLayout(for:)`. + // The text container is unbounded horizontally, so lines never + // wrap; `paragraphStyle.minimumLineHeight == maximumLineHeight` + // clamps every line to the same `defaultLineHeight`. That makes + // the geometry exactly `lineCount * lineHeight + insets`, and + // removes an O(glyph count) main-thread layout pass that SwiftUI + // would otherwise re-run on every cell during `LazyVStack` layout. + let height = CGFloat(context.coordinator.lineCount) * VCodeView.lineHeight + + textView.textContainerInset.height * 2 return CGSize(width: width, height: height) } @@ -347,25 +340,17 @@ private struct CodeTextView: NSViewRepresentable { class Coordinator: NSObject, NSTextViewDelegate { var parent: CodeTextView - // Last successful `sizeThatFits` measurement. Invalidated whenever - // the text storage is replaced (`updateNSView`, `textDidChange`). - var lastMeasuredLength: Int = -1 - var lastMeasuredWidth: CGFloat = -1 - var lastMeasuredHeight: CGFloat = 0 + // Newline count for the current text. Drives `sizeThatFits` + // geometry directly, avoiding `NSLayoutManager.ensureLayout(for:)`. + var lineCount: Int = 1 init(parent: CodeTextView) { self.parent = parent } - func invalidateMeasurementCache() { - lastMeasuredLength = -1 - lastMeasuredWidth = -1 - lastMeasuredHeight = 0 - } - func textDidChange(_ notification: Notification) { guard let textView = notification.object as? NSTextView else { return } - invalidateMeasurementCache() + lineCount = VCodeView.countLines(in: textView.string) parent.text = textView.string parent.onTextChange?(textView.string) } diff --git a/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift b/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift index f27a742260f..be445ffe48f 100644 --- a/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift +++ b/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift @@ -177,6 +177,11 @@ public struct VSelectableTextView: NSViewRepresentable { // Reference: https://developer.apple.com/documentation/appkit/nstextview/1449309-layoutmanager let textStorage = NSTextStorage() let layoutManager = NSLayoutManager() + // Confine glyph generation to the requested bounding rect so that + // attaching the text view to its hosting view does not force full + // document layout on the main thread. + // https://developer.apple.com/documentation/appkit/nslayoutmanager/allowsnoncontiguouslayout + layoutManager.allowsNonContiguousLayout = true textStorage.addLayoutManager(layoutManager) let textContainer = NSTextContainer(size: NSSize( width: 0, diff --git a/clients/shared/DesignSystem/Components/Display/VCodeView.swift b/clients/shared/DesignSystem/Components/Display/VCodeView.swift index 1a9a2c8b593..47599507e1d 100644 --- a/clients/shared/DesignSystem/Components/Display/VCodeView.swift +++ b/clients/shared/DesignSystem/Components/Display/VCodeView.swift @@ -239,6 +239,14 @@ struct VCodeTextView: NSViewRepresentable { func makeNSView(context: Context) -> VCodeHorizontalScrollView { let textStorage = NSTextStorage() let layoutManager = NSLayoutManager() + // Non-contiguous layout confines glyph generation to the requested + // bounding rect. Without this, adding the NSTextView to its scroll + // view triggers full-document glyph layout from character 0 on the + // main thread (via `_setSuperview:` → `setNeedsDisplayInRect:` → + // `_glyphRangeForBoundingRect:`), producing multi-second hangs for + // large code blocks. + // https://developer.apple.com/documentation/appkit/nslayoutmanager/allowsnoncontiguouslayout + layoutManager.allowsNonContiguousLayout = true textStorage.addLayoutManager(layoutManager) let textContainer = NSTextContainer(size: NSSize( width: CGFloat.greatestFiniteMagnitude, @@ -326,33 +334,21 @@ struct VCodeTextView: NSViewRepresentable { nsView: VCodeHorizontalScrollView, context: Context ) -> CGSize? { - guard let textView = nsView.documentView as? ClickReportingTextView, - let layoutManager = textView.layoutManager, - let textContainer = textView.textContainer, - let textStorage = textView.textStorage else { return nil } + guard let textView = nsView.documentView as? ClickReportingTextView else { return nil } let width = proposal.width ?? 400 - // SwiftUI calls `sizeThatFits` for every cell on every `LazyVStack` - // layout pass. Returning the cached size when - // `(textStorage.length, width)` matches the last measurement avoids - // rerunning `ensureLayout`, which is O(n) in glyph count. `applyText` - // invalidates on both the plain-text apply and async syntax-highlight - // completion (font variants can change line height). - let coordinator = context.coordinator - let length = textStorage.length - if length == coordinator.lastMeasuredLength, - width == coordinator.lastMeasuredWidth { - return CGSize(width: width, height: coordinator.lastMeasuredHeight) - } - - layoutManager.ensureLayout(for: textContainer) - let usedRect = layoutManager.usedRect(for: textContainer) - let height = usedRect.height + textView.textContainerInset.height * 2 - - coordinator.lastMeasuredLength = length - coordinator.lastMeasuredWidth = width - coordinator.lastMeasuredHeight = height + // Height is derived directly from line count and the pinned + // per-line height, bypassing `NSLayoutManager.ensureLayout(for:)`. + // The text container is unbounded horizontally, so lines never + // wrap; `paragraphStyle.minimumLineHeight == maximumLineHeight` + // clamps every line (including bold/italic syntax-highlight runs) + // to the same `defaultLineHeight`. That makes the geometry exactly + // `lineCount * lineHeight + insets`, and removes an O(glyph count) + // main-thread layout pass that SwiftUI would otherwise re-run on + // every cell during `LazyVStack` layout. + let height = CGFloat(context.coordinator.lineCount) * VCodeView.lineHeight + + textView.textContainerInset.height * 2 return CGSize(width: width, height: height) } @@ -366,21 +362,12 @@ struct VCodeTextView: NSViewRepresentable { var lastMatchCount: Int = 0 var paragraphStyle: NSParagraphStyle? - // Last successful `sizeThatFits` measurement. Invalidated wherever - // the text storage is replaced: `applyText` and the async - // syntax-highlight completion. - var lastMeasuredLength: Int = -1 - var lastMeasuredWidth: CGFloat = -1 - var lastMeasuredHeight: CGFloat = 0 + // Newline count for the current text. Drives `sizeThatFits` + // geometry directly, avoiding `NSLayoutManager.ensureLayout(for:)`. + var lineCount: Int = 1 private var highlightTask: Task? - func invalidateMeasurementCache() { - lastMeasuredLength = -1 - lastMeasuredWidth = -1 - lastMeasuredHeight = 0 - } - deinit { highlightTask?.cancel() } @@ -397,7 +384,7 @@ struct VCodeTextView: NSViewRepresentable { to textView: NSTextView ) { lastText = text - invalidateMeasurementCache() + lineCount = VCodeView.countLines(in: text) // Apply plain text immediately so the view is never empty guard let textStorage = textView.textStorage else { return } @@ -428,10 +415,6 @@ struct VCodeTextView: NSViewRepresentable { storage.setAttributedString(highlighted) storage.endEditing() - // Invalidate measurement cache — font variants from - // highlighting (bold/italic) may change line heights. - self?.invalidateMeasurementCache() - // Re-apply search highlights that were wiped by setAttributedString if let self, !self.currentMatchRanges.isEmpty { self.applySearchHighlights(