Properly handle in cluster ShapedBuffer split (#19090)

* Properly handle in cluster ShapedBuffer split

* Allow ShapedBuffer split for empty text length

---------

Co-authored-by: Julien Lebosquain <[email protected]>

Author: Gillibald

src/Avalonia.Base/Media/TextFormatting/ShapedBuffer.cs

@@ -2,6 +2,7 @@
 using System.Buffers;
 using System.Collections;
 using System.Collections.Generic;
+using System.Linq;
 using System.Runtime.CompilerServices;
 using Avalonia.Utilities;
 
@@ -89,90 +90,110 @@ public GlyphInfo this[int index]
 
         public IEnumerator<GlyphInfo> GetEnumerator() => _glyphInfos.GetEnumerator();
 
+        internal void ResetBidiLevel(sbyte paragraphEmbeddingLevel) => BidiLevel = paragraphEmbeddingLevel;
+
+        int IReadOnlyCollection<GlyphInfo>.Count => _glyphInfos.Length;
+
+        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+
         /// <summary>
-        /// Finds a glyph index for given character index.
+        /// Splits the <see cref="TextRun"/> at specified length.
         /// </summary>
-        /// <param name="characterIndex">The character index.</param>
-        /// <returns>
-        /// The glyph index.
-        /// </returns>
-        private int FindGlyphIndex(int characterIndex)
+        /// <param name="textLength">The text length.</param>
+        /// <returns>The split result.</returns>
+        public SplitResult<ShapedBuffer> Split(int textLength)
         {
-            if (characterIndex < _glyphInfos[0].GlyphCluster)
+            // make sure we do not overshoot
+            textLength = Math.Min(Text.Length, textLength);
+
+            if (textLength <= 0)
             {
-                return 0;
+                var emptyBuffer = new ShapedBuffer(
+                    Text.Slice(0, 0), _glyphInfos.Slice(_glyphInfos.Start, 0),
+                    GlyphTypeface, FontRenderingEmSize, BidiLevel);
+
+                return new SplitResult<ShapedBuffer>(emptyBuffer, this);
             }
 
-            if (characterIndex > _glyphInfos[_glyphInfos.Length - 1].GlyphCluster)
+            // nothing to split
+            if (textLength == Text.Length)
             {
-                return _glyphInfos.Length - 1;
+                return new SplitResult<ShapedBuffer>(this, null);
             }
 
-            var comparer = GlyphInfo.ClusterAscendingComparer;
-
+            var sliceStart = _glyphInfos.Start;
             var glyphInfos = _glyphInfos.Span;
+            var glyphInfosLength = _glyphInfos.Length;
 
-            var searchValue = new GlyphInfo(default, characterIndex, default);
+            // the first glyph’s cluster is our “zero” for this sub‐buffer.
+            // we want an absolute target cluster = baseCluster + textLength
+            var baseCluster = glyphInfos[0].GlyphCluster;
+            var targetCluster = baseCluster + textLength;
 
-            var start = glyphInfos.BinarySearch(searchValue, comparer);
+            // binary‐search for a dummy with cluster == targetCluster
+            var searchValue = new GlyphInfo(0, targetCluster, 0, default);
+            var foundIndex = glyphInfos.BinarySearch(searchValue, GlyphInfo.ClusterAscendingComparer);
 
-            if (start < 0)
-            {
-                while (characterIndex > 0 && start < 0)
-                {
-                    characterIndex--;
+            int splitGlyphIndex;    // how many glyph‐slots go into "leading"
+            int splitCharCount;   // how many chars go into "leading" Text
 
-                    searchValue = new GlyphInfo(default, characterIndex, default);
-
-                    start = glyphInfos.BinarySearch(searchValue, comparer);
-                }
+            if (foundIndex >= 0)
+            {
+                // found a glyph info whose cluster == targetCluster
+                // back up to the start of the cluster
+                var i = foundIndex;
 
-                if (start < 0)
+                while (i > 0 && glyphInfos[i - 1].GlyphCluster == targetCluster)
                 {
-                    return -1;
+                    i--;
                 }
+                    
+                splitGlyphIndex = i;
+                splitCharCount = targetCluster - baseCluster;
             }
-
-            while (start > 0 && glyphInfos[start - 1].GlyphCluster == glyphInfos[start].GlyphCluster)
+            else
             {
-                start--;
-            }
-
-            return start;
-        }
+                // no exact match need to invert so ~foundIndex is the insertion point
+                // the first cluster > targetCluster
+                var invertedIndex = ~foundIndex;
 
-        /// <summary>
-        /// Splits the <see cref="TextRun"/> at specified length.
-        /// </summary>
-        /// <param name="length">The length.</param>
-        /// <returns>The split result.</returns>
-        internal SplitResult<ShapedBuffer> Split(int length)
-        {
-            if (Text.Length == length)
-            {
-                return new SplitResult<ShapedBuffer>(this, null);
+                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;
+                }
             }
 
-            var firstCluster = _glyphInfos[0].GlyphCluster;
-            var lastCluster = _glyphInfos[_glyphInfos.Length - 1].GlyphCluster;
+            var firstGlyphs = _glyphInfos.Slice(sliceStart, splitGlyphIndex);
+            var secondGlyphs = _glyphInfos.Slice(sliceStart + splitGlyphIndex, glyphInfosLength - splitGlyphIndex);
 
-            var start = firstCluster < lastCluster ? firstCluster : lastCluster;
+            var firstText = Text.Slice(0, splitCharCount);
+            var secondText = Text.Slice(splitCharCount);
 
-            var glyphCount = FindGlyphIndex(start + length);
+            var leading = new ShapedBuffer(
+                firstText, firstGlyphs,
+                GlyphTypeface, FontRenderingEmSize, BidiLevel);
 
-            var first = new ShapedBuffer(Text.Slice(0, length),
-                _glyphInfos.Take(glyphCount), 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)
+            {
+                return new SplitResult<ShapedBuffer>(leading, null);
+            }
 
-            var second = new ShapedBuffer(Text.Slice(length),
-                _glyphInfos.Skip(glyphCount), GlyphTypeface, FontRenderingEmSize, BidiLevel);
+            var trailing = new ShapedBuffer(
+                secondText, secondGlyphs,
+                GlyphTypeface, FontRenderingEmSize, BidiLevel);
 
-            return new SplitResult<ShapedBuffer>(first, second);
+            return new SplitResult<ShapedBuffer>(leading, trailing);
         }
-
-        internal void ResetBidiLevel(sbyte paragraphEmbeddingLevel) => BidiLevel = paragraphEmbeddingLevel;
-
-        int IReadOnlyCollection<GlyphInfo>.Count => _glyphInfos.Length;
-
-        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();       
     }
 }

