Fix #484

Author: JimBobSquarePants

src/SixLabors.Fonts/Buffer{T}.cs

@@ -27,7 +27,7 @@ public Buffer(int length)
         this.length = length;
 
         using ByteMemoryManager<T> manager = new(this.buffer);
-        this.Memory = manager.Memory.Slice(0, this.length);
+        this.Memory = manager.Memory[..this.length];
         this.span = this.Memory.Span;
 
         this.isDisposed = false;

src/SixLabors.Fonts/ObjectPool{T}.cs

@@ -0,0 +1,127 @@
+// Copyright (c) Six Labors.
+// Licensed under the Six Labors Split License.
+
+using System.Collections.Concurrent;
+
+namespace SixLabors.Fonts;
+
+/// <summary>
+/// A pool for reusing objects of type <typeparamref name="T"/>.
+/// </summary>
+/// <typeparam name="T">The type to pool objects for.</typeparam>
+/// <remarks>
+/// This implementation keeps a cache of retained objects.
+/// This means that if objects are returned when the pool has already reached "maximumRetained" objects they will be available to be Garbage Collected.
+/// </remarks>
+internal sealed class ObjectPool<T>
+    where T : class
+{
+    private readonly Func<T> createFunc;
+    private readonly Func<T, bool> returnFunc;
+    private readonly int maxCapacity;
+    private int numItems;
+
+    private readonly ConcurrentQueue<T> items = new();
+    private T? fastItem;
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="ObjectPool{T}"/> class.
+    /// </summary>
+    /// <param name="policy">The pooling policy to use.</param>
+    public ObjectPool(IPooledObjectPolicy<T> policy)
+        : this(policy, Environment.ProcessorCount * 2)
+    {
+    }
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="ObjectPool{T}"/> class.
+    /// </summary>
+    /// <param name="policy">The pooling policy to use.</param>
+    /// <param name="maximumRetained">The maximum number of objects to retain in the pool.</param>
+    public ObjectPool(IPooledObjectPolicy<T> policy, int maximumRetained)
+    {
+        // cache the target interface methods, to avoid interface lookup overhead
+        this.createFunc = policy.Create;
+        this.returnFunc = policy.Return;
+        this.maxCapacity = maximumRetained - 1;  // -1 to account for _fastItem
+    }
+
+    /// <summary>
+    /// Gets an object from the pool if one is available, otherwise creates one.
+    /// </summary>
+    /// <returns>A <typeparamref name="T"/>.</returns>
+    public T Get()
+    {
+        T? item = this.fastItem;
+        if (item == null || Interlocked.CompareExchange(ref this.fastItem, null, item) != item)
+        {
+            if (this.items.TryDequeue(out item))
+            {
+                _ = Interlocked.Decrement(ref this.numItems);
+                return item;
+            }
+
+            // no object available, so go get a brand new one
+            return this.createFunc();
+        }
+
+        return item;
+    }
+
+    /// <summary>
+    /// Return an object to the pool.
+    /// </summary>
+    /// <param name="obj">The object to add to the pool.</param>
+    public void Return(T obj) => this.ReturnCore(obj);
+
+    /// <summary>
+    /// Returns an object to the pool.
+    /// </summary>
+    /// <returns>true if the object was returned to the pool</returns>
+    private bool ReturnCore(T obj)
+    {
+        if (!this.returnFunc(obj))
+        {
+            // policy says to drop this object
+            return false;
+        }
+
+        if (this.fastItem != null || Interlocked.CompareExchange(ref this.fastItem, obj, null) != null)
+        {
+            if (Interlocked.Increment(ref this.numItems) <= this.maxCapacity)
+            {
+                this.items.Enqueue(obj);
+                return true;
+            }
+
+            // no room, clean up the count and drop the object on the floor
+            _ = Interlocked.Decrement(ref this.numItems);
+            return false;
+        }
+
+        return true;
+    }
+}
+
+/// <summary>
+/// Represents a policy for managing pooled objects.
+/// </summary>
+/// <typeparam name="T">The type of object which is being pooled.</typeparam>
+#pragma warning disable SA1201 // Elements should appear in the correct order
+internal interface IPooledObjectPolicy<T>
+#pragma warning restore SA1201 // Elements should appear in the correct order
+    where T : notnull
+{
+    /// <summary>
+    /// Create a <typeparamref name="T"/>.
+    /// </summary>
+    /// <returns>The <typeparamref name="T"/> which was created.</returns>
+    public T Create();
+
+    /// <summary>
+    /// Runs some processing when an object was returned to the pool. Can be used to reset the state of an object and indicate if the object should be returned to the pool.
+    /// </summary>
+    /// <param name="obj">The object to return to the pool.</param>
+    /// <returns><see langword="true" /> if the object should be returned to the pool. <see langword="false" /> if it's not possible/desirable for the pool to keep the object.</returns>
+    public bool Return(T obj);
+}

