Merge pull request #604 from googlefonts/extend-filters

filters/featureWriters: use '...' as placeholder for loading from UFO lib

Lib/ufo2ft/featureCompiler.py

@@ -17,6 +17,7 @@
     KernFeatureWriter,
     MarkFeatureWriter,
     ast,
+    isValidFeatureWriter,
     loadFeatureWriters,
 )
 
@@ -165,10 +166,17 @@ def __init__(self, ufo, ttFont=None, glyphSet=None, featureWriters=None, **kwarg
               under the key "com.github.googlei18n.ufo2ft.featureWriters"
               (see loadFeatureWriters).
             - if that is not found, the default list of writers will be used:
-              [KernFeatureWriter, MarkFeatureWriter]. This generates "kern"
-              (or "dist" for Indic scripts), "mark" and "mkmk" features.
+              (see FeatureCompiler.defaultFeatureWriters, and the individual
+              feature writer classes for the list of features generated).
             If the featureWriters list is empty, no automatic feature is
             generated and only pre-existing features are compiled.
+            The ``featureWriters`` parameter overrides both the writers from
+            the UFO lib and the default writers list. To extend instead of
+            replace the latter, the list can contain a special value ``...``
+            (i.e. the ``ellipsis`` singleton, not the str literal '...')
+            which gets replaced by either the UFO.lib writers or the default
+            ones; thus one can insert additional writers either before or after
+            these.
         """
         BaseFeatureCompiler.__init__(self, ufo, ttFont, glyphSet)
 
@@ -184,10 +192,41 @@ def __init__(self, ufo, ttFont=None, glyphSet=None, featureWriters=None, **kwarg
                 stacklevel=2,
             )
 
+    def _load_custom_feature_writers(self, featureWriters=None):
+        # Args:
+        #   ufo: Font
+        #   featureWriters: Optional[List[Union[FeatureWriter, EllipsisType]]])
+        # Returns: List[FeatureWriter]
+
+        # by default, load the feature writers from the lib or the default ones;
+        # ellipsis is used as a placeholder so one can optionally insert additional
+        # featureWriters=[w1, ..., w2] either before or after these, or override
+        # them by omitting the ellipsis.
+        if featureWriters is None:
+            featureWriters = [...]
+        result = []
+        seen_ellipsis = False
+        for writer in featureWriters:
+            if writer is ...:
+                if seen_ellipsis:
+                    raise ValueError("ellipsis not allowed more than once")
+                writers = loadFeatureWriters(self.ufo)
+                if writers is not None:
+                    result.extend(writers)
+                else:
+                    result.extend(self.defaultFeatureWriters)
+                seen_ellipsis = True
+            else:
+                klass = writer if isclass(writer) else type(writer)
+                if not isValidFeatureWriter(klass):
+                    raise TypeError(f"Invalid feature writer: {writer!r}")
+                result.append(writer)
+        return result
+
     def initFeatureWriters(self, featureWriters=None):
         """Initialize feature writer classes as specified in the UFO lib.
-        If none are defined in the UFO, the default feature writers are used:
-        currently, KernFeatureWriter and MarkFeatureWriter.
+        If none are defined in the UFO, the default feature writers are used
+        (see FeatureCompiler.defaultFeatureWriters).
         The 'featureWriters' argument can be used to override these.
         The method sets the `self.featureWriters` attribute with the list of
         writers.
@@ -197,10 +236,7 @@ def initFeatureWriters(self, featureWriters=None):
         used in the subsequent feature writers to resolve substitutions from
         glyphs with unicodes to their alternates.
         """
-        if featureWriters is None:
-            featureWriters = loadFeatureWriters(self.ufo)
-            if featureWriters is None:
-                featureWriters = self.defaultFeatureWriters
+        featureWriters = self._load_custom_feature_writers(featureWriters)
 
         gsubWriters = []
         others = []

Lib/ufo2ft/featureWriters/ast.py

@@ -209,9 +209,9 @@ def getGDEFGlyphClasses(feaLib):
     """Return GDEF GlyphClassDef base/mark/ligature/component glyphs, or
     None if no GDEF table is defined in the feature file.
     """
-    for st in feaLib.statements:
-        if isinstance(st, ast.TableBlock) and st.name == "GDEF":
-            for st in st.statements:
+    for s in feaLib.statements:
+        if isinstance(s, ast.TableBlock) and s.name == "GDEF":
+            for st in s.statements:
                 if isinstance(st, ast.GlyphClassDefStatement):
                     return _GDEFGlyphClasses(
                         frozenset(st.baseGlyphs.glyphSet())

Lib/ufo2ft/featureWriters/markFeatureWriter.py

@@ -201,15 +201,15 @@ def colorGraph(adjacency):
     """
     # Basic implementation
     # https://en.wikipedia.org/wiki/Greedy_coloring
-    color = dict()
+    colors = dict()
     # Sorted for reproducibility, probably not the optimal vertex order
     for node in sorted(adjacency):
         usedNeighbourColors = {
-            color[neighbour] for neighbour in adjacency[node] if neighbour in color
+            colors[neighbour] for neighbour in adjacency[node] if neighbour in colors
         }
-        color[node] = firstAvailable(usedNeighbourColors)
+        colors[node] = firstAvailable(usedNeighbourColors)
     groups = defaultdict(list)
-    for node, color in color.items():
+    for node, color in colors.items():
         groups[color].append(node)
     return list(groups.values())
 

Lib/ufo2ft/filters/__init__.py

@@ -85,7 +85,7 @@ def isValidFilter(klass):
     a '__call__' (bound method), with the signature matching the same method
     from the BaseFilter class:
 
-           def __call__(self, font, feaFile, compiler=None)
+           def __call__(self, font, glyphSet=None)
     """
     if not isclass(klass):
         logger.error(f"{klass!r} is not a class")

Lib/ufo2ft/preProcessor.py

@@ -1,14 +1,42 @@
+import itertools
+
 from ufo2ft.constants import (
     COLOR_LAYER_MAPPING_KEY,
     COLOR_LAYERS_KEY,
     COLOR_PALETTES_KEY,
 )
-from ufo2ft.filters import loadFilters
+from ufo2ft.filters import isValidFilter, loadFilters
 from ufo2ft.filters.decomposeComponents import DecomposeComponentsFilter
 from ufo2ft.fontInfoData import getAttrWithFallback
 from ufo2ft.util import _GlyphSet
 
 
+def _load_custom_filters(ufo, filters=None):
+    # Args:
+    #   ufo: Font
+    #   filters: Optional[List[Union[Filter, EllipsisType]]])
+    # Returns: List[Filter]
+
+    # by default, load the filters from the lib; ellipsis is used as a placeholder
+    # so one can optionally insert additional filters=[f1, ..., f2] either
+    # before or after these, or override them by omitting the ellipsis.
+    if filters is None:
+        filters = [...]
+    seen_ellipsis = False
+    result = []
+    for f in filters:
+        if f is ...:
+            if seen_ellipsis:
+                raise ValueError("ellipsis not allowed more than once")
+            result.extend(itertools.chain(*loadFilters(ufo)))
+            seen_ellipsis = True
+        else:
+            if not isValidFilter(type(f)):
+                raise TypeError(f"Invalid filter: {f!r}")
+            result.append(f)
+    return result
+
+
 class BasePreProcessor:
     """Base class for objects that performs pre-processing operations on
     the UFO glyphs, such as decomposing composites, removing overlaps, or
@@ -26,8 +54,17 @@ class BasePreProcessor:
     initialization of the default filters.
 
     Custom filters can be applied before or after the default filters.
-    These are specified in the UFO lib.plist under the private key
+    These can be specified in the UFO lib.plist under the private key
     "com.github.googlei18n.ufo2ft.filters".
+    Alternatively the optional ``filters`` parameter can be used. This is a
+    list of filter instances (subclasses of BaseFilter) that overrides
+    those defined in the UFO lib. The list can be empty, meaning no custom
+    filters are run. If ``filters`` contain the special value ``...`` (i.e.
+    the actual ``ellipsis`` singleton, not the str literal '...'), then all
+    the filters from the UFO lib are loaded in its place. This allows to
+    insert additional filters before or after those already defined in the
+    UFO lib, as opposed to discard/replace them which is the default behavior
+    when ``...`` is absent.
     """
 
     def __init__(
@@ -37,7 +74,7 @@ def __init__(
         layerName=None,
         skipExportGlyphs=None,
         filters=None,
-        **kwargs
+        **kwargs,
     ):
         self.ufo = ufo
         self.inplace = inplace
@@ -46,11 +83,10 @@ def __init__(
             ufo, layerName, copy=not inplace, skipExportGlyphs=skipExportGlyphs
         )
         self.defaultFilters = self.initDefaultFilters(**kwargs)
-        if filters is None:
-            self.preFilters, self.postFilters = loadFilters(ufo)
-        else:
-            self.preFilters = [f for f in filters if f.pre]
-            self.postFilters = [f for f in filters if not f.pre]
+
+        filters = _load_custom_filters(ufo, filters)
+        self.preFilters = [f for f in filters if f.pre]
+        self.postFilters = [f for f in filters if not f.pre]
 
     def initDefaultFilters(self, **kwargs):
         return []  # pragma: no cover
@@ -260,18 +296,9 @@ def __init__(
             self.defaultFilters.append([])
             _init_explode_color_layer_glyphs_filter(ufo, self.defaultFilters[-1])
 
-        self.preFilters, self.postFilters = [], []
-        if filters is None:
-            for ufo in ufos:
-                pre, post = loadFilters(ufo)
-                self.preFilters.append(pre)
-                self.postFilters.append(post)
-        else:
-            pre = [f for f in filters if f.pre]
-            post = [f for f in filters if not f.pre]
-            for _ in ufos:
-                self.preFilters.append(pre)
-                self.postFilters.append(post)
+        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 cu2qu.ufo import fonts_to_quadratic

tests/featureCompiler_test.py

@@ -78,6 +78,13 @@ def test_include_not_found(self, FontClass, tmpdir, caplog):
         assert "change the file name in the include" in caplog.text
 
 
+class DummyFeatureWriter:
+    tableTag = "GPOS"
+
+    def write(self, font, feaFile, compiler=None):
+        pass
+
+
 class FeatureCompilerTest:
     def test_ttFont(self, FontClass):
         ufo = FontClass()
@@ -174,6 +181,24 @@ def test_loadFeatureWriters_from_UFO_lib(self, FontClass):
         assert isinstance(compiler.featureWriters[0], KernFeatureWriter)
         assert "GPOS" in ttFont
 
+    def test_loadFeatureWriters_from_both_UFO_lib_and_argument(self, FontClass):
+        ufo = FontClass()
+        ufo.lib[FEATURE_WRITERS_KEY] = [{"class": "KernFeatureWriter"}]
+        compiler = FeatureCompiler(ufo, featureWriters=[..., DummyFeatureWriter])
+
+        assert len(compiler.featureWriters) == 2
+        assert isinstance(compiler.featureWriters[0], KernFeatureWriter)
+        assert isinstance(compiler.featureWriters[1], DummyFeatureWriter)
+
+    def test_loadFeatureWriters_from_both_defaults_and_argument(self, FontClass):
+        ufo = FontClass()
+        compiler = FeatureCompiler(ufo, featureWriters=[DummyFeatureWriter, ...])
+
+        assert len(compiler.featureWriters) == 1 + len(
+            FeatureCompiler.defaultFeatureWriters
+        )
+        assert isinstance(compiler.featureWriters[0], DummyFeatureWriter)
+
     def test_GSUB_writers_run_first(self, FontClass):
         class FooFeatureWriter(BaseFeatureWriter):
 

tests/preProcessor_test.py

@@ -116,6 +116,21 @@ def test_custom_filters_as_argument(self, FontClass):
         # filter2, before overlaps were removed in a post-filter filter1
         assert len(glyphSets0["d"].components) == 0
 
+    def test_custom_filters_in_both_lib_and_argument_with_ellipsis(self, FontClass):
+        from ufo2ft.filters import TransformationsFilter
+
+        ufo = FontClass(getpath("TestFont.ufo"))
+        ufo.lib[FILTERS_KEY] = [
+            {"name": "transformations", "kwargs": {"OffsetX": 10}, "pre": True}
+        ]
+
+        glyphSet = TTFPreProcessor(
+            ufo, filters=[..., TransformationsFilter(OffsetY=-10)]
+        ).process()
+
+        a = glyphSet["a"]
+        assert (a[0][0].x, a[0][0].y) == (ufo["a"][0][0].x + 10, ufo["a"][0][0].y - 10)
+
 
 class TTFInterpolatablePreProcessorTest:
     def test_no_inplace(self, FontClass):
@@ -205,6 +220,34 @@ def test_custom_filters_as_argument(self, FontClass):
         # filter2, before overlaps were removed in a post-filter filter1
         assert len(glyphSets[0]["d"].components) == 0
 
+    def test_custom_filters_in_both_lib_and_argument_with_ellipsis(self, FontClass):
+        from ufo2ft.filters import TransformationsFilter
+
+        ufo1 = FontClass(getpath("TestFont.ufo"))
+        ufo1.lib[FILTERS_KEY] = [
+            {"name": "transformations", "kwargs": {"OffsetX": 10}, "pre": True}
+        ]
+
+        ufo2 = FontClass(getpath("TestFont.ufo"))
+        ufo2.lib[FILTERS_KEY] = [
+            {"name": "transformations", "kwargs": {"OffsetX": 20}, "pre": True}
+        ]
+
+        glyphSets = TTFInterpolatablePreProcessor(
+            [ufo1, ufo2], filters=[..., TransformationsFilter(OffsetY=-10)]
+        ).process()
+
+        a1 = glyphSets[0]["a"]
+        assert (a1[0][0].x, a1[0][0].y) == (
+            ufo1["a"][0][0].x + 10,
+            ufo1["a"][0][0].y - 10,
+        )
+        a2 = glyphSets[1]["a"]
+        assert (a2[0][0].x, a2[0][0].y) == (
+            ufo2["a"][0][0].x + 20,
+            ufo2["a"][0][0].y - 10,
+        )
+
 
 class SkipExportGlyphsTest:
     def test_skip_export_glyphs_filter(self, FontClass):