diff --git a/Lib/fontmake/font_project.py b/Lib/fontmake/font_project.py index f658b1a8..182d047e 100644 --- a/Lib/fontmake/font_project.py +++ b/Lib/fontmake/font_project.py @@ -31,6 +31,7 @@ import ufoLib2 from fontTools import designspaceLib from fontTools.designspaceLib.split import splitInterpolable +from fontTools.misc.cliTools import makeOutputFileName from fontTools.misc.loggingTools import Timer from fontTools.misc.plistlib import load as readPlist from fontTools.ttLib import TTFont @@ -72,6 +73,10 @@ GLYPHS_PREFIX + "customParameter.InstanceDescriptorAsGSInstance.TTFAutohint options" ) +# tempLib keys for fontmake's internal use only, won't be saved to disk +INSTANCE_LOCATION_KEY = "com.github.googlefonts.fontmake.instance_location" +INSTANCE_FILENAME_KEY = "com.github.googlefonts.fontmake.instance_filename" + def needs_subsetting(ufo): if KEEP_GLYPHS_OLD_KEY in ufo.lib or KEEP_GLYPHS_NEW_KEY in ufo.lib: @@ -124,7 +129,7 @@ def open_ufo(self, path): def save_ufo_as(self, font, path, ufo_structure="package"): try: font.save( - path, + _ensure_parent_dir(path), overwrite=True, validate=self.validate_ufo, structure=ufo_structure, @@ -145,14 +150,13 @@ def build_master_ufos( generate_GDEF=True, ufo_structure="package", glyph_data=None, + save_ufos=True, ): """Build UFOs and designspace from Glyphs source.""" import glyphsLib if master_dir is None: master_dir = self._output_dir("ufo") - if not os.path.isdir(master_dir): - os.mkdir(master_dir) if instance_dir is None: instance_dir = self._output_dir("ufo", is_instance=True) @@ -194,25 +198,35 @@ def build_master_ufos( # multiple sources can have the same font/filename (but different layer), # we want to save a font only once for source in designspace.sources: - if source.path in masters: - assert source.font is masters[source.path] - continue ufo_path = os.path.join(master_dir, source.filename) - # no need to also set the relative 'filename' attribute as that - # will be auto-updated on writing the designspace document source.path = ufo_path + # relative 'filename' would be auto-updated upon writing the designspace + # but we may not always write one out, thus we keep it up-to-date + source.filename = os.path.relpath(ufo_path, designspace_dir) + if ufo_path in masters: + assert source.font is masters[ufo_path] + continue masters[ufo_path] = source.font - if designspace_path is None: - designspace_path = os.path.join(master_dir, designspace.filename) - designspace.write(designspace_path) if mti_source: self.add_mti_features_to_master_ufos(mti_source, masters) - for ufo_path, ufo in masters.items(): - self.save_ufo_as(ufo, ufo_path, ufo_structure) + save_ds = designspace_path is not None or save_ufos + if designspace_path is None: + designspace_path = os.path.join(master_dir, designspace.filename) + + if save_ds: + logger.info("Saving %s", designspace_path) + designspace.write(_ensure_parent_dir(designspace_path)) + else: + designspace.path = designspace_path + + if save_ufos: + for ufo_path, ufo in masters.items(): + logger.info("Saving %s", ufo_path) + self.save_ufo_as(ufo, ufo_path, ufo_structure) - return designspace_path + return designspace @timer() def add_mti_features_to_master_ufos(self, mti_source, masters): @@ -241,23 +255,6 @@ def build_ttfs(self, ufos, **kwargs): """Build OpenType binaries with TrueType outlines.""" self.save_otfs(ufos, ttf=True, **kwargs) - def _load_designspace_sources(self, designspace): - if isinstance(designspace, (str, os.PathLike)): - ds_path = os.fspath(designspace) - else: - # reload designspace from its path so we have a new copy - # that can be modified in-place. - ds_path = designspace.path - if ds_path is not None: - try: - designspace = designspaceLib.DesignSpaceDocument.fromfile(ds_path) - except Exception as e: - raise FontmakeError("Reading Designspace failed", ds_path) from e - - designspace.loadSourceFonts(opener=self.open_ufo) - - return designspace - def _build_interpolatable_masters( self, designspace, @@ -413,7 +410,7 @@ def build_variable_fonts( for name, font in fonts.items(): output_path = vf_name_to_output_path[name] logger.info("Saving %s", output_path) - font.save(output_path) + font.save(_ensure_parent_dir(output_path)) def _iter_compile(self, ufos, ttf=False, debugFeatureFile=None, **kwargs): # generator function that calls ufo2ft compiler for each ufo and @@ -595,12 +592,16 @@ def save_otfs( inplace=True, # avoid extra copy ) + if interpolate_layout_from is not None: + master_locations = self._designspace_full_source_locations( + interpolate_layout_from + ) for font, ufo in zip(fonts, ufos): - if interpolate_layout_from is not None: - master_locations, instance_locations = self._designspace_locations( - interpolate_layout_from - ) - loc = instance_locations[_normpath(ufo.path)] + if ( + interpolate_layout_from is not None + and INSTANCE_LOCATION_KEY in ufo.tempLib + ): + loc = ufo.tempLib[INSTANCE_LOCATION_KEY] gpos_src = interpolate_layout( interpolate_layout_from, loc, finder, mapped=True ) @@ -639,7 +640,7 @@ def save_otfs( otf_path = output_path logger.info("Saving %s", otf_path) - font.save(otf_path) + font.save(_ensure_parent_dir(otf_path)) # 'subset' is an Optional[bool], can be None, True or False. # When False, we never subset; when True, we always do; when @@ -660,7 +661,11 @@ def save_otfs( ) try: logger.info("Autohinting %s", hinted_otf_path) - ttfautohint(otf_path, hinted_otf_path, args=autohint_thisfont) + ttfautohint( + otf_path, + _ensure_parent_dir(hinted_otf_path), + args=autohint_thisfont, + ) except TTFAError: # copy unhinted font to destination before re-raising error shutil.copyfile(otf_path, hinted_otf_path) @@ -682,7 +687,7 @@ def _save_interpolatable_fonts(self, designspace, output_dir, ttf): suffix=source.layerName, ) logger.info("Saving %s", otf_path) - source.font.save(otf_path) + source.font.save(_ensure_parent_dir(otf_path)) source.path = otf_path source.layerName = None for instance in designspace.instances: @@ -778,6 +783,9 @@ def run_from_glyphs( write_skipexportglyphs=True, generate_GDEF=True, glyph_data=None, + output=(), + output_dir=None, + interpolate=False, **kwargs, ): """Run toolchain from Glyphs source. @@ -798,9 +806,21 @@ def run_from_glyphs( glyph_data: A list of GlyphData XML file paths. kwargs: Arguments passed along to run_from_designspace. """ + # only save *master* UFOs when explicitly requested: i.e. outputs contain + # 'ufo' and the -i/--interpolate option was not passed (that's for *instances*) + # or a --master-dir was set + save_ufos = "ufo" in output and (not interpolate or master_dir is not None) + # take --output-dir to mean same as --master-dir when -o ufo and not -i + if ( + set(output) == {"ufo"} + and not interpolate + and master_dir is None + and output_dir is not None + ): + master_dir = output_dir logger.info("Building master UFOs and designspace from Glyphs source") - designspace_path = self.build_master_ufos( + designspace = self.build_master_ufos( glyphs_path, designspace_path=designspace_path, master_dir=master_dir, @@ -811,6 +831,7 @@ def run_from_glyphs( generate_GDEF=generate_GDEF, ufo_structure=kwargs.get("ufo_structure"), glyph_data=glyph_data, + save_ufos=save_ufos, ) # 'include' statements in features.fea should be resolved relative to # the input .glyphs path, like Glyphs.app would do, and not relative @@ -818,12 +839,44 @@ def run_from_glyphs( fea_include_dir = os.path.dirname(glyphs_path) try: self.run_from_designspace( - designspace_path, fea_include_dir=fea_include_dir, **kwargs + designspace, + output=output, + fea_include_dir=fea_include_dir, + output_dir=output_dir, + interpolate=interpolate, + **kwargs, ) except FontmakeError as e: e.source_trail.append(glyphs_path) raise + def _instance_ufo_path( + self, instance, designspace_path, output_dir=None, ext=".ufo" + ): + """Return an instance path, optionally overriding output dir or extension""" + # prefer absolute instance.path over relative instance.filename + instance_path = instance.path + if instance_path is not None: + instance_dir = Path(instance_path).parent + elif instance.filename is not None: + instance_path = Path(designspace_path).parent / instance.filename + instance_dir = instance_path.parent + else: + # if neither is set, make one up from UFO family/style names + instance_path = self._font_name(instance.font) + instance_dir = None + + # let --output-dir override the instance UFO directory + if output_dir is not None: + instance_dir = output_dir + + # fall back to 'instance_ufo/' if we can't find a suitable directory + if instance_dir is None: + instance_dir = self._output_dir("ufo", is_instance=True) + + instance_path = Path(instance_dir) / f"{Path(instance_path).stem}{ext}" + return os.path.normpath(instance_path) + def interpolate_instance_ufos( self, designspace, @@ -832,6 +885,9 @@ def interpolate_instance_ufos( expand_features_to_instances=False, fea_include_dir=None, ufo_structure="package", + save_ufos=True, + output_path=None, + output_dir=None, ): """Interpolate master UFOs with Instantiator and return instance UFOs. @@ -856,12 +912,9 @@ def interpolate_instance_ufos( """ from glyphsLib.interpolation import apply_instance_data_to_ufo - logger.info("Interpolating master UFOs from designspace") - try: - designspace = designspaceLib.DesignSpaceDocument.fromfile(designspace.path) - except Exception as e: - raise FontmakeError("Reading Designspace failed", designspace.path) from e + assert not (output_path and output_dir), "mutually exclusive args" + logger.info("Interpolating master UFOs from designspace") for _location, subDoc in splitInterpolable(designspace): try: generator = instantiator.Instantiator.from_designspace( @@ -904,24 +957,41 @@ def interpolate_instance_ufos( apply_instance_data_to_ufo(instance.font, instance, subDoc) - # TODO: Making filenames up on the spot is complicated, ideally don't save - # anything if filename is not set, but make something up when "ufo" is in - # output formats, but also consider output_path. - if instance.filename is None: - raise ValueError( - "It is currently required that instances have filenames set." - ) - ufo_path = os.path.join( - os.path.dirname(designspace.path), instance.filename - ) - os.makedirs(os.path.dirname(ufo_path), exist_ok=True) - self.save_ufo_as(instance.font, ufo_path, ufo_structure) + if save_ufos: + ext = ".ufoz" if ufo_structure == "zip" else ".ufo" + if output_path is not None: + # we don't know in advance how many instances we will generate + # (depends on splitInterpolable and include filter); if we + # overwrite or stop in the middle of the build it'd be worse, + # so we make the output_path unique using #1, #2, etc. suffix + instance_path = makeOutputFileName(output_path, extension=ext) + else: + instance_path = self._instance_ufo_path( + instance, designspace.path, output_dir, ext + ) + logger.info("Saving %s", instance_path) + self.save_ufo_as(instance.font, instance_path, ufo_structure) + elif instance.filename is not None: + # saving a UFO sets its path attribute; when saving the binary font + # compiled from this UFO, self._output_path() uses the ufo.path to + # make up the output path. Since we aren't saving the UFO in this + # case, the ufo.path is not set (it's a read-only attribute). + # Therefore we resort to this temporary lib key to pass down the DS + # instance filename to self._output_path() method and make sure the + # font is named correctly whether or not the UFO is saved to disk. + instance.font.tempLib[INSTANCE_FILENAME_KEY] = instance.filename + + # --interpolate-binary-layout needs to know the location of each + # instance UFO; the previous code relied on matching instance.path and + # ufo.path, but we no longer necessarily save the UFO thus the ufo.path + # may not be set. Instead store the location directly in the ufo.tempLib + instance.font.tempLib[INSTANCE_LOCATION_KEY] = instance.location yield instance.font def run_from_designspace( self, - designspace_path, + designspace, output=(), interpolate=False, variable_fonts: str = ".*", @@ -938,7 +1008,7 @@ def run_from_designspace( instance fonts (ttf or otf), interpolatable or variable fonts. Args: - designspace_path: Path to designspace document. + designspace: Path to designspace or DesignSpaceDocument object. interpolate: If True output all instance fonts, otherwise just masters. If the value is a string, only build instance(s) that match given name. The string is compiled into a regular @@ -976,12 +1046,25 @@ def run_from_designspace( % (argname, ", ".join(sorted(interp_outputs))) ) - try: - designspace = designspaceLib.DesignSpaceDocument.fromfile(designspace_path) - except Exception as e: - raise FontmakeError("Reading Designspace failed", designspace_path) from e + if isinstance(designspace, (str, os.PathLike)): + designspace_path = os.fspath(designspace) + try: + designspace = designspaceLib.DesignSpaceDocument.fromfile( + designspace_path + ) + except Exception as e: + raise FontmakeError( + "Reading Designspace failed", designspace_path + ) from e + elif isinstance(designspace, designspaceLib.DesignSpaceDocument): + # get our own DS copy so we can modify in-place + designspace = designspace.deepcopyExceptFonts() + else: + raise TypeError( + f"expected path or DesignSpaceDocument, found {type(designspace.__name__)}" + ) - designspace = self._load_designspace_sources(designspace) + designspace.loadSourceFonts(opener=self.open_ufo) # if no --feature-writers option was passed, check in the designspace's # element if user supplied a custom featureWriters configuration; @@ -1058,13 +1141,24 @@ def _run_from_designspace_static( expand_features_to_instances=False, fea_include_dir=None, ufo_structure="package", + output_path=None, + output_dir=None, **kwargs, ): + save_ufos = "ufo" in outputs ufos = [] if not interpolate or masters_as_instances: - ufos.extend(s.path for s in designspace.sources if s.path) + unique_srcs = {id(s.font): s.font for s in designspace.sources} + ufos.extend(unique_srcs.values()) if interpolate: pattern = interpolate if isinstance(interpolate, str) else None + # use --output-{path,dir} options for instance UFOs if 'ufo' is the only -o + ufo_output_path = ufo_output_dir = None + if set(outputs) == {"ufo"}: + if output_path is not None: + ufo_output_path = output_path + if output_dir is not None: + ufo_output_dir = output_dir ufos.extend( self.interpolate_instance_ufos( designspace, @@ -1073,6 +1167,9 @@ def _run_from_designspace_static( expand_features_to_instances=expand_features_to_instances, fea_include_dir=fea_include_dir, ufo_structure=ufo_structure, + save_ufos=save_ufos, + output_path=ufo_output_path, + output_dir=ufo_output_dir, ) ) @@ -1096,6 +1193,8 @@ def _run_from_designspace_static( interpolate_layout_dir=interpolate_layout_dir, feature_writers=feature_writers, fea_include_dir=fea_include_dir, + output_path=output_path, + output_dir=output_dir, **kwargs, ) @@ -1154,31 +1253,27 @@ def run_from_ufos(self, ufos, output=(), **kwargs): # the `ufos` parameter can be a list of UFO objects # or it can be a path (string) with a glob syntax - ufo_paths = [] if isinstance(ufos, str): - ufo_paths = glob.glob(ufos) - ufos = [self.open_ufo(x) for x in ufo_paths] + ufos = [self.open_ufo(x) for x in glob.glob(ufos)] elif isinstance(ufos, list): # ufos can be either paths or open Font objects, so normalize them ufos = [self.open_ufo(x) if isinstance(x, str) else x for x in ufos] - ufo_paths = [x.path for x in ufos] else: raise TypeError( "UFOs parameter is neither a ufoLib2.Font object, a path or a glob, " f"nor a list of any of these: {ufos:r}." ) - need_reload = False cff_version = 1 if "otf" in output else 2 if "otf-cff2" in output else None + + # if building both OTF & TTF we must tell ufo2ft to compile with inplace=False + inplace = not (cff_version is not None and "ttf" in output) + if cff_version is not None: - self.build_otfs(ufos, cff_version=cff_version, **kwargs) - need_reload = True + self.build_otfs(ufos, cff_version=cff_version, inplace=inplace, **kwargs) if "ttf" in output: - if need_reload: - ufos = [self.open_ufo(path) for path in ufo_paths] - self.build_ttfs(ufos, **kwargs) - need_reload = True + self.build_ttfs(ufos, inplace=inplace, **kwargs) @staticmethod def _search_instances(designspace, pattern): @@ -1254,36 +1349,39 @@ def _output_path( if isinstance(ufo_or_font_name, str): font_name = ufo_or_font_name - elif ufo_or_font_name.path: - font_name = os.path.splitext( - os.path.basename(os.path.normpath(ufo_or_font_name.path)) - )[0] else: - font_name = self._font_name(ufo_or_font_name) + ufo = ufo_or_font_name + if ufo.path: + font_name = os.path.splitext( + os.path.basename(os.path.normpath(ufo.path)) + )[0] + elif INSTANCE_FILENAME_KEY in ufo.tempLib: + font_name = Path(ufo.tempLib[INSTANCE_FILENAME_KEY]).stem + else: + font_name = self._font_name(ufo) if output_dir is None: output_dir = self._output_dir( ext, is_instance, interpolatable, autohinted, is_variable ) - if not os.path.exists(output_dir): - os.makedirs(output_dir) if suffix: return os.path.join(output_dir, f"{font_name}-{suffix}.{ext}") else: return os.path.join(output_dir, f"{font_name}.{ext}") - def _designspace_locations(self, designspace): - """Map font filenames to their locations in a designspace.""" + def _designspace_full_source_locations(self, designspace): + """Map "full" sources' paths to their locations in a designspace. - maps = [] - for elements in (designspace.sources, designspace.instances): - location_map = {} - for element in elements: - path = _normpath(element.path) - location_map[path] = element.location - maps.append(location_map) - return maps + 'Sparse layer' sources only contributing glyph outlines but no + info/kerning/features are ignored. + """ + location_map = {} + default_source = designspace.findDefault() + for source in designspace.sources: + if source is default_source or source.layerName is not None: + location_map[_normpath(source.path)] = source.location + return location_map def _closest_location(self, location_map, target): """Return path of font whose location is closest to target.""" @@ -1314,3 +1412,8 @@ def _varLib_finder(source, directory="", ext="ttf"): def _normpath(fname): return os.path.normcase(os.path.normpath(fname)) + + +def _ensure_parent_dir(path): + Path(path).parent.mkdir(parents=True, exist_ok=True) + return path diff --git a/tests/test_main.py b/tests/test_main.py index 0abe3a68..4a8bdcc2 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -363,15 +363,13 @@ def test_shared_features_expansion(data_dir, tmp_path): "-i", "--expand-features-to-instances", "-o", - "ttf", + "ufo", "--output-dir", str(tmp_path), ] ) - test_feature_file = ( - tmp_path / "sources/instance_ufo/DesignspaceTest-Light.ufo/features.fea" - ) + test_feature_file = tmp_path / "DesignspaceTest-Light.ufo/features.fea" assert test_feature_file.read_text() == "# test" @@ -1049,11 +1047,12 @@ def test_main_designspace_v5_can_use_output_path_with_1_vf(data_dir, tmp_path): "--variable-fonts", "MutatorSansVariable_Width", "--output-path", - str(tmp_path / "MySingleVF.ttf"), + str(tmp_path / "output" / "MySingleVF.ttf"), ] ) - assert (tmp_path / "MySingleVF.ttf").exists() + # 'output' subfolder was created automatically + assert (tmp_path / "output" / "MySingleVF.ttf").exists() def test_main_designspace_v5_dont_interpolate_discrete_axis(data_dir, tmp_path):