From 4459a577fd39844f091bf266c61f250896c2b918 Mon Sep 17 00:00:00 2001 From: Benedikt Stebner Date: Wed, 26 Aug 2026 14:13:05 +0000 Subject: [PATCH] [Text] Snap a right-to-left buffer split to the cluster boundary (#22066) * Add failing test for a right-to-left split inside a cluster Splitting a right-to-left shaped buffer keeps a straddling cluster's glyphs together in the leading half, but cuts the text at the requested offset instead of following them. The halves then disagree about which characters their glyphs cover: the leading half claims one character while holding the glyphs for two, and the trailing half claims a character whose glyphs it doesn't have. The left-to-right path already snaps the text boundary to the cluster. * Snap a right-to-left split to the cluster the glyphs stay with A cluster straddling the split point keeps all its glyphs in the leading half, so the text has to be cut where those glyphs end, not at the requested offset. Cutting at the offset produced two halves whose glyph and character coverage disagreed, and their shared cluster cache then described neither: measuring the trailing half returned counts that cut a cluster, and splitting it again could hand back a leading half holding every glyph and no trailing half at all - a null run that crashed the bidi reorderer. The ascending path already snapped this way; this makes the descending path match. --- .../Media/TextFormatting/ShapedBuffer.cs | 17 +++++-- .../Media/TextFormatting/TextShaperTests.cs | 50 +++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Base/Media/TextFormatting/ShapedBuffer.cs b/src/Avalonia.Base/Media/TextFormatting/ShapedBuffer.cs index 0a9f2f1e86..cfeb290f2e 100644 --- a/src/Avalonia.Base/Media/TextFormatting/ShapedBuffer.cs +++ b/src/Avalonia.Base/Media/TextFormatting/ShapedBuffer.cs @@ -731,20 +731,27 @@ namespace Avalonia.Media.TextFormatting splitGlyphIndex = ~foundIndex; } - // Visual leading = glyphs [0, splitGlyphIndex) → logically text[textLength..] (our "second") - // Visual trailing = glyphs [splitGlyphIndex, end) → logically text[0..textLength] (our "first") + // Visual leading = glyphs [0, splitGlyphIndex) → logically the trailing text (our "second") + // Visual trailing = glyphs [splitGlyphIndex, end) → logically the leading text (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); + // The glyph boundary sits past the requested offset when it falls inside a cluster, + // because the whole cluster stays with "first". The text boundary has to follow it so + // each half's text matches its glyphs - the ascending path snaps the same way. + var splitCharCount = splitGlyphIndex > 0 + ? Math.Min(glyphInfos[splitGlyphIndex - 1].GlyphCluster - baseCluster, Text.Length) + : Text.Length; + + var firstText = Text.Slice(0, splitCharCount); + var secondText = Text.Slice(splitCharCount); // share the parent's cluster cache. The cache stores // clusters in logical order, so "first" (text[0..textLength]) gets the // leading slice and "second" gets the trailing slice — same indexing // as the LTR case. EnsureClusterCache(); - var firstClusterCount = FindClusterOffsetForSplit(textLength); + var firstClusterCount = FindClusterOffsetForSplit(splitCharCount); var first = new ShapedBuffer( firstText, firstGlyphs, diff --git a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextShaperTests.cs b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextShaperTests.cs index 8f039a97e2..3e5f15c775 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextShaperTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextShaperTests.cs @@ -71,6 +71,56 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting } } + [Fact] + public void Should_Not_Split_RightToLeft_Cluster() + { + // Arabic letters carry their harakat in the same cluster, so the first cluster of this + // text spans two characters. Splitting inside it keeps the cluster's glyphs together in + // the leading half - the text boundary has to follow them, or the two halves disagree + // about which characters their glyphs cover. + const string text = "أَبْجَدِيَّة"; + + using (Start()) + { + var codepoint = Codepoint.ReadAt(text, 0, out _); + + Assert.True(FontManager.Current.TryMatchCharacter(codepoint, FontStyle.Normal, FontWeight.Normal, + FontStretch.Normal, null, null, out var typeface)); + + var options = new TextShaperOptions(typeface.GlyphTypeface, 12, 1, CultureInfo.InvariantCulture); + var buffer = TextShaper.Current.ShapeText(text.AsMemory(), options); + + // Precondition: the first cluster covers the first two characters. + Assert.False(buffer.IsLeftToRight); + Assert.Equal(0, buffer[buffer.Length - 1].GlyphCluster); + Assert.Equal(0, buffer[buffer.Length - 2].GlyphCluster); + Assert.Equal(2, buffer[buffer.Length - 3].GlyphCluster); + + var splitResult = buffer.Split(1); + + var first = splitResult.First; + var second = splitResult.Second; + + Assert.NotNull(first); + Assert.NotNull(second); + + // The split snaps forward past the cluster, exactly like the left-to-right path. + Assert.Equal(2, first!.Text.Length); + Assert.Equal(2, first.Length); + + // No character and no glyph is lost or duplicated. + Assert.Equal(text.Length, first.Text.Length + second!.Text.Length); + Assert.Equal(buffer.Length, first.Length + second.Length); + + // Every glyph of the trailing half belongs to the characters the trailing half owns. + for (var i = 0; i < second.Length; i++) + { + Assert.True(second[i].GlyphCluster >= first.Text.Length, + $"Glyph {i} has cluster {second[i].GlyphCluster}, which the leading half owns."); + } + } + } + [Fact] public void Should_Split_RightToLeft() {