Repository: googlefonts/ufo2ft
Pull request #650 (2022-08-24 13:43:41)

dottedCircleFilter: use defcon.Glyph with defcon.Font, not a ufoLib2.Glyph
Fixes https://github.com/googlefonts/ufo2ft/issues/644


Patch:
diff --git a/Lib/ufo2ft/filters/dottedCircleFilter.py b/Lib/ufo2ft/filters/dottedCircleFilter.py
index 0b9771d..54e7e65 100644
--- a/Lib/ufo2ft/filters/dottedCircleFilter.py
+++ b/Lib/ufo2ft/filters/dottedCircleFilter.py
@@ -48,13 +48,12 @@ import math
 from statistics import mean
 
 from fontTools.misc.fixedTools import otRound
-from ufoLib2.objects import Glyph
 
 from ufo2ft.constants import OPENTYPE_CATEGORIES_KEY
 from ufo2ft.featureCompiler import parseLayoutFeatures
 from ufo2ft.featureWriters import ast
 from ufo2ft.filters import BaseFilter
-from ufo2ft.util import _GlyphSet, _LazyFontName
+from ufo2ft.util import _getNewGlyphFactory, _GlyphSet, _LazyFontName, _setGlyphMargin
 
 logger = logging.getLogger(__name__)
 
@@ -149,7 +148,9 @@ class DottedCircleFilter(BaseFilter):
         """Add a new dotted circle glyph, drawing its outlines"""
         font = self.context.font
         logger.debug("Adding dotted circle glyph")
-        glyph = Glyph(name="uni25CC", unicodes=[0x25CC])
+
+        proto = font.layers.defaultLayer.instantiateGlyphObject()
+        glyph = _getNewGlyphFactory(proto)(name="uni25CC", unicodes=[0x25CC])
         pen = glyph.getPen()
 
         bigradius = (font.info.xHeight - 2 * self.options.margin) / 2
@@ -165,7 +166,7 @@ class DottedCircleFilter(BaseFilter):
             cy = middleY + bigradius * math.sin(angle)
             circle(pen, (cx, cy), littleradius)
 
-        glyph.setRightMargin(self.options.sidebearing)
+        _setGlyphMargin(glyph, "right", self.options.sidebearing)
 
         glyphSet["uni25CC"] = glyph
         return glyph
@@ -182,7 +183,6 @@ class DottedCircleFilter(BaseFilter):
         # the position of the anchor so we can average them.
         all_anchors = {}
         any_added = False
-        anchorclass = None
         for glyph in font:
             width = None
             try:
@@ -196,7 +196,6 @@ class DottedCircleFilter(BaseFilter):
             if width is None:
                 width = glyph.width
             for anchor in glyph.anchors:
-                anchorclass = anchor.__class__
                 if anchor.name.startswith("_"):
                     all_anchors[anchor.name] = []
                     continue
@@ -222,14 +221,9 @@ class DottedCircleFilter(BaseFilter):
                 anchor_x,
                 anchor_y,
             )
-            try:
-                newanchor = anchorclass()
-                newanchor.x = otRound(anchor_x)
-                newanchor.y = otRound(anchor_y)
-            except TypeError:
-                newanchor = anchorclass(otRound(anchor_x), otRound(anchor_y))
-            newanchor.name = anchor
-            dotted_circle_glyph.appendAnchor(newanchor)
+            dotted_circle_glyph.appendAnchor(
+                {"x": otRound(anchor_x), "y": otRound(anchor_y), "name": anchor}
+            )
             any_added = True
         return any_added
 
diff --git a/Lib/ufo2ft/util.py b/Lib/ufo2ft/util.py
index 8549b00..8bc8e25 100644
--- a/Lib/ufo2ft/util.py
+++ b/Lib/ufo2ft/util.py
@@ -1,11 +1,9 @@
-from __future__ import annotations
-
 import importlib
 import logging
 import re
 from copy import deepcopy
 from inspect import currentframe, getfullargspec
-from typing import TYPE_CHECKING, Callable
+from typing import Set
 
 from fontTools import subset, ttLib, unicodedata
 from fontTools.designspaceLib import DesignSpaceDocument
@@ -15,9 +13,6 @@ from fontTools.misc.transform import Identity, Transform
 from fontTools.pens.reverseContourPen import ReverseContourPen
 from fontTools.pens.transformPen import TransformPen
 
-if TYPE_CHECKING:
-    from fontTools.ttLib.tables.G_S_U_B_ import table_G_S_U_B_
-
 logger = logging.getLogger(__name__)
 
 
@@ -108,16 +103,18 @@ def _getNewGlyphFactory(glyph):
     cls = glyph.__class__
     if "name" in getfullargspec(cls.__init__).args:
 
-        def newGlyph(name):
-            return cls(name=name)
+        def newGlyph(name, **kwargs):
+            return cls(name=name, **kwargs)
 
     else:
 
-        def newGlyph(name):
+        def newGlyph(name, **kwargs):
             # use instantiateGlyphObject() to keep any custom sub-element classes
             # https://github.com/googlefonts/ufo2ft/issues/363
             g2 = glyph.layer.instantiateGlyphObject()
             g2.name = name
