From 07b48075cec97c0c40fc05d326776f869d6dc885 Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Mon, 5 Oct 2020 20:11:32 +0300 Subject: [PATCH 1/8] add failing test for #4806 (skip it for now as it's deadlock) --- .../Media/TextFormatting/TextLayoutTests.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs index f3e1c37705..4785f84133 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs @@ -575,6 +575,24 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting } } + [Fact(Skip = "Infinity loop issue #4806")] + public void Should_Wrap_Min_OneCharacter_EveryLine() + { + using (Start()) + { + var layout = new TextLayout( + s_singleLineText, + Typeface.Default, + 12, + Brushes.Black, + textWrapping: TextWrapping.Wrap, + maxWidth: 3); + + //every character should be new line as there not enough space for even one character + Assert.Equal(s_singleLineText.Length, layout.TextLines.Count); + } + } + private static IDisposable Start() { var disposable = UnitTestApplication.Start(TestServices.MockPlatformRenderInterface From 0f81b0fc9302e846d59845787e0bb23833ece5e1 Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Mon, 5 Oct 2020 20:14:55 +0300 Subject: [PATCH 2/8] add failing test for formatter for #4806 --- .../TextFormatting/TextFormatterTests.cs | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextFormatterTests.cs b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextFormatterTests.cs index adcc79e029..7f9713930a 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextFormatterTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextFormatterTests.cs @@ -293,6 +293,34 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting } } + [Fact] + public void Wrap_Should_Not_Produce_Empty_Lines() + { + using (Start()) + { + const string text = "012345"; + + var defaultProperties = new GenericTextRunProperties(Typeface.Default); + var paragraphProperties = new GenericTextParagraphProperties(defaultProperties, textWrapping: TextWrapping.Wrap); + var textSource = new SingleBufferTextSource(text, defaultProperties); + var formatter = new TextFormatterImpl(); + + var textSourceIndex = 0; + + while (textSourceIndex < text.Length) + { + var textLine = + formatter.FormatLine(textSource, textSourceIndex, 3, paragraphProperties); + + Assert.NotEqual(0, textLine.TextRange.Length); + + textSourceIndex += textLine.TextRange.Length; + } + + Assert.Equal(text.Length, textSourceIndex); + } + } + public static IDisposable Start() { var disposable = UnitTestApplication.Start(TestServices.MockPlatformRenderInterface From 37c2d0afb9b4b878aa2383b0c3f5d6ccdc56ed4d Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Mon, 5 Oct 2020 20:15:27 +0300 Subject: [PATCH 3/8] possible fix for #4806 for text formatting with wrap --- .../Media/TextFormatting/TextFormatterImpl.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs b/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs index 3e85f0f6f0..7e31d7e6ee 100644 --- a/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs +++ b/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs @@ -391,6 +391,10 @@ namespace Avalonia.Media.TextFormatting measuredLength = currentBreakPosition + lineBreaker.Current.PositionWrap; } } + else if (measuredLength == 0 && currentLength == 0) + { + measuredLength = 1; + } } currentLength += measuredLength; From 14b26187987493b5ac048dac826e5453fcd79d9a Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Mon, 5 Oct 2020 20:17:37 +0300 Subject: [PATCH 4/8] enable back the test for text Wrapping --- .../Media/TextFormatting/TextLayoutTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs index 4785f84133..26e8ce4797 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs @@ -575,7 +575,7 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting } } - [Fact(Skip = "Infinity loop issue #4806")] + [Fact] public void Should_Wrap_Min_OneCharacter_EveryLine() { using (Start()) From 4a0f517ecc2632d6f9768de4577d3e374c478b5f Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Tue, 6 Oct 2020 16:16:39 +0300 Subject: [PATCH 5/8] Revert "possible fix for #4806 for text formatting with wrap" This reverts commit 37c2d0afb9b4b878aa2383b0c3f5d6ccdc56ed4d. --- .../Media/TextFormatting/TextFormatterImpl.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs b/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs index 7e31d7e6ee..3e85f0f6f0 100644 --- a/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs +++ b/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs @@ -391,10 +391,6 @@ namespace Avalonia.Media.TextFormatting measuredLength = currentBreakPosition + lineBreaker.Current.PositionWrap; } } - else if (measuredLength == 0 && currentLength == 0) - { - measuredLength = 1; - } } currentLength += measuredLength; From c2d58f5a0b1402741a23552c6e9a0da467ca434e Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Tue, 6 Oct 2020 16:26:30 +0300 Subject: [PATCH 6/8] fix #4806 ensure at least one character is returned on the first text run measure --- .../Media/TextFormatting/TextFormatterImpl.cs | 12 ++++++++++-- .../Media/TextFormatting/TextLineImpl.cs | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs b/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs index 3e85f0f6f0..e65f96bf61 100644 --- a/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs +++ b/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs @@ -47,8 +47,9 @@ namespace Avalonia.Media.TextFormatting /// /// The text run. /// The available width. + /// Index of the textCharacters parent TextRun /// - internal static int MeasureCharacters(ShapedTextCharacters textCharacters, double availableWidth) + internal static int MeasureCharacters(ShapedTextCharacters textCharacters, double availableWidth, int textRunIndex) { var glyphRun = textCharacters.GlyphRun; @@ -73,6 +74,13 @@ namespace Avalonia.Media.TextFormatting if (currentWidth + advance > availableWidth) { + if(glyphCount == 0 && textRunIndex == 0) + { + //we need to return at least one characted on the first run + //or we risk to get a infinity loop when width is less than one character width + //issue #4806 + glyphCount = 1; + } break; } @@ -350,7 +358,7 @@ namespace Avalonia.Media.TextFormatting if (currentWidth + currentRun.Size.Width > availableWidth) { - var measuredLength = MeasureCharacters(currentRun, paragraphWidth - currentWidth); + var measuredLength = MeasureCharacters(currentRun, paragraphWidth - currentWidth, runIndex); var breakFound = false; diff --git a/src/Avalonia.Visuals/Media/TextFormatting/TextLineImpl.cs b/src/Avalonia.Visuals/Media/TextFormatting/TextLineImpl.cs index f5e87d097b..aa7759fc16 100644 --- a/src/Avalonia.Visuals/Media/TextFormatting/TextLineImpl.cs +++ b/src/Avalonia.Visuals/Media/TextFormatting/TextLineImpl.cs @@ -75,7 +75,7 @@ namespace Avalonia.Media.TextFormatting if (currentWidth > availableWidth) { - var measuredLength = TextFormatterImpl.MeasureCharacters(currentRun, availableWidth); + var measuredLength = TextFormatterImpl.MeasureCharacters(currentRun, availableWidth, runIndex); var currentBreakPosition = 0; From a19d327629ab93ddbacb64ab191af9e8ecef745c Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Tue, 6 Oct 2020 23:03:54 +0300 Subject: [PATCH 7/8] Revert "fix #4806 ensure at least one character is returned on the first text run measure" This reverts commit c2d58f5a0b1402741a23552c6e9a0da467ca434e. --- .../Media/TextFormatting/TextFormatterImpl.cs | 12 ++---------- .../Media/TextFormatting/TextLineImpl.cs | 2 +- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs b/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs index e65f96bf61..3e85f0f6f0 100644 --- a/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs +++ b/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs @@ -47,9 +47,8 @@ namespace Avalonia.Media.TextFormatting /// /// The text run. /// The available width. - /// Index of the textCharacters parent TextRun /// - internal static int MeasureCharacters(ShapedTextCharacters textCharacters, double availableWidth, int textRunIndex) + internal static int MeasureCharacters(ShapedTextCharacters textCharacters, double availableWidth) { var glyphRun = textCharacters.GlyphRun; @@ -74,13 +73,6 @@ namespace Avalonia.Media.TextFormatting if (currentWidth + advance > availableWidth) { - if(glyphCount == 0 && textRunIndex == 0) - { - //we need to return at least one characted on the first run - //or we risk to get a infinity loop when width is less than one character width - //issue #4806 - glyphCount = 1; - } break; } @@ -358,7 +350,7 @@ namespace Avalonia.Media.TextFormatting if (currentWidth + currentRun.Size.Width > availableWidth) { - var measuredLength = MeasureCharacters(currentRun, paragraphWidth - currentWidth, runIndex); + var measuredLength = MeasureCharacters(currentRun, paragraphWidth - currentWidth); var breakFound = false; diff --git a/src/Avalonia.Visuals/Media/TextFormatting/TextLineImpl.cs b/src/Avalonia.Visuals/Media/TextFormatting/TextLineImpl.cs index aa7759fc16..f5e87d097b 100644 --- a/src/Avalonia.Visuals/Media/TextFormatting/TextLineImpl.cs +++ b/src/Avalonia.Visuals/Media/TextFormatting/TextLineImpl.cs @@ -75,7 +75,7 @@ namespace Avalonia.Media.TextFormatting if (currentWidth > availableWidth) { - var measuredLength = TextFormatterImpl.MeasureCharacters(currentRun, availableWidth, runIndex); + var measuredLength = TextFormatterImpl.MeasureCharacters(currentRun, availableWidth); var currentBreakPosition = 0; From 40d74cf682b3ad6e88ff99ace2fbbae2ac2e3fe3 Mon Sep 17 00:00:00 2001 From: Benedikt Schroeder Date: Tue, 6 Oct 2020 19:00:53 +0200 Subject: [PATCH 8/8] Fix baseline alignment of multiple runs Make sure we always wrap at least one character --- .../ControlCatalog/Pages/TextBlockPage.xaml | 4 +- src/Avalonia.Visuals/Media/GlyphRun.cs | 17 +---- .../TextFormatting/ShapedTextCharacters.cs | 2 +- .../Media/TextFormatting/TextFormatterImpl.cs | 67 +++++++++++++------ .../Media/TextFormatting/TextLineImpl.cs | 44 ++++++------ 5 files changed, 74 insertions(+), 60 deletions(-) diff --git a/samples/ControlCatalog/Pages/TextBlockPage.xaml b/samples/ControlCatalog/Pages/TextBlockPage.xaml index 4a1c196917..d4f72f161a 100644 --- a/samples/ControlCatalog/Pages/TextBlockPage.xaml +++ b/samples/ControlCatalog/Pages/TextBlockPage.xaml @@ -18,8 +18,8 @@ - - + + diff --git a/src/Avalonia.Visuals/Media/GlyphRun.cs b/src/Avalonia.Visuals/Media/GlyphRun.cs index 14ab083b4f..155339b985 100644 --- a/src/Avalonia.Visuals/Media/GlyphRun.cs +++ b/src/Avalonia.Visuals/Media/GlyphRun.cs @@ -18,7 +18,7 @@ namespace Avalonia.Media private double _fontRenderingEmSize; private Size? _size; private int _biDiLevel; - private Point? _baselineOrigin; + private Point _baselineOrigin; private ReadOnlySlice _glyphIndices; private ReadOnlySlice _glyphAdvances; @@ -97,9 +97,7 @@ namespace Avalonia.Media { get { - _baselineOrigin ??= CalculateBaselineOrigin(); - - return _baselineOrigin.Value; + return _baselineOrigin; } set => Set(ref _baselineOrigin, value); } @@ -540,15 +538,6 @@ namespace Avalonia.Media return GlyphAdvances[index]; } - /// - /// Calculates the default baseline origin of the . - /// - /// The baseline origin. - private Point CalculateBaselineOrigin() - { - return new Point(0, -GlyphTypeface.Ascent * Scale); - } - /// /// Calculates the size of the . /// @@ -611,8 +600,6 @@ namespace Avalonia.Media throw new InvalidOperationException(); } - _baselineOrigin = new Point(0, -GlyphTypeface.Ascent * Scale); - var platformRenderInterface = AvaloniaLocator.Current.GetService(); _glyphRunImpl = platformRenderInterface.CreateGlyphRun(this, out var width); diff --git a/src/Avalonia.Visuals/Media/TextFormatting/ShapedTextCharacters.cs b/src/Avalonia.Visuals/Media/TextFormatting/ShapedTextCharacters.cs index 09ecc0a026..9f6f2b2f43 100644 --- a/src/Avalonia.Visuals/Media/TextFormatting/ShapedTextCharacters.cs +++ b/src/Avalonia.Visuals/Media/TextFormatting/ShapedTextCharacters.cs @@ -52,7 +52,7 @@ namespace Avalonia.Media.TextFormatting return; } - if (Properties.Typeface == null) + if (Properties.Typeface == default) { return; } diff --git a/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs b/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs index 3e85f0f6f0..4a7282af27 100644 --- a/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs +++ b/src/Avalonia.Visuals/Media/TextFormatting/TextFormatterImpl.cs @@ -43,18 +43,23 @@ namespace Avalonia.Media.TextFormatting } /// - /// Measures the number of characters that fits into available width. + /// Measures the number of characters that fit into available width. /// /// The text run. /// The available width. - /// - internal static int MeasureCharacters(ShapedTextCharacters textCharacters, double availableWidth) + /// The count of fitting characters. + /// + /// true if characters fit into the available width; otherwise, false. + /// + internal static bool TryMeasureCharacters(ShapedTextCharacters textCharacters, double availableWidth, out int count) { var glyphRun = textCharacters.GlyphRun; if (glyphRun.Size.Width < availableWidth) { - return glyphRun.Characters.Length; + count = glyphRun.Characters.Length; + + return true; } var glyphCount = 0; @@ -96,21 +101,34 @@ namespace Avalonia.Media.TextFormatting } } + if (glyphCount == 0) + { + count = 0; + + return false; + } + if (glyphCount == glyphRun.GlyphIndices.Length) { - return glyphRun.Characters.Length; + count = glyphRun.Characters.Length; + + return true; } if (glyphRun.GlyphClusters.IsEmpty) { - return glyphCount; + count = glyphCount; + + return true; } var firstCluster = glyphRun.GlyphClusters[0]; var lastCluster = glyphRun.GlyphClusters[glyphCount]; - return lastCluster - firstCluster; + count = lastCluster - firstCluster; + + return count > 0; } /// @@ -350,29 +368,38 @@ namespace Avalonia.Media.TextFormatting if (currentWidth + currentRun.Size.Width > availableWidth) { - var measuredLength = MeasureCharacters(currentRun, paragraphWidth - currentWidth); - var breakFound = false; var currentBreakPosition = 0; - if (measuredLength < currentRun.Text.Length) + if (TryMeasureCharacters(currentRun, paragraphWidth - currentWidth, out var measuredLength)) { - var lineBreaker = new LineBreakEnumerator(currentRun.Text); - - while (currentBreakPosition < measuredLength && lineBreaker.MoveNext()) + if (measuredLength < currentRun.Text.Length) { - var nextBreakPosition = lineBreaker.Current.PositionWrap; + var lineBreaker = new LineBreakEnumerator(currentRun.Text); - if (nextBreakPosition == 0 || nextBreakPosition > measuredLength) + while (currentBreakPosition < measuredLength && lineBreaker.MoveNext()) { - break; - } + var nextBreakPosition = lineBreaker.Current.PositionWrap; + + if (nextBreakPosition == 0 || nextBreakPosition > measuredLength) + { + break; + } - breakFound = lineBreaker.Current.Required || - lineBreaker.Current.PositionWrap != currentRun.Text.Length; + breakFound = lineBreaker.Current.Required || + lineBreaker.Current.PositionWrap != currentRun.Text.Length; - currentBreakPosition = nextBreakPosition; + currentBreakPosition = nextBreakPosition; + } + } + } + else + { + // Make sure we wrap at least one character. + if (currentLength == 0) + { + measuredLength = 1; } } diff --git a/src/Avalonia.Visuals/Media/TextFormatting/TextLineImpl.cs b/src/Avalonia.Visuals/Media/TextFormatting/TextLineImpl.cs index f5e87d097b..d13b4836ea 100644 --- a/src/Avalonia.Visuals/Media/TextFormatting/TextLineImpl.cs +++ b/src/Avalonia.Visuals/Media/TextFormatting/TextLineImpl.cs @@ -39,7 +39,9 @@ namespace Avalonia.Media.TextFormatting foreach (var textRun in _textRuns) { - using (drawingContext.PushPostTransform(Matrix.CreateTranslation(currentX, 0))) + var offsetY = LineMetrics.TextBaseline; + + using (drawingContext.PushPostTransform(Matrix.CreateTranslation(currentX, offsetY))) { textRun.Draw(drawingContext); } @@ -75,37 +77,35 @@ namespace Avalonia.Media.TextFormatting if (currentWidth > availableWidth) { - var measuredLength = TextFormatterImpl.MeasureCharacters(currentRun, availableWidth); - - var currentBreakPosition = 0; - - if (measuredLength < textRange.End) + if (TextFormatterImpl.TryMeasureCharacters(currentRun, availableWidth, out var measuredLength)) { - var lineBreaker = new LineBreakEnumerator(currentRun.Text); - - while (currentBreakPosition < measuredLength && lineBreaker.MoveNext()) + if (collapsingProperties.Style == TextCollapsingStyle.TrailingWord && measuredLength < textRange.End) { - var nextBreakPosition = lineBreaker.Current.PositionWrap; + var currentBreakPosition = 0; - if (nextBreakPosition == 0) - { - break; - } + var lineBreaker = new LineBreakEnumerator(currentRun.Text); - if (nextBreakPosition > measuredLength) + while (currentBreakPosition < measuredLength && lineBreaker.MoveNext()) { - break; + var nextBreakPosition = lineBreaker.Current.PositionWrap; + + if (nextBreakPosition == 0) + { + break; + } + + if (nextBreakPosition > measuredLength) + { + break; + } + + currentBreakPosition = nextBreakPosition; } - currentBreakPosition = nextBreakPosition; + measuredLength = currentBreakPosition; } } - if (collapsingProperties.Style == TextCollapsingStyle.TrailingWord) - { - measuredLength = currentBreakPosition; - } - collapsedLength += measuredLength; var splitResult = TextFormatterImpl.SplitTextRuns(_textRuns, collapsedLength);