Merge pull request #766 from googlefonts/instruction-compiler-noise

[instructionCompiler] make 'Number of components differ' logging less noisy
googlefonts · Jul 19, 2023 · a706857 · a706857
2 parents eaaea3f + 0d74766
commit a706857
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 7 deletions.
diff --git a/Lib/ufo2ft/instructionCompiler.py b/Lib/ufo2ft/instructionCompiler.py
@@ -39,6 +39,11 @@ def __init__(
         if not autoUseMyMetrics:
             # If autoUseMyMetrics is False, replace the method with a no-op
             self.autoUseMyMetrics = lambda ttGlyph, glyphName: None
+        # Only warn--that a glyph's number of components are different in TTFont
+        # vs original UFO--when the USE_MY_METRICS flags are set manually, don't
+        # bother when automatically set:
+        # https://github.com/googlefonts/ufo2ft/issues/743
+        self.warn_if_components_mismatch = not autoUseMyMetrics
 
     def _check_glyph_hash(
         self, glyphName: str, ttglyph: TTGlyph, glyph_hash: Optional[str]
@@ -176,13 +181,14 @@ def _set_composite_flags(self, glyph: Glyph, ttglyph: TTGlyph) -> None:
         # Set component flags
 
         if len(ttglyph.components) != len(glyph.components):
-            # May happen if nested components have been flattened by a filter
-            logger.error(
-                "Number of components differ between UFO and TTF "
-                f"in glyph '{glyph.name}' ({len(glyph.components)} vs. "
-                f"{len(ttglyph.components)}, not setting component flags from"
-                "UFO. They may still be set heuristically."
-            )
+            if self.warn_if_components_mismatch:
+                # May happen if nested components have been flattened by a filter
+                logger.error(
+                    "Number of components differ between UFO and TTF "
+                    f"in glyph '{glyph.name}' ({len(glyph.components)} vs. "
+                    f"{len(ttglyph.components)}, not setting component flags from"
+                    "UFO. They may still be set heuristically."
+                )
             self.autoUseMyMetrics(ttglyph, glyph.name)
             return
 

diff --git a/tests/instructionCompiler_test.py b/tests/instructionCompiler_test.py
@@ -566,6 +566,28 @@ def test_set_composite_flags_metrics_no_id(self, quadufo, quadfont):
         assert not ttglyph.components[0].flags & USE_MY_METRICS
         assert not ttglyph.components[1].flags & USE_MY_METRICS
 
+    @pytest.mark.parametrize("autoUseMyMetrics", [True, False])
+    def test_set_composite_flags_auto_use_my_metrics_warn_if_components_mismatch(
+        self, quadufo, quadfont, autoUseMyMetrics, caplog
+    ):
+        # check we only log error message for component number mismatch when NOT setting
+        # USE_MY_METRICS flags automatically (otherwise it'd be unintended noise)
+        do_issue_warning = not autoUseMyMetrics
+
+        ic = InstructionCompiler(quadufo, quadfont, autoUseMyMetrics=autoUseMyMetrics)
+        name = "h"
+
+        glyph = quadufo[name]
+        ttglyph = quadfont["glyf"][name]
+        assert len(glyph.components) == len(ttglyph.components)
+        glyph.clearComponents()  # to produce an artificial len(components) mismatch
+        assert len(glyph.components) != len(ttglyph.components)
+
+        with caplog.at_level(logging.ERROR, logger="ufo2ft.instructionCompiler"):
+            ic._set_composite_flags(glyph=glyph, ttglyph=ttglyph)
+
+        assert ("Number of components differ" in caplog.text) is do_issue_warning
+
     # update_maxp
 
     def test_update_maxp_no_ttdata(self, quaduforeversed, quadfont):