Repository: googlefonts/ufo2ft
Pull request #840 (2024-05-15 13:45:40)

[variable features] [mark fea writer] Instead of skipping a glyph altogether, only skip missing source layers
Instead of skipping a glyph altogether, only skip missing source layers, so we can still build meaningful variable anchors.

Situation:
- a set of ufo's is sparse (at least the default ufo contains all glyphs)
- some ufo sources contain a glyph with anchors (for the mark feature), but others don't

Currently, I get a failure in this situation:
```
Traceback (most recent call last):
  File "/Users/just/code/git/BlackFoundry/fontra/venv/lib/python3.12/site-packages/fontmake/font_project.py", line 1208, in run_from_designspace
    self._run_from_designspace_interpolatable(
  File "/Users/just/code/git/BlackFoundry/fontra/venv/lib/python3.12/site-packages/fontmake/font_project.py", line 1312, in _run_from_designspace_interpolatable
    self.build_variable_fonts(
  File "/Users/just/code/git/BlackFoundry/fontra/venv/lib/python3.12/site-packages/fontmake/font_project.py", line 450, in build_variable_fonts
    fonts = ufo2ft.compileVariableTTFs(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/just/code/git/BlackFoundry/fontra/venv/lib/python3.12/site-packages/ufo2ft/__init__.py", line 172, in compileVariableTTFs
    return VariableTTFsCompiler(**kwargs).compile_variable(designSpaceDoc)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/just/code/git/BlackFoundry/fontra/venv/lib/python3.12/site-packages/ufo2ft/_compilers/baseCompiler.py", line 423, in compile_variable
    self.compile_all_variable_features(
  File "/Users/just/code/git/BlackFoundry/fontra/venv/lib/python3.12/site-packages/ufo2ft/_compilers/baseCompiler.py", line 456, in compile_all_variable_features
    self.compile_variable_features(ufoDoc, ttFont, defaultGlyphset)
  File "/Users/just/code/git/BlackFoundry/fontra/venv/lib/python3.12/site-packages/ufo2ft/_compilers/baseCompiler.py", line 464, in compile_variable_features
    featureCompiler.compile()
  File "/Users/just/code/git/BlackFoundry/fontra/venv/lib/python3.12/site-packages/ufo2ft/featureCompiler.py", line 148, in compile
    self.setupFeatures()
  File "/Users/just/code/git/BlackFoundry/fontra/venv/lib/python3.12/site-packages/ufo2ft/featureCompiler.py", line 439, in setupFeatures
    writer.write(self.designspace, featureFile, compiler=self)
  File "/Users/just/code/git/BlackFoundry/fontra/venv/lib/python3.12/site-packages/ufo2ft/featureWriters/baseFeatureWriter.py", line 146, in write
    self.setContext(font, feaFile, compiler=compiler)
  File "/Users/just/code/git/BlackFoundry/fontra/venv/lib/python3.12/site-packages/ufo2ft/featureWriters/markFeatureWriter.py", line 325, in setContext
    ctx.anchorLists = self._getAnchorLists()
                      ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/just/code/git/BlackFoundry/fontra/venv/lib/python3.12/site-packages/ufo2ft/featureWriters/markFeatureWriter.py", line 359, in _getAnchorLists
    x, y = self._getAnchor(glyphName, anchorName, anchor=anchor)
    ^^^^
TypeError: cannot unpack non-iterable NoneType object
```

This is because _getAnchor() returns None for such a glyph in the `self.context.isVariable` code path.

However, it is trivial to make this work by simply skipping the sources that do not contain the glyph. That's what this PR does.


Patch:
diff --git a/Lib/ufo2ft/featureWriters/baseFeatureWriter.py b/Lib/ufo2ft/featureWriters/baseFeatureWriter.py
index b947e2a..0a5c901 100644
--- a/Lib/ufo2ft/featureWriters/baseFeatureWriter.py
+++ b/Lib/ufo2ft/featureWriters/baseFeatureWriter.py
@@ -418,7 +418,7 @@ class BaseFeatureWriter:
             found = False
             for source in designspace.sources:
                 if glyphName not in source.font:
-                    return None
+                    continue
                 glyph = source.font[glyphName]
                 for anchor in glyph.anchors:
                     if anchor.name == anchorName:


Test patch:
diff --git a/tests/data/TestVarfea-Regular.ufo/fontinfo.plist b/tests/data/TestVarfea-Regular.ufo/fontinfo.plist
index 9f3eadf..4de8ad3 100644
--- a/tests/data/TestVarfea-Regular.ufo/fontinfo.plist
+++ b/tests/data/TestVarfea-Regular.ufo/fontinfo.plist
@@ -10,6 +10,8 @@
     <integer>-400</integer>
     <key>familyName</key>
     <string>TestVarfea</string>
+    <key>guidelines</key>
+    <array/>
     <key>italicAngle</key>
     <integer>0</integer>
     <key>openTypeHeadCreated</key>
@@ -25,11 +27,19 @@
       <integer>600</integer>
       <integer>616</integer>
     </array>
+    <key>postscriptFamilyBlues</key>
+    <array/>
+    <key>postscriptFamilyOtherBlues</key>
+    <array/>
     <key>postscriptOtherBlues</key>
     <array>
       <integer>-416</integer>
       <integer>-400</integer>
     </array>
+    <key>postscriptStemSnapH</key>
+    <array/>
+    <key>postscriptStemSnapV</key>
+    <array/>
     <key>postscriptUnderlinePosition</key>
     <integer>-100</integer>
     <key>postscriptUnderlineThickness</key>
