Merge pull request #541 from SixLabors/js/interpreter-pool-reset

Fix TrueTypeInterpreter pool state pollution

Author: JimBobSquarePants

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

@@ -162,11 +162,15 @@ public void InitializeFunctionDefs(byte[] instructions)
     /// <param name="cvProgram">The raw prep program bytecode, or <see langword="null"/> if absent.</param>
     public void SetControlValueTable(short[]? cvt, float scale, float ppem, byte[]? cvProgram)
     {
-        if (this.scale == scale || cvt == null)
+        if (this.scale == scale)
         {
             return;
         }
-        else
+
+        // A missing CVT table must not skip the prep program: fonts may carry a prep
+        // program without control values, and prep still establishes the graphics state,
+        // storage, and twilight points that glyph programs build on.
+        if (cvt != null)
         {
             if (this.controlValueTable.Length == 0 && cvt.Length > 0)
             {
@@ -175,16 +179,52 @@ public void SetControlValueTable(short[]? cvt, float scale, float ppem, byte[]?
 
             for (int i = 0; i < cvt.Length; i++)
             {
-                this.controlValueTable[i] = cvt[i] * scale;
+                // Match FreeType's tt_size_run_prep CVT scaling, which produces 26.6
+                // fixed-point pixel values (FT_MulFix rounds to the nearest 1/64).
+                // Scaling in unquantized float lets control values land on the other
+                // side of a rounding boundary, flipping prep's round-to-grid decisions
+                // for some sizes (Arial's x-height at 13 ppem rounds to 8px instead of
+                // FreeType's 7px). FreeType notes the operation is "very sensitive to
+                // rounding".
+                this.controlValueTable[i] = MathF.Round(cvt[i] * scale * 64F) / 64F;
             }
         }
+        else
+        {
+            this.controlValueTable = [];
+        }
 
         this.scale = scale;
         this.ppem = (int)Math.Round(ppem);
-        this.zp0 = this.zp1 = this.zp2 = this.points;
         this.state.Reset();
         this.stack.Clear();
 
+        // Restore the interpreter to the same state a freshly created interpreter would be in
+        // immediately before running the prep program. A pooled interpreter may have been used
+        // to hint glyphs at a previous size, leaving behind storage writes, twilight points,
+        // rounding state and zone pointers. The prep program reads and builds on this state, so
+        // without restoring it the prep result — and therefore the hinted outline — depends on
+        // the interpreter's history. That made hinting non-deterministic when a font family was
+        // rendered concurrently from a shared interpreter pool (see issue #484).
+        // FreeType does the same in tt_size_run_prep (ttobjs.c): it zeroes the twilight zone
+        // and the storage area before every prep execution, deliberately discarding any
+        // storage writes made by the font program (fpgm).
+        this.ResetTwilightZone();
+        Array.Clear(this.storage, 0, this.storage.Length);
+        this.prepStorage = null;
+        this.inGlyphProgram = false;
+        this.callStackSize = 0;
+        this.fdotp = 0;
+        this.roundThreshold = 0;
+        this.roundPhase = 0;
+        this.roundPeriod = 0;
+        this.iupXCalled = false;
+        this.iupYCalled = false;
+        this.isComposite = false;
+        this.contours = [];
+        this.points = default;
+        this.zp0 = this.zp1 = this.zp2 = this.points;
+
         if (cvProgram != null)
         {
             // Initialize safety counters for the prep program (no glyph points yet).

tests/Images/ReferenceOutput/Test_Hinting_Robustness_-Arial-.png

@@ -1,7 +1,9 @@
 // Copyright (c) Six Labors.
 // Licensed under the Six Labors Split License.
 
+using System.Numerics;
 using System.Text;
+using SixLabors.Fonts.Rendering;
 using SixLabors.Fonts.Unicode;
 
 namespace SixLabors.Fonts.Tests;
@@ -70,4 +72,58 @@ public void Test_Hinting_Robustness(string path, string name)
             options,
             properties: name);
     }
+
+    // The TrueType bytecode interpreter is pooled and reused across renders for the same
+    // font. When a pooled interpreter is reused for a different pixel size it re-runs the
+    // font's prep (CVT) program, which must execute from the same pristine state as a freshly
+    // created interpreter. If transient interpreter state (twilight zone, storage, rounding
+    // state, zone pointers, ...) is not reset first, the prep result — and therefore the
+    // hinted glyph outline — depends on which sizes were rendered previously on that
+    // interpreter. Because interpreters are shared through a pool, that made hinting output
+    // non-deterministic when a single font family was rendered concurrently from multiple
+    // threads
+    [Fact]
+    public void Hinting_OutputIsIndependentOfPreviouslyRenderedSizes()
+    {
+        const string text = "The quick brown fox 12345";
+        const float dpi = 150F;
+        const float targetSize = 7F;
+        const float otherSize = 12F;
+
+        static List<Vector2> RenderControlPoints(string text, float size, float dpi, float? warmUpSize)
+        {
+            FontCollection collection = new();
+            FontFamily family = collection.Add(TestFonts.Arial);
+
+            if (warmUpSize is { } w)
+            {
+                RenderTo(family, text, w, dpi, new GlyphRenderer());
+            }
+
+            GlyphRenderer renderer = new();
+            RenderTo(family, text, size, dpi, renderer);
+            return renderer.ControlPoints;
+        }
+
+        static void RenderTo(FontFamily family, string text, float size, float dpi, GlyphRenderer renderer)
+        {
+            Font font = family.CreateFont(size);
+            TextOptions options = new(font)
+            {
+                Dpi = dpi,
+                HintingMode = HintingMode.Standard,
+            };
+
+            TextRenderer.RenderTextTo(renderer, text, options);
+        }
+
+        // Render the target size on a font whose interpreter has processed nothing else.
+        List<Vector2> reference = RenderControlPoints(text, targetSize, dpi, warmUpSize: null);
+
+        // Render the same target size, but on a font whose pooled interpreter has already
+        // processed a different size. With a correct per-size reset this is byte-for-byte equal.
+        List<Vector2> afterOtherSize = RenderControlPoints(text, targetSize, dpi, warmUpSize: otherSize);
+
+        Assert.Equal(reference, afterOtherSize);
+    }
 }