PR #2 by @ArjenSchwarz · merging fix/issue-26-selection-layout-loop → main · view on GitHub
tasks.md).ViewModifiers created a child view inside a GeometryReader that read @Environment(TextSelectionModel.self). That Observation dependency, registered inside the layout subgraph, re-armed on every measurement pass → endless re-layout.GeometryReader) and pass it into the child view as a plain stored value. Selection still updates correctly; only the layout-invalidating subscription leaves the hot path.TextSelectionBackground→AppKitTextSelectionView and AttachmentOverlay→AttachmentView. The attachment path fired even for fragments with zero attachments, so fixing only the selection path would have left the hang reproducible.Ready to merge
A small, surgical fix (+73/−23 across 4 files) for an upstream-known infinite SwiftUI layout loop (gonzalezreal/textual#26). It removes the per-measurement @Environment(TextSelectionModel.self) subscription from inside two GeometryReaders and passes the model down as a plain value instead — matching the package's own existing idiom (TextLinkInteraction already does this for openURL). Three independent review lenses (reuse, quality, efficiency) confirmed the fix is correct, complete for both the selection and attachment paths, and introduces no new per-render waste. macOS and iOS builds pass; no unresolved review comments. The only follow-ups are minor and out of scope (a pre-existing zero-attachment GeometryReader allocation, and a manual in-app confirmation of the hang fix).
Shown verbatim — the markdown the author wrote, unmodified.
## Problem
On macOS, Prism froze indefinitely (main thread pegged, app unresponsive) while reading certain markdown documents (e.g. nested task lists with text selection enabled). A stackshot showed the main thread stuck in `GraphHost.flushTransactions → AG::Subgraph::update`, churning text layout forever, with `TextSelectionBackground.body` and `AttachmentOverlay.body` the only app frames on the stack.
This is upstream **gonzalezreal/textual issue #26** ("Infinite SwiftUI layout loop in TextSelectionBackground + GeometryReader + @Environment"), still open upstream with no merged fix.
## Root cause
`TextSelectionBackground` and `AttachmentOverlay` each create a child view **inside a `GeometryReader`** that reads `@Environment(TextSelectionModel.self)`:
- `AppKitTextSelectionView` (in `TextSelectionBackground`)
- `AttachmentView` (in `AttachmentOverlay`)
An `@Environment` observable read in that position registers an Observation dependency inside the layout subgraph. Each `GeometryReader` measurement re-arms that dependency, SwiftUI invalidates the subgraph, which re-measures — an unbounded layout-invalidation loop that never lets the run loop return to processing events.
`AttachmentView` read the model even when a fragment has **no attachments**, so the loop triggers on plain text too.
## Fix
Read `@Environment(TextSelectionModel.self)` at the **modifier** level (outside the `GeometryReader`) in both `TextSelectionBackground` and `AttachmentOverlay`, and pass it down as a plain value to `AppKitTextSelectionView` / `AttachmentView`.
Selection still updates correctly: the child views already observe `selectedRange` via their own `onChange` / Canvas reads. Only the layout-invalidating environment subscription leaves the hot path. No public API change.
## Testing
- `swift build` clean.
- `swift test`: all selection/attachment/footnote suites pass (`TextSelectionModelTests`, `FootnoteReferenceTests`, etc.). The only failures are pre-existing `CodeTokenizerTests` (HighlightSwift resource bundle not loadable under bare `swift test`) — identical on `main`.
Relates to upstream gonzalezreal/textual#26.
cc9d5cb Fix infinite layout loop from @Environment reads inside GeometryReader (issue #26) The Prism app froze (beachball) on macOS when you opened certain Markdown files — the whole window stopped responding. The bug was in Textual, the library Prism uses to draw Markdown text.
SwiftUI lays views out by repeatedly measuring them until the sizes settle. Here, the code that draws the text-selection highlight (and attachment overlays) accidentally told SwiftUI "something changed" every single time it measured. So measuring never finished — the app got stuck re-measuring forever and could no longer respond to clicks.
The offending line read a piece of shared state (the current text selection) from the environment while inside a measuring block (GeometryReader). The fix moves that read just outside the measuring block and hands the value in directly. Same data, same on-screen behaviour — but the measuring loop now settles instead of running forever.
Each text fragment in Textual applies three modifiers: TextSelectionBackground, AttachmentOverlay, and TextLinkInteraction. The first two use background/overlayPreferenceValue(Text.LayoutKey) + a GeometryReader to position selection rectangles / attachment glyphs against the resolved Text.Layout. The child views (AppKitTextSelectionView, AttachmentView) read the shared @Observable TextSelectionModel from @Environment.
An @Environment read of an observable, performed inside the GeometryReader child, registers an Observation/AttributeGraph dependency inside the layout subgraph. Every geometry measurement re-arms that dependency, SwiftUI marks the subgraph dirty, and it re-measures — an unbounded GraphHost.flushTransactions → AG::Subgraph::update loop that never returns control to the run loop.
Hoist the @Environment(TextSelectionModel.self) read to the ViewModifier level (outside the GeometryReader) and pass the model into the child as a plain let. This is exactly what the sibling TextLinkInteraction modifier already does for @Environment(\.openURL), so the change aligns the two selection/attachment modifiers with the package's own established idiom.
The correctness hinge is Swift Observation semantics: field-level tracking arms when a tracked property is read during a view's body/closure evaluation, independent of how the object reference was obtained. TextSelectionModel is a stable @State-owned singleton injected once via .environment(model) at interaction scope, so its identity never changes. Passing it as a stored let instead of an @Environment property changes nothing for the downstream reads of selectedRange in AppKitTextSelectionView.onChange/updateSelectionRects and AttachmentView.opacity(...) (inside Canvas, not layout) — those still register the dependency and re-render on selection change.
What is removed is the modifier-vs-child placement of the subscription. Neither modifier body reads selectedRange, so no @Observable field-tracking is armed at the modifier level; the modifier only subscribes to the (stable) environment object identity. The GeometryReader child no longer establishes any per-measurement observation dependency, so the layout subgraph stops re-dirtying itself. Invalidation scope is no broader than before — arguably narrower.
Both per-fragment paths are fixed. The attachment path matters even with no attachments: AttachmentOverlay is applied to every fragment and the old @Environment declaration fired Observation tracking unconditionally, before opacity() ever guarded on attachments. Conditional compilation is consistent across all four files: the disabled build uses AttachmentView's 3-arg init and excludes AppKitTextSelectionView entirely. The dual #if/#else init/call-site is forced (Swift forbids #if inside a parameter list) and matches house style. A remaining, pre-existing inefficiency (every fragment is wrapped in overlayPreferenceValue+GeometryReader+AttachmentView even when attachments.isEmpty) is unrelated to the loop and left for a follow-up.
Sources/Textual/Internal/TextInteraction/Shared/TextSelectionBackground.swift
Why it matters. This is the loop site named in upstream issue #26 — the selection-highlight modifier applied to every text fragment.
What to look at. TextSelectionBackground.swift (adds the @Environment property at struct scope; passes textSelectionModel into AppKitTextSelectionView)
Sources/Textual/Internal/TextInteraction/AppKit/AppKitTextSelectionView.swift
Why it matters. The child view is the thing created inside the GeometryReader; removing its @Environment declaration is what actually takes the subscription off the layout path.
What to look at. AppKitTextSelectionView.swift (drops @Environment(TextSelectionModel.self); adds stored let + init param)
Sources/Textual/Internal/Attachment/AttachmentView.swift
Why it matters. A SECOND instance of the same antipattern, not covered by issue #26. The crash report showed AttachmentOverlay.body in the loop too, and the read fired even for fragments with zero attachments — so the hang was reproducible without fixing this.
TextLinkInteraction idiom in the same package.AttachmentView carried the identical @Environment(TextSelectionModel.self)-inside-GeometryReader shape and ran for every fragment (including attachment-free ones). The original hang stackshot showed AttachmentOverlay.body in the loop, so both paths were fixed.#if inside a parameter list, and TextSelectionModel does not exist in the non-AppKit build, so the initializer and its call must be conditionally compiled in full. This matches conditional-compilation usage elsewhere in the package.| Severity | Area | Finding | Resolution |
|---|---|---|---|
| minor | AttachmentView.swift / AttachmentOverlay.swift — conditional compilation | The fix introduces a dual #if/#else initializer in AttachmentView and a dual #if/#else AttachmentView(...) call site in AttachmentOverlay (4-param selection-enabled vs 3-param disabled). Mild copy-paste. | Acceptable as-is: forced by Swift (no #if inside a parameter list) and the model type is absent off-AppKit; matches the package's house style. Left for the author to decide. |
| minor | AttachmentOverlay.swift — zero-attachment overhead (pre-existing, out of scope) | AttachmentOverlay still wraps every fragment in overlayPreferenceValue + GeometryReader + AttachmentView even when attachments.isEmpty, building an empty Canvas/ForEach. This is unchanged by the PR and does not feed the loop. | Optional follow-up: short-circuit AttachmentOverlay.body to return content unmodified when attachments.isEmpty, skipping the GeometryReader entirely. Not part of this loop fix. |
Click to expand.
diff --git a/Sources/Textual/Internal/Attachment/AttachmentOverlay.swift b/Sources/Textual/Internal/Attachment/AttachmentOverlay.swiftindex 0270458..d69ccde 100644--- a/Sources/Textual/Internal/Attachment/AttachmentOverlay.swift+++ b/Sources/Textual/Internal/Attachment/AttachmentOverlay.swift@@ -8,8 +8,16 @@ import SwiftUI // `Text.LayoutKey` preference; this modifier reads the anchored layout, converts its anchor to a // concrete origin using `GeometryReader`, and installs an `AttachmentView` that draws attachments // at their run bounds.+//+// The `TextSelectionModel` (used to dim attachments inside the selection) is read from+// `@Environment` here, at the modifier level, and passed into `AttachmentView`. It must NOT be read+// with `@Environment` inside the `GeometryReader` below: doing so re-arms the layout subgraph on+// every measurement and triggers an infinite SwiftUI layout invalidation loop (issue #26). struct AttachmentOverlay: ViewModifier {+ #if TEXTUAL_ENABLE_TEXT_SELECTION && canImport(AppKit)+ @Environment(TextSelectionModel.self) private var textSelectionModel: TextSelectionModel?+ #endif private let attachments: Set<AnyAttachment> init(attachments: Set<AnyAttachment>) {@@ -21,11 +29,20 @@ struct AttachmentOverlay: ViewModifier { .overlayPreferenceValue(Text.LayoutKey.self) { value in if let anchoredLayout = value.first { GeometryReader { geometry in- AttachmentView(- attachments: attachments,- origin: geometry[anchoredLayout.origin],- layout: anchoredLayout.layout- )+ #if TEXTUAL_ENABLE_TEXT_SELECTION && canImport(AppKit)+ AttachmentView(+ attachments: attachments,+ origin: geometry[anchoredLayout.origin],+ layout: anchoredLayout.layout,+ textSelectionModel: textSelectionModel+ )+ #else+ AttachmentView(+ attachments: attachments,+ origin: geometry[anchoredLayout.origin],+ layout: anchoredLayout.layout+ )+ #endif } } }diff --git a/Sources/Textual/Internal/Attachment/AttachmentView.swift b/Sources/Textual/Internal/Attachment/AttachmentView.swiftindex 5cb50ee..ec98b98 100644--- a/Sources/Textual/Internal/Attachment/AttachmentView.swift+++ b/Sources/Textual/Internal/Attachment/AttachmentView.swift@@ -11,24 +11,42 @@ import SwiftUI // Selection integration: // On macOS, when text selection is enabled, object-style attachments are dimmed when they fall // inside the selected range. Inline-style attachments (for example, emoji) are not dimmed.+//+// The `TextSelectionModel` is supplied by the parent (`AttachmentOverlay`) instead of being read+// from `@Environment` here. This view is created inside a `GeometryReader`, and an `@Environment`+// observable read in that position re-arms the layout subgraph on every measurement, producing an+// infinite SwiftUI layout invalidation loop (issue #26). The read happens even when a fragment has+// no attachments, so it must be hoisted out of the GeometryReader regardless of attachment count. struct AttachmentView: View {- #if TEXTUAL_ENABLE_TEXT_SELECTION && canImport(AppKit)- @Environment(TextSelectionModel.self) private var textSelectionModel: TextSelectionModel?- #endif private let attachments: Set<AnyAttachment> private let origin: CGPoint private let layout: Text.Layout+ #if TEXTUAL_ENABLE_TEXT_SELECTION && canImport(AppKit)+ private let textSelectionModel: TextSelectionModel? - init(- attachments: Set<AnyAttachment>,- origin: CGPoint,- layout: Text.Layout- ) {- self.attachments = attachments- self.origin = origin- self.layout = layout- }+ init(+ attachments: Set<AnyAttachment>,+ origin: CGPoint,+ layout: Text.Layout,+ textSelectionModel: TextSelectionModel?+ ) {+ self.attachments = attachments+ self.origin = origin+ self.layout = layout+ self.textSelectionModel = textSelectionModel+ }+ #else+ init(+ attachments: Set<AnyAttachment>,+ origin: CGPoint,+ layout: Text.Layout+ ) {+ self.attachments = attachments+ self.origin = origin+ self.layout = layout+ }+ #endif var body: some View { Canvas { context, _ indiff --git a/Sources/Textual/Internal/TextInteraction/AppKit/AppKitTextSelectionView.swift b/Sources/Textual/Internal/TextInteraction/AppKit/AppKitTextSelectionView.swiftindex 41e4db9..65263f9 100644--- a/Sources/Textual/Internal/TextInteraction/AppKit/AppKitTextSelectionView.swift+++ b/Sources/Textual/Internal/TextInteraction/AppKit/AppKitTextSelectionView.swift@@ -5,20 +5,24 @@ // // `AppKitTextSelectionView` renders selection highlights for a single `Text.Layout`. //- // Each text fragment provides its own resolved layout and origin. The view reads the shared- // `TextSelectionModel` from the environment, computes selection rectangles for the current- // range within this layout, and paints them in a `Canvas` behind the text.+ // Each text fragment provides its own resolved layout and origin. The shared `TextSelectionModel`+ // is supplied by the parent (`TextSelectionBackground`) rather than read from `@Environment` here:+ // this view is created inside a `GeometryReader`, and an `@Environment` observable read in that+ // position re-arms the layout subgraph on every measurement, producing an infinite layout+ // invalidation loop (see issue #26). The view computes selection rectangles for the current range+ // within this layout and paints them in a `Canvas` behind the text. struct AppKitTextSelectionView: View {- @Environment(TextSelectionModel.self) private var textSelectionModel: TextSelectionModel? @State private var selectionRects: [TextSelectionRect] = [] private let layout: Text.Layout private let origin: CGPoint+ private let textSelectionModel: TextSelectionModel? - init(layout: Text.Layout, origin: CGPoint) {+ init(layout: Text.Layout, origin: CGPoint, textSelectionModel: TextSelectionModel?) { self.layout = layout self.origin = origin+ self.textSelectionModel = textSelectionModel } var body: some View {diff --git a/Sources/Textual/Internal/TextInteraction/Shared/TextSelectionBackground.swift b/Sources/Textual/Internal/TextInteraction/Shared/TextSelectionBackground.swiftindex 4638723..d118b41 100644--- a/Sources/Textual/Internal/TextInteraction/Shared/TextSelectionBackground.swift+++ b/Sources/Textual/Internal/TextInteraction/Shared/TextSelectionBackground.swift@@ -7,8 +7,18 @@ import SwiftUI // The platform selection interaction stores a `TextSelectionModel` in the environment at fragment // scope. This modifier reads the fragment’s anchored `Text.Layout` and forwards it to the AppKit // selection view so it can convert the current selected range into highlight rectangles.+//+// The `TextSelectionModel` is read from `@Environment` here, at the modifier level, and passed into+// `AppKitTextSelectionView` as a plain value. It must NOT be read with `@Environment` inside the+// `GeometryReader` below: doing so re-arms the layout subgraph on every measurement and triggers an+// infinite SwiftUI layout invalidation loop (issue #26). Reading it outside the GeometryReader keeps+// the observation dependency off the layout hot path. struct TextSelectionBackground: ViewModifier {+ #if TEXTUAL_ENABLE_TEXT_SELECTION && canImport(AppKit)+ @Environment(TextSelectionModel.self) private var textSelectionModel: TextSelectionModel?+ #endif+ func body(content: Content) -> some View { #if TEXTUAL_ENABLE_TEXT_SELECTION && canImport(AppKit) content@@ -17,7 +27,8 @@ struct TextSelectionBackground: ViewModifier { GeometryReader { geometry in AppKitTextSelectionView( layout: anchoredLayout.layout,- origin: geometry[anchoredLayout.origin]+ origin: geometry[anchoredLayout.origin],+ textSelectionModel: textSelectionModel ) } }tasks.md and confirm no freeze) has not been verified live — the SwiftUI view-test classes that would exercise it crash pre-existingly in headless make test-quick. A manual launch is the missing check.cc9d5cb. A squash-merge would create a new SHA on main; re-pin Prism to that commit afterwards for a clean, durable reference.