From 6103fe9afd3d25b9bb20106927a1f07e5a0525e6 Mon Sep 17 00:00:00 2001 From: Gary Tokman Date: Fri, 10 Jul 2026 13:26:02 -0400 Subject: [PATCH 1/3] fix(selection): hit-test line selection against resolved line rects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tap/drag line selection mapped the touch point to a line arithmetically (font.lineHeight + codeLineSpacing per row), while the selection highlight draws at the resolved CoreText line rects. The real per-line advance differs from the assumed one by fractions of a point (~0.5pt for SF Mono 14 + 4pt spacing), which accumulates with the line number — several lines of finger/highlight misalignment a few hundred rows into a diff or code block. Lines containing fallback-font glyphs (emoji, CJK) are taller still and add a full extra jump. Hit-test in the selection overlay's coordinate space against the same rects it draws, so the picked line always matches the rendered highlight. Each line owns the spacing gap below it, mirroring the old rounding. The arithmetic path remains as a fallback for gestures that land before the first layout pass resolves rects. Applies to DiffView.rowIndex(at:) and CodeView.lineIndex(at:) on both UIKit and AppKit. Co-Authored-By: Claude Fable 5 --- .../Components/CodeView/CodeView.swift | 33 ++++++++++++++++++- .../Components/DiffView/DiffView.swift | 32 ++++++++++++++++++ .../LineSelectionOverlayView.swift | 30 +++++++++++++++++ 3 files changed, 94 insertions(+), 1 deletion(-) diff --git a/Sources/MarkdownView/Components/CodeView/CodeView.swift b/Sources/MarkdownView/Components/CodeView/CodeView.swift index 7015297..2ad2c86 100644 --- a/Sources/MarkdownView/Components/CodeView/CodeView.swift +++ b/Sources/MarkdownView/Components/CodeView/CodeView.swift @@ -112,6 +112,23 @@ import Litext } private func lineIndex(at point: CGPoint) -> Int? { + // Hit-test in the selection overlay's space against the resolved + // CoreText line rects it draws, so the picked line always matches + // the rendered highlight. Estimating from a uniform row advance + // (font.lineHeight + spacing) drifts from the real layout as + // blocks grow: the actual per-line advance differs by fractions + // of a point that accumulate over hundreds of lines, and lines + // with fallback-font glyphs are taller still. + if selectionOverlay.hasLineRects { + let overlayPoint = selectionOverlay.convert(point, from: self) + guard let line = selectionOverlay.lineIndex( + atY: overlayPoint.y, + trailingGap: CodeViewConfiguration.codeLineSpacing + ), line <= cachedLineCount else { return nil } + return line + } + + // Fallback before the first layout pass resolves line rects. let localPoint = scrollView.convert(point, from: self) let contentPoint = CGPoint( x: localPoint.x + scrollView.contentOffset.x, @@ -120,7 +137,6 @@ import Litext let font = theme.fonts.code let lineHeight = font.lineHeight let rowAdvance = lineHeight + CodeViewConfiguration.codeLineSpacing - let barHeight = CodeViewConfiguration.barHeight(theme: theme) let adjustedY = contentPoint.y guard adjustedY >= CodeViewConfiguration.codePadding else { return nil } let line = Int((adjustedY - CodeViewConfiguration.codePadding) / rowAdvance) + 1 @@ -406,6 +422,21 @@ import Litext } private func lineIndex(at point: CGPoint) -> Int? { + // Hit-test in the selection overlay's space against the resolved + // CoreText line rects it draws, so the picked line always matches + // the rendered highlight (the arithmetic estimate below drifts + // from the real layout as blocks grow). The overlay is flipped, + // so the converted point is top-down like the rects. + if selectionOverlay.hasLineRects { + let overlayPoint = selectionOverlay.convert(point, from: nil) + guard let line = selectionOverlay.lineIndex( + atY: overlayPoint.y, + trailingGap: CodeViewConfiguration.codeLineSpacing + ), line <= cachedLineCount else { return nil } + return line + } + + // Fallback before the first layout pass resolves line rects. let localPoint = convert(point, from: nil) let font = theme.fonts.code let lineHeight = font.ascender + abs(font.descender) + font.leading diff --git a/Sources/MarkdownView/Components/DiffView/DiffView.swift b/Sources/MarkdownView/Components/DiffView/DiffView.swift index 96216ab..682f5e0 100644 --- a/Sources/MarkdownView/Components/DiffView/DiffView.swift +++ b/Sources/MarkdownView/Components/DiffView/DiffView.swift @@ -886,6 +886,23 @@ private func makeSideBySideAttributedText( } private func rowIndex(at point: CGPoint) -> Int? { + // Hit-test in the selection overlay's space against the resolved + // CoreText line rects it draws, so the picked row always matches + // the rendered highlight. Estimating from a uniform row advance + // (font.lineHeight + spacing) drifts from the real layout as + // blocks grow: the actual per-line advance differs by fractions + // of a point that accumulate over hundreds of rows, and lines + // with fallback-font glyphs are taller still. + if selectionOverlay.hasLineRects { + let overlayPoint = selectionOverlay.convert(point, from: self) + guard let row = selectionOverlay.lineIndex( + atY: overlayPoint.y, + trailingGap: CodeViewConfiguration.codeLineSpacing + ), row <= diffRowCount() else { return nil } + return row + } + + // Fallback before the first layout pass resolves line rects. let localPoint = scrollView.convert(point, from: self) let contentPoint = CGPoint( x: localPoint.x + scrollView.contentOffset.x, @@ -1751,6 +1768,21 @@ private func makeSideBySideAttributedText( } private func rowIndex(at point: CGPoint) -> Int? { + // Hit-test in the selection overlay's space against the resolved + // CoreText line rects it draws, so the picked row always matches + // the rendered highlight (the arithmetic estimate below drifts + // from the real layout as blocks grow). The overlay is flipped, + // so the converted point is top-down like the rects. + if selectionOverlay.hasLineRects { + let overlayPoint = selectionOverlay.convert(point, from: nil) + guard let row = selectionOverlay.lineIndex( + atY: overlayPoint.y, + trailingGap: CodeViewConfiguration.codeLineSpacing + ), row <= diffRowCount() else { return nil } + return row + } + + // Fallback before the first layout pass resolves line rects. let localPoint = convert(point, from: nil) let barHeight = DiffViewConfiguration.barHeight(theme: theme) let font = theme.fonts.code diff --git a/Sources/MarkdownView/Components/LineSelection/LineSelectionOverlayView.swift b/Sources/MarkdownView/Components/LineSelection/LineSelectionOverlayView.swift index d3c02cc..ed63c89 100644 --- a/Sources/MarkdownView/Components/LineSelection/LineSelectionOverlayView.swift +++ b/Sources/MarkdownView/Components/LineSelection/LineSelectionOverlayView.swift @@ -15,6 +15,8 @@ private var lineRects: [CGRect] = [] + var hasLineRects: Bool { !lineRects.isEmpty } + override init(frame: CGRect) { super.init(frame: frame) isOpaque = false @@ -29,6 +31,19 @@ setNeedsDisplay() } + /// Maps a y in this overlay's coordinate space to a 1-based line index + /// using the same rects the highlight draws, so a hit-tested line always + /// matches the rendered selection. Each line owns the spacing gap below + /// it; nil above the first line or below the last (plus one trailing gap). + func lineIndex(atY y: CGFloat, trailingGap: CGFloat) -> Int? { + guard let first = lineRects.first, let last = lineRects.last else { return nil } + guard y >= first.minY, y < last.maxY + trailingGap else { return nil } + for index in 0 ..< (lineRects.count - 1) where y < lineRects[index + 1].minY { + return index + 1 + } + return lineRects.count + } + func clearSelection() { selectedRange = nil } @@ -74,6 +89,8 @@ private var lineRects: [CGRect] = [] + var hasLineRects: Bool { !lineRects.isEmpty } + override init(frame: CGRect) { super.init(frame: frame) wantsLayer = true @@ -90,6 +107,19 @@ needsDisplay = true } + /// Maps a y in this overlay's coordinate space to a 1-based line index + /// using the same rects the highlight draws, so a hit-tested line always + /// matches the rendered selection. Each line owns the spacing gap below + /// it; nil above the first line or below the last (plus one trailing gap). + func lineIndex(atY y: CGFloat, trailingGap: CGFloat) -> Int? { + guard let first = lineRects.first, let last = lineRects.last else { return nil } + guard y >= first.minY, y < last.maxY + trailingGap else { return nil } + for index in 0 ..< (lineRects.count - 1) where y < lineRects[index + 1].minY { + return index + 1 + } + return lineRects.count + } + func clearSelection() { selectedRange = nil } From b60efc9e10867051b29272fd148989c0be02b0b3 Mon Sep 17 00:00:00 2001 From: Gary Tokman Date: Fri, 10 Jul 2026 14:01:09 -0400 Subject: [PATCH 2/3] fix(selection): AppKit hit-test used window coords for a view-local point Review follow-up: the AppKit mouse handlers convert event.locationInWindow into view-local coordinates before calling rowIndex(at:)/lineIndex(at:), so converting again with `from: nil` treated a local point as window coordinates. Convert from `self` instead. The pre-existing arithmetic fallback had the same double conversion; use the local point directly there too. Also adds unit coverage for LineSelectionOverlayView.lineIndex(atY:): inside lines, gap ownership, above-first / trailing-gap bounds, single line, and uneven line heights. Co-Authored-By: Claude Fable 5 --- .../Components/CodeView/CodeView.swift | 7 +- .../Components/DiffView/DiffView.swift | 7 +- .../LineSelectionOverlayViewTests.swift | 98 +++++++++++++++++++ 3 files changed, 106 insertions(+), 6 deletions(-) create mode 100644 Tests/MarkdownViewTests/LineSelectionOverlayViewTests.swift diff --git a/Sources/MarkdownView/Components/CodeView/CodeView.swift b/Sources/MarkdownView/Components/CodeView/CodeView.swift index 2ad2c86..82e727e 100644 --- a/Sources/MarkdownView/Components/CodeView/CodeView.swift +++ b/Sources/MarkdownView/Components/CodeView/CodeView.swift @@ -421,6 +421,8 @@ import Litext lineSelectionEndedHandler?(currentLineSelectionInfo()) } + /// `point` is in this view's coordinate space (the mouse handlers + /// convert from the window before calling). private func lineIndex(at point: CGPoint) -> Int? { // Hit-test in the selection overlay's space against the resolved // CoreText line rects it draws, so the picked line always matches @@ -428,7 +430,7 @@ import Litext // from the real layout as blocks grow). The overlay is flipped, // so the converted point is top-down like the rects. if selectionOverlay.hasLineRects { - let overlayPoint = selectionOverlay.convert(point, from: nil) + let overlayPoint = selectionOverlay.convert(point, from: self) guard let line = selectionOverlay.lineIndex( atY: overlayPoint.y, trailingGap: CodeViewConfiguration.codeLineSpacing @@ -437,12 +439,11 @@ import Litext } // Fallback before the first layout pass resolves line rects. - let localPoint = convert(point, from: nil) let font = theme.fonts.code let lineHeight = font.ascender + abs(font.descender) + font.leading let rowAdvance = lineHeight + CodeViewConfiguration.codeLineSpacing let barHeight = CodeViewConfiguration.barHeight(theme: theme) - let adjustedY = localPoint.y - barHeight + let adjustedY = point.y - barHeight guard adjustedY >= CodeViewConfiguration.codePadding else { return nil } let line = Int((adjustedY - CodeViewConfiguration.codePadding) / rowAdvance) + 1 guard line >= 1, line <= cachedLineCount else { return nil } diff --git a/Sources/MarkdownView/Components/DiffView/DiffView.swift b/Sources/MarkdownView/Components/DiffView/DiffView.swift index 682f5e0..75a57ea 100644 --- a/Sources/MarkdownView/Components/DiffView/DiffView.swift +++ b/Sources/MarkdownView/Components/DiffView/DiffView.swift @@ -1767,6 +1767,8 @@ private func makeSideBySideAttributedText( displayRows.effectiveCount } + /// `point` is in this view's coordinate space (the mouse handlers + /// convert from the window before calling). private func rowIndex(at point: CGPoint) -> Int? { // Hit-test in the selection overlay's space against the resolved // CoreText line rects it draws, so the picked row always matches @@ -1774,7 +1776,7 @@ private func makeSideBySideAttributedText( // from the real layout as blocks grow). The overlay is flipped, // so the converted point is top-down like the rects. if selectionOverlay.hasLineRects { - let overlayPoint = selectionOverlay.convert(point, from: nil) + let overlayPoint = selectionOverlay.convert(point, from: self) guard let row = selectionOverlay.lineIndex( atY: overlayPoint.y, trailingGap: CodeViewConfiguration.codeLineSpacing @@ -1783,12 +1785,11 @@ private func makeSideBySideAttributedText( } // Fallback before the first layout pass resolves line rects. - let localPoint = convert(point, from: nil) let barHeight = DiffViewConfiguration.barHeight(theme: theme) let font = theme.fonts.code let lineHeight = font.ascender + abs(font.descender) + font.leading let rowAdvance = lineHeight + CodeViewConfiguration.codeLineSpacing - let adjustedY = localPoint.y - barHeight - DiffViewConfiguration.contentVerticalPadding(theme: theme) + let adjustedY = point.y - barHeight - DiffViewConfiguration.contentVerticalPadding(theme: theme) guard adjustedY >= 0 else { return nil } let row = Int(adjustedY / rowAdvance) + 1 guard row >= 1, row <= diffRowCount() else { return nil } diff --git a/Tests/MarkdownViewTests/LineSelectionOverlayViewTests.swift b/Tests/MarkdownViewTests/LineSelectionOverlayViewTests.swift new file mode 100644 index 0000000..c3f29f7 --- /dev/null +++ b/Tests/MarkdownViewTests/LineSelectionOverlayViewTests.swift @@ -0,0 +1,98 @@ +import XCTest +@testable import MarkdownView +#if canImport(AppKit) + import AppKit +#elseif canImport(UIKit) + import UIKit +#endif + +@MainActor +final class LineSelectionOverlayViewTests: XCTestCase { + private let lineHeight: CGFloat = 20 + private let gap: CGFloat = 4 + private let firstLineY: CGFloat = 10 + + /// Synthetic resolved line rects: three 20pt lines separated by a 4pt gap, + /// starting at y=10 — line 1 spans 10..<30, line 2 spans 34..<54, line 3 + /// spans 58..<78. + private func makeOverlay(lineCount: Int = 3) -> LineSelectionOverlayView { + let overlay = LineSelectionOverlayView(frame: .zero) + let rects = (0 ..< lineCount).map { index in + CGRect( + x: 0, + y: firstLineY + CGFloat(index) * (lineHeight + gap), + width: 100, + height: lineHeight + ) + } + overlay.updateLineRects(rects) + return overlay + } + + func testHasLineRects() { + let empty = LineSelectionOverlayView(frame: .zero) + XCTAssertFalse(empty.hasLineRects) + XCTAssertTrue(makeOverlay().hasLineRects) + } + + func testEmptyRectsReturnsNil() { + let overlay = LineSelectionOverlayView(frame: .zero) + XCTAssertNil(overlay.lineIndex(atY: 10, trailingGap: gap)) + } + + func testPointInsideLine() { + let overlay = makeOverlay() + XCTAssertEqual(overlay.lineIndex(atY: 10, trailingGap: gap), 1) + XCTAssertEqual(overlay.lineIndex(atY: 29, trailingGap: gap), 1) + XCTAssertEqual(overlay.lineIndex(atY: 44, trailingGap: gap), 2) + XCTAssertEqual(overlay.lineIndex(atY: 70, trailingGap: gap), 3) + } + + func testPointInGapBelongsToLineAbove() { + let overlay = makeOverlay() + // Gap between line 1 (ends 30) and line 2 (starts 34). + XCTAssertEqual(overlay.lineIndex(atY: 30, trailingGap: gap), 1) + XCTAssertEqual(overlay.lineIndex(atY: 33.5, trailingGap: gap), 1) + // Gap between line 2 (ends 54) and line 3 (starts 58). + XCTAssertEqual(overlay.lineIndex(atY: 56, trailingGap: gap), 2) + } + + func testPointAboveFirstLineReturnsNil() { + let overlay = makeOverlay() + XCTAssertNil(overlay.lineIndex(atY: 9.5, trailingGap: gap)) + XCTAssertNil(overlay.lineIndex(atY: -5, trailingGap: gap)) + } + + func testTrailingGapBelongsToLastLine() { + let overlay = makeOverlay() + // Line 3 ends at 78; it owns one trailing gap (< 82). + XCTAssertEqual(overlay.lineIndex(atY: 78, trailingGap: gap), 3) + XCTAssertEqual(overlay.lineIndex(atY: 81.5, trailingGap: gap), 3) + XCTAssertNil(overlay.lineIndex(atY: 82, trailingGap: gap)) + XCTAssertNil(overlay.lineIndex(atY: 200, trailingGap: gap)) + } + + func testSingleLine() { + let overlay = makeOverlay(lineCount: 1) + XCTAssertNil(overlay.lineIndex(atY: 9, trailingGap: gap)) + XCTAssertEqual(overlay.lineIndex(atY: 10, trailingGap: gap), 1) + XCTAssertEqual(overlay.lineIndex(atY: 33, trailingGap: gap), 1) + XCTAssertNil(overlay.lineIndex(atY: 34, trailingGap: gap)) + } + + /// The motivating case for rect-based hit-testing: lines whose real + /// heights vary (fallback-font glyphs, accumulated layout drift) must + /// still resolve to the rect that contains the point. + func testUnevenLineHeights() { + let overlay = LineSelectionOverlayView(frame: .zero) + overlay.updateLineRects([ + CGRect(x: 0, y: 0, width: 100, height: 20), + CGRect(x: 0, y: 24, width: 100, height: 32), // taller (emoji/CJK) + CGRect(x: 0, y: 60, width: 100, height: 20), + ]) + XCTAssertEqual(overlay.lineIndex(atY: 12, trailingGap: gap), 1) + XCTAssertEqual(overlay.lineIndex(atY: 40, trailingGap: gap), 2) + XCTAssertEqual(overlay.lineIndex(atY: 59, trailingGap: gap), 2) + XCTAssertEqual(overlay.lineIndex(atY: 61, trailingGap: gap), 3) + } +} From 409c033d6767466fa7bf21528e4fbb7f1def6eb4 Mon Sep 17 00:00:00 2001 From: Gary Tokman Date: Fri, 10 Jul 2026 14:03:38 -0400 Subject: [PATCH 3/3] fix(selection): keep trailing blank rows selectable past resolved rects Review follow-up (P2): CoreText emits no line for a trailing blank row (text ending in a newline), so logical rows can outnumber resolved rects and the rect-capped hit-test made the visible trailing blank row unselectable. Extrapolate past the last rect at its own advance; the callers' existing logical-line-count guard still rejects points beyond the real content. Co-Authored-By: Claude Fable 5 --- .../LineSelectionOverlayView.swift | 32 +++++++++++++++---- .../LineSelectionOverlayViewTests.swift | 18 +++++++++-- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/Sources/MarkdownView/Components/LineSelection/LineSelectionOverlayView.swift b/Sources/MarkdownView/Components/LineSelection/LineSelectionOverlayView.swift index ed63c89..a49d39d 100644 --- a/Sources/MarkdownView/Components/LineSelection/LineSelectionOverlayView.swift +++ b/Sources/MarkdownView/Components/LineSelection/LineSelectionOverlayView.swift @@ -34,14 +34,24 @@ /// Maps a y in this overlay's coordinate space to a 1-based line index /// using the same rects the highlight draws, so a hit-tested line always /// matches the rendered selection. Each line owns the spacing gap below - /// it; nil above the first line or below the last (plus one trailing gap). + /// it; nil above the first line. Indices past the last rect are + /// extrapolated at the last line's advance — CoreText emits no line for + /// a trailing blank row (text ending in a newline), so logical rows can + /// outnumber resolved rects. Callers cap the result to their logical + /// line count, which turns points beyond the real content into nil. func lineIndex(atY y: CGFloat, trailingGap: CGFloat) -> Int? { guard let first = lineRects.first, let last = lineRects.last else { return nil } - guard y >= first.minY, y < last.maxY + trailingGap else { return nil } + guard y >= first.minY else { return nil } for index in 0 ..< (lineRects.count - 1) where y < lineRects[index + 1].minY { return index + 1 } - return lineRects.count + if y < last.maxY + trailingGap { + return lineRects.count + } + let advance = last.height + trailingGap + guard advance > 0 else { return nil } + let extra = Int((y - (last.maxY + trailingGap)) / advance) + 1 + return lineRects.count + extra } func clearSelection() { @@ -110,14 +120,24 @@ /// Maps a y in this overlay's coordinate space to a 1-based line index /// using the same rects the highlight draws, so a hit-tested line always /// matches the rendered selection. Each line owns the spacing gap below - /// it; nil above the first line or below the last (plus one trailing gap). + /// it; nil above the first line. Indices past the last rect are + /// extrapolated at the last line's advance — CoreText emits no line for + /// a trailing blank row (text ending in a newline), so logical rows can + /// outnumber resolved rects. Callers cap the result to their logical + /// line count, which turns points beyond the real content into nil. func lineIndex(atY y: CGFloat, trailingGap: CGFloat) -> Int? { guard let first = lineRects.first, let last = lineRects.last else { return nil } - guard y >= first.minY, y < last.maxY + trailingGap else { return nil } + guard y >= first.minY else { return nil } for index in 0 ..< (lineRects.count - 1) where y < lineRects[index + 1].minY { return index + 1 } - return lineRects.count + if y < last.maxY + trailingGap { + return lineRects.count + } + let advance = last.height + trailingGap + guard advance > 0 else { return nil } + let extra = Int((y - (last.maxY + trailingGap)) / advance) + 1 + return lineRects.count + extra } func clearSelection() { diff --git a/Tests/MarkdownViewTests/LineSelectionOverlayViewTests.swift b/Tests/MarkdownViewTests/LineSelectionOverlayViewTests.swift index c3f29f7..500e03b 100644 --- a/Tests/MarkdownViewTests/LineSelectionOverlayViewTests.swift +++ b/Tests/MarkdownViewTests/LineSelectionOverlayViewTests.swift @@ -68,8 +68,18 @@ final class LineSelectionOverlayViewTests: XCTestCase { // Line 3 ends at 78; it owns one trailing gap (< 82). XCTAssertEqual(overlay.lineIndex(atY: 78, trailingGap: gap), 3) XCTAssertEqual(overlay.lineIndex(atY: 81.5, trailingGap: gap), 3) - XCTAssertNil(overlay.lineIndex(atY: 82, trailingGap: gap)) - XCTAssertNil(overlay.lineIndex(atY: 200, trailingGap: gap)) + } + + /// CoreText emits no line for a trailing blank row (text ending in a + /// newline), so logical rows can outnumber resolved rects. Points past + /// the last rect extrapolate at the last line's advance; callers cap to + /// their logical line count. + func testExtrapolatesBeyondLastRect() { + let overlay = makeOverlay() + // Line 3's owned region ends at 82; each extrapolated row advances 24. + XCTAssertEqual(overlay.lineIndex(atY: 82, trailingGap: gap), 4) + XCTAssertEqual(overlay.lineIndex(atY: 105.5, trailingGap: gap), 4) + XCTAssertEqual(overlay.lineIndex(atY: 106, trailingGap: gap), 5) } func testSingleLine() { @@ -77,7 +87,9 @@ final class LineSelectionOverlayViewTests: XCTestCase { XCTAssertNil(overlay.lineIndex(atY: 9, trailingGap: gap)) XCTAssertEqual(overlay.lineIndex(atY: 10, trailingGap: gap), 1) XCTAssertEqual(overlay.lineIndex(atY: 33, trailingGap: gap), 1) - XCTAssertNil(overlay.lineIndex(atY: 34, trailingGap: gap)) + // Beyond the owned region: extrapolated to the (possibly virtual) + // next row, for the caller's count guard to accept or reject. + XCTAssertEqual(overlay.lineIndex(atY: 34, trailingGap: gap), 2) } /// The motivating case for rect-based hit-testing: lines whose real