Merge pull request #11667 from Gillibald/textAlignmentFixes

Text] Fixes

src/Avalonia.Base/Media/GlyphRun.cs

@@ -643,12 +643,13 @@ namespace Avalonia.Media
                 lastCluster = _glyphInfos[_glyphInfos.Count - 1].GlyphCluster;
             }
 
+            var isReversed = firstCluster > lastCluster;
+
             if (!IsLeftToRight)
             {
                 (lastCluster, firstCluster) = (firstCluster, lastCluster);
             }
 
-            var isReversed = firstCluster > lastCluster;
             var height = GlyphTypeface.Metrics.LineSpacing * Scale;
             var widthIncludingTrailingWhitespace = 0d;
 
@@ -766,15 +767,13 @@ namespace Avalonia.Media
 
             if (!charactersSpan.IsEmpty)
             {
-                var characterIndex = 0;
+                var characterIndex = charactersSpan.Length - 1;
 
                 for (var i = 0; i < _glyphInfos.Count; i++)
                 {
                     var currentCluster = _glyphInfos[i].GlyphCluster;
                     var codepoint = Codepoint.ReadAt(charactersSpan, characterIndex, out var characterLength);
 
-                    characterIndex += characterLength;
-
                     if (!codepoint.IsWhiteSpace)
                     {
                         break;
@@ -784,9 +783,9 @@ namespace Avalonia.Media
 
                     var j = i;
 
-                    while (j - 1 >= 0)
+                    while (j + 1 < _glyphInfos.Count)
                     {
-                        var nextCluster = _glyphInfos[--j].GlyphCluster;
+                        var nextCluster = _glyphInfos[++j].GlyphCluster;
 
                         if (currentCluster == nextCluster)
                         {
@@ -798,6 +797,8 @@ namespace Avalonia.Media
                         break;
                     }
 
+                    characterIndex -= clusterLength;
+
                     if (codepoint.IsBreakChar)
                     {
                         newLineLength += clusterLength;

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

@@ -684,7 +684,9 @@ namespace Avalonia.Media.TextFormatting
             var textRuns = new TextRun[] { new ShapedTextRun(shapedBuffer, properties) };
 
             var line = new TextLineImpl(textRuns, firstTextSourceIndex, 0, paragraphWidth, paragraphProperties, flowDirection);
+
             line.FinalizeLine();
+
             return line;
         }
 

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

@@ -128,7 +128,7 @@ namespace Avalonia.Media.TextFormatting
         /// <summary>
         /// Gets the text spacing.
         /// </summary>
-        public double LetterSpacing  => _paragraphProperties.LetterSpacing;
+        public double LetterSpacing => _paragraphProperties.LetterSpacing;
 
         /// <summary>
         /// Gets the text lines.
@@ -271,11 +271,13 @@ namespace Avalonia.Media.TextFormatting
 
             var currentY = 0.0;
 
-            foreach (var textLine in _textLines)
+            for (var i = 0; i < _textLines.Length; i++)
             {
+                var textLine = _textLines[i];
+
                 var end = textLine.FirstTextSourceIndex + textLine.Length;
 
-                if (end <= textPosition && end < _textSourceLength)
+                if (end <= textPosition && i + 1 < _textLines.Length)
                 {
                     currentY += textLine.Height;
 
@@ -511,7 +513,7 @@ namespace Avalonia.Media.TextFormatting
             {
                 var textLine = TextFormatterImpl.CreateEmptyTextLine(0, double.PositiveInfinity, _paragraphProperties);
 
-                UpdateMetrics(textLine, ref lineStartOfLongestLine, ref origin, ref first, 
+                UpdateMetrics(textLine, ref lineStartOfLongestLine, ref origin, ref first,
                     ref accBlackBoxLeft, ref accBlackBoxTop, ref accBlackBoxRight, ref accBlackBoxBottom);
 
                 return new TextLine[] { textLine };
@@ -638,13 +640,13 @@ namespace Avalonia.Media.TextFormatting
         }
 
         private void UpdateMetrics(
-            TextLine currentLine, 
-            ref double lineStartOfLongestLine, 
-            ref Point origin, 
-            ref bool first, 
+            TextLine currentLine,
+            ref double lineStartOfLongestLine,
+            ref Point origin,
+            ref bool first,
             ref double accBlackBoxLeft,
-            ref double accBlackBoxTop, 
-            ref double accBlackBoxRight, 
+            ref double accBlackBoxTop,
+            ref double accBlackBoxRight,
             ref double accBlackBoxBottom)
         {
             var blackBoxLeft = origin.X + currentLine.Start + currentLine.OverhangLeading;

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

@@ -371,14 +371,16 @@ namespace Avalonia.Media.TextFormatting
 
                 IndexedTextRun currentIndexedRun = _indexedTextRuns[i];
 
-                while(currentIndexedRun.TextSourceCharacterIndex != currentPosition)
+                while (currentIndexedRun.TextSourceCharacterIndex != currentPosition)
                 {
-                    if(i + 1 < _indexedTextRuns.Count)
+                    if (i + 1 == _indexedTextRuns.Count)
                     {
-                        i++;
-
-                        currentIndexedRun = _indexedTextRuns[i];
+                        break;
                     }
+
+                    i++;
+
+                    currentIndexedRun = _indexedTextRuns[i];
                 }
 
                 return currentIndexedRun;
@@ -430,7 +432,7 @@ namespace Avalonia.Media.TextFormatting
 
             if (currentTextRun == null)
             {
-                return 0;
+                return Start;
             }
 
             var directionalWidth = 0.0;
@@ -584,6 +586,8 @@ namespace Avalonia.Media.TextFormatting
             var currentPosition = FirstTextSourceIndex;
             var remainingLength = textLength;
 
+            TextBounds? lastBounds = null;
+
             static FlowDirection GetDirection(TextRun textRun, FlowDirection currentDirection)
             {
                 if (textRun is ShapedTextRun shapedTextRun)
@@ -604,12 +608,14 @@ namespace Avalonia.Media.TextFormatting
 
                 while (currentIndexedRun.TextSourceCharacterIndex != currentPosition)
                 {
-                    if (i + 1 < _indexedTextRuns.Count)
+                    if (i + 1 == _indexedTextRuns.Count)
                     {
-                        i++;
-
-                        currentIndexedRun = _indexedTextRuns[i];
+                        break;
                     }
+
+                    i++;
+
+                    currentIndexedRun = _indexedTextRuns[i];
                 }
 
                 return currentIndexedRun;
@@ -632,6 +638,40 @@ namespace Avalonia.Media.TextFormatting
                 return distance;
             }
 
+            bool TryMergeWithLastBounds(TextBounds currentBounds, TextBounds lastBounds)
+            {
+                if (currentBounds.FlowDirection != lastBounds.FlowDirection)
+                {
+                    return false;
+                }
+
+                if (currentBounds.Rectangle.Left == lastBounds.Rectangle.Right)
+                {
+                    foreach (var runBounds in currentBounds.TextRunBounds)
+                    {
+                        lastBounds.TextRunBounds.Add(runBounds);
+                    }
+
+                    lastBounds.Rectangle = lastBounds.Rectangle.Union(currentBounds.Rectangle);
+
+                    return true;
+                }
+
+                if (currentBounds.Rectangle.Right == lastBounds.Rectangle.Left)
+                {
+                    for (int i = 0; i < currentBounds.TextRunBounds.Count; i++)
+                    {
+                        lastBounds.TextRunBounds.Insert(i, currentBounds.TextRunBounds[i]);
+                    }
+
+                    lastBounds.Rectangle = lastBounds.Rectangle.Union(currentBounds.Rectangle);
+
+                    return true;
+                }
+
+                return false;
+            }
+
             while (remainingLength > 0 && currentPosition < FirstTextSourceIndex + Length)
             {
                 var currentIndexedRun = FindIndexedRun();
@@ -667,67 +707,21 @@ namespace Avalonia.Media.TextFormatting
                     directionalWidth = currentDrawable.Size.Width;
                 }
 
-                if (currentTextRun is not TextEndOfLine)
-                {
-                    if (currentDirection == FlowDirection.LeftToRight)
-                    {
-                        // Find consecutive runs of same direction
-                        for (; lastRunIndex + 1 < _textRuns.Length; lastRunIndex++)
-                        {
-                            var nextRun = _textRuns[lastRunIndex + 1];
-
-                            var nextDirection = GetDirection(nextRun, currentDirection);
-
-                            if (currentDirection != nextDirection)
-                            {
-                                break;
-                            }
-
-                            if (nextRun is DrawableTextRun nextDrawable)
-                            {
-                                directionalWidth += nextDrawable.Size.Width;
-                            }
-                        }
-                    }
-                    else
-                    {
-                        // Find consecutive runs of same direction
-                        for (; firstRunIndex - 1 > 0; firstRunIndex--)
-                        {
-                            var previousRun = _textRuns[firstRunIndex - 1];
-
-                            var previousDirection = GetDirection(previousRun, currentDirection);
-
-                            if (currentDirection != previousDirection)
-                            {
-                                break;
-                            }
-
-                            if (previousRun is DrawableTextRun previousDrawable)
-                            {
-                                directionalWidth += previousDrawable.Size.Width;
-
-                                currentX -= previousDrawable.Size.Width;
-                            }
-                        }
-                    }
-                }
-
                 int coveredLength;
-                TextBounds? textBounds;
+                TextBounds? currentBounds;
 
                 switch (currentDirection)
                 {
                     case FlowDirection.RightToLeft:
                         {
-                            textBounds = GetTextRunBoundsRightToLeft(firstRunIndex, lastRunIndex, currentX + directionalWidth, firstTextSourceIndex,
+                            currentBounds = GetTextRunBoundsRightToLeft(firstRunIndex, lastRunIndex, currentX + directionalWidth, firstTextSourceIndex,
                                     currentPosition, remainingLength, out coveredLength, out currentPosition);
 
                             break;
                         }
                     default:
                         {
-                            textBounds = GetTextBoundsLeftToRight(firstRunIndex, lastRunIndex, currentX, firstTextSourceIndex,
+                            currentBounds = GetTextBoundsLeftToRight(firstRunIndex, lastRunIndex, currentX, firstTextSourceIndex,
                                     currentPosition, remainingLength, out coveredLength, out currentPosition);
 
                             break;
@@ -736,7 +730,18 @@ namespace Avalonia.Media.TextFormatting
 
                 if (coveredLength > 0)
                 {
-                    result.Add(textBounds);
+                    if (lastBounds != null && TryMergeWithLastBounds(currentBounds, lastBounds))
+                    {
+                        currentBounds = lastBounds;
+
+                        result[result.Count - 1] = currentBounds;
+                    }
+                    else
+                    {
+                        result.Add(currentBounds);
+                    }
+
+                    lastBounds = currentBounds;
 
                     remainingLength -= coveredLength;
                 }
@@ -997,14 +1002,14 @@ namespace Avalonia.Media.TextFormatting
 
         public void FinalizeLine()
         {
+            _indexedTextRuns = BidiReorderer.Instance.BidiReorder(_textRuns, _paragraphProperties.FlowDirection, FirstTextSourceIndex);
+
             _textLineMetrics = CreateLineMetrics();
 
             if (_textLineBreak is null && _textRuns.Length > 1 && _textRuns[_textRuns.Length - 1] is TextEndOfLine textEndOfLine)
             {
                 _textLineBreak = new TextLineBreak(textEndOfLine);
-            }
-
-            _indexedTextRuns = BidiReorderer.Instance.BidiReorder(_textRuns, _paragraphProperties.FlowDirection, FirstTextSourceIndex);
+            }      
         }
 
         /// <summary>

tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs

@@ -1112,6 +1112,21 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting
             }
         }
 
+        [Fact]
+        public void Should_HitTestTextPosition_EndOfLine_RTL()
+        {
+            var text = "גש\r\n";
+
+            using (Start())
+            {
+                var textLayout = new TextLayout(text, Typeface.Default, 12, Brushes.Black, flowDirection: FlowDirection.RightToLeft);
+
+                var rect = textLayout.HitTestTextPosition(text.Length);
+
+                Assert.Equal(14.0625, rect.Top);
+            }
+        }
+
 
         private static IDisposable Start()
         {

tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs

@@ -2,12 +2,10 @@
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
-using System.Runtime.InteropServices;
 using Avalonia.Headless;
 using Avalonia.Media;
 using Avalonia.Media.TextFormatting;
 using Avalonia.UnitTests;
-using Avalonia.Utilities;
 using Xunit;
 
 namespace Avalonia.Skia.UnitTests.Media.TextFormatting
@@ -1072,7 +1070,7 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting
         }
 
         [Fact]
-        public void Should_GetTextBounds_BiDi()
+        public void Should_GetTextBounds_Bidi()
         {
             var text = "אבגדה 12345 ABCDEF אבגדה";
 
@@ -1114,12 +1112,39 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting
 
                 bounds = textLine.GetTextBounds(0, 25);
 
-                Assert.Equal(5, bounds.Count);
+                Assert.Equal(4, bounds.Count);
 
                 Assert.Equal(textLine.WidthIncludingTrailingWhitespace, bounds.Last().Rectangle.Right);
             }
         }
 
+        [Fact]
+        public void Should_GetTextBounds_Bidi_2()
+        {
+            var text = "אבג ABC אבג 123";
+
+            using (Start())
+            {
+                var defaultProperties = new GenericTextRunProperties(Typeface.Default);
+                var textSource = new SingleBufferTextSource(text, defaultProperties, true);
+
+                var formatter = new TextFormatterImpl();
+
+                var textLine =
+                    formatter.FormatLine(textSource, 0, double.PositiveInfinity,
+                        new GenericTextParagraphProperties(FlowDirection.LeftToRight, TextAlignment.Left,
+                        true, true, defaultProperties, TextWrapping.NoWrap, 0, 0, 0));
+
+                var bounds = textLine.GetTextBounds(0, text.Length);
+
+                Assert.Equal(4, bounds.Count);
+
+                var right = bounds.Last().Rectangle.Right;
+
+                Assert.Equal(textLine.WidthIncludingTrailingWhitespace, right);
+            }
+        }
+
         private class FixedRunsTextSource : ITextSource
         {
             private readonly IReadOnlyList<TextRun> _textRuns;