From 648edcdbc4ef0159973c0e4cfae6bc6e4fe56804 Mon Sep 17 00:00:00 2001 From: Benedikt Stebner Date: Wed, 26 Aug 2026 22:08:04 +0000 Subject: [PATCH] [Text] Do not shape the space after fallback text with the fallback font (#22067) * Add failing tests for whitespace absorbed into a fallback run A fallback run is extended for as long as the fallback font has glyphs instead of ending where the primary font regains coverage. Practically every font maps U+0020, so the space that follows fallback text is pulled into the fallback run and shaped with its space glyph. - itemization: a Hebrew letter followed by " b" must produce a 1-char fallback run, not a 2-char one that swallows the space - measurement: the space after an emoji must have the same advance as the same space elsewhere in the line * Merge abutting text bounds within the usual float tolerance The two edges being compared are reached by summing glyph advances along different paths, so abutting bounds can land an ULP apart and a single directional span gets reported as two rectangles. Compare them the way the rest of layout compares coordinates. * End a fallback run where the default typeface regains coverage Whitespace was exempt from the return-to-primary check so that a space wouldn't split a fallback run. Practically every font maps U+0020, so the exemption let a fallback run reach past the text the default typeface couldn't render and shape the following space with the fallback's own advance - a full em in most emoji fonts, which is the long space reported after an emoji. - the check now applies to spacing whitespace (Zs), so a fallback run ends at the first cluster the default typeface can render; control and format codepoints stay exempt - many fonts map the default-ignorable bidi controls, and a default typeface whose cmap merely has such a mark must not pull it out of the fallback run: the mark renders nothing either way, and the split cuts the run for no reason - a default typeface that cannot shape the run's script is still not a return target for text, but it does reclaim the spacing whitespace between the words, which carries no shaping - a run of pure whitespace no longer becomes the anti-thrashing bias for the next run, so the words on either side of a space keep resolving to the same fallback font Test expectations that encoded the old run structure move with it: a line of emoji separated by spaces is no longer a single run, glyph clusters are relative to each run's own text (the affected helpers now add the run's start), and the spaces of a right-to-left line measure with the primary font, which widens those lines. --- .../Media/TextFormatting/TextCharacters.cs | 63 ++++- .../Media/TextFormatting/TextLineImpl.cs | 7 +- .../TextFormatting/TextCharactersTests.cs | 216 ++++++++++++++++++ .../TextFormatting/TextFormatterTests.cs | 23 +- .../Media/TextFormatting/TextLayoutTests.cs | 26 ++- .../Media/TextFormatting/TextLineTests.cs | 41 ++-- 6 files changed, 332 insertions(+), 44 deletions(-) diff --git a/src/Avalonia.Base/Media/TextFormatting/TextCharacters.cs b/src/Avalonia.Base/Media/TextFormatting/TextCharacters.cs index 94fdc5d3e3..591189ae6e 100644 --- a/src/Avalonia.Base/Media/TextFormatting/TextCharacters.cs +++ b/src/Avalonia.Base/Media/TextFormatting/TextCharacters.cs @@ -67,10 +67,36 @@ namespace Avalonia.Media.TextFormatting text = text.Slice(shapeableRun.Length); - previousProperties = shapeableRun.Properties; + // Whitespace says nothing about which font the text around it wants, and it belongs to + // the default typeface whenever that covers it - so a run of pure whitespace must not + // become the anti-thrashing bias for what follows. Otherwise the words on either side + // of a space each resolve their fallback from scratch and can land on different fonts. + if (!IsWhiteSpaceOnly(shapeableRun.Text.Span)) + { + previousProperties = shapeableRun.Properties; + } } } + /// + /// Returns whether every codepoint in is whitespace. Returns on the + /// first codepoint that isn't, so a run of text costs a single lookup. + /// + private static bool IsWhiteSpaceOnly(ReadOnlySpan text) + { + var codepoints = new CodepointEnumerator(text); + + while (codepoints.MoveNext(out var codepoint)) + { + if (!codepoint.IsWhiteSpace) + { + return false; + } + } + + return true; + } + /// /// Creates a shapeable text run with unique properties. /// @@ -149,18 +175,19 @@ namespace Avalonia.Media.TextFormatting GlyphTypeface? fallbackGlyphTypeface = null; var fallbackResolved = false; - // A primary that cannot shape this tier's script is not a valid "return target": pass - // null so the return-to-primary check doesn't hand clusters back to it, which would - // otherwise block a shaping-capable fallback that merely shares the primary's cmap. + // A primary that cannot shape this tier's script is not a valid "return target" for + // text: handing clusters back to it would block a shaping-capable fallback that merely + // shares the primary's cmap. It still reclaims the spacing whitespace between the + // words, which needs no shaping - see TryGetShapeableLength. var defaultCanShape = !requireShapingCapability || defaultGlyphTypeface.CanShapeScript(firstScript); - var primaryForReturn = defaultCanShape ? defaultGlyphTypeface : null; for (var pass = 0; pass < 2; pass++) { var requireFullCluster = pass == 0; if (defaultCanShape && - TryGetShapeableLength(textSpan, defaultGlyphTypeface, null, requireFullCluster, out count)) + TryGetShapeableLength(textSpan, defaultGlyphTypeface, null, defaultCanShapeScript: false, + requireFullCluster, out count)) { // Primary font: the properties already carry this typeface, so reuse them // directly. This avoids a needless copy and preserves a custom @@ -170,7 +197,8 @@ namespace Avalonia.Media.TextFormatting if (allowPreviousTypeface && previousGlyphTypeface is not null && (!requireShapingCapability || previousGlyphTypeface.CanShapeScript(firstScript)) && - TryGetShapeableLength(textSpan, previousGlyphTypeface, primaryForReturn, requireFullCluster, out count)) + TryGetShapeableLength(textSpan, previousGlyphTypeface, defaultGlyphTypeface, defaultCanShape, + requireFullCluster, out count)) { return new UnshapedTextRun(text.Slice(0, count), defaultProperties.WithTypeface(previousTypeface!.Value), biDiLevel); @@ -200,7 +228,8 @@ namespace Avalonia.Media.TextFormatting } if (fallbackGlyphTypeface is not null && - TryGetShapeableLength(textSpan, fallbackGlyphTypeface, primaryForReturn, requireFullCluster, out count)) + TryGetShapeableLength(textSpan, fallbackGlyphTypeface, defaultGlyphTypeface, defaultCanShape, + requireFullCluster, out count)) { return new UnshapedTextRun(text.Slice(0, count), defaultProperties.WithTypeface(fallbackTypeface), biDiLevel); @@ -249,7 +278,12 @@ namespace Avalonia.Media.TextFormatting /// /// The characters to shape. /// The typeface that is used to find matching characters. - /// The default typeface. + /// The default typeface, or null when there is none to + /// return to (the probe for the default typeface itself). + /// + /// Whether the default typeface can shape this run's script. When false it only reclaims + /// spacing whitespace, which needs no shaping. + /// /// /// When true, a grapheme cluster only counts as supported when the typeface has a glyph /// for every scalar it contains (base plus combining marks); when false, only the base @@ -261,6 +295,7 @@ namespace Avalonia.Media.TextFormatting ReadOnlySpan text, GlyphTypeface glyphTypeface, GlyphTypeface? defaultGlyphTypeface, + bool defaultCanShapeScript, bool requireFullCluster, out int length) { @@ -287,8 +322,14 @@ namespace Avalonia.Media.TextFormatting var clusterText = text.Slice(currentGrapheme.Offset, currentGrapheme.Length); - if (!currentCodepoint.IsWhiteSpace - && defaultGlyphTypeface != null + // A fallback run ends where the default typeface regains coverage, spacing whitespace + // included - practically every font maps U+0020, so exempting it would let the run + // shape the following space with the fallback's own advance. A default typeface that + // cannot shape this script still reclaims that whitespace, which carries no shaping. + // Only Zs qualifies: control and format codepoints (bidi controls, prepended number + // signs) keep their cluster with the probed font. + if (defaultGlyphTypeface != null + && (defaultCanShapeScript || currentCodepoint.GeneralCategory == GeneralCategory.SpaceSeparator) && ClusterIsCovered(clusterText, currentCodepoint, defaultGlyphTypeface, requireFullCluster)) { break; diff --git a/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs b/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs index 2289f9508e..16f0e2690b 100644 --- a/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs +++ b/src/Avalonia.Base/Media/TextFormatting/TextLineImpl.cs @@ -834,7 +834,10 @@ namespace Avalonia.Media.TextFormatting return false; } - if (currentBounds.Rectangle.Left == lastBounds.Rectangle.Right) + // The two edges are computed by summing glyph advances along different paths, so + // abutting bounds can land an ULP apart - compare them the way the rest of layout + // compares coordinates, or a single directional span gets reported as two. + if (MathUtilities.AreClose(currentBounds.Rectangle.Left, lastBounds.Rectangle.Right)) { foreach (var runBounds in currentBounds.TextRunBounds) { @@ -846,7 +849,7 @@ namespace Avalonia.Media.TextFormatting return true; } - if (currentBounds.Rectangle.Right == lastBounds.Rectangle.Left) + if (MathUtilities.AreClose(currentBounds.Rectangle.Right, lastBounds.Rectangle.Left)) { for (int i = 0; i < currentBounds.TextRunBounds.Count; i++) { diff --git a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextCharactersTests.cs b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextCharactersTests.cs index afa6be06af..b62ffa59c1 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextCharactersTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextCharactersTests.cs @@ -31,6 +31,13 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting private const string NotoSansScFont = "Avalonia.Skia.UnitTests.Fonts.NotoSansSC-Subset.ttf"; private const string NotoSansJpFont = "Avalonia.Skia.UnitTests.Fonts.NotoSansJP-Subset.ttf"; + // A colour emoji font, of the kind every platform ships: it covers the emoji block and, like + // practically every font, U+0020 - at an advance of its own that is not the primary's. + private const string EmojiFont = "Avalonia.Skia.UnitTests.Assets.TwitterColorEmoji-SVGinOT.ttf"; + + // U+1F642 ๐Ÿ™‚ โ€” covered by the emoji font only. + private const int EmojiCodepoint = 0x1F642; + // U+4E2D ไธญ โ€” a CJK ideograph covered by neither curated font, and with no platform fallback, // so it has no match at all. private const int NoMatchCodepoint = 0x4E2D; @@ -329,6 +336,215 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting } } + // A fallback run must end where the primary font regains coverage, whitespace included. + // Practically every font maps U+0020, so a run that is extended for as long as the fallback + // has glyphs swallows the space that follows the fallback text and shapes it with the + // fallback's space glyph - which is a full em in most emoji fonts. + [Fact] + public void GetShapeableCharacters_Does_Not_Absorb_Whitespace_Into_A_Fallback_Run() + { + using (Start(PrimaryFont, FallbackFont)) + { + var fontManager = FontManager.Current; + + var defaultProperties = new GenericTextRunProperties(Typeface.Default); + var defaultGlyphTypeface = defaultProperties.CachedGlyphTypeface; + var defaultFontFamily = defaultProperties.Typeface.FontFamily; + + // Preconditions: the primary lacks the Hebrew letter but covers both the space and the + // letter after it, and the fallback that covers the Hebrew letter maps the space too - + // which is what lets the fallback run reach past the letter today. + Assert.False(defaultGlyphTypeface.CharacterToGlyphMap.TryGetGlyph(FallbackCodepoint, out _)); + Assert.True(defaultGlyphTypeface.CharacterToGlyphMap.TryGetGlyph(' ', out _)); + Assert.True(defaultGlyphTypeface.CharacterToGlyphMap.TryGetGlyph('b', out _)); + + Assert.True(fontManager.TryMatchCharacter(FallbackCodepoint, FontStyle.Normal, FontWeight.Normal, + FontStretch.Normal, defaultFontFamily, null, out var fallbackTypeface)); + Assert.True(fontManager.TryGetGlyphTypeface(fallbackTypeface, out var fallbackGlyphTypeface)); + Assert.True(fallbackGlyphTypeface.CharacterToGlyphMap.TryGetGlyph(' ', out _)); + + var text = (char.ConvertFromUtf32(FallbackCodepoint) + " b").AsMemory(); + + var textCharacters = new TextCharacters(text, defaultProperties); + + var results = FormattingObjectPool.Instance.TextRunLists.Rent(); + + try + { + TextRunProperties? previousProperties = null; + + textCharacters.GetShapeableCharacters(text, 0, fontManager, ref previousProperties, results); + + Assert.Equal(2, results.Count); + + // The fallback run covers the Hebrew letter only. Before the fix it was 2 characters + // long: the space was pulled into the fallback run and rendered with its metrics. + Assert.Equal(1, results[0].Length); + Assert.Equal(fallbackTypeface, results[0].Properties!.Typeface); + + // The space returns to the primary along with the rest of the text. + Assert.Equal(2, results[1].Length); + Assert.Equal(defaultProperties.Typeface, results[1].Properties!.Typeface); + } + finally + { + FormattingObjectPool.RentedList? toReturn = results; + FormattingObjectPool.Instance.TextRunLists.Return(ref toReturn); + } + } + } + + // The user-visible half of the same defect: the absorbed space is measured with the fallback + // font, so a space typed after an emoji has a different advance than the same space elsewhere + // in the line - a full em with the platform emoji fonts, and a narrower space with the emoji + // font bundled here. Either way it is not the primary's. + // https://github.com/AvaloniaUI/Avalonia/issues/14011 + [Fact] + public void FormatLine_Keeps_A_Space_After_A_Fallback_Run_At_The_Primary_Width() + { + using (Start(PrimaryFont, EmojiFont)) + { + var fontManager = FontManager.Current; + + var defaultProperties = new GenericTextRunProperties(Typeface.Default); + var defaultGlyphTypeface = defaultProperties.CachedGlyphTypeface; + + Assert.False(defaultGlyphTypeface.CharacterToGlyphMap.TryGetGlyph(EmojiCodepoint, out _)); + + Assert.True(fontManager.TryMatchCharacter(EmojiCodepoint, FontStyle.Normal, FontWeight.Normal, + FontStretch.Normal, defaultProperties.Typeface.FontFamily, null, out var emojiTypeface)); + Assert.True(fontManager.TryGetGlyphTypeface(emojiTypeface, out var emojiGlyphTypeface)); + + // The whole point of the test: the two fonts disagree about how wide a space is, so + // whichever font shapes it is directly observable in the line width. + Assert.NotEqual(SpaceAdvanceInEm(defaultGlyphTypeface), SpaceAdvanceInEm(emojiGlyphTypeface), 3); + + var formatter = new TextFormatterImpl(); + + double Width(string text) + { + var textLine = formatter.FormatLine(new SingleBufferTextSource(text, defaultProperties), 0, + double.PositiveInfinity, new GenericTextParagraphProperties(defaultProperties)); + + Assert.NotNull(textLine); + + return textLine.WidthIncludingTrailingWhitespace; + } + + var emoji = char.ConvertFromUtf32(EmojiCodepoint); + + // Isolate the space by differencing, so the surrounding glyphs' advances cancel out. + var plainSpace = Width("a b") - Width("ab"); + var spaceAfterFallback = Width(emoji + " b") - Width(emoji + "b"); + + Assert.Equal(plainSpace, spaceAfterFallback, 3); + } + } + + private static double SpaceAdvanceInEm(GlyphTypeface glyphTypeface) + { + Assert.True(glyphTypeface.CharacterToGlyphMap.TryGetGlyph(' ', out var glyph)); + Assert.True(glyphTypeface.TryGetHorizontalGlyphAdvance(glyph, out var advance)); + + return (double)advance / glyphTypeface.Metrics.DesignEmHeight; + } + + // The previous run's font is reused as an anti-thrashing bias. A space belongs to the primary + // font, so it forms a run of its own between two fallback words - and that run must not become + // the bias, or each word re-runs the fallback search and the two can land on different fonts. + [Fact] + public void GetShapeableCharacters_Keeps_The_Previous_Fallback_Across_A_Space() + { + using (Start(PrimaryFont, NotoSansScFont, NotoSansJpFont)) + { + var fontManager = FontManager.Current; + + var defaultProperties = new GenericTextRunProperties(Typeface.Default); + + // The previous run resolved to the Simplified-Chinese font. + var scTypeface = new Typeface(new FontFamily("fonts:SystemFonts#Noto Sans SC")); + Assert.True(fontManager.TryGetGlyphTypeface(scTypeface, out var scGlyphTypeface)); + + const int han = 0x4E2D; // ไธญ, covered by both regional fonts. + + // Preconditions: the primary covers the space but not the ideograph, the previous font + // covers the ideograph, and a fresh search for it would pick the *other* font - so the + // font of the second run tells us whether the bias survived the space. + Assert.True(defaultProperties.CachedGlyphTypeface.CharacterToGlyphMap.TryGetGlyph(' ', out _)); + Assert.False(defaultProperties.CachedGlyphTypeface.CharacterToGlyphMap.TryGetGlyph(han, out _)); + Assert.True(scGlyphTypeface.CharacterToGlyphMap.TryGetGlyph(han, out _)); + + Assert.True(fontManager.TryMatchCharacter(han, FontStyle.Normal, FontWeight.Normal, + FontStretch.Normal, defaultProperties.Typeface.FontFamily, null, out var freshMatch)); + Assert.True(fontManager.TryGetGlyphTypeface(freshMatch, out var freshGlyphTypeface)); + Assert.Equal("Noto Sans JP", freshGlyphTypeface.FamilyName); + + var text = (" " + char.ConvertFromUtf32(han)).AsMemory(); + + var textCharacters = new TextCharacters(text, defaultProperties); + + var results = FormattingObjectPool.Instance.TextRunLists.Rent(); + + try + { + TextRunProperties? previousProperties = new GenericTextRunProperties(scTypeface); + + textCharacters.GetShapeableCharacters(text, 0, fontManager, ref previousProperties, results); + + Assert.Equal(2, results.Count); + + Assert.Equal(1, results[0].Length); + Assert.Equal(defaultProperties.Typeface, results[0].Properties!.Typeface); + + Assert.True(fontManager.TryGetGlyphTypeface(results[1].Properties!.Typeface, out var runGlyphTypeface)); + Assert.Equal("Noto Sans SC", runGlyphTypeface.FamilyName); + } + finally + { + FormattingObjectPool.RentedList? toReturn = results; + FormattingObjectPool.Instance.TextRunLists.Return(ref toReturn); + } + } + } + + // Only spacing whitespace (Zs) returns to the default typeface. Codepoint.IsWhiteSpace also + // covers control and format codepoints - including the default-ignorable bidi controls, which + // many fonts map. A default typeface that cannot shape the script must not pull a + // right-to-left mark out of the fallback run just because its cmap has it: the mark renders + // nothing either way, and splitting there cuts the run for no reason. + [Fact] + public void TryGetShapeableLength_Does_Not_Reclaim_A_Bidi_Control_As_Whitespace() + { + using (Start(PrimaryFont, FallbackFont)) + { + // DejaVu Sans plays the default: its cmap has the Arabic letter, the right-to-left + // mark and the space, but the test probes the tier where it cannot shape Arabic. + // Cascadia Code plays the probed fallback; it has the letter and needs no glyph for + // the default-ignorable mark. + var defaultGlyphTypeface = new Typeface(FontFamily.Parse( + "resm:Avalonia.Skia.UnitTests.Fonts?assembly=Avalonia.Skia.UnitTests#DejaVu Sans")).GlyphTypeface; + var probedGlyphTypeface = new Typeface(FontFamily.Parse( + "resm:Avalonia.Skia.UnitTests.Fonts?assembly=Avalonia.Skia.UnitTests#Cascadia Code")).GlyphTypeface; + + const int alef = 0x0627; + const int rightToLeftMark = 0x200F; + + Assert.True(probedGlyphTypeface.CharacterToGlyphMap.TryGetGlyph(alef, out _)); + Assert.True(defaultGlyphTypeface.CharacterToGlyphMap.TryGetGlyph(rightToLeftMark, out _)); + Assert.True(defaultGlyphTypeface.CharacterToGlyphMap.TryGetGlyph(' ', out _)); + + // Letter, mark, letter, then a space: the mark stays inside the fallback run, the + // space still returns to the default. + var text = "ุงโ€ุง z"; + + Assert.True(TextCharacters.TryGetShapeableLength(text.AsSpan(), probedGlyphTypeface, + defaultGlyphTypeface, defaultCanShapeScript: false, requireFullCluster: true, + out var length)); + + Assert.Equal(3, length); + } + } + // A spread of combining marks (all grapheme-cluster Extend) likely present in a broad fallback // font but absent from a minimal monospace primary. The F1 test picks the first workable one. private static readonly int[] CombiningMarkCandidates = diff --git a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextFormatterTests.cs b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextFormatterTests.cs index 182c633418..0ffa620822 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextFormatterTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextFormatterTests.cs @@ -319,7 +319,7 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting } [Fact] - public void Should_Produce_A_Single_Fallback_Run() + public void Should_Not_Absorb_Whitespace_Into_A_Fallback_Run() { using (Start()) { @@ -337,7 +337,18 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting Assert.NotNull(textLine); - Assert.Equal(1, textLine.TextRuns.Count); + // Four emoji in a fallback font, separated by three spaces the primary font covers: + // the spaces keep the primary's metrics instead of the emoji font's, so they form + // runs of their own. + Assert.Equal(7, textLine.TextRuns.Count); + + for (var i = 0; i < textLine.TextRuns.Count; i++) + { + var isSpace = i % 2 == 1; + + Assert.Equal(isSpace ? 1 : 2, textLine.TextRuns[i].Length); + Assert.Equal(isSpace, defaultProperties.Typeface == textLine.TextRuns[i].Properties!.Typeface); + } } } @@ -410,11 +421,15 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting } } + // The expectations concatenate the run texts in visual run order, so where the spaces sit + // in the string depends on how the line is cut into runs. Each space is a run of its own + // now (the primary font owns it, not the Hebrew fallback), which regroups the same + // characters - the right-to-left rows below show the same content, differently split. [Theory] [InlineData("one ืฉืชื™ื™ื three ืืจื‘ืข", "one ืฉืชื™ื™ื thrโ€ฆ", FlowDirection.LeftToRight, false)] - [InlineData("one ืฉืชื™ื™ื three ืืจื‘ืข", "โ€ฆthrืฉืชื™ื™ื one", FlowDirection.RightToLeft, false)] + [InlineData("one ืฉืชื™ื™ื three ืืจื‘ืข", "โ€ฆthr ืฉืชื™ื™ื one", FlowDirection.RightToLeft, false)] [InlineData("one ืฉืชื™ื™ื three ืืจื‘ืข", "one ืฉืชื™ื™ืโ€ฆ", FlowDirection.LeftToRight, true)] - [InlineData("one ืฉืชื™ื™ื three ืืจื‘ืข", "โ€ฆืฉืชื™ื™ื one", FlowDirection.RightToLeft, true)] + [InlineData("one ืฉืชื™ื™ื three ืืจื‘ืข", "โ€ฆ ืฉืชื™ื™ื one", FlowDirection.RightToLeft, true)] public void TextTrimming_Should_Trim_Correctly(string text, string trimmed, FlowDirection direction, bool wordEllipsis) { const double Width = 160.0; diff --git a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs index 1d33defa83..d8732ef8bc 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLayoutTests.cs @@ -475,7 +475,7 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting [Theory] [InlineData("โ˜๐Ÿฟ", new int[] { 0 })] - [InlineData("โ˜๐Ÿฟ ab", new int[] { 0, 3, 0, 1 })] + [InlineData("โ˜๐Ÿฟ ab", new int[] { 0, 0, 1, 2 })] [InlineData("ab โ˜๐Ÿฟ", new int[] { 0, 1, 2, 0 })] public void Should_Create_Valid_Clusters_For_Text(string text, int[] clusters) { @@ -985,9 +985,13 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting var textLine = layout.TextLines[0]; + // Runs come in visual order, so TextRuns[0] is the leftmost one - the last word of + // this right-to-left line. Its glyph clusters are relative to its own text, so the + // run's start has to be added to compare them with a text source index. var firstRun = (ShapedTextRun)textLine.TextRuns[0]; - var firstCluster = firstRun.ShapedBuffer[0].GlyphCluster; + var firstCluster = TextTestHelper.GetStartCharIndex(firstRun.Text) + + firstRun.ShapedBuffer[0].GlyphCluster; var characterHit = textLine.GetCharacterHitFromDistance(0); @@ -1059,13 +1063,13 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting return rawClusters; } - // Clusters can be either run-local or text-source relative depending on split history. - if (rawClusters.Min() < runStart) - { - return rawClusters.Select(cluster => cluster + runStart); - } + // A run's clusters are relative to the text it was shaped from, which is its + // own text for a freshly shaped run but the parent's for a split child. The + // smallest cluster is the run's first character either way, so rebasing on it + // maps both onto text source indices. + var baseCluster = rawClusters.Min(); - return rawClusters; + return rawClusters.Select(cluster => cluster - baseCluster + runStart); }).ToList(); var glyphAdvances = shapedRuns.SelectMany(x => x.ShapedBuffer, (_, glyph) => glyph.GlyphAdvance).ToList(); @@ -1101,8 +1105,10 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting [InlineData("mgfg๐Ÿงdf f sdf", "g๐Ÿงd", 20, 40)] [InlineData("ูˆู‡. ูˆู‚ุฏ ุชุนุฑุถ ู„ุงู†ุชู‚ุงุฏุงุช", "ุฏุงุช", 5, 30)] [InlineData("ูˆู‡. ูˆู‚ุฏ ุชุนุฑุถ ู„ุงู†ุชู‚ุงุฏุงุช", "ุชุนุฑุถ", 20, 50)] - [InlineData(" ุนู„ู…ูŠุฉ ๐Ÿ˜ฑูˆู…ุถู„ู„ุฉ ุŒ", " ุนู„ู…ูŠุฉ ๐Ÿ˜ฑูˆู…ุถู„ู„ุฉ ุŒ", 40, 100)] - [InlineData("ููŠ ุนุงู… 2018 ุŒ ุฑูุนุช ู„", "ููŠ ุนุงู… 2018 ุŒ ุฑูุนุช ู„", 100, 120)] + // The spaces of an Arabic line are drawn with the primary font rather than the Arabic + // fallback, which is wider at this size - hence the bands sit above where they used to. + [InlineData(" ุนู„ู…ูŠุฉ ๐Ÿ˜ฑูˆู…ุถู„ู„ุฉ ุŒ", " ุนู„ู…ูŠุฉ ๐Ÿ˜ฑูˆู…ุถู„ู„ุฉ ุŒ", 80, 120)] + [InlineData("ููŠ ุนุงู… 2018 ุŒ ุฑูุนุช ู„", "ููŠ ุนุงู… 2018 ุŒ ุฑูุนุช ู„", 120, 150)] [Theory] public void HitTestTextRange_Range_ValidLength(string text, string textToSelect, double minWidth, double maxWidth) { diff --git a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs index 1ec027a12c..6dc0ac4f2e 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs @@ -1413,11 +1413,14 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting foreach (var textRun in shapedTextRuns) { + // Glyph clusters are relative to the run's own text, so they only line up across a + // multi-run line once the run's start is added - same as BuildGlyphClusters. + var runOffset = TextTestHelper.GetStartCharIndex(textRun.Text); var shapedBuffer = textRun.ShapedBuffer; for (var index = 0; index < shapedBuffer.Length; index++) { - var currentCluster = shapedBuffer[index].GlyphCluster; + var currentCluster = shapedBuffer[index].GlyphCluster + runOffset; var advance = shapedBuffer[index].GlyphAdvance; @@ -1427,13 +1430,10 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting } else { - var rect = rects[index - 1]; - - rects.Remove(rect); - - rect = rect.WithWidth(rect.Width + advance); + // Another glyph of the cluster that produced the last rect: widen it. + var rect = rects[rects.Count - 1]; - rects.Add(rect); + rects[rects.Count - 1] = rect.WithWidth(rect.Width + advance); } currentX += advance; @@ -1571,32 +1571,36 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting Assert.NotNull(textLine); - var textBounds = textLine.GetTextBounds(0, 4); + // Runs come in visual order: the Latin word sits leftmost, then the space, then the + // Hebrew word. The space belongs to the primary font, so it is a run of its own. + var latinRun = Assert.IsType(textLine.TextRuns[0]); + var spaceRun = Assert.IsType(textLine.TextRuns[1]); + var hebrewRun = Assert.IsType(textLine.TextRuns[2]); - var secondRun = Assert.IsType(textLine.TextRuns[1]); + var hebrewAndSpaceWidth = hebrewRun.Size.Width + spaceRun.Size.Width; + + var textBounds = textLine.GetTextBounds(0, 4); Assert.Equal(1, textBounds.Count); - Assert.Equal(secondRun.Size.Width, textBounds.Sum(x => x.Rectangle.Width)); + Assert.Equal(hebrewAndSpaceWidth, textBounds.Sum(x => x.Rectangle.Width)); textBounds = textLine.GetTextBounds(4, 3); - var firstRun = Assert.IsType(textLine.TextRuns[0]); - Assert.Equal(1, textBounds.Count); Assert.Equal(3, textBounds[0].TextRunBounds.Sum(x => x.Length)); - Assert.Equal(firstRun.Size.Width, textBounds.Sum(x => x.Rectangle.Width)); + Assert.Equal(latinRun.Size.Width, textBounds.Sum(x => x.Rectangle.Width)); textBounds = textLine.GetTextBounds(0, 5); Assert.Equal(2, textBounds.Count); Assert.Equal(5, textBounds.Sum(x => x.TextRunBounds.Sum(x => x.Length))); - Assert.Equal(secondRun.Size.Width, textBounds[1].Rectangle.Width); + Assert.Equal(hebrewAndSpaceWidth, textBounds[1].Rectangle.Width); Assert.Equal(7.201171875, textBounds[0].Rectangle.Width); Assert.Equal(textLine.Start + 7.201171875, textBounds[0].Rectangle.Right, 2); - Assert.Equal(textLine.Start + firstRun.Size.Width, textBounds[1].Rectangle.Left, 2); + Assert.Equal(textLine.Start + latinRun.Size.Width, textBounds[1].Rectangle.Left, 2); textBounds = textLine.GetTextBounds(0, text.Length); @@ -1737,13 +1741,16 @@ namespace Avalonia.Skia.UnitTests.Media.TextFormatting Assert.Equal(1, bounds.Count); - Assert.Equal(71.165859375, bounds[0].Rectangle.Right); + // The space between the Hebrew word and the digits is drawn with the primary font + // rather than the Hebrew fallback, which is 4.08 wider at this size, so everything + // laid out after it sits that much further right. + Assert.Equal(75.247031249999992, bounds[0].Rectangle.Right); bounds = textLine.GetTextBounds(11, 1); Assert.Equal(1, bounds.Count); - Assert.Equal(71.165859375, bounds[0].Rectangle.Left); + Assert.Equal(75.247031249999992, bounds[0].Rectangle.Left); bounds = textLine.GetTextBounds(0, 25);