From d5869b4ed1f806b69ccf55186e1f4bbc969f6dcf Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 24 Jul 2023 10:44:01 +0100 Subject: [PATCH 1/3] remove MutatorMath integration Fixes #834 --- Lib/fontmake/__main__.py | 24 +----- Lib/fontmake/font_project.py | 161 +++-------------------------------- requirements.txt | 3 - setup.py | 1 - tests/test_main.py | 51 ----------- 5 files changed, 15 insertions(+), 225 deletions(-) diff --git a/Lib/fontmake/__main__.py b/Lib/fontmake/__main__.py index 6801f065..67ddfa0f 100644 --- a/Lib/fontmake/__main__.py +++ b/Lib/fontmake/__main__.py @@ -288,7 +288,7 @@ def main(args=None): 'match a given "name" attribute, you can pass as argument ' "the full instance name or a regular expression. " 'E.g.: -i "Noto Sans Bold"; or -i ".* UI Condensed". ' - "(for Glyphs or MutatorMath sources only). ", + "(for Glyphs or DesignSpace sources only). ", ) outputGroup.add_argument( "--variable-fonts", @@ -308,14 +308,6 @@ def main(args=None): """ ), ) - outputGroup.add_argument( - "--use-mutatormath", - action="store_true", - help=( - "Use MutatorMath to generate instances (supports extrapolation and " - "anisotropic locations)." - ), - ) outputGroup.add_argument( "-M", "--masters-as-instances", @@ -536,7 +528,7 @@ def main(args=None): const=True, metavar="MASTER_DIR", help="Interpolate layout tables from compiled master binaries. " - "Requires Glyphs or MutatorMath source.", + "Requires Glyphs or DesignSpace source.", ) layoutGroup.add_argument( "--feature-writer", @@ -643,7 +635,6 @@ def main(args=None): "interpolate", "masters_as_instances", "interpolate_binary_layout", - "use_mutatormath", ], "variable output", ) @@ -656,16 +647,6 @@ def main(args=None): positive=False, ) - if args.get("use_mutatormath"): - for module in ("defcon", "mutatorMath"): - try: - __import__(module) - except ImportError: - parser.error( - f"{module} module not found; reinstall fontmake with the " - "[mutatormath] extra" - ) - PRINT_TRACEBACK = level == "DEBUG" try: project = FontProject(validate_ufo=args.pop("validate_ufo")) @@ -705,7 +686,6 @@ def main(args=None): [ "interpolate", "variable_fonts", - "use_mutatormath", "interpolate_binary_layout", "round_instances", "expand_features_to_instances", diff --git a/Lib/fontmake/font_project.py b/Lib/fontmake/font_project.py index 2df284c8..f658b1a8 100644 --- a/Lib/fontmake/font_project.py +++ b/Lib/fontmake/font_project.py @@ -21,7 +21,6 @@ import shutil import tempfile from collections import OrderedDict -from contextlib import contextmanager from functools import partial from pathlib import Path from re import fullmatch @@ -74,48 +73,6 @@ ) -@contextmanager -def temporarily_disabling_axis_maps(designspace_path): - """Context manager to prevent MutatorMath from warping designspace locations. - - MutatorMath assumes that the masters and instances' locations are in - user-space coordinates -- whereas they actually are in internal design-space - coordinates, and thus they do not need any 'bending'. To work around this we - we create a temporary designspace document without the axis maps, and with the - min/default/max triplet mapped "forward" from user-space coordinates (input) - to internal designspace coordinates (output). - - Args: - designspace_path: A path to a designspace document. - - Yields: - A temporary path string to the thus modified designspace document. - After the context is exited, it removes the temporary file. - - Related issues: - https://github.com/LettError/designSpaceDocument/issues/16 - https://github.com/fonttools/fonttools/pull/1395 - """ - try: - designspace = designspaceLib.DesignSpaceDocument.fromfile(designspace_path) - except Exception as e: - raise FontmakeError("Reading Designspace failed", designspace_path) from e - - for axis in designspace.axes: - axis.minimum = axis.map_forward(axis.minimum) - axis.default = axis.map_forward(axis.default) - axis.maximum = axis.map_forward(axis.maximum) - del axis.map[:] - - fd, temp_designspace_path = tempfile.mkstemp() - os.close(fd) - try: - designspace.write(temp_designspace_path) - yield temp_designspace_path - finally: - os.remove(temp_designspace_path) - - def needs_subsetting(ufo): if KEEP_GLYPHS_OLD_KEY in ufo.lib or KEEP_GLYPHS_NEW_KEY in ufo.lib: return True @@ -189,7 +146,7 @@ def build_master_ufos( ufo_structure="package", glyph_data=None, ): - """Build UFOs and MutatorMath designspace from Glyphs source.""" + """Build UFOs and designspace from Glyphs source.""" import glyphsLib if master_dir is None: @@ -962,84 +919,6 @@ def interpolate_instance_ufos( yield instance.font - def interpolate_instance_ufos_mutatormath( - self, - designspace, - include=None, - round_instances=False, - expand_features_to_instances=False, - fea_include_dir=None, - ): - """Interpolate master UFOs with MutatorMath and return instance UFOs. - - Args: - designspace: a DesignSpaceDocument object containing sources and - instances. - include (str): optional regular expression pattern to match the - DS instance 'name' attribute and only interpolate the matching - instances. - round_instances (bool): round instances' coordinates to integer. - expand_features_to_instances: parses the master feature file, expands all - include()s and writes the resulting full feature file to all instance - UFOs. Use this if you share feature files among masters in external - files. Otherwise, the relative include paths can break as instances - may end up elsewhere. Only done on interpolation. - Returns: - list of defcon.Font objects corresponding to the UFO instances. - Raises: - FontmakeError: if any of the sources defines a custom 'layer', for - this is not supported by MutatorMath. - ValueError: "expand_features_to_instances" is True but no source in the - designspace document is designated with ''. - """ - from glyphsLib.interpolation import apply_instance_data - from mutatorMath.ufo.document import DesignSpaceDocumentReader - - if any(source.layerName is not None for source in designspace.sources): - raise FontmakeError( - "MutatorMath doesn't support DesignSpace sources with 'layer' " - "attribute", - None, - ) - - with temporarily_disabling_axis_maps(designspace.path) as temp_designspace_path: - builder = DesignSpaceDocumentReader( - temp_designspace_path, - ufoVersion=3, - roundGeometry=round_instances, - verbose=True, - ) - logger.info("Interpolating master UFOs from designspace") - if include is not None: - instances = self._search_instances(designspace, pattern=include) - for instance_name in instances: - builder.readInstance(("name", instance_name)) - filenames = set(instances.values()) - else: - builder.readInstances() - filenames = None # will include all instances - - logger.info("Applying instance data from designspace") - instance_ufos = apply_instance_data(designspace, include_filenames=filenames) - - if expand_features_to_instances: - logger.debug("Expanding features to instance UFOs") - master_source = next( - (s for s in designspace.sources if s.copyFeatures), None - ) - if not master_source: - raise ValueError("No source is designated as the master for features.") - else: - master_source_font = builder.sources[master_source.name][0] - master_source_features = parseLayoutFeatures( - master_source_font, includeDir=fea_include_dir - ).asFea() - for instance_ufo in instance_ufos: - instance_ufo.features.text = master_source_features - instance_ufo.save() - - return instance_ufos - def run_from_designspace( self, designspace_path, @@ -1052,7 +931,6 @@ def run_from_designspace( feature_writers=None, filters=None, expand_features_to_instances=False, - use_mutatormath=False, check_compatibility=None, **kwargs, ): @@ -1074,8 +952,8 @@ def run_from_designspace( masters_as_instances: If True, output master fonts as instances. interpolate_binary_layout: Interpolate layout tables from compiled master binaries. - round_instances: apply integer rounding when interpolating with - MutatorMath. + round_instances: apply integer rounding when interpolating static + instance UFOs. kwargs: Arguments passed along to run_from_ufos. Raises: @@ -1145,7 +1023,6 @@ def run_from_designspace( round_instances=round_instances, feature_writers=feature_writers, expand_features_to_instances=expand_features_to_instances, - use_mutatormath=use_mutatormath, filters=filters, **kwargs, ) @@ -1180,7 +1057,6 @@ def _run_from_designspace_static( feature_writers=None, expand_features_to_instances=False, fea_include_dir=None, - use_mutatormath=False, ufo_structure="package", **kwargs, ): @@ -1189,27 +1065,16 @@ def _run_from_designspace_static( ufos.extend(s.path for s in designspace.sources if s.path) if interpolate: pattern = interpolate if isinstance(interpolate, str) else None - if use_mutatormath: - ufos.extend( - self.interpolate_instance_ufos_mutatormath( - designspace, - include=pattern, - round_instances=round_instances, - expand_features_to_instances=expand_features_to_instances, - fea_include_dir=fea_include_dir, - ) - ) - else: - ufos.extend( - self.interpolate_instance_ufos( - designspace, - include=pattern, - round_instances=round_instances, - expand_features_to_instances=expand_features_to_instances, - fea_include_dir=fea_include_dir, - ufo_structure=ufo_structure, - ) + ufos.extend( + self.interpolate_instance_ufos( + designspace, + include=pattern, + round_instances=round_instances, + expand_features_to_instances=expand_features_to_instances, + fea_include_dir=fea_include_dir, + ufo_structure=ufo_structure, ) + ) if interpolate_binary_layout is False: interpolate_layout_from = interpolate_layout_dir = None @@ -1299,7 +1164,7 @@ def run_from_ufos(self, ufos, output=(), **kwargs): ufo_paths = [x.path for x in ufos] else: raise TypeError( - "UFOs parameter is neither a defcon.Font object, a path or a glob, " + "UFOs parameter is neither a ufoLib2.Font object, a path or a glob, " f"nor a list of any of these: {ufos:r}." ) diff --git a/requirements.txt b/requirements.txt index 4ecfa373..c589709c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,10 +2,7 @@ fonttools[unicode,ufo,lxml]==4.41.1; platform_python_implementation == 'CPython' fonttools[unicode,ufo]==4.41.1; platform_python_implementation != 'CPython' glyphsLib==6.2.5 ufo2ft==2.32.0 -MutatorMath==3.0.1 fontMath==0.9.3 -defcon[lxml]==0.10.2; platform_python_implementation == 'CPython' -defcon==0.10.2; platform_python_implementation != 'CPython' booleanOperations==0.9.0 ufoLib2==0.16.0 attrs==23.1.0 diff --git a/setup.py b/setup.py index 4b6d8578..21721145 100644 --- a/setup.py +++ b/setup.py @@ -33,7 +33,6 @@ "lxml": [ # "lxml>=4.2.4", ], - "mutatormath": ["MutatorMath>=2.1.2"], "autohint": ["ttfautohint-py>=0.5.0"], } # use a special 'all' key as shorthand to includes all the extra dependencies diff --git a/tests/test_main.py b/tests/test_main.py index 9c9d1f3f..0abe3a68 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -71,57 +71,6 @@ def test_interpolation_designspace_5(data_dir, tmp_path): } -def test_interpolation_mutatormath(data_dir, tmp_path): - shutil.copytree(data_dir / "DesignspaceTest", tmp_path / "sources") - - fontmake.__main__.main( - [ - "-m", - str(tmp_path / "sources" / "DesignspaceTest.designspace"), - "-i", - "--use-mutatormath", - "--output-dir", - str(tmp_path), - ] - ) - - assert {p.name for p in tmp_path.glob("*.*")} == { - "MyFont-Regular.ttf", - "MyFont-Regular.otf", - } - - test_output_ttf = fontTools.ttLib.TTFont(tmp_path / "MyFont-Regular.ttf") - assert test_output_ttf["OS/2"].usWeightClass == 400 - glyph = test_output_ttf["glyf"]["l"] - assert glyph.xMin == 50 - assert glyph.xMax == 170 - - test_output_otf = fontTools.ttLib.TTFont(tmp_path / "MyFont-Regular.otf") - assert test_output_otf["OS/2"].usWeightClass == 400 - glyph_set = test_output_otf.getGlyphSet() - charstrings = list(test_output_otf["CFF "].cff.values())[0].CharStrings - glyph = charstrings["l"] - x_min, _, x_max, _ = glyph.calcBounds(glyph_set) - assert x_min == 50 - assert x_max == 170 - - -def test_interpolation_mutatormath_source_layer(data_dir, tmp_path): - shutil.copytree(data_dir / "MutatorSans", tmp_path / "layertest") - - with pytest.raises(SystemExit, match="sources with 'layer'"): - fontmake.__main__.main( - [ - "-m", - str(tmp_path / "layertest" / "MutatorSans.designspace"), - "-i", - "--use-mutatormath", - "--output-dir", - str(tmp_path), - ] - ) - - def test_interpolation_and_masters_as_instances(data_dir, tmp_path): shutil.copytree(data_dir / "DesignspaceTest", tmp_path / "sources") From 806ec2ce689d33638081a317e83c0a7655067368 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 26 Jul 2023 12:06:58 +0100 Subject: [PATCH 2/3] keep old --use-mutatormath option to print error when passed nudging to ufoProcessor --- Lib/fontmake/__main__.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Lib/fontmake/__main__.py b/Lib/fontmake/__main__.py index 67ddfa0f..5e1395ba 100644 --- a/Lib/fontmake/__main__.py +++ b/Lib/fontmake/__main__.py @@ -15,7 +15,7 @@ import logging import os import sys -from argparse import ArgumentParser, FileType +from argparse import SUPPRESS, ArgumentParser, FileType from collections import namedtuple from contextlib import contextmanager from textwrap import dedent @@ -308,6 +308,8 @@ def main(args=None): """ ), ) + # no longer show option in --help but keep to produce nice error message + outputGroup.add_argument("--use-mutatormath", action="store_true", help=SUPPRESS) outputGroup.add_argument( "-M", "--masters-as-instances", @@ -612,6 +614,13 @@ def main(args=None): args = vars(parser.parse_args(args)) + use_mutatormath = args.pop("use_mutatormath") + if use_mutatormath: + parser.error( + "MutatorMath is no longer supported by fontmake. " + "Try to use ufoProcessor: https://github.com/LettError/ufoProcessor" + ) + level = args.pop("verbose") _configure_logging(level, timing=args.pop("timing")) From b4388b887727edcb60c22f10d37a1b4952a0d418 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 26 Jul 2023 12:14:09 +0100 Subject: [PATCH 3/3] keep empty 'mutatormath' in extras_requires to avoid installation failing In case one has specified explicitly fontmake[mutatormath] upon installing with pip --- setup.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/setup.py b/setup.py index 21721145..c64cbac3 100644 --- a/setup.py +++ b/setup.py @@ -33,6 +33,9 @@ "lxml": [ # "lxml>=4.2.4", ], + # MutatorMath is no longer supported but a dummy extras is kept below + # to avoid fontmake installation failing if requested + "mutatormath": [], "autohint": ["ttfautohint-py>=0.5.0"], } # use a special 'all' key as shorthand to includes all the extra dependencies