From df2f961ffda841eed454b13b11833c7093177fe8 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 15 Mar 2024 17:32:09 +0000 Subject: [PATCH 01/27] require fonttools >= 4.50 with new DecomposingFilterPointPen --- requirements.txt | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index 33a9792e..fa113d6d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -fonttools[ufo,lxml]==4.49.0 +fonttools[ufo,lxml]==4.50.0 defcon==0.10.3 compreffor==0.5.5 booleanOperations==0.9.0 diff --git a/setup.py b/setup.py index f4cb0ca2..74b5fdab 100644 --- a/setup.py +++ b/setup.py @@ -29,7 +29,7 @@ setup_requires=pytest_runner + wheel + ["setuptools_scm"], tests_require=["pytest>=2.8"], install_requires=[ - "fonttools[ufo]>=4.49.0", + "fonttools[ufo]>=4.50.0", "cffsubr>=0.3.0", "booleanOperations>=0.9.0", "fontMath>=0.9.3", From 51c27d988ee5b3a2a47db6bbe190eaa0e1ae9346 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 14 Mar 2024 17:55:12 +0000 Subject: [PATCH 02/27] [instantiator] add a map-like object of interpolated glyphs to be used with decomposing pen --- Lib/ufo2ft/instantiator.py | 95 ++++++++++++++++++++++++++++++++++---- 1 file changed, 87 insertions(+), 8 deletions(-) diff --git a/Lib/ufo2ft/instantiator.py b/Lib/ufo2ft/instantiator.py index d9f64891..7af5cc76 100644 --- a/Lib/ufo2ft/instantiator.py +++ b/Lib/ufo2ft/instantiator.py @@ -33,7 +33,7 @@ import copy import logging import typing -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import ( TYPE_CHECKING, Any, @@ -51,12 +51,17 @@ import fontTools.misc.fixedTools from fontTools import designspaceLib, varLib -from ufo2ft.util import _getNewGlyphFactory, importUfoModule, openFontFactory +from ufo2ft.util import ( + _getNewGlyphFactory, + importUfoModule, + openFontFactory, + zip_strict, +) if TYPE_CHECKING: from collections.abc import Iterable, KeysView - from ufoLib2.objects import Font, Glyph, Info, Layer + from ufoLib2.objects import Font, Glyph, Info logger = logging.getLogger(__name__) @@ -207,7 +212,7 @@ class Instantiator: round_geometry: bool skip_export_glyphs: List[str] special_axes: Mapping[str, designspaceLib.AxisDescriptor] - source_layers: List[Tuple[Location, Layer]] + source_layers: List[Tuple[Location, Dict[str, Glyph]]] default_source_idx: int glyph_factory: Optional[Callable[[str], Glyph]] # create new ufoLib/defcon Glyph @@ -399,7 +404,7 @@ def glyph_names(self) -> KeysView[str]: def source_locations(self) -> Iterable[Location]: return (loc for loc, _ in self.source_layers) - def normalize(self, location): + def normalize(self, location: Location) -> Location: return varLib.models.normalizeLocation(location, self.axis_bounds) def generate_instance(self, instance: designspaceLib.InstanceDescriptor) -> Font: @@ -489,13 +494,16 @@ def generate_instance(self, instance: designspaceLib.InstanceDescriptor) -> Font return font - def new_glyph(self, name): + def new_glyph(self, name: str) -> Glyph: assert self.glyph_factory is not None return self.glyph_factory(name=name) def generate_glyph_instance( - self, glyph_name, normalized_location, output_glyph=None - ): + self, + glyph_name: str, + normalized_location: Location, + output_glyph: Glyph | None = None, + ) -> Glyph: """Generate an instance of a single glyph at the given location. The location must be pecified using normalized coordinates. @@ -607,6 +615,27 @@ def _generate_instance_info( slant_axis.map_backward(location[slant_axis.name]) ) + @property + def interpolated_layers(self) -> list[InterpolatedLayer]: + """Return one InterpolatedLayer for each source location.""" + return [ + InterpolatedLayer(self, loc, source_layer) + for loc, source_layer in self.source_layers + ] + + def replace_source_layers(self, new_layers: list[dict[str, Glyph]]): + """Replace source layers with `new_layers` and clear the cached glyph models. + + Raises `ValueError` if len(new_layers) != len(self.source_layers). + """ + self.source_layers[:] = [ + (loc, new_glyphs) + for (loc, _), new_glyphs in zip_strict(self.source_layers, new_layers) + ] + # this forces to reload the glyph variation models when an instance is requested + self.glyph_mutators.clear() + self.glyph_mutators.update((gn, None) for gn in self.glyph_names) + def _error_msg_no_default(designspace: designspaceLib.DesignSpaceDocument) -> str: if any(axis.map for axis in designspace.axes): @@ -907,3 +936,53 @@ def instance_at(self, normalized_location: Location) -> FontMathObject: return copy.deepcopy(self.location_to_master[normalized_location_key]) return self.model.interpolateFromMasters(normalized_location, self.masters) + + +@dataclass(frozen=True, repr=False) +class InterpolatedLayer(Mapping): + """Mapping of glyphs keyed by name, interpolated on demand. + + If the given location corresponds to one of the source layers, and the + latter contains the glyph, this is used directly; otherwise, a new glyph + instance is generated on-the-fly at that location, and cached for subsequent + retrieval. + + This is useful for APIs that expect a dict of glyphs for resolving component + references, e.g. FontTools pens. + """ + + instantiator: Instantiator + # axis coordinates for this layer (already normalized!) + location: Location + # source ufoLib2/defcon Layer (None if location isn't among the source locations) + source_layer: dict[str, Glyph] | None = None + _cache: dict[str, Glyph] = field(default_factory=dict) + + def __iter__(self) -> Iterable[str]: + return iter(self.instantiator.glyph_names) + + def __len__(self) -> int: + return len(self.instantiator.glyph_names) + + def __getitem__(self, glyph_name: str) -> Glyph: + try: + return self._cache.setdefault( + glyph_name, self._get(glyph_name) or self._interpolate(glyph_name) + ) + except InstantiatorError as e: + raise KeyError(glyph_name) from e + + def __repr__(self): + return ( + f"<{self.__class__.__name__} {self.location} " + f"({len(self)} glyphs) at 0x{id(self):12x}>" + ) + + def _get(self, glyph_name: str) -> Glyph | None: + glyph = None + if self.source_layer is not None: + glyph = self.source_layer.get(glyph_name) + return glyph + + def _interpolate(self, glyph_name: str) -> Glyph: + return self.instantiator.generate_glyph_instance(glyph_name, self.location) From a49d83e4d65695cb084e8619d1cc50c43bcf30c3 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 14 Mar 2024 18:11:14 +0000 Subject: [PATCH 03/27] [util] added decomposeCompositeGlyph helper that uses new decomposing filter pen --- Lib/ufo2ft/util.py | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/Lib/ufo2ft/util.py b/Lib/ufo2ft/util.py index b245e74a..2c1689ea 100644 --- a/Lib/ufo2ft/util.py +++ b/Lib/ufo2ft/util.py @@ -14,6 +14,7 @@ from fontTools.feaLib.builder import addOpenTypeFeatures from fontTools.misc.fixedTools import otRound from fontTools.misc.transform import Identity, Transform +from fontTools.pens.filterPen import DecomposingFilterPointPen from fontTools.pens.reverseContourPen import ReverseContourPen from fontTools.pens.transformPen import TransformPen @@ -49,6 +50,39 @@ def makeOfficialGlyphOrder(font, glyphOrder=None): return order +def decomposeCompositeGlyph( + glyph, + glyphSet, + skipMissing=True, + reverseFlipped=True, + include=None, + decomposeNested=True, +): + """Decompose composite glyph in-place resolving references from glyphSet.""" + if len(glyph.components) == 0: + return + pen = DecomposingFilterPointPen( + glyph.getPointPen(), + glyphSet, + reverseFlipped=reverseFlipped, + include=include, + decomposeNested=decomposeNested, + ) + for component in list(glyph.components): + try: + component.drawPoints(pen) + except pen.MissingComponentError: + if skipMissing: + logger.warning( + "dropping non-existent component '%s' in glyph '%s'", + component.baseGlyph, + glyph.name, + ) + else: + raise + glyph.removeComponent(component) + + class _GlyphSet(dict): @classmethod def from_layer(cls, font, layerName=None, copy=False, skipExportGlyphs=None): @@ -166,6 +200,7 @@ def _setGlyphMargin(glyph, side, margin): raise NotImplementedError(f"Unsupported Glyph class: {type(glyph)!r}") +# DEPRECATED: use ufo2ft.util.decomposeCompositeGlyph above def deepCopyContours( glyphSet, parent, composite, transformation, specificComponents=None ): From 7f5e3fa47d07cd30afeb5ff06dba12c4588f7a00 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 14 Mar 2024 18:47:56 +0000 Subject: [PATCH 04/27] [filters] define new BaseIFilter base class for interpolatable filters These filters take multiple glyphSets as input and zip through the glyphs with the same name, processing them as a group instead of one glyph at a time. --- Lib/ufo2ft/filters/__init__.py | 21 ++-- Lib/ufo2ft/filters/base.py | 180 ++++++++++++++++++++++++++++++++- 2 files changed, 193 insertions(+), 8 deletions(-) diff --git a/Lib/ufo2ft/filters/__init__.py b/Lib/ufo2ft/filters/__init__.py index 841c3cc5..1d7cd202 100644 --- a/Lib/ufo2ft/filters/__init__.py +++ b/Lib/ufo2ft/filters/__init__.py @@ -5,7 +5,7 @@ from ufo2ft.constants import FILTERS_KEY from ufo2ft.util import _loadPluginFromString -from .base import BaseFilter +from .base import BaseFilter, BaseIFilter from .cubicToQuadratic import CubicToQuadraticFilter from .decomposeComponents import DecomposeComponentsFilter from .decomposeTransformedComponents import DecomposeTransformedComponentsFilter @@ -20,6 +20,7 @@ __all__ = [ "BaseFilter", + "BaseIFilter", "CubicToQuadraticFilter", "DecomposeComponentsFilter", "DecomposeTransformedComponentsFilter", @@ -85,13 +86,15 @@ def loadFilters(ufo): return preFilters, postFilters -def isValidFilter(klass): +def isValidFilter(klass, *bases): """Return True if 'klass' is a valid filter class. A valid filter class is a class (of type 'type'), that has a '__call__' (bound method), with the signature matching the same method - from the BaseFilter class: + from the BaseFilter or BaseIFilter classes, respectively: def __call__(self, font, glyphSet=None) + + def __call__(self, fonts, glyphSets=None, instantiator=None, **kwargs) """ if not isclass(klass): logger.error(f"{klass!r} is not a class") @@ -99,10 +102,14 @@ def __call__(self, font, glyphSet=None) if not callable(klass): logger.error(f"{klass!r} is not callable") return False - if getfullargspec(klass.__call__).args != getfullargspec(BaseFilter.__call__).args: - logger.error(f"{klass!r} '__call__' method has incorrect signature") - return False - return True + for baseClass in bases or (BaseFilter, BaseIFilter): + if ( + getfullargspec(klass.__call__).args + == getfullargspec(baseClass.__call__).args + ): + return True + logger.error(f"{klass!r} '__call__' method has incorrect signature") + return False def loadFilterFromString(spec): diff --git a/Lib/ufo2ft/filters/base.py b/Lib/ufo2ft/filters/base.py index 0c90e045..6e206543 100644 --- a/Lib/ufo2ft/filters/base.py +++ b/Lib/ufo2ft/filters/base.py @@ -1,9 +1,26 @@ +from __future__ import annotations + import logging +import sys from types import SimpleNamespace +from typing import TYPE_CHECKING from fontTools.misc.loggingTools import Timer -from ufo2ft.util import _GlyphSet, _LazyFontName, getMaxComponentDepth +from ufo2ft.util import ( + _getNewGlyphFactory, + _GlyphSet, + _LazyFontName, + getMaxComponentDepth, + zip_strict, +) + +if TYPE_CHECKING: + from typing import Any + + from ufoLib2.objects import Font, Glyph + + from ufo2ft.instantiator import Instantiator, InterpolatedLayer # reuse the "ufo2ft.filters" logger logger = logging.getLogger("ufo2ft.filters") @@ -153,6 +170,8 @@ def set_context(self, font, glyphSet): """ self.context = SimpleNamespace(font=font, glyphSet=glyphSet) self.context.modified = set() + proto = font.layers.defaultLayer.instantiateGlyphObject() + self.context.glyphFactory = _getNewGlyphFactory(proto) return self.context def filter(self, glyph): @@ -216,3 +235,162 @@ def __call__(self, font, glyphSet=None): "" if num == 1 else "s", ) return modified + + @classmethod + def getInterpolatableFilterClass(cls) -> BaseIFilter | None: + """Return interpolatable filter class if one is found in the same module. + + We search for a class with the same name and the 'IFilter' suffix + (where the 'I' stands for "interpolatable"). + + Subclasses can override this if they wish to use a different class name + or module. + """ + module = sys.modules[cls.__module__] + filter_name = cls.__name__ + if filter_name.endswith("Filter"): + filter_name = filter_name[:-6] + ifilter_name = f"{filter_name}IFilter" + return getattr(module, ifilter_name, None) + + +class BaseIFilter(BaseFilter): + """Interpolatable variant that zips through mutliple glyphs at a time.""" + + def set_context( + self, + fonts: list[Font], + glyphSets: list[dict[str, Glyph]], + instantiator: Instantiator | None = None, + **kwargs: dict[str, Any], + ) -> SimpleNamespace: + """Populate a `self.context` namespace, which is reset before each + new filter call. + + Subclasses can override this to provide contextual information + which depends on other data in the fonts that is not available in + the glyphs objects currently being filtered, or set any other + temporary attributes. + + The default implementation simply sets the current fonts, glyphSets, + and optional instantiator and initializes an empty set that keeps track + of the names of the glyphs that were modified. + + Any extra keyword arguments are passed to the context namespace. + + Returns the namespace instance. + """ + assert len(fonts) == len(glyphSets) + self.context = SimpleNamespace( + fonts=fonts, + glyphSets=glyphSets, + instantiator=instantiator, + **kwargs, + ) + self.context.modified = set() + proto = fonts[0].layers.defaultLayer.instantiateGlyphObject() + self.context.glyphFactory = _getNewGlyphFactory(proto) + return self.context + + def filter(self, glyphName: str, glyphs: list) -> bool: + """This is where the filter is applied to a set of interpolatable glyphs. + + Subclasses must override this method, and return True + when the glyph was modified. + """ + raise NotImplementedError + + def __call__( + self, + fonts: list[Font], + glyphSets: list[dict[str, Glyph]] | None = None, + instantiator: Instantiator | None = None, + **kwargs: dict[str, Any], + ) -> set[str]: + """Run this filter on all the included glyphs from the given glyphSets. + Return the set of glyph names that were modified, if any. + + If `glyphSets` (list[dict]) argument is provided, run the filter on + the glyphs contained therein (which may be copies). + Otherwise, run the filter in-place on the fonts' default + glyph sets. + + The `instantiator` optional argument allows interpolatable filters to + generate glyph instances on demand at any location in the designspace. + + Any extra keyword arguments are passed on to the `set_context` method. + """ + logger.info("Running interpolatable %s", self.name) + + if glyphSets is None: + glyphSets = [_GlyphSet.from_layer(font) for font in fonts] + + context = self.set_context(fonts, glyphSets, instantiator, **kwargs) + + filter_ = self.filter + include = self.include + modified = context.modified + + # process composite glyphs in decreasing component depth order (i.e. composites + # with more deeply nested components before shallower ones) to avoid + # order-dependent interferences while filtering glyphs with nested components + # https://github.com/googlefonts/ufo2ft/issues/621 + allGlyphNames = set.union(*(set(glyphSet.keys()) for glyphSet in glyphSets)) + + def comp_depth(g): + for glyphSet in glyphSets: + if g in glyphSet: + return -getMaxComponentDepth(glyphSet[g], glyphSet) + raise AssertionError + + orderedGlyphs = sorted(allGlyphNames, key=comp_depth) + + with Timer() as t: + for glyphName in orderedGlyphs: + if glyphName in modified: + continue + glyphs = [ + glyphSet[glyphName] + for glyphSet in glyphSets + if glyphName in glyphSet + ] + if any(include(g) for g in glyphs) and filter_(glyphName, glyphs): + modified.add(glyphName) + + num = len(modified) + if num > 0: + timing_logger.debug( + "Took %.3fs to run %s on %d glyph%s", + t, + self.name, + len(modified), + "" if num == 1 else "s", + ) + return modified + + @classmethod + def getInterpolatableFilterClass(cls) -> "BaseIFilter" | None: + """Return the same class as self.""" + return cls # no-op + + def getDefaultGlyphSet(self) -> dict[str, Glyph]: + """Return the current glyphSet corresponding to the default location.""" + if self.context.instantiator is not None: + default_idx = self.context.instantiator.default_source_idx + for i, glyphSet in enumerate(self.context.glyphSets): + if i == default_idx: + return glyphSet + else: + raise AssertionError("No default source?!") + else: + # we don't have enough info to know which glyphSet corresponds + # to the default source location so we just guess it's going to + # be the larger one given it contains all glyphs by definition. + return max(self.context.glyphSets, key=lambda glyphSet: len(glyphSet)) + + def getInterpolatedLayers(self) -> list[InterpolatedLayer] | list[None]: + """Return InterpolatedLayers at source locations or Nones if no Instantiator.""" + if self.context.instantiator is not None: + return self.context.instantiator.interpolated_layers + else: + return [None] * len(self.context.glyphSets) From d33657961b8ebb25caf086d65e5ec7abe04fbe3f Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 14 Mar 2024 18:51:09 +0000 Subject: [PATCH 05/27] [BaseIFilter] add ensureCompositeDefinedAtComponentLocations this will be used in filters that decompose composite glyphs, to make sure the latter appears the same after decomposition in situations where the composite glyph is defined at fewer source locations than some of its component glyphs. --- Lib/ufo2ft/filters/base.py | 66 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/Lib/ufo2ft/filters/base.py b/Lib/ufo2ft/filters/base.py index 6e206543..ffc9292e 100644 --- a/Lib/ufo2ft/filters/base.py +++ b/Lib/ufo2ft/filters/base.py @@ -3,7 +3,7 @@ import logging import sys from types import SimpleNamespace -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, FrozenSet, Tuple from fontTools.misc.loggingTools import Timer @@ -16,7 +16,7 @@ ) if TYPE_CHECKING: - from typing import Any + from typing import Any, TypeAlias from ufoLib2.objects import Font, Glyph @@ -254,6 +254,9 @@ def getInterpolatableFilterClass(cls) -> BaseIFilter | None: return getattr(module, ifilter_name, None) +HashableLocation: TypeAlias = FrozenSet[Tuple[str, float]] + + class BaseIFilter(BaseFilter): """Interpolatable variant that zips through mutliple glyphs at a time.""" @@ -394,3 +397,62 @@ def getInterpolatedLayers(self) -> list[InterpolatedLayer] | list[None]: return self.context.instantiator.interpolated_layers else: return [None] * len(self.context.glyphSets) + + @staticmethod + def hashableLocation(location: dict[str, float]) -> HashableLocation: + """Convert (normalized) location dict to a hashable set of tuples.""" + return frozenset((k, v) for k, v in location.items() if v != 0.0) + + def glyphSourceLocations(self, glyphName) -> set[HashableLocation]: + """Return locations of all the sources that have a glyph.""" + assert self.context.instantiator is not None + return { + self.hashableLocation(location) + for glyphSet, location in zip_strict( + self.context.glyphSets, self.context.instantiator.source_locations + ) + if glyphName in glyphSet + } + + def locationsFromComponentGlyphs( + self, + glyphName: str, + include: set[str] | None = None, + ) -> set[HashableLocation]: + """Return locations from all the components' base glyphs, recursively.""" + assert self.context.instantiator is not None + locations = set() + for glyphSet in self.context.glyphSets: + if glyphName in glyphSet: + glyph = glyphSet[glyphName] + for component in glyph.components: + if include is None or component.baseGlyph in include: + locations |= self.glyphSourceLocations(component.baseGlyph) + locations |= self.locationsFromComponentGlyphs( + component.baseGlyph, include + ) + return locations + + def ensureCompositeDefinedAtComponentLocations( + self, + glyphName: str, + include: set[str] | None = None, + ): + """Ensure the composite glyph is defined at all its components' locations. + + The Instantiator is used to interpolate the glyph at the missing locations. + If we have no Instantiator, we can't interpolate so this does nothing. + """ + if self.context.instantiator is None: + return + + haveLocations = self.glyphSourceLocations(glyphName) + needLocations = self.locationsFromComponentGlyphs(glyphName, include) + locationsToAdd = needLocations - haveLocations + if locationsToAdd: + for glyphSet, interpolatedLayer in zip_strict( + self.context.glyphSets, self.context.instantiator.interpolated_layers + ): + if self.hashableLocation(interpolatedLayer.location) in locationsToAdd: + assert glyphName not in glyphSet + glyphSet[glyphName] = interpolatedLayer[glyphName] From 3fd3a44ec55c89bb12c2aa7f486d58ad8bd9c2d4 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 14 Mar 2024 18:55:30 +0000 Subject: [PATCH 06/27] [filters] Add interpolatable DecomposeComponentsIFilter --- Lib/ufo2ft/filters/__init__.py | 3 +- Lib/ufo2ft/filters/decomposeComponents.py | 40 +++++++++++++++++++---- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/Lib/ufo2ft/filters/__init__.py b/Lib/ufo2ft/filters/__init__.py index 1d7cd202..615d3c1e 100644 --- a/Lib/ufo2ft/filters/__init__.py +++ b/Lib/ufo2ft/filters/__init__.py @@ -7,7 +7,7 @@ from .base import BaseFilter, BaseIFilter from .cubicToQuadratic import CubicToQuadraticFilter -from .decomposeComponents import DecomposeComponentsFilter +from .decomposeComponents import DecomposeComponentsFilter, DecomposeComponentsIFilter from .decomposeTransformedComponents import DecomposeTransformedComponentsFilter from .dottedCircle import DottedCircleFilter from .explodeColorLayerGlyphs import ExplodeColorLayerGlyphsFilter @@ -23,6 +23,7 @@ "BaseIFilter", "CubicToQuadraticFilter", "DecomposeComponentsFilter", + "DecomposeComponentsIFilter", "DecomposeTransformedComponentsFilter", "DottedCircleFilter", "ExplodeColorLayerGlyphsFilter", diff --git a/Lib/ufo2ft/filters/decomposeComponents.py b/Lib/ufo2ft/filters/decomposeComponents.py index 79df4384..91181298 100644 --- a/Lib/ufo2ft/filters/decomposeComponents.py +++ b/Lib/ufo2ft/filters/decomposeComponents.py @@ -1,13 +1,41 @@ -from fontTools.misc.transform import Transform +from __future__ import annotations -import ufo2ft.util -from ufo2ft.filters import BaseFilter +from typing import TYPE_CHECKING + +from ufo2ft.filters import BaseFilter, BaseIFilter +from ufo2ft.util import decomposeCompositeGlyph, zip_strict + +if TYPE_CHECKING: + from ufoLib2.objects import Glyph class DecomposeComponentsFilter(BaseFilter): - def filter(self, glyph): + # pre=True so by default this is run before the RemoveOverlaps filter, + # in case a component overlaps other contours or components, to ensure + # the decomposed contours will be merged correctly: + # https://github.com/googlefonts/gftools/pull/425 + _pre = True + + def filter(self, glyph: Glyph) -> bool: if not glyph.components: return False - ufo2ft.util.deepCopyContours(self.context.glyphSet, glyph, glyph, Transform()) - glyph.clearComponents() + decomposeCompositeGlyph(glyph, self.context.glyphSet) + return True + + +class DecomposeComponentsIFilter(BaseIFilter): + _pre = True + + def filter(self, glyphName: str, glyphs: list[Glyph]) -> bool: + if not any(glyph.components for glyph in glyphs): + return False + + self.ensureCompositeDefinedAtComponentLocations(glyphName) + + for glyphSet, interpolatedLayer in zip_strict( + self.context.glyphSets, self.getInterpolatedLayers() + ): + glyph = glyphSet.get(glyphName) + if glyph is not None: + decomposeCompositeGlyph(glyph, interpolatedLayer or glyphSet) return True From 533df0b597b80f674f8ab2cf370ea662578eb1af Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 14 Mar 2024 19:07:37 +0000 Subject: [PATCH 07/27] [filters] Add SkipExportGlyphsFilter and SkipExportGlyphsIFilter --- Lib/ufo2ft/filters/__init__.py | 3 + Lib/ufo2ft/filters/skipExportGlyphs.py | 106 +++++++++++++++++++++++++ 2 files changed, 109 insertions(+) create mode 100644 Lib/ufo2ft/filters/skipExportGlyphs.py diff --git a/Lib/ufo2ft/filters/__init__.py b/Lib/ufo2ft/filters/__init__.py index 615d3c1e..0b7341cb 100644 --- a/Lib/ufo2ft/filters/__init__.py +++ b/Lib/ufo2ft/filters/__init__.py @@ -15,6 +15,7 @@ from .propagateAnchors import PropagateAnchorsFilter from .removeOverlaps import RemoveOverlapsFilter from .reverseContourDirection import ReverseContourDirectionFilter +from .skipExportGlyphs import SkipExportGlyphsFilter, SkipExportGlyphsIFilter from .sortContours import SortContoursFilter from .transformations import TransformationsFilter @@ -31,6 +32,8 @@ "PropagateAnchorsFilter", "RemoveOverlapsFilter", "ReverseContourDirectionFilter", + "SkipExportGlyphsFilter", + "SkipExportGlyphsIFilter", "SortContoursFilter", "TransformationsFilter", "loadFilters", diff --git a/Lib/ufo2ft/filters/skipExportGlyphs.py b/Lib/ufo2ft/filters/skipExportGlyphs.py new file mode 100644 index 00000000..83b78f35 --- /dev/null +++ b/Lib/ufo2ft/filters/skipExportGlyphs.py @@ -0,0 +1,106 @@ +from __future__ import annotations + +from ufo2ft.filters import BaseFilter, BaseIFilter +from ufo2ft.util import decomposeCompositeGlyph, zip_strict + + +class SkipExportGlyphsFilter(BaseFilter): + """Subset a glyphSet while decomposing references to pruned component glyphs.""" + + _pre = True + _args = ("skipExportGlyphs",) + + def start(self): + self.options.skipExportGlyphs = frozenset(self.options.skipExportGlyphs) + + def filter(self, glyph) -> bool: + if not glyph.components or self.options.skipExportGlyphs.isdisjoint( + comp.baseGlyph for comp in glyph.components + ): + return False + + # decomposeNested=False because at this stage we are only interested + # in pruning component references to specific non-export glyphs, not + # decomposing entire composite glyphs per se; it's conceivable that + # after a component is replaced by its direct referent and the latter + # in turn only comprises components, the parent can remain a composite + # glyph and need not be fully decomposed to contours; any further + # decompositions (e.g. of mixed glyphs) can be performed later. + decomposeCompositeGlyph( + glyph, + self.context.glyphSet, + decomposeNested=False, + include=self.options.skipExportGlyphs, + ) + return True + + def __call__(self, font, glyphSet=None): + if not self.options.skipExportGlyphs: + return self.context.modified # nothing to do + + modified = super().__call__(font, glyphSet) + + # now that all component references to non-export glyphs have been removed, + # the glyphSet can be subset in-place + glyphSet = self.context.glyphSet + for glyphName in self.options.skipExportGlyphs: + if glyphName in glyphSet: + del glyphSet[glyphName] + # technically this glyph was 'removed' rather than 'modified' but + # filters only return one set... + modified.add(glyphName) + + return modified + + +class SkipExportGlyphsIFilter(BaseIFilter): + """Interpolatable variant of SkipExportGlyphsFilter.""" + + _pre = True + _args = ("skipExportGlyphs",) + + def start(self): + self.options.skipExportGlyphs = frozenset(self.options.skipExportGlyphs) + + def filter(self, glyphName: str, glyphs: list) -> bool: + if not any(glyph.components for glyph in glyphs) or all( + self.options.skipExportGlyphs.isdisjoint( + comp.baseGlyph for comp in glyph.components + ) + for glyph in glyphs + ): + return False + + self.ensureCompositeDefinedAtComponentLocations( + glyphName, include=self.options.skipExportGlyphs + ) + + for glyphSet, interpolatedLayer in zip_strict( + self.context.glyphSets, self.getInterpolatedLayers() + ): + glyph = glyphSet.get(glyphName) + if glyph is not None: + decomposeCompositeGlyph( + glyph, + interpolatedLayer or glyphSet, + decomposeNested=False, + include=self.options.skipExportGlyphs, + ) + return True + + def __call__(self, fonts, glyphSets=None, instantiator=None, **kwargs): + if not self.options.skipExportGlyphs: + return self.context.modified # nothing to do + + modified = super().__call__( + fonts, glyphSets, instantiator=instantiator, **kwargs + ) + + for glyphName in self.options.skipExportGlyphs: + for glyphSet in self.context.glyphSets: + if glyphName in glyphSet: + del glyphSet[glyphName] + # mark removed glyphs among the 'modified' ones + modified.add(glyphName) + + return modified From 1f057ac1726e976c7c2b2f06d8d0b6ecc14eddee Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 14 Mar 2024 19:09:41 +0000 Subject: [PATCH 08/27] [filters] Add interpolatable DecomposeTransformedComponentsIFilter --- Lib/ufo2ft/filters/__init__.py | 6 ++- .../filters/decomposeTransformedComponents.py | 38 +++++++++++-------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/Lib/ufo2ft/filters/__init__.py b/Lib/ufo2ft/filters/__init__.py index 0b7341cb..762fb7c3 100644 --- a/Lib/ufo2ft/filters/__init__.py +++ b/Lib/ufo2ft/filters/__init__.py @@ -8,7 +8,10 @@ from .base import BaseFilter, BaseIFilter from .cubicToQuadratic import CubicToQuadraticFilter from .decomposeComponents import DecomposeComponentsFilter, DecomposeComponentsIFilter -from .decomposeTransformedComponents import DecomposeTransformedComponentsFilter +from .decomposeTransformedComponents import ( + DecomposeTransformedComponentsFilter, + DecomposeTransformedComponentsIFilter, +) from .dottedCircle import DottedCircleFilter from .explodeColorLayerGlyphs import ExplodeColorLayerGlyphsFilter from .flattenComponents import FlattenComponentsFilter @@ -26,6 +29,7 @@ "DecomposeComponentsFilter", "DecomposeComponentsIFilter", "DecomposeTransformedComponentsFilter", + "DecomposeTransformedComponentsIFilter", "DottedCircleFilter", "ExplodeColorLayerGlyphsFilter", "FlattenComponentsFilter", diff --git a/Lib/ufo2ft/filters/decomposeTransformedComponents.py b/Lib/ufo2ft/filters/decomposeTransformedComponents.py index 0c39769f..f3227345 100644 --- a/Lib/ufo2ft/filters/decomposeTransformedComponents.py +++ b/Lib/ufo2ft/filters/decomposeTransformedComponents.py @@ -1,20 +1,28 @@ -from fontTools.misc.transform import Identity, Transform +from fontTools.misc.transform import Identity -import ufo2ft.util -from ufo2ft.filters import BaseFilter +from ufo2ft.filters.decomposeComponents import ( + DecomposeComponentsFilter, + DecomposeComponentsIFilter, +) +IDENTITY_2x2 = Identity[:4] -class DecomposeTransformedComponentsFilter(BaseFilter): + +def _isTransformed(component): + return component.transformation[:4] != IDENTITY_2x2 + + +class DecomposeTransformedComponentsFilter(DecomposeComponentsFilter): def filter(self, glyph): - if not glyph.components: - return False - needs_decomposition = False - for component in glyph.components: - if component.transformation[:4] != Identity[:4]: - needs_decomposition = True - break - if not needs_decomposition: + if any(_isTransformed(c) for c in glyph.components): + return super().filter(glyph) + return False + + +class DecomposeTransformedComponentsIFilter(DecomposeComponentsIFilter): + def filter(self, glyphName, glyphs): + # We decomposes the glyph in *all* masters if any contains a transformed + # component: https://github.com/googlefonts/ufo2ft/issues/507 + if not any(any(_isTransformed(c) for c in g.components) for g in glyphs): return False - ufo2ft.util.deepCopyContours(self.context.glyphSet, glyph, glyph, Transform()) - glyph.clearComponents() - return True + return super().filter(glyphName, glyphs) From f7bb5bc9ce73d2ad729c783b62ad241b559502aa Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 14 Mar 2024 19:15:32 +0000 Subject: [PATCH 09/27] [filters] Add interpolatable FlattenComponentsIFilter --- Lib/ufo2ft/filters/__init__.py | 3 +- Lib/ufo2ft/filters/flattenComponents.py | 87 +++++++++++++++++++------ 2 files changed, 69 insertions(+), 21 deletions(-) diff --git a/Lib/ufo2ft/filters/__init__.py b/Lib/ufo2ft/filters/__init__.py index 762fb7c3..5b175f84 100644 --- a/Lib/ufo2ft/filters/__init__.py +++ b/Lib/ufo2ft/filters/__init__.py @@ -14,7 +14,7 @@ ) from .dottedCircle import DottedCircleFilter from .explodeColorLayerGlyphs import ExplodeColorLayerGlyphsFilter -from .flattenComponents import FlattenComponentsFilter +from .flattenComponents import FlattenComponentsFilter, FlattenComponentsIFilter from .propagateAnchors import PropagateAnchorsFilter from .removeOverlaps import RemoveOverlapsFilter from .reverseContourDirection import ReverseContourDirectionFilter @@ -33,6 +33,7 @@ "DottedCircleFilter", "ExplodeColorLayerGlyphsFilter", "FlattenComponentsFilter", + "FlattenComponentsIFilter", "PropagateAnchorsFilter", "RemoveOverlapsFilter", "ReverseContourDirectionFilter", diff --git a/Lib/ufo2ft/filters/flattenComponents.py b/Lib/ufo2ft/filters/flattenComponents.py index 03fda214..62f774b6 100644 --- a/Lib/ufo2ft/filters/flattenComponents.py +++ b/Lib/ufo2ft/filters/flattenComponents.py @@ -1,37 +1,84 @@ +from __future__ import annotations + import logging from fontTools.misc.transform import Transform -from ufo2ft.filters import BaseFilter +from ufo2ft.filters import BaseFilter, BaseIFilter +from ufo2ft.util import zip_strict logger = logging.getLogger(__name__) class FlattenComponentsFilter(BaseFilter): + """Replace nested components with their referents so that max depth is 1.""" + def __call__(self, font, glyphSet=None): - if super().__call__(font, glyphSet): - modified = self.context.modified - if modified: - logger.info("Flattened composite glyphs: %i" % len(modified)) - return modified + modified = super().__call__(font, glyphSet) + if modified: + logger.info("Flattened composite glyphs: %i" % len(modified)) + return modified def filter(self, glyph): + return _flattenGlyphComponents(glyph, self.context.glyphSet) + + +class FlattenComponentsIFilter(BaseIFilter): + """Interpolatable variant of FlattenComponentsFilter.""" + + def __call__(self, fonts, glyphSets=None, instantiator=None, **kwargs): + modified = super().__call__(fonts, glyphSets, instantiator, **kwargs) + if modified: + logger.info("Flattened composite glyphs: %i" % len(modified)) + return modified + + def filter(self, glyphName: str, glyphs: list) -> bool: flattened = False - if not glyph.components: + if not any(glyph.components for glyph in glyphs): + return flattened + + defaultGlyphSet = self.getDefaultGlyphSet() + if not any(_haveNestedComponents(g, defaultGlyphSet) for g in glyphs): return flattened - pen = glyph.getPen() - for comp in list(glyph.components): - flattened_tuples = _flattenComponent( - self.context.glyphSet, comp, found_in=glyph - ) - if flattened_tuples[0] != (comp.baseGlyph, comp.transformation): - flattened = True - glyph.removeComponent(comp) - for flattened_tuple in flattened_tuples: - pen.addComponent(*flattened_tuple) - if flattened: - self.context.modified.add(glyph.name) + + for glyphSet, interpolatedLayer in zip_strict( + self.context.glyphSets, self.getInterpolatedLayers() + ): + glyph = glyphSet.get(glyphName) + if glyph is not None: + flattened = _flattenGlyphComponents( + glyph, interpolatedLayer or glyphSet + ) + + return flattened + + +def _isSimpleOrMixed(glyph): + return not glyph.components or len(glyph) > 0 + + +def _haveNestedComponents(glyph, glyphSet): + return not _isSimpleOrMixed(glyph) and any( + glyphSet[compo.baseGlyph].components + for compo in glyph.components + if compo.baseGlyph in glyphSet + ) + + +def _flattenGlyphComponents(glyph, glyphSet): + flattened = False + if not glyph.components: return flattened + components = list(glyph.components) + glyph.clearComponents() + pen = glyph.getPointPen() + for comp in components: + flattened_tuples = _flattenComponent(glyphSet, comp, found_in=glyph) + if flattened_tuples[0] != (comp.baseGlyph, comp.transformation): + flattened = True + for flattened_tuple in flattened_tuples: + pen.addComponent(*flattened_tuple) + return flattened def _flattenComponent(glyphSet, component, found_in): @@ -43,7 +90,7 @@ def _flattenComponent(glyphSet, component, found_in): glyph = glyphSet[component.baseGlyph] # Any contour will cause components to be decomposed - if not glyph.components or len(glyph) > 0: + if _isSimpleOrMixed(glyph): transformation = Transform(*component.transformation) return [(component.baseGlyph, transformation)] From 7c382549a74f7ef935d68f78e9e8e77af7b13653 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 15 Mar 2024 12:07:33 +0000 Subject: [PATCH 10/27] [util] use SkipExportGlyphsFilter in _GlyphSet.from_layer also note the preProcess_test.py: previously if a glyph was marked non-export (e.g. _o.numero), we were decomposing it to contours as a whole alonside all of its nested components (e.g. 'o'), but that's unnecessary; now we only strictly decompose the components whose base glyph is marked as non-export, without recursing into their nested components, as there may still be a chance that after decomposing only those, the parent glyph can remain as a pure composite. --- Lib/ufo2ft/util.py | 27 +++++---------------------- tests/preProcessor_test.py | 7 +++++-- 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/Lib/ufo2ft/util.py b/Lib/ufo2ft/util.py index 2c1689ea..70ecc8d3 100644 --- a/Lib/ufo2ft/util.py +++ b/Lib/ufo2ft/util.py @@ -13,7 +13,7 @@ from fontTools.designspaceLib import DesignSpaceDocument from fontTools.feaLib.builder import addOpenTypeFeatures from fontTools.misc.fixedTools import otRound -from fontTools.misc.transform import Identity, Transform +from fontTools.misc.transform import Identity from fontTools.pens.filterPen import DecomposingFilterPointPen from fontTools.pens.reverseContourPen import ReverseContourPen from fontTools.pens.transformPen import TransformPen @@ -98,32 +98,15 @@ def from_layer(cls, font, layerName=None, copy=False, skipExportGlyphs=None): else: self = cls((g.name, g) for g in layer) self.lib = layer.lib + self.name = layer.name if layerName is not None else None # If any glyphs in the skipExportGlyphs list are used as components, decompose # them in the containing glyphs... if skipExportGlyphs: - for glyph in self.values(): - if any(c.baseGlyph in skipExportGlyphs for c in glyph.components): - deepCopyContours(self, glyph, glyph, Transform(), skipExportGlyphs) - if hasattr(glyph, "removeComponent"): # defcon - for c in [ - component - for component in glyph.components - if component.baseGlyph in skipExportGlyphs - ]: - glyph.removeComponent(c) - else: # ufoLib2 - glyph.components[:] = [ - c - for c in glyph.components - if c.baseGlyph not in skipExportGlyphs - ] - # ... and then remove them from the glyph set, if even present. - for glyph_name in skipExportGlyphs: - if glyph_name in self: - del self[glyph_name] + from ufo2ft.filters.skipExportGlyphs import SkipExportGlyphsFilter + + SkipExportGlyphsFilter(skipExportGlyphs)(font, self) - self.name = layer.name if layerName is not None else None return self diff --git a/tests/preProcessor_test.py b/tests/preProcessor_test.py index eec9ec5e..8d1ce8a4 100644 --- a/tests/preProcessor_test.py +++ b/tests/preProcessor_test.py @@ -338,8 +338,11 @@ def test_skip_export_glyphs_filter_nested(self, FontClass): skipExportGlyphs = ["_o.numero"] glyphSet = _GlyphSet.from_layer(ufo, skipExportGlyphs=skipExportGlyphs) - assert len(glyphSet["numero"].components) == 1 # The "N" component - assert len(glyphSet["numero"]) == 2 # The two contours of "o" and "_o.numero" + # "numero" now contains two components "N" and "o", and one contour from the + # decomposed "_o.numero" + assert {c.baseGlyph for c in glyphSet["numero"].components} == {"N", "o"} + assert len(glyphSet["numero"]) == 1 + assert set(glyphSet.keys()) == {"N", "numero", "o"} # "_o.numero" is gone def test_skip_export_glyphs_designspace(self, FontClass): # Designspace has a public.skipExportGlyphs lib key excluding "b" and "d". From 6d5462b84006674302b398408d4638cc225797ba Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 15 Mar 2024 12:17:17 +0000 Subject: [PATCH 11/27] [util] support keyword-only params in prune_unknown_kwargs --- Lib/ufo2ft/util.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Lib/ufo2ft/util.py b/Lib/ufo2ft/util.py index 70ecc8d3..b85e144d 100644 --- a/Lib/ufo2ft/util.py +++ b/Lib/ufo2ft/util.py @@ -601,7 +601,11 @@ def prune_unknown_kwargs(kwargs, *callables): """ known_args = set() for func in callables: - known_args.update(getfullargspec(func).args) + arg_spec = getfullargspec(func) + known_args.update(arg_spec.args) + # also handle optional keyword-only arguments + if arg_spec.kwonlydefaults: + known_args.update(arg_spec.kwonlydefaults) return {k: v for k, v in kwargs.items() if k in known_args} From 1f9d9184b4104673f8dcf5b0bb16df68a17fb76f Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 15 Mar 2024 12:32:52 +0000 Subject: [PATCH 12/27] make OutlineOTFCompiler and PostProcessor accept CFFOptimization enum in addition to bool this will simplify the OTFCompiler a bit --- Lib/ufo2ft/_compilers/otfCompiler.py | 8 -------- Lib/ufo2ft/outlineCompiler.py | 3 +++ Lib/ufo2ft/postProcessor.py | 8 ++++++-- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/Lib/ufo2ft/_compilers/otfCompiler.py b/Lib/ufo2ft/_compilers/otfCompiler.py index 48bbe3ae..5c1fa088 100644 --- a/Lib/ufo2ft/_compilers/otfCompiler.py +++ b/Lib/ufo2ft/_compilers/otfCompiler.py @@ -20,20 +20,12 @@ class OTFCompiler(BaseCompiler): _tables: Optional[list] = None extraSubstitutions: Optional[dict] = None - def compileOutlines(self, ufo, glyphSet): - kwargs = prune_unknown_kwargs(self.__dict__, self.outlineCompilerClass) - kwargs["optimizeCFF"] = self.optimizeCFF >= CFFOptimization.SPECIALIZE - kwargs["tables"] = self._tables - outlineCompiler = self.outlineCompilerClass(ufo, glyphSet=glyphSet, **kwargs) - return outlineCompiler.compile() - def postprocess(self, font, ufo, glyphSet, info=None): if self.postProcessorClass is not None: postProcessor = self.postProcessorClass( font, ufo, glyphSet=glyphSet, info=info ) kwargs = prune_unknown_kwargs(self.__dict__, postProcessor.process) - kwargs["optimizeCFF"] = self.optimizeCFF >= CFFOptimization.SUBROUTINIZE font = postProcessor.process(**kwargs) self._glyphSet = glyphSet return font diff --git a/Lib/ufo2ft/outlineCompiler.py b/Lib/ufo2ft/outlineCompiler.py index 38693c82..ac470e9e 100644 --- a/Lib/ufo2ft/outlineCompiler.py +++ b/Lib/ufo2ft/outlineCompiler.py @@ -37,6 +37,7 @@ OPENTYPE_META_KEY, OPENTYPE_POST_UNDERLINE_POSITION_KEY, UNICODE_VARIATION_SEQUENCES_KEY, + CFFOptimization, ) from ufo2ft.errors import InvalidFontData from ufo2ft.fontInfoData import ( @@ -1304,6 +1305,8 @@ def __init__( colrClipBoxQuantization=colrClipBoxQuantization, ftConfig=ftConfig, ) + if not isinstance(optimizeCFF, bool): + optimizeCFF = optimizeCFF >= CFFOptimization.SPECIALIZE self.optimizeCFF = optimizeCFF self._defaultAndNominalWidths = None diff --git a/Lib/ufo2ft/postProcessor.py b/Lib/ufo2ft/postProcessor.py index 7b7de419..c5378c60 100644 --- a/Lib/ufo2ft/postProcessor.py +++ b/Lib/ufo2ft/postProcessor.py @@ -10,6 +10,7 @@ GLYPHS_DONT_USE_PRODUCTION_NAMES, KEEP_GLYPH_NAMES, USE_PRODUCTION_NAMES, + CFFOptimization, ) logger = logging.getLogger(__name__) @@ -81,8 +82,9 @@ def process( when this is present if the UFO lib and is set to True, this is equivalent to 'useProductionNames' set to False. - optimizeCFF (bool): - Subroubtinize CFF or CFF2 table, if present. + optimizeCFF (bool | CFFOptimization): + If True or >= CFFOptimization.SUBROUTINIZE, subroubtinize CFF or CFF2 table + (if present). cffVersion (Optiona[int]): The output CFF format, choose between 1 or 2. By default, it's the same as @@ -95,6 +97,8 @@ def process( NOTE: compreffor currently doesn't support input fonts with CFF2 table. """ if self._get_cff_version(self.otf): + if not isinstance(optimizeCFF, bool): + optimizeCFF = optimizeCFF >= CFFOptimization.SUBROUTINIZE self.process_cff( optimizeCFF=optimizeCFF, cffVersion=cffVersion, From 2b08ba4a0671b93fc549bcb0e3bb60756d9bb157 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 15 Mar 2024 12:42:04 +0000 Subject: [PATCH 13/27] [outlineCompiler] add empty glyphs for missing components for non-default TTF masters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For TrueType only, when the glyphSet for a non default master contains composite glyphs that point to missing component base glyphs, we create and add empty glyphs so that the composite glyph is not dropped from the master TTF as invalid (i.e. pointing to nowhere); varLib will ignore those empty glyphs when building gvar variations, so the additional glyphs will not add extra masters. The glyph metrics (HVAR, VVAR) similarly will not have additional masters for the empty glyphs (we use a sentinel value understood by varLib 'skip me'). The composite glyph, on the other hand, will still be considered when building variations. One may use this technique to adjust the placement of components in a composite glyph, and/or its advance width/height, but only for some intermediate locations without requiring to define intermediate masters for the component glyphs themselves. --- Lib/ufo2ft/outlineCompiler.py | 47 +++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/Lib/ufo2ft/outlineCompiler.py b/Lib/ufo2ft/outlineCompiler.py index ac470e9e..d8816c62 100644 --- a/Lib/ufo2ft/outlineCompiler.py +++ b/Lib/ufo2ft/outlineCompiler.py @@ -50,6 +50,7 @@ from ufo2ft.instructionCompiler import InstructionCompiler from ufo2ft.util import ( _copyGlyph, + _getNewGlyphFactory, calcCodePageRanges, colrClipBoxQuantization, getMaxComponentDepth, @@ -115,11 +116,16 @@ def __init__( colrAutoClipBoxes=True, colrClipBoxQuantization=colrClipBoxQuantization, ftConfig=None, + *, + compilingVFDefaultSource=True, ): self.ufo = font # use the previously filtered glyphSet, if any if glyphSet is None: glyphSet = {g.name: g for g in font} + # this is set to False by Interpolatable{O,T}TFCompiler when building a VF's + # non-default masters. E.g. it's used by makeMissingRequiredGlyphs method below. + self.compilingVFDefaultSource = compilingVFDefaultSource self.makeMissingRequiredGlyphs(font, glyphSet, self.sfntVersion, notdefGlyph) self.allGlyphs = glyphSet # store the glyph order @@ -259,8 +265,7 @@ def makeUnicodeToGlyphNameMapping(self): """ return makeUnicodeToGlyphNameMapping(self.allGlyphs, self.glyphOrder) - @staticmethod - def makeMissingRequiredGlyphs(font, glyphSet, sfntVersion, notdefGlyph=None): + def makeMissingRequiredGlyphs(self, font, glyphSet, sfntVersion, notdefGlyph=None): """ Add .notdef to the glyph set if it is not present. @@ -290,6 +295,10 @@ def makeMissingRequiredGlyphs(font, glyphSet, sfntVersion, notdefGlyph=None): glyphSet[".notdef"] = notdefGlyph + def glyphFactory(self): + layer = self.ufo.layers.defaultLayer + return _getNewGlyphFactory(layer.instantiateGlyphObject()) + def makeOfficialGlyphOrder(self, glyphOrder): """ Make the final glyph order. @@ -1288,6 +1297,8 @@ def __init__( colrAutoClipBoxes=True, colrClipBoxQuantization=colrClipBoxQuantization, ftConfig=None, + *, + compilingVFDefaultSource=True, ): if roundTolerance is not None: self.roundTolerance = float(roundTolerance) @@ -1304,6 +1315,7 @@ def __init__( colrAutoClipBoxes=colrAutoClipBoxes, colrClipBoxQuantization=colrClipBoxQuantization, ftConfig=ftConfig, + compilingVFDefaultSource=compilingVFDefaultSource, ) if not isinstance(optimizeCFF, bool): optimizeCFF = optimizeCFF >= CFFOptimization.SPECIALIZE @@ -1628,6 +1640,8 @@ def __init__( roundCoordinates=True, glyphDataFormat=0, ftConfig=None, + *, + compilingVFDefaultSource=True, ): super().__init__( font, @@ -1639,12 +1653,41 @@ def __init__( colrAutoClipBoxes=colrAutoClipBoxes, colrClipBoxQuantization=colrClipBoxQuantization, ftConfig=ftConfig, + compilingVFDefaultSource=compilingVFDefaultSource, ) self.autoUseMyMetrics = autoUseMyMetrics self.dropImpliedOnCurves = dropImpliedOnCurves self.roundCoordinates = roundCoordinates self.glyphDataFormat = glyphDataFormat + def makeMissingRequiredGlyphs(self, font, glyphSet, sfntVersion, notdefGlyph=None): + """ + Add .notdef to the glyph set if it is not present. + + When compiling non-default interpolatable master TTFs used to build a VF, + if any 'sparse' composite glyphs reference missing components, we add empty base + glyphs so that the master TTFs' glyf table will keep the composites; varLib will + ignores these empty glyphs when building variations. + """ + super().makeMissingRequiredGlyphs(font, glyphSet, sfntVersion, notdefGlyph) + + if not self.compilingVFDefaultSource: + newGlyph = self.glyphFactory() + for glyphName in list(glyphSet.keys()): + glyph = glyphSet[glyphName] + for comp in glyph.components: + if comp.baseGlyph not in glyphSet: + logger.info( + "Added missing '%s' component base glyph, referenced from '%s'", + comp.baseGlyph, + glyphName, + ) + # use sentinel value for width/height to signal varLib this glyph + # doesn't participate in {H,V}VAR glyph metrics variations + glyphSet[comp.baseGlyph] = newGlyph( + comp.baseGlyph, width=0xFFFF, height=0xFFFF + ) + def compileGlyphs(self): """Compile and return the TrueType glyphs for this font.""" allGlyphs = self.allGlyphs From 20342e3f99da2fe616b6a1c536045ae1f34b6574 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 15 Mar 2024 12:51:10 +0000 Subject: [PATCH 14/27] [dottedCircle] use BaseFilter.context.glyphFactory; 'modified' must be a set() --- Lib/ufo2ft/filters/dottedCircle.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Lib/ufo2ft/filters/dottedCircle.py b/Lib/ufo2ft/filters/dottedCircle.py index 80662500..1b416e29 100644 --- a/Lib/ufo2ft/filters/dottedCircle.py +++ b/Lib/ufo2ft/filters/dottedCircle.py @@ -55,7 +55,7 @@ from ufo2ft.featureWriters import ast from ufo2ft.filters import BaseFilter from ufo2ft.fontInfoData import getAttrWithFallback -from ufo2ft.util import _getNewGlyphFactory, _GlyphSet, _LazyFontName, _setGlyphMargin +from ufo2ft.util import _GlyphSet, _LazyFontName, _setGlyphMargin logger = logging.getLogger(__name__) @@ -114,7 +114,7 @@ def __call__(self, font, glyphSet=None): dotted_circle_glyph = self.check_dotted_circle() if dotted_circle_glyph == DO_NOTHING: - return [] + return set() if not dotted_circle_glyph: dotted_circle_glyph = self.draw_dotted_circle(glyphSet) @@ -126,9 +126,9 @@ def __call__(self, font, glyphSet=None): self.ensure_base(dotted_circle_glyph) if added_glyph or added_anchors: - return [dotted_circle_glyph.name] + return {dotted_circle_glyph.name} else: - return [] + return set() def check_dotted_circle(self): """Check for the presence of a dotted circle glyph and return it""" @@ -150,8 +150,7 @@ def draw_dotted_circle(self, glyphSet): font = self.context.font logger.debug("Adding dotted circle glyph") - proto = font.layers.defaultLayer.instantiateGlyphObject() - glyph = _getNewGlyphFactory(proto)(name="uni25CC", unicodes=[0x25CC]) + glyph = self.context.glyphFactory(name="uni25CC", unicodes=[0x25CC]) pen = glyph.getPen() xHeight = getAttrWithFallback(font.info, "xHeight") From 9fb607f2a1d8a67e68fc61b8600e84906888ac43 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 15 Mar 2024 15:31:16 +0000 Subject: [PATCH 15/27] [instantiator] let self.glyph_mutators be empty, we alredy keep track of the default layer glyphs --- Lib/ufo2ft/instantiator.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/Lib/ufo2ft/instantiator.py b/Lib/ufo2ft/instantiator.py index 7af5cc76..aa822e8a 100644 --- a/Lib/ufo2ft/instantiator.py +++ b/Lib/ufo2ft/instantiator.py @@ -324,6 +324,7 @@ def from_designspace( f"Cannot set up kerning for interpolation: {e}'" ) from e + # left empty as the glyph sources will be loaded later on-demand glyph_mutators: Dict[str, Variator] = {} glyph_name_to_unicodes: Dict[str, List[int]] = {} source_layers = [] @@ -331,8 +332,6 @@ def from_designspace( glyph_factory = None if do_glyphs: for glyph_name in glyph_names: - # these will be loaded later on-demand - glyph_mutators[glyph_name] = None glyph_name_to_unicodes[glyph_name] = default_font[glyph_name].unicodes # collect (normalized location, source layer) tuples for i, source in enumerate(designspace.sources): @@ -510,13 +509,7 @@ def generate_glyph_instance( If output_glyph is None, the instance is generated in a new Glyph object and returned. Otherwise, the instance is extracted to the given Glyph object. """ - try: - glyph_mutator = self.glyph_mutators[glyph_name] - except KeyError as e: - raise InstantiatorError( - f"Failed to generate instance for glyph {glyph_name!r}: " - f"not found in the default source." - ) from e + glyph_mutator = self.glyph_mutators.get(glyph_name) if glyph_mutator is None: sources = collect_glyph_masters( self.source_layers, @@ -634,7 +627,6 @@ def replace_source_layers(self, new_layers: list[dict[str, Glyph]]): ] # this forces to reload the glyph variation models when an instance is requested self.glyph_mutators.clear() - self.glyph_mutators.update((gn, None) for gn in self.glyph_names) def _error_msg_no_default(designspace: designspaceLib.DesignSpaceDocument) -> str: @@ -744,14 +736,21 @@ def collect_glyph_masters( other_glyph_empty = False for i, (normalized_location, source_layer) in enumerate(source_layers): - # Sparse fonts do not and layers may not contain every glyph. + this_is_default = i == default_source_idx if glyph_name not in source_layer: - continue + if this_is_default: + # Default layer must contain every glyph by definition. + raise InstantiatorError( + f"glyph {glyph_name!r} not found in the default source" + ) + else: + # Sparse fonts do not and layers may not contain every glyph. + continue source_glyph = source_layer[glyph_name] if not (len(source_glyph) or source_glyph.components): - if i == default_source_idx: + if this_is_default: default_glyph_empty = True else: other_glyph_empty = True From 765fd4838e4d38ce131d74d718e12d6e540b5ad9 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 15 Mar 2024 16:49:27 +0000 Subject: [PATCH 16/27] [preProcessor] define {Base,OTF}InterpolatablePreProcessor Factored out some shared code from TTFInterpolatablePreProcessor into a new BaseInterpolatablePreProcessor, and defined a new OTFInterpolatablePreProcessor. The interpolatable pre-processors take an optional Instantiator instance, and pass that on to interpolatable filters so they can generate glyph instances on the fly. They also attempt to convert a list of non-interpolatable filters to an equivalent interpolatable filter (e.g. DecomposeTransformedComponentsFilters => DecomposeTransformedComponentsIFilter) so that client code (fontmake) that sets up filters doesn't need to change. --- Lib/ufo2ft/preProcessor.py | 284 ++++++++++++++++++++++++++++--------- 1 file changed, 218 insertions(+), 66 deletions(-) diff --git a/Lib/ufo2ft/preProcessor.py b/Lib/ufo2ft/preProcessor.py index 84a7fe90..0792a07f 100644 --- a/Lib/ufo2ft/preProcessor.py +++ b/Lib/ufo2ft/preProcessor.py @@ -1,4 +1,7 @@ +from __future__ import annotations + import itertools +from typing import TYPE_CHECKING from ufo2ft.constants import ( COLOR_LAYER_MAPPING_KEY, @@ -6,12 +9,16 @@ COLOR_PALETTES_KEY, ) from ufo2ft.filters import isValidFilter, loadFilters -from ufo2ft.filters.decomposeComponents import DecomposeComponentsFilter -from ufo2ft.filters.decomposeTransformedComponents import ( - DecomposeTransformedComponentsFilter, +from ufo2ft.filters.base import BaseFilter, BaseIFilter +from ufo2ft.filters.decomposeComponents import ( + DecomposeComponentsFilter, + DecomposeComponentsIFilter, ) from ufo2ft.fontInfoData import getAttrWithFallback -from ufo2ft.util import _GlyphSet +from ufo2ft.util import _GlyphSet, zip_strict + +if TYPE_CHECKING: + from ufo2ft.instantiator import Instantiator def _load_custom_filters(ufo, filters=None): @@ -240,7 +247,157 @@ def initDefaultFilters( return filters -class TTFInterpolatablePreProcessor: +class BaseInterpolatablePreProcessor: + """Base class for interpolatable pre-processors. + + These apply filters to same-named glyphs from multiple source layers at once, + ensuring that outlines are kept interpolation compatible. + + The optional `instantiator` can be used by filters to interpolate glyph + instances (e.g. when decomposing composite glyphs defined at more or less + source locations as some of their components' base glyphs). + """ + + def __init__( + self, + ufos, + inplace=False, + layerNames=None, + skipExportGlyphs=None, + filters=None, + *, + instantiator: Instantiator | None = None, + **kwargs, + ): + self.ufos = ufos + self.inplace = inplace + + if layerNames is None: + layerNames = [None] * len(ufos) + assert len(ufos) == len(layerNames) + self.layerNames = layerNames + + if instantiator is not None and len(instantiator.source_layers) != len(ufos): + raise ValueError( + f"Expected {len(ufos)} sources for instantiator; " + f"found {len(instantiator.source_layers)}" + ) + self.instantiator = instantiator + + # For each UFO, make a mapping of name to glyph object (and ensure it + # contains none of the glyphs to be skipped, or any references to it). + self.glyphSets = [ + _GlyphSet.from_layer(ufo, layerName, copy=not inplace) + for ufo, layerName in zip_strict(ufos, layerNames) + ] + if skipExportGlyphs: + from ufo2ft.filters.skipExportGlyphs import SkipExportGlyphsIFilter + + self._run(SkipExportGlyphsIFilter(skipExportGlyphs)) + + self.defaultFilters = self.initDefaultFilters(**kwargs) + + filterses = [_load_custom_filters(ufo, filters) for ufo in ufos] + self.preFilters = [[f for f in filters if f.pre] for filters in filterses] + self.postFilters = [[f for f in filters if not f.pre] for filters in filterses] + + def initDefaultFilters(self, **kwargs): + filterses = [] + for ufo in self.ufos: + filterses.append([]) + _init_explode_color_layer_glyphs_filter(ufo, filterses[-1]) + return filterses + + def process(self): + # first apply all custom pre-filters, then all default filters, and finally + # all custom post-filters + for filterses in (self.preFilters, self.defaultFilters, self.postFilters): + for filters in itertools.zip_longest(*filterses): + self._run(*filters) + return self.glyphSets + + def _update_instantiator(self): + # the instantiator's source layers must be updated after each filter is run, + # since each filter can modify/remove/add glyphs. + if self.instantiator is not None: + self.instantiator.replace_source_layers(self.glyphSets) + + def _run_interpolatable(self, filter_: BaseIFilter) -> set[str]: + # apply a single, interpolatable filter to all the glyphSets + modified = filter_(self.ufos, self.glyphSets, self.instantiator) + if modified: + self._update_instantiator() + return modified + + @staticmethod + def _try_as_interpolatable_filter( + filters: list[BaseFilter | None], + ) -> BaseIFilter | None: + # Try to combine multiple filters into a single interpolatable variant + assert len(filters) > 0 + filter_ = next(filter(None, filters)) + filter_class = type(filter_) + + if not all( + ( + type(f) is filter_class + and f.options == filter_.options + and f.pre == filter_.pre + ) + for f in filters[1:] + ): + return None + + if isinstance(filter_, BaseIFilter): + return filter_ + + ifilter_class = None + try: + ifilter_class = filter_class.getInterpolatableFilterClass() + except AttributeError: + pass + if ifilter_class is None: + return None + + if not isValidFilter(ifilter_class, BaseIFilter): + raise ValueError(f"Invalid interpolatable filter class: {ifilter_class!r}") + + # in the unlikely scenario individual filters have different includes, + # this effectively takes the union of those + def include(g): + return any(f.include(g) for f in filters) + + return ifilter_class( + pre=filter_.pre, + include=include, + **filter_.options.__dict__, + ) + + def _run(self, *filters: tuple[BaseFilter | None]) -> set[str]: + # apply either multiple (one per glyphSet) or a single filter to all glyphSets + if len(filters) == 1: + assert filters[0] is not None + if isinstance(filters[0], BaseIFilter): + return self._run_interpolatable(filters[0]) + + filters = [filters[0]] * len(self.ufos) + + # attempt to convert mutltiple filters to single interpolatable variant (if any) + if ifilter := self._try_as_interpolatable_filter(filters): + return self._run_interpolatable(ifilter) + + # or else apply individual filters to the respective glyphSet, one at a time, + # and hope for the best... + modified = set() + for filter_, ufo, glyphSet in zip_strict(filters, self.ufos, self.glyphSets): + if filter_ is not None: + modified |= filter_(ufo, glyphSet) + if modified: + self._update_instantiator() + return modified + + +class TTFInterpolatablePreProcessor(BaseInterpolatablePreProcessor): """Preprocessor for building TrueType-flavored OpenType fonts with interpolatable quadratic outlines. @@ -276,107 +433,82 @@ def __init__( skipExportGlyphs=None, filters=None, allQuadratic=True, + *, + instantiator: Instantiator | None = None, + **kwargs, ): from fontTools.cu2qu.ufo import DEFAULT_MAX_ERR - self.ufos = ufos - self.inplace = inplace + super().__init__( + ufos, + inplace=inplace, + layerNames=layerNames, + skipExportGlyphs=skipExportGlyphs, + filters=filters, + instantiator=instantiator, + **kwargs, + ) self.flattenComponents = flattenComponents - - if layerNames is None: - layerNames = [None] * len(ufos) - assert len(ufos) == len(layerNames) - self.layerNames = layerNames - - # For each UFO, make a mapping of name to glyph object (and ensure it - # contains none of the glyphs to be skipped, or any references to it). - self.glyphSets = [ - _GlyphSet.from_layer( - ufo, layerName, copy=not inplace, skipExportGlyphs=skipExportGlyphs - ) - for ufo, layerName in zip(ufos, layerNames) - ] self.convertCubics = convertCubics self._conversionErrors = [ (conversionError or DEFAULT_MAX_ERR) * getAttrWithFallback(ufo.info, "unitsPerEm") - for ufo in ufos + for ufo in self.ufos ] self._reverseDirection = reverseDirection self._rememberCurveType = rememberCurveType self.allQuadratic = allQuadratic - self.defaultFilters = [] - for ufo in ufos: - self.defaultFilters.append([]) - _init_explode_color_layer_glyphs_filter(ufo, self.defaultFilters[-1]) - - filterses = [_load_custom_filters(ufo, filters) for ufo in ufos] - self.preFilters = [[f for f in filters if f.pre] for filters in filterses] - self.postFilters = [[f for f in filters if not f.pre] for filters in filterses] - def process(self): from fontTools.cu2qu.ufo import fonts_to_quadratic - needs_decomposition = set() - # first apply all custom pre-filters - for funcs, ufo, glyphSet in zip(self.preFilters, self.ufos, self.glyphSets): - for func in funcs: - if isinstance(func, DecomposeTransformedComponentsFilter): - needs_decomposition |= func(ufo, glyphSet) - else: - func(ufo, glyphSet) + for funcs in itertools.zip_longest(*self.preFilters): + self._run(*funcs) + # TrueType fonts cannot mix contours and components, so pick out all glyphs + # that have both contours _and_ components. + needs_decomposition = { + gname + for glyphSet in self.glyphSets + for gname, glyph in glyphSet.items() + if len(glyph) > 0 and glyph.components + } + # Variable fonts can only variate glyf components' x or y offsets, not their + # 2x2 transformation matrix; decompose of these don't match across masters self.check_for_nonmatching_components(needs_decomposition) - - # If we decomposed a glyph in some masters, we must ensure it is decomposed in - # all masters. (https://github.com/googlefonts/ufo2ft/issues/507) if needs_decomposition: - decompose = DecomposeComponentsFilter(include=needs_decomposition) - for ufo, glyphSet in zip(self.ufos, self.glyphSets): - decompose(ufo, glyphSet) + self._run(DecomposeComponentsIFilter(include=needs_decomposition)) # then apply all default filters - for funcs, ufo, glyphSet in zip(self.defaultFilters, self.ufos, self.glyphSets): - for func in funcs: - func(ufo, glyphSet) + for funcs in itertools.zip_longest(*self.defaultFilters): + self._run(*funcs) if self.convertCubics: - fonts_to_quadratic( + if fonts_to_quadratic( self.glyphSets, max_err=self._conversionErrors, reverse_direction=self._reverseDirection, dump_stats=True, remember_curve_type=self._rememberCurveType and self.inplace, all_quadratic=self.allQuadratic, - ) + ): + self._update_instantiator() elif self._reverseDirection: from ufo2ft.filters.reverseContourDirection import ( ReverseContourDirectionFilter, ) - reverseDirection = ReverseContourDirectionFilter(include=lambda g: len(g)) - for ufo, glyphSet in zip(self.ufos, self.glyphSets): - reverseDirection(ufo, glyphSet) - - # TrueType fonts cannot mix contours and components, so pick out all glyphs - # that have contours (`bool(len(g)) == True`) and decompose their - # components, if any. - decompose = DecomposeComponentsFilter(include=lambda g: len(g)) - for ufo, glyphSet in zip(self.ufos, self.glyphSets): - decompose(ufo, glyphSet) + self._run(ReverseContourDirectionFilter(include=lambda g: len(g))) if self.flattenComponents: - from ufo2ft.filters.flattenComponents import FlattenComponentsFilter + from ufo2ft.filters.flattenComponents import FlattenComponentsIFilter - for ufo, glyphSet in zip(self.ufos, self.glyphSets): - FlattenComponentsFilter()(ufo, glyphSet) + self._run(FlattenComponentsIFilter(include=lambda g: len(g.components))) # finally apply all custom post-filters - for funcs, ufo, glyphSet in zip(self.postFilters, self.ufos, self.glyphSets): - for func in funcs: - func(ufo, glyphSet) + for funcs in itertools.zip_longest(*self.postFilters): + self._run(*funcs) return self.glyphSets @@ -411,3 +543,23 @@ def check_for_nonmatching_components(self, needs_decomposition): if any(transform != transforms[0] for transform in transforms): needs_decomposition.add(glyph) break + + +class OTFInterpolatablePreProcessor(BaseInterpolatablePreProcessor): + """Interpolatable pre-processor for CFF-flavored fonts. + + By default, besides any user-defined custom pre/post filters, this decomposes + all composite glyphs, which aren't a thing in PostScript outlines. + + Unlike the non-interpolatable OTFPreProcessor, overlaps are *not* removed as + that could make outlines incompatible for interpolation. + """ + + def initDefaultFilters(self, **kwargs): + filterses = super().initDefaultFilters(**kwargs) + # this interpolatable filter will only run once on all the glyphSets, + # (see _try_as_interpolatable_filter) + decompose = DecomposeComponentsIFilter() + for filters in filterses: + filters.append(decompose) + return filterses From 74cef4ad434f6cbd35286e5adbebd5c3f8b797a8 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 15 Mar 2024 17:48:17 +0000 Subject: [PATCH 17/27] [_compilers] factor out shared code between interpolatable compilers --- Lib/ufo2ft/_compilers/baseCompiler.py | 49 +++++++++++++++++-- .../_compilers/interpolatableOTFCompiler.py | 48 ++++-------------- .../_compilers/interpolatableTTFCompiler.py | 43 +--------------- Lib/ufo2ft/_compilers/otfCompiler.py | 13 ----- .../_compilers/variableCFF2sCompiler.py | 4 +- 5 files changed, 58 insertions(+), 99 deletions(-) diff --git a/Lib/ufo2ft/_compilers/baseCompiler.py b/Lib/ufo2ft/_compilers/baseCompiler.py index c581452e..0c41f49c 100644 --- a/Lib/ufo2ft/_compilers/baseCompiler.py +++ b/Lib/ufo2ft/_compilers/baseCompiler.py @@ -18,6 +18,7 @@ ) from ufo2ft.postProcessor import PostProcessor from ufo2ft.util import ( + _LazyFontName, _notdefGlyphFallback, colrClipBoxQuantization, ensure_all_sources_have_names, @@ -48,7 +49,6 @@ class BaseCompiler: feaIncludeDir: Optional[str] = None skipFeatureCompilation: bool = False ftConfig: dict = field(default_factory=dict) - _tables: Optional[list] = None def __post_init__(self): self.logger = logging.getLogger("ufo2ft") @@ -93,7 +93,6 @@ def preprocess(self, ufo_or_ufos): def compileOutlines(self, ufo, glyphSet): kwargs = prune_unknown_kwargs(self.__dict__, self.outlineCompilerClass) - kwargs["tables"] = self._tables outlineCompiler = self.outlineCompilerClass(ufo, glyphSet=glyphSet, **kwargs) return outlineCompiler.compile() @@ -153,7 +152,6 @@ def compileFeatures( @dataclass class BaseInterpolatableCompiler(BaseCompiler): - variableFontNames: Optional[list] = None """Create FontTools TrueType fonts from the DesignSpaceDocument UFO sources with interpolatable outlines. Cubic curves are converted compatibly to quadratic curves using the Cu2Qu conversion algorithm. @@ -179,6 +177,51 @@ class BaseInterpolatableCompiler(BaseCompiler): "maxp", "post" and "vmtx"), and no OpenType layout tables. """ + extraSubstitutions: Optional[dict] = None + variableFontNames: Optional[list] = None + + def compile(self, ufos): + if self.layerNames is None: + self.layerNames = [None] * len(ufos) + assert len(ufos) == len(self.layerNames) + self.glyphSets = self.preprocess(ufos) + + for i, (ufo, glyphSet, layerName) in enumerate( + zip(ufos, self.glyphSets, self.layerNames) + ): + yield self.compile_one(ufo, glyphSet, layerName) + + def compile_one(self, ufo, glyphSet, layerName): + fontName = _LazyFontName(ufo) + if layerName is not None: + self.logger.info("Building OpenType tables for %s-%s", fontName, layerName) + else: + self.logger.info("Building OpenType tables for %s", fontName) + + ttf = self.compileOutlines(ufo, glyphSet, layerName) + + # Only the default layer is likely to have all glyphs used in feature + # code. + if layerName is None and not self.skipFeatureCompilation: + if self.debugFeatureFile: + self.debugFeatureFile.write("\n### %s ###\n" % fontName) + self.compileFeatures(ufo, ttf, glyphSet=glyphSet) + + ttf = self.postprocess(ttf, ufo, glyphSet) + + if layerName is not None and "post" in ttf: + # for sparse masters (i.e. containing only a subset of the glyphs), we + # need to include the post table in order to store glyph names, so that + # fontTools.varLib can interpolate glyphs with same name across masters. + # However we want to prevent the underlinePosition/underlineThickness + # fields in such sparse masters to be included when computing the deltas + # for the MVAR table. Thus, we set them to this unlikely, limit value + # (-36768) which is a signal varLib should ignore them when building MVAR. + ttf["post"].underlinePosition = -0x8000 + ttf["post"].underlineThickness = -0x8000 + + return ttf + def compile_designspace(self, designSpaceDoc): ufos = self._pre_compile_designspace(designSpaceDoc) ttfs = self.compile(ufos) diff --git a/Lib/ufo2ft/_compilers/interpolatableOTFCompiler.py b/Lib/ufo2ft/_compilers/interpolatableOTFCompiler.py index 9ebdde9d..1d70e093 100644 --- a/Lib/ufo2ft/_compilers/interpolatableOTFCompiler.py +++ b/Lib/ufo2ft/_compilers/interpolatableOTFCompiler.py @@ -1,4 +1,3 @@ -import dataclasses from dataclasses import dataclass from typing import Optional, Type @@ -6,57 +5,28 @@ from ufo2ft.constants import SPARSE_OTF_MASTER_TABLES, CFFOptimization from ufo2ft.outlineCompiler import OutlineOTFCompiler -from ufo2ft.preProcessor import OTFPreProcessor +from ufo2ft.preProcessor import OTFInterpolatablePreProcessor from ufo2ft.util import prune_unknown_kwargs from .baseCompiler import BaseInterpolatableCompiler -from .otfCompiler import OTFCompiler -# We want the designspace handling of BaseInterpolatableCompiler but -# we also need to pick up the OTF-specific compileOutlines/postprocess -# methods from OTFCompiler. @dataclass -class InterpolatableOTFCompiler(OTFCompiler, BaseInterpolatableCompiler): - preProcessorClass: Type = OTFPreProcessor +class InterpolatableOTFCompiler(BaseInterpolatableCompiler): + preProcessorClass: Type = OTFInterpolatablePreProcessor outlineCompilerClass: Type = OutlineOTFCompiler - featureCompilerClass: Optional[Type] = None roundTolerance: Optional[float] = None optimizeCFF: CFFOptimization = CFFOptimization.NONE colrLayerReuse: bool = False colrAutoClipBoxes: bool = False - extraSubstitutions: Optional[dict] = None skipFeatureCompilation: bool = False - excludeVariationTables: tuple = () - # We can't use the same compile method as interpolatableTTFCompiler - # because that has a TTFInterpolatablePreProcessor which preprocesses - # all UFOs together, whereas we need to do the preprocessing one at - # at a time. - def compile(self, ufos): - otfs = [] - for ufo, layerName in zip(ufos, self.layerNames): - # There's a Python bug where dataclasses.asdict() doesn't work with - # dataclasses that contain a defaultdict. - save_extraSubstitutions = self.extraSubstitutions - self.extraSubstitutions = None - args = { - **dataclasses.asdict(self), - **dict( - layerName=layerName, - removeOverlaps=False, - overlapsBackend=None, - optimizeCFF=CFFOptimization.NONE, - _tables=SPARSE_OTF_MASTER_TABLES if layerName else None, - ), - } - # Remove interpolatable-specific args - args = prune_unknown_kwargs(args, OTFCompiler) - compiler = OTFCompiler(**args) - self.extraSubstitutions = save_extraSubstitutions - otfs.append(compiler.compile(ufo)) - self.glyphSets.append(compiler._glyphSet) - return otfs + def compileOutlines(self, ufo, glyphSet, layerName=None): + kwargs = prune_unknown_kwargs(self.__dict__, self.outlineCompilerClass) + kwargs["tables"] = SPARSE_OTF_MASTER_TABLES if layerName is not None else None + kwargs["optimizeCFF"] = CFFOptimization.NONE + outlineCompiler = self.outlineCompilerClass(ufo, glyphSet=glyphSet, **kwargs) + return outlineCompiler.compile() def _merge(self, designSpaceDoc, excludeVariationTables): return varLib.build_many( diff --git a/Lib/ufo2ft/_compilers/interpolatableTTFCompiler.py b/Lib/ufo2ft/_compilers/interpolatableTTFCompiler.py index 4244a067..f797b848 100644 --- a/Lib/ufo2ft/_compilers/interpolatableTTFCompiler.py +++ b/Lib/ufo2ft/_compilers/interpolatableTTFCompiler.py @@ -6,7 +6,7 @@ from ufo2ft.constants import SPARSE_TTF_MASTER_TABLES from ufo2ft.outlineCompiler import OutlineTTFCompiler from ufo2ft.preProcessor import TTFInterpolatablePreProcessor -from ufo2ft.util import _LazyFontName, prune_unknown_kwargs +from ufo2ft.util import prune_unknown_kwargs from .baseCompiler import BaseInterpolatableCompiler @@ -22,51 +22,10 @@ class InterpolatableTTFCompiler(BaseInterpolatableCompiler): layerNames: Optional[str] = None colrLayerReuse: bool = False colrAutoClipBoxes: bool = False - extraSubstitutions: Optional[bool] = None autoUseMyMetrics: bool = True allQuadratic: bool = True skipFeatureCompilation: bool = False - def compile(self, ufos): - if self.layerNames is None: - self.layerNames = [None] * len(ufos) - assert len(ufos) == len(self.layerNames) - self.glyphSets = self.preprocess(ufos) - - for ufo, glyphSet, layerName in zip(ufos, self.glyphSets, self.layerNames): - yield self.compile_one(ufo, glyphSet, layerName) - - def compile_one(self, ufo, glyphSet, layerName): - fontName = _LazyFontName(ufo) - if layerName is not None: - self.logger.info("Building OpenType tables for %s-%s", fontName, layerName) - else: - self.logger.info("Building OpenType tables for %s", fontName) - - ttf = self.compileOutlines(ufo, glyphSet, layerName) - - # Only the default layer is likely to have all glyphs used in feature - # code. - if layerName is None and not self.skipFeatureCompilation: - if self.debugFeatureFile: - self.debugFeatureFile.write("\n### %s ###\n" % fontName) - self.compileFeatures(ufo, ttf, glyphSet=glyphSet) - - ttf = self.postprocess(ttf, ufo, glyphSet) - - if layerName is not None: - # for sparse masters (i.e. containing only a subset of the glyphs), we - # need to include the post table in order to store glyph names, so that - # fontTools.varLib can interpolate glyphs with same name across masters. - # However we want to prevent the underlinePosition/underlineThickness - # fields in such sparse masters to be included when computing the deltas - # for the MVAR table. Thus, we set them to this unlikely, limit value - # (-36768) which is a signal varLib should ignore them when building MVAR. - ttf["post"].underlinePosition = -0x8000 - ttf["post"].underlineThickness = -0x8000 - - return ttf - def compileOutlines(self, ufo, glyphSet, layerName=None): kwargs = prune_unknown_kwargs(self.__dict__, self.outlineCompilerClass) kwargs["glyphDataFormat"] = 0 if self.allQuadratic else 1 diff --git a/Lib/ufo2ft/_compilers/otfCompiler.py b/Lib/ufo2ft/_compilers/otfCompiler.py index 5c1fa088..964add06 100644 --- a/Lib/ufo2ft/_compilers/otfCompiler.py +++ b/Lib/ufo2ft/_compilers/otfCompiler.py @@ -4,7 +4,6 @@ from ufo2ft.constants import CFFOptimization from ufo2ft.outlineCompiler import OutlineOTFCompiler from ufo2ft.preProcessor import OTFPreProcessor -from ufo2ft.util import prune_unknown_kwargs from .baseCompiler import BaseCompiler @@ -17,15 +16,3 @@ class OTFCompiler(BaseCompiler): roundTolerance: Optional[float] = None cffVersion: int = 1 subroutinizer: Optional[str] = None - _tables: Optional[list] = None - extraSubstitutions: Optional[dict] = None - - def postprocess(self, font, ufo, glyphSet, info=None): - if self.postProcessorClass is not None: - postProcessor = self.postProcessorClass( - font, ufo, glyphSet=glyphSet, info=info - ) - kwargs = prune_unknown_kwargs(self.__dict__, postProcessor.process) - font = postProcessor.process(**kwargs) - self._glyphSet = glyphSet - return font diff --git a/Lib/ufo2ft/_compilers/variableCFF2sCompiler.py b/Lib/ufo2ft/_compilers/variableCFF2sCompiler.py index b79cb543..c31d383d 100644 --- a/Lib/ufo2ft/_compilers/variableCFF2sCompiler.py +++ b/Lib/ufo2ft/_compilers/variableCFF2sCompiler.py @@ -3,14 +3,14 @@ from ufo2ft.constants import CFFOptimization from ufo2ft.outlineCompiler import OutlineOTFCompiler -from ufo2ft.preProcessor import OTFPreProcessor +from ufo2ft.preProcessor import OTFInterpolatablePreProcessor from .interpolatableOTFCompiler import InterpolatableOTFCompiler @dataclass class VariableCFF2sCompiler(InterpolatableOTFCompiler): - preProcessorClass: Type = OTFPreProcessor + preProcessorClass: Type = OTFInterpolatablePreProcessor outlineCompilerClass: Type = OutlineOTFCompiler roundTolerance: Optional[float] = None colrAutoClipBoxes: bool = False From 12ee0dcbf18d74d011b544f6df77e640e4232aa5 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 15 Mar 2024 17:53:52 +0000 Subject: [PATCH 18/27] [baseCompiler] set up Instantiator when running from DS --- Lib/ufo2ft/_compilers/baseCompiler.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Lib/ufo2ft/_compilers/baseCompiler.py b/Lib/ufo2ft/_compilers/baseCompiler.py index 0c41f49c..d1b6c1fd 100644 --- a/Lib/ufo2ft/_compilers/baseCompiler.py +++ b/Lib/ufo2ft/_compilers/baseCompiler.py @@ -16,6 +16,7 @@ VariableFeatureCompiler, _featuresCompatible, ) +from ufo2ft.instantiator import Instantiator from ufo2ft.postProcessor import PostProcessor from ufo2ft.util import ( _LazyFontName, @@ -180,15 +181,26 @@ class BaseInterpolatableCompiler(BaseCompiler): extraSubstitutions: Optional[dict] = None variableFontNames: Optional[list] = None + # used to generate glyph instances on-the-fly (e.g. decomposing sparse composites) + instantiator: Optional[Instantiator] = field(init=False, default=None) + # We may need to compile things differently based on whether the source is default + # or not: e.g. handling of composite glyphs pointing to missing components. + compilingVFDefaultSource: bool = field(init=False, default=True) + def compile(self, ufos): if self.layerNames is None: self.layerNames = [None] * len(ufos) assert len(ufos) == len(self.layerNames) self.glyphSets = self.preprocess(ufos) + default_idx = ( + self.instantiator.default_source_idx if self.instantiator else None + ) for i, (ufo, glyphSet, layerName) in enumerate( zip(ufos, self.glyphSets, self.layerNames) ): + if default_idx is not None: + self.compilingVFDefaultSource = i == default_idx yield self.compile_one(ufo, glyphSet, layerName) def compile_one(self, ufo, glyphSet, layerName): @@ -249,6 +261,11 @@ def _pre_compile_designspace(self, designSpaceDoc): for left, right in rule.subs: self.extraSubstitutions[left].add(right) + # used to interpolate glyphs on-the-fly in filters (e.g. DecomposeComponents) + self.instantiator = Instantiator.from_designspace( + designSpaceDoc, round_geometry=False, do_info=False, do_kerning=False + ) + return ufos def _post_compile_designspace(self, designSpaceDoc, fonts): From a248bacec7e2b85024938daea40162aae0a790f5 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 15 Mar 2024 18:02:39 +0000 Subject: [PATCH 19/27] [tests] fix up variable CFF2 test expectations Thanks to our new shiny interpolatable DecomposeComponentsIFilter, our test variable CFF2 font looks as it should have been from day one! The test font contained a composite glyph 'edotabove' which used 'e' as one of its component, and the latter's glyph in turn defined one extra master at the middle (in a sparse layer); since in CFF fonts we have to decompose all composites, before we would simply ignore that 'e' had this intermediate master and the decomposed 'edotabove' would have one fewer set of deltas and thus look incorrectly and unlike 'e'. Now it is decomposed correctly, with the intermediate master 'bubbling up' from the 'e' component to the 'edotabove' composite glyph. --- tests/data/TestVariableFont-CFF2-cffsubr.ttx | 34 ++++++++----------- tests/data/TestVariableFont-CFF2-post3.ttx | 14 ++++---- ...stVariableFont-CFF2-sparse-notdefGlyph.ttx | 14 ++++---- ...stVariableFont-CFF2-useProductionNames.ttx | 14 ++++---- tests/data/TestVariableFont-CFF2.ttx | 14 ++++---- tests/outlineCompiler_test.py | 7 +++- 6 files changed, 48 insertions(+), 49 deletions(-) diff --git a/tests/data/TestVariableFont-CFF2-cffsubr.ttx b/tests/data/TestVariableFont-CFF2-cffsubr.ttx index b2a8804a..501f4f6c 100644 --- a/tests/data/TestVariableFont-CFF2-cffsubr.ttx +++ b/tests/data/TestVariableFont-CFF2-cffsubr.ttx @@ -206,17 +206,17 @@ - 2 blend + 1 vsindex + 127 228 -1 70 -25 1 2 blend rmoveto - -16 -94 78 -2 9 88 -19 -48 44 7 -4 29 6 blend + 449 -2 1 -45 -2 -2 2 blend + -5 79 -255 208 -276 -252 148 -279 338 63 -17 84 -280 -54 -82 188 170 153 163 -124 -355 6 27 0 0 -27 0 36 0 -29 0 -34 0 31 0 -1 0 2 0 -45 -2 13 28 100 37 0 13 0 -2 55 -40 -54 -32 -86 -30 -57 -85 -60 34 57 84 146 -5 0 21 blend rlineto - -280 -54 -82 188 170 153 163 -124 -355 - - 1 2 blend + 2 blend rmoveto - 449 -2 + -16 -94 78 -2 9 88 @@ -241,24 +241,18 @@ rlineto - -21 597 -8 28 -107 callsubr + -21 597 -8 28 -106 callsubr + -19 -48 44 7 -4 29 6 blend + rlineto - 1 vsindex - 127 228 -1 70 -25 -105 callsubr - 1 -45 -2 -2 2 blend - -5 79 -255 208 -276 -252 148 -279 338 63 -17 84 -106 callsubr - 6 27 0 0 -27 0 36 0 -29 0 -34 0 31 0 -1 0 2 0 -45 -2 13 28 100 37 0 13 0 -2 55 -40 -54 -32 -86 -30 -57 -85 -60 34 57 84 146 -5 0 21 blend - rlineto + -107 callsubr - 127 228 70 -105 callsubr - -45 -2 2 blend - -5 79 -255 208 -276 -252 148 -279 338 63 -17 84 -27 36 -29 -34 31 -1 2 -45 13 100 10 blend - -106 callsubr - 55 -54 -86 -57 -60 57 146 7 blend - 6 rlineto - 167 395 -84 118 -107 callsubr + -107 callsubr + 167 395 -5 -84 43 118 -106 callsubr + -7 -19 -17 -48 16 44 3 7 -1 -4 10 29 6 blend + rlineto 559 459 rmoveto diff --git a/tests/data/TestVariableFont-CFF2-post3.ttx b/tests/data/TestVariableFont-CFF2-post3.ttx index d8f17e02..ddbd8b29 100644 --- a/tests/data/TestVariableFont-CFF2-post3.ttx +++ b/tests/data/TestVariableFont-CFF2-post3.ttx @@ -220,15 +220,15 @@ rlineto - 127 228 70 1 2 blend + 1 vsindex + 127 228 -1 70 -25 1 2 blend rmoveto - 449 -2 -45 -2 2 blend - -5 79 -255 208 -276 -252 148 -279 338 63 -17 84 -27 36 -29 -34 31 -1 2 -45 13 100 10 blend - -280 -54 -82 188 170 153 163 -124 -355 55 -54 -86 -57 -60 57 146 7 blend - 6 rlineto - 167 395 -84 118 2 blend + 449 -2 1 -45 -2 -2 2 blend + -5 79 -255 208 -276 -252 148 -279 338 63 -17 84 -280 -54 -82 188 170 153 163 -124 -355 6 27 0 0 -27 0 36 0 -29 0 -34 0 31 0 -1 0 2 0 -45 -2 13 28 100 37 0 13 0 -2 55 -40 -54 -32 -86 -30 -57 -85 -60 34 57 84 146 -5 0 21 blend + rlineto + 167 395 -5 -84 43 118 2 blend rmoveto - -16 -94 78 -2 9 88 -19 -48 44 7 -4 29 6 blend + -16 -94 78 -2 9 88 -7 -19 -17 -48 16 44 3 7 -1 -4 10 29 6 blend rlineto diff --git a/tests/data/TestVariableFont-CFF2-sparse-notdefGlyph.ttx b/tests/data/TestVariableFont-CFF2-sparse-notdefGlyph.ttx index 34f11787..94c9532a 100644 --- a/tests/data/TestVariableFont-CFF2-sparse-notdefGlyph.ttx +++ b/tests/data/TestVariableFont-CFF2-sparse-notdefGlyph.ttx @@ -55,15 +55,15 @@ rlineto - 127 228 70 1 2 blend + 1 vsindex + 127 228 -1 70 -25 1 2 blend rmoveto - 449 -2 -45 -2 2 blend - -5 79 -255 208 -276 -252 148 -279 338 63 -17 84 -27 36 -29 -34 31 -1 2 -45 13 100 10 blend - -280 -54 -82 188 170 153 163 -124 -355 55 -54 -86 -57 -60 57 146 7 blend - 6 rlineto - 167 395 -84 118 2 blend + 449 -2 1 -45 -2 -2 2 blend + -5 79 -255 208 -276 -252 148 -279 338 63 -17 84 -280 -54 -82 188 170 153 163 -124 -355 6 27 0 0 -27 0 36 0 -29 0 -34 0 31 0 -1 0 2 0 -45 -2 13 28 100 37 0 13 0 -2 55 -40 -54 -32 -86 -30 -57 -85 -60 34 57 84 146 -5 0 21 blend + rlineto + 167 395 -5 -84 43 118 2 blend rmoveto - -16 -94 78 -2 9 88 -19 -48 44 7 -4 29 6 blend + -16 -94 78 -2 9 88 -7 -19 -17 -48 16 44 3 7 -1 -4 10 29 6 blend rlineto diff --git a/tests/data/TestVariableFont-CFF2-useProductionNames.ttx b/tests/data/TestVariableFont-CFF2-useProductionNames.ttx index 9bf82105..b01e9939 100644 --- a/tests/data/TestVariableFont-CFF2-useProductionNames.ttx +++ b/tests/data/TestVariableFont-CFF2-useProductionNames.ttx @@ -242,15 +242,15 @@ 213 -66 rlineto - 127 228 70 1 2 blend + 1 vsindex + 127 228 -1 70 -25 1 2 blend rmoveto - 449 -2 -45 -2 2 blend - -5 79 -255 208 -276 -252 148 -279 338 63 -17 84 -27 36 -29 -34 31 -1 2 -45 13 100 10 blend - -280 -54 -82 188 170 153 163 -124 -355 55 -54 -86 -57 -60 57 146 7 blend - 6 rlineto - 167 395 -84 118 2 blend + 449 -2 1 -45 -2 -2 2 blend + -5 79 -255 208 -276 -252 148 -279 338 63 -17 84 -280 -54 -82 188 170 153 163 -124 -355 6 27 0 0 -27 0 36 0 -29 0 -34 0 31 0 -1 0 2 0 -45 -2 13 28 100 37 0 13 0 -2 55 -40 -54 -32 -86 -30 -57 -85 -60 34 57 84 146 -5 0 21 blend + rlineto + 167 395 -5 -84 43 118 2 blend rmoveto - -16 -94 78 -2 9 88 -19 -48 44 7 -4 29 6 blend + -16 -94 78 -2 9 88 -7 -19 -17 -48 16 44 3 7 -1 -4 10 29 6 blend rlineto diff --git a/tests/data/TestVariableFont-CFF2.ttx b/tests/data/TestVariableFont-CFF2.ttx index c53ebc96..bd4501df 100644 --- a/tests/data/TestVariableFont-CFF2.ttx +++ b/tests/data/TestVariableFont-CFF2.ttx @@ -240,15 +240,15 @@ rlineto - 127 228 70 1 2 blend + 1 vsindex + 127 228 -1 70 -25 1 2 blend rmoveto - 449 -2 -45 -2 2 blend - -5 79 -255 208 -276 -252 148 -279 338 63 -17 84 -27 36 -29 -34 31 -1 2 -45 13 100 10 blend - -280 -54 -82 188 170 153 163 -124 -355 55 -54 -86 -57 -60 57 146 7 blend - 6 rlineto - 167 395 -84 118 2 blend + 449 -2 1 -45 -2 -2 2 blend + -5 79 -255 208 -276 -252 148 -279 338 63 -17 84 -280 -54 -82 188 170 153 163 -124 -355 6 27 0 0 -27 0 36 0 -29 0 -34 0 31 0 -1 0 2 0 -45 -2 13 28 100 37 0 13 0 -2 55 -40 -54 -32 -86 -30 -57 -85 -60 34 57 84 146 -5 0 21 blend + rlineto + 167 395 -5 -84 43 118 2 blend rmoveto - -16 -94 78 -2 9 88 -19 -48 44 7 -4 29 6 blend + -16 -94 78 -2 9 88 -7 -19 -17 -48 16 44 3 7 -1 -4 10 29 6 blend rlineto diff --git a/tests/outlineCompiler_test.py b/tests/outlineCompiler_test.py index 03760e8b..5755ce5d 100644 --- a/tests/outlineCompiler_test.py +++ b/tests/outlineCompiler_test.py @@ -1324,7 +1324,12 @@ def test_custom_layer_compilation_interpolatable_otf_from_ds(designspace, inplac "dotabovecomb", "edotabove", ] - assert master_otfs[1].getGlyphOrder() == [".notdef", "e"] + # 'edotabove' composite glyph needed to be decomposed because these are CFF fonts; + # and because one of its components 'e' has an additional intermediate master, the + # latter 'bubbled up' to the parent glyph when this got decomposed; hence why + # we see 'edotabove' in master_otfs[1] below, but we do not in the previous test + # with interpolatalbe TTFs where 'edotabove' stays a composite glyph. + assert master_otfs[1].getGlyphOrder() == [".notdef", "e", "edotabove"] assert master_otfs[2].getGlyphOrder() == [ ".notdef", "a", From 1717ee9de5c218d837c10880861a7bab732cff27 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 18 Mar 2024 10:55:44 +0000 Subject: [PATCH 20/27] [.gitignore] exclude local venv/ dirs --- .gitignore | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.gitignore b/.gitignore index 44831735..a0c36662 100644 --- a/.gitignore +++ b/.gitignore @@ -31,5 +31,11 @@ htmlcov/ # autosaved emacs files *~ +# virtual environments dirs +venv/ +.venv/ +venv3*/ +.venv3*/ + # autogenerated by setuptools-scm Lib/ufo2ft/_version.py From 3b14826e1d1acf1b76226596e860f6f814860369 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 18 Mar 2024 10:57:39 +0000 Subject: [PATCH 21/27] [tox.ini] tell isort to --gitignore --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index fed40c4a..570b2811 100644 --- a/tox.ini +++ b/tox.ini @@ -22,7 +22,7 @@ deps = -r dev-requirements.txt commands = black --check --diff . - isort --check-only --diff . + isort --gitignore --check-only --diff . flake8 [testenv:htmlcov] From dd59f6dfb365fd1a1e43fe666f7401d6a43338dc Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 18 Mar 2024 15:43:07 +0000 Subject: [PATCH 22/27] raise error (instead of warning) for missing components while decomposing At some point we decided to skip missing components when decomposing composites and just issue a logging warning. But that was not a good idea, as it hides other font bugs (or even code bugs), and it is no longer needed because we now support sparse composite glyphs. We may reintroduce the skip behavior as an option if need be. --- Lib/ufo2ft/util.py | 2 +- tests/filters/decomposeComponents_test.py | 17 ++++++----------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/Lib/ufo2ft/util.py b/Lib/ufo2ft/util.py index b85e144d..118c237a 100644 --- a/Lib/ufo2ft/util.py +++ b/Lib/ufo2ft/util.py @@ -53,7 +53,7 @@ def makeOfficialGlyphOrder(font, glyphOrder=None): def decomposeCompositeGlyph( glyph, glyphSet, - skipMissing=True, + skipMissing=False, reverseFlipped=True, include=None, decomposeNested=True, diff --git a/tests/filters/decomposeComponents_test.py b/tests/filters/decomposeComponents_test.py index 70deaa44..555ebc40 100644 --- a/tests/filters/decomposeComponents_test.py +++ b/tests/filters/decomposeComponents_test.py @@ -1,10 +1,10 @@ -import logging +import pytest +from fontTools.pens.basePen import MissingComponentError from ufo2ft.filters.decomposeComponents import DecomposeComponentsFilter -from ufo2ft.util import logger -def test_missing_component_is_dropped(FontClass, caplog): +def test_missing_component_error(FontClass, caplog): ufo = FontClass() a = ufo.newGlyph("a") a.width = 100 @@ -24,15 +24,10 @@ def test_missing_component_is_dropped(FontClass, caplog): assert len(ufo["aacute"]) == 0 assert len(ufo["aacute"].components) == 2 - with caplog.at_level(logging.WARNING, logger=logger.name): - filter_ = DecomposeComponentsFilter() - - assert filter_(ufo) - assert len(ufo["aacute"]) == 1 - assert len(ufo["aacute"].components) == 0 + filter_ = DecomposeComponentsFilter() - assert len(caplog.records) == 1 - assert "dropping non-existent component" in caplog.text + with pytest.raises(MissingComponentError, match="'acute'"): + filter_(ufo) def test_nested_components(FontClass): From 3aa23aa2aa10d099a0d72795107758a5921fc091 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 19 Mar 2024 14:09:16 +0000 Subject: [PATCH 23/27] [instantiator] simplify default constructor to aid testing and use design coordinates for source locations as well as interpolated layers, to aid debugging --- Lib/ufo2ft/instantiator.py | 132 +++++++++++++++++++++++-------------- 1 file changed, 84 insertions(+), 48 deletions(-) diff --git a/Lib/ufo2ft/instantiator.py b/Lib/ufo2ft/instantiator.py index aa822e8a..067eb1ff 100644 --- a/Lib/ufo2ft/instantiator.py +++ b/Lib/ufo2ft/instantiator.py @@ -34,6 +34,7 @@ import logging import typing from dataclasses import dataclass, field +from functools import cached_property from typing import ( TYPE_CHECKING, Any, @@ -82,6 +83,9 @@ # `{"wght": (100.0, 400.0, 900.0), "wdth": (75.0, 100.0, 100.0)}`. AxisBounds = Dict[str, Tuple[float, float, float]] +# A bunch of glyphs at a given location (in design coordinates) +SourceLayer = Tuple[Location, Dict[str, "Glyph"]] + # For mapping `wdth` axis user values to the OS2 table's width class field. WDTH_VALUE_TO_OS2_WIDTH_CLASS = { 50: 1, @@ -199,22 +203,43 @@ class Instantiator: font instance object at an arbitary location within the design space.""" axis_bounds: AxisBounds # Design space! - copy_feature_text: str - copy_nonkerning_groups: Mapping[str, List[str]] - copy_info: Optional[Info] - copy_lib: Mapping[str, Any] - default_design_location: Location - designspace_rules: List[designspaceLib.RuleDescriptor] - glyph_mutators: Mapping[str, Optional["Variator"]] - glyph_name_to_unicodes: Dict[str, List[int]] - info_mutator: Optional["Variator"] - kerning_mutator: Optional["Variator"] - round_geometry: bool - skip_export_glyphs: List[str] - special_axes: Mapping[str, designspaceLib.AxisDescriptor] - source_layers: List[Tuple[Location, Dict[str, Glyph]]] - default_source_idx: int - glyph_factory: Optional[Callable[[str], Glyph]] # create new ufoLib/defcon Glyph + source_layers: List[SourceLayer] # at least 1 default layer required (can be empty) + copy_feature_text: str = "" + copy_nonkerning_groups: Mapping[str, List[str]] = field(default_factory=dict) + copy_info: Optional[Info] = None + copy_lib: Mapping[str, Any] = field(default_factory=dict) + designspace_rules: List[designspaceLib.RuleDescriptor] = field(default_factory=list) + glyph_mutators: Mapping[str, Optional["Variator"]] = field(default_factory=dict) + info_mutator: Optional["Variator"] = None + kerning_mutator: Optional["Variator"] = None + round_geometry: bool = False + skip_export_glyphs: List[str] = field(default_factory=list) + special_axes: Mapping[str, designspaceLib.AxisDescriptor] = field( + default_factory=dict + ) + # computed attributes (see __post_init__ below) + default_source_idx: int = field(init=False) + default_design_location: Location = field(init=False) + + def __post_init__(self): + default_location = { + axis: default for axis, (_, default, _) in self.axis_bounds.items() + } + default_location_items = default_location.items() + default_source_idx = None + for i, (location, _) in enumerate(self.source_layers): + if location.items() <= default_location_items: + default_source_idx = i + break + else: + raise InstantiatorError( + f"Missing source layer at default location: {default_location}" + ) + assert default_source_idx is not None + object.__setattr__(self, "default_source_idx", default_source_idx) + object.__setattr__( + self, "default_design_location", {**default_location, **location} + ) @classmethod def from_designspace( @@ -291,6 +316,7 @@ def from_designspace( # Construct Variators axis_bounds: AxisBounds = {} # Design space! + default_design_location = {} axis_order: List[str] = [] special_axes = {} for axis in designspace.axes: @@ -300,6 +326,7 @@ def from_designspace( axis.map_forward(axis.default), axis.map_forward(axis.maximum), ) + default_design_location[axis.name] = axis_bounds[axis.name][1] # Some axes relate to existing OpenType fields and get special attention. if axis.tag in {"wght", "wdth", "slnt"}: special_axes[axis.tag] = axis @@ -326,29 +353,20 @@ def from_designspace( # left empty as the glyph sources will be loaded later on-demand glyph_mutators: Dict[str, Variator] = {} - glyph_name_to_unicodes: Dict[str, List[int]] = {} source_layers = [] - default_source_idx = -1 - glyph_factory = None if do_glyphs: - for glyph_name in glyph_names: - glyph_name_to_unicodes[glyph_name] = default_font[glyph_name].unicodes - # collect (normalized location, source layer) tuples - for i, source in enumerate(designspace.sources): + # collect (location, source layer) tuples + for source in designspace.sources: if source.layerName is None: layer = source.font.layers.defaultLayer else: layer = source.font.layers[source.layerName] - if glyph_factory is None and len(layer) > 0: - glyph_factory = _getNewGlyphFactory(next(iter(layer))) - normalized_location = varLib.models.normalizeLocation( - source.location, axis_bounds - ) + source_location = source.location if source is designspace.default: - assert all(v == 0.0 for v in normalized_location.values()) - default_source_idx = i - source_layers.append((normalized_location, {g.name: g for g in layer})) - assert default_source_idx != -1 + # sanity check that the source marked as the default really matches + # the default location (i.e. is a subset of) + assert source_location.items() <= default_design_location.items() + source_layers.append((source_location, {g.name: g for g in layer})) # Construct defaults to copy over copy_feature_text: str = default_font.features.text if do_info else "" @@ -373,24 +391,24 @@ def from_designspace( return cls( axis_bounds, + source_layers, copy_feature_text, copy_nonkerning_groups, copy_info, copy_lib, - designspace.default.location, designspace.rules, glyph_mutators, - glyph_name_to_unicodes, info_mutator, kerning_mutator, round_geometry, skip_export_glyphs, special_axes, - source_layers, - default_source_idx, - glyph_factory, ) + @property + def axis_order(self): + return list(self.axis_bounds.keys()) + @property def default_source_glyphs(self) -> Dict[str, Glyph]: return self.source_layers[self.default_source_idx][1] @@ -401,7 +419,9 @@ def glyph_names(self) -> KeysView[str]: @property def source_locations(self) -> Iterable[Location]: - return (loc for loc, _ in self.source_layers) + return ( + {**self.default_design_location, **loc} for loc, _ in self.source_layers + ) def normalize(self, location: Location) -> Location: return varLib.models.normalizeLocation(location, self.axis_bounds) @@ -493,8 +513,16 @@ def generate_instance(self, instance: designspaceLib.InstanceDescriptor) -> Font return font + @cached_property + def glyph_factory(self) -> Callable[[str], Glyph]: + glyphs = self.default_source_glyphs + if len(glyphs) > 0: + glyph_factory = _getNewGlyphFactory(next(iter(glyphs.values()))) + else: + raise InstantiatorError("Default source has no glyphs, can't make new ones") + return glyph_factory + def new_glyph(self, name: str) -> Glyph: - assert self.glyph_factory is not None return self.glyph_factory(name=name) def generate_glyph_instance( @@ -505,7 +533,7 @@ def generate_glyph_instance( ) -> Glyph: """Generate an instance of a single glyph at the given location. - The location must be pecified using normalized coordinates. + The location must be specified using normalized coordinates. If output_glyph is None, the instance is generated in a new Glyph object and returned. Otherwise, the instance is extracted to the given Glyph object. """ @@ -519,7 +547,7 @@ def generate_glyph_instance( ) try: glyph_mutator = self.glyph_mutators[glyph_name] = Variator.from_masters( - sources, axis_order=list(self.axis_bounds) + sources, self.axis_order ) except varLib.errors.VarLibError as e: raise InstantiatorError( @@ -535,9 +563,9 @@ def generate_glyph_instance( output_glyph = self.new_glyph(glyph_name) # onlyGeometry=True does not set name and unicodes, in ufoLib2 we can't - # modify a glyph's name. Copy unicodes from default font. + # modify a glyph's name. Copy unicodes from default layer. glyph_instance.extractGlyph(output_glyph, onlyGeometry=True) - output_glyph.unicodes = list(self.glyph_name_to_unicodes[glyph_name]) + output_glyph.unicodes = list(self.default_source_glyphs[glyph_name].unicodes) return output_glyph @@ -611,8 +639,9 @@ def _generate_instance_info( @property def interpolated_layers(self) -> list[InterpolatedLayer]: """Return one InterpolatedLayer for each source location.""" + default = self.default_design_location return [ - InterpolatedLayer(self, loc, source_layer) + InterpolatedLayer(self, {**default, **loc}, source_layer) for loc, source_layer in self.source_layers ] @@ -719,7 +748,7 @@ def collect_kerning_masters( def collect_glyph_masters( - source_layers: List[Tuple[Location, Dict[str, Glyph]]], + source_layers: List[SourceLayer], glyph_name: str, axis_bounds: AxisBounds, default_source_idx: int, @@ -735,7 +764,7 @@ def collect_glyph_masters( default_glyph_empty = False other_glyph_empty = False - for i, (normalized_location, source_layer) in enumerate(source_layers): + for i, (location, source_layer) in enumerate(source_layers): this_is_default = i == default_source_idx if glyph_name not in source_layer: if this_is_default: @@ -755,6 +784,7 @@ def collect_glyph_masters( else: other_glyph_empty = True + normalized_location = varLib.models.normalizeLocation(location, axis_bounds) locations_and_masters.append( (normalized_location, fontMath.MathGlyph(source_glyph, strict=True)) ) @@ -951,12 +981,16 @@ class InterpolatedLayer(Mapping): """ instantiator: Instantiator - # axis coordinates for this layer (already normalized!) + # axis coordinates for this layer in design space location: Location # source ufoLib2/defcon Layer (None if location isn't among the source locations) source_layer: dict[str, Glyph] | None = None _cache: dict[str, Glyph] = field(default_factory=dict) + @cached_property + def normalized_location(self): + return self.instantiator.normalize(self.location) + def __iter__(self) -> Iterable[str]: return iter(self.instantiator.glyph_names) @@ -984,4 +1018,6 @@ def _get(self, glyph_name: str) -> Glyph | None: return glyph def _interpolate(self, glyph_name: str) -> Glyph: - return self.instantiator.generate_glyph_instance(glyph_name, self.location) + return self.instantiator.generate_glyph_instance( + glyph_name, self.normalized_location + ) From 2319e0a46a79ee41de4efdb6c3554649c727b8d0 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 20 Mar 2024 13:15:05 +0000 Subject: [PATCH 24/27] [decomposeComponents_test] test interpolatable decomposeComponents filter --- tests/filters/decomposeComponents_test.py | 260 +++++++++++++++++++++- 1 file changed, 259 insertions(+), 1 deletion(-) diff --git a/tests/filters/decomposeComponents_test.py b/tests/filters/decomposeComponents_test.py index 555ebc40..1d0bbce1 100644 --- a/tests/filters/decomposeComponents_test.py +++ b/tests/filters/decomposeComponents_test.py @@ -1,7 +1,12 @@ import pytest from fontTools.pens.basePen import MissingComponentError -from ufo2ft.filters.decomposeComponents import DecomposeComponentsFilter +from ufo2ft.filters.decomposeComponents import ( + DecomposeComponentsFilter, + DecomposeComponentsIFilter, +) +from ufo2ft.instantiator import Instantiator +from ufo2ft.util import _GlyphSet def test_missing_component_error(FontClass, caplog): @@ -60,3 +65,256 @@ def test_nested_components(FontClass): assert not ufo["nine.lf"].components assert len(ufo["nine"]) == 1 assert not ufo["nine"].components + + +@pytest.fixture +def ufos_and_glyphSets(FontClass): + """Return two parallel lists of UFOs and glyphSets for testing. + + This fixture creates two UFOs, a Regular and a Bold, each containing 5 glyphs: + "agrave" composite glyph composed from "a" and "gravecomb" components, + both in turn simple contour glyphs, and "igrave" composed of "dotlessi" and + "gravecomb" components. + The Regular UFO also contains a 'sparse' Medium layer with only two glyphs: + a different "agrave" composite glyph, but no "a" nor "gravecomb" components; + and a different shape for the "dotlessi" glyph, but no "igrave" nor "gravecomb". + + The decomposing (interpolatable) filter should interpolate the missing + components or composites on-the-fly using the instantiator, when available. + """ + + regular_ufo = FontClass() + + a = regular_ufo.newGlyph("a") + a.width = 500 + a.unicodes = [ord("a")] + pen = a.getPointPen() + pen.beginPath() + pen.addPoint((100, 0), "line") + pen.addPoint((400, 0), "line") + pen.addPoint((400, 500), "line") + pen.addPoint((100, 500), "line") + pen.endPath() + + i = regular_ufo.newGlyph("dotlessi") + i.width = 300 + i.unicodes = [0x0131] + pen = i.getPointPen() + pen.beginPath() + pen.addPoint((100, 0), "line") + pen.addPoint((200, 0), "line") + pen.addPoint((200, 500), "line") + pen.addPoint((100, 500), "line") + pen.endPath() + + gravecomb = regular_ufo.newGlyph("gravecomb") + gravecomb.unicodes = [0x0300] + pen = gravecomb.getPointPen() + pen.beginPath() + pen.addPoint((30, 550), "line") + pen.addPoint((0, 750), "line") + pen.addPoint((-50, 750), "line") + pen.addPoint((0, 550), "line") + pen.endPath() + + agrave = regular_ufo.newGlyph("agrave") + agrave.width = a.width + agrave.unicodes = [0x00E0] + pen = agrave.getPointPen() + pen.addComponent("a", (1, 0, 0, 1, 0, 0)) + pen.addComponent("gravecomb", (1, 0, 0, 1, 250, 0)) + + igrave = regular_ufo.newGlyph("igrave") + igrave.width = i.width + igrave.unicodes = [0x00EC] + pen = igrave.getPointPen() + pen.addComponent("dotlessi", (1, 0, 0, 1, 0, 0)) + pen.addComponent("gravecomb", (1, 0, 0, 1, 150, 0)) + + # The Medium layer has "agrave" but does not have "a" and "gravecomb" + medium_layer = regular_ufo.newLayer("Medium") + + agrave = medium_layer.newGlyph("agrave") + agrave.width = 550 + pen = agrave.getPointPen() + pen.addComponent("a", (1, 0, 0, 1, 0, 0)) + pen.addComponent("gravecomb", (1, 0, 0, 1, 275, 0)) + + # The Medium layer also has a different "dotlessi" glyph, which the + # other layers don't have. + i = medium_layer.newGlyph("dotlessi") + i.width = 350 + pen = i.getPointPen() + pen.beginPath() + pen.addPoint((100, 0), "line") + pen.addPoint((250, 0), "line") + pen.addPoint((175, 500), "line") + pen.addPoint((175, 500), "line") + pen.endPath() + + bold_ufo = FontClass() + + a = bold_ufo.newGlyph("a") + a.width = 600 + pen = a.getPointPen() + pen.beginPath() + pen.addPoint((150, 0), "line") + pen.addPoint((450, 0), "line") + pen.addPoint((450, 500), "line") + pen.addPoint((150, 500), "line") + pen.endPath() + + i = bold_ufo.newGlyph("dotlessi") + i.width = 400 + pen = i.getPointPen() + pen.beginPath() + pen.addPoint((100, 0), "line") + pen.addPoint((300, 0), "line") + pen.addPoint((300, 500), "line") + pen.addPoint((100, 500), "line") + pen.endPath() + + gravecomb = bold_ufo.newGlyph("gravecomb") + pen = gravecomb.getPointPen() + pen.beginPath() + pen.addPoint((40, 550), "line") + pen.addPoint((0, 750), "line") + pen.addPoint((-70, 750), "line") + pen.addPoint((0, 550), "line") + pen.endPath() + + agrave = bold_ufo.newGlyph("agrave") + agrave.width = a.width + pen = agrave.getPointPen() + pen.addComponent("a", (1, 0, 0, 1, 0, 0)) + pen.addComponent("gravecomb", (1, 0, 0, 1, 300, 0)) + + igrave = bold_ufo.newGlyph("igrave") + igrave.width = i.width + pen = igrave.getPointPen() + pen.addComponent("dotlessi", (1, 0, 0, 1, 0, 0)) + pen.addComponent("gravecomb", (1, 0, 0, 1, 200, 0)) + + ufos = [regular_ufo, regular_ufo, bold_ufo] + glyphSets = [ + _GlyphSet.from_layer(regular_ufo), + _GlyphSet.from_layer(regular_ufo, layerName="Medium"), + _GlyphSet.from_layer(bold_ufo), + ] + return ufos, glyphSets + + +class DecomposeComponentsIFilterTest: + def test_composite_with_intermediate_master(self, ufos_and_glyphSets): + ufos, glyphSets = ufos_and_glyphSets + regular_glyphs, medium_glyphs, bold_glyphs = glyphSets + assert "agrave" in medium_glyphs + assert {"a", "gravecomb"}.isdisjoint(medium_glyphs) + + instantiator = Instantiator( + {"Weight": (100, 100, 200)}, + [ + ({"Weight": 100}, regular_glyphs), + ({"Weight": 150}, medium_glyphs), + ({"Weight": 200}, bold_glyphs), + ], + ) + filter_ = DecomposeComponentsIFilter(include={"agrave"}) + + modified = filter_(ufos, glyphSets, instantiator=instantiator) + + assert modified == {"agrave"} + + agrave = regular_glyphs["agrave"] + assert len(agrave.components) == 0 + assert [[(p.x, p.y) for p in c] for c in agrave] == [ + [(100, 0), (400, 0), (400, 500), (100, 500)], + [(280, 550), (250, 750), (200, 750), (250, 550)], + ] + + # 'agrave' was fully decomposed also in the medium layer, despite the + # latter not containing sources for the "a" and "gravecomb" component glyphs. + # These were interpolated on-the-fly while decomposing the composite glyph. + agrave = medium_glyphs["agrave"] + assert len(agrave.components) == 0 + assert [[(p.x, p.y) for p in c] for c in agrave] == [ + [(125, 0), (425, 0), (425, 500), (125, 500)], + [(310, 550), (275, 750), (215, 750), (275, 550)], + ] + + agrave = bold_glyphs["agrave"] + assert len(agrave.components) == 0 + assert [[(p.x, p.y) for p in c] for c in agrave] == [ + [(150, 0), (450, 0), (450, 500), (150, 500)], + [(340, 550), (300, 750), (230, 750), (300, 550)], + ] + + def test_component_with_intermediate_master(self, ufos_and_glyphSets): + ufos, glyphSets = ufos_and_glyphSets + regular_glyphs, medium_glyphs, bold_glyphs = glyphSets + assert {"dotlessi", "gravecomb", "igrave"}.issubset(regular_glyphs) + assert {"dotlessi", "gravecomb", "igrave"}.issubset(bold_glyphs) + assert "dotlessi" in medium_glyphs + assert {"igrave", "gravecomb"}.isdisjoint(medium_glyphs) + + instantiator = Instantiator( + {"Weight": (100, 100, 200)}, + [ + ({"Weight": 100}, regular_glyphs), + ({"Weight": 150}, medium_glyphs), + ({"Weight": 200}, bold_glyphs), + ], + ) + filter_ = DecomposeComponentsIFilter(include={"igrave"}) + + modified = filter_(ufos, glyphSets, instantiator=instantiator) + + assert modified == {"igrave"} + + igrave = regular_glyphs["igrave"] + assert len(igrave.components) == 0 + assert [[(p.x, p.y) for p in c] for c in igrave] == [ + [(100, 0), (200, 0), (200, 500), (100, 500)], + [(180, 550), (150, 750), (100, 750), (150, 550)], + ] + + # 'igrave' was also decomposed in the Medium layer, despite it was not + # originally present; it was added by the filter and interpolated on-the-fly, + # because Medium contained a different 'dotlessi' used as a component. + igrave = medium_glyphs["igrave"] + assert len(igrave.components) == 0 + assert [[(p.x, p.y) for p in c] for c in igrave] == [ + [(100, 0), (250, 0), (175, 500), (175, 500)], + [(210, 550), (175, 750), (115, 750), (175, 550)], + ] + assert {"dotlessi", "igrave"}.issubset(medium_glyphs) + assert "gravecomb" not in medium_glyphs + + igrave = bold_glyphs["igrave"] + assert len(igrave.components) == 0 + assert [[(p.x, p.y) for p in c] for c in igrave] == [ + [(100, 0), (300, 0), (300, 500), (100, 500)], + [(240, 550), (200, 750), (130, 750), (200, 550)], + ] + + def test_without_instantiator(self, ufos_and_glyphSets): + # without an instantiator (i.e. when the filter is run from the legacy + # `compileInterpolatableTTFs` without a designspace as input but only a buch + # of UFOs), the filter will raise a MissingComponentError while + # trying to decompose 'agrave', because it can't interpolate the missing + # components 'a' and 'gravecomb' + ufos, glyphSets = ufos_and_glyphSets + medium_glyphs = glyphSets[1] + assert {"agrave", "dotlessi"}.issubset(medium_glyphs) + assert {"a", "gravecomb", "igrave"}.isdisjoint(medium_glyphs) + + with pytest.raises(MissingComponentError, match="'a'"): + DecomposeComponentsIFilter(include={"agrave"})(ufos, glyphSets) + + # the filter will not fail to decompose 'igrave' in Regular or Bold, however the + # Medium master will not contain decomposed outlines for 'igrave', and + # in the VF produced from these masters the 'igrave' will appear different + # at runtime from 'dotlessi' when the Medium instance is selected. + modified = DecomposeComponentsIFilter(include={"igrave"})(ufos, glyphSets) + assert modified == {"igrave"} + assert "igrave" not in medium_glyphs From c9213182b00b59781af02c37c146e044069b39d7 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 20 Mar 2024 17:36:05 +0000 Subject: [PATCH 25/27] [integration_test] test flattenComponents=True when compiling VF from DS this will exercise the interpolatable FlattenComponentsIFilter --- .../glyphs.M_edium/a.glif | 13 +++++++++++ .../glyphs.M_edium/contents.plist | 10 ++++++++ .../glyphs.M_edium/e.glif | 9 ++++++++ .../layercontents.plist | 4 ++++ tests/data/NestedComponents.designspace | 23 +++++++++++++++++++ tests/integration_test.py | 8 +++++++ 6 files changed, 67 insertions(+) create mode 100644 tests/data/NestedComponents-Regular.ufo/glyphs.M_edium/a.glif create mode 100644 tests/data/NestedComponents-Regular.ufo/glyphs.M_edium/contents.plist create mode 100644 tests/data/NestedComponents-Regular.ufo/glyphs.M_edium/e.glif create mode 100644 tests/data/NestedComponents.designspace diff --git a/tests/data/NestedComponents-Regular.ufo/glyphs.M_edium/a.glif b/tests/data/NestedComponents-Regular.ufo/glyphs.M_edium/a.glif new file mode 100644 index 00000000..5b9c9721 --- /dev/null +++ b/tests/data/NestedComponents-Regular.ufo/glyphs.M_edium/a.glif @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/tests/data/NestedComponents-Regular.ufo/glyphs.M_edium/contents.plist b/tests/data/NestedComponents-Regular.ufo/glyphs.M_edium/contents.plist new file mode 100644 index 00000000..7ef307d1 --- /dev/null +++ b/tests/data/NestedComponents-Regular.ufo/glyphs.M_edium/contents.plist @@ -0,0 +1,10 @@ + + + + + a + a.glif + e + e.glif + + diff --git a/tests/data/NestedComponents-Regular.ufo/glyphs.M_edium/e.glif b/tests/data/NestedComponents-Regular.ufo/glyphs.M_edium/e.glif new file mode 100644 index 00000000..ce115a7b --- /dev/null +++ b/tests/data/NestedComponents-Regular.ufo/glyphs.M_edium/e.glif @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/tests/data/NestedComponents-Regular.ufo/layercontents.plist b/tests/data/NestedComponents-Regular.ufo/layercontents.plist index cf95d357..97841021 100644 --- a/tests/data/NestedComponents-Regular.ufo/layercontents.plist +++ b/tests/data/NestedComponents-Regular.ufo/layercontents.plist @@ -6,5 +6,9 @@ public.default glyphs + + Medium + glyphs.M_edium + diff --git a/tests/data/NestedComponents.designspace b/tests/data/NestedComponents.designspace new file mode 100644 index 00000000..92ea6e26 --- /dev/null +++ b/tests/data/NestedComponents.designspace @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/integration_test.py b/tests/integration_test.py index 41bef319..33ea57d2 100644 --- a/tests/integration_test.py +++ b/tests/integration_test.py @@ -165,6 +165,14 @@ def test_nestedComponents_interpolatable(self, FontClass): for ttf in ttfs: assert ttf["maxp"].maxComponentDepth == 1 + def test_nestedComponents_variable(self, FontClass): + designspace = DesignSpaceDocument.fromfile( + getpath("NestedComponents.designspace") + ) + designspace.loadSourceFonts(FontClass) + vf = compileVariableTTF(designspace, flattenComponents=True) + assert vf["maxp"].maxComponentDepth == 1 + def test_interpolatableTTFs_lazy(self, FontClass): # two same UFOs **must** be interpolatable ufos = [FontClass(getpath("TestFont.ufo")) for _ in range(2)] From 6b939bb703b4e04631f033765d1fcd0f8ef5fbf2 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 22 Mar 2024 12:52:00 +0000 Subject: [PATCH 26/27] [preProcessor_test] Add test for SkipExportGlyphsIFilter --- .../fontinfo.plist | 20 ++++++ .../glyphs/A_.glif | 36 ++++++++++ .../glyphs/A_stroke.glif | 13 ++++ .../glyphs/_stroke.glif | 12 ++++ .../glyphs/contents.plist | 12 ++++ .../layercontents.plist | 10 +++ .../SkipExportGlyphsTest-Bold.ufo/lib.plist | 17 +++++ .../metainfo.plist | 10 +++ .../fontinfo.plist | 20 ++++++ .../glyphs.{151}/_stroke.glif | 12 ++++ .../glyphs.{151}/contents.plist | 8 +++ .../glyphs.{170}/A_stroke.glif | 11 +++ .../glyphs.{170}/contents.plist | 8 +++ .../glyphs/A_.glif | 36 ++++++++++ .../glyphs/A_stroke.glif | 13 ++++ .../glyphs/_stroke.glif | 12 ++++ .../glyphs/contents.plist | 12 ++++ .../layercontents.plist | 18 +++++ .../lib.plist | 17 +++++ .../metainfo.plist | 10 +++ tests/data/SkipExportGlyphsTest.designspace | 44 ++++++++++++ tests/preProcessor_test.py | 72 +++++++++++++++++++ 22 files changed, 423 insertions(+) create mode 100644 tests/data/SkipExportGlyphsTest-Bold.ufo/fontinfo.plist create mode 100644 tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/A_.glif create mode 100644 tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/A_stroke.glif create mode 100644 tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/_stroke.glif create mode 100644 tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/contents.plist create mode 100644 tests/data/SkipExportGlyphsTest-Bold.ufo/layercontents.plist create mode 100644 tests/data/SkipExportGlyphsTest-Bold.ufo/lib.plist create mode 100644 tests/data/SkipExportGlyphsTest-Bold.ufo/metainfo.plist create mode 100644 tests/data/SkipExportGlyphsTest-Regular.ufo/fontinfo.plist create mode 100644 tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{151}/_stroke.glif create mode 100644 tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{151}/contents.plist create mode 100644 tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{170}/A_stroke.glif create mode 100644 tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{170}/contents.plist create mode 100644 tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/A_.glif create mode 100644 tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/A_stroke.glif create mode 100644 tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/_stroke.glif create mode 100644 tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/contents.plist create mode 100644 tests/data/SkipExportGlyphsTest-Regular.ufo/layercontents.plist create mode 100644 tests/data/SkipExportGlyphsTest-Regular.ufo/lib.plist create mode 100644 tests/data/SkipExportGlyphsTest-Regular.ufo/metainfo.plist create mode 100644 tests/data/SkipExportGlyphsTest.designspace diff --git a/tests/data/SkipExportGlyphsTest-Bold.ufo/fontinfo.plist b/tests/data/SkipExportGlyphsTest-Bold.ufo/fontinfo.plist new file mode 100644 index 00000000..82d703a6 --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Bold.ufo/fontinfo.plist @@ -0,0 +1,20 @@ + + + + + ascender + 760 + capHeight + 714 + descender + -240 + familyName + SkipExportGlyphsTest + styleName + Bold + unitsPerEm + 1000 + xHeight + 553 + + diff --git a/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/A_.glif b/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/A_.glif new file mode 100644 index 00000000..50c75105 --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/A_.glif @@ -0,0 +1,36 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/A_stroke.glif b/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/A_stroke.glif new file mode 100644 index 00000000..450aa8c6 --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/A_stroke.glif @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/_stroke.glif b/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/_stroke.glif new file mode 100644 index 00000000..7192f585 --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/_stroke.glif @@ -0,0 +1,12 @@ + + + + + + + + + + + + diff --git a/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/contents.plist b/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/contents.plist new file mode 100644 index 00000000..ecef523a --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/contents.plist @@ -0,0 +1,12 @@ + + + + + A + A_.glif + Astroke + A_stroke.glif + _stroke + _stroke.glif + + diff --git a/tests/data/SkipExportGlyphsTest-Bold.ufo/layercontents.plist b/tests/data/SkipExportGlyphsTest-Bold.ufo/layercontents.plist new file mode 100644 index 00000000..b9c1a4f2 --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Bold.ufo/layercontents.plist @@ -0,0 +1,10 @@ + + + + + + public.default + glyphs + + + diff --git a/tests/data/SkipExportGlyphsTest-Bold.ufo/lib.plist b/tests/data/SkipExportGlyphsTest-Bold.ufo/lib.plist new file mode 100644 index 00000000..fa8de64a --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Bold.ufo/lib.plist @@ -0,0 +1,17 @@ + + + + + public.glyphOrder + + A + Astroke + _stroke + + public.postscriptNames + + Astroke + uni023A + + + diff --git a/tests/data/SkipExportGlyphsTest-Bold.ufo/metainfo.plist b/tests/data/SkipExportGlyphsTest-Bold.ufo/metainfo.plist new file mode 100644 index 00000000..7b8b34ac --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Bold.ufo/metainfo.plist @@ -0,0 +1,10 @@ + + + + + creator + com.github.fonttools.ufoLib + formatVersion + 3 + + diff --git a/tests/data/SkipExportGlyphsTest-Regular.ufo/fontinfo.plist b/tests/data/SkipExportGlyphsTest-Regular.ufo/fontinfo.plist new file mode 100644 index 00000000..c973724e --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Regular.ufo/fontinfo.plist @@ -0,0 +1,20 @@ + + + + + ascender + 760 + capHeight + 714 + descender + -240 + familyName + SkipExportGlyphsTest + styleName + Regular + unitsPerEm + 1000 + xHeight + 536 + + diff --git a/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{151}/_stroke.glif b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{151}/_stroke.glif new file mode 100644 index 00000000..e8588c9f --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{151}/_stroke.glif @@ -0,0 +1,12 @@ + + + + + + + + + + + + diff --git a/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{151}/contents.plist b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{151}/contents.plist new file mode 100644 index 00000000..b6d78546 --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{151}/contents.plist @@ -0,0 +1,8 @@ + + + + + _stroke + _stroke.glif + + diff --git a/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{170}/A_stroke.glif b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{170}/A_stroke.glif new file mode 100644 index 00000000..b2879953 --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{170}/A_stroke.glif @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{170}/contents.plist b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{170}/contents.plist new file mode 100644 index 00000000..387ea4af --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{170}/contents.plist @@ -0,0 +1,8 @@ + + + + + Astroke + A_stroke.glif + + diff --git a/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/A_.glif b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/A_.glif new file mode 100644 index 00000000..9735a33c --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/A_.glif @@ -0,0 +1,36 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/A_stroke.glif b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/A_stroke.glif new file mode 100644 index 00000000..605da109 --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/A_stroke.glif @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/_stroke.glif b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/_stroke.glif new file mode 100644 index 00000000..24bcb3d1 --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/_stroke.glif @@ -0,0 +1,12 @@ + + + + + + + + + + + + diff --git a/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/contents.plist b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/contents.plist new file mode 100644 index 00000000..ecef523a --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/contents.plist @@ -0,0 +1,12 @@ + + + + + A + A_.glif + Astroke + A_stroke.glif + _stroke + _stroke.glif + + diff --git a/tests/data/SkipExportGlyphsTest-Regular.ufo/layercontents.plist b/tests/data/SkipExportGlyphsTest-Regular.ufo/layercontents.plist new file mode 100644 index 00000000..c10175d2 --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Regular.ufo/layercontents.plist @@ -0,0 +1,18 @@ + + + + + + public.default + glyphs + + + {170} + glyphs.{170} + + + {151} + glyphs.{151} + + + diff --git a/tests/data/SkipExportGlyphsTest-Regular.ufo/lib.plist b/tests/data/SkipExportGlyphsTest-Regular.ufo/lib.plist new file mode 100644 index 00000000..fa8de64a --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Regular.ufo/lib.plist @@ -0,0 +1,17 @@ + + + + + public.glyphOrder + + A + Astroke + _stroke + + public.postscriptNames + + Astroke + uni023A + + + diff --git a/tests/data/SkipExportGlyphsTest-Regular.ufo/metainfo.plist b/tests/data/SkipExportGlyphsTest-Regular.ufo/metainfo.plist new file mode 100644 index 00000000..7b8b34ac --- /dev/null +++ b/tests/data/SkipExportGlyphsTest-Regular.ufo/metainfo.plist @@ -0,0 +1,10 @@ + + + + + creator + com.github.fonttools.ufoLib + formatVersion + 3 + + diff --git a/tests/data/SkipExportGlyphsTest.designspace b/tests/data/SkipExportGlyphsTest.designspace new file mode 100644 index 00000000..ca60eaf1 --- /dev/null +++ b/tests/data/SkipExportGlyphsTest.designspace @@ -0,0 +1,44 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + public.skipExportGlyphs + + _stroke + + + + diff --git a/tests/preProcessor_test.py b/tests/preProcessor_test.py index 8d1ce8a4..a58c3798 100644 --- a/tests/preProcessor_test.py +++ b/tests/preProcessor_test.py @@ -374,6 +374,78 @@ def test_skip_export_glyphs_designspace(self, FontClass): assert not hasattr(glyphs["e"], "components") assert glyphs["f"].isComposite() + def test_skip_export_glyphs_designspace_variable(self, FontClass): + # The designspace has a public.skipExportGlyphs lib key excluding "_stroke"; + # there are four sources, a Regular, Medium, Semibold and Bold; there is a + # composite glyph "Astroke" that is composed of "A" and "_stroke". + designspace = designspaceLib.DesignSpaceDocument.fromfile( + getpath("SkipExportGlyphsTest.designspace") + ) + designspace.loadSourceFonts(FontClass) + + vf = ufo2ft.compileVariableTTF(designspace, useProductionNames=False) + + # We expect that "_stroke" glyph is not exported and "Astroke" is decomposed to + # simple contour glyph. + glyf = vf["glyf"] + assert "_stroke" not in glyf + assert "Astroke" in glyf + + Astroke = glyf["Astroke"] + assert not Astroke.isComposite() + assert Astroke.numberOfContours == 3 + + # 'Astroke' composite glyph should have 3 delta sets in gvar: two (corresponding + # to the Semibold and Bold masters) were already in the sources, and a third one + # was added by the preprocessor for the Medium master, because "_stroke" was + # present in the Medium and marked as non-export, to be decomposed. + gvar = vf["gvar"] + assert len(gvar.variations["Astroke"]) == 3 + + # Now we add a new composite glyph "_stroke.alt" to the full Regular and Bold + # sources and replace reference to the "_stroke" component in the "Astroke" + # with the new "_stroke.alt". So "Astroke" has now a component which in turn + # references another component (i.e. nested components). We also set the + # public.skipExportGlyphs to exclude "_stroke.alt" from the export, but not + # "_stroke" anymore, which should now be exported. + designspace.lib["public.skipExportGlyphs"] = ["_stroke.alt"] + num_Astroke_sources = 0 + for source in designspace.sources: + if source.layerName is None: + layer = source.font.layers.defaultLayer + stroke_alt = layer.newGlyph("_stroke.alt") + stroke_alt.getPen().addComponent("_stroke", (1, 0, 0, 1, 0, -100)) + else: + layer = source.font.layers[source.layerName] + if "Astroke" in layer: + Astroke = layer["Astroke"] + for component in Astroke.components: + if component.baseGlyph == "_stroke": + component.baseGlyph = "_stroke.alt" + num_Astroke_sources += 1 + + vf = ufo2ft.compileVariableTTF(designspace, useProductionNames=False) + + # we expect that "_stroke.alt" glyph is not exported and the reference to it + # in "Astroke" is replaced with "_stroke" with the offset adjusted. "Astroke" + # itself should NOT be decomposed to simple glyph. + glyf = vf["glyf"] + assert "_stroke.alt" not in glyf + assert "_stroke" in glyf + assert "Astroke" in glyf + + Astroke = glyf["Astroke"] + assert Astroke.isComposite() + assert [c.glyphName for c in Astroke.components] == ["A", "_stroke"] + stroke_comp = Astroke.components[1] + assert (stroke_comp.x, stroke_comp.y) == (0, -100) + + # 'Astroke' composite glyph should have 2 delta sets in gvar: i.e. one for each + # of the non-default masters it was originally present in. No additional + # master should be added by the preprocessor in this case. + gvar = vf["gvar"] + assert len(gvar.variations["Astroke"]) == num_Astroke_sources - 1 + def test_skip_export_glyphs_multi_ufo(self, FontClass): # Bold has a public.skipExportGlyphs lib key excluding "b", "d" and "f". ufo1 = FontClass(getpath("IncompatibleMasters/NewFont-Regular.ufo")) From f200412936fe65d006196b9f8efc1447626b68b1 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 25 Mar 2024 16:03:55 +0000 Subject: [PATCH 27/27] [filters] Add interpolatable PropagateAnchorsIFilter --- Lib/ufo2ft/filters/__init__.py | 3 +- Lib/ufo2ft/filters/base.py | 7 +++ Lib/ufo2ft/filters/propagateAnchors.py | 37 +++++++++++- Lib/ufo2ft/util.py | 6 +- .../glyphs/A_stroke.glif | 4 -- .../glyphs.{170}/A_stroke.glif | 2 - .../glyphs/A_stroke.glif | 4 -- tests/filters/propagateAnchors_test.py | 56 ++++++++++++++++++- 8 files changed, 104 insertions(+), 15 deletions(-) diff --git a/Lib/ufo2ft/filters/__init__.py b/Lib/ufo2ft/filters/__init__.py index 5b175f84..e6921322 100644 --- a/Lib/ufo2ft/filters/__init__.py +++ b/Lib/ufo2ft/filters/__init__.py @@ -15,7 +15,7 @@ from .dottedCircle import DottedCircleFilter from .explodeColorLayerGlyphs import ExplodeColorLayerGlyphsFilter from .flattenComponents import FlattenComponentsFilter, FlattenComponentsIFilter -from .propagateAnchors import PropagateAnchorsFilter +from .propagateAnchors import PropagateAnchorsFilter, PropagateAnchorsIFilter from .removeOverlaps import RemoveOverlapsFilter from .reverseContourDirection import ReverseContourDirectionFilter from .skipExportGlyphs import SkipExportGlyphsFilter, SkipExportGlyphsIFilter @@ -35,6 +35,7 @@ "FlattenComponentsFilter", "FlattenComponentsIFilter", "PropagateAnchorsFilter", + "PropagateAnchorsIFilter", "RemoveOverlapsFilter", "ReverseContourDirectionFilter", "SkipExportGlyphsFilter", diff --git a/Lib/ufo2ft/filters/base.py b/Lib/ufo2ft/filters/base.py index ffc9292e..e286b6ce 100644 --- a/Lib/ufo2ft/filters/base.py +++ b/Lib/ufo2ft/filters/base.py @@ -376,6 +376,13 @@ def getInterpolatableFilterClass(cls) -> "BaseIFilter" | None: """Return the same class as self.""" return cls # no-op + def getDefaultFont(self) -> Font: + if self.context.instantiator is not None: + return self.context.fonts[self.context.instantiator.default_source_idx] + else: + # as good a guess as any... + return self.context.fonts[0] + def getDefaultGlyphSet(self) -> dict[str, Glyph]: """Return the current glyphSet corresponding to the default location.""" if self.context.instantiator is not None: diff --git a/Lib/ufo2ft/filters/propagateAnchors.py b/Lib/ufo2ft/filters/propagateAnchors.py index a4147102..cc96c378 100644 --- a/Lib/ufo2ft/filters/propagateAnchors.py +++ b/Lib/ufo2ft/filters/propagateAnchors.py @@ -17,8 +17,8 @@ import fontTools.pens.boundsPen from fontTools.misc.transform import Transform -from ufo2ft.filters import BaseFilter -from ufo2ft.util import OpenTypeCategories +from ufo2ft.filters import BaseFilter, BaseIFilter +from ufo2ft.util import OpenTypeCategories, zip_strict logger = logging.getLogger(__name__) @@ -50,6 +50,39 @@ def filter(self, glyph): return len(glyph.anchors) > before +class PropagateAnchorsIFilter(BaseIFilter): + def set_context(self, *args, **kwargs): + ctx = super().set_context(*args, **kwargs) + ctx.processed = [set() for _ in range(len(ctx.glyphSets))] + ctx.categories = OpenTypeCategories.load(self.getDefaultFont()) + return ctx + + def __call__(self, fonts, glyphSets=None, instantiator=None, **kwargs): + modified = super().__call__(fonts, glyphSets, instantiator, **kwargs) + if modified: + logger.info("Glyphs with propagated anchors: %i" % len(modified)) + return modified + + def filter(self, glyphName, glyphs): + modified = False + if not any(glyph.components for glyph in glyphs): + return modified + before = len(self.context.modified) + for i, (glyphSet, interpolatedLayer) in enumerate( + zip_strict(self.context.glyphSets, self.getInterpolatedLayers()) + ): + glyph = glyphSet.get(glyphName) + if glyph is not None: + _propagate_glyph_anchors( + interpolatedLayer or glyphSet, + glyph, + self.context.processed[i], + self.context.modified, + self.context.categories, + ) + return len(self.context.modified) > before + + def _propagate_glyph_anchors(glyphSet, composite, processed, modified, categories): """ Propagate anchors from base glyphs to a given composite diff --git a/Lib/ufo2ft/util.py b/Lib/ufo2ft/util.py index 118c237a..cc9241c2 100644 --- a/Lib/ufo2ft/util.py +++ b/Lib/ufo2ft/util.py @@ -760,7 +760,11 @@ def load(cls, font): openTypeCategories = font.lib.get(OPENTYPE_CATEGORIES_KEY, {}) # Handle case where we are a variable feature writer if not openTypeCategories and isinstance(font, DesignSpaceDocument): - font = font.sources[0].font + designspace = font + default = designspace.findDefault() + if default is None: + raise InvalidDesignSpaceData("No default source found in designspace") + font = default.font openTypeCategories = font.lib.get(OPENTYPE_CATEGORIES_KEY, {}) for glyphName, category in openTypeCategories.items(): diff --git a/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/A_stroke.glif b/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/A_stroke.glif index 450aa8c6..66b91588 100644 --- a/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/A_stroke.glif +++ b/tests/data/SkipExportGlyphsTest-Bold.ufo/glyphs/A_stroke.glif @@ -2,10 +2,6 @@ - - - - diff --git a/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{170}/A_stroke.glif b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{170}/A_stroke.glif index b2879953..5de9f243 100644 --- a/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{170}/A_stroke.glif +++ b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs.{170}/A_stroke.glif @@ -2,8 +2,6 @@ - - diff --git a/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/A_stroke.glif b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/A_stroke.glif index 605da109..c6234223 100644 --- a/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/A_stroke.glif +++ b/tests/data/SkipExportGlyphsTest-Regular.ufo/glyphs/A_stroke.glif @@ -2,10 +2,6 @@ - - - - diff --git a/tests/filters/propagateAnchors_test.py b/tests/filters/propagateAnchors_test.py index 6b28682a..2bb59f60 100644 --- a/tests/filters/propagateAnchors_test.py +++ b/tests/filters/propagateAnchors_test.py @@ -1,8 +1,15 @@ import pytest +from fontTools.designspaceLib import DesignSpaceDocument from fontTools.misc.loggingTools import CapturingLogHandler import ufo2ft.filters -from ufo2ft.filters.propagateAnchors import PropagateAnchorsFilter, logger +from ufo2ft.filters.propagateAnchors import ( + PropagateAnchorsFilter, + PropagateAnchorsIFilter, + logger, +) +from ufo2ft.instantiator import Instantiator +from ufo2ft.util import _GlyphSet @pytest.fixture( @@ -257,3 +264,50 @@ def test_CantarellAnchorPropagation_reduced_filter(FontClass, datadir): anchors_o = {(a.name, a.x, a.y) for a in ufo["ocircumflextilde"].anchors} assert ("top", 284.0, 730.0) in anchors_o + + +class PropagateAnchorsIFilterTest: + def test_propagate_from_interpolated_components(self, FontClass, data_dir): + ds_path = data_dir / "SkipExportGlyphsTest.designspace" + ds = DesignSpaceDocument.fromfile(ds_path) + ds.loadSourceFonts(FontClass) + + ufos = [s.font for s in ds.sources] + glyphSets = [_GlyphSet.from_layer(s.font, s.layerName) for s in ds.sources] + + assert len(ufos) == len(glyphSets) == 4 + + # the composite glyph 'Astroke' has no anchors, but 'A' has some + for glyphSet in glyphSets: + if "Astroke" in glyphSet: + assert not glyphSet["Astroke"].anchors + if "A" in glyphSet: + assert glyphSet["A"].anchors + + # in glyphSets[2] the 'Astroke' component base glyphs are missing so their + # propagated anchors are supposed to be interpolated on the fly + assert "Astroke" in glyphSets[2] + assert {c.baseGlyph for c in glyphSets[2]["Astroke"].components}.isdisjoint( + glyphSets[2].keys() + ) + assert not glyphSets[2]["Astroke"].anchors + + instantiator = Instantiator.from_designspace( + ds, do_kerning=False, do_info=False + ) + + philter = PropagateAnchorsIFilter() + + modified = philter(ufos, glyphSets, instantiator) + + assert modified == {"Astroke"} + + assert [dict(a) for a in glyphSets[2]["Astroke"].anchors] == [ + {"name": "bottom", "x": 458, "y": 0}, + {"name": "center", "x": 457, "y": 358}, + {"name": "top", "x": 457, "y": 714}, + {"name": "topright", "x": 716, "y": 714}, + ] + assert {c.baseGlyph for c in glyphSets[2]["Astroke"].components}.isdisjoint( + glyphSets[2].keys() + )