From 473bf99b6d96cb92b9421d68c8f7225f341bb5c1 Mon Sep 17 00:00:00 2001
From: Just van Rossum <justvanrossum@gmail.com>
Date: Sat, 14 Sep 2024 09:07:38 +0200
Subject: [PATCH 1/4] Add GDEF to reproduce the .width AttributeError bug

---
 .../MutatorSansBoldWideMutated.ufo/features.fea           | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Tests/data/MutatorSans/MutatorSansBoldWideMutated.ufo/features.fea b/Tests/data/MutatorSans/MutatorSansBoldWideMutated.ufo/features.fea
index 769bd3f6..3af4ed8c 100644
--- a/Tests/data/MutatorSans/MutatorSansBoldWideMutated.ufo/features.fea
+++ b/Tests/data/MutatorSans/MutatorSansBoldWideMutated.ufo/features.fea
@@ -1 +1,9 @@
 include(features_test.fea)
+
+table GDEF {
+    GlyphClassDef
+    [ A ],
+    [  ],
+    [ macroncmb ],
+    ;
+} GDEF;

From dafb1bfa75d594ef2291fb5b7c60a6afc848a0c4 Mon Sep 17 00:00:00 2001
From: Just van Rossum <justvanrossum@gmail.com>
Date: Sat, 14 Sep 2024 09:38:04 +0200
Subject: [PATCH 2/4] Also fetch advance width, ufo2ft sometimes needs it

---
 Lib/fontgoggles/compile/ufoCompiler.py | 41 +++++++++++++++++++-------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/Lib/fontgoggles/compile/ufoCompiler.py b/Lib/fontgoggles/compile/ufoCompiler.py
index c54af62d..189b27ec 100644
--- a/Lib/fontgoggles/compile/ufoCompiler.py
+++ b/Lib/fontgoggles/compile/ufoCompiler.py
@@ -35,7 +35,7 @@ def compileUFOToFont(ufoPath):
     if ".notdef" not in glyphOrder:
         # We need a .notdef glyph, so let's make one.
         glyphOrder.insert(0, ".notdef")
-    cmap, revCmap, anchors = fetchCharacterMappingAndAnchors(glyphSet, ufoPath, ufo2=ufo2)
+    widths, cmap, revCmap, anchors = fetchGlyphInfo(glyphSet, ufoPath, ufo2=ufo2)
     fb = FontBuilder(round(info.unitsPerEm))
     fb.setupGlyphOrder(glyphOrder)
     fb.setupCharacterMap(cmap)
@@ -46,7 +46,7 @@ def compileUFOToFont(ufoPath):
     # changes.
     ttFont["FGAx"] = newTable("FGAx")
     ttFont["FGAx"].data = pickle.dumps(anchors)
-    ufo = MinimalFontObject(ufoPath, reader, revCmap, anchors)
+    ufo = MinimalFontObject(ufoPath, reader, widths, revCmap, anchors)
     feaComp = FeatureCompiler(ufo, ttFont)
     try:
         feaComp.compile()
@@ -68,13 +68,15 @@ def compileUFOToPath(ufoPath, ttPath):
     ttFont.save(ttPath, reorderTables=False)
 
 
-_unicodeOrAnchorGLIFPattern = re.compile(rb'(<\s*(anchor|unicode)\s+([^>]+)>)')
+_tagGLIFPattern = re.compile(rb'(<\s*(advance|anchor|unicode)\s+([^>]+)>)')
 _ufo2AnchorPattern = re.compile(rb"<contour>\s+(<point\s+[^>]+move[^>]+name[^>]+>)\s+</contour>")
 _unicodeAttributeGLIFPattern = re.compile(rb'hex\s*=\s*\"([0-9A-Fa-f]+)\"')
+_widthAttributeGLIFPattern = re.compile(rb'width\s*=\s*\"([0-9A-Fa-f]+)\"')
 
 
-def fetchCharacterMappingAndAnchors(glyphSet, ufoPath, glyphNames=None, ufo2=False):
+def fetchGlyphInfo(glyphSet, ufoPath, glyphNames=None, ufo2=False):
     # This seems about 2.3 times faster than reader.getCharacterMapping()
+    widths = {}
     cmap = {}  # unicode: glyphName
     revCmap = {}
     anchors = {}  # glyphName: [(anchorName, x, y), ...]
