Skip to content

Commit ec1eeb5

Browse files
authored
fix(editor): lay out wrapped text once per row instead of once per prefix (#2165)
1 parent a5d15d6 commit ec1eeb5

3 files changed

Lines changed: 94 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1717

1818
### Fixed
1919

20+
- Viewing a very long single line with word wrap on could use enormous amounts of memory and stall or kill the app. This hit the JSON value viewer, chat code blocks and the SQL review sheet, which always wrap, and the SQL editor when Word Wrap is on. Wrapped text is now laid out once instead of once per wrapped row, so a long line stays fast no matter how long it is.
2021
- Format Query crashed the app on a string literal that was still open and ended in a backslash, as in `select * from t where c like 'C:\`. It formats such a query without crashing now. Format Query also used to move the last character of an unclosed `/*` comment out of the comment and reformat it as code; the whole comment is left alone now.
2122
- Pasting a large block of text into the query editor could crash the app. A paste of more than about a thousand characters is parsed in the background, and the editor's syntax highlighting was updated from that background work while the editor was still applying the same paste on screen. Highlighting is now updated on the main thread again, as the rest of the editor already did. (#2158)
2223
- A strip along the right edge of the SQL editor, as wide as 140 points, took clicks and did nothing with them. Clicking there now puts the caret on the line you clicked, like any other empty part of the editor. The same strip sat in the trigger editor, the JSON view, the structure DDL, the SQL review sheet, the import preview and chat code blocks, where it swallowed text selection instead. (#2156)

LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextLine/Typesetter/Typesetter.swift

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,11 @@ final public class Typesetter {
180180
constrainingWidth: displayData.maxWidth - context.fragmentContext.width
181181
)
182182

183-
// Indicates the subrange on the range that the typesetter knows about. This may not be the entire line
184-
let typesetSubrange = NSRange(location: context.currentPosition - range.location, length: lineBreak)
183+
// Indicates the subrange on the range that the typesetter knows about. This may not be the entire line.
184+
// `lineBreak` is an offset into the run, not a length, so the fragment has to be measured from where
185+
// this fragment starts. Using it as a length re-typesets everything before it once per fragment.
186+
let startOffset = context.currentPosition - range.location
187+
let typesetSubrange = NSRange(location: startOffset, length: lineBreak - startOffset)
185188
let typesetData = typesetLine(typesetter: typesetter, range: typesetSubrange)
186189

187190
// The typesetter won't tell us if 0 characters can fit in the constrained space. This checks to
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
//
2+
// TypesetterWrapLengthTests.swift
3+
// TableProTests
4+
//
5+
// Regression tests for wrapped line typesetting. `suggestLineBreak` returns an offset into the run,
6+
// not a length, but the typesetter passed it straight through as the CTLine length. Every fragment
7+
// after the first then re-typeset all the text before it, so glyph work and retained memory grew
8+
// with the square of the line length and a long single line could exhaust memory.
9+
//
10+
// These live here rather than in CodeEditTextViewTests because the TablePro scheme does not run
11+
// that package's test target, so a test there would never gate a regression.
12+
//
13+
14+
import AppKit
15+
@testable import CodeEditTextView
16+
import Foundation
17+
import Testing
18+
19+
@MainActor
20+
@Suite("Typesetter wrapped fragment lengths")
21+
struct TypesetterWrapLengthTests {
22+
private static let attributes: [NSAttributedString.Key: Any] = [
23+
.font: NSFont.monospacedSystemFont(ofSize: 10, weight: .regular)
24+
]
25+
26+
private func typesetWrapped(characterCount: Int, maxWidth: CGFloat) -> Typesetter {
27+
let typesetter = Typesetter()
28+
typesetter.typeset(
29+
NSAttributedString(string: String(repeating: "A", count: characterCount), attributes: Self.attributes),
30+
documentRange: NSRange(location: 0, length: characterCount),
31+
displayData: TextLine.DisplayData(
32+
maxWidth: maxWidth,
33+
lineHeightMultiplier: 1.0,
34+
estimatedLineHeight: 20.0,
35+
breakStrategy: .character
36+
),
37+
markedRanges: nil,
38+
attachments: []
39+
)
40+
return typesetter
41+
}
42+
43+
@Test("A wrapped line typesets each character exactly once")
44+
func wrappedLineTypesetsEachCharacterOnce() {
45+
let characterCount = 1_000
46+
let typesetter = typesetWrapped(characterCount: characterCount, maxWidth: 150)
47+
48+
var typesetCharacters = 0
49+
for fragment in typesetter.lineFragments {
50+
typesetCharacters += fragment.data.contents.reduce(0) { $0 + $1.length }
51+
}
52+
53+
#expect(
54+
typesetCharacters == characterCount,
55+
"Each character must be typeset once, not once per following fragment"
56+
)
57+
}
58+
59+
@Test("Each wrapped fragment typesets exactly the characters it covers")
60+
func eachFragmentTypesetsOnlyItsOwnCharacters() {
61+
let typesetter = typesetWrapped(characterCount: 1_000, maxWidth: 150)
62+
63+
for fragment in typesetter.lineFragments {
64+
let typesetLength = fragment.data.contents.reduce(0) { $0 + $1.length }
65+
#expect(
66+
typesetLength == fragment.range.length,
67+
"A fragment covering \(fragment.range.length) characters typeset \(typesetLength)"
68+
)
69+
}
70+
}
71+
72+
@Test("Wrapping stays linear as the line grows")
73+
func wrappingStaysLinearAsTheLineGrows() {
74+
let small = typesetWrapped(characterCount: 1_000, maxWidth: 150)
75+
let large = typesetWrapped(characterCount: 4_000, maxWidth: 150)
76+
77+
func typesetCharacters(in typesetter: Typesetter) -> Int {
78+
var total = 0
79+
for fragment in typesetter.lineFragments {
80+
total += fragment.data.contents.reduce(0) { $0 + $1.length }
81+
}
82+
return total
83+
}
84+
85+
// Four times the text must cost four times the typesetting, not sixteen.
86+
#expect(typesetCharacters(in: large) == typesetCharacters(in: small) * 4)
87+
}
88+
}

0 commit comments

Comments
 (0)