src/SixLabors.Fonts/StreamFontMetrics.TrueType.cs

@@ -22,7 +22,30 @@ namespace SixLabors.Fonts;
 /// </content>
 internal partial class StreamFontMetrics
 {
-    private TrueTypeInterpreter? interpreter;
+    // Bounded pool of interpreters shared across threads.
+    // Size tied to logical CPU count.
+    private readonly ObjectPool<TrueTypeInterpreter>? interpreterPool;
+
+    private TrueTypeInterpreter CreateInterpreter()
+    {
+        TrueTypeFontTables tables = this.trueTypeFontTables!;
+        MaximumProfileTable maxp = tables.Maxp;
+
+        TrueTypeInterpreter interpreter = new(
+            maxp.MaxStackElements,
+            maxp.MaxStorage,
+            maxp.MaxFunctionDefs,
+            maxp.MaxInstructionDefs,
+            maxp.MaxTwilightPoints);
+
+        FpgmTable? fpgm = tables.Fpgm;
+        if (fpgm is not null)
+        {
+            interpreter.InitializeFunctionDefs(fpgm.Instructions);
+        }
+
+        return interpreter;
+    }
 
     internal void ApplyTrueTypeHinting(HintingMode hintingMode, GlyphMetrics metrics, ref GlyphVector glyphVector, Vector2 scaleXY, float pixelSize)
     {
@@ -31,37 +54,34 @@ internal void ApplyTrueTypeHinting(HintingMode hintingMode, GlyphMetrics metrics
             return;
         }
 
-        TrueTypeFontTables tables = this.trueTypeFontTables!;
-        if (this.interpreter == null)
+        if (this.trueTypeFontTables is null || this.interpreterPool is null)
         {
-            MaximumProfileTable maxp = tables.Maxp;
-            this.interpreter = new TrueTypeInterpreter(
-                maxp.MaxStackElements,
-                maxp.MaxStorage,
-                maxp.MaxFunctionDefs,
-                maxp.MaxInstructionDefs,
-                maxp.MaxTwilightPoints);
-
-            FpgmTable? fpgm = tables.Fpgm;
-            if (fpgm is not null)
-            {
-                this.interpreter.InitializeFunctionDefs(fpgm.Instructions);
-            }
+            return;
         }
 
-        CvtTable? cvt = tables.Cvt;
-        PrepTable? prep = tables.Prep;
-        float scaleFactor = pixelSize / this.UnitsPerEm;
-        this.interpreter.SetControlValueTable(cvt?.ControlValues, scaleFactor, pixelSize, prep?.Instructions);
+        TrueTypeFontTables tables = this.trueTypeFontTables;
+        TrueTypeInterpreter interpreter = this.interpreterPool.Get();
 
-        Bounds bounds = glyphVector.Bounds;
+        try
+        {
+            CvtTable? cvt = tables.Cvt;
+            PrepTable? prep = tables.Prep;
+            float scaleFactor = pixelSize / this.UnitsPerEm;
+            interpreter.SetControlValueTable(cvt?.ControlValues, scaleFactor, pixelSize, prep?.Instructions);
+
+            Bounds bounds = glyphVector.Bounds;
 
-        Vector2 pp1 = new(MathF.Round(bounds.Min.X - (metrics.LeftSideBearing * scaleXY.X)), 0);
-        Vector2 pp2 = new(MathF.Round(pp1.X + (metrics.AdvanceWidth * scaleXY.X)), 0);
-        Vector2 pp3 = new(0, MathF.Round(bounds.Max.Y + (metrics.TopSideBearing * scaleXY.Y)));
-        Vector2 pp4 = new(0, MathF.Round(pp3.Y - (metrics.AdvanceHeight * scaleXY.Y)));
+            Vector2 pp1 = new(MathF.Round(bounds.Min.X - (metrics.LeftSideBearing * scaleXY.X)), 0);
+            Vector2 pp2 = new(MathF.Round(pp1.X + (metrics.AdvanceWidth * scaleXY.X)), 0);
+            Vector2 pp3 = new(0, MathF.Round(bounds.Max.Y + (metrics.TopSideBearing * scaleXY.Y)));
+            Vector2 pp4 = new(0, MathF.Round(pp3.Y - (metrics.AdvanceHeight * scaleXY.Y)));
 
-        GlyphVector.Hint(hintingMode, ref glyphVector, this.interpreter, pp1, pp2, pp3, pp4);
+            GlyphVector.Hint(hintingMode, ref glyphVector, interpreter, pp1, pp2, pp3, pp4);
+        }
+        finally
+        {
+            this.interpreterPool.Return(interpreter);
+        }
     }
 
     private static StreamFontMetrics LoadTrueTypeFont(FontReader reader)
@@ -224,4 +244,19 @@ private GlyphMetrics CreateTrueTypeGlyphMetrics(
             textDecorations,
             glyphType);
     }
