Browse Source

[Text] Fixes double reorder of cached bidi runs (#21351)

* Fix double reorder with cached runs

* Revert change

* Add covering unit tests

* Remove worktrees

* Update API baseline
pull/21371/head
Benedikt Stebner 7 days ago
committed by GitHub
parent
commit
507e16797d
No known key found for this signature in database GPG Key ID: B5690EEEBB952194
  1. 28
      api/Avalonia.nupkg.xml
  2. 64
      src/Avalonia.Base/Media/TextFormatting/BidiReorderer.cs
  3. 120
      src/Avalonia.Base/Media/TextFormatting/ShapedBuffer.cs
  4. 112
      src/Avalonia.Base/Media/TextFormatting/ShapedTextRun.cs
  5. 5
      src/Avalonia.Base/Media/TextFormatting/TextEllipsisHelper.cs
  6. 134
      src/Avalonia.Base/Media/TextFormatting/TextFormatterImpl.cs
  7. 26
      src/Avalonia.Base/Media/TextFormatting/TextRunCache.cs
  8. 9
      src/HarfBuzz/Avalonia.HarfBuzz/HarfBuzzTextShaper.cs
  9. 9
      tests/Avalonia.Skia.UnitTests/Media/GlyphRunTests.cs
  10. 4
      tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs
  11. 228
      tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextRunCacheTests.cs

28
api/Avalonia.nupkg.xml

@ -0,0 +1,28 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- https://learn.microsoft.com/dotnet/fundamentals/package-validation/diagnostic-ids -->
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Avalonia.Media.TextFormatting.ShapedBuffer.Reverse</Target>
<Left>baseline/Avalonia/lib/net10.0/Avalonia.Base.dll</Left>
<Right>current/Avalonia/lib/net10.0/Avalonia.Base.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Avalonia.Media.TextFormatting.ShapedTextRun.get_IsReversed</Target>
<Left>baseline/Avalonia/lib/net10.0/Avalonia.Base.dll</Left>
<Right>current/Avalonia/lib/net10.0/Avalonia.Base.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Avalonia.Media.TextFormatting.ShapedBuffer.Reverse</Target>
<Left>baseline/Avalonia/lib/net8.0/Avalonia.Base.dll</Left>
<Right>current/Avalonia/lib/net8.0/Avalonia.Base.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Avalonia.Media.TextFormatting.ShapedTextRun.get_IsReversed</Target>
<Left>baseline/Avalonia/lib/net8.0/Avalonia.Base.dll</Left>
<Right>current/Avalonia/lib/net8.0/Avalonia.Base.dll</Right>
</Suppression>
</Suppressions>

64
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)
{

120
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<GlyphInfo>.Shared.Rent(bufferLength);
_glyphInfos = new ArraySlice<GlyphInfo>(_rentedBuffer, 0, bufferLength);
_glyphInfos = new ArraySlice<GlyphInfo>(_rentedBuffer, 0, bufferLength);
GlyphTypeface = glyphTypeface;
FontRenderingEmSize = fontRenderingEmSize;
BidiLevel = bidiLevel;
@ -55,7 +55,7 @@ namespace Avalonia.Media.TextFormatting
/// <summary>
/// The buffer's bidi level.
/// </summary>
public sbyte BidiLevel { get; private set; }
public sbyte BidiLevel { get; }
/// <summary>
/// The buffer's reading direction.
@ -66,14 +66,6 @@ namespace Avalonia.Media.TextFormatting
/// The text that is represended by this buffer.
/// </summary>
public ReadOnlyMemory<char> Text { get; }
/// <summary>
/// Reverses the buffer.
/// </summary>
public void Reverse()
{
_glyphInfos.Span.Reverse();
}
public void Dispose()
{
@ -95,7 +87,20 @@ namespace Avalonia.Media.TextFormatting
public IEnumerator<GlyphInfo> GetEnumerator() => _glyphInfos.GetEnumerator();
internal void ResetBidiLevel(sbyte paragraphEmbeddingLevel) => BidiLevel = paragraphEmbeddingLevel;
/// <summary>
/// Creates a view of this buffer that reports the given <paramref name="paragraphEmbeddingLevel"/>
/// as its <see cref="BidiLevel"/> while sharing the same underlying glyphs.
/// Used for trailing-whitespace handling where the visual direction follows the paragraph.
/// </summary>
internal ShapedBuffer WithBidiLevel(sbyte paragraphEmbeddingLevel)
{
if (BidiLevel == paragraphEmbeddingLevel)
{
return this;
}
return new ShapedBuffer(Text, _glyphInfos, GlyphTypeface, FontRenderingEmSize, paragraphEmbeddingLevel);
}
int IReadOnlyCollection<GlyphInfo>.Count => _glyphInfos.Length;
@ -126,25 +131,30 @@ namespace Avalonia.Media.TextFormatting
return new SplitResult<ShapedBuffer>(this, null);
}
return IsLeftToRight ? SplitAscending(textLength) : SplitDescending(textLength);
}
/// <summary>
/// Split a buffer whose glyphs are ordered by ascending cluster (LTR visual / logical order).
/// </summary>
private SplitResult<ShapedBuffer> 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<ShapedBuffer>(leading, null);
}
@ -200,5 +204,69 @@ namespace Avalonia.Media.TextFormatting
return new SplitResult<ShapedBuffer>(leading, trailing);
}
/// <summary>
/// Split a buffer whose glyphs are ordered by descending cluster (RTL visual order).
/// The leading <see cref="ShapedBuffer"/> corresponds to text[0..textLength], whose glyphs
/// live at the <em>tail</em> of the visual array.
/// </summary>
private SplitResult<ShapedBuffer> 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<ShapedBuffer>(first, null);
}
var second = new ShapedBuffer(
secondText, secondGlyphs,
GlyphTypeface, FontRenderingEmSize, BidiLevel);
return new SplitResult<ShapedBuffer>(first, second);
}
}
}

