Skip to content

fix(editor): lay out wrapped text once per row instead of once per prefix - #2165

Merged
datlechin merged 2 commits into
mainfrom
fix/typesetter-wrap-quadratic
Aug 17, 2026
Merged

fix(editor): lay out wrapped text once per row instead of once per prefix#2165
datlechin merged 2 commits into
mainfrom
fix/typesetter-wrap-quadratic

Conversation

@datlechin

Copy link
Copy Markdown
Member

What you see now

A very long single line no longer makes the app balloon in memory and stall or get killed when word
wrap is on. Wrapped text is laid out once instead of once per wrapped row.

Found while investigating #2158. Different subsystem and different trigger, so it ships on its own.

Root cause

suggestLineBreak returns an absolute offset into the run being typeset, not a length:

  • CTTypesetter+SuggestLineBreak.swift:54: breakIndex = startingOffset + CTTypesetterSuggestClusterBreak(...)
  • :78: the word-break path does the same with subrange.location + ...
  • its own doc comment at :17 says "An offset relative to the entire string"

Typesetter.swift:184 passed that straight through as an NSRange length:

let typesetSubrange = NSRange(location: context.currentPosition - range.location, length: lineBreak)

On the first fragment startOffset is 0, so the two coincide and nothing looks wrong. From the second
fragment on, the typeset range is too long by exactly startOffset, so fragment k typesets roughly
k × w characters instead of w. CTTypesetterCreateLine only clamps at the end of the string, so
nothing catches it. Every over-long CTLine is then retained in LineFragment.contents
(TypesetContext.swift:51), which makes both the glyph work and the retained memory grow with the
square of the line length.

Fragment ranges were never affected: appendText advances with
lineBreak + typesettingRange.location (TypesetContext.swift:56), which is correct under absolute
semantics. That is why the existing wrap test passes today and why this went unnoticed. Only the
CTLine range, its measured width, and what derives from them were wrong.

Measured on this fix's own test shape (1,000 chars at maxWidth 150): 10,920 glyphs before, 1,000
after
. A prior lane measured 260,000 chars producing ~160.9M glyphs instead of 260,000.

The change

One expression, matching how appendText already advances:

let startOffset = context.currentPosition - range.location
let typesetSubrange = NSRange(location: startOffset, length: lineBreak - startOffset)

This is identical to upstream's open PR CodeEditApp/CodeEditTextView#122 ("Fix Word Duplication at
Wrap Boundaries"), which patches the same blob. The user-visible bug upstream reports, duplicated
words at wrap boundaries, is the drawing half of the same defect.

Deliberately not clamped with max(0, ...): a CFRange length of 0 means "to the end of the
string", so a zero clamp would silently restore the huge line. lineBreak > startOffset holds
anyway, because CTTypesetterSuggestClusterBreak returns at least 1 for any offset below the length,
and the currentPosition < range.max loop guard excludes the equal case.

Also deliberately untouched: the if lineBreak == 1 guard on the next line has the same units
confusion, but a length-based guard there can spin forever, since popCurrentData() plus continue
does not advance currentPosition. It needs its own analysis and is recorded as follow-up.

Reachability

Needs wrapLines = true, otherwise maxLineLayoutWidth is .greatestFiniteMagnitude
(TextLayoutManager.swift:115), the loop runs once per run, and startOffset stays 0.

Always-wrapped surfaces: JSONCodeEditor.swift:46, AIChatCodeBlockView.swift:200,
SQLReviewSheet.swift:303. The SQL editor wraps only when the Word Wrap setting is on, which
defaults to off (EditorSettings.swift:84). The minimap mirrors the text view. Severity scales with
the longest single line, so a long JSON value or a big one-line paste is the worst case.

Files

  • LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextLine/Typesetter/Typesetter.swift
  • TableProTests/Views/Editor/TypesetterWrapLengthTests.swift (new)
  • CHANGELOG.md

Verification

Step Result
generate, build PASS
test TypesetterWrapLengthTests pre-fix FAIL, 3 executed, 0 passed, 3 failed
test TypesetterWrapLengthTests GutterHighlightTests SQLEditorCoordinatorTests after PASS, 13 executed, 13 passed
swift test --filter TypesetterTests (the package's own suite) PASS, 9 tests, 0 failures
swiftlint PASS, 0 violations

The new suite lives in TableProTests because the TablePro scheme does not run
CodeEditTextViewTests, so a test there would never gate a regression. It asserts an algorithmic
invariant, not wall-clock time, so it cannot flake on a loaded machine:

  1. total characters typeset across fragments equals the line length
  2. each fragment typesets exactly the characters its own range covers
  3. four times the text costs four times the typesetting, not sixteen

Upstream's 9 existing typesetter tests still pass, including the wrap test that asserts fragment
ranges, which confirms fragment ranges are unchanged.

Limitations

  • The 3.7 GB RSS and SIGKILL figures come from an earlier investigation lane and were not
    re-measured here. What this PR measured directly is the glyph count, 10,920 vs 1,000 on the test
    shape.
  • This is the first local divergence in Typesetter.swift, which is otherwise byte-identical to
    upstream. A future package sync must not revert it. Upstream PR Remove Create Table UI feature #122 is still open.
  • Fragment widths are no longer inflated, which feeds LineFragmentView sizing and
    maxLineWidth. Existing attachment tests pass, but they use short strings where startOffset is 0.
  • The required cross-vendor Codex review could not run: Codex returned a usage-limit error. Reported
    as not started rather than as passed.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin
datlechin merged commit ec1eeb5 into main Aug 17, 2026
3 of 4 checks passed
@datlechin
datlechin deleted the fix/typesetter-wrap-quadratic branch August 17, 2026 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant