Use a combination of exception handling and compatibility lists for tricky fonts

Author: JimBobSquarePants

src/SixLabors.Fonts/Tables/TrueType/Glyphs/GlyphVector.cs

@@ -92,11 +92,12 @@ public static void Hint(
             controlPoints[i] = glyph.ControlPoints[i];
         }
 
-        interpreter.HintGlyph(controlPoints, glyph.EndPoints, glyph.Instructions, glyph.IsComposite);
-
-        for (int i = 0; i < glyph.ControlPoints.Count; i++)
+        if (interpreter.TryHintGlyph(controlPoints, glyph.EndPoints, glyph.Instructions, glyph.IsComposite))
         {
-            glyph.ControlPoints[i] = controlPoints[i];
+            for (int i = 0; i < glyph.ControlPoints.Count; i++)
+            {
+                glyph.ControlPoints[i] = controlPoints[i];
+            }
         }
     }
 

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

@@ -41,6 +41,7 @@ internal class TrueTypeInterpreter
     private readonly ExecutionStack stack;
     private readonly InstructionStream[] functions;
     private readonly InstructionStream[] instructionDefs;
+    private float[] baseControlValueTable;
     private float[] controlValueTable;
     private readonly int[] storage;
     private IReadOnlyList<ushort> contours;
@@ -66,7 +67,9 @@ internal class TrueTypeInterpreter
     private const int MaxCallStack = 128;
     private const float Epsilon = 0.000001F;
 
+#if DEBUG
     private readonly List<OpCode> debugList = [];
+#endif
 
     public TrueTypeInterpreter(int maxStack, int maxStorage, int maxFunctions, int maxInstructionDefs, int maxTwilightPoints)
     {
@@ -78,6 +81,7 @@ public TrueTypeInterpreter(int maxStack, int maxStorage, int maxFunctions, int m
         this.cvtState = default;
         this.twilight = new Zone(maxTwilightPoints, isTwilight: true);
         this.controlValueTable = [];
+        this.baseControlValueTable = [];
         this.contours = Array.Empty<ushort>();
     }
 
@@ -129,51 +133,142 @@ public void SetControlValueTable(short[]? cvt, float scale, float ppem, byte[]?
                 this.cvtState.Loop = 1;
             }
         }
+
+        if (this.controlValueTable.Length > 0)
+        {
+            if (this.baseControlValueTable.Length != this.controlValueTable.Length)
+            {
+                this.baseControlValueTable = new float[this.controlValueTable.Length];
+            }
+
+            Array.Copy(this.controlValueTable, this.baseControlValueTable, this.controlValueTable.Length);
+        }
+        else
+        {
+            this.baseControlValueTable = [];
+        }
     }
 