+
+    private sealed class TrueTypeInterpreterPooledObjectPolicy
+     : IPooledObjectPolicy<TrueTypeInterpreter>
+    {
+        private readonly StreamFontMetrics owner;
+
+        public TrueTypeInterpreterPooledObjectPolicy(StreamFontMetrics owner)
+            => this.owner = owner;
+
+        public TrueTypeInterpreter Create()
+            => this.owner.CreateInterpreter();
+
+        public bool Return(TrueTypeInterpreter interpreter)
+            => true; // Always accept returned instances.
+    }
 }

src/SixLabors.Fonts/StreamFontMetrics.cs

@@ -11,6 +11,7 @@
 using SixLabors.Fonts.Tables.General.Kern;
 using SixLabors.Fonts.Tables.General.Post;
 using SixLabors.Fonts.Tables.TrueType;
+using SixLabors.Fonts.Tables.TrueType.Hinting;
 using SixLabors.Fonts.Unicode;
 
 namespace SixLabors.Fonts;
@@ -84,6 +85,8 @@ internal StreamFontMetrics(CompactFontTables tables)
         (HorizontalMetrics HorizontalMetrics, VerticalMetrics VerticalMetrics) metrics = this.Initialize(tables);
         this.horizontalMetrics = metrics.HorizontalMetrics;
         this.verticalMetrics = metrics.VerticalMetrics;
+
+        this.interpreterPool = new ObjectPool<TrueTypeInterpreter>(new TrueTypeInterpreterPooledObjectPolicy(this));
     }
 
     public HeadTable.HeadFlags HeadFlags { get; private set; }

src/SixLabors.Fonts/Tables/TrueType/Hinting/TrueTypeInterpreter.cs

@@ -76,6 +76,7 @@ public void SetControlValueTable(short[]? cvt, float scale, float ppem, byte[]?
                 this.controlValueTable = new float[cvt.Length];
             }
 
+            // TODO: How about SIMD here? Will the JIT vectorize this?
             for (int i = 0; i < cvt.Length; i++)
             {
                 this.controlValueTable[i] = cvt[i] * scale;

tests/SixLabors.Fonts.Tests/Issues/Issues_484.cs

@@ -0,0 +1,51 @@
+// Copyright (c) Six Labors.
+// Licensed under the Six Labors Split License.
+
+namespace SixLabors.Fonts.Tests.Issues;
+
+public class Issues_484
+{
+    [Fact]
+    public void Test_Issue_484()
+        => Parallel.For(0, 2, static _ => Test_Issue_484_Core());
+
+    private static void Test_Issue_484_Core()
+    {
+        FontCollection fontCollection = new();
+        string arial = fontCollection.Add(TestFonts.Arial).Name;
+
+        FontFamily arialFamily = fontCollection.Get(arial);
+        Font arialFont = arialFamily.CreateFont(12, FontStyle.Regular);
+
+        TextOptions textOptions = new(arialFont)
+        {
+            HintingMode = HintingMode.Standard
+        };
+
+        FontRectangle advance = TextMeasurer.MeasureAdvance("Hello, World!", textOptions);
+        Assert.NotEqual(FontRectangle.Empty, advance);
+    }
+
+    [Fact]
+    public void Test_Issue_484_B()
+    {
+        FontCollection fontCollection = new();
+        string arial = fontCollection.Add(TestFonts.Arial).Name;
+
+        FontFamily arialFamily = fontCollection.Get(arial);
+        Font arialFont = arialFamily.CreateFont(12, FontStyle.Regular);
+
+        Parallel.For(0, 2, _ => Test_Issue_484_Core_B(arialFont));
+    }
+
+    private static void Test_Issue_484_Core_B(Font font)
+    {
+        TextOptions textOptions = new(font)
+        {
+            HintingMode = HintingMode.Standard
+        };
+
+        FontRectangle advance = TextMeasurer.MeasureAdvance("Hello, World!", textOptions);
+        Assert.NotEqual(FontRectangle.Empty, advance);
+    }
+}