diff --git a/tests/data/TestVarfea-Regular.ufo/glyphs/a.glif b/tests/data/TestVarfea-Regular.ufo/glyphs/a.glif
new file mode 100644
index 0000000..630c31f
--- /dev/null
+++ b/tests/data/TestVarfea-Regular.ufo/glyphs/a.glif
@@ -0,0 +1,14 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="a" format="2">
+  <advance width="500"/>
+  <unicode hex="0061"/>
+  <anchor x="250" y="400" name="top"/>
+  <outline>
+  </outline>
+  <lib>
+    <dict>
+      <key>public.markColor</key>
+      <string>0.5955,0.7484,1,1</string>
+    </dict>
+  </lib>
+</glyph>
diff --git a/tests/data/TestVarfea-Regular.ufo/glyphs/contents.plist b/tests/data/TestVarfea-Regular.ufo/glyphs/contents.plist
index 4572eab..ed6ff7f 100644
--- a/tests/data/TestVarfea-Regular.ufo/glyphs/contents.plist
+++ b/tests/data/TestVarfea-Regular.ufo/glyphs/contents.plist
@@ -2,10 +2,14 @@
 <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
 <plist version="1.0">
   <dict>
+    <key>a</key>
+    <string>a.glif</string>
     <key>alef-ar.fina</key>
     <string>alef-ar.fina.glif</string>
     <key>dotabove-ar</key>
     <string>dotabove-ar.glif</string>
+    <key>gravecmb</key>
+    <string>gravecmb.glif</string>
     <key>peh-ar.init</key>
     <string>peh-ar.init.glif</string>
     <key>peh-ar.init.BRACKET.varAlt01</key>
diff --git a/tests/data/TestVarfea-Regular.ufo/glyphs/gravecmb.glif b/tests/data/TestVarfea-Regular.ufo/glyphs/gravecmb.glif
new file mode 100644
index 0000000..4756761
--- /dev/null
+++ b/tests/data/TestVarfea-Regular.ufo/glyphs/gravecmb.glif
@@ -0,0 +1,14 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="gravecmb" format="2">
+  <advance width="500"/>
+  <unicode hex="0300"/>
+  <anchor x="250" y="400" name="_top"/>
+  <outline>
+  </outline>
+  <lib>
+    <dict>
+      <key>public.markColor</key>
+      <string>0.5955,0.7484,1,1</string>
+    </dict>
+  </lib>
+</glyph>
diff --git a/tests/data/TestVarfea-Regular.ufo/glyphs/layerinfo.plist b/tests/data/TestVarfea-Regular.ufo/glyphs/layerinfo.plist
index e4d188f..a4c24cc 100644
--- a/tests/data/TestVarfea-Regular.ufo/glyphs/layerinfo.plist
+++ b/tests/data/TestVarfea-Regular.ufo/glyphs/layerinfo.plist
@@ -2,6 +2,8 @@
 <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
 <plist version="1.0">
   <dict>
+    <key>color</key>
+    <string>0.5,0,0.5,0.7</string>
     <key>lib</key>
     <dict>
       <key>com.schriftgestaltung.layerId</key>
diff --git a/tests/data/TestVarfea-Regular.ufo/lib.plist b/tests/data/TestVarfea-Regular.ufo/lib.plist
index 0b5415f..68a4f5e 100644
--- a/tests/data/TestVarfea-Regular.ufo/lib.plist
+++ b/tests/data/TestVarfea-Regular.ufo/lib.plist
@@ -58,6 +58,9 @@
       <string>peh-ar.init</string>
       <string>space</string>
       <string>dotabove-ar</string>
+      <string>peh-ar.init.BRACKET.varAlt01</string>
+      <string>a</string>
+      <string>gravecmb</string>
     </array>
     <key>public.postscriptNames</key>
     <dict>
diff --git a/tests/featureWriters/variableFeatureWriter_test.py b/tests/featureWriters/variableFeatureWriter_test.py
index b6be9c9..5ce5577 100644
--- a/tests/featureWriters/variableFeatureWriter_test.py
+++ b/tests/featureWriters/variableFeatureWriter_test.py
@@ -17,6 +17,7 @@ def test_variable_features(FontClass):
     assert dedent("\n" + tmp.getvalue()) == dedent(
         """
         markClass dotabove-ar <anchor (wght=100:100 wght=1000:125) (wght=100:320 wght=1000:416)> @MC_top;
+        markClass gravecmb <anchor 250 400> @MC_top;
 
         lookup kern_Arab {
             lookupflag IgnoreMarks;
@@ -37,6 +38,8 @@ def test_variable_features(FontClass):
             lookup mark2base {
                 pos base alef-ar.fina
                     <anchor (wght=100:211 wght=1000:214) (wght=100:730 wght=1000:797)> mark @MC_top;
+                pos base a
+                    <anchor 250 400> mark @MC_top;
             } mark2base;
 
         } mark;
@@ -46,12 +49,12 @@ def test_variable_features(FontClass):
         } GDEF;
 
         feature curs {
-            lookup curs {
+            lookup curs_rtl {
                 lookupflag RightToLeft IgnoreMarks;
                 pos cursive alef-ar.fina <anchor (wght=100:299 wght=1000:330) (wght=100:97 wght=1000:115)> <anchor NULL>;
                 pos cursive peh-ar.init <anchor NULL> <anchor (wght=100:161 wght=1000:73) (wght=100:54 wght=1000:89)>;
                 pos cursive peh-ar.init.BRACKET.varAlt01 <anchor NULL> <anchor (wght=100:89 wght=1000:73) (wght=100:53 wght=1000:85)>;
-            } curs;
+            } curs_rtl;
 
         } curs;
 """  # noqa: B950