Repository: googlefonts/ufo2ft
Pull request #814 (2024-01-23 15:40:06)

Check glyph hashes more thoroughly
To determine if the TT instructions were still valid, the stored glyph hash was only checked against the computed hash of the `TTFont` glyph.

This PR changes this so there are two checks:

1. The stored glyph hash is checked against a computed UFO glyph hash to see if the instructions match the UFO glyph.
2. The computed UFO glyph hash is checked against a computed hash of the `TTFont` glyph to see if the UFO glyph matches the TTF glyph.

In the case of composite glyphs, the original code would fail for any fractional transformations that cannot be expressed exactly in F2Dot14. This PR fixes that by using the new `transformRoundFunc` parameter of the `RoundingPointPen` (https://github.com/fonttools/fonttools/pull/3426). Thus it depends on a yet unreleased version of FontTools which contains the addition.


Patch:
diff --git a/Lib/ufo2ft/instructionCompiler.py b/Lib/ufo2ft/instructionCompiler.py
index 86887e3..15675a9 100644
--- a/Lib/ufo2ft/instructionCompiler.py
+++ b/Lib/ufo2ft/instructionCompiler.py
@@ -2,10 +2,13 @@ from __future__ import annotations
 
 import array
 import logging
+from functools import partial
 from typing import TYPE_CHECKING, Optional
 
 from fontTools import ttLib
+from fontTools.misc.fixedTools import floatToFixedToFloat
 from fontTools.pens.hashPointPen import HashPointPen
+from fontTools.pens.roundingPen import RoundingPointPen
 from fontTools.ttLib import newTable
 from fontTools.ttLib.tables._g_l_y_f import (
     OVERLAP_COMPOUND,
@@ -42,27 +45,28 @@ class InstructionCompiler:
             self.autoUseMyMetrics = lambda ttGlyph, glyphName: None
 
     def _check_glyph_hash(
-        self, glyphName: str, ttglyph: TTGlyph, glyph_hash: Optional[str]
+        self, glyph: Glyph, ttglyph: TTGlyph, stored_hash: Optional[str]
     ) -> bool:
-        """Check if the supplied glyph hash from the ufo matches the current outlines."""
-        if glyph_hash is None:
+        """Check if the supplied stored glyph hash from the ufo matches the TTGlyph."""
+        if stored_hash is None:
             # The glyph hash is required
             logger.error(
-                f"Glyph hash missing, glyph '{glyphName}' will have "
+                f"Glyph hash missing, glyph '{glyph.name}' will have "
                 "no instructions in font."
             )
             return False
 
-        # Check the glyph hash against the TTGlyph that is being built
-
-        ttwidth = self.otf["hmtx"][glyphName][0]
+        ttwidth = self.otf["hmtx"][glyph.name][0]
         hash_pen = HashPointPen(ttwidth, self.otf.getGlyphSet())
-        ttglyph.drawPoints(hash_pen, self.otf["glyf"])
+        round_pen = RoundingPointPen(
+            hash_pen, transformRoundFunc=partial(floatToFixedToFloat, precisionBits=14)
+        )
+        ttglyph.drawPoints(round_pen, self.otf["glyf"])
 
-        if glyph_hash != hash_pen.hash:
+        if stored_hash != hash_pen.hash:
             logger.error(
-                f"The stored hash for glyph '{glyphName}' does not match the TrueType "
-                "output glyph. Glyph will have no instructions in the font."
+                f"The stored hash for glyph '{glyph.name}' does not match the "
+                "TrueType output glyph. Glyph will have no instructions in the font."
             )
             return False
         return True
@@ -132,7 +136,7 @@ class InstructionCompiler:
     ) -> None:
         self._check_tt_data_format(ttdata, f"glyph '{glyph.name}'")
         glyph_hash = ttdata.get("id", None)
-        if not self._check_glyph_hash(glyph.name, ttglyph, glyph_hash):
+        if not self._check_glyph_hash(glyph, ttglyph, glyph_hash):
             return
 
         # Compile the glyph program


Test patch:
diff --git a/tests/infoCompiler_test.py b/tests/infoCompiler_test.py
index 66ba6bb..b35fa6e 100644
--- a/tests/infoCompiler_test.py
+++ b/tests/infoCompiler_test.py
@@ -20,7 +20,6 @@ def testufo(FontClass):
 
 
 class InfoCompilerTest:
-
     def test_head(self, testttf, testufo):
         info = {"versionMajor": 5, "versionMinor": 6}
         compiler = InfoCompiler(testttf, testufo, info)
diff --git a/tests/instructionCompiler_test.py b/tests/instructionCompiler_test.py
index 87d2d66..b06b251 100644
--- a/tests/instructionCompiler_test.py
+++ b/tests/instructionCompiler_test.py
@@ -1,8 +1,11 @@
 import logging
+from functools import partial
 
 import pytest
 from fontTools.cu2qu.ufo import font_to_quadratic
+from fontTools.misc.fixedTools import floatToFixedToFloat
 from fontTools.pens.hashPointPen import HashPointPen
+from fontTools.pens.roundingPen import RoundingPointPen
 from fontTools.ttLib.tables._g_l_y_f import (
     OVERLAP_COMPOUND,
     ROUND_XY_TO_GRID,
@@ -40,7 +43,10 @@ def expect_maxp(
 
 def get_hash_ufo(glyph, ufo):
     hash_pen = HashPointPen(glyph.width, ufo)
-    glyph.drawPoints(hash_pen)
+    round_pen = RoundingPointPen(
+        hash_pen, transformRoundFunc=partial(floatToFixedToFloat, precisionBits=14)
+    )
+    glyph.drawPoints(round_pen)
     return hash_pen.hash
 
 
@@ -89,7 +95,7 @@ class InstructionCompilerTest:
         ttglyph = quadfont["glyf"]["a"]
 
         ic = InstructionCompiler(quaduforeversed, quadfont)
-        result = ic._check_glyph_hash(glyph.name, ttglyph, ufo_hash)
+        result = ic._check_glyph_hash(glyph, ttglyph, ufo_hash)
         assert result
 
     def test_check_glyph_hash_missing(self, quaduforeversed, quadfont):
@@ -97,7 +103,7 @@ class InstructionCompilerTest:
 
         ic = InstructionCompiler(quaduforeversed, quadfont)
         result = ic._check_glyph_hash(
-            glyph.name,
+            glyph,
             quadfont["glyf"]["a"],
             None,
         )
@@ -113,7 +119,7 @@ class InstructionCompilerTest:
 
         ic = InstructionCompiler(testufo, quadfont)
         result = ic._check_glyph_hash(
-            glyph.name,
+            glyph,
             ttglyph,
             ufo_hash,
         )
@@ -129,7 +135,7 @@ class InstructionCompilerTest:
 
         ic = InstructionCompiler(testufo, quadfont)
         result = ic._check_glyph_hash(
-            glyph.name,
+            glyph,
             ttglyph,
             ufo_hash,
         )
@@ -146,7 +152,7 @@ class InstructionCompilerTest:
 
         ic = InstructionCompiler(quaduforeversed, quadfont)
         result = ic._check_glyph_hash(
-            glyph.name,
+            glyph,
             ttglyph,
             ufo_hash,
         )