From bfdd3fa325880fa88bce9047b817ab2eb2435e60 Mon Sep 17 00:00:00 2001 From: Benedikt Stebner Date: Thu, 1 Jun 2023 07:13:36 +0200 Subject: [PATCH 1/6] Prevent infinite loop --- samples/Sandbox/MainWindow.axaml | 1 + .../Media/TextFormatting/TextLineImpl.cs | 22 +++++++++------ .../Media/TextFormatting/TextLineTests.cs | 28 ++++++++++++++++++- 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/samples/Sandbox/MainWindow.axaml b/samples/Sandbox/MainWindow.axaml index 6929f192c7..f0f099b95b 100644 --- a/samples/Sandbox/MainWindow.axaml +++ b/samples/Sandbox/MainWindow.axaml @@ -1,4 +1,5 @@ + diff --git a/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs b/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs index c2ec78e187..0b1bd2e189 100644 --- a/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs +++ b/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; @@ -604,12 +606,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; diff --git a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs index 1d07e780e6..6373fb4e91 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs @@ -9,6 +9,7 @@ using Avalonia.Media.TextFormatting; using Avalonia.UnitTests; using Avalonia.Utilities; using Xunit; +using static System.Net.Mime.MediaTypeNames; namespace Avalonia.Skia.UnitTests.Media.TextFormatting { @@ -1072,7 +1073,7 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting } [Fact] - public void Should_GetTextBounds_BiDi() + public void Should_GetTextBounds_Bidi() { var text = "אבגדה 12345 ABCDEF אבגדה"; @@ -1120,6 +1121,31 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting } } + [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(5, bounds.Count); + + Assert.Equal(textLine.WidthIncludingTrailingWhitespace, bounds.Last().Rectangle.Right); + } + } + private class FixedRunsTextSource : ITextSource { private readonly IReadOnlyList _textRuns; From e7c31e7de8037e8d54e16bb19d64001649c1724d Mon Sep 17 00:00:00 2001 From: Benedikt Stebner Date: Tue, 6 Jun 2023 10:59:50 +0200 Subject: [PATCH 2/6] Fix combined TextRunBounds --- .../Media/TextFormatting/TextLineImpl.cs | 101 +++++++++--------- .../Media/TextFormatting/TextLineTests.cs | 8 +- 2 files changed, 56 insertions(+), 53 deletions(-) diff --git a/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs b/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs index 0b1bd2e189..8ce547dcbc 100644 --- a/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs +++ b/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs @@ -586,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) @@ -636,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(); @@ -671,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; @@ -740,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; } diff --git a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs index 6373fb4e91..8814207a26 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs @@ -1115,7 +1115,7 @@ 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); } @@ -1140,9 +1140,11 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting var bounds = textLine.GetTextBounds(0, text.Length); - Assert.Equal(5, bounds.Count); + Assert.Equal(4, bounds.Count); - Assert.Equal(textLine.WidthIncludingTrailingWhitespace, bounds.Last().Rectangle.Right); + var right = bounds.Last().Rectangle.Right; + + Assert.Equal(textLine.WidthIncludingTrailingWhitespace, right); } } From 70a89c554b6f621f8b60370bfef54306051ca325 Mon Sep 17 00:00:00 2001 From: Benedikt Stebner Date: Tue, 6 Jun 2023 11:01:15 +0200 Subject: [PATCH 3/6] Revert change --- samples/Sandbox/MainWindow.axaml | 1 - 1 file changed, 1 deletion(-) diff --git a/samples/Sandbox/MainWindow.axaml b/samples/Sandbox/MainWindow.axaml index f0f099b95b..6929f192c7 100644 --- a/samples/Sandbox/MainWindow.axaml +++ b/samples/Sandbox/MainWindow.axaml @@ -1,5 +1,4 @@ - From 29c873ce6aa4e806b96077adc23567efff2c7804 Mon Sep 17 00:00:00 2001 From: Benedikt Stebner Date: Tue, 6 Jun 2023 11:18:47 +0200 Subject: [PATCH 4/6] Fix caret position for empty TextBox with RTL flow direction --- src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs b/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs index 8ce547dcbc..588a30de85 100644 --- a/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs +++ b/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs @@ -432,7 +432,7 @@ namespace Avalonia.Media.TextFormatting if (currentTextRun == null) { - return 0; + return Start; } var directionalWidth = 0.0; From cc507ba359703bfe040cda1595b0ac6b28d8d93c Mon Sep 17 00:00:00 2001 From: Benedikt Stebner Date: Tue, 6 Jun 2023 16:58:13 +0200 Subject: [PATCH 5/6] Handle extra lines during hit testing --- src/Avalonia.Base/Media/GlyphRun.cs | 13 ++++++----- .../Media/TextFormatting/TextFormatterImpl.cs | 2 ++ .../Media/TextFormatting/TextLayout.cs | 22 ++++++++++--------- .../Media/TextFormatting/TextLineImpl.cs | 18 +++++++-------- .../Media/TextFormatting/TextLayoutTests.cs | 15 +++++++++++++ 5 files changed, 45 insertions(+), 25 deletions(-) diff --git a/src/Avalonia.Base/Media/GlyphRun.cs b/src/Avalonia.Base/Media/GlyphRun.cs index 350d8817f1..0f70386424 100644 --- a/src/Avalonia.Base/Media/GlyphRun.cs +++ b/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; diff --git a/src/Avalonia.Base/Media/TextFormatting/TextFormatterImpl.cs b/src/Avalonia.Base/Media/TextFormatting/TextFormatterImpl.cs index a609800fb8..d2198a2cbf 100644 --- a/src/Avalonia.Base/Media/TextFormatting/TextFormatterImpl.cs +++ b/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; } diff --git a/src/Avalonia.Base/Media/TextFormatting/TextLayout.cs b/src/Avalonia.Base/Media/TextFormatting/TextLayout.cs index 4ccb3f6a37..b6b6d11a49 100644 --- a/src/Avalonia.Base/Media/TextFormatting/TextLayout.cs +++ b/src/Avalonia.Base/Media/TextFormatting/TextLayout.cs @@ -128,7 +128,7 @@ namespace Avalonia.Media.TextFormatting /// /// Gets the text spacing. /// - public double LetterSpacing => _paragraphProperties.LetterSpacing; + public double LetterSpacing => _paragraphProperties.LetterSpacing; /// /// 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; diff --git a/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs b/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs index 588a30de85..ca31d9a6d0 100644 --- a/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs +++ b/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs @@ -640,14 +640,14 @@ namespace Avalonia.Media.TextFormatting bool TryMergeWithLastBounds(TextBounds currentBounds, TextBounds lastBounds) { - if(currentBounds.FlowDirection != lastBounds.FlowDirection) + if (currentBounds.FlowDirection != lastBounds.FlowDirection) { return false; } - if(currentBounds.Rectangle.Left == lastBounds.Rectangle.Right) + if (currentBounds.Rectangle.Left == lastBounds.Rectangle.Right) { - foreach(var runBounds in currentBounds.TextRunBounds) + foreach (var runBounds in currentBounds.TextRunBounds) { lastBounds.TextRunBounds.Add(runBounds); } @@ -657,7 +657,7 @@ namespace Avalonia.Media.TextFormatting return true; } - if(currentBounds.Rectangle.Right == lastBounds.Rectangle.Left) + if (currentBounds.Rectangle.Right == lastBounds.Rectangle.Left) { for (int i = 0; i < currentBounds.TextRunBounds.Count; i++) { @@ -730,7 +730,7 @@ namespace Avalonia.Media.TextFormatting if (coveredLength > 0) { - if(lastBounds != null && TryMergeWithLastBounds(currentBounds, lastBounds)) + if (lastBounds != null && TryMergeWithLastBounds(currentBounds, lastBounds)) { currentBounds = lastBounds; @@ -739,7 +739,7 @@ namespace Avalonia.Media.TextFormatting else { result.Add(currentBounds); - } + } lastBounds = currentBounds; @@ -1002,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); + } } /// diff --git a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs index 71aeb4397e..ac444e37df 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs +++ b/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() { From c2bbdf80b16a29be48f0a616dcf7fe742ee230f7 Mon Sep 17 00:00:00 2001 From: Benedikt Stebner Date: Tue, 6 Jun 2023 17:05:51 +0200 Subject: [PATCH 6/6] Remove redundant usings --- .../Media/TextFormatting/TextLineTests.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs index 8814207a26..12427e1f9e 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs @@ -2,14 +2,11 @@ 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; -using static System.Net.Mime.MediaTypeNames; namespace Avalonia.Skia.UnitTests.Media.TextFormatting {