From 5c59e07fc077371d8387f4f90c4a8fe2c6fb8455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 27 Oct 2026 15:49:54 -0400 Subject: [PATCH 1/7] Store PrintRunes runs from precomputed cell bits The append loop of TextBuffer.PrintRunes moves into a small static method, AppendSimpleRunes, so its state stays in registers. Each rune becomes one OR into the template's rune-free bits plus one 16-byte vector store (two for a wide glyph and its continuation). Before, every field of the template was rebuilt and every write was bounds-checked. Widths come from one table per AmbiguousAsWide setting with ambiguous widths already resolved, so the loop does one load per rune instead of a class lookup plus a static read. A line with one effective column stays on the bounds-checked WriteRunes path. New tests compare bulk and per-character printing with ambiguous widths both narrow and wide, including runs that wrap. Co-Authored-By: Claude Opus 5.5 --- src/Devolutions.Terminal.Core/CellRow.cs | 29 ++- src/Devolutions.Terminal.Core/TextBuffer.cs | 190 ++++++++++++------ src/Devolutions.Terminal.Core/WcWidth.cs | 6 +- .../AsciiFastPathTests.cs | 27 ++- 4 files changed, 185 insertions(+), 67 deletions(-) diff --git a/src/Devolutions.Terminal.Core/CellRow.cs b/src/Devolutions.Terminal.Core/CellRow.cs index 399ffa7..2bd5552 100644 --- a/src/Devolutions.Terminal.Core/CellRow.cs +++ b/src/Devolutions.Terminal.Core/CellRow.cs @@ -91,6 +91,27 @@ public static PackedCell From( public PackedCell WithRune(Rune rune) => new((_meta & ~RuneMask) | (uint)rune.Value, _foreground, _background, _flags); + /// This cell's 16 bytes with the rune cleared, for . + public Vector128 RuneFreeBits => Unsafe.BitCast>(WithRune(default)); + + /// + /// Stores the cell whose are given with + /// as its rune: one OR and one 16-byte write instead of + /// a write per field. + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void StoreWithRune(ref PackedCell destination, Vector128 runeFreeBits, int rune) + { + if (!BitConverter.IsLittleEndian) + { + destination = Unsafe.BitCast, PackedCell>(runeFreeBits).WithRune(Unsafe.BitCast(rune)); + return; + } + + // On little-endian hardware the rune bits are the low bits of the first word. + (runeFreeBits | Vector128.CreateScalar((ulong)(uint)rune)).StoreUnsafe(ref Unsafe.As(ref destination)); + } + public PackedCell WithWidth(byte storedWidth, bool isWideContinuation) => new( (_meta & ~((7u << WidthShift) | (1u << WideShift))) | @@ -268,20 +289,22 @@ public void WriteRunes( { PrepareWrite(start, end - start); var cells = _cells.AsSpan(start, end - start); + var narrowBits = narrow.RuneFreeBits; + var wideBits = wide.RuneFreeBits; var x = 0; for (var index = 0; index < runes.Length; index++) { - var rune = runes[index]; + var rune = runes[index].Value; if (widths[index] == 2) { - cells[x] = wide.WithRune(rune); + PackedCell.StoreWithRune(ref cells[x], wideBits, rune); cells[x + 1] = continuation; x += 2; _mayHoldWide = true; } else { - cells[x++] = narrow.WithRune(rune); + PackedCell.StoreWithRune(ref cells[x++], narrowBits, rune); } } diff --git a/src/Devolutions.Terminal.Core/TextBuffer.cs b/src/Devolutions.Terminal.Core/TextBuffer.cs index d2997a5..095eb69 100644 --- a/src/Devolutions.Terminal.Core/TextBuffer.cs +++ b/src/Devolutions.Terminal.Core/TextBuffer.cs @@ -1,6 +1,8 @@ using System.Text; using System.Globalization; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Runtime.Intrinsics; namespace Devolutions.Terminal.Core; @@ -547,6 +549,7 @@ public bool PrintRunes(ReadOnlySpan runes, out Rune lastAdvanced) } UpdateRuneTemplates(); + var simpleWidths = CurrentSimpleWidths(); var hyperlink = CurrentHyperlinkUri; var widths = _segmentWidths; var line = GetLiveLine(CursorY); @@ -562,7 +565,7 @@ public bool PrintRunes(ReadOnlySpan runes, out Rune lastAdvanced) var rune = runes[index]; // Only the per-rune path below can set a pending Prepend. var width = _pendingPrepend is null && !_mayHoldLonePrepend - ? SimpleWidth(rune, previousEndsWithoutZwj) + ? SimpleWidth(rune, simpleWidths, previousEndsWithoutZwj) : 0; if (width == 0) { @@ -605,45 +608,23 @@ public bool PrintRunes(ReadOnlySpan runes, out Rune lastAdvanced) var row = line.Cells; var end = CursorX; var next = index; - if (!row.MayHoldWide && row.CanAppendAt(CursorX)) + // A line narrower than a wide glyph (one effective column) keeps the + // bounds-checked path below. + if (!row.MayHoldWide && row.CanAppendAt(CursorX) && CursorX + width <= effectiveColumns) { // No wide glyph in the row can straddle the segment, so classify and // store in one pass. - var cells = row.BeginAppend(CursorX); - var narrow = _narrowTemplate; - var wide = _wideTemplate; - var continuation = _continuationTemplate; - var limit = effectiveColumns - CursorX; - var written = 0; - var wroteWide = false; - while (true) - { - var stored = runes[next]; - if (width == 2) - { - cells[written] = wide.WithRune(stored); - cells[written + 1] = continuation; - wroteWide = true; - } - else - { - cells[written] = narrow.WithRune(stored); - } - - written += width; - next++; - if (next == runes.Length) - { - break; - } - - width = SimpleWidth(runes[next], previousEndsWithoutZwj: true); - if (width == 0 || written + width > limit) - { - break; - } - } - + next = index + AppendSimpleRunes( + row.BeginAppend(CursorX), + runes[index..], + width, + effectiveColumns - CursorX, + simpleWidths, + _narrowBits, + _wideBits, + _continuationTemplate, + out var written, + out var wroteWide); end = CursorX + written; row.EndAppend(CursorX, end, hyperlink, wroteWide); } @@ -656,7 +637,7 @@ public bool PrintRunes(ReadOnlySpan runes, out Rune lastAdvanced) end += width; next++; width = next < runes.Length && next - index < widths.Length - ? SimpleWidth(runes[next], previousEndsWithoutZwj: true) + ? SimpleWidth(runes[next], simpleWidths, previousEndsWithoutZwj: true) : 0; } while (width != 0 && end + width <= effectiveColumns); @@ -692,12 +673,15 @@ public bool PrintRunes(ReadOnlySpan runes, out Rune lastAdvanced) return advanced; } - // Packed cells for runes stored by PrintRunes. The narrow one is packed per - // call (a few integer operations) and doubles as the cache key: the wide and - // continuation cells are derived again only when it changes. + // Packed cells for runes stored by PrintRunes, and the narrow and wide ones + // without their rune for PackedCell.StoreWithRune. The narrow one is packed per + // call (a few integer operations) and doubles as the cache key: the others are + // derived again only when it changes. private PackedCell _narrowTemplate; private PackedCell _wideTemplate; private PackedCell _continuationTemplate; + private Vector128 _narrowBits; + private Vector128 _wideBits; private bool _templatesBuilt; private readonly byte[] _segmentWidths = new byte[256]; @@ -712,49 +696,135 @@ private void UpdateRuneTemplates() _narrowTemplate = narrow; _wideTemplate = narrow.WithWidth(2, isWideContinuation: false); _continuationTemplate = narrow.WithWidth(0, isWideContinuation: true); + _narrowBits = narrow.RuneFreeBits; + _wideBits = _wideTemplate.RuneFreeBits; _templatesBuilt = true; } + /// + /// The append loop of , kept apart so its state stays in + /// registers. Stores from the first, whose simple + /// fits in cells, while each + /// next rune is also simple and still fits. Every rune after the first follows + /// a cell stored here, so none can join it. Returns the runes stored; + /// is the cells they took. + /// + private static int AppendSimpleRunes( + Span cells, + ReadOnlySpan runes, + int width, + int limit, + byte[] simpleWidths, + Vector128 narrowBits, + Vector128 wideBits, + PackedCell continuation, + out int written, + out bool wroteWide) + { + // These bound every write below, which therefore skips per-cell checks. + if ((uint)limit > (uint)cells.Length || width is not (1 or 2) || width > limit || runes.IsEmpty) + { + throw new ArgumentOutOfRangeException(nameof(limit)); + } + + ref var cell = ref MemoryMarshal.GetReference(cells); + ref var rune = ref MemoryMarshal.GetReference(runes); + // Vector arguments can arrive by reference; locals keep them in registers + // instead of reloading them after every store. + var narrow = narrowBits; + var wide = wideBits; + var continuationBits = Unsafe.BitCast>(continuation); + var count = 0; + var stored = 0; + while (true) + { + var value = Unsafe.Add(ref rune, count).Value; + if (width == 2) + { + PackedCell.StoreWithRune(ref Unsafe.Add(ref cell, stored), wide, value); + continuationBits.StoreUnsafe(ref Unsafe.As(ref Unsafe.Add(ref cell, stored + 1))); + } + else + { + PackedCell.StoreWithRune(ref Unsafe.Add(ref cell, stored), narrow, value); + } + + stored += width; + if (++count == runes.Length) + { + break; + } + + width = SimpleWidth(Unsafe.Add(ref rune, count), simpleWidths, previousEndsWithoutZwj: true); + if (width == 0 || stored + width > limit) + { + break; + } + } + + // Only a wide rune takes more than one cell. + written = stored; + wroteWide = stored != count; + return count; + } + /// /// The width of a rune may store without the per-rune /// path, or 0. Such a rune is not a Prepend and cannot extend the previous /// grapheme: either it has no join property at all, or it is an /// Extended_Pictographic scalar whose only possible join (GB11) needs combining /// text ending in ZWJ, which the previous cell is known not to have. The caller - /// has ruled out a pending or lone Prepend. + /// has ruled out a pending or lone Prepend. is + /// . /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static int SimpleWidth(Rune rune, bool previousEndsWithoutZwj) + private static int SimpleWidth(Rune rune, byte[] simpleWidths, bool previousEndsWithoutZwj) { - var simpleClass = rune.Value < SimpleClasses.Length ? SimpleClasses[rune.Value] : ClassifySimple(rune); - if ((simpleClass & NeverSimple) != 0 || - ((simpleClass & SimpleAfterZwjFreeCell) != 0 && !previousEndsWithoutZwj)) - { - return 0; - } - - return WcWidth.ResolveWidth((byte)(simpleClass & 3)); + var simple = (uint)rune.Value < (uint)simpleWidths.Length + ? simpleWidths[rune.Value] + : SimpleWidthBeyondTables(rune); + return (simple & SimpleAfterZwjFreeCell) != 0 && !previousEndsWithoutZwj ? 0 : simple & 3; } - // SimpleWidth's inputs in one load per rune for planes 0 and 1: the width class - // in the low two bits, plus whether the rune needs the per-rune path or may - // skip it only after a cell without a trailing ZWJ. + // SimpleWidth's inputs in one load per rune for planes 0 and 1, with ambiguous + // widths already resolved: the width in the low two bits (0 when the rune needs + // the per-rune path), plus whether it may skip that path only after a cell + // without a trailing ZWJ. There is one table per AmbiguousAsWide setting; the + // wide one is built on first use. private const byte SimpleAfterZwjFreeCell = 4; private const byte NeverSimple = 8; - private static readonly byte[] SimpleClasses = BuildSimpleClasses(); + private static readonly byte[] SimpleWidthsAmbiguousNarrow = BuildSimpleWidths(ambiguousAsWide: false); + private static byte[]? _simpleWidthsAmbiguousWide; - private static byte[] BuildSimpleClasses() + private static byte[] CurrentSimpleWidths() => + WcWidth.AmbiguousAsWide + ? (_simpleWidthsAmbiguousWide ??= BuildSimpleWidths(ambiguousAsWide: true)) + : SimpleWidthsAmbiguousNarrow; + + private static byte[] BuildSimpleWidths(bool ambiguousAsWide) { - var classes = new byte[0x20000]; - for (var value = 0; value < classes.Length; value++) + var widths = new byte[0x20000]; + for (var value = 0; value < widths.Length; value++) { if (value is < 0xD800 or > 0xDFFF) { - classes[value] = ClassifySimple(new Rune(value)); + widths[value] = ResolveSimple(ClassifySimple(new Rune(value)), ambiguousAsWide); } } - return classes; + return widths; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static byte SimpleWidthBeyondTables(Rune rune) => + ResolveSimple(ClassifySimple(rune), WcWidth.AmbiguousAsWide); + + private static byte ResolveSimple(byte simpleClass, bool ambiguousAsWide) + { + var width = WcWidth.ResolveWidth((byte)(simpleClass & 3), ambiguousAsWide); + return (simpleClass & NeverSimple) != 0 || width == 0 + ? (byte)0 + : (byte)(width | (simpleClass & SimpleAfterZwjFreeCell)); } // Built from ClassifyJoin, not the JoinClasses table, which static diff --git a/src/Devolutions.Terminal.Core/WcWidth.cs b/src/Devolutions.Terminal.Core/WcWidth.cs index 93ce412..2136171 100644 --- a/src/Devolutions.Terminal.Core/WcWidth.cs +++ b/src/Devolutions.Terminal.Core/WcWidth.cs @@ -29,8 +29,10 @@ public static int Width(Rune rune) internal static byte WidthClass(Rune rune) => rune.Value < Classes.Length ? Classes[rune.Value] : Classify(rune.Value); - internal static int ResolveWidth(byte widthClass) => - widthClass == Ambiguous ? (AmbiguousAsWide ? 2 : 1) : widthClass; + internal static int ResolveWidth(byte widthClass) => ResolveWidth(widthClass, AmbiguousAsWide); + + internal static int ResolveWidth(byte widthClass, bool ambiguousAsWide) => + widthClass == Ambiguous ? (ambiguousAsWide ? 2 : 1) : widthClass; private static byte[] BuildClasses() { diff --git a/tests/Devolutions.Terminal.Core.Tests/AsciiFastPathTests.cs b/tests/Devolutions.Terminal.Core.Tests/AsciiFastPathTests.cs index 5a1e1db..0440c17 100644 --- a/tests/Devolutions.Terminal.Core.Tests/AsciiFastPathTests.cs +++ b/tests/Devolutions.Terminal.Core.Tests/AsciiFastPathTests.cs @@ -9,8 +9,9 @@ namespace Devolutions.Terminal.Core.Tests; /// and the engine stores them in bulk; with bulk storage disabled every byte /// takes the per-character path, and both must leave the terminal in exactly the /// same state. DEC Special Graphics maps to -/// ambiguous-width box drawing, so stay clear of tests toggling -/// . +/// ambiguous-width box drawing, and one test here sets +/// , so this class stays clear of tests +/// toggling it. /// [Collection("Unicode cell width observations")] public sealed class AsciiFastPathTests @@ -49,6 +50,28 @@ public void BulkAsciiMatchesPerCharacterPrinting(string name, string stream) AssertBulkMatchesReference(name, Encoding.UTF8.GetBytes(stream), 20, 5, 8); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public void BulkRunesMatchPerCharacterPrintingWithAmbiguousWidths(bool ambiguousAsWide) + { + // Bulk printing reads one width table per AmbiguousAsWide setting. Latin-1 + // symbols, punctuation and box drawing are ambiguous; the long runs wrap. + var stream = + "±§ —…“” ┌─┐\r\n" + + new string('─', 15) + "\r\n" + + new string('a', 13) + "±─中│°═║"; + WcWidth.AmbiguousAsWide = ambiguousAsWide; + try + { + AssertBulkMatchesReference($"ambiguous as wide: {ambiguousAsWide}", Encoding.UTF8.GetBytes(stream), 20, 5, 8); + } + finally + { + WcWidth.AmbiguousAsWide = false; + } + } + [Fact] public void BulkAsciiMatchesPerCharacterPrintingOnRandomStreams() { From 94fd449ba16f3b66ec34e1b607c60cc50b93c925 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 27 Oct 2026 15:50:01 -0400 Subject: [PATCH 2/7] Cut the fixed cost of short prints Short writes between escape sequences (cursor-motion, `yes`) paid a setup cost on every call that exceeded the store itself: - The packed print templates are cached. Setting CurrentAttributes, CurrentProtection or the shell-integration state marks them stale. The ASCII template rebuilds apart from the rune templates, so a style change before every cell does not also rebuild those. - A single printable byte that fits on the line, with no wrap pending, is one CellRow.StoreAscii call instead of the segment loop. - The DEC Special Graphics check before each print compares length and first character inline instead of calling string.Equals. - CircularBuffer.ValidateIndex throws from a separate method, so the index check on every line access can inline. Co-Authored-By: Claude Opus 5.5 --- src/Devolutions.Terminal.Core/CellRow.cs | 24 ++++ .../CircularBuffer.cs | 9 +- .../TerminalEngine.cs | 11 +- src/Devolutions.Terminal.Core/TextBuffer.cs | 106 +++++++++++++++--- 4 files changed, 129 insertions(+), 21 deletions(-) diff --git a/src/Devolutions.Terminal.Core/CellRow.cs b/src/Devolutions.Terminal.Core/CellRow.cs index 2bd5552..1282beb 100644 --- a/src/Devolutions.Terminal.Core/CellRow.cs +++ b/src/Devolutions.Terminal.Core/CellRow.cs @@ -452,6 +452,30 @@ public void WriteAscii(int start, ReadOnlySpan text, PackedCell template, } } + /// + /// Equivalent to + /// with the single byte . + /// + public void StoreAscii(int x, byte value, PackedCell template, string? hyperlink) + { + PrepareWrite(x, 1); + _cells[x] = template.WithRune(new Rune(value)); + if (_combining is not null) + { + _combining[x] = null; + } + + if (hyperlink is not null) + { + _hyperlinks ??= new string?[_cells.Length]; + _hyperlinks[x] = hyperlink; + } + else if (_hyperlinks is not null) + { + _hyperlinks[x] = null; + } + } + public void CopyTo(int sourceIndex, CellRow destination, int destinationIndex, int count) { if (count <= 0) diff --git a/src/Devolutions.Terminal.Core/CircularBuffer.cs b/src/Devolutions.Terminal.Core/CircularBuffer.cs index ffadab7..59eaaac 100644 --- a/src/Devolutions.Terminal.Core/CircularBuffer.cs +++ b/src/Devolutions.Terminal.Core/CircularBuffer.cs @@ -1,3 +1,5 @@ +using System.Diagnostics.CodeAnalysis; + namespace Devolutions.Terminal.Core; internal sealed class CircularBuffer @@ -173,11 +175,16 @@ private void Move(int source, int destination, int count) private int Wrap(int index) => index >= _items.Length ? index - _items.Length : index; + // Every line access checks its index; keeping the throw in its own method lets + // the check inline. private void ValidateIndex(int index) { if ((uint)index >= (uint)Count) { - throw new ArgumentOutOfRangeException(nameof(index)); + ThrowIndexOutOfRange(); } } + + [DoesNotReturn] + private static void ThrowIndexOutOfRange() => throw new ArgumentOutOfRangeException("index"); } diff --git a/src/Devolutions.Terminal.Core/TerminalEngine.cs b/src/Devolutions.Terminal.Core/TerminalEngine.cs index 03397eb..e710a98 100644 --- a/src/Devolutions.Terminal.Core/TerminalEngine.cs +++ b/src/Devolutions.Terminal.Core/TerminalEngine.cs @@ -366,7 +366,7 @@ _drcsDesignator is not null && } else if (AnsiMode && isGlCharacter && - string.Equals(_gsets[activeGset], "0", StringComparison.Ordinal) && + IsDecSpecialGraphics(_gsets[activeGset]) && TryMapVt52Graphics(rune, out var lineDrawing)) { // DEC Special Graphics shares its glyph table with VT52 graphics mode. @@ -405,6 +405,11 @@ _drcsDesignator is not null && // Differential tests turn this off to get the per-character reference path. internal bool BulkPrintEnabled { get; set; } = true; + // The designator of DEC Special Graphics. Checked before every print, so it + // compares inline rather than calling string equality. + private static bool IsDecSpecialGraphics(string? designator) => + designator is { Length: 1 } && designator[0] == '0'; + void IVtDispatch.PrintAscii(ReadOnlySpan text) { // Bulk storage applies only when Print would store each byte unchanged: no @@ -417,7 +422,7 @@ void IVtDispatch.PrintAscii(ReadOnlySpan text) !AutoWrap || InsertMode || _singleShift >= 0 || - string.Equals(gset, "0", StringComparison.Ordinal) || + IsDecSpecialGraphics(gset) || (_drcsDesignator is not null && string.Equals(gset, _drcsDesignator, StringComparison.Ordinal))) { foreach (var value in text) @@ -445,7 +450,7 @@ void IVtDispatch.PrintRunes(ReadOnlySpan runes) !AutoWrap || InsertMode || _singleShift >= 0 || - string.Equals(gl, "0", StringComparison.Ordinal) || + IsDecSpecialGraphics(gl) || (_drcsDesignator is not null && (string.Equals(gl, _drcsDesignator, StringComparison.Ordinal) || string.Equals(_gsets[_gr], _drcsDesignator, StringComparison.Ordinal)))) diff --git a/src/Devolutions.Terminal.Core/TextBuffer.cs b/src/Devolutions.Terminal.Core/TextBuffer.cs index 095eb69..f6ace39 100644 --- a/src/Devolutions.Terminal.Core/TextBuffer.cs +++ b/src/Devolutions.Terminal.Core/TextBuffer.cs @@ -70,6 +70,8 @@ public BufferLine(CellRow cells, long logicalLineId) private readonly int _historySize; private bool[] _tabStops; private int _scrollOffset; + private CellAttributes _currentAttributes = CellAttributes.Default; + private bool _currentProtection; private ShellIntegrationKind _shellIntegration; private ShellMark? _activeMark; private bool _activeMarkHasOutput; @@ -115,8 +117,26 @@ public int ScrollOffset public int TotalLines => _lines.Count; public int HistoryCount => ViewportStart; public long CoordinateVersion { get; private set; } - public CellAttributes CurrentAttributes { get; set; } = CellAttributes.Default; - public bool CurrentProtection { get; set; } + public CellAttributes CurrentAttributes + { + get => _currentAttributes; + set + { + _currentAttributes = value; + InvalidatePrintTemplates(); + } + } + + public bool CurrentProtection + { + get => _currentProtection; + set + { + _currentProtection = value; + InvalidatePrintTemplates(); + } + } + public string? CurrentHyperlinkUri { get; set; } public int SavedCursorX { get; set; } public int SavedCursorY { get; set; } @@ -470,8 +490,29 @@ public bool PrintAscii(ReadOnlySpan text) index = 1; } - var template = PackedCell.From(new Rune(' '), CurrentAttributes, CurrentProtection, false, 1, _shellIntegration); + var template = AsciiTemplate(); var hyperlink = CurrentHyperlinkUri; + if (index == text.Length - 1 && !WrapPending) + { + // A lone byte that fits (one between escape sequences, or the y of + // `yes`): what the loop below does for it, as a single store. + var line = GetLiveLine(CursorY); + var effectiveColumns = RightBound(line) + 1; + if (CursorX < effectiveColumns) + { + var row = line.Cells; + ClearGlyphAt(row, CursorX); + row.StoreAscii(CursorX, text[index], template, hyperlink); + if (++CursorX >= effectiveColumns) + { + CursorX = effectiveColumns - 1; + WrapPending = true; + } + + return true; + } + } + while (index < text.Length) { if (WrapPending) @@ -673,32 +714,63 @@ public bool PrintRunes(ReadOnlySpan runes, out Rune lastAdvanced) return advanced; } - // Packed cells for runes stored by PrintRunes, and the narrow and wide ones - // without their rune for PackedCell.StoreWithRune. The narrow one is packed per - // call (a few integer operations) and doubles as the cache key: the others are - // derived again only when it changes. + // Packed cells stored by PrintAscii and PrintRunes: the ASCII (narrow) cell, + // the wide and continuation cells, and the narrow and wide ones without their + // rune for PackedCell.StoreWithRune. They depend only on the current + // attributes, protection and shell-integration state, whose setters mark them + // stale. The ASCII one is rebuilt apart from the rest, so a style change before + // every ASCII cell does not also pay for the rune templates. + private PackedCell _asciiTemplate; + private bool _asciiTemplateStale = true; private PackedCell _narrowTemplate; private PackedCell _wideTemplate; private PackedCell _continuationTemplate; private Vector128 _narrowBits; private Vector128 _wideBits; - private bool _templatesBuilt; + private bool _runeTemplatesStale = true; private readonly byte[] _segmentWidths = new byte[256]; + private void InvalidatePrintTemplates() + { + _asciiTemplateStale = true; + _runeTemplatesStale = true; + } + + private void SetShellIntegration(ShellIntegrationKind kind) + { + _shellIntegration = kind; + InvalidatePrintTemplates(); + } + + private PackedCell CurrentTemplate() => + PackedCell.From(new Rune(' '), _currentAttributes, _currentProtection, false, 1, _shellIntegration); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private PackedCell AsciiTemplate() + { + if (_asciiTemplateStale) + { + _asciiTemplate = CurrentTemplate(); + _asciiTemplateStale = false; + } + + return _asciiTemplate; + } + private void UpdateRuneTemplates() { - var narrow = PackedCell.From(new Rune(' '), CurrentAttributes, CurrentProtection, false, 1, _shellIntegration); - if (_templatesBuilt && narrow.SameBits(_narrowTemplate)) + if (!_runeTemplatesStale) { return; } + var narrow = CurrentTemplate(); _narrowTemplate = narrow; _wideTemplate = narrow.WithWidth(2, isWideContinuation: false); _continuationTemplate = narrow.WithWidth(0, isWideContinuation: true); _narrowBits = narrow.RuneFreeBits; _wideBits = _wideTemplate.RuneFreeBits; - _templatesBuilt = true; + _runeTemplatesStale = false; } /// @@ -1695,7 +1767,7 @@ public void Reset(bool keepHistory) SavedAttributes = CellAttributes.Default; SavedProtection = false; CurrentHyperlinkUri = null; - _shellIntegration = ShellIntegrationKind.None; + SetShellIntegration(ShellIntegrationKind.None); _activeMark = null; _activeMarkHasOutput = false; _pendingPrepend = null; @@ -1765,7 +1837,7 @@ public void StartPrompt() { if (_activeMark is not null) { - _shellIntegration = ShellIntegrationKind.Prompt; + SetShellIntegration(ShellIntegrationKind.Prompt); return; } @@ -1773,25 +1845,25 @@ public void StartPrompt() _activeMark = new ShellMark(CursorX); line.Marks.Add(_activeMark); _activeMarkHasOutput = false; - _shellIntegration = ShellIntegrationKind.Prompt; + SetShellIntegration(ShellIntegrationKind.Prompt); } public void StartCommand() { EnsureShellMark(); - _shellIntegration = ShellIntegrationKind.Command; + SetShellIntegration(ShellIntegrationKind.Command); } public void StartOutput() { EnsureShellMark(); _activeMarkHasOutput = true; - _shellIntegration = ShellIntegrationKind.Output; + SetShellIntegration(ShellIntegrationKind.Output); } public void EndCommand(uint? exitCode) { - _shellIntegration = ShellIntegrationKind.None; + SetShellIntegration(ShellIntegrationKind.None); if (_activeMark is { } mark) { mark.ExitCode = exitCode; From f0725e0129c1f06ef349c36cb9719ecbb284aef6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 27 Oct 2026 15:51:22 -0400 Subject: [PATCH 3/7] Cut CSI dispatch overhead - A complete plain CSI sequence is parsed straight into the arrays the dispatcher receives. Before, it went into the parser's working arrays and was then copied. A handler that feeds the parser again still takes the copying path, so it cannot overwrite the outer sequence. - Subparameter flags are no longer cleared after each sequence. Every push writes its own flag, so the clearing loop had no effect. - The parser calls the sealed TerminalEngine's DispatchCsi directly instead of through IVtDispatch. Other dispatchers still use the interface. - In the engine, a sequence with neither a private marker nor an intermediate byte skips the special-case checks and goes straight to the switch on its final byte (DispatchPlainCsi). Behaviour is unchanged, including device attributes (`CSI c`) and intermediate-byte sequences that match no special case. A new test feeds the parser again from inside a CSI handler and checks that the outer sequence's parameters stay intact. Co-Authored-By: Claude Opus 5.5 --- .../TerminalEngine.cs | 36 +++-- src/Devolutions.Terminal.Core/VtParser.cs | 126 +++++++++++++----- .../CsiFastPathTests.cs | 54 ++++++++ 3 files changed, 174 insertions(+), 42 deletions(-) diff --git a/src/Devolutions.Terminal.Core/TerminalEngine.cs b/src/Devolutions.Terminal.Core/TerminalEngine.cs index e710a98..1410735 100644 --- a/src/Devolutions.Terminal.Core/TerminalEngine.cs +++ b/src/Devolutions.Terminal.Core/TerminalEngine.cs @@ -2303,13 +2303,21 @@ private void DispatchShellIntegration(ReadOnlySpan data) ShellIntegrationChanged?.Invoke(this, EventArgs.Empty); } - private void DispatchCsi( + internal void DispatchCsi( char final, ReadOnlySpan parameters, ReadOnlySpan subparameters, byte intermediate, byte privateMarker) { + // Nearly every sequence has neither a private marker nor an intermediate, + // and none of the special cases below handles one of those. + if ((privateMarker | intermediate) == 0) + { + DispatchPlainCsi(final, parameters, subparameters); + return; + } + if (intermediate == 0 && final == 'u' && DispatchKittyKeyboard(parameters, privateMarker)) { return; @@ -2506,17 +2514,9 @@ bool DispatchModifyOtherKeys(ReadOnlySpan parameters, byte privateMarker) return; } - if (final == 'c') + if (final == 'c' && privateMarker == (byte)'>') { - if (privateMarker == (byte)'>') - { - Respond("\u001b[>0;10;1c"); - } - else if (privateMarker == 0 && Param(parameters, 0, 0) == 0) - { - Respond("\u001b[?61;4;6;7;14;21;22;23;24;28;32;42c"); - } - + Respond("\u001b[>0;10;1c"); return; } @@ -2525,6 +2525,13 @@ bool DispatchModifyOtherKeys(ReadOnlySpan parameters, byte privateMarker) return; } + DispatchPlainCsi(final, parameters, subparameters); + } + + // The sequences without a private marker. The ones with an intermediate that + // DispatchCsi does not handle land here too, as they always have. + private void DispatchPlainCsi(char final, ReadOnlySpan parameters, ReadOnlySpan subparameters) + { switch (final) { case 'A': @@ -2592,6 +2599,13 @@ bool DispatchModifyOtherKeys(ReadOnlySpan parameters, byte privateMarker) break; case 'b': RepeatLastCharacter(Count(parameters)); + break; + case 'c': + if (Param(parameters, 0, 0) == 0) + { + Respond("\u001b[?61;4;6;7;14;21;22;23;24;28;32;42c"); + } + break; case 'd': // SetCursor re-applies the origin-mode left margin to the column, diff --git a/src/Devolutions.Terminal.Core/VtParser.cs b/src/Devolutions.Terminal.Core/VtParser.cs index 0f9284e..894515f 100644 --- a/src/Devolutions.Terminal.Core/VtParser.cs +++ b/src/Devolutions.Terminal.Core/VtParser.cs @@ -40,6 +40,7 @@ private enum State } private readonly IVtDispatch _dispatch; + private readonly TerminalEngine? _engine; private readonly int[] _parameters = new int[MaxParameters]; private readonly bool[] _subparameters = new bool[MaxParameters]; private readonly int[] _csiParameters = new int[MaxParameters]; @@ -71,6 +72,7 @@ private enum State public VtParser(IVtDispatch dispatch) { _dispatch = dispatch; + _engine = dispatch as TerminalEngine; } public void Process(ReadOnlySpan data) @@ -290,31 +292,72 @@ private int TryProcessCsi(ReadOnlySpan data) return 0; } - // The state after ESC and '[' (and the private marker, if any). - ClearSequence(); - _state = State.CsiParam; - _privateMarker = privateMarker; - for (var index = start; index < final; index++) + if (_csiDispatchDepth > 0) { - var value = data[index]; - if (value == (byte)';') + // A handler feeding this parser again must not overwrite the outer + // sequence's dispatch arrays: take the state machine's copying path, + // from the state after ESC and '[' (and the private marker, if any). + ClearSequence(); + _state = State.CsiParam; + _privateMarker = privateMarker; + for (var index = start; index < final; index++) { - PushParameter(); - _nextIsSubparameter = false; + var value = data[index]; + if (value == (byte)';') + { + PushParameter(); + _nextIsSubparameter = false; + } + else if (value == (byte)':') + { + PushParameter(); + _nextIsSubparameter = true; + } + else + { + _currentParameter = _currentParameter < 0 ? 0 : _currentParameter; + _currentParameter = Math.Min((_currentParameter * 10) + (value - '0'), 65535); + } } - else if (value == (byte)':') + + DispatchCsi(data[final]); + return final + 1; + } + + // Parse straight into the dispatch arrays with the pushes the state machine + // would make, writing every flag below the count. The sequence fields are + // untouched: ground state never reads them before ESC or CSI clears them. + var parameters = _csiParameters; + var subparameters = _csiSubparameters; + var count = 0; + var current = -1; + var nextIsSubparameter = false; + for (var index = start; index < final; index++) + { + var value = data[index]; + if (value is (byte)';' or (byte)':') { - PushParameter(); - _nextIsSubparameter = true; + // At most MaxParameters separators, so this push always fits. + subparameters[count] = nextIsSubparameter; + parameters[count++] = current; + current = -1; + nextIsSubparameter = value == (byte)':'; } else { - _currentParameter = _currentParameter < 0 ? 0 : _currentParameter; - _currentParameter = Math.Min((_currentParameter * 10) + (value - '0'), 65535); + current = Math.Min((Math.Max(current, 0) * 10) + (value - '0'), 65535); } } - DispatchCsi(data[final]); + // The final push; one past the limit is dropped and the sequence still + // dispatches, as in DispatchCsi. + if ((current >= 0 || count > 0) && count < MaxParameters) + { + subparameters[count] = nextIsSubparameter; + parameters[count++] = current; + } + + InvokeCsi(data[final], parameters, subparameters, count, 0, privateMarker); return final + 1; } @@ -744,10 +787,6 @@ private void DispatchCsi(byte final) PushParameterIfNeeded(); } - var enterVt52 = final == (byte)'l' && - _privateMarker == (byte)'?' && - _parameterCount == 1 && - _parameters[0] == 2; _state = State.Ground; var parameterCount = _parameterCount; var intermediate = _intermediate; @@ -766,15 +805,44 @@ private void DispatchCsi(byte final) subparameters[index] = _subparameters[index]; } ClearSequence(); + InvokeCsi(final, parameters, subparameters, parameterCount, intermediate, privateMarker); + } + + private void InvokeCsi( + byte final, + int[] parameters, + bool[] subparameters, + int count, + byte intermediate, + byte privateMarker) + { + var enterVt52 = final == (byte)'l' && + privateMarker == (byte)'?' && + count == 1 && + parameters[0] == 2; _csiDispatchDepth++; try { - _dispatch.CsiDispatch( - (char)final, - parameters.AsSpan(0, parameterCount), - subparameters.AsSpan(0, parameterCount), - intermediate, - privateMarker); + // The engine is sealed: call it directly rather than through the + // interface and its explicit implementation. + if (_engine is not null) + { + _engine.DispatchCsi( + (char)final, + parameters.AsSpan(0, count), + subparameters.AsSpan(0, count), + intermediate, + privateMarker); + } + else + { + _dispatch.CsiDispatch( + (char)final, + parameters.AsSpan(0, count), + subparameters.AsSpan(0, count), + intermediate, + privateMarker); + } } finally { @@ -1151,6 +1219,8 @@ private void PushParameter() return; } + // Every push writes its flag, so the flags below the count always belong + // to the current sequence and nothing needs to clear them. _subparameters[_parameterCount] = _nextIsSubparameter; _parameters[_parameterCount++] = _currentParameter; _currentParameter = -1; @@ -1166,12 +1236,6 @@ private void PushParameterIfNeeded() private void ClearSequence() { - // PushParameter sets flags only below the count, so only those can be set. - for (var index = 0; index < _parameterCount; index++) - { - _subparameters[index] = false; - } - _parameterCount = 0; _currentParameter = -1; _intermediate = 0; diff --git a/tests/Devolutions.Terminal.Core.Tests/CsiFastPathTests.cs b/tests/Devolutions.Terminal.Core.Tests/CsiFastPathTests.cs index fb16e87..78b3966 100644 --- a/tests/Devolutions.Terminal.Core.Tests/CsiFastPathTests.cs +++ b/tests/Devolutions.Terminal.Core.Tests/CsiFastPathTests.cs @@ -87,6 +87,18 @@ public void RandomUtf8StreamsDispatchIdentically() } } + [Fact] + public void NestedCsiLeavesTheOuterParametersIntact() + { + // The one-pass parse writes into reused dispatch arrays; a handler that + // feeds the parser again must not overwrite the sequence it is handling. + var dispatch = new ReentrantDispatch(); + dispatch.Parser = new VtParser(dispatch); + dispatch.Parser.Process(Encoding.UTF8.GetBytes($"{Esc}[1;2:3m")); + + Assert.Equal(["H [9,8] [False,False]", "m [1,2,3] [False,False,True]"], dispatch.Calls); + } + private static void AssertSameDispatch(string name, byte[] bytes, Random random) { var bytewise = new RecordingDispatch(); @@ -165,4 +177,46 @@ public void DcsDispatch( public void ApcDispatch(ReadOnlySpan data) => Calls.Add($"apc {data.ToString()}"); } + + // Handles SGR by feeding a cursor move first, then records the SGR it was given. + private sealed class ReentrantDispatch : IVtDispatch + { + public VtParser Parser { get; set; } = null!; + + public List Calls { get; } = []; + + public void Print(Rune rune) + { + } + + public void ExecuteC0(byte control) + { + } + + public void EscDispatch(char final, byte intermediate) + { + } + + public void CsiDispatch(char final, ReadOnlySpan parameters, byte intermediate, bool privateMarker) => + throw new InvalidOperationException("the parser dispatches the detailed overload"); + + public void CsiDispatch( + char final, + ReadOnlySpan parameters, + ReadOnlySpan subparameters, + byte intermediate, + byte privateMarker) + { + if (final == 'm') + { + Parser.Process(Encoding.UTF8.GetBytes($"{Esc}[9;8H")); + } + + Calls.Add($"{final} [{string.Join(',', parameters.ToArray())}] [{string.Join(',', subparameters.ToArray())}]"); + } + + public void OscDispatch(int command, ReadOnlySpan data) + { + } + } } From 27c5c09aef0b596af62c4c27688ad1559949478e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 27 Oct 2026 15:51:36 -0400 Subject: [PATCH 4/7] Store combining text as interned IDs On the utf8 workload, the first combining mark on a line allocated a 120-slot string array (about 1 KB per line, since rows are recycled every line), and each mark built two full Cells and concatenated a new string. That came to 9.4 MB allocated per MiB of input. - CombiningTable (one per TextBuffer) maps a cell's combining text to an int ID; 0 means none, and the empty string has its own ID. A cache of (ID, mark) to ID means adding a mark already seen allocates nothing. - CellRow stores IDs in an int[] side array with no GC references, kept as a spare when the row is reset so recycled lines allocate nothing. CellRow.AppendCombining adds a mark without building Cells. Rows that do not share a table copy the text between them instead of the ID. - The table compacts once it passes max(4096, 2 x IDs in use): IDs that no line holds are dropped and the rest renumbered, so a stream of ever-new clusters cannot grow it without bound. - PrintRunes stores a zero-width rune with no grapheme-join property (U+0301, ZWJ) straight into the previous cell's combining text. It used to go through PrintCore, which could only end in the same append. utf8 allocation fell from 9.4 MB to 0.47 MB per MiB. CellRowModelTests now shares a table in half its iterations, adds random appends and empty combining text. A new test checks that compaction keeps every live cell's text while 12,000 new clusters pass through. Co-Authored-By: Claude Opus 5.5 --- src/Devolutions.Terminal.Core/CellRow.cs | 155 +++++++++++++++--- .../CombiningTable.cs | 96 +++++++++++ src/Devolutions.Terminal.Core/TextBuffer.cs | 114 ++++++++++--- .../CellRowModelTests.cs | 20 ++- .../CombiningTableTests.cs | 63 +++++++ 5 files changed, 399 insertions(+), 49 deletions(-) create mode 100644 src/Devolutions.Terminal.Core/CombiningTable.cs create mode 100644 tests/Devolutions.Terminal.Core.Tests/CombiningTableTests.cs diff --git a/src/Devolutions.Terminal.Core/CellRow.cs b/src/Devolutions.Terminal.Core/CellRow.cs index 1282beb..746b437 100644 --- a/src/Devolutions.Terminal.Core/CellRow.cs +++ b/src/Devolutions.Terminal.Core/CellRow.cs @@ -203,7 +203,9 @@ private static TermColor UnpackColor(ColorKind kind, uint value) => /// One line of packed cells. The indexer converts to and from the public /// so buffer code keeps its value semantics; hot paths use the /// bulk members. Side arrays exist only while some cell of the line carries -/// combining text or a hyperlink, and are bounded by the line width. +/// combining text or a hyperlink, and are bounded by the line width. Combining +/// text is stored as IDs; rows that share a table +/// copy IDs between them, others copy the text. /// /// /// Only cells in [_lo, _hi) are stored; every other cell is logically @@ -215,7 +217,12 @@ private static TermColor UnpackColor(ColorKind kind, uint value) => internal sealed class CellRow { private readonly PackedCell[] _cells; - private string?[]? _combining; + private readonly CombiningTable _table; + // Combining text IDs; 0 is none. + private int[]? _combining; + // The combining array a Reset released, reused (cleared) by the next line + // content needing one, so recycled scrollback lines allocate none. + private int[]? _spareCombining; private string?[]? _hyperlinks; private PackedCell _blank; private int _lo; @@ -224,33 +231,48 @@ internal sealed class CellRow // so writes cannot split one and need no clearing around them. private bool _mayHoldWide; - public CellRow(int columns, CellAttributes attributes) + public CellRow(int columns, CellAttributes attributes, CombiningTable? table = null) { _cells = GC.AllocateUninitializedArray(columns); _blank = PackedCell.Blank(attributes); + _table = table ?? new CombiningTable(); } - private CellRow(PackedCell[] cells) + private CellRow(PackedCell[] cells, CombiningTable table) { _cells = cells; + _table = table; } public int Length => _cells.Length; public Cell this[int x] { - get => CellAt(x).ToCell(_combining?[x], _hyperlinks?[x]); + get => CellAt(x).ToCell(CombiningAt(x), _hyperlinks?[x]); set { PrepareWrite(x, 1); var packed = PackedCell.From(value); _cells[x] = packed; NoteStored(packed); - SetSide(ref _combining, x, value.CombiningCharacters); + SetCombining(x, _table.Intern(value.CombiningCharacters)); SetSide(ref _hyperlinks, x, value.HyperlinkUri); } } + /// + /// Equivalent to reading cell and storing it back with + /// appended to its combining text. + /// + public void AppendCombining(int x, Rune rune) + { + var cell = CellAt(x); + PrepareWrite(x, 1); + _cells[x] = cell; + NoteStored(cell); + SetCombining(x, _table.Extend(CombiningIdAt(x), rune)); + } + /// Equivalent to the indexer storing a cell with these fields, without building one. public void Store( int x, @@ -267,7 +289,7 @@ public void Store( var packed = PackedCell.From(rune, attributes, isProtected, isWideContinuation, storedWidth, shellIntegration); _cells[x] = packed; NoteStored(packed); - SetSide(ref _combining, x, combining); + SetCombining(x, _table.Intern(combining)); SetSide(ref _hyperlinks, x, hyperlink); } @@ -326,7 +348,7 @@ public void StorePacked(int x, PackedCell cell, string? hyperlink) PrepareWrite(x, 1); _cells[x] = cell; NoteStored(cell); - SetSide(ref _combining, x, null); + SetCombining(x, 0); SetSide(ref _hyperlinks, x, hyperlink); } @@ -337,13 +359,14 @@ public void StorePacked(int x, PackedCell cell, string? hyperlink) public Rune RuneAt(int x) => CellAt(x).Rune; - public string? CombiningAt(int x) => _combining?[x]; + public string? CombiningAt(int x) => _table[CombiningIdAt(x)]; - public bool IsBlank(int x) => CellAt(x).IsBlankIgnoringCombining && string.IsNullOrEmpty(_combining?[x]); + public bool IsBlank(int x) => + CellAt(x).IsBlankIgnoringCombining && CombiningIdAt(x) is var id && (id == 0 || _table[id]!.Length == 0); - public static CellRow FromCells(ReadOnlySpan cells, int columns) + public static CellRow FromCells(ReadOnlySpan cells, int columns, CombiningTable? table = null) { - var row = new CellRow(columns, CellAttributes.Default); + var row = new CellRow(columns, CellAttributes.Default, table); var count = Math.Min(cells.Length, columns); for (var x = 0; x < count; x++) { @@ -371,7 +394,12 @@ public void Reset(CellAttributes attributes) _lo = 0; _hi = 0; _mayHoldWide = false; - _combining = null; + if (_combining is not null) + { + _spareCombining = _combining; + _combining = null; + } + _hyperlinks = null; } @@ -419,7 +447,16 @@ public void Fill(int start, int count, in Cell cell) var packed = PackedCell.From(cell); _cells.AsSpan(start, count).Fill(packed); NoteStored(packed); - FillSide(ref _combining, start, count, cell.CombiningCharacters); + var combining = _table.Intern(cell.CombiningCharacters); + if (combining != 0) + { + CombiningArray().AsSpan(start, count).Fill(combining); + } + else + { + ClearSide(_combining, start, count); + } + FillSide(ref _hyperlinks, start, count, cell.HyperlinkUri); } @@ -462,7 +499,7 @@ public void StoreAscii(int x, byte value, PackedCell template, string? hyperlink _cells[x] = template.WithRune(new Rune(value)); if (_combining is not null) { - _combining[x] = null; + _combining[x] = 0; } if (hyperlink is not null) @@ -487,13 +524,35 @@ public void CopyTo(int sourceIndex, CellRow destination, int destinationIndex, i destination.PrepareWrite(destinationIndex, count); destination._mayHoldWide |= _mayHoldWide; _cells.AsSpan(sourceIndex, count).CopyTo(destination._cells.AsSpan(destinationIndex, count)); - CopySide(_combining, ref destination._combining, sourceIndex, destinationIndex, count, destination.Length); + if (ReferenceEquals(_table, destination._table)) + { + if (_combining is not null) + { + Array.Copy(_combining, sourceIndex, destination.CombiningArray(), destinationIndex, count); + } + else + { + ClearSide(destination._combining, destinationIndex, count); + } + } + else + { + // IDs mean nothing in another table; carry the text. (A row copying + // within itself always takes the branch above.) + for (var offset = 0; offset < count; offset++) + { + destination.SetCombining( + destinationIndex + offset, + destination._table.Intern(CombiningAt(sourceIndex + offset))); + } + } + CopySide(_hyperlinks, ref destination._hyperlinks, sourceIndex, destinationIndex, count, destination.Length); } - public CellRow Clone() => new((PackedCell[])_cells.Clone()) + public CellRow Clone() => new((PackedCell[])_cells.Clone(), _table) { - _combining = (string?[]?)_combining?.Clone(), + _combining = (int[]?)_combining?.Clone(), _hyperlinks = (string?[]?)_hyperlinks?.Clone(), _blank = _blank, _lo = _lo, @@ -623,6 +682,64 @@ private void Materialize(int start, int end) PrepareWrite(start, end - start); } + private int CombiningIdAt(int x) => _combining?[x] ?? 0; + + private void SetCombining(int x, int id) + { + if (id != 0) + { + CombiningArray()[x] = id; + } + else if (_combining is not null) + { + _combining[x] = 0; + } + } + + // The combining array, taking the spare one or allocating it on first use. + private int[] CombiningArray() + { + if (_combining is null) + { + if (_spareCombining is { } spare) + { + Array.Clear(spare); + _combining = spare; + _spareCombining = null; + } + else + { + _combining = new int[_cells.Length]; + } + } + + return _combining; + } + + /// Marks in every combining ID this row holds. + public void MarkCombining(bool[] live) + { + if (_combining is not null) + { + foreach (var id in _combining) + { + live[id] = true; + } + } + } + + /// Renumbers this row's combining IDs after . + public void RemapCombining(int[] map) + { + if (_combining is not null) + { + for (var x = 0; x < _combining.Length; x++) + { + _combining[x] = map[_combining[x]]; + } + } + } + private void SetSide(ref string?[]? side, int x, string? value) { if (value is not null) @@ -649,7 +766,7 @@ private void FillSide(ref string?[]? side, int start, int count, string? value) } } - private static void ClearSide(string?[]? side, int start, int count) + private static void ClearSide(T[]? side, int start, int count) { if (side is not null) { diff --git a/src/Devolutions.Terminal.Core/CombiningTable.cs b/src/Devolutions.Terminal.Core/CombiningTable.cs new file mode 100644 index 0000000..c713694 --- /dev/null +++ b/src/Devolutions.Terminal.Core/CombiningTable.cs @@ -0,0 +1,96 @@ +using System.Text; + +namespace Devolutions.Terminal.Core; + +/// +/// Interns the combining text of cells (the scalars after the base rune) as int +/// IDs, so rows store ints instead of strings: a cluster seen before costs no +/// allocation, and the side arrays hold nothing for the GC to trace. ID 0 means no +/// combining text; the empty string has an ID of its own, as a cell can carry it. +/// One table serves the rows of one , on its thread. +/// +internal sealed class CombiningTable +{ + // Compaction never runs below this many IDs, and otherwise waits for the + // table to double since the last one, so its cost stays amortized. + private const int MinimumCompactionCount = 4096; + + private readonly Dictionary _ids = new(StringComparer.Ordinal); + // (ID, scalar) to the ID of that text with the scalar appended. + private readonly Dictionary _extended = []; + private string[] _texts = new string[16]; + private int _count = 1; + private int _compactAt = MinimumCompactionCount; + + /// The number of IDs handed out, counting 0. + public int Count => _count; + + public bool NeedsCompaction => _count > _compactAt; + + public string? this[int id] => id == 0 ? null : _texts[id]; + + public int Intern(string? text) + { + if (text is null) + { + return 0; + } + + if (_ids.TryGetValue(text, out var id)) + { + return id; + } + + if (_count == _texts.Length) + { + Array.Resize(ref _texts, _texts.Length * 2); + } + + id = _count++; + _texts[id] = text; + _ids.Add(text, id); + return id; + } + + /// The ID of 's text with appended. + public int Extend(int id, Rune rune) + { + var key = ((long)id << 21) | (uint)rune.Value; + if (!_extended.TryGetValue(key, out var extended)) + { + extended = Intern((this[id] ?? string.Empty) + rune); + _extended.Add(key, extended); + } + + return extended; + } + + /// + /// Drops every ID not marked in (indexed by ID, at least + /// long) and renumbers the rest. Returns the old-to-new map + /// that every row using this table must then apply. + /// + public int[] Compact(bool[] live) + { + var map = new int[_count]; + var texts = new string[Math.Max(16, _texts.Length)]; + var count = 1; + _ids.Clear(); + _extended.Clear(); + for (var id = 1; id < _count; id++) + { + if (live[id]) + { + map[id] = count; + texts[count] = _texts[id]; + _ids.Add(_texts[id], count); + count++; + } + } + + _texts = texts; + _count = count; + _compactAt = Math.Max(MinimumCompactionCount, count * 2); + return map; + } +} diff --git a/src/Devolutions.Terminal.Core/TextBuffer.cs b/src/Devolutions.Terminal.Core/TextBuffer.cs index f6ace39..af386ae 100644 --- a/src/Devolutions.Terminal.Core/TextBuffer.cs +++ b/src/Devolutions.Terminal.Core/TextBuffer.cs @@ -67,6 +67,7 @@ public BufferLine(CellRow cells, long logicalLineId) } private CircularBuffer _lines; + private readonly CombiningTable _combiningTable = new(); private readonly int _historySize; private bool[] _tabStops; private int _scrollOffset; @@ -116,6 +117,9 @@ public int ScrollOffset public bool WrapPending { get; set; } public int TotalLines => _lines.Count; public int HistoryCount => ViewportStart; + + // For tests: the combining table's size, which compaction keeps bounded. + internal int CombiningIdCount => _combiningTable.Count; public long CoordinateVersion { get; private set; } public CellAttributes CurrentAttributes { @@ -346,7 +350,7 @@ public void ReplaceViewport( var projection = y < rows.Count ? rows[y] : new TextBufferProjectionRow([], false, []); - var cells = CellRow.FromCells(projection.Cells, Columns); + var cells = CellRow.FromCells(projection.Cells, Columns, _combiningTable); _mayHoldLonePrepend = true; var line = new BufferLine(cells, ++_nextLogicalLineId) { @@ -363,6 +367,7 @@ public void ReplaceViewport( } _lines.ResetCapacity(Rows + _historySize, replacement); + CompactCombiningIfNeeded(); CursorX = Math.Clamp(cursorX, 0, Columns - 1); CursorY = Math.Clamp(cursorY, 0, Rows - 1); ScrollOffset = 0; @@ -605,9 +610,22 @@ public bool PrintRunes(ReadOnlySpan runes, out Rune lastAdvanced) { var rune = runes[index]; // Only the per-rune path below can set a pending Prepend. - var width = _pendingPrepend is null && !_mayHoldLonePrepend - ? SimpleWidth(rune, simpleWidths, previousEndsWithoutZwj) - : 0; + var simple = _pendingPrepend is null && !_mayHoldLonePrepend + ? SimpleEntry(rune, simpleWidths) + : (byte)0; + if (simple == PlainExtend) + { + // All PrintCore does with it: no join rule can hold and its width + // is 0, so it extends the previous cell's combining text. That text + // then ends with it, or (dropped on a blank or missing cell) is + // empty, so only a ZWJ can leave one at the end. + AppendCombining(rune); + previousEndsWithoutZwj = rune.Value != 0x200D; + index++; + continue; + } + + var width = SimpleWidth(simple, previousEndsWithoutZwj); if (width == 0) { previousEndsWithoutZwj = PrintAndGetAdvance(rune) > 0; @@ -850,13 +868,18 @@ private static int AppendSimpleRunes( /// . /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static int SimpleWidth(Rune rune, byte[] simpleWidths, bool previousEndsWithoutZwj) - { - var simple = (uint)rune.Value < (uint)simpleWidths.Length + private static int SimpleWidth(Rune rune, byte[] simpleWidths, bool previousEndsWithoutZwj) => + SimpleWidth(SimpleEntry(rune, simpleWidths), previousEndsWithoutZwj); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static int SimpleWidth(byte simple, bool previousEndsWithoutZwj) => + (simple & SimpleAfterZwjFreeCell) != 0 && !previousEndsWithoutZwj ? 0 : simple & 3; + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static byte SimpleEntry(Rune rune, byte[] simpleWidths) => + (uint)rune.Value < (uint)simpleWidths.Length ? simpleWidths[rune.Value] : SimpleWidthBeyondTables(rune); - return (simple & SimpleAfterZwjFreeCell) != 0 && !previousEndsWithoutZwj ? 0 : simple & 3; - } // SimpleWidth's inputs in one load per rune for planes 0 and 1, with ambiguous // widths already resolved: the width in the low two bits (0 when the rune needs @@ -865,6 +888,10 @@ private static int SimpleWidth(Rune rune, byte[] simpleWidths, bool previousEnds // wide one is built on first use. private const byte SimpleAfterZwjFreeCell = 4; private const byte NeverSimple = 8; + // In ClassifySimple's result: the rune has no join property at all. In the + // table: a zero-width such rune (a plain Extend mark, ZWJ), with width 0. + private const byte JoinFree = 16; + private const byte PlainExtend = 16; private static readonly byte[] SimpleWidthsAmbiguousNarrow = BuildSimpleWidths(ambiguousAsWide: false); private static byte[]? _simpleWidthsAmbiguousWide; @@ -894,9 +921,17 @@ private static byte SimpleWidthBeyondTables(Rune rune) => private static byte ResolveSimple(byte simpleClass, bool ambiguousAsWide) { var width = WcWidth.ResolveWidth((byte)(simpleClass & 3), ambiguousAsWide); - return (simpleClass & NeverSimple) != 0 || width == 0 - ? (byte)0 - : (byte)(width | (simpleClass & SimpleAfterZwjFreeCell)); + if ((simpleClass & NeverSimple) != 0) + { + return 0; + } + + if (width == 0) + { + return (simpleClass & JoinFree) != 0 ? PlainExtend : (byte)0; + } + + return (byte)(width | (simpleClass & SimpleAfterZwjFreeCell)); } // Built from ClassifyJoin, not the JoinClasses table, which static @@ -913,7 +948,7 @@ private static byte ClassifySimple(Rune rune) return (joinClass & ExtendsOnlyAfterZwj) != 0 ? (byte)(SimpleAfterZwjFreeCell | widthClass) - : widthClass; + : (byte)(JoinFree | widthClass); } /// @@ -1082,6 +1117,11 @@ private void PrintCore(Rune rune, bool autoWrap, out int advance) combining, CurrentHyperlinkUri, _shellIntegration); + if (combining is not null) + { + CompactCombiningIfNeeded(); + } + if (width == 2 && CursorX + 1 < Columns) { row.Store( @@ -1925,7 +1965,7 @@ private void TrimHistory() } private BufferLine NewBlankLine(CellAttributes attributes) => - new(new CellRow(Columns, attributes), ++_nextLogicalLineId); + new(new CellRow(Columns, attributes, _combiningTable), ++_nextLogicalLineId); /// /// Blanks a line that just left the buffer (or a scroll region) and gives it a @@ -1948,9 +1988,9 @@ private BufferLine RecycleLine(BufferLine line, CellAttributes attributes) return line; } - private static BufferLine ResizeLineCells(BufferLine source, int columns) + private BufferLine ResizeLineCells(BufferLine source, int columns) { - var cells = new CellRow(columns, CellAttributes.Default); + var cells = new CellRow(columns, CellAttributes.Default, _combiningTable); source.Cells.CopyTo(0, cells, 0, Math.Min(source.Cells.Length, columns)); RepairWideCells(cells); @@ -2291,19 +2331,43 @@ private void AppendCombining(Rune rune) } var row = GetLiveLine(y).Cells; - if (row[x].IsWideContinuation && x > 0) + if (row.IsWideContinuation(x) && x > 0) { x--; } - var cell = row[x]; - if (cell.IsBlank) + if (row.IsBlank(x)) { return; } - cell.CombiningCharacters = (cell.CombiningCharacters ?? string.Empty) + rune; - row[x] = cell; + row.AppendCombining(x, rune); + CompactCombiningIfNeeded(); + } + + /// + /// Drops the combining IDs no line holds any more once the table has grown + /// enough, so a stream of ever-new clusters cannot grow it without bound. Runs + /// between row operations: no row method holds an ID across it. + /// + private void CompactCombiningIfNeeded() + { + if (!_combiningTable.NeedsCompaction) + { + return; + } + + var live = new bool[_combiningTable.Count]; + for (var index = 0; index < _lines.Count; index++) + { + _lines[index].Cells.MarkCombining(live); + } + + var map = _combiningTable.Compact(live); + for (var index = 0; index < _lines.Count; index++) + { + _lines[index].Cells.RemapCombining(map); + } } private bool TryAppendJoinedRune(Rune rune, out int advance) @@ -2316,13 +2380,12 @@ private bool TryAppendJoinedRune(Rune rune, out int advance) } var (row, x, desiredWidth, lineColumns) = target.Value; - var previous = row[x]; - var previousWidth = previous.DisplayWidth; + var previousWidth = row.DisplayWidth(x); advance = Math.Max(0, desiredWidth - previousWidth); - previous.CombiningCharacters = (previous.CombiningCharacters ?? string.Empty) + rune; - row[x] = previous; + row.AppendCombining(x, rune); if (desiredWidth > previousWidth && x + desiredWidth <= lineColumns) { + var previous = row[x]; ClearGlyphAt(row, x + 1); previous.StoredWidth = (byte)desiredWidth; row[x] = previous; @@ -2347,6 +2410,7 @@ private bool TryAppendJoinedRune(Rune rune, out int advance) } } + CompactCombiningIfNeeded(); return true; } diff --git a/tests/Devolutions.Terminal.Core.Tests/CellRowModelTests.cs b/tests/Devolutions.Terminal.Core.Tests/CellRowModelTests.cs index cb8c474..8cd5ba6 100644 --- a/tests/Devolutions.Terminal.Core.Tests/CellRowModelTests.cs +++ b/tests/Devolutions.Terminal.Core.Tests/CellRowModelTests.cs @@ -5,8 +5,8 @@ namespace Devolutions.Terminal.Core.Tests; /// -/// packs cells, keeps rare strings in side arrays and -/// leaves a logically blank tail unwritten. Random operation sequences must +/// packs cells, keeps rare strings in side arrays (combining +/// text as interned IDs) and leaves a logically blank tail unwritten. Random operation sequences must /// leave it indistinguishable from a plain array. /// public sealed class CellRowModelTests @@ -20,7 +20,10 @@ public void RandomOperationsMatchPlainCellArrays() for (var iteration = 0; iteration < 300; iteration++) { var attributes = RandomAttributes(random); - var rows = new[] { new CellRow(Columns, attributes), new CellRow(Columns, CellAttributes.Default) }; + // Rows of one buffer share a combining table and copy IDs; rows with + // their own tables copy the text. + var table = iteration % 2 == 0 ? new CombiningTable() : null; + var rows = new[] { new CellRow(Columns, attributes, table), new CellRow(Columns, CellAttributes.Default, table) }; var models = new[] { Blank(attributes), Blank(CellAttributes.Default) }; for (var step = 0; step < 40; step++) { @@ -30,7 +33,7 @@ public void RandomOperationsMatchPlainCellArrays() var start = random.Next(Columns); var count = random.Next(0, Columns - start + 1); string operation; - switch (random.Next(9)) + switch (random.Next(10)) { case 0: var cell = RandomCell(random); @@ -119,6 +122,12 @@ public void RandomOperationsMatchPlainCellArrays() operation = $"append {start}+{written}"; break; + case 8: + var mark = new Rune(random.Next(2) == 0 ? 0x0301 : 0x200D); + row.AppendCombining(start, mark); + model[start].CombiningCharacters = (model[start].CombiningCharacters ?? string.Empty) + mark; + operation = $"combine {start}"; + break; default: rows[target] = row.Clone(); operation = "clone"; @@ -174,7 +183,8 @@ private static CellAttributes RandomAttributes(Random random) => random.Next(3) IsProtected = random.Next(4) == 0, IsWideContinuation = random.Next(5) == 0, StoredWidth = (byte)random.Next(0, 3), - CombiningCharacters = random.Next(4) == 0 ? "\u0301" : null, + // A cell can carry empty combining text, which is not the same as none. + CombiningCharacters = random.Next(8) switch { 0 or 1 => "\u0301", 2 => string.Empty, _ => null }, HyperlinkUri = random.Next(4) == 0 ? "https://example.test" : null, ShellIntegration = (ShellIntegrationKind)random.Next(4), }; diff --git a/tests/Devolutions.Terminal.Core.Tests/CombiningTableTests.cs b/tests/Devolutions.Terminal.Core.Tests/CombiningTableTests.cs new file mode 100644 index 0000000..9e1bdaa --- /dev/null +++ b/tests/Devolutions.Terminal.Core.Tests/CombiningTableTests.cs @@ -0,0 +1,63 @@ +using System.Text; +using Devolutions.Terminal.Core; +using Xunit; + +namespace Devolutions.Terminal.Core.Tests; + +/// +/// Rows store combining text as IDs from one table per buffer. The table must +/// stay bounded however many distinct clusters pass through, without changing +/// the text of any cell still in the buffer. +/// +public sealed class CombiningTableTests +{ + private const int Columns = 10; + + [Fact] + public void CompactionKeepsTheTextOfEveryLiveCell() + { + // The bottom line is written once, before any compaction. The top line is + // then overwritten 1,200 times with mark sequences never printed before, + // so the table repeatedly drops the IDs no line holds any more and + // renumbers the rest, the bottom line's included. + var buffer = new TextBuffer(Columns, 3, 4, hasHistory: true); + var serial = 0; + buffer.SetCursor(2, 0); + var kept = PrintLine(buffer, ref serial); + string[] last = []; + for (var pass = 0; pass < 1200; pass++) + { + buffer.SetCursor(0, 0); + last = PrintLine(buffer, ref serial); + } + + // 12,000 clusters and their prefixes passed through; only the 20 cells + // with marks keep IDs, so one compaction threshold bounds the table. + Assert.InRange(buffer.CombiningIdCount, 1, 4097); + Assert.Equal(last, buffer.GetRow(0).Select(static cell => cell.Text).ToArray()); + Assert.Equal(kept, buffer.GetRow(2).Select(static cell => cell.Text).ToArray()); + } + + // Prints a letter per cell, each followed by four marks from U+0300-U+030F + // that spell out the next serial number, and returns the cell texts. + private static string[] PrintLine(TextBuffer buffer, ref int serial) + { + var texts = new string[Columns]; + for (var x = 0; x < Columns; x++) + { + var text = new StringBuilder().Append((char)('a' + x)); + buffer.Print(new Rune('a' + x)); + for (var digit = 0; digit < 4; digit++) + { + var mark = new Rune(0x0300 + ((serial >> (4 * digit)) & 0xF)); + buffer.Print(mark); + text.Append(mark.ToString()); + } + + serial++; + texts[x] = text.ToString(); + } + + return texts; + } +} From 0f274e7258926062fbbe8fb63beb324d4c6569b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 27 Oct 2026 15:51:43 -0400 Subject: [PATCH 5/7] Decode 3- and 4-byte UTF-8 runs four at a time CJK and emoji text is long runs of same-length sequences, and decoding them was the largest single cost on those workloads. DecodeWideBlock reads 16 bytes with Vector128 (so Arm64 gets it too): - 3-byte block: four sequences in 12 bytes. One masked compare checks the E0-EF lead and continuation bytes, a shuffle moves each sequence into an int lane, and vector compares reject overlong forms and surrogates. - 4-byte block: four sequences in 16 bytes. Leads must be F0-F7, and one unsigned range check keeps U+10000-U+10FFFF, which also rejects overlong forms and F5-F7 leads. Any block that fails a check returns 0 and the scalar decoder handles it unchanged. On CJK, decoding is about 3x faster and emoji about 2.5x. A new differential test feeds long runs of both lengths, edge scalars, ASCII that shifts block alignment, runs longer than the 256-rune buffer, and every malformed form the checks must reject, whole, byte by byte and in random chunks. Invoke-TerminalBenchmarks.ps1 gains -MinMs, passed to the bench as --min-ms. Laptop CPUs leave turbo a few seconds into sustained load, so 500 ms samples land on both sides of that drop; ~3000 ms samples keep spreads and verdicts stable. Co-Authored-By: Claude Opus 5.5 --- scripts/Invoke-TerminalBenchmarks.ps1 | 10 +++ src/Devolutions.Terminal.Core/VtParser.cs | 86 ++++++++++++++++++ .../CsiFastPathTests.cs | 90 +++++++++++++++++++ 3 files changed, 186 insertions(+) diff --git a/scripts/Invoke-TerminalBenchmarks.ps1 b/scripts/Invoke-TerminalBenchmarks.ps1 index c771b3a..f3d0d7f 100644 --- a/scripts/Invoke-TerminalBenchmarks.ps1 +++ b/scripts/Invoke-TerminalBenchmarks.ps1 @@ -21,6 +21,12 @@ param( [ValidateRange(1, 100)] [int] $RegressionPercent = 15, + # Minimum length of each vt sample in milliseconds (the bench's default is 500). + # Laptop CPUs drop out of turbo a few seconds into sustained load; samples of + # ~3000 ms all run at the sustained clock, so medians and spreads stay stable. + [ValidateRange(0, 600000)] + [int] $MinMs = 0, + # Upstream ghostty-bench built from the pinned commit (see docs/performance-benchmarks.md). # When given, every vt workload is also replayed through `+terminal-stream`. [string] $GhosttyBench, @@ -101,6 +107,10 @@ if ($Suite -contains "vt") { foreach ($name in $names) { $partial = Join-Path $OutputDirectory "vt-$name.json" $vtArgs = @("vt", "--mb", $megabytes, "--runs", $runs, "--frame-kb", $FrameKb, "--workloads", $name, "--json", $partial) + if ($MinMs -gt 0) { + $vtArgs += @("--min-ms", $MinMs) + } + if (-not $RequireBeatGhostty) { $vtArgs += "--no-gate" } diff --git a/src/Devolutions.Terminal.Core/VtParser.cs b/src/Devolutions.Terminal.Core/VtParser.cs index 894515f..29d07c0 100644 --- a/src/Devolutions.Terminal.Core/VtParser.cs +++ b/src/Devolutions.Terminal.Core/VtParser.cs @@ -1,4 +1,7 @@ using System.Buffers; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Runtime.Intrinsics; using System.Text; namespace Devolutions.Terminal.Core; @@ -176,6 +179,19 @@ private int DecodePrintableRun(ReadOnlySpan data) continue; } + // CJK and emoji text is long runs of 3- or 4-byte sequences: take four + // at a time when the next 16 bytes allow. + if (value >= 0xE0 && data.Length - index >= 16 && runes.Length - count >= 4) + { + var consumed = DecodeWideBlock(data[index..], ref runes[count]); + if (consumed > 0) + { + index += consumed; + count += 4; + continue; + } + } + // Two- to four-byte sequences, decoded inline with the validity rules // of the byte path (no overlong forms, no surrogates, nothing past // U+10FFFF); everything else goes through the general decoder. @@ -237,6 +253,76 @@ private int DecodePrintableRun(ReadOnlySpan data) return index; } + /// + /// Decodes four well-formed 3-byte sequences (12 bytes) or four well-formed + /// 4-byte sequences (16 bytes) at the start of into + /// and the three runes after it, with the + /// validity rules of the scalar decode: no overlong forms, no surrogates, + /// nothing past U+10FFFF. Returns the bytes consumed, or 0 for anything else + /// (mixed lengths, ASCII, malformed input), which the scalar decode then takes. + /// + private static int DecodeWideBlock(ReadOnlySpan data, ref Rune destination) + { + if (!Vector128.IsHardwareAccelerated || data.Length < 16) + { + return 0; + } + + var block = Vector128.LoadUnsafe(ref MemoryMarshal.GetReference(data)); + ref var output = ref Unsafe.As(ref destination); + + // Lead bytes E0-EF at 0, 3, 6 and 9, continuation bytes (10xxxxxx) between. + var threeByte = Vector128.Create((byte)0xF0, 0xC0, 0xC0, 0xF0, 0xC0, 0xC0, 0xF0, 0xC0, 0xC0, 0xF0, 0xC0, 0xC0, 0, 0, 0, 0); + var threeLeads = Vector128.Create((byte)0xE0, 0x80, 0x80, 0xE0, 0x80, 0x80, 0xE0, 0x80, 0x80, 0xE0, 0x80, 0x80, 0, 0, 0, 0); + if (Vector128.EqualsAll(block & threeByte, threeLeads)) + { + // Each sequence into an int lane as lead << 16 | second << 8 | third. + var lanes = Vector128.Shuffle( + block, + Vector128.Create((byte)2, 1, 0, 0xFF, 5, 4, 3, 0xFF, 8, 7, 6, 0xFF, 11, 10, 9, 0xFF)).AsInt32(); + var scalars = ((lanes & Vector128.Create(0x0F0000)) >>> 4) | + ((lanes & Vector128.Create(0x3F00)) >>> 2) | + (lanes & Vector128.Create(0x3F)); + var invalid = Vector128.LessThan(scalars, Vector128.Create(0x800)) | + Vector128.Equals(scalars & Vector128.Create(0xF800), Vector128.Create(0xD800)); + if (invalid != Vector128.Zero) + { + return 0; + } + + scalars.StoreUnsafe(ref output); + return 12; + } + + // Lead bytes F0-F7 at 0, 4, 8 and 12; the range check below rejects F5-F7. + var fourByte = Vector128.Create((byte)0xF8, 0xC0, 0xC0, 0xC0, 0xF8, 0xC0, 0xC0, 0xC0, 0xF8, 0xC0, 0xC0, 0xC0, 0xF8, 0xC0, 0xC0, 0xC0); + var fourLeads = Vector128.Create((byte)0xF0, 0x80, 0x80, 0x80, 0xF0, 0x80, 0x80, 0x80, 0xF0, 0x80, 0x80, 0x80, 0xF0, 0x80, 0x80, 0x80); + if (Vector128.EqualsAll(block & fourByte, fourLeads)) + { + // Each sequence into an int lane, lead byte highest. + var lanes = Vector128.Shuffle( + block, + Vector128.Create((byte)3, 2, 1, 0, 7, 6, 5, 4, 11, 10, 9, 8, 15, 14, 13, 12)).AsInt32(); + var scalars = ((lanes >>> 6) & Vector128.Create(0x1C0000)) | + ((lanes >>> 4) & Vector128.Create(0x3F000)) | + ((lanes >>> 2) & Vector128.Create(0xFC0)) | + (lanes & Vector128.Create(0x3F)); + // U+10000-U+10FFFF: one unsigned compare after the subtraction. + var outOfRange = Vector128.GreaterThan( + (scalars - Vector128.Create(0x10000)).AsUInt32(), + Vector128.Create(0xFFFFFu)); + if (outOfRange != Vector128.Zero) + { + return 0; + } + + scalars.StoreUnsafe(ref output); + return 16; + } + + return 0; + } + /// /// Parses a complete, plain CSI sequence (ESC [, optional private /// marker, digits, ';' and ':' separators, final byte) in one pass instead of diff --git a/tests/Devolutions.Terminal.Core.Tests/CsiFastPathTests.cs b/tests/Devolutions.Terminal.Core.Tests/CsiFastPathTests.cs index 78b3966..c193c9b 100644 --- a/tests/Devolutions.Terminal.Core.Tests/CsiFastPathTests.cs +++ b/tests/Devolutions.Terminal.Core.Tests/CsiFastPathTests.cs @@ -87,6 +87,96 @@ public void RandomUtf8StreamsDispatchIdentically() } } + [Fact] + public void RandomWideSequenceRunsDispatchIdentically() + { + // Runs of 3- and 4-byte sequences are decoded four at a time; any block + // holding one malformed or out-of-range sequence must fall back and decode + // exactly as the byte path does. Edge scalars sit next to the rejected forms. + int[] threeByteScalars = [0x800, 0xD7FF, 0xE000, 0xFFFF, 0x4E00, 0x9FFF, 0x3042]; + int[] fourByteScalars = [0x10000, 0x10FFFF, 0x1F600, 0x1F64F, 0x20000]; + byte[][] malformed = + [ + [0xE0, 0x9F, 0xBF], // overlong U+07FF + [0xED, 0xA0, 0x80], // surrogate U+D800 + [0xED, 0xBF, 0xBF], // surrogate U+DFFF + [0xF0, 0x8F, 0xBF, 0xBF], // overlong U+FFFF + [0xF4, 0x90, 0x80, 0x80], // U+110000 + [0xF5, 0x80, 0x80, 0x80], // lead past F4 + [0xF7, 0xBF, 0xBF, 0xBF], + [0xF8, 0x90, 0x80, 0x80], // never a lead; its low bits would give U+10000 + [0xFC, 0x80, 0x80, 0x80], // likewise U+100000 + [0xE4, 0xB8], // truncated + [0xF0, 0x9F, 0x98], // truncated + [0xF1, 0x80, 0x80], // truncated, would read as a 3-byte sequence past U+0800 + [0xF4, 0x8F, 0xBF], // truncated, likewise + [0xE4, 0xC3, 0xA9], // lead, then a 2-byte sequence where continuations belong + [0xE4, 0xB8, 0xC3], // lead, continuation, then a 2-byte lead + [0xF0, 0x9F, 0x98, 0xC3], // three of four bytes, then a 2-byte lead + [0xF0, 0xC3, 0x98, 0x80], + [0x80], [0xBF], [0xC3, 0xA9], [0x0A], [0x1B, (byte)'[', (byte)'m'], + ]; + byte[] soup = [0x80, 0xBF, 0xC3, 0xE4, 0xED, 0xEF, 0xF0, 0xF4, 0xF5, 0x41]; + var random = new Random(2027); + for (var iteration = 0; iteration < 400; iteration++) + { + var bytes = new List(); + for (var piece = 0; piece < 12; piece++) + { + var kind = random.Next(12); + if (kind == 8) + { + bytes.AddRange(malformed[random.Next(malformed.Length)]); + } + else if (kind == 9) + { + // Random lead and continuation bytes in any order. + for (var count = random.Next(1, 5); count > 0; count--) + { + bytes.Add(soup[random.Next(soup.Length)]); + } + } + else if (kind == 10) + { + // A short ASCII stretch joins the run and shifts block alignment + // within the 256-rune buffer the parser decodes into. + for (var count = random.Next(1, 4); count > 0; count--) + { + bytes.Add((byte)random.Next(0x21, 0x7F)); + } + } + else + { + // A run of one length class: usually long enough for a few blocks, + // sometimes longer than the 256-rune buffer. + var four = kind is >= 4 and < 8; + var length = kind == 11 ? random.Next(250, 300) : random.Next(1, 14); + for (var index = 0; index < length; index++) + { + var scalar = random.Next(3) == 0 + ? (four ? fourByteScalars : threeByteScalars)[random.Next(four ? fourByteScalars.Length : threeByteScalars.Length)] + : four ? random.Next(0x10000, 0x110000) : RandomThreeByteScalar(random); + bytes.AddRange(Encoding.UTF8.GetBytes(char.ConvertFromUtf32(scalar))); + } + } + } + + AssertSameDispatch($"wide runs #{iteration}", [.. bytes], random); + } + + static int RandomThreeByteScalar(Random random) + { + while (true) + { + var scalar = random.Next(0x800, 0x10000); + if (scalar is < 0xD800 or > 0xDFFF) + { + return scalar; + } + } + } + } + [Fact] public void NestedCsiLeavesTheOuterParametersIntact() { From f0a6f9aa8de9ea2b92c4168461217dae401ca55b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 27 Oct 2026 16:00:25 -0400 Subject: [PATCH 6/7] Update the Ghostty comparison with round 5 results Builtin now leads raw libghostty-vt on all 13 vt workloads, up from 8 of 13. cjk, emoji and utf8 went from about 0.8x to 1.12x, 1.21x and 1.36x; cursor-motion and scroll-region from 0.90x and 0.96x to 1.09x and 1.11x. - Results come from `-MinMs 3000`. This laptop's CPU leaves turbo about 2.7 s into sustained load, so 500 ms samples landed on both sides of that drop; the methodology section now says so. - The engine section describes round 5: interned combining text, the vectorized UTF-8 decode, the CSI dispatch changes and the cached print templates. - The remaining-gaps section is rewritten around the new stage timings, and records that prefetching recycled lines did not help. - The frame-preparation and Ghostty-mode tables are marked as round 4 measurements; round 5 did not touch that code. Co-Authored-By: Claude Opus 5.5 --- docs/performance-benchmarks.md | 187 +++++++++++++++++++-------------- 1 file changed, 111 insertions(+), 76 deletions(-) diff --git a/docs/performance-benchmarks.md b/docs/performance-benchmarks.md index fe6bb1a..421aac8 100644 --- a/docs/performance-benchmarks.md +++ b/docs/performance-benchmarks.md @@ -135,6 +135,11 @@ Methodology: - A sample replays the corpus into the same engine until at least `--min-ms` (500 ms by default) has elapsed, so fast targets are not dominated by timer noise. + - Laptop CPUs leave turbo a few seconds into sustained load: on the machine + below, throughput falls about 3x after ~2.7 s and stays there, whatever + the engine does. With 500 ms samples, runs land on both sides of that drop + and spreads reach 50–180%. Samples of ~3 s (`--min-ms 3000`, or the + script's `-MinMs 3000`) all run at the sustained clock. - **Conservative verdict.** - `faster` only when builtin's *slowest* run beats Ghostty's *fastest*. - `slower` when the reverse holds. @@ -169,6 +174,7 @@ Workloads (`--workloads a,b,...`): ```powershell pwsh scripts/Invoke-TerminalBenchmarks.ps1 # vt + render, results in artifacts/bench/ pwsh scripts/Invoke-TerminalBenchmarks.ps1 -Quick # fast local signal +pwsh scripts/Invoke-TerminalBenchmarks.ps1 -Suite vt -MinMs 3000 # ~3 s samples: stable on CPUs that leave turbo under load pwsh scripts/Invoke-TerminalBenchmarks.ps1 -Baseline artifacts/bench//vt.json # fails if builtin/ghostty-vt drops >15% on identical inputs pwsh scripts/Invoke-TerminalBenchmarks.ps1 -RequireBeatGhostty # fails unless builtin wins every vt workload pwsh scripts/Invoke-TerminalBenchmarks.ps1 -GhosttyBench \ghostty-bench.exe # also replays the corpora through upstream +terminal-stream @@ -198,17 +204,19 @@ lists the full set of gates. Setup for these numbers: -- One Windows 11 x64 workstation (24 logical CPUs, AVX2), .NET 10.0.13 (JIT, - tiered compilation off), and the pinned `libghostty-vt` (ReleaseFast). -- 120x30 grid, 16 KiB chunks, 8 MiB per workload, 7 timed runs plus 1 warmup. +- One Windows 11 x64 laptop (Intel Core Ultra 9 285HX, 24 cores, AVX2), + .NET 10.0.14 (JIT, tiered compilation off), and the pinned `libghostty-vt` + (ReleaseFast). +- 120x30 grid, 16 KiB chunks, 8 MiB per workload, 7 timed runs plus 1 warmup, + samples of at least 3 s. - Each workload ran in its own process, via: ```powershell - pwsh scripts/Invoke-TerminalBenchmarks.ps1 -Suite vt,render + pwsh scripts/Invoke-TerminalBenchmarks.ps1 -Suite vt -MinMs 3000 ``` -This machine's absolute speed drifted by up to 2.5x between invocations -(Ghostty itself moved from ~540 to ~1,600 MB/s on `light-cells`). So read the +This machine's absolute speed drifts by up to 2.5x between invocations, +mostly with the CPU's turbo state (see *Fair timing* above). So read the **builtin / ghostty-vt ratio**, which is measured within each run, and treat anything within about ±10% of 1.0 as a tie. Throughput is the median MB/s. @@ -216,19 +224,24 @@ anything within about ±10% of 1.0 as a tie. Throughput is the median MB/s. | Workload | builtin | ghostty-vt | ghostty (DT) | ratio | verdict | builtin alloc / MiB | screens | | --- | ---: | ---: | ---: | ---: | --- | ---: | --- | -| ascii | 1,144 | 922 | 411 | **1.24x** | overlap | 31 KB | identical | -| ascii-lines | 879 | 741 | 361 | **1.19x** | **faster** | 40 KB | identical | -| light-cells | 2,024 | 1,647 | 60 | **1.23x** | **faster** | 0 | identical | -| scrolling | 42.5 | 31.9 | 29.5 | **1.33x** | overlap | 0.7 MB | identical | -| dense-cells | 207 | 144 | 53 | **1.44x** | **faster** | 0 | identical | -| styled | 78.4 | 26.9 | 24.8 | **2.91x** | **faster** | 5.4 MB | 1 row differs | -| osc | 124 | 27.0 | 25.1 | **4.59x** | **faster** | 6.9 MB | identical | -| mixed | 667 | 362 | 227 | **1.84x** | **faster** | 54 KB | identical | -| scroll-region | 641 | 669 | 56 | 0.96x | overlap | 0 | identical | -| cursor-motion | 152 | 168 | 94 | 0.90x | slower | 33 KB | identical | -| emoji | 588 | 736 | 267 | 0.80x | overlap | 61 KB | identical | -| utf8 | 61.3 | 79.6 | 70.1 | 0.77x | slower | 8.7 MB | identical | -| cjk | 458 | 596 | 299 | 0.77x | slower | 78 KB | identical | +| osc | 147 | 25.3 | 23.5 | **5.82x** | **faster** | 6.7 MB | identical | +| styled | 45.9 | 18.0 | 13.2 | **2.54x** | **faster** | 1.4 MB | 1 row differs | +| mixed | 787 | 366 | 229 | **2.15x** | **faster** | 8 KB | identical | +| dense-cells | 262 | 138 | 51 | **1.90x** | **faster** | 0 | identical | +| scrolling | 56.5 | 32.6 | 31.9 | **1.73x** | **faster** | 104 KB | identical | +| utf8 | 80.1 | 59.0 | 69.3 | **1.36x** | overlap | 92 KB | identical | +| light-cells | 2,265 | 1,737 | 62 | **1.30x** | **faster** | 0 | identical | +| emoji* | 816 | 672 | 301 | **1.21x** | **faster** | 7 KB | identical | +| ascii | 1,145 | 971 | 448 | **1.18x** | **faster** | 5 KB | identical | +| ascii-lines | 716 | 626 | 286 | **1.14x** | overlap | 9 KB | identical | +| cjk* | 583 | 522 | 237 | **1.12x** | **faster** | 10 KB | identical | +| scroll-region | 865 | 782 | 58 | **1.11x** | **faster** | 0 | identical | +| cursor-motion | 188 | 172 | 95 | **1.09x** | **faster** | 32 KB | identical | + +\* From a later invocation of those two workloads alone. In the full run they +landed in a noisy stretch (spreads over 50%) and measured 0.98x and 1.22x. +Other quiet runs of the same code measured 1.07–1.21x on cjk and 1.18x on +emoji. `ghostty (DT)` skips projection without `--frame-kb`. The one-row difference in `styled` is a behavior difference, not a measurement artifact: builtin drops a @@ -237,21 +250,23 @@ combining mark that follows a space, and Ghostty keeps it. How builtin got here, as the median builtin/ghostty-vt ratio per round (each round measured against its own in-run Ghostty control): -| Workload | `dc1cd26` | round 1 | round 2 (packed cells) | round 3 | round 4 | -| --- | ---: | ---: | ---: | ---: | ---: | -| ascii | 0.009 | 0.20 | 0.55 | 1.21 | 1.24 | -| ascii-lines | 0.009 | 0.20 | 0.50 | 1.13 | 1.19 | -| light-cells | 0.009 | 0.32 | 0.48 | 1.20 | 1.23 | -| scrolling | 0.015 | 0.14 | 0.40 | 1.11 | 1.33 | -| dense-cells | 0.42 | 0.63 | 0.62 | 1.57 | 1.44 | -| styled | 0.30 | 1.33 | 1.14 | 2.82 | 2.91 | -| osc | 0.29 | 2.60 | 4.52 | 4.57 | 4.59 | -| mixed | 0.03 | 0.39 | 0.45 | 1.56 | 1.84 | -| scroll-region | 0.018 | 0.22 | 0.34 | 0.90 | 0.96 | -| cursor-motion | 0.11 | 0.23 | 0.17 | 0.94 | 0.90 | -| utf8 | 0.10 | 0.36 | 0.27 | 0.57 | 0.77 | -| cjk | — | — | — | 0.53 | 0.77 | -| emoji | — | — | — | 0.50 | 0.80 | +| Workload | `dc1cd26` | round 1 | round 2 (packed cells) | round 3 | round 4 (`f45b95b`) | round 5 | +| --- | ---: | ---: | ---: | ---: | ---: | ---: | +| ascii | 0.009 | 0.20 | 0.55 | 1.21 | 1.24 | 1.18 | +| ascii-lines | 0.009 | 0.20 | 0.50 | 1.13 | 1.19 | 1.14 | +| light-cells | 0.009 | 0.32 | 0.48 | 1.20 | 1.23 | 1.30 | +| scrolling | 0.015 | 0.14 | 0.40 | 1.11 | 1.33 | 1.73 | +| dense-cells | 0.42 | 0.63 | 0.62 | 1.57 | 1.44 | 1.90 | +| styled | 0.30 | 1.33 | 1.14 | 2.82 | 2.91 | 2.54 | +| osc | 0.29 | 2.60 | 4.52 | 4.57 | 4.59 | 5.82 | +| mixed | 0.03 | 0.39 | 0.45 | 1.56 | 1.84 | 2.15 | +| scroll-region | 0.018 | 0.22 | 0.34 | 0.90 | 0.96 | 1.11 | +| cursor-motion | 0.11 | 0.23 | 0.17 | 0.94 | 0.90 | 1.09 | +| utf8 | 0.10 | 0.36 | 0.27 | 0.57 | 0.77 | 1.36 | +| cjk | — | — | — | 0.53 | 0.77 | 1.12 | +| emoji | — | — | — | 0.50 | 0.80 | 1.21 | + +Round 5 was measured with 3 s samples; earlier rounds used 500 ms samples. The `-Baseline` gate flagged real regressions twice: @@ -260,12 +275,15 @@ The `-Baseline` gate flagged real regressions twice: the short-ASCII path. Each was confirmed with an interleaved A/B build and fixed before moving on. -Moves of about ±10% between rounds (dense-cells and cursor-motion from round 3 -to round 4, for example) are within this machine's noise. +Round 5 passed the gate after each change. Moves of about ±10% between rounds +(ascii and ascii-lines into round 5, for example) are within this machine's +noise. styled's 2.54x comes from a run with a 136% spread; single-workload +runs of the same code measured 3.39–3.61x. ### DT's Ghostty mode vs raw `libghostty-vt`, with frames These numbers use `--frame-kb 256`, so a projection is taken every 256 KiB. +They were measured in round 4; round 5 did not change the Ghostty engine. | Workload | ghostty (DT) before | after | ghostty-vt | ratio before → after | | --- | ---: | ---: | ---: | ---: | @@ -280,7 +298,7 @@ per-cell P/Invoke projection still dominates. ### Frame preparation and CPU paint -`render`, 120x30, median p50 / p95: +`render`, 120x30, median p50 / p95, measured in round 4: | Stage | before packed cells | now | | --- | --- | --- | @@ -298,14 +316,18 @@ No Ghostty counterpart exists on Windows. Every fast path is checked against the per-character path by differential tests (`AsciiFastPathTests`, `CsiFastPathTests`, `CellRowModelTests`, -`PackedCellStorageTests`). Each new branch was also checked by deliberately -breaking it and confirming a test fails. +`PackedCellStorageTests`, `CombiningTableTests`). Each new branch was also +checked by deliberately breaking it and confirming a test fails. - **Packed, reference-free storage** (`CellRow`, `PackedCell`). - A cell is 16 bytes with no GC references, down from about 40 with two `string?` fields. - - Combining text and hyperlinks live in per-line side arrays that exist only - while needed and are freed when the line is recycled. + - Hyperlinks live in a per-line side array that exists only while needed. + - Combining text is interned per buffer (`CombiningTable`), and lines store + int IDs. A cluster seen before costs no allocation, a recycled line keeps + its ID array, and the table compacts so ever-new clusters cannot grow it + without bound. This took utf8 from 9.4 MB to under 0.1 MB allocated per + MiB. - The public `Cell` and `TermColor` are unchanged, and every value round-trips losslessly. - **Logically blank cells are never written.** @@ -318,17 +340,30 @@ breaking it and confirming a test fails. - **ASCII:** the parser hands whole printable-ASCII runs to `IVtDispatch.PrintAscii`. They are stored a row segment at a time, two 64-bit (or, with AVX2, two 256-bit per four cells) stores per cell batch. - - **UTF-8 runs:** well-formed UTF-8 runs (2–4 byte sequences decoded inline) - go to `IVtDispatch.PrintRunes`. + - **UTF-8 runs:** well-formed UTF-8 runs go to `IVtDispatch.PrintRunes`. + Runs of 3- or 4-byte sequences (CJK, emoji) are decoded four at a time + from 16-byte `Vector128` loads, with the scalar decoder's validity rules + (no overlong forms, surrogates or values past U+10FFFF); anything else + falls back to the scalar decoder. On CJK this decodes about 3x faster. - **Grapheme joins:** precomputed tables for planes 0 and 1 rule out joins, so runes that cannot join are stored a segment at a time. That includes - emoji after a cell without a trailing ZWJ. + emoji after a cell without a trailing ZWJ. A zero-width rune with no join + property (a combining accent, ZWJ) goes straight into the previous cell's + combining text. - **Rows without wide glyphs:** a conservative per-row "may hold wide cells" flag means classifying and storing happen in one pass, with no clearing - around wide glyphs. Every freshly recycled scrollback line qualifies. -- **CSI fast path.** A complete plain CSI sequence is parsed in one pass, then - goes through the same `DispatchCsi` as the byte path. Parameters are copied - into reused arrays. + around wide glyphs. Every freshly recycled scrollback line qualifies. That + pass ORs each rune into precomputed template bits and writes one 16-byte + store per cell, with widths from a table where ambiguous widths are already + resolved. + - **Short writes:** the packed print templates are cached and rebuilt only + when the attributes, protection or shell-integration state change, and a + single printable byte between escape sequences is one store. +- **CSI fast path.** A complete plain CSI sequence is parsed in one pass, + straight into the arrays the dispatcher receives, and the parser calls the + engine directly instead of through `IVtDispatch`. Sequences with no private + marker or intermediate byte skip the special cases and go straight to the + switch on the final byte. - **C0 fast path.** CR, LF and other controls in ground state go straight to `ExecuteC0` instead of through the string-state checks. - **Scroll regions** rotate line references with `Array.Copy`, not one @@ -336,36 +371,36 @@ breaking it and confirming a test fails. ### What still separates builtin from Ghostty -- **Non-ASCII text** (`cjk`, `emoji`, `utf8`: about 0.8x). On CJK, per scalar: - - | Stage | Cost | - | --- | --- | - | Builtin: decode | ~2.0 ns | - | Builtin: storage (two 16-byte cells per wide character) | ~3.6 ns | - | Builtin: dispatch | ~0.6 ns | - | Ghostty: everything | ~5 ns | - - What was tried and measured: - - **Runtime SIMD transcoder.** Swapping the scalar decoder for the runtime's - SIMD `Utf8.ToUtf16` (plus a UTF-16-to-rune pass) was *slower* at these run - lengths, and was reverted. - - **Unchecked scalar decoding.** A `ref`-based decoder with unchecked `Rune` - construction measured the same as the indexed one. - - Closing the rest needs either a purpose-built SIMD UTF-8 decoder or 8-byte - cells. -- **cursor-motion and scroll-region** are ties within noise (0.90–0.96x). Per - escape sequence, builtin still pays for an interface call into the engine - and that call's own checks before each short write. -- **Cell size.** Packed cells are 16 bytes against Ghostty's 8, so a cleared - or copied row moves twice the memory. Getting to 8 bytes means interning - styles, which is a larger change. +Builtin now leads on every workload. The closest leads, `cursor-motion`, +`scroll-region` and `cjk` (1.09–1.12x), sit near the ±10% band of this +machine's noise, and `ascii-lines` and `utf8` still get `overlap` verdicts +from run-to-run spread. Timed stage by stage on CJK after round 5 (one engine +reused with history full, per scalar, all stages in one process): + +| Stage | Cost | +| --- | --- | +| UTF-8 decode, block path | ~0.5–1.0 ns | +| Parser per-line work (CR/LF dispatch, run setup) | ~0.7–1 ns | +| Cell stores (two 16-byte cells per wide character) | ~2 ns | +| Buffer CR/LF, scrolling and line reuse | ~0.7 ns | + +- **Parser per-line overhead** now costs about as much as the decode. Each + CR and LF still goes through an interface call, and each line's run of + characters pays its setup once. +- **Cell size.** Packed cells are 16 bytes against Ghostty's 8, so storing, + clearing or copying a row moves twice the memory. Getting to 8 bytes means + interning styles, which is a larger change. +- **What did not help.** Prefetching the scrollback lines about to be reused + measured slower: with history full, steady-state throughput is the same at + 100 and 9,000 lines, so cold recycled lines are not a bottleneck. **Variance.** Within isolated processes the ratio repeats to about ±10% on -this machine. In one long process running every workload, ratios drifted by -up to 50%, because heap state carried over between workloads. For that reason -the script runs one process per workload, and `-Baseline` only compares -workloads with the same SHA-256 and gates on the ratio, not on absolute MB/s. +this machine with 3 s samples. With 500 ms samples, runs straddle the CPU's +turbo drop and single workloads swing by 50% or more. In one long process +running every workload, ratios also drifted by up to 50%, because heap state +carried over between workloads. For that reason the script runs one process +per workload, and `-Baseline` only compares workloads with the same SHA-256 +and gates on the ratio, not on absolute MB/s. ## Definition of "matches or beats Ghostty" From c1455e44dbbdbe739fcf01923f5831106cf89aba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 27 Oct 2026 16:12:21 -0400 Subject: [PATCH 7/7] Keep windows on the displays that exist When an external display is disconnected, Windows moves its windows onto the remaining display without shrinking them. A terminal sized for the larger display then overhangs the laptop panel with its caption buttons out of reach. A saved position can also point at a display that is gone, and the window was restored there as-is. WindowPlacement.Fit takes a window rectangle and the current work areas. When they cover all of the window it returns it unchanged, so a window spanning two connected displays stays put. Otherwise it shrinks the window to fit and moves it inside the work area it overlaps most, or the nearest one when it overlaps none. Mirrored displays count once. MainWindow applies it: - at launch, after restoring a saved position or applying a requested one, before maximizing, so the window maximizes on a display that exists; - when Screens.Changed fires, and again a second later, once Windows has finished moving windows off a removed display; - when the window returns to Normal, since the OS restores the bounds it remembered from before the change. A maximized or full-screen window keeps its state; its saved normal bounds are fitted instead, so un-maximizing and the persisted layout land on a display that exists. Minimized windows are left alone. Windows Terminal only checks, at launch, that the window's top-left corner is on some monitor, and never shrinks a window. Co-Authored-By: Claude Opus 5.5 --- .../Platform/WindowPlacement.cs | 98 +++++++++++++++++++ .../Views/MainWindow.axaml.cs | 82 ++++++++++++++++ .../WindowPlacementTests.cs | 89 +++++++++++++++++ 3 files changed, 269 insertions(+) create mode 100644 src/Devolutions.Terminal.App/Platform/WindowPlacement.cs create mode 100644 tests/Devolutions.Terminal.App.Tests/WindowPlacementTests.cs diff --git a/src/Devolutions.Terminal.App/Platform/WindowPlacement.cs b/src/Devolutions.Terminal.App/Platform/WindowPlacement.cs new file mode 100644 index 0000000..0101b26 --- /dev/null +++ b/src/Devolutions.Terminal.App/Platform/WindowPlacement.cs @@ -0,0 +1,98 @@ +using Avalonia; + +namespace Devolutions.Terminal.App.Platform; + +/// +/// Keeps a window reachable on the displays that exist now. A display can go +/// away while the app runs or between runs (a laptop undocked); Windows moves a +/// window off a removed display without shrinking it, and a restored position +/// can point at a display that is gone, so a window can end up larger than the +/// screen or outside it, with its caption buttons out of reach. +/// +public static class WindowPlacement +{ + /// + /// Returns unchanged when the work areas cover all of + /// it, so a window spanning two connected displays stays put. Otherwise returns + /// it shrunk to fit and moved inside the work area it overlaps most, or the + /// nearest one when it overlaps none. All values are physical pixels. + /// + public static PixelRect Fit(PixelRect bounds, IReadOnlyList workAreas) + { + ArgumentNullException.ThrowIfNull(workAreas); + if (bounds.Width <= 0 || bounds.Height <= 0) + { + return bounds; + } + + // Mirrored displays report the same work area; count it once. + var areas = workAreas.Where(static area => area.Width > 0 && area.Height > 0).Distinct().ToArray(); + if (areas.Length == 0) + { + return bounds; + } + + long covered = 0; + long bestOverlap = 0; + var target = areas[0]; + foreach (var area in areas) + { + var overlap = OverlapArea(bounds, area); + covered += overlap; + if (overlap > bestOverlap) + { + bestOverlap = overlap; + target = area; + } + } + + // Work areas do not overlap one another, so full coverage means every pixel + // of the window is on some display. + if (covered >= (long)bounds.Width * bounds.Height) + { + return bounds; + } + + if (bestOverlap == 0) + { + target = Nearest(bounds, areas); + } + + var width = Math.Min(bounds.Width, target.Width); + var height = Math.Min(bounds.Height, target.Height); + return new PixelRect( + Math.Clamp(bounds.X, target.X, target.Right - width), + Math.Clamp(bounds.Y, target.Y, target.Bottom - height), + width, + height); + } + + private static long OverlapArea(PixelRect a, PixelRect b) + { + long width = Math.Min(a.Right, b.Right) - Math.Max(a.X, b.X); + long height = Math.Min(a.Bottom, b.Bottom) - Math.Max(a.Y, b.Y); + return width > 0 && height > 0 ? width * height : 0; + } + + // The work area closest to the window's center. + private static PixelRect Nearest(PixelRect bounds, PixelRect[] areas) + { + var centerX = bounds.X + (bounds.Width / 2L); + var centerY = bounds.Y + (bounds.Height / 2L); + var nearest = areas[0]; + var nearestDistance = long.MaxValue; + foreach (var area in areas) + { + var dx = centerX - Math.Clamp(centerX, area.X, area.Right); + var dy = centerY - Math.Clamp(centerY, area.Y, area.Bottom); + var distance = (dx * dx) + (dy * dy); + if (distance < nearestDistance) + { + nearestDistance = distance; + nearest = area; + } + } + + return nearest; + } +} diff --git a/src/Devolutions.Terminal.App/Views/MainWindow.axaml.cs b/src/Devolutions.Terminal.App/Views/MainWindow.axaml.cs index a158068..a3a7d16 100644 --- a/src/Devolutions.Terminal.App/Views/MainWindow.axaml.cs +++ b/src/Devolutions.Terminal.App/Views/MainWindow.axaml.cs @@ -188,6 +188,20 @@ public MainWindow( { UpdateFullscreenChrome(); } + + // Restoring from maximized brings back the bounds the OS remembered, + // which may no longer fit the displays. + if (args.Property == WindowStateProperty && WindowState == WindowState.Normal) + { + Dispatcher.UIThread.Post(FitNormalBoundsToScreens, DispatcherPriority.Background); + } + }; + Screens.Changed += (_, _) => + { + // Windows moves windows off a removed display shortly after reporting + // the change; fit again once that has settled. + Dispatcher.UIThread.Post(FitNormalBoundsToScreens, DispatcherPriority.Background); + DispatcherTimer.RunOnce(FitNormalBoundsToScreens, TimeSpan.FromSeconds(1)); }; AddHandler(KeyDownEvent, OnWindowKeyDown, RoutingStrategies.Tunnel); AddHandler(TextInputEvent, OnWindowTextInput, RoutingStrategies.Tunnel); @@ -344,6 +358,12 @@ private void ApplyLaunchOptions(TerminalWindowActivation activation) Height = Math.Max(240, (rows * cell.Height) + 16 + titleBarHeight); } + // Without a requested position the OS places the window on a display. + if (activation.PositionX is not null && activation.PositionY is not null) + { + FitNormalBoundsToScreens(); + } + if (activation.LaunchMode.HasFlag(TerminalWindowLaunchMode.Fullscreen)) { WindowState = WindowState.FullScreen; @@ -3935,6 +3955,60 @@ private void MainWindow_OnSizeChanged(object? sender, SizeChangedEventArgs e) RebuildTabs(); } + /// + /// Fits the normal-state bounds to the displays that exist now (see + /// ). A normal window moves and shrinks at + /// once; a maximized or full-screen one keeps its state, and the saved normal + /// bounds it would restore to are fitted instead. Minimized windows sit at an + /// off-screen placeholder position and are left alone. + /// + private void FitNormalBoundsToScreens() + { + if (WindowState == WindowState.Minimized) + { + return; + } + + var workAreas = Screens.All.Select(static screen => screen.WorkingArea).ToArray(); + var normal = WindowState == WindowState.Normal; + if (workAreas.Length == 0 || (!normal && _normalPosition is null)) + { + return; + } + + var position = normal ? Position : _normalPosition!.Value; + var width = normal ? (double.IsNaN(Width) ? Bounds.Width : Width) : _normalSize.Width; + var height = normal ? (double.IsNaN(Height) ? Bounds.Height : Height) : _normalSize.Height; + var scale = ScalingAt(position); + var bounds = new PixelRect( + position.X, + position.Y, + Math.Max(1, (int)Math.Round(width * scale)), + Math.Max(1, (int)Math.Round(height * scale))); + var fitted = WindowPlacement.Fit(bounds, workAreas); + if (fitted == bounds) + { + return; + } + + var fittedScale = ScalingAt(fitted.Position); + _normalPosition = fitted.Position; + _normalSize = new WindowSizeState + { + Width = fitted.Width / fittedScale, + Height = fitted.Height / fittedScale, + }; + if (normal) + { + Position = fitted.Position; + Width = _normalSize.Width; + Height = _normalSize.Height; + } + + double ScalingAt(PixelPoint point) => + Screens.ScreenFromPoint(point)?.Scaling ?? Screens.Primary?.Scaling ?? 1; + } + private void CaptureNormalWindowBounds() { if (WindowState != WindowState.Normal) @@ -4253,6 +4327,14 @@ private void ApplyPersistedWindowState(WindowLayoutState? state) Height = _normalSize.Height; } + // A saved position may belong to a display that is gone; fit before + // maximizing, so the window maximizes on a display that exists. Without + // one the OS places the window on a display. + if (_normalPosition is not null) + { + FitNormalBoundsToScreens(); + } + switch (state?.LaunchMode) { case LaunchMode.Maximized: diff --git a/tests/Devolutions.Terminal.App.Tests/WindowPlacementTests.cs b/tests/Devolutions.Terminal.App.Tests/WindowPlacementTests.cs new file mode 100644 index 0000000..03b9274 --- /dev/null +++ b/tests/Devolutions.Terminal.App.Tests/WindowPlacementTests.cs @@ -0,0 +1,89 @@ +using Avalonia; +using Devolutions.Terminal.App.Platform; +using Xunit; + +namespace Devolutions.Terminal.App.Tests; + +public sealed class WindowPlacementTests +{ + // A 1920x1080 laptop panel with a 48-pixel taskbar, and a 2560x1440 external + // display to its right. + private static readonly PixelRect Laptop = new(0, 0, 1920, 1032); + private static readonly PixelRect External = new(1920, 0, 2560, 1392); + + [Fact] + public void LeavesWindowInsideOneDisplayAlone() + { + var bounds = new PixelRect(100, 80, 1200, 800); + Assert.Equal(bounds, WindowPlacement.Fit(bounds, [Laptop, External])); + } + + [Fact] + public void LeavesWindowSpanningConnectedDisplaysAlone() + { + var bounds = new PixelRect(1500, 100, 1000, 800); + Assert.Equal(bounds, WindowPlacement.Fit(bounds, [Laptop, External])); + } + + [Fact] + public void ShrinksWindowLargerThanTheRemainingDisplay() + { + // Sized for the external display, then moved onto the laptop panel by + // Windows when the external one was disconnected. + var bounds = new PixelRect(100, 50, 2400, 1300); + Assert.Equal(Laptop, WindowPlacement.Fit(bounds, [Laptop])); + } + + [Fact] + public void MovesWindowPartlyOffTheEdgeBackInside() + { + var bounds = new PixelRect(1500, 600, 800, 600); + Assert.Equal(new PixelRect(1120, 432, 800, 600), WindowPlacement.Fit(bounds, [Laptop])); + } + + [Fact] + public void MovesWindowOnARemovedDisplayToTheNearestOne() + { + // Restored at a position on the external display, which is gone. + var bounds = new PixelRect(2500, 200, 1200, 800); + Assert.Equal(new PixelRect(720, 200, 1200, 800), WindowPlacement.Fit(bounds, [Laptop])); + + // With a display left of the laptop too, the laptop is still the nearest. + var left = new PixelRect(-2560, 0, 2560, 1392); + Assert.Equal(new PixelRect(720, 200, 1200, 800), WindowPlacement.Fit(bounds, [left, Laptop])); + } + + [Fact] + public void FitsIntoTheDisplayItOverlapsMostWhenPartIsOnNoDisplay() + { + // The external display is taller, so the bottom of a window spanning both + // hangs below the laptop panel, where there is no display. + var bounds = new PixelRect(1700, 600, 1000, 700); + Assert.Equal(new PixelRect(1920, 600, 1000, 700), WindowPlacement.Fit(bounds, [Laptop, External])); + } + + [Fact] + public void HandlesDisplaysLeftOfAndAbovePrimary() + { + var left = new PixelRect(-2560, -360, 2560, 1392); + var bounds = new PixelRect(-3000, -500, 1000, 700); + Assert.Equal(new PixelRect(-2560, -360, 1000, 700), WindowPlacement.Fit(bounds, [left, Laptop])); + } + + [Fact] + public void CountsMirroredDisplaysOnce() + { + // Two displays mirroring the same area would otherwise cover the off-screen + // half of the window twice over. + var bounds = new PixelRect(1400, 100, 1000, 700); + Assert.Equal(new PixelRect(920, 100, 1000, 700), WindowPlacement.Fit(bounds, [Laptop, Laptop])); + } + + [Fact] + public void LeavesBoundsAloneWithoutDisplayInformation() + { + var bounds = new PixelRect(5000, 5000, 800, 600); + Assert.Equal(bounds, WindowPlacement.Fit(bounds, [])); + Assert.Equal(bounds, WindowPlacement.Fit(bounds, [new PixelRect(0, 0, 0, 0)])); + } +}