src/Avalonia.Base/Media/TextFormatting/ShapedTextRun.cs

@@ -182,9 +182,9 @@ internal SplitResult<ShapedTextRun> Split(int length)
 
 #if DEBUG
 
-            if (first.Length != length)
+            if (first.Length < length)
             {
-                throw new InvalidOperationException("Split length mismatch.");
+                throw new InvalidOperationException("Split length too small.");
             }
 #endif
             var second = new ShapedTextRun(splitBuffer.Second!, Properties);

src/Avalonia.Base/Media/TextFormatting/TextFormatterImpl.cs

@@ -371,15 +371,31 @@ private static void ShapeTogether(IReadOnlyList<UnshapedTextRun> textRuns, ReadO
         {
             var shapedBuffer = textShaper.ShapeText(text, options);
 
+            var previousLength = 0;
+
             for (var i = 0; i < textRuns.Count; i++)
             {
                 var currentRun = textRuns[i];
 
-                var splitResult = shapedBuffer.Split(currentRun.Length);
+                var splitResult = shapedBuffer.Split(previousLength + currentRun.Length);
 
-                results.Add(new ShapedTextRun(splitResult.First, currentRun.Properties));
+                if(splitResult.First.Length == 0)
+                {
+                    previousLength += currentRun.Length;
+                }
+                else
+                {
+                    previousLength = 0;
 
-                shapedBuffer = splitResult.Second!;
+                    results.Add(new ShapedTextRun(splitResult.First, currentRun.Properties));
+                }
+              
+                if(splitResult.Second is null)
+                {
+                    return;
+                }
+
+                shapedBuffer = splitResult.Second;
             }
         }
 
@@ -921,7 +937,20 @@ private static TextLineImpl PerformTextWrapping(List<TextRun> textRuns, bool can
                     ResetTrailingWhitespaceBidiLevels(preSplitRuns, paragraphProperties.FlowDirection, objectPool);
                 }
 
-                var textLine = new TextLineImpl(preSplitRuns.ToArray(), firstTextSourceIndex, measuredLength,
+                var remainingTextRuns = new TextRun[preSplitRuns.Count];
+                //Measured lenght might have changed after a possible line break was found so we need to calculate the real length
+                var splitLength = 0;
+
+                for(var i = 0; i < preSplitRuns.Count; i++)
+                {
+                    var currentRun = preSplitRuns[i];
+
+                    remainingTextRuns[i] = currentRun;
+
+                    splitLength += currentRun.Length;
+                }
+
+                var textLine = new TextLineImpl(remainingTextRuns, firstTextSourceIndex, splitLength,
                     paragraphWidth, paragraphProperties, resolvedFlowDirection,
                     textLineBreak);
 

tests/Avalonia.Skia.UnitTests/Fonts/CascadiaCode.ttf

@@ -2,6 +2,7 @@
 using System.Globalization;
 using Avalonia.Media;
 using Avalonia.Media.TextFormatting;
+using Avalonia.Media.TextFormatting.Unicode;
 using Avalonia.UnitTests;
 using Xunit;
 
@@ -40,6 +41,74 @@ public void Should_Apply_IncrementalTabWidth()
             }
         }
 
