From 04431c92a4959c26f7da6fbb62fba2c929d6fd25 Mon Sep 17 00:00:00 2001 From: Antonio Cavedoni Date: Thu, 16 Feb 2023 11:43:06 +0100 Subject: [PATCH 1/4] Build from non-default layer source (also fixes #603) --- Lib/fontmake/instantiator.py | 52 +++++++++++++------ .../MutatorSans-non-default-layer.designspace | 29 +++++++++++ tests/test_instantiator.py | 12 +++++ 3 files changed, 76 insertions(+), 17 deletions(-) create mode 100644 tests/data/MutatorSans/MutatorSans-non-default-layer.designspace diff --git a/Lib/fontmake/instantiator.py b/Lib/fontmake/instantiator.py index e5d522c8..d85b478b 100644 --- a/Lib/fontmake/instantiator.py +++ b/Lib/fontmake/instantiator.py @@ -147,6 +147,8 @@ "woffMinorVersion", } +DEFAULT_LAYER_NAME = "foreground" + # Custom exception for this module class InstantiatorError(Exception): @@ -224,21 +226,33 @@ def from_designspace( # because the math behind varLib and MutatorMath uses the default font as the # point of reference for all data. default_font = designspace.default.font + non_default_layer_name = designspace.default.layerName + non_default_layer_flag = ( + non_default_layer_name is not None + and non_default_layer_name != DEFAULT_LAYER_NAME + ) + glyph_names: Set[str] = set(default_font.keys()) - for source in designspace.sources: - other_names = set(source.font.keys()) - diff_names = other_names - glyph_names - if diff_names: - logger.warning( - "The source %s (%s) contains glyphs that are missing from the " - "default source, which will be ignored: %s. If this is unintended, " - "check that these glyphs have the exact same name as the " - "corresponding glyphs in the default source.", - source.name, - source.filename, - ", ".join(sorted(diff_names)), - ) + if non_default_layer_flag: + if non_default_layer_name in default_font.layers: + layer = default_font.layers[non_default_layer_name] + glyph_names = layer.keys() + logger.info(f"Building from layer {layer.name}") + else: + for source in designspace.sources: + other_names = set(source.font.keys()) + diff_names = other_names - glyph_names + if diff_names: + logger.warning( + "The source %s (%s) contains glyphs that are missing from the " + "default source, which will be ignored: %s. If this is unintended, " + "check that these glyphs have the exact same name as the " + "corresponding glyphs in the default source.", + source.name, + source.filename, + ", ".join(sorted(diff_names)), + ) # Construct Variators axis_bounds: AxisBounds = {} # Design space! @@ -517,9 +531,11 @@ def collect_info_masters( ) -> List[Tuple[Location, FontMathObject]]: """Return master Info objects wrapped by MathInfo.""" locations_and_masters = [] + for source in designspace.sources: - if source.layerName is not None: - continue # No font info in source layers. + if source.layerName is not None and source.layerName != DEFAULT_LAYER_NAME: + if source != designspace.default: + continue # No font info in source layers. normalized_location = varLib.models.normalizeLocation( source.location, axis_bounds @@ -541,9 +557,11 @@ def collect_kerning_masters( groups = designspace.default.font.groups locations_and_masters = [] + for source in designspace.sources: - if source.layerName is not None: - continue # No kerning in source layers. + if source.layerName is not None and source.layerName != DEFAULT_LAYER_NAME: + if source != designspace.default: + continue # No kerning in source layers. # If a source has groups, they should match the default's. if source.font.groups and source.font.groups != groups: diff --git a/tests/data/MutatorSans/MutatorSans-non-default-layer.designspace b/tests/data/MutatorSans/MutatorSans-non-default-layer.designspace new file mode 100644 index 00000000..33144cb4 --- /dev/null +++ b/tests/data/MutatorSans/MutatorSans-non-default-layer.designspace @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/test_instantiator.py b/tests/test_instantiator.py index 997f6af8..0461da96 100644 --- a/tests/test_instantiator.py +++ b/tests/test_instantiator.py @@ -420,6 +420,18 @@ def test_interpolation_masters_as_instances(data_dir): assert instance_font["l"].width == 280 +def test_non_default_layer(data_dir, caplog): + designspace = designspaceLib.DesignSpaceDocument.fromfile( + data_dir / "MutatorSans" / "MutatorSans-non-default-layer.designspace" + ) + designspace.loadSourceFonts(ufoLib2.Font.open) + generator = fontmake.instantiator.Instantiator.from_designspace( + designspace, round_geometry=True + ) + instance_font = generator.generate_instance(designspace.instances[0]) + assert {g.name for g in instance_font} == {"A", "S", "W"} + + def test_instance_attributes(data_dir): designspace = designspaceLib.DesignSpaceDocument.fromfile( data_dir / "DesignspaceTest" / "DesignspaceTest-instance-attrs.designspace" From b113bd4a27f3a30ebac014bd4aacf7273c9abaeb Mon Sep 17 00:00:00 2001 From: Antonio Cavedoni Date: Thu, 16 Feb 2023 12:06:06 +0100 Subject: [PATCH 2/4] black, flake8, etc. --- Lib/fontmake/__main__.py | 2 +- Lib/fontmake/errors.py | 2 +- Lib/fontmake/font_project.py | 7 +++++-- Lib/fontmake/instantiator.py | 13 +++++++------ 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/Lib/fontmake/__main__.py b/Lib/fontmake/__main__.py index a3bde8f3..69ba3f21 100644 --- a/Lib/fontmake/__main__.py +++ b/Lib/fontmake/__main__.py @@ -137,7 +137,7 @@ def parse_mutually_exclusive_inputs(parser, args): ): ufo_paths.append(filename) else: - parser.error(f"Unknown input file extension: '{filename}'") + parser.error("Unknown input file extension: {!r}".format(filename)) count = sum(bool(p) for p in (glyphs_path, ufo_paths, designspace_path)) if count == 0: diff --git a/Lib/fontmake/errors.py b/Lib/fontmake/errors.py index 2f8d6a55..eaa0d153 100644 --- a/Lib/fontmake/errors.py +++ b/Lib/fontmake/errors.py @@ -26,7 +26,7 @@ def __init__(self, msg, source_file): def __str__(self): trail = " -> ".join( - f"'{str(_try_relative_path(s))}'" + "{!r}".format(f"{str(_try_relative_path(s))}") for s in reversed(self.source_trail) if s is not None ) diff --git a/Lib/fontmake/font_project.py b/Lib/fontmake/font_project.py index 246171ba..eb065a4d 100644 --- a/Lib/fontmake/font_project.py +++ b/Lib/fontmake/font_project.py @@ -574,6 +574,7 @@ def save_otfs( warnings.warn( "the 'subroutinize' argument is deprecated, use 'optimize_cff'", UserWarning, + stacklevel=2, ) if subroutinize: optimize_cff = CFFOptimization.SUBROUTINIZE @@ -909,13 +910,15 @@ def interpolate_instance_ufos( if include is not None and not fullmatch(include, instance.name): continue - logger.info(f'Generating instance UFO for "{instance.name}"') + logger.info("Generating instance UFO for {!r}".format(instance.name)) try: instance.font = generator.generate_instance(instance) except instantiator.InstantiatorError as e: raise FontmakeError( - f"Interpolating instance '{instance.styleName}' failed.", + "Interpolating instance {!r} failed.".format( + instance.styleName + ), designspace.path, ) from e diff --git a/Lib/fontmake/instantiator.py b/Lib/fontmake/instantiator.py index d85b478b..d5d4325b 100644 --- a/Lib/fontmake/instantiator.py +++ b/Lib/fontmake/instantiator.py @@ -293,7 +293,7 @@ def from_designspace( glyph_mutators[glyph_name] = Variator.from_masters(items, axis_order) except varLib.errors.VarLibError as e: raise InstantiatorError( - f"Cannot set up glyph '{glyph_name}' for interpolation: {e}'" + f"Cannot set up glyph {glyph_name} for interpolation: {e}'" ) from e glyph_name_to_unicodes[glyph_name] = default_font[glyph_name].unicodes @@ -396,7 +396,7 @@ def generate_instance( # whatever reason (usually outline incompatibility)... if glyph_name not in self.skip_export_glyphs: raise InstantiatorError( - f"Failed to generate instance of glyph '{glyph_name}': " + f"Failed to generate instance of glyph {glyph_name}: " f"{str(e)}. (Note: the most common cause for an error here is " "that the glyph outlines are not point-for-point compatible or " "have the same starting point or are in the same order in all " @@ -508,9 +508,9 @@ def _error_msg_no_default(designspace: designspaceLib.DesignSpaceDocument) -> st return ( "Can't generate UFOs from this Designspace because there is no default " - f"master source at location '{default_location}'. Check that all 'default' " + "master source at location {!r}. Check that all 'default' " "values of all axes together point to a single actual master source. " - f"{bonus_msg}" + "{!s}".format(default_location, bonus_msg) ) @@ -683,8 +683,9 @@ def swap_glyph_names(font: ufoLib2.Font, name_old: str, name_new: str): if name_old not in font or name_new not in font: raise InstantiatorError( - f"Cannot swap glyphs '{name_old}' and '{name_new}', as either or both are " - "missing." + "Cannot swap glyphs {!r} and {!r}, as either or both are missing".format( + name_old, name_new + ) ) # 1. Swap outlines and glyph width. Ignore lib content and other properties. From 4233e31cb7d4d4a4dc1c1457ff5db03bc1fb73f2 Mon Sep 17 00:00:00 2001 From: Antonio Cavedoni Date: Thu, 16 Feb 2023 13:19:36 +0100 Subject: [PATCH 3/4] Removed "foreground" layer name check, fixed linting issues --- Lib/fontmake/__main__.py | 2 +- Lib/fontmake/errors.py | 2 +- Lib/fontmake/instantiator.py | 31 ++++++++++++++++--------------- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/Lib/fontmake/__main__.py b/Lib/fontmake/__main__.py index 69ba3f21..fc914182 100644 --- a/Lib/fontmake/__main__.py +++ b/Lib/fontmake/__main__.py @@ -137,7 +137,7 @@ def parse_mutually_exclusive_inputs(parser, args): ): ufo_paths.append(filename) else: - parser.error("Unknown input file extension: {!r}".format(filename)) + parser.error(f"Unknown input file extension: {filename!r}") count = sum(bool(p) for p in (glyphs_path, ufo_paths, designspace_path)) if count == 0: diff --git a/Lib/fontmake/errors.py b/Lib/fontmake/errors.py index eaa0d153..c5cefa03 100644 --- a/Lib/fontmake/errors.py +++ b/Lib/fontmake/errors.py @@ -26,7 +26,7 @@ def __init__(self, msg, source_file): def __str__(self): trail = " -> ".join( - "{!r}".format(f"{str(_try_relative_path(s))}") + f"{str(_try_relative_path(s))!r}" for s in reversed(self.source_trail) if s is not None ) diff --git a/Lib/fontmake/instantiator.py b/Lib/fontmake/instantiator.py index d5d4325b..bafe2935 100644 --- a/Lib/fontmake/instantiator.py +++ b/Lib/fontmake/instantiator.py @@ -147,8 +147,6 @@ "woffMinorVersion", } -DEFAULT_LAYER_NAME = "foreground" - # Custom exception for this module class InstantiatorError(Exception): @@ -227,18 +225,21 @@ def from_designspace( # point of reference for all data. default_font = designspace.default.font non_default_layer_name = designspace.default.layerName - non_default_layer_flag = ( - non_default_layer_name is not None - and non_default_layer_name != DEFAULT_LAYER_NAME - ) + default_source_uses_non_default_layer = non_default_layer_name is not None glyph_names: Set[str] = set(default_font.keys()) - if non_default_layer_flag: - if non_default_layer_name in default_font.layers: + if default_source_uses_non_default_layer: + try: layer = default_font.layers[non_default_layer_name] - glyph_names = layer.keys() - logger.info(f"Building from layer {layer.name}") + except KeyError as e: + raise InstantiatorError( + f"Layer {non_default_layer_name!r} not found " + f"in {designspace.default.filename}" + ) from e + layer = default_font.layers[non_default_layer_name] + glyph_names = layer.keys() + logger.info(f"Building from layer {layer.name}") else: for source in designspace.sources: other_names = set(source.font.keys()) @@ -396,7 +397,7 @@ def generate_instance( # whatever reason (usually outline incompatibility)... if glyph_name not in self.skip_export_glyphs: raise InstantiatorError( - f"Failed to generate instance of glyph {glyph_name}: " + f"Failed to generate instance of glyph {glyph_name!r}: " f"{str(e)}. (Note: the most common cause for an error here is " "that the glyph outlines are not point-for-point compatible or " "have the same starting point or are in the same order in all " @@ -533,8 +534,8 @@ def collect_info_masters( locations_and_masters = [] for source in designspace.sources: - if source.layerName is not None and source.layerName != DEFAULT_LAYER_NAME: - if source != designspace.default: + if source.layerName is not None: + if source is not designspace.default: continue # No font info in source layers. normalized_location = varLib.models.normalizeLocation( @@ -559,8 +560,8 @@ def collect_kerning_masters( locations_and_masters = [] for source in designspace.sources: - if source.layerName is not None and source.layerName != DEFAULT_LAYER_NAME: - if source != designspace.default: + if source.layerName is not None: + if source is not designspace.default: continue # No kerning in source layers. # If a source has groups, they should match the default's. From bd6ad417977b283765e0ebc3c90318001e9bd43c Mon Sep 17 00:00:00 2001 From: Antonio Cavedoni Date: Thu, 16 Feb 2023 15:30:29 +0100 Subject: [PATCH 4/4] Always report missing glyphs even on non-default layers --- Lib/fontmake/instantiator.py | 48 +++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/Lib/fontmake/instantiator.py b/Lib/fontmake/instantiator.py index bafe2935..51e49b93 100644 --- a/Lib/fontmake/instantiator.py +++ b/Lib/fontmake/instantiator.py @@ -225,11 +225,10 @@ def from_designspace( # point of reference for all data. default_font = designspace.default.font non_default_layer_name = designspace.default.layerName - default_source_uses_non_default_layer = non_default_layer_name is not None glyph_names: Set[str] = set(default_font.keys()) - if default_source_uses_non_default_layer: + if non_default_layer_name is not None: try: layer = default_font.layers[non_default_layer_name] except KeyError as e: @@ -240,20 +239,27 @@ def from_designspace( layer = default_font.layers[non_default_layer_name] glyph_names = layer.keys() logger.info(f"Building from layer {layer.name}") - else: - for source in designspace.sources: - other_names = set(source.font.keys()) - diff_names = other_names - glyph_names - if diff_names: - logger.warning( - "The source %s (%s) contains glyphs that are missing from the " - "default source, which will be ignored: %s. If this is unintended, " - "check that these glyphs have the exact same name as the " - "corresponding glyphs in the default source.", - source.name, - source.filename, - ", ".join(sorted(diff_names)), - ) + + for source in designspace.sources: + other_names = set(source.font.keys()) + diff_names = other_names - glyph_names + if diff_names: + max_diff_glyphs = 10 + logger.warning( + "The source %s (%s)%s contains glyphs that are missing from the " + "default source, which will be ignored: %s%s; if this is unintended, " + "check that these glyphs have the exact same name as the " + "corresponding glyphs in the default source.", + source.name, + source.filename, + f" [layer: {source.layerName}]" + if non_default_layer_name is not None + else "", + ", ".join(sorted(diff_names)[0:max_diff_glyphs]), + f"... ({len(diff_names)} total)" + if len(diff_names) > max_diff_glyphs + else "", + ) # Construct Variators axis_bounds: AxisBounds = {} # Design space! @@ -534,9 +540,8 @@ def collect_info_masters( locations_and_masters = [] for source in designspace.sources: - if source.layerName is not None: - if source is not designspace.default: - continue # No font info in source layers. + if source.layerName is not None and source is not designspace.default: + continue # No font info in non-default source layers. normalized_location = varLib.models.normalizeLocation( source.location, axis_bounds @@ -560,9 +565,8 @@ def collect_kerning_masters( locations_and_masters = [] for source in designspace.sources: - if source.layerName is not None: - if source is not designspace.default: - continue # No kerning in source layers. + if source.layerName is not None and source is not designspace.default: + continue # No kerning in non-default source layers. # If a source has groups, they should match the default's. if source.font.groups and source.font.groups != groups: