Skip to content

Commit eabfa87

Browse files
iSapozhnikiSapozhnik
authored
Merge pull request #4 from iSapozhnik/bugfix/width-and-height-diff
Fix TextDiff layout parity and add performance baselines
2 parents 490226b + 214d5eb commit eabfa87

23 files changed

Lines changed: 363 additions & 96 deletions

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ DerivedData/
66
.swiftpm/configuration/registries.json
77
.swiftpm/xcode/package.xcworkspace/contents.xcworkspacedata
88
.netrc
9+
.snapshot-artifacts/

‎.swiftpm/xcode/xcshareddata/xcbaselines/TextDiffTests.xcbaseline/BF20AB95-BD61-4DE5-BFD2-9B7DB182F4A1.plist‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
3+
<plist version="1.0">
4+
<dict>
5+
<key>classNames</key>
6+
<dict>
7+
<key>DiffLayouterPerformanceTests</key>
8+
<dict>
9+
<key>testLayoutPerformance1000Words()</key>
10+
<dict>
11+
<key>com.apple.dt.XCTMetric_Clock.time.monotonic</key>
12+
<dict>
13+
<key>baselineAverage</key>
14+
<real>0.058013</real>
15+
<key>baselineIntegrationDisplayName</key>
16+
<string>Local Baseline</string>
17+
</dict>
18+
</dict>
19+
<key>testLayoutPerformance200Words()</key>
20+
<dict>
21+
<key>com.apple.dt.XCTMetric_Clock.time.monotonic</key>
22+
<dict>
23+
<key>baselineAverage</key>
24+
<real>0.013204</real>
25+
<key>baselineIntegrationDisplayName</key>
26+
<string>Local Baseline</string>
27+
</dict>
28+
</dict>
29+
<key>testLayoutPerformance500Words()</key>
30+
<dict>
31+
<key>com.apple.dt.XCTMetric_Clock.time.monotonic</key>
32+
<dict>
33+
<key>baselineAverage</key>
34+
<real>0.027967</real>
35+
<key>baselineIntegrationDisplayName</key>
36+
<string>Local Baseline</string>
37+
</dict>
38+
</dict>
39+
</dict>
40+
</dict>
41+
</dict>
42+
</plist>

‎.swiftpm/xcode/xcshareddata/xcbaselines/TextDiffTests.xcbaseline/Info.plist‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
3+
<plist version="1.0">
4+
<dict>
5+
<key>runDestinationsByUUID</key>
6+
<dict>
7+
<key>BF20AB95-BD61-4DE5-BFD2-9B7DB182F4A1</key>
8+
<dict>
9+
<key>localComputer</key>
10+
<dict>
11+
<key>busSpeedInMHz</key>
12+
<integer>0</integer>
13+
<key>cpuCount</key>
14+
<integer>1</integer>
15+
<key>cpuKind</key>
16+
<string>Apple M1</string>
17+
<key>cpuSpeedInMHz</key>
18+
<integer>0</integer>
19+
<key>logicalCPUCoresPerPackage</key>
20+
<integer>8</integer>
21+
<key>modelCode</key>
22+
<string>MacBookPro17,1</string>
23+
<key>physicalCPUCoresPerPackage</key>
24+
<integer>8</integer>
25+
<key>platformIdentifier</key>
26+
<string>com.apple.platform.macosx</string>
27+
</dict>
28+
<key>targetArchitecture</key>
29+
<string>arm64</string>
30+
</dict>
31+
</dict>
32+
</dict>
33+
</plist>

‎Sources/TextDiff/AppKit/DiffTextLayoutMetrics.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ import AppKit
22