@@ -86,12 +88,13 @@ def fetchCharacterMappingAndAnchors(glyphSet, ufoPath, glyphNames=None, ufo2=Fal
         if b"<!--" in data:
             # Fall back to proper parser, assuming this to be uncommon
             # (This does not work for UFO 2)
-            unicodes, glyphAnchors = fetchUnicodesAndAnchors(data)
+            width, unicodes, glyphAnchors = fetchUnicodesAndAnchors(data)
         else:
             # Fast route with regex
+            width = None
             unicodes = []
             glyphAnchors = []
-            for rawElement, tag, rawAttributes in _unicodeOrAnchorGLIFPattern.findall(data):
+            for rawElement, tag, rawAttributes in _tagGLIFPattern.findall(data):
                 if tag == b"unicode":
                     m = _unicodeAttributeGLIFPattern.match(rawAttributes)
                     try:
@@ -101,11 +104,17 @@ def fetchCharacterMappingAndAnchors(glyphSet, ufoPath, glyphNames=None, ufo2=Fal
                 elif tag == b"anchor":
                     root = ET.fromstring(rawElement)
                     glyphAnchors.append(_parseAnchorAttrs(root.attrib))
+                elif tag == b"advance":
+                    m = _widthAttributeGLIFPattern.search(rawAttributes)
+                    if m is not None:
+                        width = float(m.group(1))
             if ufo2:
                 for rawElement in _ufo2AnchorPattern.findall(data):
                     root = ET.fromstring(rawElement)
                     glyphAnchors.append(_parseAnchorAttrs(root.attrib))
 
+        widths[glyphName] = width
+
         uniqueUnicodes = []
         for codePoint in unicodes:
             if codePoint not in cmap:
@@ -127,7 +136,7 @@ def fetchCharacterMappingAndAnchors(glyphSet, ufoPath, glyphNames=None, ufo2=Fal
         logger = logging.getLogger("fontgoggles.font.ufoFont")
         logger.warning("Some code points in '%s' are assigned to multiple glyphs: %s",
                        ufoPath, dupMessage)
-    return cmap, revCmap, anchors
+    return widths, cmap, revCmap, anchors
 
 
 def fetchUnicodesAndAnchors(glif):
@@ -136,7 +145,7 @@ def fetchUnicodesAndAnchors(glif):
     """
     parser = FetchUnicodesAndAnchorsParser()
     parser.parse(glif)
-    return parser.unicodes, parser.anchors
+    return parser.advanceWidth, parser.unicodes, parser.anchors
 
 
 def _parseNumber(s):
@@ -158,6 +167,7 @@ class FetchUnicodesAndAnchorsParser(BaseGlifParser):
     def __init__(self):
         self.unicodes = []
         self.anchors = []
+        self.advanceWidth = None
         super().__init__()
 
     def startElementHandler(self, name, attrs):
@@ -173,6 +183,8 @@ def startElementHandler(self, name, attrs):
                         pass
             elif name == "anchor":
                 self.anchors.append(_parseAnchorAttrs(attrs))
+            elif name == "advance":
+                self.advanceWidth = _parseNumber(attrs.get("width"))
         super().startElementHandler(name, attrs)
 
 
@@ -184,8 +196,9 @@ class MinimalFontObject:
     # unicodes and anchors, and at the font level, only features, groups,
     # kerning and lib are needed.
 
-    def __init__(self, ufoPath, reader, revCmap, anchors):
+    def __init__(self, ufoPath, reader, widths, revCmap, anchors):
         self.path = ufoPath
+        self._widths = widths
         self._revCmap = revCmap
         self._anchors = anchors
         self._glyphNames = set(reader.getGlyphSet().contents.keys())
@@ -212,15 +225,21 @@ def __getitem__(self, glyphName):
         # TODO: should we even bother caching?
         glyph = self._glyphs.get(glyphName)
         if glyph is None:
-            glyph = MinimalGlyphObject(glyphName, self._revCmap.get(glyphName), self._anchors.get(glyphName, ()))
+            glyph = MinimalGlyphObject(
+                glyphName,
+                self._widths.get(glyphName, 0),
+                self._revCmap.get(glyphName),
+                self._anchors.get(glyphName, ()),
+            )
             self._glyphs[glyphName] = glyph
         return glyph
 
 
 class MinimalGlyphObject:
 
-    def __init__(self, name, unicodes, anchors):
+    def __init__(self, name, width, unicodes, anchors):
         self.name = name
+        self.width = width
         self.unicodes = unicodes
         self.anchors = [MinimalAnchorObject(name, x, y) for name, x, y in anchors]
 

From c9dde250338aa5ada40f1e77b1a9f7ce02d067d0 Mon Sep 17 00:00:00 2001
From: Just van Rossum <justvanrossum@gmail.com>
Date: Sat, 14 Sep 2024 09:38:28 +0200
Subject: [PATCH 3/4] Test fetching advance widths

---
 Tests/test_ufoCompiler.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Tests/test_ufoCompiler.py b/Tests/test_ufoCompiler.py
index cae61f01..a21c0270 100644
--- a/Tests/test_ufoCompiler.py
+++ b/Tests/test_ufoCompiler.py
@@ -2,7 +2,7 @@
 import os
 import pytest
 from fontTools.ufoLib import UFOReader
-from fontgoggles.compile.ufoCompiler import fetchCharacterMappingAndAnchors
+from fontgoggles.compile.ufoCompiler import fetchGlyphInfo
 from fontgoggles.compile.compilerPool import compileUFOToPath
 from testSupport import getFontPath
 
@@ -10,7 +10,9 @@
 def test_ufoCharacterMapping():
     ufoPath = getFontPath("MutatorSansBoldWideMutated.ufo")
     reader = UFOReader(ufoPath)
-    cmap, revCmap, anchors = fetchCharacterMappingAndAnchors(reader.getGlyphSet(), ufoPath)
+    widths, cmap, revCmap, anchors = fetchGlyphInfo(reader.getGlyphSet(), ufoPath)
+    assert widths["A"] == 1290
+    assert widths["B"] == 1270
     assert cmap[0x0041] == "A"
     assert revCmap["A"] == [0x0041]
     # MutatorSansBoldWideMutated.ufo/glyphs/A_.glif contains a commented-out <unicode>
@@ -28,7 +30,7 @@ def test_ufoCharacterMapping():
 def test_ufoCharacterMapping_glyphNames():
     ufoPath = getFontPath("MutatorSansBoldWideMutated.ufo")
     reader = UFOReader(ufoPath)
-    cmap, revCmap, anchors = fetchCharacterMappingAndAnchors(reader.getGlyphSet(), ufoPath, ["A"])
+    widths, cmap, revCmap, anchors = fetchGlyphInfo(reader.getGlyphSet(), ufoPath, ["A"])
     assert cmap[0x0041] == "A"
     assert revCmap["A"] == [0x0041]
     assert anchors == {"A": [("top", 645, 815)]}

From 5d16d49958141d31a26998b86ec08cb34daaee72 Mon Sep 17 00:00:00 2001
From: Just van Rossum <justvanrossum@gmail.com>
Date: Sat, 14 Sep 2024 09:38:46 +0200
Subject: [PATCH 4/4] Adapt to new name and signature of fetch func

---
 Lib/fontgoggles/font/ufoFont.py | 10 ++++++----
 Tests/test_ufoState.py          |  4 ++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/Lib/fontgoggles/font/ufoFont.py b/Lib/fontgoggles/font/ufoFont.py
index c856a285..412a7999 100644
--- a/Lib/fontgoggles/font/ufoFont.py
+++ b/Lib/fontgoggles/font/ufoFont.py
@@ -20,7 +20,7 @@
 from .baseFont import BaseFont
 from .glyphDrawing import GlyphDrawing, GlyphLayersDrawing
 from ..compile.compilerPool import compileUFOToBytes
-from ..compile.ufoCompiler import fetchCharacterMappingAndAnchors
+from ..compile.ufoCompiler import fetchGlyphInfo
 from ..misc.hbShape import HBShape
 from ..misc.properties import cachedProperty
 
@@ -400,9 +400,11 @@ def getUpdateInfo(self):
             changedGlyphNames = {glyphName for glyphName, mtime in prev.glyphModTimes ^ self.glyphModTimes}
             deletedGlyphNames = {glyphName for glyphName in changedGlyphNames if glyphName not in self.glyphSet}
 
-            _, changedUnicodes, changedAnchors = fetchCharacterMappingAndAnchors(self.glyphSet,
-                                                                                 self.reader.fs.getsyspath("/"),
-                                                                                 changedGlyphNames - deletedGlyphNames)
+            _, _, changedUnicodes, changedAnchors = fetchGlyphInfo(
+                self.glyphSet,
+                self.reader.fs.getsyspath("/"),
+                changedGlyphNames - deletedGlyphNames,
+            )
 
             # Within the changed glyphs, let's see if their anchors changed
             for gn in changedGlyphNames:
diff --git a/Tests/test_ufoState.py b/Tests/test_ufoState.py
index 9ed6dfe0..847c3fff 100644
--- a/Tests/test_ufoState.py
+++ b/Tests/test_ufoState.py
@@ -4,7 +4,7 @@
 from fontTools.ufoLib import UFOReaderWriter
 from fontTools.ufoLib.glifLib import Glyph
 from fontgoggles.font.ufoFont import UFOState
-from fontgoggles.compile.ufoCompiler import fetchCharacterMappingAndAnchors
+from fontgoggles.compile.ufoCompiler import fetchGlyphInfo
 from testSupport import getFontPath
 
 
@@ -13,7 +13,7 @@ def test_getUpdateInfo(tmpdir):
     ufoPath = shutil.copytree(ufoSource, tmpdir / "test.ufo")
     reader = UFOReaderWriter(ufoPath, validate=False)
     glyphSet = reader.getGlyphSet()
-    cmap, unicodes, anchors = fetchCharacterMappingAndAnchors(glyphSet, ufoPath)
+    widths, cmap, unicodes, anchors = fetchGlyphInfo(glyphSet, ufoPath)
 
     state = UFOState(reader, glyphSet, getUnicodesAndAnchors=lambda: (unicodes, anchors))