From 5cc53e2bfc87b40e339f8fc5485f41fedb2100f1 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 23 Jan 2024 16:37:43 +0000 Subject: [PATCH 1/2] move parsing public.openTypeCategories to util module --- .../featureWriters/baseFeatureWriter.py | 44 +-------------- Lib/ufo2ft/util.py | 55 ++++++++++++++++++- 2 files changed, 55 insertions(+), 44 deletions(-) diff --git a/Lib/ufo2ft/featureWriters/baseFeatureWriter.py b/Lib/ufo2ft/featureWriters/baseFeatureWriter.py index 32a15e4b..70a7e9ba 100644 --- a/Lib/ufo2ft/featureWriters/baseFeatureWriter.py +++ b/Lib/ufo2ft/featureWriters/baseFeatureWriter.py @@ -6,10 +6,10 @@ from fontTools.feaLib.variableScalar import VariableScalar from fontTools.misc.fixedTools import otRound -from ufo2ft.constants import OPENTYPE_CATEGORIES_KEY from ufo2ft.errors import InvalidFeaturesData from ufo2ft.featureWriters import ast from ufo2ft.util import ( + OpenTypeCategories, collapse_varscalar, get_userspace_location, quantize, @@ -351,47 +351,7 @@ def extraSubstitutions(self): def getOpenTypeCategories(self): """Return 'public.openTypeCategories' values as a tuple of sets of unassigned, bases, ligatures, marks, components.""" - font = self.context.font - unassigned, bases, ligatures, marks, components = ( - set(), - set(), - set(), - set(), - set(), - ) - 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 - openTypeCategories = font.lib.get(OPENTYPE_CATEGORIES_KEY, {}) - - for glyphName, category in openTypeCategories.items(): - if category == "unassigned": - unassigned.add(glyphName) - elif category == "base": - bases.add(glyphName) - elif category == "ligature": - ligatures.add(glyphName) - elif category == "mark": - marks.add(glyphName) - elif category == "component": - components.add(glyphName) - else: - self.log.warning( - f"The '{OPENTYPE_CATEGORIES_KEY}' value of {glyphName} in " - f"{font.info.familyName} {font.info.styleName} is '{category}' " - "when it should be 'unassigned', 'base', 'ligature', 'mark' " - "or 'component'." - ) - return namedtuple( - "OpenTypeCategories", "unassigned base ligature mark component" - )( - frozenset(unassigned), - frozenset(bases), - frozenset(ligatures), - frozenset(marks), - frozenset(components), - ) + return OpenTypeCategories.load(self.context.font) def getGDEFGlyphClasses(self): """Return a tuple of GDEF GlyphClassDef base, ligature, mark, component diff --git a/Lib/ufo2ft/util.py b/Lib/ufo2ft/util.py index 70c023ff..1b280546 100644 --- a/Lib/ufo2ft/util.py +++ b/Lib/ufo2ft/util.py @@ -5,7 +5,7 @@ import re from copy import deepcopy from inspect import currentframe, getfullargspec -from typing import Any, Mapping, Set +from typing import Any, Mapping, NamedTuple, Set from fontTools import subset, ttLib, unicodedata from fontTools.designspaceLib import DesignSpaceDocument @@ -15,7 +15,7 @@ from fontTools.pens.reverseContourPen import ReverseContourPen from fontTools.pens.transformPen import TransformPen -from ufo2ft.constants import UNICODE_SCRIPT_ALIASES +from ufo2ft.constants import OPENTYPE_CATEGORIES_KEY, UNICODE_SCRIPT_ALIASES from ufo2ft.errors import InvalidDesignSpaceData, InvalidFontData from ufo2ft.fontInfoData import getAttrWithFallback @@ -713,3 +713,54 @@ def collapse_varscalar(varscalar, threshold=0): if not any(abs(v - values[0]) > threshold for v in values[1:]): return list(varscalar.values.values())[0] return varscalar + + +class OpenTypeCategories(NamedTuple): + unassigned: frozenset[str] + base: frozenset[str] + ligature: frozenset[str] + mark: frozenset[str] + component: frozenset[str] + + @classmethod + def load(cls, font): + """Return 'public.openTypeCategories' values as a tuple of sets of + unassigned, bases, ligatures, marks, components.""" + unassigned, bases, ligatures, marks, components = ( + set(), + set(), + set(), + set(), + set(), + ) + 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 + openTypeCategories = font.lib.get(OPENTYPE_CATEGORIES_KEY, {}) + + for glyphName, category in openTypeCategories.items(): + if category == "unassigned": + unassigned.add(glyphName) + elif category == "base": + bases.add(glyphName) + elif category == "ligature": + ligatures.add(glyphName) + elif category == "mark": + marks.add(glyphName) + elif category == "component": + components.add(glyphName) + else: + logging.getLogger("ufo2ft").warning( + f"The '{OPENTYPE_CATEGORIES_KEY}' value of {glyphName} in " + f"{font.info.familyName} {font.info.styleName} is '{category}' " + "when it should be 'unassigned', 'base', 'ligature', 'mark' " + "or 'component'." + ) + return cls( + frozenset(unassigned), + frozenset(bases), + frozenset(ligatures), + frozenset(marks), + frozenset(components), + ) From b5830b861ba0231a27e7e05c4f24d09e517ce2b3 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 23 Jan 2024 16:40:16 +0000 Subject: [PATCH 2/2] do not propagate anchors to composite mark glyphs with anchors This is meant to match Glyphs.app's logic, see https://github.com/googlefonts/ufo2ft/issues/802 (A similar fix should be added to glyphsLib's anchor_propagation.py until the latter is superseded by the ufo2ft filter) --- .../featureWriters/baseFeatureWriter.py | 2 +- Lib/ufo2ft/filters/propagateAnchors.py | 23 +++++++++----- tests/filters/propagateAnchors_test.py | 31 ++++++++++++++++++- 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/Lib/ufo2ft/featureWriters/baseFeatureWriter.py b/Lib/ufo2ft/featureWriters/baseFeatureWriter.py index 70a7e9ba..b947e2a1 100644 --- a/Lib/ufo2ft/featureWriters/baseFeatureWriter.py +++ b/Lib/ufo2ft/featureWriters/baseFeatureWriter.py @@ -1,5 +1,5 @@ import logging -from collections import OrderedDict, namedtuple +from collections import OrderedDict from types import SimpleNamespace from fontTools.designspaceLib import DesignSpaceDocument diff --git a/Lib/ufo2ft/filters/propagateAnchors.py b/Lib/ufo2ft/filters/propagateAnchors.py index a346ed1f..a4147102 100644 --- a/Lib/ufo2ft/filters/propagateAnchors.py +++ b/Lib/ufo2ft/filters/propagateAnchors.py @@ -18,6 +18,7 @@ from fontTools.misc.transform import Transform from ufo2ft.filters import BaseFilter +from ufo2ft.util import OpenTypeCategories logger = logging.getLogger(__name__) @@ -26,14 +27,14 @@ class PropagateAnchorsFilter(BaseFilter): def set_context(self, font, glyphSet): ctx = super().set_context(font, glyphSet) ctx.processed = set() + ctx.categories = OpenTypeCategories.load(font) return ctx def __call__(self, font, glyphSet=None): - if super().__call__(font, glyphSet): - modified = self.context.modified - if modified: - logger.info("Glyphs with propagated anchors: %i" % len(modified)) - return modified + modified = super().__call__(font, glyphSet) + if modified: + logger.info("Glyphs with propagated anchors: %i" % len(modified)) + return modified def filter(self, glyph): if not glyph.components: @@ -44,11 +45,12 @@ def filter(self, glyph): glyph, self.context.processed, self.context.modified, + self.context.categories, ) return len(glyph.anchors) > before -def _propagate_glyph_anchors(glyphSet, composite, processed, modified): +def _propagate_glyph_anchors(glyphSet, composite, processed, modified, categories): """ Propagate anchors from base glyphs to a given composite glyph, and to all composite glyphs used in between. @@ -58,7 +60,12 @@ def _propagate_glyph_anchors(glyphSet, composite, processed, modified): return processed.add(composite.name) - if not composite.components: + if not composite.components or ( + # "If it is a 'mark' and there are anchors, it will not look into components" + # Georg said: https://github.com/googlefonts/ufo2ft/issues/802#issuecomment-1904109457 + composite.name in categories.mark + and composite.anchors + ): return base_components = [] @@ -74,7 +81,7 @@ def _propagate_glyph_anchors(glyphSet, composite, processed, modified): "in glyph {}".format(component.baseGlyph, composite.name) ) else: - _propagate_glyph_anchors(glyphSet, glyph, processed, modified) + _propagate_glyph_anchors(glyphSet, glyph, processed, modified, categories) if any(a.name.startswith("_") for a in glyph.anchors): mark_components.append(component) else: diff --git a/tests/filters/propagateAnchors_test.py b/tests/filters/propagateAnchors_test.py index 695f3ba0..6b28682a 100644 --- a/tests/filters/propagateAnchors_test.py +++ b/tests/filters/propagateAnchors_test.py @@ -106,6 +106,25 @@ ("addComponent", ("macroncomb", (1, 0, 0, 1, 175, 0))), ], }, + { + "name": "r", + "width": 350, + "outline": [ + ("moveTo", ((0, 0),)), + ("lineTo", ((0, 300),)), + ("lineTo", ((175, 300),)), + ("closePath", ()), + ], + "anchors": [(175, 300, "top"), (175, 0, "bottom")], + }, + { + "name": "rcombbelow", + "width": 0, + "outline": [ + ("addComponent", ("r", (0.5, 0, 0, 0.5, -100, -100))), + ], + "anchors": [(0, 0, "_bottom")], + }, ] } ] @@ -120,6 +139,12 @@ def font(request, FontClass): getattr(pen, operator)(*operands) for x, y, name in param.get("anchors", []): glyph.appendAnchor(dict(x=x, y=y, name=name)) + # classify as 'mark' all glyphs with zero width and 'comb' in their name + font.lib["public.openTypeCategories"] = { + g["name"]: "mark" + for g in request.param["glyphs"] + if g.get("width", 0) == 0 and "comb" in g["name"] + } return font @@ -149,6 +174,10 @@ def font(request, FontClass): ], {"a_a"}, ), + # the composite glyph is a mark with anchors, hence propagation is not performed, + # i.e. 'top' and 'bottom' are *not* copied to 'rcombbelow': + # https://github.com/googlefonts/ufo2ft/issues/802 + "rcombbelow": ([("_bottom", 0, 0)], set()), } @@ -173,7 +202,7 @@ def test_include_one_glyph_at_a_time(self, font, name): def test_whole_font(self, font): philter = PropagateAnchorsFilter() modified = philter(font) - assert modified == set(EXPECTED) + assert modified == {k for k in EXPECTED if k in EXPECTED[k][1]} for name, (expected_anchors, _) in EXPECTED.items(): assert [(a.name, a.x, a.y) for a in font[name].anchors] == expected_anchors