33
enum DiffTextLayoutMetrics {
44
static func verticalTextInset(for style: TextDiffStyle) -> CGFloat {
5-
ceil(max(2, style.chipInsets.top + 2, style.chipInsets.bottom + 2))
5+
ceil(max(0, style.chipInsets.top, style.chipInsets.bottom))
66
}
77

88
static func lineHeight(for style: TextDiffStyle) -> CGFloat {
9-
let textHeight = ceil(style.font.ascender - style.font.descender + style.font.leading)
9+
let textHeight = style.font.ascender - style.font.descender + style.font.leading
1010
let chipHeight = textHeight + style.chipInsets.top + style.chipInsets.bottom
1111
return ceil(chipHeight + max(0, style.lineSpacing))
1212
}

‎Sources/TextDiff/AppKit/DiffTokenLayouter.swift‎

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ enum DiffTokenLayouter {
2727
contentInsets: NSEdgeInsets
2828
) -> DiffLayout {
2929
let lineHeight = DiffTextLayoutMetrics.lineHeight(for: style)
30+
let textHeight = ceil(style.font.ascender - style.font.descender + style.font.leading)
3031
let maxLineWidth = availableWidth > 0 ? availableWidth : .greatestFiniteMagnitude
3132
let lineStartX = contentInsets.left
3233
let maxLineX = lineStartX + maxLineWidth
@@ -37,12 +38,16 @@ enum DiffTokenLayouter {
3738
var maxUsedX = lineStartX
3839
var lineCount = 1
3940
var lineHasContent = false
41+
let lineText = NSMutableString()
42+
var lineTextWidth: CGFloat = 0
4043
var previousChangedLexical = false
4144

4245
func moveToNewLine() {
4346
lineTop += lineHeight
4447
cursorX = lineStartX
4548
lineHasContent = false
49+
lineText.setString("")
50+
lineTextWidth = 0
4651
previousChangedLexical = false
4752
lineCount += 1
4853
}
@@ -65,9 +70,15 @@ enum DiffTokenLayouter {
6570
}
6671

6772
let attributedText = attributedToken(for: segment, style: style)
68-
let textSize = measuredTextSize(for: piece.text, font: style.font)
73+
var textMeasurement = measuredIncrementalTextWidth(
74+
for: piece.text,
75+
font: style.font,
76+
lineText: lineText,
77+
lineTextWidth: lineTextWidth
78+
)
79+
var textSize = CGSize(width: textMeasurement.textWidth, height: textHeight)
6980
let chipInsets = effectiveChipInsets(for: style)
70-
let runWidth = isChangedLexical ? textSize.width + chipInsets.left + chipInsets.right : textSize.width
81+
var runWidth = isChangedLexical ? textSize.width + chipInsets.left + chipInsets.right : textSize.width
7182
let requiredWidth = leadingGap + runWidth
7283

7384
let wrapped = lineHasContent && cursorX + requiredWidth > maxLineX
@@ -79,6 +90,15 @@ enum DiffTokenLayouter {
7990
if piece.tokenKind == .whitespace {
8091
continue
8192
}
93+
94+
textMeasurement = measuredIncrementalTextWidth(
95+
for: piece.text,
96+
font: style.font,
97+
lineText: lineText,
98+
lineTextWidth: lineTextWidth
99+
)
100+
textSize = CGSize(width: textMeasurement.textWidth, height: textHeight)
101+
runWidth = isChangedLexical ? textSize.width + chipInsets.left + chipInsets.right : textSize.width
82102
}
83103

84104
cursorX += leadingGap
@@ -118,6 +138,7 @@ enum DiffTokenLayouter {
118138
cursorX += runWidth
119139
maxUsedX = max(maxUsedX, cursorX)
120140
lineHasContent = true
141+
lineTextWidth = textMeasurement.combinedLineWidth
121142
previousChangedLexical = isChangedLexical
122143
}
123144

@@ -146,9 +167,28 @@ enum DiffTokenLayouter {
146167
return NSAttributedString(string: segment.text, attributes: attributes)
147168
}
148169

149-
private static func measuredTextSize(for text: String, font: NSFont) -> CGSize {
150-
let measured = (text as NSString).size(withAttributes: [.font: font])
151-
return CGSize(width: ceil(measured.width), height: ceil(measured.height))
170+
private static func measuredIncrementalTextWidth(
171+
for text: String,
172+
font: NSFont,
173+
lineText: NSMutableString,
174+
lineTextWidth: CGFloat
175+
) -> IncrementalTextWidth {
176+
guard !text.isEmpty else {
177+
return IncrementalTextWidth(
178+
textWidth: 0,
179+
combinedLineWidth: lineTextWidth
180+
)
181+
}
182+
183+
lineText.append(text)
184+
// TODO: Fix this later
185+
// This now appends each token to lineText and calls size(withAttributes:) on the entire accumulated line every iteration, which makes layout cost grow quadratically with line length. On long unwrapped diffs (hundreds/thousands of tokens), this is a significant regression from the prior per-token measurement approach and can noticeably slow rendering even though the new performance tests only capture baselines and do not enforce thresholds.
186+
let combinedWidth = lineText.size(withAttributes: [.font: font]).width
187+
let textWidth = max(0, combinedWidth - lineTextWidth)
188+
return IncrementalTextWidth(
189+
textWidth: textWidth,
190+
combinedLineWidth: combinedWidth
191+
)
152192
}
153193

154194
private static func effectiveChipInsets(for style: TextDiffStyle) -> NSEdgeInsets {
@@ -262,3 +302,8 @@ private struct LayoutPiece {
262302
let text: String
263303
let isLineBreak: Bool
264304
}
305+
306+
private struct IncrementalTextWidth {
307+
let textWidth: CGFloat
308+
let combinedLineWidth: CGFloat
309+
}

‎Sources/TextDiff/TextDiffView.swift‎

Lines changed: 50 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public struct TextDiffView: View {
3535
style: style,
3636
mode: mode
3737
)
38-
.accessibilityLabel("Text diff")
38+
.accessibilityLabel("Text diff")
3939
}
4040
}
4141

@@ -49,6 +49,7 @@ public struct TextDiffView: View {
4949
}
5050

5151
#Preview("TextDiffView") {
52+
let font: NSFont = .systemFont(ofSize: 16, weight: .regular)
5253
let style = TextDiffStyle(
5354
additionsStyle: TextDiffChangeStyle(
5455
fillColor: .systemGreen.withAlphaComponent(0.28),
@@ -62,7 +63,7 @@ public struct TextDiffView: View {
6263
strikethrough: true
6364
),
6465
textColor: .labelColor,
65-
font: .systemFont(ofSize: 16, weight: .regular),
66+
font: font,
6667
chipCornerRadius: 3,
6768
chipInsets: NSEdgeInsets(top: 0, left: 0, bottom: 0, right: 0),
6869
interChipSpacing: 1,
@@ -72,11 +73,11 @@ public struct TextDiffView: View {
7273
Text("Diff by characters")
7374
.bold()
7475
TextDiffView(
75-
original: "Add a diff view! Looks good!",
76-
updated: "Added a diff view. It looks good!",
77-
style: style,
78-
mode: .character
79-
)
76+
original: "Add a diff view! Looks good!",
77+
updated: "Added a diff view. It looks good!",
78+
style: style,
79+
mode: .character
80+
)
8081
HStack {
8182
Text("dog → fog:")
8283
TextDiffView(
@@ -89,12 +90,12 @@ public struct TextDiffView: View {
8990
Divider()
9091
Text("Diff by words")
9192
.bold()
92-
TextDiffView(
93-
original: "Add a diff view! Looks good!",
94-
updated: "Added a diff view. It looks good!",
95-
style: style,
96-
mode: .token
97-
)
93+
TextDiffView(
94+
original: "Add a diff view! Looks good!",
95+
updated: "Added a diff view. It looks good!",
96+
style: style,
97+
mode: .token
98+
)
9899
HStack {
99100
Text("dog → fog:")
100101
TextDiffView(
@@ -127,3 +128,39 @@ public struct TextDiffView: View {
127128
.padding()
128129
.frame(width: 320)
129130
}
131+
132+
#Preview("Height diff") {
133+
let font: NSFont = .systemFont(ofSize: 32, weight: .regular)
134+
let style = TextDiffStyle(
135+
additionsStyle: TextDiffChangeStyle(
136+
fillColor: .systemGreen.withAlphaComponent(0.28),
137+
strokeColor: .systemGreen.withAlphaComponent(0.75),
138+
textColorOverride: .labelColor
139+
),
140+
removalsStyle: TextDiffChangeStyle(
141+
fillColor: .systemRed.withAlphaComponent(0.24),
142+
strokeColor: .systemRed.withAlphaComponent(0.75),
143+
textColorOverride: .secondaryLabelColor,
144+
strikethrough: true
145+
),
146+
textColor: .labelColor,
147+
font: font,
148+
chipCornerRadius: 3,
149+
chipInsets: NSEdgeInsets(top: 0, left: 0, bottom: 0, right: 0),
150+
interChipSpacing: 1,
151+
lineSpacing: 0
152+
)
153+
ZStack(alignment: .topLeading) {
154+
Text("Add ed a diff view. It looks good! Add ed a diff view. It looks good!")
155+
.font(.system(size: 32, weight: .regular, design: nil))
156+
.foregroundStyle(.red.opacity(0.7))
157+
158+
TextDiffView(
159+
original: "Add ed a diff view. It looks good! Add ed a diff view. It looks good.",
160+
updated: "Add ed a diff view. It looks good! Add ed a diff view. It looks good!",
161+
style: style,
162+
mode: .character
163+
)
164+
}
165+
.padding()
166+
}

‎Tests/TextDiffTests/DiffLayouterPerformanceTests.swift‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import AppKit
2+
import XCTest
3+
@testable import TextDiff
4+
5+
// swift test --filter DiffLayouterPerformanceTests 2>&1 | xcsift
6+
7+
final class DiffLayouterPerformanceTests: XCTestCase {
8+
func testLayoutPerformance200Words() {
9+
runLayoutPerformanceTest(wordCount: 200)
10+
}
11+
12+
func testLayoutPerformance500Words() {
13+
runLayoutPerformanceTest(wordCount: 500)
14+
}
15+
16+
func testLayoutPerformance1000Words() {
17+
runLayoutPerformanceTest(wordCount: 1000)
18+
}
19+
20+
private func runLayoutPerformanceTest(wordCount: Int) {
21+
let style = TextDiffStyle.default
22+
let verticalInset = DiffTextLayoutMetrics.verticalTextInset(for: style)
23+
let contentInsets = NSEdgeInsets(top: verticalInset, left: 0, bottom: verticalInset, right: 0)
24+
let availableWidth: CGFloat = 520
25+
26+
let original = Self.largeText(wordCount: wordCount)
27+
let updated = Self.replacingLastWord(in: original)
28+
let segments = TextDiffEngine.diff(original: original, updated: updated, mode: .character)
29+
30+
measure(metrics: [XCTClockMetric()]) {
31+
let layout = DiffTokenLayouter.layout(
32+
segments: segments,
33+
style: style,
34+
availableWidth: availableWidth,
35+
contentInsets: contentInsets
36+
)
37+
XCTAssertFalse(layout.runs.isEmpty)
38+
}
39+
}
40+
41+
private static func largeText(wordCount: Int) -> String {
42+
let vocabulary = [
43+
"alpha", "beta", "gamma", "delta", "epsilon", "theta", "lambda", "sigma",
44+
"swift", "layout", "render", "token", "word", "segment", "measure", "width"
45+
]
46+
var words: [String] = []
47+
words.reserveCapacity(wordCount)
48+
49+
for index in 0..<wordCount {
50+
words.append(vocabulary[index % vocabulary.count])
51+
}
52+
53+
return words.joined(separator: " ")
54+
}
55+
56+
private static func replacingLastWord(in text: String) -> String {
57+
guard let lastSpace = text.lastIndex(of: " ") else {
58+
return "changed"
59+
}
60+
return String(text[..<lastSpace]) + " changed"
61+
}
62+
}

0 commit comments

Comments
 (0)