112
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
/// <summary>
/// A text run that holds shaped characters.
/// </summary>
/// <remarks>
/// Glyph data in the underlying <see cref="ShapedBuffer"/> is immutable after shaping:
/// LTR buffers are in ascending-cluster (logical) order, RTL buffers are in
/// descending-cluster (visual) order. <see cref="BidiReorderer"/> only reorders runs,
/// it never mutates glyph order.
///
/// Ref-counted: the initial constructor call establishes one reference. Call
/// <see cref="AddRef"/> when taking an additional reference (e.g., when a
/// <see cref="TextRunCache"/> stores a shaped run a formatter is also about to use),
/// and <see cref="Dispose"/> to release. The underlying shaped buffer is disposed only
/// when the last reference is released.
/// </remarks>
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();
/// <summary>
/// Takes an additional reference to this run. Must be paired with <see cref="Dispose"/>.
/// </summary>
internal ShapedTextRun AddRef()
{
Interlocked.Increment(ref _refCount);
return this;
}
/// <inheritdoc/>
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;
}
/// <summary>
/// Measures the number of characters that fit into available width.
/// </summary>
@ -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<ShapedTextRun> 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<ShapedTextRun>(null, first);
}
throw new InvalidOperationException($"Cannot split: requested length {length} consumes entire run.");
return new SplitResult<ShapedTextRun>(first, null);
}
var second = new ShapedTextRun(splitBuffer.Second, Properties);
if (isReversed)
{
return new SplitResult<ShapedTextRun>(second, first);
}
var second = new ShapedTextRun(splitBuffer.Second, Properties);
return new SplitResult<ShapedTextRun>(first, second);
}
@ -217,6 +236,11 @@ namespace Avalonia.Media.TextFormatting
public void Dispose()
{
if (Interlocked.Decrement(ref _refCount) != 0)
{
return;
}
_glyphRun?.Dispose();
ShapedBuffer.Dispose();
}

5
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)

134
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<TextRun>(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
}
/// <summary>
/// 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.
/// </summary>
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;
}
/// <summary>
/// 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
/// <see cref="ShapedTextRun"/> so that the caller owns a disposable reference.
/// </summary>
private static TextRun[] CreateNonOwningRuns(IReadOnlyList<TextRun> runs)
private static TextRun[] AddRefShapedRuns(IReadOnlyList<TextRun> runs)
{
var result = new TextRun[runs.Count];
for (var i = 0; i < runs.Count; i++)
{
result[i] = CreateNonOwningRun(runs[i]);
}
return result;
}
/// <summary>
/// Creates a non-owning list of shaped text runs for use with text wrapping.
/// </summary>
private static List<TextRun> CreateNonOwningRunsList(IReadOnlyList<TextRun> runs)
{
var result = new List<TextRun>(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();
}
}
}

26
src/Avalonia.Base/Media/TextFormatting/TextRunCache.cs

@ -125,20 +125,27 @@ namespace Avalonia.Media.TextFormatting
}
/// <summary>
/// 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 <see cref="ShapedTextRun"/>; the caller retains its
/// original references unchanged.
/// </summary>
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();
}
}
}
/// <inheritdoc/>
public void Dispose()
{

9
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 _);

9
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()

4
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();

228
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()
{

Loading…
Cancel
Save