+            for k, v in kwargs.items():
+                setattr(g2, k, v)
             return g2
 
     return newGlyph
@@ -146,6 +143,21 @@ def _copyGlyph(glyph, glyphFactory=None, reverseContour=False):
     return copy
 
 
+def _setGlyphMargin(glyph, side, margin):
+    # defcon.Glyph has @property setters for the margins, whereas ufoLib2.Glyph
+    # has regular instance methods
+    assert side in {"left", "right", "top", "bottom"}
+    if hasattr(glyph, f"set{side.title()}Margin"):  # ufoLib2
+        getattr(glyph, f"set{side.title()}Margin")(margin)
+        assert getattr(glyph, f"get{side.title()}Margin")() == margin
+    elif hasattr(glyph, f"{side}Margin"):  # defcon
+        descriptor = getattr(type(glyph), f"{side}Margin")
+        descriptor.__set__(glyph, margin)
+        assert descriptor.__get__(glyph) == margin
+    else:
+        raise NotImplementedError(f"Unsupported Glyph class: {type(glyph)!r}")
+
+
 def deepCopyContours(
     glyphSet, parent, composite, transformation, specificComponents=None
 ):
@@ -267,30 +279,25 @@ def closeGlyphsOverGSUB(gsub, glyphs):
     gsub.closure_glyphs(subsetter)
 
 
-def classifyGlyphs(
-    unicodeFunc: Callable[[int], str | bool | list[str] | set[str] | tuple[str] | None],
-    cmap: dict[int, str],
-    gsub: table_G_S_U_B_ | None = None,
-) -> dict[str | bool, set[str]]:
-    """Returns a dictionary of glyph sets associated with the given Unicode
-    properties.
-
-    'unicodeFunc' is a callable that takes a Unicode codepoint and
-    returns a string, bool, or collection of strings, denoting some Unicode
+def classifyGlyphs(unicodeFunc, cmap, gsub=None):
+    """'unicodeFunc' is a callable that takes a Unicode codepoint and
+    returns a string, or collection of strings, denoting some Unicode
     property associated with the given character (or None if a character
     is considered 'neutral'). 'cmap' is a dictionary mapping Unicode
     codepoints to glyph names. 'gsub' is an (optional) fonttools GSUB
     table object, used to find all the glyphs that are "reachable" via
     substitutions from the initial sets of glyphs defined in the cmap.
-    """
 
-    glyphSets: dict[str | bool, set[str]] = {}
-    neutralGlyphs: set[str] = set()
+    Returns a dictionary of glyph sets associated with the given Unicode
+    properties.
+    """
+    glyphSets = {}
+    neutralGlyphs = set()
     for uv, glyphName in cmap.items():
         key_or_keys = unicodeFunc(uv)
         if key_or_keys is None:
             neutralGlyphs.add(glyphName)
-        elif isinstance(key_or_keys, (list, set, tuple)):
+        elif isinstance(key_or_keys, (list, set)):
             for key in key_or_keys:
                 glyphSets.setdefault(key, set()).add(glyphName)
         else:
@@ -552,7 +559,7 @@ def ensure_all_sources_have_names(doc: DesignSpaceDocument) -> None:
     This may rename sources with a "temp_master.N" name, designspaceLib's default
     stand-in.
     """
-    used_names: set[str] = set()
+    used_names: Set[str] = set()
     counter = 0
     for source in doc.sources:
         while source.name is None or source.name in used_names:


Test patch:
diff --git a/tests/filters/dottedCircle_test.py b/tests/filters/dottedCircle_test.py
index f4c8530..9f9f4e5 100644
--- a/tests/filters/dottedCircle_test.py
+++ b/tests/filters/dottedCircle_test.py
@@ -8,9 +8,20 @@ def test_dotted_circle_filter(FontClass, datadir):
     assert "uni25CC" not in font
     philter = DottedCircleFilter()
     glyphset = _GlyphSet.from_layer(font)
+
     modified = philter(font, glyphset)
+
     assert "uni25CC" in modified
-    anchors = list(sorted(glyphset["uni25CC"].anchors, key=lambda x: x.name))
+
+    dotted_circle = glyphset["uni25CC"]
+
+    # check the Glyph's module is the same as the Font's (both ufoLib2 or defcon,
+    # not mixed): https://github.com/googlefonts/ufo2ft/issues/644
+    font_ufo_module = type(font).__module__.split(".")[0]
+    glyph_ufo_module = type(dotted_circle).__module__.split(".")[0]
+    assert glyph_ufo_module == font_ufo_module
+
+    anchors = list(sorted(dotted_circle.anchors, key=lambda x: x.name))
     assert anchors[0].x == 464
     assert anchors[0].y == -17
     assert anchors[0].name == "bottom"
@@ -19,5 +30,6 @@ def test_dotted_circle_filter(FontClass, datadir):
     assert anchors[1].y == 546
     assert anchors[1].name == "top"
 
-    assert len(glyphset["uni25CC"]) == 12
-    assert int(glyphset["uni25CC"].width) == 688
+    assert len(dotted_circle) == 12
+    assert int(dotted_circle.width) == 688
+    assert dotted_circle.unicodes == [0x25CC]