+        [Fact]
+        public void Should_Not_Split_Cluster()
+        {
+            using (Start())
+            {
+                var typeface = new Typeface(FontFamily.Parse("resm:Avalonia.Skia.UnitTests.Fonts?assembly=Avalonia.Skia.UnitTests#Cascadia Code"));
+         
+                var buffer = TextShaper.Current.ShapeText("a\"๊a", new TextShaperOptions(typeface.GlyphTypeface));
+
+                var splitResult = buffer.Split(1);
+
+                Assert.Equal(1, splitResult.First.Length);
+
+                buffer = splitResult.Second;
+
+                Assert.NotNull(buffer);
+
+                //\"๊  
+                splitResult = buffer.Split(1);
+
+                Assert.Equal(2, splitResult.First.Length);
+
+                buffer = splitResult.Second;
+
+                Assert.NotNull(buffer);
+            }
+        }
+
+        [Fact]
+        public void Should_Split_RightToLeft()
+        {
+            var 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 buffer = TextShaper.Current.ShapeText(text, new TextShaperOptions(typeface.GlyphTypeface));
+
+                var splitResult = buffer.Split(6);
+
+                var first = splitResult.First;
+
+                Assert.Equal(6, first.Length);
+            }
+        }
+
+        [Fact]
+        public void Should_Split_Zero_Length()
+        {
+            var text = "ABC";
+
+            using (Start())
+            {
+                var buffer = TextShaper.Current.ShapeText(text, new TextShaperOptions(Typeface.Default.GlyphTypeface));
+
+                var splitResult = buffer.Split(0);
+
+                Assert.Equal(0, splitResult.First.Length);
+
+                Assert.NotNull(splitResult.Second);
+
+                Assert.Equal(text.Length, splitResult.Second.Length);
+            }
+        }
+
         private static IDisposable Start()
         {
             var disposable = UnitTestApplication.Start(TestServices.MockPlatformRenderInterface