diff --git a/.gitignore b/.gitignore index 0023a53..23038c8 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ DerivedData/ .swiftpm/configuration/registries.json .swiftpm/xcode/package.xcworkspace/contents.xcworkspacedata .netrc +.snapshot-artifacts/ diff --git a/.swiftpm/xcode/xcshareddata/xcbaselines/TextDiffTests.xcbaseline/BF20AB95-BD61-4DE5-BFD2-9B7DB182F4A1.plist b/.swiftpm/xcode/xcshareddata/xcbaselines/TextDiffTests.xcbaseline/BF20AB95-BD61-4DE5-BFD2-9B7DB182F4A1.plist new file mode 100644 index 0000000..f9e14a8 --- /dev/null +++ b/.swiftpm/xcode/xcshareddata/xcbaselines/TextDiffTests.xcbaseline/BF20AB95-BD61-4DE5-BFD2-9B7DB182F4A1.plist @@ -0,0 +1,42 @@ + + + + + classNames + + DiffLayouterPerformanceTests + + testLayoutPerformance1000Words() + + com.apple.dt.XCTMetric_Clock.time.monotonic + + baselineAverage + 0.058013 + baselineIntegrationDisplayName + Local Baseline + + + testLayoutPerformance200Words() + + com.apple.dt.XCTMetric_Clock.time.monotonic + + baselineAverage + 0.013204 + baselineIntegrationDisplayName + Local Baseline + + + testLayoutPerformance500Words() + + com.apple.dt.XCTMetric_Clock.time.monotonic + + baselineAverage + 0.027967 + baselineIntegrationDisplayName + Local Baseline + + + + + + diff --git a/.swiftpm/xcode/xcshareddata/xcbaselines/TextDiffTests.xcbaseline/Info.plist b/.swiftpm/xcode/xcshareddata/xcbaselines/TextDiffTests.xcbaseline/Info.plist new file mode 100644 index 0000000..1a5d3e5 --- /dev/null +++ b/.swiftpm/xcode/xcshareddata/xcbaselines/TextDiffTests.xcbaseline/Info.plist @@ -0,0 +1,33 @@ + + + + + runDestinationsByUUID + + BF20AB95-BD61-4DE5-BFD2-9B7DB182F4A1 + + localComputer + + busSpeedInMHz + 0 + cpuCount + 1 + cpuKind + Apple M1 + cpuSpeedInMHz + 0 + logicalCPUCoresPerPackage + 8 + modelCode + MacBookPro17,1 + physicalCPUCoresPerPackage + 8 + platformIdentifier + com.apple.platform.macosx + + targetArchitecture + arm64 + + + + diff --git a/Sources/TextDiff/AppKit/DiffTextLayoutMetrics.swift b/Sources/TextDiff/AppKit/DiffTextLayoutMetrics.swift index b73b532..fe2ab50 100644 --- a/Sources/TextDiff/AppKit/DiffTextLayoutMetrics.swift +++ b/Sources/TextDiff/AppKit/DiffTextLayoutMetrics.swift @@ -2,11 +2,11 @@ import AppKit enum DiffTextLayoutMetrics { static func verticalTextInset(for style: TextDiffStyle) -> CGFloat { - ceil(max(2, style.chipInsets.top + 2, style.chipInsets.bottom + 2)) + ceil(max(0, style.chipInsets.top, style.chipInsets.bottom)) } static func lineHeight(for style: TextDiffStyle) -> CGFloat { - let textHeight = ceil(style.font.ascender - style.font.descender + style.font.leading) + let textHeight = style.font.ascender - style.font.descender + style.font.leading let chipHeight = textHeight + style.chipInsets.top + style.chipInsets.bottom return ceil(chipHeight + max(0, style.lineSpacing)) } diff --git a/Sources/TextDiff/AppKit/DiffTokenLayouter.swift b/Sources/TextDiff/AppKit/DiffTokenLayouter.swift index a7f2676..75ed788 100644 --- a/Sources/TextDiff/AppKit/DiffTokenLayouter.swift +++ b/Sources/TextDiff/AppKit/DiffTokenLayouter.swift @@ -27,6 +27,7 @@ enum DiffTokenLayouter { contentInsets: NSEdgeInsets ) -> DiffLayout { let lineHeight = DiffTextLayoutMetrics.lineHeight(for: style) + let textHeight = ceil(style.font.ascender - style.font.descender + style.font.leading) let maxLineWidth = availableWidth > 0 ? availableWidth : .greatestFiniteMagnitude let lineStartX = contentInsets.left let maxLineX = lineStartX + maxLineWidth @@ -37,12 +38,16 @@ enum DiffTokenLayouter { var maxUsedX = lineStartX var lineCount = 1 var lineHasContent = false + let lineText = NSMutableString() + var lineTextWidth: CGFloat = 0 var previousChangedLexical = false func moveToNewLine() { lineTop += lineHeight cursorX = lineStartX lineHasContent = false + lineText.setString("") + lineTextWidth = 0 previousChangedLexical = false lineCount += 1 } @@ -65,9 +70,15 @@ enum DiffTokenLayouter { } let attributedText = attributedToken(for: segment, style: style) - let textSize = measuredTextSize(for: piece.text, font: style.font) + var textMeasurement = measuredIncrementalTextWidth( + for: piece.text, + font: style.font, + lineText: lineText, + lineTextWidth: lineTextWidth + ) + var textSize = CGSize(width: textMeasurement.textWidth, height: textHeight) let chipInsets = effectiveChipInsets(for: style) - let runWidth = isChangedLexical ? textSize.width + chipInsets.left + chipInsets.right : textSize.width + var runWidth = isChangedLexical ? textSize.width + chipInsets.left + chipInsets.right : textSize.width let requiredWidth = leadingGap + runWidth let wrapped = lineHasContent && cursorX + requiredWidth > maxLineX @@ -79,6 +90,15 @@ enum DiffTokenLayouter { if piece.tokenKind == .whitespace { continue } + + textMeasurement = measuredIncrementalTextWidth( + for: piece.text, + font: style.font, + lineText: lineText, + lineTextWidth: lineTextWidth + ) + textSize = CGSize(width: textMeasurement.textWidth, height: textHeight) + runWidth = isChangedLexical ? textSize.width + chipInsets.left + chipInsets.right : textSize.width } cursorX += leadingGap @@ -118,6 +138,7 @@ enum DiffTokenLayouter { cursorX += runWidth maxUsedX = max(maxUsedX, cursorX) lineHasContent = true + lineTextWidth = textMeasurement.combinedLineWidth previousChangedLexical = isChangedLexical } @@ -146,9 +167,28 @@ enum DiffTokenLayouter { return NSAttributedString(string: segment.text, attributes: attributes) } - private static func measuredTextSize(for text: String, font: NSFont) -> CGSize { - let measured = (text as NSString).size(withAttributes: [.font: font]) - return CGSize(width: ceil(measured.width), height: ceil(measured.height)) + private static func measuredIncrementalTextWidth( + for text: String, + font: NSFont, + lineText: NSMutableString, + lineTextWidth: CGFloat + ) -> IncrementalTextWidth { + guard !text.isEmpty else { + return IncrementalTextWidth( + textWidth: 0, + combinedLineWidth: lineTextWidth + ) + } + + lineText.append(text) + // TODO: Fix this later + // 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. + let combinedWidth = lineText.size(withAttributes: [.font: font]).width + let textWidth = max(0, combinedWidth - lineTextWidth) + return IncrementalTextWidth( + textWidth: textWidth, + combinedLineWidth: combinedWidth + ) } private static func effectiveChipInsets(for style: TextDiffStyle) -> NSEdgeInsets { @@ -262,3 +302,8 @@ private struct LayoutPiece { let text: String let isLineBreak: Bool } + +private struct IncrementalTextWidth { + let textWidth: CGFloat + let combinedLineWidth: CGFloat +} diff --git a/Sources/TextDiff/TextDiffView.swift b/Sources/TextDiff/TextDiffView.swift index ae188ae..c1cabb7 100644 --- a/Sources/TextDiff/TextDiffView.swift +++ b/Sources/TextDiff/TextDiffView.swift @@ -35,7 +35,7 @@ public struct TextDiffView: View { style: style, mode: mode ) - .accessibilityLabel("Text diff") + .accessibilityLabel("Text diff") } } @@ -49,6 +49,7 @@ public struct TextDiffView: View { } #Preview("TextDiffView") { + let font: NSFont = .systemFont(ofSize: 16, weight: .regular) let style = TextDiffStyle( additionsStyle: TextDiffChangeStyle( fillColor: .systemGreen.withAlphaComponent(0.28), @@ -62,7 +63,7 @@ public struct TextDiffView: View { strikethrough: true ), textColor: .labelColor, - font: .systemFont(ofSize: 16, weight: .regular), + font: font, chipCornerRadius: 3, chipInsets: NSEdgeInsets(top: 0, left: 0, bottom: 0, right: 0), interChipSpacing: 1, @@ -72,11 +73,11 @@ public struct TextDiffView: View { Text("Diff by characters") .bold() TextDiffView( - original: "Add a diff view! Looks good!", - updated: "Added a diff view. It looks good!", - style: style, - mode: .character - ) + original: "Add a diff view! Looks good!", + updated: "Added a diff view. It looks good!", + style: style, + mode: .character + ) HStack { Text("dog → fog:") TextDiffView( @@ -89,12 +90,12 @@ public struct TextDiffView: View { Divider() Text("Diff by words") .bold() - TextDiffView( - original: "Add a diff view! Looks good!", - updated: "Added a diff view. It looks good!", - style: style, - mode: .token - ) + TextDiffView( + original: "Add a diff view! Looks good!", + updated: "Added a diff view. It looks good!", + style: style, + mode: .token + ) HStack { Text("dog → fog:") TextDiffView( @@ -127,3 +128,39 @@ public struct TextDiffView: View { .padding() .frame(width: 320) } + +#Preview("Height diff") { + let font: NSFont = .systemFont(ofSize: 32, weight: .regular) + let style = TextDiffStyle( + additionsStyle: TextDiffChangeStyle( + fillColor: .systemGreen.withAlphaComponent(0.28), + strokeColor: .systemGreen.withAlphaComponent(0.75), + textColorOverride: .labelColor + ), + removalsStyle: TextDiffChangeStyle( + fillColor: .systemRed.withAlphaComponent(0.24), + strokeColor: .systemRed.withAlphaComponent(0.75), + textColorOverride: .secondaryLabelColor, + strikethrough: true + ), + textColor: .labelColor, + font: font, + chipCornerRadius: 3, + chipInsets: NSEdgeInsets(top: 0, left: 0, bottom: 0, right: 0), + interChipSpacing: 1, + lineSpacing: 0 + ) + ZStack(alignment: .topLeading) { + Text("Add ed a diff view. It looks good! Add ed a diff view. It looks good!") + .font(.system(size: 32, weight: .regular, design: nil)) + .foregroundStyle(.red.opacity(0.7)) + + TextDiffView( + original: "Add ed a diff view. It looks good! Add ed a diff view. It looks good.", + updated: "Add ed a diff view. It looks good! Add ed a diff view. It looks good!", + style: style, + mode: .character + ) + } + .padding() +} diff --git a/Tests/TextDiffTests/DiffLayouterPerformanceTests.swift b/Tests/TextDiffTests/DiffLayouterPerformanceTests.swift new file mode 100644 index 0000000..a8598de --- /dev/null +++ b/Tests/TextDiffTests/DiffLayouterPerformanceTests.swift @@ -0,0 +1,62 @@ +import AppKit +import XCTest +@testable import TextDiff + +// swift test --filter DiffLayouterPerformanceTests 2>&1 | xcsift + +final class DiffLayouterPerformanceTests: XCTestCase { + func testLayoutPerformance200Words() { + runLayoutPerformanceTest(wordCount: 200) + } + + func testLayoutPerformance500Words() { + runLayoutPerformanceTest(wordCount: 500) + } + + func testLayoutPerformance1000Words() { + runLayoutPerformanceTest(wordCount: 1000) + } + + private func runLayoutPerformanceTest(wordCount: Int) { + let style = TextDiffStyle.default + let verticalInset = DiffTextLayoutMetrics.verticalTextInset(for: style) + let contentInsets = NSEdgeInsets(top: verticalInset, left: 0, bottom: verticalInset, right: 0) + let availableWidth: CGFloat = 520 + + let original = Self.largeText(wordCount: wordCount) + let updated = Self.replacingLastWord(in: original) + let segments = TextDiffEngine.diff(original: original, updated: updated, mode: .character) + + measure(metrics: [XCTClockMetric()]) { + let layout = DiffTokenLayouter.layout( + segments: segments, + style: style, + availableWidth: availableWidth, + contentInsets: contentInsets + ) + XCTAssertFalse(layout.runs.isEmpty) + } + } + + private static func largeText(wordCount: Int) -> String { + let vocabulary = [ + "alpha", "beta", "gamma", "delta", "epsilon", "theta", "lambda", "sigma", + "swift", "layout", "render", "token", "word", "segment", "measure", "width" + ] + var words: [String] = [] + words.reserveCapacity(wordCount) + + for index in 0.. String { + guard let lastSpace = text.lastIndex(of: " ") else { + return "changed" + } + return String(text[..= 0) + + var largerTop = base + largerTop.chipInsets = NSEdgeInsets(top: 6, left: 2, bottom: 0, right: 2) + let largerTopInset = DiffTextLayoutMetrics.verticalTextInset(for: largerTop) + #expect(largerTopInset >= baseInset) + + var largerBottom = base + largerBottom.chipInsets = NSEdgeInsets(top: 0, left: 2, bottom: 7, right: 2) + let largerBottomInset = DiffTextLayoutMetrics.verticalTextInset(for: largerBottom) + #expect(largerBottomInset >= baseInset) } @Test diff --git a/Tests/TextDiffTests/TextDiffSnapshotTests.swift b/Tests/TextDiffTests/TextDiffSnapshotTests.swift index dbdbc54..5f5a7de 100644 --- a/Tests/TextDiffTests/TextDiffSnapshotTests.swift +++ b/Tests/TextDiffTests/TextDiffSnapshotTests.swift @@ -1,73 +1,83 @@ import AppKit import SnapshotTesting -import Testing import TextDiff +import XCTest -@Suite(.snapshots(record: .missing)) -@MainActor -struct TextDiffSnapshotTests { - @Test - func token_basic_replacement() { +final class TextDiffSnapshotTests: XCTestCase { + override func invokeTest() { + withSnapshotTesting(record: .missing) { + super.invokeTest() + } + } + + @MainActor + func testTokenBasicReplacement() { assertTextDiffSnapshot( original: "Apply old value in this sentence.", updated: "Apply new value in this sentence.", mode: .token, - size: CGSize(width: 500, height: 120) + size: CGSize(width: 500, height: 120), + testName: "token_basic_replacement()" ) } - @Test - func character_suffix_refinement() { + @MainActor + func testCharacterSuffixRefinement() { assertTextDiffSnapshot( original: "Add a diff", updated: "Added a diff", mode: .character, - size: CGSize(width: 320, height: 110) + size: CGSize(width: 320, height: 110), + testName: "character_suffix_refinement()" ) } - @Test - func punctuation_replacement() { + @MainActor + func testPunctuationReplacement() { assertTextDiffSnapshot( original: "Wait!", updated: "Wait.", mode: .token, - size: CGSize(width: 320, height: 100) + size: CGSize(width: 320, height: 100), + testName: "punctuation_replacement()" ) } - @Test - func whitespace_only_layout_change() { + @MainActor + func testWhitespaceOnlyLayoutChange() { assertTextDiffSnapshot( original: "Hello world", updated: "Hello world\n", mode: .token, - size: CGSize(width: 340, height: 110) + size: CGSize(width: 340, height: 110), + testName: "whitespace_only_layout_change()" ) } - @Test - func multiline_insertion_wrap() { + @MainActor + func testMultilineInsertionWrap() { assertTextDiffSnapshot( original: "line1\nline2", updated: "line1\nlineX\nline2", mode: .token, - size: CGSize(width: 300, height: 150) + size: CGSize(width: 300, height: 150), + testName: "multiline_insertion_wrap()" ) } - @Test - func narrow_width_wrapping() { + @MainActor + func testNarrowWidthWrapping() { assertTextDiffSnapshot( original: sampleOriginalSentence, updated: sampleUpdatedSentence, mode: .token, - size: CGSize(width: 220, height: 180) + size: CGSize(width: 220, height: 180), + testName: "narrow_width_wrapping()" ) } - @Test - func custom_style_spacing_strikethrough() { + @MainActor + func testCustomStyleSpacingStrikethrough() { var style = TextDiffStyle.default style.removalsStyle.strikethrough = true style.interChipSpacing = 1 @@ -77,7 +87,8 @@ struct TextDiffSnapshotTests { updated: sampleUpdatedSentence, mode: .character, style: style, - size: CGSize(width: 300, height: 180) + size: CGSize(width: 300, height: 180), + testName: "custom_style_spacing_strikethrough()" ) } diff --git a/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/character_suffix_refinement.1.png b/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/character_suffix_refinement.1.png index 2589385..4031876 100644 Binary files a/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/character_suffix_refinement.1.png and b/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/character_suffix_refinement.1.png differ diff --git a/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/custom_style_spacing_strikethrough.1.png b/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/custom_style_spacing_strikethrough.1.png index dc0b727..dd5178d 100644 Binary files a/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/custom_style_spacing_strikethrough.1.png and b/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/custom_style_spacing_strikethrough.1.png differ diff --git a/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/multiline_insertion_wrap.1.png b/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/multiline_insertion_wrap.1.png index 3006387..fd10089 100644 Binary files a/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/multiline_insertion_wrap.1.png and b/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/multiline_insertion_wrap.1.png differ diff --git a/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/punctuation_replacement.1.png b/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/punctuation_replacement.1.png index 8b47eda..90d6985 100644 Binary files a/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/punctuation_replacement.1.png and b/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/punctuation_replacement.1.png differ diff --git a/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/token_basic_replacement.1.png b/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/token_basic_replacement.1.png index 86ca2b5..2cdec60 100644 Binary files a/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/token_basic_replacement.1.png and b/Tests/TextDiffTests/__Snapshots__/NSTextDiffSnapshotTests/token_basic_replacement.1.png differ diff --git a/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/character_suffix_refinement.1.png b/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/character_suffix_refinement.1.png index 2589385..4031876 100644 Binary files a/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/character_suffix_refinement.1.png and b/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/character_suffix_refinement.1.png differ diff --git a/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/custom_style_spacing_strikethrough.1.png b/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/custom_style_spacing_strikethrough.1.png index dc0b727..dd5178d 100644 Binary files a/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/custom_style_spacing_strikethrough.1.png and b/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/custom_style_spacing_strikethrough.1.png differ diff --git a/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/multiline_insertion_wrap.1.png b/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/multiline_insertion_wrap.1.png index 3006387..fd10089 100644 Binary files a/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/multiline_insertion_wrap.1.png and b/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/multiline_insertion_wrap.1.png differ diff --git a/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/narrow_width_wrapping.1.png b/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/narrow_width_wrapping.1.png index 9c8a367..626e00d 100644 Binary files a/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/narrow_width_wrapping.1.png and b/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/narrow_width_wrapping.1.png differ diff --git a/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/punctuation_replacement.1.png b/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/punctuation_replacement.1.png index 8b47eda..90d6985 100644 Binary files a/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/punctuation_replacement.1.png and b/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/punctuation_replacement.1.png differ diff --git a/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/token_basic_replacement.1.png b/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/token_basic_replacement.1.png index 86ca2b5..2cdec60 100644 Binary files a/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/token_basic_replacement.1.png and b/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/token_basic_replacement.1.png differ diff --git a/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/whitespace_only_layout_change.1.png b/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/whitespace_only_layout_change.1.png index abcdb77..1098d6f 100644 Binary files a/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/whitespace_only_layout_change.1.png and b/Tests/TextDiffTests/__Snapshots__/TextDiffSnapshotTests/whitespace_only_layout_change.1.png differ