diff --git a/claude.md b/claude.md index 562e6ffe..7a375eec 100644 --- a/claude.md +++ b/claude.md @@ -190,6 +190,12 @@ apart. enrichment on top. Copying is `IViewerWindow.SetClipboard` rather than a `ViewerActions` member, because a clipboard belongs to a toolkit the way a window does, and it is answered before the owner link: the text is already in this process. +- Columns are cells of `CellGrid`, decided in the model rather than by any head's fonts: a wide + character (CJK, fullwidth, emoji) takes two, a combining mark or joiner none. Every head draws a + row as `CellGrid.Segments`, each at its column, rather than as one string - a row of plain text + is one segment at column 0 - so a character a fallback font draws at its own width moves nothing + after it, and the highlight, the hit test and the copy count the same cells. Selection ends snap + to cluster boundaries (`CellGrid.Snap`), so a wide character is taken whole or not at all. - An entry opens at its first change, not line 1: every path that changes what is being read goes through `ViewerSession.Open`, so none resets to row 0 on its own. The minimal view ("Changes only", `SessionState.Minimal`) is a second `DiffView` built with each entry - changes plus diff --git a/native/include/deview.h b/native/include/deview.h index 884625c4..0965cbcf 100644 --- a/native/include/deview.h +++ b/native/include/deview.h @@ -51,17 +51,38 @@ enum DeviewQueueFlags { DEVIEW_QUEUE_HEADER = 1 << 2 }; +/* + * A run of a row's text and the cell column it starts at. A renderer draws a row as its segments, + * each at its column times the cell width, rather than as one string: a character the font draws + * wider or narrower than a cell - CJK in a fallback font, a combining mark - then moves nothing + * after it, and a column means the same thing to the highlight, the hit test and the copy. A row + * of plain text is one segment at column 0. + * + * textOffset and textLength are into DeviewScreen.strings, like every other text reference, and + * always inside the row's own text. + */ +typedef struct DeviewSegment { + int32_t textOffset; + int32_t textLength; + int32_t column; +} DeviewSegment; + typedef struct DeviewRow { int32_t kind; /* -1 when the row is filler or folded and has no line number. */ int32_t lineNumber; + /* Flattened: a tab is already four spaces, so every character is drawn where it is counted. */ int32_t textOffset; int32_t textLength; + /* segmentCount entries of DeviewScreen.segments from segmentOffset: how to draw the text. */ + int32_t segmentOffset; + int32_t segmentCount; + /* - * What of this row the reader has selected, in characters of the text above rather than in - * pixels: the managed side flattens tabs before it counts, so a column here multiplied by the - * cell width is where the highlight goes. + * What of this row the reader has selected, in cells of the grid the segments are drawn on + * rather than in pixels, so a column here multiplied by the cell width is where the highlight + * goes. * * selectLength is 0 on a row with nothing selected, which is every row of almost every frame. * The managed side has already resolved which side the drag is in and clipped the range to the @@ -139,6 +160,9 @@ typedef struct DeviewScreen { const DeviewRow* rows; int32_t rowCount; + const DeviewSegment* segments; + int32_t segmentCount; + const DeviewButton* buttons; int32_t buttonCount; @@ -272,8 +296,12 @@ typedef struct DeviewInput { * which between them are text selection. DeviewRow is a widened array element, so this is the * same kind of bump 6 was. deview_set_clipboard is added beside them, because the selection is * only worth having if it can be copied and each toolkit owns its own clipboard. + * 9: DeviewRow carries segments, drawn each at its cell column, and its text arrives flattened. + * Drawing a row as one string let each renderer's fonts decide where a wide or combining + * character went, while a selection counted cells, so the two disagreed past the first one. + * DeviewRow is widened and DeviewScreen gains an array, the same kind of bump 6 and 8 were. */ -#define DEVIEW_VERSION 8 +#define DEVIEW_VERSION 9 /* * The Swift implementation imports this header for the struct layouts, because Swift does not diff --git a/native/src/deview.cpp b/native/src/deview.cpp index f616f38e..bdd22bce 100644 --- a/native/src/deview.cpp +++ b/native/src/deview.cpp @@ -685,6 +685,47 @@ int GutterDigits(const DeviewScreen* screen) return digits; } +/* + * A row's text, each segment at its cell column: see DeviewSegment. A row that is one segment is + * drawn as the whole row always was, through the text item that also lays the row out; that is a + * row of plain text, which is nearly all of them. Any other row puts its segments on the window's + * draw list at their columns, clipped to the table cell like the item would be, and keeps its + * place in the layout with an item as tall as a line. + */ +void RowText(const DeviewScreen* screen, const DeviewRow& row, ImVec2 textPos) +{ + if (row.segmentCount <= 1 || + screen->segments == nullptr || + row.segmentOffset < 0 || + row.segmentOffset + row.segmentCount > screen->segmentCount) + { + Text(screen, row.textOffset, row.textLength); + return; + } + + const float cell = ImGui::CalcTextSize("M").x; + ImDrawList* list = ImGui::GetWindowDrawList(); + const ImU32 colour = ImGui::GetColorU32(ImGuiCol_Text); + for (int index = 0; index < row.segmentCount; index++) + { + const DeviewSegment& segment = screen->segments[row.segmentOffset + index]; + const char* begin; + const char* end; + if (!Slice(screen, segment.textOffset, segment.textLength, &begin, &end)) + { + continue; + } + + list->AddText( + ImVec2(textPos.x + static_cast(segment.column) * cell, textPos.y), + colour, + begin, + end); + } + + ImGui::Dummy(ImVec2(0.0f, ImGui::GetTextLineHeight())); +} + void DrawRow(const DeviewScreen* screen, const DeviewPane& pane, int index, int column, int digits, PaneHit& hit) { /* Before the row count check, so a pane shorter than the body still reports where its rows @@ -753,7 +794,7 @@ void DrawRow(const DeviewScreen* screen, const DeviewPane& pane, int index, int } ImGui::PushStyleColor(ImGuiCol_Text, RowColour(row.kind)); - Text(screen, row.textOffset, row.textLength); + RowText(screen, row, textPos); ImGui::PopStyleColor(); } diff --git a/native/swift/Sources/Deview/Frame.swift b/native/swift/Sources/Deview/Frame.swift index d674f238..52da960e 100644 --- a/native/swift/Sources/Deview/Frame.swift +++ b/native/swift/Sources/Deview/Frame.swift @@ -31,7 +31,12 @@ struct Frame: Equatable { var lineNumber: Int32 = -1 var text = "" - /// What of `text` the reader has selected, in characters. Length 0 on a row with nothing + /// How to draw `text`: each segment at its cell column, so a character a font draws + /// wider or narrower than a cell moves nothing after it. One segment at column 0 for a + /// row of plain text. + var segments: [Segment] = [] + + /// What of `text` the reader has selected, in cells. Length 0 on a row with nothing /// selected, which is every row of almost every frame. The managed side has already /// resolved which side the drag is in and clipped the range to the visible slice, so this /// is a rectangle to fill rather than a range to work out. @@ -39,6 +44,11 @@ struct Frame: Equatable { var selectLength: Int32 = 0 } + struct Segment: Equatable { + var text = "" + var column: Int32 = 0 + } + struct Pane: Equatable { var header = "" var rows: [Row] = [] @@ -137,11 +147,28 @@ struct Frame: Equatable { } let row = rows[offset] + var segments: [Segment] = [] + if let source = screen.segments { + for index in 0 ..< Int(max(0, row.segmentCount)) { + let at = Int(row.segmentOffset) + index + guard at >= 0, at < Int(screen.segmentCount) else { + continue + } + + let segment = source[at] + segments.append( + Segment( + text: string(screen, segment.textOffset, segment.textLength), + column: segment.column)) + } + } + pane.rows.append( Row( kind: row.kind, lineNumber: row.lineNumber, text: string(screen, row.textOffset, row.textLength), + segments: segments, selectStart: row.selectStart, selectLength: row.selectLength)) } diff --git a/native/swift/Sources/Deview/Renderer.swift b/native/swift/Sources/Deview/Renderer.swift index 42bcf8a7..8951d932 100644 --- a/native/swift/Sources/Deview/Renderer.swift +++ b/native/swift/Sources/Deview/Renderer.swift @@ -352,11 +352,23 @@ final class Renderer { } text(gutter, in: CGRect(x: bounds.minX, y: bounds.minY, width: width, height: bounds.height), Palette.dim, context) - text( - row.text, - in: CGRect(x: bounds.minX + width, y: bounds.minY, width: bounds.width - width, height: bounds.height), - Palette.foreground(row.kind), - context) + + // Each segment at its column rather than the row as one line, so a character Core Text + // takes from a fallback font, at that font's width, moves nothing after it. A row of plain + // text is one segment at column 0, drawn exactly as the whole row was. + let segments = row.segments.isEmpty ? [Frame.Segment(text: row.text, column: 0)] : row.segments + for segment in segments { + let left = bounds.minX + width + CGFloat(segment.column) * cell.width + guard left < bounds.maxX else { + break + } + + text( + segment.text, + in: CGRect(x: left, y: bounds.minY, width: bounds.maxX - left, height: bounds.height), + Palette.foreground(row.kind), + context) + } } /// The picture a pane is, one blank line under its rows — the same placement the other two diff --git a/src/DiffEngineTray.Tests/DebugReportTests.Full.verified.txt b/src/DiffEngineTray.Tests/DebugReportTests.Full.verified.txt index 359a791a..a44aa977 100644 --- a/src/DiffEngineTray.Tests/DebugReportTests.Full.verified.txt +++ b/src/DiffEngineTray.Tests/DebugReportTests.Full.verified.txt @@ -6,7 +6,7 @@ Tracking: True Deletes (1) ----------- [1] Extra.verified.txt - File: {Directory}\Extra.verified.txt (missing) + File: {Directory}\Extra.verified.txt (exists) Group: Moves (1) diff --git a/src/DiffEngineTray.Tests/DebugReportTests.cs b/src/DiffEngineTray.Tests/DebugReportTests.cs index 8e6c3e58..d3c852e8 100644 --- a/src/DiffEngineTray.Tests/DebugReportTests.cs +++ b/src/DiffEngineTray.Tests/DebugReportTests.cs @@ -49,7 +49,11 @@ public async Task Full() // view is where the whole message is readable rather than the menu's "!" viewer.Queue.Add(new(@"c:\repo\failed.cs|7", "Failed.cs:7", "the file is locked")); await using var tracker = new RecordingTracker(); - tracker.AddDelete(Path.Combine(directory, "Extra.verified.txt")); + // A file that exists, as a pending delete's is. The tracker's scan drops a delete whose file + // has gone, every two seconds, and on a slow runner that scan landed mid test + var extra = Path.Combine(directory, "Extra.verified.txt"); + File.WriteAllText(extra, ""); + tracker.AddDelete(extra); tracker.AddMove( received, verified, diff --git a/src/DiffEngineViewer.Linux/runtimes/linux-arm64/native/libdiffengine_viewer.so b/src/DiffEngineViewer.Linux/runtimes/linux-arm64/native/libdiffengine_viewer.so index a58e9e34..c89e7e3f 100644 Binary files a/src/DiffEngineViewer.Linux/runtimes/linux-arm64/native/libdiffengine_viewer.so and b/src/DiffEngineViewer.Linux/runtimes/linux-arm64/native/libdiffengine_viewer.so differ diff --git a/src/DiffEngineViewer.Linux/runtimes/linux-x64/native/libdiffengine_viewer.so b/src/DiffEngineViewer.Linux/runtimes/linux-x64/native/libdiffengine_viewer.so index 4d99b4fe..8f9709ff 100644 Binary files a/src/DiffEngineViewer.Linux/runtimes/linux-x64/native/libdiffengine_viewer.so and b/src/DiffEngineViewer.Linux/runtimes/linux-x64/native/libdiffengine_viewer.so differ diff --git a/src/DiffEngineViewer.Mac/runtimes/osx-arm64/native/libdiffengine_viewer.dylib b/src/DiffEngineViewer.Mac/runtimes/osx-arm64/native/libdiffengine_viewer.dylib index 05c76421..09697e70 100644 Binary files a/src/DiffEngineViewer.Mac/runtimes/osx-arm64/native/libdiffengine_viewer.dylib and b/src/DiffEngineViewer.Mac/runtimes/osx-arm64/native/libdiffengine_viewer.dylib differ diff --git a/src/DiffEngineViewer.Mac/runtimes/osx-x64/native/libdiffengine_viewer.dylib b/src/DiffEngineViewer.Mac/runtimes/osx-x64/native/libdiffengine_viewer.dylib index 05c76421..09697e70 100644 Binary files a/src/DiffEngineViewer.Mac/runtimes/osx-x64/native/libdiffengine_viewer.dylib and b/src/DiffEngineViewer.Mac/runtimes/osx-x64/native/libdiffengine_viewer.dylib differ diff --git a/src/DiffEngineViewer.Tests/CellGridTests.cs b/src/DiffEngineViewer.Tests/CellGridTests.cs new file mode 100644 index 00000000..e38493e0 --- /dev/null +++ b/src/DiffEngineViewer.Tests/CellGridTests.cs @@ -0,0 +1,86 @@ +/// +/// The character grid every head draws a row on and every selection counts in. What a character +/// takes is decided here and not by any head's fonts, which is what keeps the highlight, the hit +/// test and the copy on one run of text. +/// +public class CellGridTests +{ + [Test] + [Arguments("", 0)] + [Arguments("plain text", 10)] + // Latin with its extensions, Greek and Cyrillic: one cell each, like ASCII + [Arguments("Ωμέγα Привет żółw", 17)] + // Wide: two cells each + [Arguments("中文", 4)] + [Arguments("ｆｕｌｌ", 8)] + [Arguments("한국어", 6)] + // A combining mark takes no cell of its own + [Arguments("é", 1)] + [Arguments("é̂x", 2)] + // Outside the basic plane: a surrogate pair is one character + [Arguments("\U0001D400", 1)] + // Emoji are wide, and a joined sequence is one picture + [Arguments("\U0001F600", 2)] + [Arguments("\U0001F468‍\U0001F469‍\U0001F467", 2)] + // A mark with nothing before it still takes a cell, or it could not be selected + [Arguments("́", 1)] + public async Task Cells(string text, int cells) => + await Assert.That(CellGrid.Cells(text)).IsEqualTo(cells); + + [Test] + public async Task PlainTextIsOneSegmentAtColumnZero() => + await Assert.That(CellGrid.Segments("the quick brown fox")).IsEquivalentTo([new CellGrid.Segment(0, 19, 0)]); + + [Test] + public async Task EmptyTextHasNoSegments() => + await Assert.That(CellGrid.Segments("")).IsEmpty(); + + /// + /// Text the monospace font has is drawn in runs; anything else on its own at its column, so the + /// characters after it land where the grid says whatever width it is drawn at. + /// + [Test] + public async Task EveryOtherClusterIsASegmentOfItsOwn() => + await Assert.That(CellGrid.Segments("ab中ćd")) + .IsEquivalentTo( + [ + new CellGrid.Segment(0, 2, 0), + new CellGrid.Segment(2, 1, 2), + new CellGrid.Segment(3, 2, 4), + new CellGrid.Segment(5, 1, 5) + ]); + + [Test] + public async Task CyrillicIsARunLikeAscii() => + await Assert.That(CellGrid.Segments("Привет, мир")).IsEquivalentTo([new CellGrid.Segment(0, 11, 0)]); + + /// + /// A column inside a wide character moves to its end, so a selection takes it whole or not at + /// all, and the index it maps to is after the whole character. + /// + [Test] + [Arguments(0, 0, 0)] + [Arguments(1, 1, 1)] + [Arguments(2, 3, 2)] + [Arguments(3, 3, 2)] + [Arguments(4, 4, 3)] + [Arguments(9, 4, 3)] + public async Task SnapsToWholeCharacters(int cell, int snapped, int index) + { + // a is cell 0, 中 cells 1 and 2, b cell 3 + const string text = "a中b"; + + await Assert.That(CellGrid.Snap(text, cell)).IsEqualTo(snapped); + await Assert.That(CellGrid.Index(text, cell)).IsEqualTo(index); + } + + [Test] + public async Task NeverSplitsACharacterFromItsMarks() + { + // e and its two marks are cell 0, x cell 1 + const string text = "é̂x"; + + await Assert.That(CellGrid.Index(text, 1)).IsEqualTo(3); + await Assert.That(CellGrid.Index(text, 2)).IsEqualTo(4); + } +} diff --git a/src/DiffEngineViewer.Tests/DeviewStructTests.cs b/src/DiffEngineViewer.Tests/DeviewStructTests.cs index aaf0172e..37ea1512 100644 --- a/src/DiffEngineViewer.Tests/DeviewStructTests.cs +++ b/src/DiffEngineViewer.Tests/DeviewStructTests.cs @@ -58,6 +58,7 @@ public async Task KeysMatchTheHeader() => public static IEnumerable<(string, Type)> Structs() { + yield return ("DeviewSegment", typeof(DeviewSegment)); yield return ("DeviewRow", typeof(DeviewRow)); yield return ("DeviewPane", typeof(DeviewPane)); yield return ("DeviewButton", typeof(DeviewButton)); diff --git a/src/DiffEngineViewer.Tests/SelectionTests.cs b/src/DiffEngineViewer.Tests/SelectionTests.cs index ee16566b..a48d6ee7 100644 --- a/src/DiffEngineViewer.Tests/SelectionTests.cs +++ b/src/DiffEngineViewer.Tests/SelectionTests.cs @@ -172,6 +172,36 @@ public async Task CopyPutsTheSelectionOnTheClipboard() await Assert.That(copied.Message).IsEqualTo("Copied 3 lines from the selection."); } + /// + /// A wide character is two cells, and a drag that starts or ends inside one takes it whole or + /// not at all: the highlight and the copy both end at the same character boundary. + /// + [Test] + public async Task ADragInsideWideCharactersTakesThemWhole() + { + var window = new Recorder(); + // 中 is cells 0 and 1, 文 cells 2 and 3, a cell 4. From inside 中 to inside 文 + var state = Drag(Fixtures.File("中文a", "中文a"), PaneSide.Left, 0, 1, 0, 3); + + ViewerProgram.Apply(state, Input(CommandKind.Copy), link: null, window); + var highlighted = ScreenBuilder.Build(state).Left.Rows[0].Selection; + + await Assert.That(window.Copied).IsEqualTo("文"); + await Assert.That((highlighted.Start, highlighted.Length)).IsEqualTo((2, 2)); + } + + /// + /// A combining mark takes no cell: the bar after "é" is in cell 1, where every head draws it, + /// and selecting that cell copies the bar. Counted a cell a code point, cell 1 was the mark. + /// + [Test] + public async Task ACombiningMarkTakesNoCell() + { + var state = Drag(Fixtures.File("é|", "x"), PaneSide.Left, 0, 1, 0, 2); + + await Assert.That(Copy(state)).IsEqualTo("|"); + } + [Test] public async Task CopyWithNothingSelectedSaysSoAndWritesNothing() { @@ -379,24 +409,25 @@ public async Task A_status_change_keeps_the_selection() } /// - /// A head reports cells, and draws a character outside the basic plane in one: a drag across - /// the emoji alone ends at column 1, and copies all of it rather than half. + /// A head reports cells, and a character outside the basic plane is one character on the grid, + /// not two: a drag across 𝐀 (U+1D400) alone ends at column 1, and copies all of it rather than + /// half. Not an emoji, which is wide and so two cells: see CellGridTests. /// [Test] public async Task A_drag_across_one_non_bmp_character_copies_all_of_it() { - var state = Drag(Fixtures.File("\U0001F600x", "x"), PaneSide.Left, 0, 0, 0, 1); + var state = Drag(Fixtures.File("\U0001D400x", "x"), PaneSide.Left, 0, 0, 0, 1); - await Assert.That(Copy(state)).IsEqualTo("\U0001F600"); + await Assert.That(Copy(state)).IsEqualTo("\U0001D400"); } /// - /// "ab" drawn in cells 1 and 2, after an emoji in cell 0: the copy is what was highlighted. + /// "ab" drawn in cells 1 and 2, after 𝐀 in cell 0: the copy is what was highlighted. /// [Test] public async Task A_drag_after_a_non_bmp_character_copies_what_was_highlighted() { - var state = Drag(Fixtures.File("\U0001F600ab", "x"), PaneSide.Left, 0, 1, 0, 3); + var state = Drag(Fixtures.File("\U0001D400ab", "x"), PaneSide.Left, 0, 1, 0, 3); await Assert.That(Copy(state)).IsEqualTo("ab"); await Assert.That(ScreenBuilder.Build(state).Left.Rows[0].Selection).IsEqualTo(new SelectionSpan(1, 2)); @@ -408,10 +439,10 @@ public async Task A_drag_after_a_non_bmp_character_copies_what_was_highlighted() [Test] public async Task Select_all_ends_on_the_last_cell() { - var state = Key(Files("\U0001F600ab", "x"), CommandKind.SelectAll); + var state = Key(Files("\U0001D400ab", "x"), CommandKind.SelectAll); await Assert.That(state.Selection!.FocusColumn).IsEqualTo(3); - await Assert.That(Copy(state)).IsEqualTo("\U0001F600ab"); + await Assert.That(Copy(state)).IsEqualTo("\U0001D400ab"); } /// diff --git a/src/DiffEngineViewer.Windows.Tests/FormsHeadTests.cs b/src/DiffEngineViewer.Windows.Tests/FormsHeadTests.cs index 0c35a84b..fdf1a8c3 100644 --- a/src/DiffEngineViewer.Windows.Tests/FormsHeadTests.cs +++ b/src/DiffEngineViewer.Windows.Tests/FormsHeadTests.cs @@ -87,6 +87,47 @@ public async Task HighlightAtColumn66CoversItsGlyph() await Assert.That(ink.Max()).IsLessThan(band.Right); } + /// + /// A bar after characters the font does not draw a cell wide, selected at the column the grid + /// puts it in: its ink has to be inside the highlight. Drawn as one string, GDI+ put the bar + /// wherever the fallback font's widths left it - after two CJK characters about a third of a + /// cell short of column 4, and after a combining mark a whole cell before the column the + /// selection counted. + /// + [Test] + [Arguments("中中|", 4)] + [Arguments("é|", 1)] + [Arguments("a한b|", 4)] + public async Task HighlightAfterACharacterOffTheGridCoversTheNext(string line, int column) + { + var state = ViewerSession.Resize( + ViewerSession.Drag(Fixtures.File(line, line), PaneSide.Left, 0, column, 0, column + 1), + columns, + rows); + + using var host = new CanvasHost(); + var bitmap = host.Draw(ScreenBuilder.Build(state)); + var highlight = Bounds(bitmap, _ => _.ToArgb() == Palette.Selection.ToArgb()); + await Assert.That(highlight).IsNotNull(); + + var band = highlight!.Value; + var ink = new List(); + for (var x = band.Left - 2; x < band.Right + 2; x++) + { + if (bitmap.GetPixel(x, band.Top + band.Height / 2).GetBrightness() > 0.6f) + { + ink.Add(x); + } + } + + Console.WriteLine($"{line}: highlight x {band.Left}..{band.Right - 1}, ink {string.Join(",", ink)}"); + await Assert.That(ink).IsNotEmpty(); + // Centred, as a bar is in its own cell. Inside the highlight is not enough: drawn as one + // string the bar after two CJK characters was still inside it, three pixels short + var offCentre = Math.Abs((ink.Min() + ink.Max()) / 2.0 - (band.Left + band.Right - 1) / 2.0); + await Assert.That(offCentre).IsLessThanOrEqualTo(1); + } + /// /// Ten image pairs drawn one after another, each accepted (received moved over /// verified) before the next, and then a screen with no picture on it. Nothing needs more than diff --git a/src/DiffEngineViewer.Windows/ViewerCanvas.cs b/src/DiffEngineViewer.Windows/ViewerCanvas.cs index bfdf0c12..d8328104 100644 --- a/src/DiffEngineViewer.Windows/ViewerCanvas.cs +++ b/src/DiffEngineViewer.Windows/ViewerCanvas.cs @@ -606,13 +606,26 @@ void DrawRow(Graphics graphics, Pane pane, int index, Rectangle bounds) font, Palette.Dim, Cellular(bounds.X, bounds.Y, gutter, bounds.Height)); - Painter.Draw( - graphics, - // No wider than the pane can show in pixels, which no line of characters can exceed - RowText.Clip(RowText.Flatten(row.Text), bounds.Width), - font, - Palette.Foreground(row.Kind), - Cellular(bounds.X + gutter, bounds.Y, bounds.Width - gutter, bounds.Height)); + // Each segment at its column on the grid rather than the row as one string, so a character + // the font draws wider or narrower than a cell moves nothing after it: see CellGrid. A row + // of plain text is one segment at column 0, drawn exactly as the whole row was. + var text = RowText.Flatten(row.Text); + foreach (var segment in CellGrid.Segments(text)) + { + var left = bounds.X + gutter + Offset(segment.Column); + if (left >= bounds.Right) + { + break; + } + + Painter.Draw( + graphics, + // No wider than the pane can show in pixels, which no line of characters can exceed + RowText.Clip(text.Substring(segment.Start, segment.Length), bounds.Right - left), + font, + Palette.Foreground(row.Kind), + Cellular(left, bounds.Y, bounds.Right - left, bounds.Height)); + } } void DrawRule(Graphics graphics, int top) => diff --git a/src/DiffEngineViewer/CellGrid.cs b/src/DiffEngineViewer/CellGrid.cs new file mode 100644 index 00000000..d15c67c0 --- /dev/null +++ b/src/DiffEngineViewer/CellGrid.cs @@ -0,0 +1,334 @@ +using System.Buffers; +using System.Globalization; + +/// +/// Where the characters of a row's flattened text sit on the grid of character cells every head +/// draws, which is what a selection's columns count. +/// +/// Decided here, once, rather than read back out of each head's text layout. A head used to draw a +/// row as one string and let its font decide where each character went, while a selection counted +/// one cell per code point: right for the monospace font's own characters, and wrong wherever a +/// character came from somewhere else. CJK falls back to a font 1.83 cells wide on Windows, a +/// combining mark takes no room at all, and Core Text substitutes fonts with widths of their own, +/// so past the first such character the highlight, the hit test and the copy each described a +/// different run. Now a head draws a row as , each at its column, and a +/// character is wherever the grid says it is whatever its font makes of it. +/// +/// +/// A cluster is a character with the marks that attach to it: combining marks, variation +/// selectors and joiners take no cell of their own, and a character after a zero width joiner +/// belongs to the one before it, so an emoji sequence is one cluster. A wide character (East Asian +/// Wide or Fullwidth, and emoji) takes two cells, anything else one. Selection columns never land +/// inside a cluster (), so what is highlighted and what is copied cannot differ +/// by half a character. +/// +/// +static class CellGrid +{ + /// + /// A run of UTF-16 units at in the flattened + /// text, drawn starting at cell . + /// + public readonly record struct Segment(int Start, int Length, int Column); + + /// + /// How to draw : runs of characters every head's monospace font + /// draws a cell wide, which can be drawn as one string, and every other cluster on its own at + /// the column the grid gives it. A row of printable ASCII, which is nearly every row, is one + /// segment at column 0, drawn exactly as a whole row always was. + /// + public static IReadOnlyList Segments(string flattened) + { + if (flattened.Length == 0) + { + return []; + } + + if (IsPlain(flattened)) + { + return [new(0, flattened.Length, 0)]; + } + + var segments = new List(); + var column = 0; + var runStart = -1; + var runColumn = 0; + foreach (var cluster in Clusters(flattened)) + { + if (cluster.Simple) + { + if (runStart < 0) + { + runStart = cluster.Start; + runColumn = column; + } + } + else + { + if (runStart >= 0) + { + segments.Add(new(runStart, cluster.Start - runStart, runColumn)); + runStart = -1; + } + + segments.Add(new(cluster.Start, cluster.Length, column)); + } + + column += cluster.Width; + } + + if (runStart >= 0) + { + segments.Add(new(runStart, flattened.Length - runStart, runColumn)); + } + + return segments; + } + + /// + /// How many cells takes. + /// + public static int Cells(string flattened) + { + if (IsPlain(flattened)) + { + return flattened.Length; + } + + var cells = 0; + foreach (var cluster in Clusters(flattened)) + { + cells += cluster.Width; + } + + return cells; + } + + /// + /// The first cluster boundary at or after , as a column. A column inside + /// a wide cluster moves to its end, so a selection takes a wide character whole or not at all. + /// + public static int Snap(string flattened, int cell) => + Boundary(flattened, cell).Column; + + /// + /// Where in the boundary finds starts: never + /// inside a surrogate pair, and never between a character and its marks. + /// + public static int Index(string flattened, int cell) => + Boundary(flattened, cell).Index; + + static (int Index, int Column) Boundary(string flattened, int cell) + { + if (cell <= 0) + { + return (0, 0); + } + + if (IsPlain(flattened)) + { + var plain = Math.Min(cell, flattened.Length); + return (plain, plain); + } + + var column = 0; + foreach (var cluster in Clusters(flattened)) + { + if (column >= cell) + { + return (cluster.Start, column); + } + + column += cluster.Width; + } + + return (flattened.Length, column); + } + + /// + /// Printable ASCII throughout, which every head draws one cell a character with nothing to + /// work out. + /// + static bool IsPlain(string text) + { + foreach (var character in text) + { + if (character is < ' ' or > '~') + { + return false; + } + } + + return true; + } + + readonly record struct Cluster(int Start, int Length, int Width, bool Simple); + + static IEnumerable Clusters(string text) + { + var index = 0; + while (index < text.Length) + { + var start = index; + var rune = Read(text, ref index); + // A mark with nothing before it for it to sit on still takes a cell, or it could be + // neither drawn anywhere nor selected + var width = !ZeroWidth(rune) && Wide(rune) ? 2 : 1; + var simple = Simple(rune); + while (index < text.Length) + { + var next = index; + var attached = Read(text, ref next); + if (!ZeroWidth(attached)) + { + break; + } + + index = next; + simple = false; + // What a joiner joins is part of the same cluster: a family emoji is one picture + if (attached.Value == 0x200D && + index < text.Length) + { + Read(text, ref index); + } + } + + yield return new(start, index - start, width, simple); + } + } + + static Rune Read(string text, ref int index) + { + if (Rune.DecodeFromUtf16(text.AsSpan(index), out var rune, out var consumed) != OperationStatus.Done) + { + // A lone surrogate: one unit, drawn as whatever the head draws for one + index++; + return Rune.ReplacementChar; + } + + index += consumed; + return rune; + } + + /// + /// Takes no cell: combining and enclosing marks, and format characters such as the zero width + /// joiner and space. Variation selectors are nonspacing marks. + /// + static bool ZeroWidth(Rune rune) => + Rune.GetUnicodeCategory(rune) is + UnicodeCategory.NonSpacingMark or + UnicodeCategory.EnclosingMark or + UnicodeCategory.Format; + + /// + /// A character the embedded monospace font has, and so draws exactly one cell wide in every + /// head, which lets a run of them be drawn as one string: Latin with its extensions, Greek and + /// Cyrillic. Everything else is drawn on its own, at its column, because where a fallback font + /// would put the character after it is not something the grid can know. + /// + static bool Simple(Rune rune) + { + var value = rune.Value; + if (ZeroWidth(rune)) + { + return false; + } + + return value is + >= 0x20 and <= 0x7E or + >= 0xA0 and <= 0x24F or + >= 0x370 and <= 0x3FF or + >= 0x400 and <= 0x52F; + } + + /// + /// East Asian Wide and Fullwidth, and the emoji blocks, by range. Not the whole Unicode + /// property, which .NET does not expose, but the blocks where wide characters actually live. + /// + static bool Wide(Rune rune) + { + var value = rune.Value; + foreach (var (first, last) in wide) + { + if (value < first) + { + return false; + } + + if (value <= last) + { + return true; + } + } + + return false; + } + + // In ascending order, which Wide relies on to stop early + static readonly (int First, int Last)[] wide = + [ + (0x1100, 0x115F), + (0x231A, 0x231B), + (0x2329, 0x232A), + (0x23E9, 0x23EC), + (0x23F0, 0x23F0), + (0x23F3, 0x23F3), + (0x25FD, 0x25FE), + (0x2614, 0x2615), + (0x2648, 0x2653), + (0x267F, 0x267F), + (0x2693, 0x2693), + (0x26A1, 0x26A1), + (0x26AA, 0x26AB), + (0x26BD, 0x26BE), + (0x26C4, 0x26C5), + (0x26CE, 0x26CE), + (0x26D4, 0x26D4), + (0x26EA, 0x26EA), + (0x26F2, 0x26F3), + (0x26F5, 0x26F5), + (0x26FA, 0x26FA), + (0x26FD, 0x26FD), + (0x2705, 0x2705), + (0x270A, 0x270B), + (0x2728, 0x2728), + (0x274C, 0x274C), + (0x274E, 0x274E), + (0x2753, 0x2755), + (0x2757, 0x2757), + (0x2795, 0x2797), + (0x27B0, 0x27B0), + (0x27BF, 0x27BF), + (0x2B1B, 0x2B1C), + (0x2B50, 0x2B50), + (0x2B55, 0x2B55), + (0x2E80, 0x303E), + (0x3041, 0x33FF), + (0x3400, 0x4DBF), + (0x4E00, 0x9FFF), + (0xA000, 0xA4CF), + (0xA960, 0xA97F), + (0xAC00, 0xD7A3), + (0xF900, 0xFAFF), + (0xFE10, 0xFE19), + (0xFE30, 0xFE6F), + (0xFF00, 0xFF60), + (0xFFE0, 0xFFE6), + (0x16FE0, 0x16FE4), + (0x17000, 0x18AFF), + (0x1B000, 0x1B2FF), + (0x1F004, 0x1F004), + (0x1F0CF, 0x1F0CF), + (0x1F18E, 0x1F18E), + (0x1F191, 0x1F19A), + (0x1F200, 0x1F251), + (0x1F300, 0x1F64F), + (0x1F680, 0x1F6FF), + (0x1F7E0, 0x1F7EB), + (0x1F90C, 0x1F9FF), + (0x1FA70, 0x1FAFF), + (0x20000, 0x2FFFD), + (0x30000, 0x3FFFD) + ]; +} diff --git a/src/DiffEngineViewer/Native/Deview.cs b/src/DiffEngineViewer/Native/Deview.cs index bb5bead5..5a05b957 100644 --- a/src/DiffEngineViewer/Native/Deview.cs +++ b/src/DiffEngineViewer/Native/Deview.cs @@ -11,7 +11,7 @@ static unsafe partial class Deview /// Must match DEVIEW_VERSION in native/include/deview.h. Bumped whenever the structs change, /// so a stale native library is reported rather than read as garbage. /// - public const int ExpectedVersion = 8; + public const int ExpectedVersion = 9; [LibraryImport(library, EntryPoint = "deview_version")] public static partial int Version(); diff --git a/src/DiffEngineViewer/Native/DeviewStructs.cs b/src/DiffEngineViewer/Native/DeviewStructs.cs index ef3833d2..4beda275 100644 --- a/src/DiffEngineViewer/Native/DeviewStructs.cs +++ b/src/DiffEngineViewer/Native/DeviewStructs.cs @@ -8,6 +8,14 @@ /// rather than only on one that can load the library. /// /// +[StructLayout(LayoutKind.Sequential)] +struct DeviewSegment +{ + public int TextOffset; + public int TextLength; + public int Column; +} + [StructLayout(LayoutKind.Sequential)] struct DeviewRow { @@ -22,7 +30,13 @@ struct DeviewRow public int TextLength; /// - /// , in characters of the flattened text. Zero length on a row with + /// The row's , in . + /// + public int SegmentOffset; + public int SegmentCount; + + /// + /// , in cells of the grid the segments are drawn on. Zero length on a row with /// nothing selected, which is every row of almost every frame. /// public int SelectStart; @@ -92,6 +106,8 @@ unsafe struct DeviewScreen public int PaneCount; public DeviewRow* Rows; public int RowCount; + public DeviewSegment* Segments; + public int SegmentCount; public DeviewButton* Buttons; public int ButtonCount; public DeviewQueueItem* Queue; diff --git a/src/DiffEngineViewer/Native/ScreenPayload.cs b/src/DiffEngineViewer/Native/ScreenPayload.cs index 00a4934b..683aff5c 100644 --- a/src/DiffEngineViewer/Native/ScreenPayload.cs +++ b/src/DiffEngineViewer/Native/ScreenPayload.cs @@ -7,6 +7,7 @@ sealed class ScreenPayload { readonly List strings = []; readonly List rows = []; + readonly List segments = []; readonly List buttons = []; readonly List queue = []; readonly List menu = []; @@ -24,6 +25,7 @@ public void Build(Screen screen) { strings.Clear(); rows.Clear(); + segments.Clear(); buttons.Clear(); queue.Clear(); menu.Clear(); @@ -102,12 +104,13 @@ public unsafe int Present() { fixed (byte* stringsPtr = CollectionsMarshal.AsSpan(strings)) fixed (DeviewRow* rowsPtr = CollectionsMarshal.AsSpan(rows)) + fixed (DeviewSegment* segmentsPtr = CollectionsMarshal.AsSpan(segments)) fixed (DeviewButton* buttonsPtr = CollectionsMarshal.AsSpan(buttons)) fixed (DeviewQueueItem* queuePtr = CollectionsMarshal.AsSpan(queue)) fixed (DeviewMenuItem* menuPtr = CollectionsMarshal.AsSpan(menu)) fixed (DeviewPane* panesPtr = panes) { - var native = Native(stringsPtr, panesPtr, rowsPtr, buttonsPtr, queuePtr, menuPtr); + var native = Native(stringsPtr, panesPtr, rowsPtr, segmentsPtr, buttonsPtr, queuePtr, menuPtr); return Deview.Present(&native); } } @@ -116,12 +119,13 @@ public unsafe int Capture(int width, int height, string pngPath) { fixed (byte* stringsPtr = CollectionsMarshal.AsSpan(strings)) fixed (DeviewRow* rowsPtr = CollectionsMarshal.AsSpan(rows)) + fixed (DeviewSegment* segmentsPtr = CollectionsMarshal.AsSpan(segments)) fixed (DeviewButton* buttonsPtr = CollectionsMarshal.AsSpan(buttons)) fixed (DeviewQueueItem* queuePtr = CollectionsMarshal.AsSpan(queue)) fixed (DeviewMenuItem* menuPtr = CollectionsMarshal.AsSpan(menu)) fixed (DeviewPane* panesPtr = panes) { - var native = Native(stringsPtr, panesPtr, rowsPtr, buttonsPtr, queuePtr, menuPtr); + var native = Native(stringsPtr, panesPtr, rowsPtr, segmentsPtr, buttonsPtr, queuePtr, menuPtr); return Deview.Capture(&native, width, height, pngPath); } } @@ -130,6 +134,7 @@ unsafe DeviewScreen Native( byte* stringsPtr, DeviewPane* panesPtr, DeviewRow* rowsPtr, + DeviewSegment* segmentsPtr, DeviewButton* buttonsPtr, DeviewQueueItem* queuePtr, DeviewMenuItem* menuPtr) => @@ -141,6 +146,8 @@ unsafe DeviewScreen Native( PaneCount = panes.Length, Rows = rowsPtr, RowCount = rows.Count, + Segments = segmentsPtr, + SegmentCount = segments.Count, Buttons = buttonsPtr, ButtonCount = buttons.Count, Queue = queuePtr, @@ -163,9 +170,13 @@ DeviewPane AddPane(Pane pane, int columns) var rowOffset = rows.Count; foreach (var row in pane.Rows) { - // Clipped to the window, for the reason RowText.Clip gives: marshalled and laid out + // Flattened, so a tab is drawn as the four cells a selection counts it as, and + // clipped to the window, for the reason RowText.Clip gives: marshalled and laid out // whole every frame otherwise - var (textOffset, textLength) = Add(RowText.Clip(row.Text, columns)); + var text = RowText.Clip(RowText.Flatten(row.Text), columns); + var (textOffset, textLength) = Add(text); + var segmentOffset = segments.Count; + AddSegments(text, textOffset); rows.Add( new() { @@ -173,6 +184,8 @@ DeviewPane AddPane(Pane pane, int columns) LineNumber = row.LineNumber ?? -1, TextOffset = textOffset, TextLength = textLength, + SegmentOffset = segmentOffset, + SegmentCount = segments.Count - segmentOffset, SelectStart = row.Selection.Start, SelectLength = row.Selection.Length }); @@ -196,6 +209,25 @@ DeviewPane AddPane(Pane pane, int columns) }; } + /// + /// The row's , as byte ranges of the UTF-8 the row's text was + /// just written as at , so no text is written twice. + /// + void AddSegments(string text, int textOffset) + { + foreach (var segment in CellGrid.Segments(text)) + { + var start = textOffset + Encoding.UTF8.GetByteCount(text.AsSpan(0, segment.Start)); + segments.Add( + new() + { + TextOffset = start, + TextLength = Encoding.UTF8.GetByteCount(text.AsSpan(segment.Start, segment.Length)), + Column = segment.Column + }); + } + } + (int Offset, int Length) Add(string text) { if (text.Length == 0) diff --git a/src/DiffEngineViewer/SelectionText.cs b/src/DiffEngineViewer/SelectionText.cs index 7dd0225a..43920cde 100644 --- a/src/DiffEngineViewer/SelectionText.cs +++ b/src/DiffEngineViewer/SelectionText.cs @@ -76,9 +76,12 @@ public static SelectionSpan Span(TextSelection? selection, PaneSide side, int ro return default; } - var length = Cells(RowText.Flatten(text)); - var from = row == startRow ? Math.Min(startColumn, length) : 0; - var to = row == endRow ? Math.Min(endColumn, length) : length; + var flattened = RowText.Flatten(text); + var length = Cells(flattened); + // Snapped to whole clusters, the same boundaries the copy cuts at, so a selection ending + // inside a wide character highlights the whole of what it copies + var from = row == startRow ? CellGrid.Snap(flattened, Math.Min(startColumn, length)) : 0; + var to = row == endRow ? CellGrid.Snap(flattened, Math.Min(endColumn, length)) : length; return to <= from ? default : new(from, to - from); } @@ -156,7 +159,10 @@ public static string Summary(TextSelection selection, QueueEntry entry) } lines++; - length += Span(selection, selection.Side, index, row.Text).Length; + // Characters rather than cells, which a wide character is two of + var text = RowText.Flatten(row.Text); + var span = Span(selection, selection.Side, index, row.Text); + length += Characters(text, Index(text, span.Start), Index(text, span.Start + span.Length)); } if (lines == 0) @@ -181,47 +187,34 @@ public static string Summary(TextSelection selection, QueueEntry entry) } /// - /// How many cells a row's flattened text takes, which is what a selection's columns count. - /// - /// A head reports a drag in cells, and every head draws one code point to a cell: GDI+ draws a - /// character outside the basic plane one cell wide, as ImGui lays out one glyph per code point. - /// Counted in UTF-16 units instead, each such character shifted the copy one place from the - /// highlight, and a selection could end between the two halves of it and copy half a - /// character. Wide CJK and combining marks still do not fit this; that takes each head - /// reporting string positions from its own layout. - /// + /// How many cells a row's flattened text takes, which is what a selection's columns count. A + /// head reports a drag in cells and draws each row on the same grid: see . /// - public static int Cells(string flattened) - { - var cells = 0; - foreach (var character in flattened) - { - if (!char.IsLowSurrogate(character)) - { - cells++; - } - } + public static int Cells(string flattened) => + CellGrid.Cells(flattened); - return cells; - } + /// + /// Where in the flattened text a cell starts: never inside a surrogate pair, and never between + /// a character and its marks. + /// + static int Index(string flattened, int cell) => + CellGrid.Index(flattened, cell); /// - /// Where in the flattened text a cell starts: never inside a surrogate pair. + /// The code points between two indexes, which is what the status line calls characters. /// - static int Index(string flattened, int cell) + static int Characters(string text, int from, int to) { - var index = 0; - for (var count = 0; count < cell && index < flattened.Length; count++) + var count = 0; + for (var index = from; index < to; index++) { - index++; - if (index < flattened.Length && - char.IsLowSurrogate(flattened[index])) + if (!char.IsLowSurrogate(text[index])) { - index++; + count++; } } - return index; + return count; } static int ClampRow(int row, IReadOnlyList rows) => diff --git a/todo.md b/todo.md index eca38019..57a901bf 100644 --- a/todo.md +++ b/todo.md @@ -11,8 +11,4 @@ Open findings from a review of `main` at 4244ebe6 (2026-09-23), rechecked on c37 The repro tests are on the local branch `review-repros`, one class per area: `ReviewReproWindowsTests`, `ReviewReproTrayTests`, `ReviewReproPatcherTests`, `ReviewReproLibraryTests`. The fixed ones have moved into the topic test classes. Each test fails on c37bf9e1 except a control (`ControlSpaceIndentedLocalLeavesTheSiblingAlone`) and a measurement (`HowLongADecodeHoldsTheFile`). -Viewer model - -- [ ] **Selection columns count one code point to a cell, which wide CJK and combining marks do not take** (verified) - - Columns now count code points, which fixed the copy and the highlight for characters outside the basic plane: GDI+ draws those one cell wide and ImGui lays out one glyph per code point (`SelectionText.Cells`). Still off: CJK falls back to a font 1.83 cells wide on Windows, a combining mark takes none, and Core Text substitutes fonts with their own widths. - - Fix: have each head report string positions from its own layout rather than cells, or put every code point on the grid. +Nothing open.