From b0a08781719bbec5db2da08eff1fd007255c7f65 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Thu, 9 May 2024 16:52:07 +0100 Subject: [PATCH 1/2] Check all layers within designspace sources Create CompatibilityChecker from designspace, instead of list[Font] Before layer names weren't being honoured when looking at sources Co-authored-by: Harry Dalton --- Lib/fontmake/compatibility.py | 50 +++++++++++++++++++++++++---------- Lib/fontmake/font_project.py | 6 ++--- tests/test_compatibility.py | 2 +- 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/Lib/fontmake/compatibility.py b/Lib/fontmake/compatibility.py index a830e6f4..ab5105cc 100644 --- a/Lib/fontmake/compatibility.py +++ b/Lib/fontmake/compatibility.py @@ -1,5 +1,11 @@ +from __future__ import annotations + import logging +from ufoLib2 import Font +from ufoLib2.objects import Layer +from fontTools.designspaceLib import DesignSpaceDocument + logger = logging.getLogger(__name__) @@ -16,20 +22,32 @@ def __exit__(self, type, value, traceback): class CompatibilityChecker: - def __init__(self, fonts): + def __init__(self, designspace: DesignSpaceDocument): self.errors = [] self.context = [] self.okay = True - self.fonts = fonts - def check(self): + self.fonts: list[Font] = [source.font for source in designspace.sources] + self.layers: list[tuple[Font, Layer]] = [ + ( + source.font, + source.font.layers.defaultLayer + if source.layerName is None + else source.font.layers[source.layerName], + ) + for source in designspace.sources + ] + + def check(self) -> bool: first = self.fonts[0] skip_export_glyphs = set(first.lib.get("public.skipExportGlyphs", ())) for glyph in first.keys(): if glyph in skip_export_glyphs: continue - self.current_fonts = [font for font in self.fonts if glyph in font] - glyphs = [font[glyph] for font in self.current_fonts] + self.current_layers = [ + (font, layer) for (font, layer) in self.layers if glyph in layer + ] + glyphs = [layer[glyph] for (_, layer) in self.current_layers] with Context(self, f"glyph {glyph}"): self.check_glyph(glyphs) return self.okay @@ -76,21 +94,21 @@ def check_contours(self, contours): with Context(self, f"point {ix}"): self.ensure_all_same(lambda x: x.type, point, "point type") - def ensure_all_same(self, func, objs, what): + def ensure_all_same(self, func, objs, what) -> bool: values = {} context = ", ".join(self.context) - for obj, font in zip(objs, self.current_fonts): - values.setdefault(func(obj), []).append(self._name_for(font)) + for obj, (font, layer) in zip(objs, self.current_layers): + values.setdefault(func(obj), []).append(self._name_for(font, layer)) if len(values) < 2: logger.debug(f"All fonts had same {what} in {context}") return True report = f"\nFonts had differing {what} in {context}:\n" debug_enabled = logger.isEnabledFor(logging.DEBUG) - for value, fonts in values.items(): - if debug_enabled or len(fonts) <= 6: - key = ", ".join(fonts) + for value, source_names in values.items(): + if debug_enabled or len(source_names) <= 6: + key = ", ".join(source_names) else: - key = f"{len(fonts)} fonts" + key = f"{len(source_names)} fonts" if len(str(value)) > 20: value = "\n " + str(value) report += f" * {key} had: {value}\n" @@ -98,6 +116,10 @@ def ensure_all_same(self, func, objs, what): self.okay = False return False - def _name_for(self, font): - names = list(filter(None, [font.info.familyName, font.info.styleName])) + def _name_for(self, font: Font, layer: Layer) -> str: + names: list[str] = [ + name + for name in (font.info.familyName, font.info.styleName, layer.name) + if name is not None and name != "public.default" + ] return " ".join(names) diff --git a/Lib/fontmake/font_project.py b/Lib/fontmake/font_project.py index e5402aee..db30db03 100644 --- a/Lib/fontmake/font_project.py +++ b/Lib/fontmake/font_project.py @@ -1177,14 +1177,14 @@ def run_from_designspace( for discrete_location, subDoc in splitInterpolable(designspace): # glyphsLib currently stores this custom parameter on the fonts, # not the designspace, so we check if it exists in any font's lib. - source_fonts = [source.font for source in subDoc.sources] explicit_check = any( - font.lib.get(COMPAT_CHECK_KEY, False) for font in source_fonts + source.font.lib.get(COMPAT_CHECK_KEY, False) + for source in subDoc.sources ) if check_compatibility is not False and ( interp_outputs or check_compatibility or explicit_check ): - if not CompatibilityChecker(source_fonts).check(): + if not CompatibilityChecker(subDoc).check(): message = "Compatibility check failed" if discrete_location: message += f" in interpolable sub-space at {discrete_location}" diff --git a/tests/test_compatibility.py b/tests/test_compatibility.py index 54459644..178285df 100644 --- a/tests/test_compatibility.py +++ b/tests/test_compatibility.py @@ -12,7 +12,7 @@ def test_compatibility_checker(data_dir, caplog): ) designspace.loadSourceFonts(opener=ufoLib2.objects.Font.open) - CompatibilityChecker([s.font for s in designspace.sources]).check() + CompatibilityChecker(designspace).check() assert "differing number of contours in glyph A" in caplog.text assert "Incompatible Sans Regular had: 2" in caplog.text From 5f7b19e81b9c58fd3ebf7cdcc31cf7b0ce0b5de7 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Thu, 9 May 2024 17:12:54 +0100 Subject: [PATCH 2/2] Clarify language around fonts/UFOs/sources Co-authored-by: Harry Dalton --- Lib/fontmake/compatibility.py | 6 +++--- Lib/fontmake/font_project.py | 2 +- tests/test_compatibility.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Lib/fontmake/compatibility.py b/Lib/fontmake/compatibility.py index ab5105cc..2366c5e2 100644 --- a/Lib/fontmake/compatibility.py +++ b/Lib/fontmake/compatibility.py @@ -100,15 +100,15 @@ def ensure_all_same(self, func, objs, what) -> bool: for obj, (font, layer) in zip(objs, self.current_layers): values.setdefault(func(obj), []).append(self._name_for(font, layer)) if len(values) < 2: - logger.debug(f"All fonts had same {what} in {context}") + logger.debug(f"All sources had same {what} in {context}") return True - report = f"\nFonts had differing {what} in {context}:\n" + report = f"\nSources had differing {what} in {context}:\n" debug_enabled = logger.isEnabledFor(logging.DEBUG) for value, source_names in values.items(): if debug_enabled or len(source_names) <= 6: key = ", ".join(source_names) else: - key = f"{len(source_names)} fonts" + key = f"{len(source_names)} sources" if len(str(value)) > 20: value = "\n " + str(value) report += f" * {key} had: {value}\n" diff --git a/Lib/fontmake/font_project.py b/Lib/fontmake/font_project.py index db30db03..3aa0b582 100644 --- a/Lib/fontmake/font_project.py +++ b/Lib/fontmake/font_project.py @@ -1158,7 +1158,7 @@ def run_from_designspace( f"expected path or DesignSpaceDocument, found {type(designspace.__name__)}" ) - logger.info("Loading %s DesignSpace source UFOs", len(designspace.sources)) + logger.info("Loading %s DesignSpace sources", len(designspace.sources)) designspace.loadSourceFonts(opener=self.open_ufo) # if no --feature-writers option was passed, check in the designspace's diff --git a/tests/test_compatibility.py b/tests/test_compatibility.py index 178285df..2af5841b 100644 --- a/tests/test_compatibility.py +++ b/tests/test_compatibility.py @@ -21,10 +21,10 @@ def test_compatibility_checker(data_dir, caplog): assert "differing anchors in glyph A" in caplog.text assert 'Incompatible Sans Bold had: "foo"' in caplog.text - assert "Fonts had differing number of components in glyph C" in caplog.text + assert "Sources had differing number of components in glyph C" in caplog.text assert ( - "Fonts had differing point type in glyph D, contour 0, point 10" in caplog.text + "Sources had differing point type in glyph D, contour 0, point 10" in caplog.text )