Ensure GlyphRunImpl has consistent bounds (#12765)

* Add GlyphRun InkBounds failing test * Ensure GlyphRunImpl has consistent bounds
AvaloniaUI · Oct 2, 2023 · d8655a4 · d8655a4
1 parent 5297811
commit d8655a4
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 24 deletions.
diff --git a/src/Skia/Avalonia.Skia/GlyphRunImpl.cs b/src/Skia/Avalonia.Skia/GlyphRunImpl.cs
@@ -25,13 +25,14 @@ public GlyphRunImpl(IGlyphTypeface glyphTypeface, double fontRenderingEmSize,
                 throw new ArgumentNullException(nameof(glyphTypeface));
             }
 
-            _glyphTypefaceImpl = (GlyphTypefaceImpl)glyphTypeface;
-
             if (glyphInfos == null)
             {
                 throw new ArgumentNullException(nameof(glyphInfos));
             }
 
+            _glyphTypefaceImpl = (GlyphTypefaceImpl)glyphTypeface;
+            FontRenderingEmSize = fontRenderingEmSize;
+
             var count = glyphInfos.Count;
             _glyphIndices = new ushort[count];
             _glyphPositions = new SKPoint[count];
@@ -50,16 +51,21 @@ public GlyphRunImpl(IGlyphTypeface glyphTypeface, double fontRenderingEmSize,
                 currentX += glyphInfos[i].GlyphAdvance;
             }
 
-            _glyphTypefaceImpl.SKFont.Size = (float)fontRenderingEmSize;
+            // Ideally the requested edging should be passed to the glyph run.
+            // Currently the edging is computed dynamically inside the drawing context, so we can't know it in advance.
+            // But the bounds depends on the edging: for now, always use SubpixelAntialias so we have consistent values.
+            // The resulting bounds may be shifted by 1px on some fonts:
+            // "F" text with Inter size 14 has a 0px left bound with SubpixelAntialias but 1px with Antialias.
+            using var font = CreateFont(SKFontEdging.SubpixelAntialias);
 
             var runBounds = new Rect();
-            var glyphBounds = ArrayPool<SKRect>.Shared.Rent(glyphInfos.Count);
+            var glyphBounds = ArrayPool<SKRect>.Shared.Rent(count);
 
-            _glyphTypefaceImpl.SKFont.GetGlyphWidths(_glyphIndices, null, glyphBounds);
+            font.GetGlyphWidths(_glyphIndices, null, glyphBounds.AsSpan(0, count));
 
             currentX = 0;
 
-            for (var i = 0; i < glyphInfos.Count; i++)
+            for (var i = 0; i < count; i++)
             {
                 var gBounds = glyphBounds[i];
                 var advance = glyphInfos[i].GlyphAdvance;
@@ -71,7 +77,6 @@ public GlyphRunImpl(IGlyphTypeface glyphTypeface, double fontRenderingEmSize,
 
             ArrayPool<SKRect>.Shared.Return(glyphBounds);
 
-            FontRenderingEmSize = fontRenderingEmSize;
             BaselineOrigin = baselineOrigin;
             Bounds = runBounds;
         }
@@ -103,12 +108,7 @@ public SKTextBlob GetTextBlob(RenderOptions renderOptions)
 
             return _textBlobCache.GetOrAdd(edging, (_) =>
             {
-                var font = _glyphTypefaceImpl.SKFont;
-
-                font.Hinting = SKFontHinting.Full;
-                font.Subpixel = edging == SKFontEdging.SubpixelAntialias;
-                font.Edging = edging;
-                font.Size = (float)FontRenderingEmSize;
+                using var font = CreateFont(edging);
 
                 var builder = SKTextBlobBuilderCache.Shared.Get();
 
@@ -125,6 +125,17 @@ public SKTextBlob GetTextBlob(RenderOptions renderOptions)
             });
         }
 
+        private SKFont CreateFont(SKFontEdging edging)
+        {
+            var font = _glyphTypefaceImpl.CreateSKFont((float)FontRenderingEmSize);
+
+            font.Hinting = SKFontHinting.Full;
+            font.Subpixel = edging == SKFontEdging.SubpixelAntialias;
+            font.Edging = edging;
+
+            return font;
+        }
+
         public void Dispose()
         {
             foreach (var pair in _textBlobCache)

diff --git a/src/Skia/Avalonia.Skia/GlyphTypefaceImpl.cs b/src/Skia/Avalonia.Skia/GlyphTypefaceImpl.cs
@@ -15,13 +15,6 @@ public GlyphTypefaceImpl(SKTypeface typeface, FontSimulations fontSimulations)
         {
             _typeface = typeface ?? throw new ArgumentNullException(nameof(typeface));
 
-            SKFont = new SKFont(typeface)
-            {
-                LinearMetrics = true,
-                Embolden = (fontSimulations & FontSimulations.Bold) != 0,
-                SkewX = (fontSimulations & FontSimulations.Oblique) != 0 ? -0.2f : 0
-            };
-
             Face = new Face(GetTable)
             {
                 UnitsPerEm = typeface.UnitsPerEm
@@ -67,8 +60,6 @@ public GlyphTypefaceImpl(SKTypeface typeface, FontSimulations fontSimulations)
 
         public Font Font { get; }
 
-        public SKFont SKFont { get; }
-
         public FontSimulations FontSimulations { get; }
 
         public int ReplacementCodepoint { get; }
@@ -170,6 +161,13 @@ public int[] GetGlyphAdvances(ReadOnlySpan<ushort> glyphs)
                 new Blob(data, size, MemoryMode.ReadOnly, releaseDelegate) : null;
         }
 
+        public SKFont CreateSKFont(float size)
+            => new(_typeface, size, skewX: (FontSimulations & FontSimulations.Oblique) != 0 ? -0.2f : 0.0f)
+            {
+                LinearMetrics = true,
+                Embolden = (FontSimulations & FontSimulations.Bold) != 0
+            };
+
         private void Dispose(bool disposing)
         {
             if (_isDisposed)

diff --git a/src/Skia/Avalonia.Skia/PlatformRenderInterface.cs b/src/Skia/Avalonia.Skia/PlatformRenderInterface.cs
@@ -81,9 +81,8 @@ public IGeometryImpl BuildGlyphRunGeometry(GlyphRun glyphRun)
 
             var fontRenderingEmSize = (float)glyphRun.FontRenderingEmSize;
 
-            var skFont = glyphTypeface.SKFont;
+            using var skFont = glyphTypeface.CreateSKFont(fontRenderingEmSize);
 
-            skFont.Size = fontRenderingEmSize;
             skFont.Hinting = SKFontHinting.None;
 
             SKPath path = new SKPath();

diff --git a/tests/Avalonia.RenderTests/Assets/Inter-Regular.ttf b/tests/Avalonia.RenderTests/Assets/Inter-Regular.ttf
diff --git a/tests/Avalonia.Skia.UnitTests/Media/GlyphRunTests.cs b/tests/Avalonia.Skia.UnitTests/Media/GlyphRunTests.cs
@@ -155,6 +155,26 @@ public void Should_Get_CharacterHit_From_Distance(string text, sbyte direction)
             }
         }
 
+        // https://github.com/AvaloniaUI/Avalonia/issues/12676
+        [Fact]
+        public void Similar_Runs_Have_Same_InkBounds_After_Blob_Creation()
+        {
+            using (Start())
+            {
+                var typeface = new Typeface("resm:Avalonia.Skia.UnitTests.Assets?assembly=Avalonia.Skia.UnitTests#Inter");
+                var options = new TextShaperOptions(typeface.GlyphTypeface, 14);
+                var shapedBuffer = TextShaper.Current.ShapeText("F", options);
+
+                var glyphRun1 = CreateGlyphRun(shapedBuffer);
+                var bounds1 = glyphRun1.InkBounds;
+                ((GlyphRunImpl)glyphRun1.PlatformImpl.Item).GetTextBlob(new RenderOptions { TextRenderingMode = TextRenderingMode.SubpixelAntialias });
+
+                var bounds2 = CreateGlyphRun(shapedBuffer).InkBounds;
+
+                Assert.Equal(bounds1, bounds2);
+            }
+        }
+
         private static List<Rect> BuildRects(GlyphRun glyphRun)
         {
             var height = glyphRun.Bounds.Height;