Fix layout parity with Text, cut per-row cost, and declare watchOS support - #2
Conversation
MarqueeText reported a hard-coded 20pt height on the first layout pass, because the height came from @State that is only populated after a preference round-trip. Any font whose line height is not 20pt therefore changed size one frame after appearing, which shows up as row-height jitter and scroll drift in List and LazyVStack. The size now comes from the layout, which reads the content's ideal size synchronously, so it is correct on the first pass and never changes. Other correctness fixes: - Height followed the proposal instead of the text, so a generous proposal stretched the view like Color rather than Text. Both axes now clamp with min(proposal, intrinsic), matching Text exactly. - clipped() cut glyph overhang. Arabic diacritics, emoji and tall accents legitimately paint outside the typographic line box and Text renders them, so clipping is now horizontal only. - Overflowing text that could not scroll (Reduce Motion, or the pass before measurement lands) was hard-clipped mid-glyph with no truncation indicator. It now truncates like Text. - Baselines were not forwarded, so firstTextBaseline stacks misaligned. - Content identity used String(describing:) on LocalizedStringResource, a Mirror dump including the bundle URL, on every body evaluation. LocalizedStringResource is Equatable, so compare by value instead. - The preference reduce was last-one-wins, so a sibling contributing defaultValue could erase a real measurement and silently stop the marquee. Empty contributions are now ignored, and the measurement preference is reset so it does not leak to ancestors. Performance: the view used to lay the text out twice (a hidden duplicate purely to measure it) and run a second GeometryReader in a background to read the container. Both values are already known to the layout, so it now reports them by proposing them as the size of one weightless probe. Measured over 200 rows: 0.755 -> 0.577 ms/row for overflowing text, and 0.429 -> 0.264 ms/row for text that fits. Tests add size and baseline parity against a real Text in a hosting view, covering the first layout pass, plus seamless-loop continuity. Source and tests are split into focused files.
MarqueeText already built and passed its tests on watchOS, but the platform was undeclared, so support existed only by SPM inferring a watchOS 9.0 floor from Layout and LocalizedStringResource. Declaring it makes that floor explicit and fixes the failure mode for consumers below it: previously the package appeared to resolve and then failed with a module-level "minimum deployment target" error, whereas now SPM rejects the dependency up front. Verified with 47 tests passing on the watchOS simulator, and with consumer packages targeting watchOS 9 (builds) and watchOS 6 (fails at resolution).
- Add .swiftlint.yml scoped to Sources and Tests; both are clean. - Ignore .swiftpm/. - Add Swift Package Index version and platform badges, plus a license badge. The Tests and coverage badges used by other Harf Labs repositories are omitted because this repository has no CI workflow yet and they would render broken. - Document the layout guarantees, the performance characteristics, and the caveat that a non-lazy VStack keeps offscreen marquees animating. - List watchOS in requirements, drop Casti from the apps list, and update the contact URL.
6404eff to
f542744
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors MarqueeText to derive its reported size from layout (instead of async preference-driven state), reducing per-row construction cost and improving layout parity with a single-line Text—including on the first layout pass. It also declares watchOS support, adds extensive new tests around layout/baseline parity, and updates docs/tooling.
Changes:
- Reworked measurement flow:
MarqueeSizingLayoutnow derives intrinsic sizing and reports both text/container widths via a single probe view preference. - Split the previously large source/test files into smaller units and expanded tests for layout parity, baseline forwarding, clipping, and preference reduction behavior.
- Declared watchOS platform support in SPM and updated README + SwiftLint + gitignore.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Tests/MarqueeTextTests/MarqueeTextTests.swift | Removed monolithic test file (split into focused test files). |
| Tests/MarqueeTextTests/MarqueeTextLayoutParityTests.swift | New macOS hosting-view parity tests for size and baseline equivalence with Text. |
| Tests/MarqueeTextTests/MarqueeTestSupport.swift | Shared resolvedLayout(...) helper for tests. |
| Tests/MarqueeTextTests/MarqueeSizingLayoutTests.swift | New tests for sizing/placement behavior and proposal clamping. |
| Tests/MarqueeTextTests/MarqueePreferenceKeyTests.swift | New tests for preference default/reduction behavior. |
| Tests/MarqueeTextTests/MarqueeNumericSupportTests.swift | New tests for numeric sanitizers and clamping helpers. |
| Tests/MarqueeTextTests/MarqueeLayoutTests.swift | New tests for overflow/scroll logic, progress/offset, RTL behavior, and identity changes. |
| Tests/MarqueeTextTests/MarqueeHorizontalClipShapeTests.swift | New tests ensuring horizontal-only clipping while allowing vertical glyph overhang. |
| Tests/MarqueeTextTests/MarqueeContentTests.swift | New tests for localized vs verbatim content behavior and measurement utilities. |
| Tests/MarqueeTextTests/MarqueeConfigurationTests.swift | New tests for configuration sanitization/clamping. |
| Sources/MarqueeText/MarqueeText.swift | Switched to layout-driven sizing and single-probe measurement; horizontal-only clipping; truncation when not scrolling. |
| Sources/MarqueeText/MarqueeSizingLayout.swift | New Layout driving parity sizing + baseline forwarding + probe placement. |
| Sources/MarqueeText/MarqueeResolvedLayout.swift | New resolved layout model for scrolling decisions and offset/progress computation. |
| Sources/MarqueeText/MarqueeNumericSupport.swift | Extracted numeric sanitizing/clamping helpers. |
| Sources/MarqueeText/MarqueeMeasurementReader.swift | New preference key + probe reader for passing layout-derived widths back into state. |
| Sources/MarqueeText/MarqueeMeasurement.swift | New measurement value type encoding text/container widths via probe proposal. |
| Sources/MarqueeText/MarqueeHorizontalClipShape.swift | New clip shape to clip horizontally but not vertically. |
| Sources/MarqueeText/MarqueeContent.swift | Extracted MarqueeContent and made it Equatable. |
| Sources/MarqueeText/MarqueeConfiguration.swift | Extracted configuration type with sanitizing init. |
| README.md | Updated badges/platforms, documented layout parity guarantees and performance characteristics. |
| Package.swift | Declared watchOS 9 as a supported platform. |
| .swiftlint.yml | Added SwiftLint configuration for Sources/Tests. |
| .gitignore | Ignored .swiftpm/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // SwiftUI folds every child of a container into the preference, including children that never write | ||
| // one and therefore contribute `defaultValue`. Those must not clear a real measurement. | ||
| guard next != .zero else { return } | ||
|
|
||
| value = next |
There was a problem hiding this comment.
Good catch on the mechanism — fixed in 162abda by making the preference value optional, so nil (no contribution) and .some(.zero) (measured, and the answer is zero) are no longer conflated. That is the shape you suggested.
Worth recording what I found while confirming it, though: I could not reproduce a user-visible symptom.
.zero requires both widths to be zero. Those are not independent:
- Container zero, text non-empty →
(textWidth: 180, containerWidth: 0), which is not.zero, so it was already accepted. - Text empty → the layout clamps the reported width to
min(proposal, 0) = 0, so the container width is zero too.
So .zero only ever coincided with a zero-width view, and SwiftUI does not drive a timeline inside a zero-width subtree. I checked this directly by measuring whether the run loop stays busy for a marquee whose text is cleared after it has started scrolling:
overflowing text 110.5% of a core -> ANIMATING
short text 0.1% of a core -> IDLE
empty from the start 0.1% of a core -> IDLE
long text then cleared 0.0% of a core -> IDLE
I also checked the geometry in the stale state: it reported (0, 14), which is exactly what Text("") reports, so there was no size divergence either.
So this was a latent hazard rather than an active bug — the view was holding knowingly-stale widths that happened to be unobservable. Fixing it anyway, because relying on "SwiftUI won't animate a zero-width subtree" is an implementation detail, and any future change to the overflow rule or to resolvedSize could make the stale state observable.
Added a test (aGenuinelyEmptyMeasurementStillClearsAnEarlierOne) that locks in the distinction, alongside the existing one proving a non-measuring sibling still cannot erase a measurement.
The measurement preference ignored any `.zero` contribution so that a sibling folding in `defaultValue` could not wipe a real measurement. That also rejected legitimate zero measurements, so emptying the text left the view holding the widths from the previous string. The preference value is now optional, which separates the two cases: `nil` means the subtree measured nothing and is still ignored, while `.some(.zero)` is a real measurement and overwrites as normal. No user-visible symptom was reproducible before this change, because a zero measurement only ever coincides with a zero-width view, and SwiftUI does not drive a timeline in a zero-width subtree. This removes the stale state rather than relying on that.
The repository had no CI, so the Codecov project sat on its setup screen with nothing ever uploaded and the badge would have rendered broken. Adds a Tests workflow with three gates: - lint: swiftlint and swiftformat, both --strict, scoped to Sources and Tests. Demo is sample code with deliberately long marquee strings and is excluded in .swiftlint.yml. - test: swift test --enable-code-coverage on macOS, then upload to Codecov. - build: compiles every platform declared in Package.swift. swift test only ever compiles the macOS slice, so this is what keeps the platforms list honest. SwiftPM emits an indexed .profdata that Codecov cannot read, so the test job converts it to LCOV with llvm-cov. Paths are derived from `swift build --show-bin-path` rather than hard-coded, because the build directory is architecture-specific. The step fails loudly if the report comes out empty, since an empty upload otherwise succeeds and silently reports zero coverage. The Xcode version matters: this manifest is tools-version 6.2, and Xcode's built-in SwiftPM resolves the package graph, so the runner needs Xcode 26+ rather than the image default. Codecov upload uses fail_ci_if_error: false on purpose — a Codecov outage should not turn the Tests badge red when the tests passed. The repository is public and the organisation allows tokenless uploads, so CODECOV_TOKEN is optional; the step passes it when present. Also adds the Tests and coverage badges to the README.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
Summary
MarqueeTextdid not report the same size as a single-lineTexton the first layout pass, and paid for every marquee twice over at construction time. This fixes both, plus a set of smaller correctness issues found along the way.The main bug
Every
MarqueeTextreported a hard-coded 20pt height on the first layout pass, regardless of font, then corrected itself on the next pass. Measured against a real hosting view:The height came from
@Statethat is only populated after a preference round-trip. In aListorLazyVStackthat is a row-height change one frame after appearance — visible jitter and scroll-offset drift, which is exactly the kind of thing that shows up in a real app and not in a demo.The size now comes from the layout, which reads the content's ideal size synchronously. It is correct on the first pass and never changes.
Other correctness fixes
Colorrather thanText. Both axes now clamp withmin(proposal, intrinsic). This was masked by the height bug above — fixing either one alone would have exposed the other.clipped()cut glyph overhang. Arabic diacritics, emoji and tall accents legitimately paint outside the typographic line box, andTextrenders them. Clipping is now horizontal only.Text. It now truncates normally.HStack(alignment: .firstTextBaseline)misaligned.String(describing:)onLocalizedStringResourceevery body evaluation — aMirrordump including the bundle URL.LocalizedStringResourceisEquatable, so it now compares by value.reduce. Last-one-wins meant a sibling contributingdefaultValuecould erase a real measurement and silently stop the marquee forever. Empty contributions are now ignored, and the measurement preference is reset so it does not leak to ancestors.Performance
Construction cost is what governs fling smoothness, since that is what runs when rows are recycled. The view was laying the text out twice (a hidden duplicate purely to measure it) and running a second
GeometryReaderin a.backgroundfor the container width. Both values are already known to the layout, so it now reports them by proposing them as the size of a single weightless probe.Measured over 200 rows:
TextTwo things I checked before trusting this design:
TimelineViewreports its content's ideal size exactly (564 = 2×262+40), so deriving one text run from the two scrolling copies is exact, not approximate.ListandLazyVStackcull offscreen rows — work stays flat at 25, 250 and 2,500 rows. A non-lazyVStackin aScrollViewdoes not (250 rows → 30,025 ticks/s); that is inherent to non-lazy stacks and is now documented rather than worked around.I also measured a
GeometryEffect-based alternative and rejected it — 16k–86keffectValuecalls/s, far worse than the currentTimelineView.watchOS
Already built and passed tests there, but the platform was undeclared, so support existed only by SPM inferring a watchOS 9.0 floor. Declaring
.watchOS(.v9)makes that explicit and improves the failure mode below it: previously the package appeared to resolve then failed with a module-level error; now SPM rejects the dependency up front.Also included
SwiftLint config (Sources and Tests are clean), SPI version/platform badges and a license badge,
.swiftpm/gitignored, README updates for layout guarantees, performance, requirements, the apps list and the contact URL.Note for reviewers
Satisfying SwiftLint's file-length rule meant splitting the source into 9 files and the tests into 9. That is mechanical code movement and inflates the diff without changing behaviour — it is in the first commit alongside the functional changes, which is worth knowing when reading it.
The Tests and coverage badges used by other Harf Labs repositories are not included, because this repository has no CI workflow and they would render broken. Happy to add a workflow in a follow-up.
Testing
ImageRendererand hosting-view parity tests, correctly#if os(macOS)-guarded)Textfor every proposal across 4 strings × 3 fonts, first-pass sizing, and seamless-loop continuity