-    public void HintGlyph(
+    /// <summary>
+    /// Attempts to apply TrueType hinting instructions to the specified glyph outline.
+    /// </summary>
+    /// <remarks>
+    /// Hinting will not be applied if the instructions buffer is empty or if grid fitting is
+    /// inhibited by the current interpreter state. If the instructions are malformed or an error occurs during
+    /// execution, the method returns <see langword="false"/> and the glyph outline remains unhinted.
+    /// </remarks>
+    /// <param name="controlPoints">An array of control points representing the glyph's outline to be hinted.</param>
+    /// <param name="endPoints">A read-only list of indices indicating the end points of each contour in the glyph.</param>
+    /// <param name="instructions">A read-only memory buffer containing the TrueType hinting instructions to execute.</param>
+    /// <param name="isComposite">Indicates whether the glyph is a composite glyph. Set to <see langword="true"/> for composite glyphs; otherwise, <see langword="false"/>.</param>
+    /// <returns><see langword="true"/> if hinting was successfully applied; otherwise, <see langword="false"/>.</returns>
+    public bool TryHintGlyph(
         ControlPoint[] controlPoints,
         IReadOnlyList<ushort> endPoints,
         ReadOnlyMemory<byte> instructions,
         bool isComposite)
     {
         if (instructions.Length == 0)
         {
-            return;
+            return false;
         }
 
-        // check if the CVT program disabled hinting
+        // Check if the CVT program disabled hinting
         if ((this.state.InstructionControl & InstructionControlFlags.InhibitGridFitting) != 0)
         {
-            return;
+            return false;
         }
 
-        // save contours and points
-        this.contours = endPoints;
-        this.zp0 = this.zp1 = this.zp2 = this.points = new Zone(controlPoints, isTwilight: false);
+        try
+        {
+            // Save contours and points
+            this.contours = endPoints;
+            this.zp0 = this.zp1 = this.zp2 = this.points = new Zone(controlPoints, isTwilight: false);
+
+            // reset all of our shared state
+            this.state = this.cvtState;
+            this.callStackSize = 0;
 
-        // reset all of our shared state
-        this.state = this.cvtState;
-        this.callStackSize = 0;
-        this.debugList.Clear();
-        this.stack.Clear();
-        this.OnVectorsUpdated();
-        this.iupXCalled = false;
-        this.iupYCalled = false;
-        this.isComposite = isComposite;
+            // FreeType's interpreter treats the storage area and glyph-level CVT modifications as non-persistent.
+            // Reset storage and restore the baseline CVT state for each glyph.
+            Array.Clear(this.storage);
 
-        // normalize the round state settings
-        switch (this.state.RoundState)
+            if (this.baseControlValueTable.Length > 0)
+            {
+                if (this.controlValueTable.Length != this.baseControlValueTable.Length)
+                {
+                    this.controlValueTable = new float[this.baseControlValueTable.Length];
+                }
+
+                Array.Copy(this.baseControlValueTable, this.controlValueTable, this.baseControlValueTable.Length);
+            }
+            else
+            {
+                this.controlValueTable = [];
+            }
+
+            this.ResetTwilightZone();
+
+#if DEBUG
+            this.debugList.Clear();
+#endif
+
+            this.stack.Clear();
+            this.OnVectorsUpdated();
+            this.iupXCalled = false;
+            this.iupYCalled = false;
+            this.isComposite = isComposite;
+
+            // normalize the round state settings
+            switch (this.state.RoundState)
+            {
+                case RoundMode.Super:
+                    this.SetSuperRound(1.0f);
+                    break;
+                case RoundMode.Super45:
+                    this.SetSuperRound(Sqrt2Over2);
+                    break;
+            }
+
+            this.Execute(new StackInstructionStream(instructions, 0), false, false);
+            return true;
+        }
+        catch (Exception)
         {
-            case RoundMode.Super:
-                this.SetSuperRound(1.0f);
-                break;
-            case RoundMode.Super45:
-                this.SetSuperRound(Sqrt2Over2);
-                break;
+            // TODO: Is there a general Reset I can call?
+            // The interpreter can fail for malformed instructions; in that case we skip hinting.
+            Array.Clear(this.points.TouchState, 0, this.points.TouchState.Length);
+
+            // Reset interpreter state so nothing leaks if the caller catches.
+            this.stack.Clear();
+            this.callStackSize = 0;
+            this.contours = Array.Empty<ushort>();
+            this.zp0 = this.zp1 = this.zp2 = this.points = default;
+
+            this.state = this.cvtState;
+            this.OnVectorsUpdated();
+            this.iupXCalled = false;
+            this.iupYCalled = false;
+            this.isComposite = false;
+            return false;
+        }
+    }
+
+    private void ResetTwilightZone()
+    {
+        // In FreeType, twilight points are defined to have original coordinates at (0,0).
+        // Reset both original and current coordinates, and clear touch state, to avoid state leaking between glyphs.
+        ControlPoint[] twCurrent = this.twilight.Current;
+        ControlPoint[] twOriginal = this.twilight.Original;
+
+        int len = twCurrent.Length;
+        for (int i = 0; i < len; i++)
+        {
+            twCurrent[i].Point = default;
+            twOriginal[i].Point = default;
         }
 
-        this.Execute(new StackInstructionStream(instructions, 0), false, false);
+        Array.Clear(this.twilight.TouchState);
     }
 
     private void Execute(StackInstructionStream stream, bool inFunction, bool allowFunctionDefs)
@@ -182,7 +277,10 @@ private void Execute(StackInstructionStream stream, bool inFunction, bool allowF
         while (!stream.Done)
         {
             OpCode opcode = stream.NextOpCode();
+
+#if DEBUG
             this.debugList.Add(opcode);
+#endif
             switch (opcode)
             {
                 // ==== PUSH INSTRUCTIONS ====
@@ -226,19 +324,16 @@ private void Execute(StackInstructionStream stream, bool inFunction, bool allowF
                 // ==== STORAGE MANAGEMENT ====
                 case OpCode.RS:
                 {
-                    int loc = this.stack.Pop();
-                    this.stack.Push((uint)loc < (uint)this.storage.Length ? this.storage[loc] : 0);
+                    int loc = CheckIndex(this.stack.Pop(), this.storage.Length);
+                    this.stack.Push(this.storage[loc]);
                 }
 
                 break;
                 case OpCode.WS:
                 {
                     int value = this.stack.Pop();
-                    int loc = this.stack.Pop();
-                    if ((uint)loc < (uint)this.storage.Length)
-                    {
-                        this.storage[loc] = value;
-                    }
+                    int loc = CheckIndex(this.stack.Pop(), this.storage.Length);
+                    this.storage[loc] = value;
                 }
 
                 break;
@@ -247,22 +342,16 @@ private void Execute(StackInstructionStream stream, bool inFunction, bool allowF
                 case OpCode.WCVTP:
                 {
                     float value = this.stack.PopFloat();
-                    int loc = this.stack.Pop();
-                    if ((uint)loc < (uint)this.controlValueTable.Length)
-                    {
-                        this.controlValueTable[loc] = value;
-                    }
+                    int loc = CheckIndex(this.stack.Pop(), this.controlValueTable.Length);
+                    this.controlValueTable[loc] = value;
                 }
 
                 break;
                 case OpCode.WCVTF:
                 {
                     int value = this.stack.Pop();
-                    int loc = this.stack.Pop();
-                    if ((uint)loc < (uint)this.controlValueTable.Length)
-                    {
-                        this.controlValueTable[loc] = value * this.scale;
-                    }
+                    int loc = CheckIndex(this.stack.Pop(), this.controlValueTable.Length);
+                    this.controlValueTable[loc] = value * this.scale;
                 }
 
                 break;
@@ -661,7 +750,7 @@ private void Execute(StackInstructionStream stream, bool inFunction, bool allowF
                 case OpCode.SHC1:
                 {
                     Vector2 displacement = this.ComputeDisplacement((int)opcode, out Zone zone, out int point);
-                    TouchState touch = this.GetTouchState();
+
                     int contour = this.stack.Pop();
                     int start = contour == 0 ? 0 : this.contours[contour - 1] + 1;
                     int count = this.zp2.IsTwilight ? this.zp2.Current.Length : this.contours[contour] + 1;
@@ -673,8 +762,8 @@ private void Execute(StackInstructionStream stream, bool inFunction, bool allowF
                         // Don't move the reference point
                         if (zone.Current != current || point != i)
                         {
-                            current[i].Point += displacement;
-                            states[i] |= touch;
+                            current[i].Point.Y += displacement.Y;
+                            states[i] |= TouchState.Y;
                         }
                     }
                 }
@@ -700,7 +789,7 @@ private void Execute(StackInstructionStream stream, bool inFunction, bool allowF
                         // Don't move the reference point
                         if (zone.Current != current || point != i)
                         {
-                            current[i].Point += displacement;
+                            current[i].Point.Y += displacement.Y;
                         }
                     }
                 }
@@ -1387,10 +1476,8 @@ private void Execute(StackInstructionStream stream, bool inFunction, bool allowF
                             amount *= 1 << (6 - this.state.DeltaShift);
 
                             // update the CVT
-                            if ((uint)cvtIndex < (uint)this.controlValueTable.Length)
-                            {
-                                this.controlValueTable[cvtIndex] += F26Dot6ToFloat(amount);
-                            }
+                            CheckIndex(cvtIndex, this.controlValueTable.Length);
+                            this.controlValueTable[cvtIndex] += F26Dot6ToFloat(amount);
                         }
                     }
                 }
@@ -1514,14 +1601,14 @@ private void Execute(StackInstructionStream stream, bool inFunction, bool allowF
         }
     }
 
-    private float ReadCvt()
+    private static int CheckIndex(int index, int length)
     {
-        int index = this.stack.Pop();
-        return (uint)index < (uint)this.controlValueTable.Length
-            ? this.controlValueTable[index]
-            : 0F;
+        Guard.MustBeBetweenOrEqualTo(index, 0, length - 1, nameof(index));
+        return index;
     }
 
+    private float ReadCvt() => this.controlValueTable[CheckIndex(this.stack.Pop(), this.controlValueTable.Length)];
+
     private void OnVectorsUpdated()
     {
         this.fdotp = Vector2.Dot(this.state.Freedom, this.state.Projection);
@@ -1860,20 +1947,18 @@ private void ShiftPoints(Vector2 displacement)
 
     private void MovePoint(Zone zone, int index, float distance)
     {
-        if (this.isComposite)
-        {
-            Vector2 point = zone.GetCurrent(index) + (distance * this.state.Freedom / this.fdotp);
-            TouchState touch = this.GetTouchState();
-            zone.Current[index].Point = point;
-            zone.TouchState[index] |= touch;
-        }
-        else
+        // Copy FreeType Interpreter V40 and ignore instructions on the x-axis.
+        // This increases resolution on the x-axis and prevents glyph explosions on legacy fonts.
+        // https://github.com/freetype/freetype/blob/3ab1875cd22536b3d715b3b104b7fb744b9c25c5/src/truetype/ttinterp.h#L298
+        Vector2 cur = zone.GetCurrent(index);
+
+        // V40: ignore x movement, apply only the Y component.
+        float dy = distance * this.state.Freedom.Y / this.fdotp;
+
+        // Only mark Y as touched if Y actually changed.
+        if (dy != 0F)
         {
-            // Copy FreeType Interpreter V40 and ignore instructions on the x-axis.
-            // This increases resolution on the x-axis and prevents glyph explosions on legacy fonts.
-            // https://github.com/freetype/freetype/blob/3ab1875cd22536b3d715b3b104b7fb744b9c25c5/src/truetype/ttinterp.h#L298
-            Vector2 point = zone.GetCurrent(index) + (distance * this.state.Freedom / this.fdotp);
-            zone.Current[index].Point.Y = point.Y;
+            zone.Current[index].Point.Y = cur.Y + dy;
             zone.TouchState[index] |= TouchState.Y;
         }
     }