diff --git a/api/Avalonia.nupkg.xml b/api/Avalonia.nupkg.xml new file mode 100644 index 0000000000..ac3e50889b --- /dev/null +++ b/api/Avalonia.nupkg.xml @@ -0,0 +1,28 @@ + + + + + CP0002 + M:Avalonia.Media.TextFormatting.ShapedBuffer.Reverse + baseline/Avalonia/lib/net10.0/Avalonia.Base.dll + current/Avalonia/lib/net10.0/Avalonia.Base.dll + + + CP0002 + M:Avalonia.Media.TextFormatting.ShapedTextRun.get_IsReversed + baseline/Avalonia/lib/net10.0/Avalonia.Base.dll + current/Avalonia/lib/net10.0/Avalonia.Base.dll + + + CP0002 + M:Avalonia.Media.TextFormatting.ShapedBuffer.Reverse + baseline/Avalonia/lib/net8.0/Avalonia.Base.dll + current/Avalonia/lib/net8.0/Avalonia.Base.dll + + + CP0002 + M:Avalonia.Media.TextFormatting.ShapedTextRun.get_IsReversed + baseline/Avalonia/lib/net8.0/Avalonia.Base.dll + current/Avalonia/lib/net8.0/Avalonia.Base.dll + + diff --git a/src/Avalonia.Base/Media/TextFormatting/BidiReorderer.cs b/src/Avalonia.Base/Media/TextFormatting/BidiReorderer.cs index 0612ecf8e0..3b459cb69c 100644 --- a/src/Avalonia.Base/Media/TextFormatting/BidiReorderer.cs +++ b/src/Avalonia.Base/Media/TextFormatting/BidiReorderer.cs @@ -70,67 +70,11 @@ namespace Avalonia.Media.TextFormatting firstTextSourceIndex += currentRun.Length; } - // Now perform a recursive reversal of each run. - // From the highest level found in the text to the lowest odd level on each line, including intermediate levels - // not actually present in the text, reverse any contiguous sequence of characters that are at that level or higher. - // https://unicode.org/reports/tr9/#L2 - sbyte max = 0; - var min = sbyte.MaxValue; - - for (var i = 0; i < textRuns.Length; i++) - { - var level = _runs[i].Level; - - if (level > max) - { - max = level; - } - - if ((level & 1) != 0 && level < min) - { - min = level; - } - } - - if (min > max) - { - min = max; - } - - if (max == 0 || (min == max && (max & 1) == 0)) - { - // Nothing to reverse. - return indexedTextRuns; - } - - // Now apply the reversal and replace the original contents. - var minLevelToReverse = max; - int currentIndex; - - while (minLevelToReverse >= min) - { - currentIndex = firstIndex; - - while (currentIndex >= 0) - { - ref var current = ref _runs[currentIndex]; - if (current.Level >= minLevelToReverse && current.Level % 2 != 0) - { - if (current.Run is ShapedTextRun { IsReversed: false } shapedTextCharacters) - { - shapedTextCharacters.Reverse(); - } - } - - currentIndex = current.NextRunIndex; - } - - minLevelToReverse--; - } - + // Shape-time already produces glyphs in visual order (RTL buffers have descending + // clusters), so L2 reversal of glyphs is no longer needed here — we only shuffle + // the run span into visual order. var index = 0; - - currentIndex = firstIndex; + var currentIndex = firstIndex; while (currentIndex >= 0) { diff --git a/src/Avalonia.Base/Media/TextFormatting/ShapedBuffer.cs b/src/Avalonia.Base/Media/TextFormatting/ShapedBuffer.cs index 4b5e4cad84..3643d7807b 100644 --- a/src/Avalonia.Base/Media/TextFormatting/ShapedBuffer.cs +++ b/src/Avalonia.Base/Media/TextFormatting/ShapedBuffer.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Buffers; using System.Collections; using System.Collections.Generic; @@ -17,7 +17,7 @@ namespace Avalonia.Media.TextFormatting { Text = text; _rentedBuffer = ArrayPool.Shared.Rent(bufferLength); - _glyphInfos = new ArraySlice(_rentedBuffer, 0, bufferLength); + _glyphInfos = new ArraySlice(_rentedBuffer, 0, bufferLength); GlyphTypeface = glyphTypeface; FontRenderingEmSize = fontRenderingEmSize; BidiLevel = bidiLevel; @@ -55,7 +55,7 @@ namespace Avalonia.Media.TextFormatting /// /// The buffer's bidi level. /// - public sbyte BidiLevel { get; private set; } + public sbyte BidiLevel { get; } /// /// The buffer's reading direction. @@ -66,14 +66,6 @@ namespace Avalonia.Media.TextFormatting /// The text that is represended by this buffer. /// public ReadOnlyMemory Text { get; } - - /// - /// Reverses the buffer. - /// - public void Reverse() - { - _glyphInfos.Span.Reverse(); - } public void Dispose() { @@ -95,7 +87,20 @@ namespace Avalonia.Media.TextFormatting public IEnumerator GetEnumerator() => _glyphInfos.GetEnumerator(); - internal void ResetBidiLevel(sbyte paragraphEmbeddingLevel) => BidiLevel = paragraphEmbeddingLevel; + /// + /// Creates a view of this buffer that reports the given + /// as its while sharing the same underlying glyphs. + /// Used for trailing-whitespace handling where the visual direction follows the paragraph. + /// + internal ShapedBuffer WithBidiLevel(sbyte paragraphEmbeddingLevel) + { + if (BidiLevel == paragraphEmbeddingLevel) + { + return this; + } + + return new ShapedBuffer(Text, _glyphInfos, GlyphTypeface, FontRenderingEmSize, paragraphEmbeddingLevel); + } int IReadOnlyCollection.Count => _glyphInfos.Length; @@ -126,25 +131,30 @@ namespace Avalonia.Media.TextFormatting return new SplitResult(this, null); } + return IsLeftToRight ? SplitAscending(textLength) : SplitDescending(textLength); + } + + /// + /// Split a buffer whose glyphs are ordered by ascending cluster (LTR visual / logical order). + /// + private SplitResult SplitAscending(int textLength) + { var sliceStart = _glyphInfos.Start; var glyphInfos = _glyphInfos.Span; var glyphInfosLength = _glyphInfos.Length; - // the first glyph’s cluster is our “zero” for this sub‐buffer. - // we want an absolute target cluster = baseCluster + textLength + // the first glyph’s cluster is our “zero” for this sub-buffer. var baseCluster = glyphInfos[0].GlyphCluster; var targetCluster = baseCluster + textLength; - // binary‐search for a dummy with cluster == targetCluster var searchValue = new GlyphInfo(0, targetCluster, 0, default); var foundIndex = glyphInfos.BinarySearch(searchValue, GlyphInfo.ClusterAscendingComparer); - int splitGlyphIndex; // how many glyph‐slots go into "leading" - int splitCharCount; // how many chars go into "leading" Text + int splitGlyphIndex; + int splitCharCount; if (foundIndex >= 0) { - // found a glyph info whose cluster == targetCluster // back up to the start of the cluster var i = foundIndex; @@ -152,26 +162,21 @@ namespace Avalonia.Media.TextFormatting { i--; } - + splitGlyphIndex = i; splitCharCount = targetCluster - baseCluster; } else { - // no exact match need to invert so ~foundIndex is the insertion point - // the first cluster > targetCluster var invertedIndex = ~foundIndex; if (invertedIndex >= glyphInfosLength) { - // happens only if targetCluster ≥ lastCluster - // put everything into leading splitGlyphIndex = glyphInfosLength; splitCharCount = Text.Length; } else { - // snap to the start of that next cluster splitGlyphIndex = invertedIndex; var nextCluster = glyphInfos[invertedIndex].GlyphCluster; splitCharCount = nextCluster - baseCluster; @@ -188,8 +193,7 @@ namespace Avalonia.Media.TextFormatting firstText, firstGlyphs, GlyphTypeface, FontRenderingEmSize, BidiLevel); - // this happens if we try to find a position inside a cluster and we moved to the end - if(secondText.Length == 0) + if (secondText.Length == 0) { return new SplitResult(leading, null); } @@ -200,5 +204,69 @@ namespace Avalonia.Media.TextFormatting return new SplitResult(leading, trailing); } + + /// + /// Split a buffer whose glyphs are ordered by descending cluster (RTL visual order). + /// The leading corresponds to text[0..textLength], whose glyphs + /// live at the tail of the visual array. + /// + private SplitResult SplitDescending(int textLength) + { + var sliceStart = _glyphInfos.Start; + var glyphInfos = _glyphInfos.Span; + var glyphInfosLength = _glyphInfos.Length; + + // Cluster values are anchored to the original text and can start from any value + // after a previous split. The logical-first cluster of this buffer is at the + // tail of the visual array (smallest cluster). The split happens at logical + // offset textLength from there. + var baseCluster = glyphInfos[glyphInfosLength - 1].GlyphCluster; + var targetCluster = baseCluster + textLength; + + var searchValue = new GlyphInfo(0, targetCluster, 0, default); + var foundIndex = glyphInfos.BinarySearch(searchValue, GlyphInfo.ClusterDescendingComparer); + + int splitGlyphIndex; // boundary between "second" (head/leading-visual) and "first" (tail/trailing-visual) + + if (foundIndex >= 0) + { + // Snap to the last glyph that still has cluster == targetCluster; that glyph + // belongs to the trailing chunk (text[textLength..]) — i.e. still "second". + while (foundIndex + 1 < glyphInfosLength + && glyphInfos[foundIndex + 1].GlyphCluster == targetCluster) + { + foundIndex++; + } + + splitGlyphIndex = foundIndex + 1; + } + else + { + splitGlyphIndex = ~foundIndex; + } + + // Visual leading = glyphs [0, splitGlyphIndex) → logically text[textLength..] (our "second") + // Visual trailing = glyphs [splitGlyphIndex, end) → logically text[0..textLength] (our "first") + var secondGlyphs = _glyphInfos.Slice(sliceStart, splitGlyphIndex); + var firstGlyphs = _glyphInfos.Slice(sliceStart + splitGlyphIndex, glyphInfosLength - splitGlyphIndex); + + var firstText = Text.Slice(0, textLength); + var secondText = Text.Slice(textLength); + + var first = new ShapedBuffer( + firstText, firstGlyphs, + GlyphTypeface, FontRenderingEmSize, BidiLevel); + + if (secondText.Length == 0 || secondGlyphs.Length == 0) + { + return new SplitResult(first, null); + } + + var second = new ShapedBuffer( + secondText, secondGlyphs, + GlyphTypeface, FontRenderingEmSize, BidiLevel); + + return new SplitResult(first, second); + } } } diff --git a/src/Avalonia.Base/Media/TextFormatting/ShapedTextRun.cs b/src/Avalonia.Base/Media/TextFormatting/ShapedTextRun.cs index 4b38a58cc6..a75734f356 100644 --- a/src/Avalonia.Base/Media/TextFormatting/ShapedTextRun.cs +++ b/src/Avalonia.Base/Media/TextFormatting/ShapedTextRun.cs @@ -1,4 +1,5 @@ -using System; +using System; +using System.Threading; using Avalonia.Media.TextFormatting.Unicode; namespace Avalonia.Media.TextFormatting @@ -6,9 +7,22 @@ namespace Avalonia.Media.TextFormatting /// /// A text run that holds shaped characters. /// + /// + /// Glyph data in the underlying is immutable after shaping: + /// LTR buffers are in ascending-cluster (logical) order, RTL buffers are in + /// descending-cluster (visual) order. only reorders runs, + /// it never mutates glyph order. + /// + /// Ref-counted: the initial constructor call establishes one reference. Call + /// when taking an additional reference (e.g., when a + /// stores a shaped run a formatter is also about to use), + /// and to release. The underlying shaped buffer is disposed only + /// when the last reference is released. + /// public sealed class ShapedTextRun : DrawableTextRun, IDisposable { private GlyphRun? _glyphRun; + private int _refCount = 1; public ShapedTextRun(ShapedBuffer shapedBuffer, TextRunProperties properties) { @@ -17,8 +31,6 @@ namespace Avalonia.Media.TextFormatting TextMetrics = new TextMetrics(properties.CachedGlyphTypeface, properties.FontRenderingEmSize); } - public bool IsReversed { get; private set; } - public sbyte BidiLevel => ShapedBuffer.BidiLevel; public ShapedBuffer ShapedBuffer { get; } @@ -42,6 +54,15 @@ namespace Avalonia.Media.TextFormatting public GlyphRun GlyphRun => _glyphRun ??= CreateGlyphRun(); + /// + /// Takes an additional reference to this run. Must be paired with . + /// + internal ShapedTextRun AddRef() + { + Interlocked.Increment(ref _refCount); + return this; + } + /// public override void Draw(DrawingContext drawingContext, Point origin) { @@ -81,15 +102,6 @@ namespace Avalonia.Media.TextFormatting } } - internal void Reverse() - { - _glyphRun = null; - - ShapedBuffer.Reverse(); - - IsReversed = !IsReversed; - } - /// /// Measures the number of characters that fit into available width. /// @@ -101,10 +113,23 @@ namespace Avalonia.Media.TextFormatting public bool TryMeasureCharacters(double availableWidth, out int length) { length = 0; + + if (ShapedBuffer.Length == 0) + { + return false; + } + var currentWidth = 0.0; var charactersSpan = GlyphRun.Characters.Span; + var isLeftToRight = ShapedBuffer.IsLeftToRight; + var bufferLength = ShapedBuffer.Length; + var textLength = Text.Length; + + // Previous visual glyph's cluster — used in RTL mode to compute the char count + // contributed by the current glyph (which spans [currentCluster, prevCluster) logically). + var previousCluster = 0; - for (var i = 0; i < ShapedBuffer.Length; i++) + for (var i = 0; i < bufferLength; i++) { var advance = ShapedBuffer[i].GlyphAdvance; var currentCluster = ShapedBuffer[i].GlyphCluster; @@ -114,23 +139,35 @@ namespace Avalonia.Media.TextFormatting break; } - if(i + 1 < ShapedBuffer.Length) - { - var nextCluster = ShapedBuffer[i + 1].GlyphCluster; - - var count = nextCluster - currentCluster; + int count; - length += count; + if (isLeftToRight) + { + if (i + 1 < bufferLength) + { + var nextCluster = ShapedBuffer[i + 1].GlyphCluster; + count = nextCluster - currentCluster; + } + else + { + Codepoint.ReadAt(charactersSpan, length, out count); + } } else { - Codepoint.ReadAt(charactersSpan, length, out var count); - - length += count; + if (i == 0) + { + count = textLength - currentCluster; + } + else + { + count = previousCluster - currentCluster; + } } - + length += count; currentWidth += advance; + previousCluster = currentCluster; } return length > 0; @@ -162,20 +199,11 @@ namespace Avalonia.Media.TextFormatting internal SplitResult Split(int length) { - var isReversed = IsReversed; - - if (isReversed) - { - Reverse(); - - length = Length - length; - } - if (length == 0) { throw new ArgumentOutOfRangeException(nameof(length), "length must be greater than zero."); } - + var splitBuffer = ShapedBuffer.Split(length); // first cannot be null as length > 0 @@ -188,19 +216,10 @@ namespace Avalonia.Media.TextFormatting if (splitBuffer.Second == null) { - // If there's no second part, return the entire run as the second in reversed mode, or throw - if (isReversed) - { - return new SplitResult(null, first); - } - throw new InvalidOperationException($"Cannot split: requested length {length} consumes entire run."); + return new SplitResult(first, null); } - var second = new ShapedTextRun(splitBuffer.Second, Properties); - if (isReversed) - { - return new SplitResult(second, first); - } + var second = new ShapedTextRun(splitBuffer.Second, Properties); return new SplitResult(first, second); } @@ -217,6 +236,11 @@ namespace Avalonia.Media.TextFormatting public void Dispose() { + if (Interlocked.Decrement(ref _refCount) != 0) + { + return; + } + _glyphRun?.Dispose(); ShapedBuffer.Dispose(); } diff --git a/src/Avalonia.Base/Media/TextFormatting/TextEllipsisHelper.cs b/src/Avalonia.Base/Media/TextFormatting/TextEllipsisHelper.cs index 685bb08f27..184f86f5ad 100644 --- a/src/Avalonia.Base/Media/TextFormatting/TextEllipsisHelper.cs +++ b/src/Avalonia.Base/Media/TextFormatting/TextEllipsisHelper.cs @@ -35,11 +35,6 @@ namespace Avalonia.Media.TextFormatting if (textRunWidth > availableWidth) { - if (shapedRun.IsReversed) - { - shapedRun.Reverse(); - } - if (shapedRun.TryMeasureCharacters(availableWidth, out var measuredLength)) { if (isWordEllipsis && measuredLength < textLine.Length) diff --git a/src/Avalonia.Base/Media/TextFormatting/TextFormatterImpl.cs b/src/Avalonia.Base/Media/TextFormatting/TextFormatterImpl.cs index 8af3470dd1..fe5e3ae8b0 100644 --- a/src/Avalonia.Base/Media/TextFormatting/TextFormatterImpl.cs +++ b/src/Avalonia.Base/Media/TextFormatting/TextFormatterImpl.cs @@ -71,7 +71,8 @@ namespace Avalonia.Media.TextFormatting nextLineBreak = new TextLineBreak(textEndOfLine, resolvedFlowDirection); } - // Store shaped runs in cache for reuse. + // Store shaped runs in cache for reuse. The cache takes its own references; + // the formatter keeps the fresh-from-shape references for the current line. if (textRunCache != null) { textRunCache.Add(firstTextSourceIndex, @@ -83,18 +84,7 @@ namespace Avalonia.Media.TextFormatting { case TextWrapping.NoWrap: { - TextRun[] lineRuns; - - if (textRunCache != null) - { - // When caching, the cache owns the shaped runs. - // Create non-owning copies for the line. - lineRuns = CreateNonOwningRuns(shapedTextRuns); - } - else - { - lineRuns = shapedTextRuns.ToArray(); - } + var lineRuns = shapedTextRuns.ToArray(); var textLine = new TextLineImpl(lineRuns, firstTextSourceIndex, textSourceLength, @@ -107,16 +97,6 @@ namespace Avalonia.Media.TextFormatting case TextWrapping.WrapWithOverflow: case TextWrapping.Wrap: { - if (textRunCache != null) - { - // When caching, create non-owning copies for wrapping. - var nonOwningRuns = CreateNonOwningRunsList(shapedTextRuns); - - return PerformTextWrapping(nonOwningRuns, false, firstTextSourceIndex, - paragraphWidth, paragraphProperties, resolvedFlowDirection, - nextLineBreak, objectPool); - } - return PerformTextWrapping(shapedTextRuns, false, firstTextSourceIndex, paragraphWidth, paragraphProperties, resolvedFlowDirection, nextLineBreak, objectPool); } @@ -150,7 +130,7 @@ namespace Avalonia.Media.TextFormatting { case TextWrapping.NoWrap: { - var lineRuns = CreateNonOwningRuns(cached.ShapedRuns); + var lineRuns = AddRefShapedRuns(cached.ShapedRuns); var textLine = new TextLineImpl(lineRuns, firstTextSourceIndex, cached.TextSourceLength, @@ -163,9 +143,16 @@ namespace Avalonia.Media.TextFormatting case TextWrapping.WrapWithOverflow: case TextWrapping.Wrap: { - var nonOwningRuns = CreateNonOwningRunsList(cached.ShapedRuns); + var runs = new List(cached.ShapedRuns.Length); - return PerformTextWrapping(nonOwningRuns, false, firstTextSourceIndex, + for (var i = 0; i < cached.ShapedRuns.Length; i++) + { + runs.Add(cached.ShapedRuns[i] is ShapedTextRun shaped + ? shaped.AddRef() + : cached.ShapedRuns[i]); + } + + return PerformTextWrapping(runs, false, firstTextSourceIndex, paragraphWidth, paragraphProperties, resolvedFlowDirection, nextLineBreak, objectPool); } @@ -175,49 +162,16 @@ namespace Avalonia.Media.TextFormatting } /// - /// Creates a non-owning copy of a text run. For shaped text runs, this creates - /// a new instance with a separate shaped buffer so that disposing the copy does - /// not dispose the original cached shaped buffer. - /// - private static TextRun CreateNonOwningRun(TextRun run) - { - if (run is ShapedTextRun shaped) - { - var buf = shaped.ShapedBuffer; - return new ShapedTextRun( - new ShapedBuffer(buf.Text, buf.GlyphInfos, buf.GlyphTypeface, buf.FontRenderingEmSize, buf.BidiLevel), - shaped.Properties); - } - - return run; - } - - /// - /// Creates non-owning copies of shaped text runs for use in text lines, - /// so that disposing the line does not dispose the cached shaped buffers. + /// Produces an array of text runs for a line, adding an extra reference to each + /// so that the caller owns a disposable reference. /// - private static TextRun[] CreateNonOwningRuns(IReadOnlyList runs) + private static TextRun[] AddRefShapedRuns(IReadOnlyList runs) { var result = new TextRun[runs.Count]; for (var i = 0; i < runs.Count; i++) { - result[i] = CreateNonOwningRun(runs[i]); - } - - return result; - } - - /// - /// Creates a non-owning list of shaped text runs for use with text wrapping. - /// - private static List CreateNonOwningRunsList(IReadOnlyList runs) - { - var result = new List(runs.Count); - - for (var i = 0; i < runs.Count; i++) - { - result.Add(CreateNonOwningRun(runs[i])); + result[i] = runs[i] is ShapedTextRun shaped ? shaped.AddRef() : runs[i]; } return result; @@ -309,6 +263,10 @@ namespace Avalonia.Media.TextFormatting { second.Add(split.Second); } + + // The split produced fresh ShapedTextRuns for each half, so the + // caller's reference to the original is no longer needed — release it. + shapedTextCharacters.Dispose(); } for (var j = 1; j < secondCount; j++) @@ -764,43 +722,53 @@ namespace Avalonia.Media.TextFormatting { if (shapedTextCharacters.ShapedBuffer.Length > 0) { + var bufferLength = shapedTextCharacters.ShapedBuffer.Length; var runLength = 0; - - for (var j = 0; j < shapedTextCharacters.ShapedBuffer.Length; j++) + var runTextLength = shapedTextCharacters.Length; + var isLeftToRight = shapedTextCharacters.ShapedBuffer.IsLeftToRight; + + // Cluster values stay anchored to the original text even after + // splits, so anchor the run's logical end from FirstCluster. + var logicalEnd = shapedTextCharacters.GlyphRun.Metrics.FirstCluster + runTextLength; + + // Walk in LOGICAL order: LTR is already logical (ascending + // clusters from j=0 onwards), RTL visual is reverse-logical so + // we iterate from the tail. The algorithm below reads + // `currentInfo` / `nextInfo` in logical progression — next + // always means "the next glyph logically after the current". + int step = isLeftToRight ? 1 : -1; + int start = isLeftToRight ? 0 : bufferLength - 1; + int end = isLeftToRight ? bufferLength : -1; + + for (var j = start; j != end; j += step) { var currentInfo = shapedTextCharacters.ShapedBuffer[j]; var clusterWidth = currentInfo.GlyphAdvance; GlyphInfo nextInfo = default; + var hasNext = false; - while (j + 1 < shapedTextCharacters.ShapedBuffer.Length) + // Collect additional glyphs belonging to the same cluster. + while ((isLeftToRight ? j + 1 < bufferLength : j - 1 >= 0)) { - nextInfo = shapedTextCharacters.ShapedBuffer[j + 1]; + nextInfo = shapedTextCharacters.ShapedBuffer[j + step]; if (currentInfo.GlyphCluster == nextInfo.GlyphCluster) { clusterWidth += nextInfo.GlyphAdvance; - j++; + j += step; continue; } + hasNext = true; break; } - var clusterLength = Math.Max(0, nextInfo.GlyphCluster - currentInfo.GlyphCluster); - - if (clusterLength == 0) - { - clusterLength = currentRun.Length - runLength; - } - - if (clusterLength == 0) - { - clusterLength = shapedTextCharacters.GlyphRun.Metrics.FirstCluster + currentRun.Length - currentInfo.GlyphCluster; - } + var nextLogicalCluster = hasNext ? nextInfo.GlyphCluster : logicalEnd; + var clusterLength = nextLogicalCluster - currentInfo.GlyphCluster; if (MathUtilities.GreaterThan(currentWidth + clusterWidth, paragraphWidth)) { @@ -1174,7 +1142,13 @@ namespace Avalonia.Media.TextFormatting { if (trailingWhitespaceRuns[i] is ShapedTextRun shapedTextRun) { - shapedTextRun.ShapedBuffer.ResetBidiLevel(paragraphEmbeddingLevel); + var newBuffer = shapedTextRun.ShapedBuffer.WithBidiLevel(paragraphEmbeddingLevel); + + if (!ReferenceEquals(newBuffer, shapedTextRun.ShapedBuffer)) + { + trailingWhitespaceRuns[i] = new ShapedTextRun(newBuffer, shapedTextRun.Properties); + shapedTextRun.Dispose(); + } } } diff --git a/src/Avalonia.Base/Media/TextFormatting/TextRunCache.cs b/src/Avalonia.Base/Media/TextFormatting/TextRunCache.cs index 8dc21fa387..d2545f42e2 100644 --- a/src/Avalonia.Base/Media/TextFormatting/TextRunCache.cs +++ b/src/Avalonia.Base/Media/TextFormatting/TextRunCache.cs @@ -125,20 +125,27 @@ namespace Avalonia.Media.TextFormatting } /// - /// Adds shaped runs to the cache for the given text source index. + /// Adds shaped runs to the cache for the given text source index. The cache takes + /// its own reference to each ; the caller retains its + /// original references unchanged. /// internal void Add(int firstTextSourceIndex, CachedShapingResult result) { + AddRefShapedRuns(result.ShapedRuns); + if (_entries != null) { - // Already in multi-entry mode. + if (_entries.TryGetValue(firstTextSourceIndex, out var existing)) + { + DisposeCachedRuns(existing); + } + _entries[firstTextSourceIndex] = result; return; } if (!_hasSingleEntry) { - // First entry: store inline. _singleKey = firstTextSourceIndex; _singleValue = result; _hasSingleEntry = true; @@ -147,7 +154,7 @@ namespace Avalonia.Media.TextFormatting if (_singleKey == firstTextSourceIndex) { - // Same key: update inline value. + DisposeCachedRuns(_singleValue); _singleValue = result; return; } @@ -162,6 +169,17 @@ namespace Avalonia.Media.TextFormatting _singleValue = default; } + private static void AddRefShapedRuns(TextRun[] runs) + { + for (var i = 0; i < runs.Length; i++) + { + if (runs[i] is ShapedTextRun shaped) + { + shaped.AddRef(); + } + } + } + /// public void Dispose() { diff --git a/src/HarfBuzz/Avalonia.HarfBuzz/HarfBuzzTextShaper.cs b/src/HarfBuzz/Avalonia.HarfBuzz/HarfBuzzTextShaper.cs index b1bc41237c..77b3ac7b08 100644 --- a/src/HarfBuzz/Avalonia.HarfBuzz/HarfBuzzTextShaper.cs +++ b/src/HarfBuzz/Avalonia.HarfBuzz/HarfBuzzTextShaper.cs @@ -63,10 +63,11 @@ namespace Avalonia.Harfbuzz font.Shape(buffer, GetFeatures(options)); - if (buffer.Direction == Direction.RightToLeft) - { - buffer.Reverse(); - } + // HarfBuzz produces glyphs in visual order for RTL by default: the first glyph + // in the buffer is the leftmost visual glyph (highest cluster value). LTR output + // already has clusters in ascending (logical = visual) order. We preserve that + // order in the ShapedBuffer so downstream rendering/hit-testing can operate on + // visual-order glyphs without an extra bidi reversal step. font.GetScale(out var scaleX, out _); diff --git a/tests/Avalonia.Skia.UnitTests/Media/GlyphRunTests.cs b/tests/Avalonia.Skia.UnitTests/Media/GlyphRunTests.cs index 8f74f0779f..4aaaa9bf38 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/GlyphRunTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/GlyphRunTests.cs @@ -455,19 +455,12 @@ namespace Avalonia.Skia.UnitTests.Media private static GlyphRun CreateGlyphRun(ShapedBuffer shapedBuffer) { - var glyphRun = new GlyphRun( + return new GlyphRun( shapedBuffer.GlyphTypeface, shapedBuffer.FontRenderingEmSize, shapedBuffer.Text, shapedBuffer, biDiLevel: shapedBuffer.BidiLevel); - - if (shapedBuffer.BidiLevel == 1) - { - shapedBuffer.Reverse(); - } - - return glyphRun; } private static IDisposable Start() diff --git a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs index 7b58320e17..6af9761202 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs @@ -105,7 +105,7 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting var runClusters = shapedRun.ShapedBuffer.Select(glyph => glyph.GlyphCluster + runOffset); - clusters.AddRange(shapedRun.IsReversed ? runClusters.Reverse() : runClusters); + clusters.AddRange(shapedRun.ShapedBuffer.IsLeftToRight ? runClusters : runClusters.Reverse()); } var nextCharacterHit = new CharacterHit(0, clusters[1] - clusters[0]); @@ -155,7 +155,7 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting var runClusters = shapedRun.ShapedBuffer.Select(glyph => glyph.GlyphCluster + runOffset); - clusters.AddRange(shapedRun.IsReversed ? runClusters.Reverse() : runClusters); + clusters.AddRange(shapedRun.ShapedBuffer.IsLeftToRight ? runClusters : runClusters.Reverse()); } clusters.Reverse(); diff --git a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextRunCacheTests.cs b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextRunCacheTests.cs index e8a2a78fa3..4ef819943c 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextRunCacheTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextRunCacheTests.cs @@ -227,6 +227,234 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting } } + // Regression tests for the double-reorder bug (commit 2837a287): + // When shaped runs with shared ShapedBuffer backing arrays were cached, the old + // BidiReorderer.Reverse() call on a non-owning copy mutated the same backing array + // that the cached run referenced. On the next layout the cache returned the already- + // reversed buffer but with IsReversed=false, causing BidiReorderer to reverse it a + // second time – putting glyphs back in logical (wrong visual) order for RTL runs. + + [Fact] + public void Bidi_Cache_Hit_Does_Not_Double_Reorder_RTL_Glyph_Clusters() + { + using (Start()) + { + // LTR paragraph containing an Arabic RTL island followed by Latin text. + // "Hello مرحبا World" – the Arabic word is at source indices 6-10. + var text = "Hello \u0645\u0631\u062D\u0628\u0627 World"; + var defaultProperties = new GenericTextRunProperties(Typeface.Default); + var paragraphProperties = new GenericTextParagraphProperties(defaultProperties); + var textSource = new SingleBufferTextSource(text, defaultProperties); + var formatter = new TextFormatterImpl(); + + using var cache = new TextRunCache(); + + // Cache miss: shapes and caches. + var lineMiss = formatter.FormatLine(textSource, 0, double.PositiveInfinity, + paragraphProperties, null, cache); + + Assert.NotNull(lineMiss); + + // Cache hit: must not double-reverse the RTL run's glyph buffer. + var lineHit = formatter.FormatLine(textSource, 0, double.PositiveInfinity, + paragraphProperties, null, cache); + + Assert.NotNull(lineHit); + Assert.Equal(lineMiss!.TextRuns.Count, lineHit!.TextRuns.Count); + + for (var i = 0; i < lineMiss.TextRuns.Count; i++) + { + var missRun = lineMiss.TextRuns[i] as ShapedTextRun; + var hitRun = lineHit.TextRuns[i] as ShapedTextRun; + + if (missRun == null || hitRun == null) + continue; + + Assert.Equal(missRun.BidiLevel, hitRun.BidiLevel); + Assert.Equal(missRun.ShapedBuffer.IsLeftToRight, hitRun.ShapedBuffer.IsLeftToRight); + Assert.Equal(missRun.ShapedBuffer.Length, hitRun.ShapedBuffer.Length); + + // For RTL runs the shaper produces glyphs in descending cluster order + // (visual right-to-left). Double-reversal would flip them back to + // ascending order (logical), causing wrong rendering. + if (!missRun.ShapedBuffer.IsLeftToRight && missRun.ShapedBuffer.Length > 1) + { + var missFirst = missRun.ShapedBuffer[0].GlyphCluster; + var missLast = missRun.ShapedBuffer[missRun.ShapedBuffer.Length - 1].GlyphCluster; + Assert.True(missFirst >= missLast, + $"Cache-miss RTL run: expected descending clusters but got first={missFirst} last={missLast}"); + + var hitFirst = hitRun.ShapedBuffer[0].GlyphCluster; + var hitLast = hitRun.ShapedBuffer[hitRun.ShapedBuffer.Length - 1].GlyphCluster; + Assert.True(hitFirst >= hitLast, + $"Cache-hit RTL run: expected descending clusters but got first={hitFirst} last={hitLast}"); + + // The individual cluster values must be identical between miss and hit. + Assert.Equal(missFirst, hitFirst); + Assert.Equal(missLast, hitLast); + } + } + } + } + + [Fact] + public void Bidi_Cache_Hit_Matches_No_Cache_For_Pure_RTL_Paragraph() + { + using (Start()) + { + // Paragraph-level RTL: all text is Arabic so the resolved flow direction is RTL. + // "مرحبا بالعالم" (Hello World in Arabic) + var text = "\u0645\u0631\u062D\u0628\u0627 \u0628\u0627\u0644\u0639\u0627\u0644\u0645"; + var defaultProperties = new GenericTextRunProperties(Typeface.Default); + var paragraphProperties = new GenericTextParagraphProperties(defaultProperties); + var textSource = new SingleBufferTextSource(text, defaultProperties); + var formatter = new TextFormatterImpl(); + + // Reference: formatted without any cache. + var lineNoCache = formatter.FormatLine(textSource, 0, double.PositiveInfinity, + paragraphProperties, null, null); + + Assert.NotNull(lineNoCache); + + using var cache = new TextRunCache(); + + var lineMiss = formatter.FormatLine(textSource, 0, double.PositiveInfinity, + paragraphProperties, null, cache); + + var lineHit = formatter.FormatLine(textSource, 0, double.PositiveInfinity, + paragraphProperties, null, cache); + + Assert.NotNull(lineMiss); + Assert.NotNull(lineHit); + Assert.Equal(lineNoCache!.Length, lineHit!.Length); + Assert.Equal(lineNoCache.TextRuns.Count, lineHit.TextRuns.Count); + + for (var i = 0; i < lineNoCache.TextRuns.Count; i++) + { + var refRun = lineNoCache.TextRuns[i] as ShapedTextRun; + var hitRun = lineHit.TextRuns[i] as ShapedTextRun; + + if (refRun == null || hitRun == null) + continue; + + Assert.Equal(refRun.BidiLevel, hitRun.BidiLevel); + Assert.Equal(refRun.ShapedBuffer.IsLeftToRight, hitRun.ShapedBuffer.IsLeftToRight); + Assert.Equal(refRun.ShapedBuffer.Length, hitRun.ShapedBuffer.Length); + + // Glyph cluster values must be identical: no double-reversal allowed. + for (var j = 0; j < refRun.ShapedBuffer.Length; j++) + { + Assert.Equal(refRun.ShapedBuffer[j].GlyphCluster, + hitRun.ShapedBuffer[j].GlyphCluster); + } + } + } + } + + [Fact] + public void TextLayout_Recreated_From_Cache_With_Bidi_Has_Same_Glyph_Order() + { + using (Start()) + { + // Mixed LTR/RTL text used for two successive TextLayout instances that share a cache. + // Before the fix, the second instance would double-reverse RTL glyph buffers. + var text = "Hello \u0645\u0631\u062D\u0628\u0627 World"; + + using var cache = new TextRunCache(); + + using var layout1 = new TextLayout(text, Typeface.Default, 12, + textRunCache: cache); + + // Second layout: triggers cache-hit path – previously double-reordered RTL runs. + using var layout2 = new TextLayout(text, Typeface.Default, 12, + textRunCache: cache); + + Assert.Equal(layout1.TextLines.Count, layout2.TextLines.Count); + + for (var lineIdx = 0; lineIdx < layout1.TextLines.Count; lineIdx++) + { + var line1 = layout1.TextLines[lineIdx]; + var line2 = layout2.TextLines[lineIdx]; + + Assert.Equal(line1.Length, line2.Length); + Assert.Equal(line1.TextRuns.Count, line2.TextRuns.Count); + + for (var i = 0; i < line1.TextRuns.Count; i++) + { + var run1 = line1.TextRuns[i] as ShapedTextRun; + var run2 = line2.TextRuns[i] as ShapedTextRun; + + if (run1 == null || run2 == null) + continue; + + Assert.Equal(run1.BidiLevel, run2.BidiLevel); + Assert.Equal(run1.ShapedBuffer.IsLeftToRight, run2.ShapedBuffer.IsLeftToRight); + Assert.Equal(run1.ShapedBuffer.Length, run2.ShapedBuffer.Length); + + for (var j = 0; j < run1.ShapedBuffer.Length; j++) + { + Assert.Equal(run1.ShapedBuffer[j].GlyphCluster, + run2.ShapedBuffer[j].GlyphCluster); + } + } + } + } + } + + [Fact] + public void Bidi_Cache_Hit_With_Wrapping_Does_Not_Double_Reorder() + { + using (Start()) + { + // Wrapped bidi text so the Wrap path through FormatLineFromCache is exercised. + var text = "Hello \u0645\u0631\u062D\u0628\u0627 World and more text to force wrapping"; + var defaultProperties = new GenericTextRunProperties(Typeface.Default); + var wrappingProperties = new GenericTextParagraphProperties( + FlowDirection.LeftToRight, TextAlignment.Left, true, false, + defaultProperties, TextWrapping.Wrap, 0, 0, 0); + var textSource = new SingleBufferTextSource(text, defaultProperties); + var formatter = new TextFormatterImpl(); + + using var cache = new TextRunCache(); + + // Cache miss pass. + var misLines = FormatAllLines(formatter, textSource, 80.0, wrappingProperties, cache); + + // Cache hit pass. + var hitLines = FormatAllLines(formatter, textSource, 80.0, wrappingProperties, cache); + + Assert.Equal(misLines.Length, hitLines.Length); + + for (var lineIdx = 0; lineIdx < misLines.Length; lineIdx++) + { + var missLine = misLines[lineIdx]; + var hitLine = hitLines[lineIdx]; + + Assert.Equal(missLine.Length, hitLine.Length); + Assert.Equal(missLine.TextRuns.Count, hitLine.TextRuns.Count); + + for (var i = 0; i < missLine.TextRuns.Count; i++) + { + var missRun = missLine.TextRuns[i] as ShapedTextRun; + var hitRun = hitLine.TextRuns[i] as ShapedTextRun; + + if (missRun == null || hitRun == null) + continue; + + Assert.Equal(missRun.BidiLevel, hitRun.BidiLevel); + Assert.Equal(missRun.ShapedBuffer.IsLeftToRight, hitRun.ShapedBuffer.IsLeftToRight); + Assert.Equal(missRun.ShapedBuffer.Length, hitRun.ShapedBuffer.Length); + + for (var j = 0; j < missRun.ShapedBuffer.Length; j++) + { + Assert.Equal(missRun.ShapedBuffer[j].GlyphCluster, + hitRun.ShapedBuffer[j].GlyphCluster); + } + } + } + } + } + [Fact] public void Dispose_Releases_Cache_Entries() {