Repository: googlefonts/ufo2ft
Pull request #720 (2023-03-03 09:33:27)

Don't skip over spacing marks when kerning
Consider the following font:

<img width="300" alt="Screenshot 2023-03-03 at 09 24 18" src="https://user-images.githubusercontent.com/106728/222682784-43bcaecc-4cda-497a-89df-fc66bd605b9d.png">

We want to kern between ka-deva and ra-deva:

<img width="466" alt="Screenshot 2023-03-03 at 09 24 55" src="https://user-images.githubusercontent.com/106728/222682939-96be7a4d-cb81-4e3a-9429-35d1c9ad750f.png">

But (to do certain bits of OpenType magic) we make highspacingdot-deva a "spacing combining" mark (spacing mark). This is so we can filter it out when doing conjuncts or something like that. (This is slightly contrived, but an analogous situation *does* exist for many Indic scripts.)

We also want kerning between ka-deva and the mark, so we make a base-mark kern:

<img width="351" alt="Screenshot 2023-03-03 at 09 27 25" src="https://user-images.githubusercontent.com/106728/222683473-7bdde97f-43e6-4b4b-afda-79b4d03f92c4.png">

But because it's a mark class in GDEF, when we apply base-base kerning, this gets skipped. So with a sequence like `ka-deva|highspacingdot-deva|ra-deva`, the result, with current ufo2ft, is:

* kern_Deva will IgnoreMarks and kern `ka-deva|ra-deva` -250
* kern_Deva_marks will kern `ka-deva|highspacingdot-deva` -150.

![shape](https://user-images.githubusercontent.com/106728/222683943-f9b6cd9f-f80b-4080-989e-7872228dfa56.png)

Instead what we need to do is, rather than filtering out all marks, only filter out non-spacing marks, allowing spacing marks to break the base-base kern, so that only base-mark kerning is applied:

![shape](https://user-images.githubusercontent.com/106728/222684477-d1082dc5-cb85-4ff1-9883-fa5d49021bde.png)



Patch:
diff --git a/Lib/ufo2ft/featureWriters/kernFeatureWriter.py b/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
index d24e64f..b4c42a0 100644
--- a/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
+++ b/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
@@ -361,7 +361,25 @@ class KernFeatureWriter(BaseFeatureWriter):
     def _makeKerningLookup(self, name, ignoreMarks=True):
         lookup = ast.LookupBlock(name)
         if ignoreMarks and self.options.ignoreMarks:
-            lookup.statements.append(ast.makeLookupFlag("IgnoreMarks"))
+            # We only want to filter the spacing marks
+            marks = self.context.gdefClasses.mark
+            spacing = []
+            if marks:
+                spacing = [mark for mark in marks if self.context.font[mark].width != 0]
+            if not spacing:
+                # Simple case, there are no spacing ("Spacing Combining") marks,
+                # do what we've always done.
+                lookup.statements.append(ast.makeLookupFlag("IgnoreMarks"))
+            else:
+                # We want spacing marks to block kerns.
+                className = "MFS_%s" % name
+                filteringClass = ast.makeGlyphClassDefinitions(
+                    {className: spacing}, feaFile=self.context.feaFile
+                )[className]
+                lookup.statements.append(filteringClass)
+                lookup.statements.append(
+                    ast.makeLookupFlag(markFilteringSet=filteringClass)
+                )
         return lookup
 
     def knownScriptsPerCodepoint(self, uv: int) -> set[str]:


Test patch:
diff --git a/tests/data/SpacingCombiningTest-Regular.ufo/fontinfo.plist b/tests/data/SpacingCombiningTest-Regular.ufo/fontinfo.plist
new file mode 100644
index 0000000..8d81514
--- /dev/null
+++ b/tests/data/SpacingCombiningTest-Regular.ufo/fontinfo.plist
@@ -0,0 +1,52 @@
+<?xml version='1.0' encoding='UTF-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>ascender</key>
+    <integer>800</integer>
+    <key>capHeight</key>
+    <integer>700</integer>
+    <key>descender</key>
+    <integer>-200</integer>
+    <key>familyName</key>
+    <string>Spacing Combining Test</string>
+    <key>italicAngle</key>
+    <integer>0</integer>
+    <key>openTypeHeadCreated</key>
+    <string>2023/03/03 08:50:23</string>
+    <key>openTypeOS2Type</key>
+    <array>
+      <integer>3</integer>
+    </array>
+    <key>postscriptBlueValues</key>
+    <array>
+      <integer>-16</integer>
+      <integer>0</integer>
+      <integer>500</integer>
+      <integer>516</integer>
+      <integer>700</integer>
+      <integer>716</integer>
+      <integer>800</integer>
+      <integer>816</integer>
+    </array>
+    <key>postscriptOtherBlues</key>
+    <array>
+      <integer>-216</integer>
+      <integer>-200</integer>
+    </array>
+    <key>postscriptUnderlinePosition</key>
+    <integer>-100</integer>
+    <key>postscriptUnderlineThickness</key>
+    <integer>50</integer>
+    <key>styleName</key>
+    <string>Regular</string>
+    <key>unitsPerEm</key>
+    <integer>1000</integer>
+    <key>versionMajor</key>
+    <integer>1</integer>
+    <key>versionMinor</key>
+    <integer>0</integer>
+    <key>xHeight</key>
+    <integer>500</integer>
+  </dict>
+</plist>
diff --git a/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/contents.plist b/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/contents.plist
new file mode 100644
index 0000000..d2f0c56
--- /dev/null
+++ b/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/contents.plist
@@ -0,0 +1,14 @@
+<?xml version='1.0' encoding='UTF-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>highspacingdot-deva</key>
+    <string>highspacingdot-deva.glif</string>
+    <key>ka-deva</key>
+    <string>ka-deva.glif</string>
+    <key>ra-deva</key>
+    <string>ra-deva.glif</string>
+    <key>space</key>
+    <string>space.glif</string>
+  </dict>
+</plist>
diff --git a/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/highspacingdot-deva.glif b/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/highspacingdot-deva.glif
new file mode 100644
index 0000000..7281ce1
--- /dev/null
+++ b/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/highspacingdot-deva.glif
@@ -0,0 +1,31 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="highspacingdot-deva" format="2">
+  <advance width="317"/>
+  <unicode hex="0971"/>
+  <outline>
+    <contour>
+      <point x="158" y="386" type="curve" smooth="yes"/>
+      <point x="223" y="386"/>
+      <point x="278" y="440"/>
+      <point x="278" y="508" type="curve" smooth="yes"/>
+      <point x="278" y="573"/>
+      <point x="223" y="627"/>
+      <point x="158" y="627" type="curve" smooth="yes"/>
+      <point x="92" y="627"/>
+      <point x="37" y="573"/>
+      <point x="37" y="508" type="curve" smooth="yes"/>
+      <point x="37" y="440"/>
+      <point x="92" y="386"/>
+    </contour>
+  </outline>
+  <lib>
+    <dict>
+      <key>com.schriftgestaltung.Glyphs.category</key>
+      <string>Mark</string>
+      <key>com.schriftgestaltung.Glyphs.lastChange</key>
+      <string>2023/03/03 09:07:06</string>
+      <key>com.schriftgestaltung.Glyphs.subCategory</key>
+      <string>Spacing Combining</string>
+    </dict>
+  </lib>
+</glyph>
diff --git a/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/ka-deva.glif b/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/ka-deva.glif
new file mode 100644
index 0000000..b05ef13
--- /dev/null
+++ b/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/ka-deva.glif
@@ -0,0 +1,18 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="ka-deva" format="2">
+  <advance width="623"/>
+  <unicode hex="0915"/>
+  <outline>
+    <contour>
+      <point x="323" y="500" type="line"/>
+      <point x="73" y="0" type="line"/>
+      <point x="573" y="0" type="line"/>
+    </contour>
+  </outline>
+  <lib>
+    <dict>
+      <key>com.schriftgestaltung.Glyphs.lastChange</key>
+      <string>2023/03/03 09:04:18</string>
+    </dict>
+  </lib>
+</glyph>
diff --git a/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/layerinfo.plist b/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/layerinfo.plist
new file mode 100644
index 0000000..2cac07e
--- /dev/null
+++ b/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/layerinfo.plist
@@ -0,0 +1,19 @@
+<?xml version='1.0' encoding='UTF-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>lib</key>
+    <dict>
+      <key>com.schriftgestaltung.layerId</key>
+      <string>m01</string>
+      <key>com.schriftgestaltung.layerOrderInGlyph.highspacingdot-deva</key>
+      <integer>0</integer>
+      <key>com.schriftgestaltung.layerOrderInGlyph.ka-deva</key>
+      <integer>0</integer>
+      <key>com.schriftgestaltung.layerOrderInGlyph.ra-deva</key>
+      <integer>0</integer>
+      <key>com.schriftgestaltung.layerOrderInGlyph.space</key>
+      <integer>0</integer>
+    </dict>
+  </dict>
+</plist>
diff --git a/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/ra-deva.glif b/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/ra-deva.glif
new file mode 100644
index 0000000..a508a70
--- /dev/null
+++ b/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/ra-deva.glif
@@ -0,0 +1,18 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="ra-deva" format="2">
+  <advance width="600"/>
+  <unicode hex="0930"/>
+  <outline>
+    <contour>
+      <point x="323" y="0" type="line"/>
+      <point x="573" y="500" type="line"/>
+      <point x="73" y="500" type="line"/>
+    </contour>
+  </outline>
+  <lib>
+    <dict>
+      <key>com.schriftgestaltung.Glyphs.lastChange</key>
+      <string>2023/03/03 09:04:26</string>
+    </dict>
+  </lib>
+</glyph>
diff --git a/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/space.glif b/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/space.glif
new file mode 100644
index 0000000..c05cd73
--- /dev/null
+++ b/tests/data/SpacingCombiningTest-Regular.ufo/glyphs/space.glif
@@ -0,0 +1,7 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="space" format="2">
+  <advance width="200"/>
+  <unicode hex="0020"/>
+  <outline>
+  </outline>
+</glyph>
diff --git a/tests/data/SpacingCombiningTest-Regular.ufo/kerning.plist b/tests/data/SpacingCombiningTest-Regular.ufo/kerning.plist
new file mode 100644
index 0000000..e06e719
--- /dev/null
+++ b/tests/data/SpacingCombiningTest-Regular.ufo/kerning.plist
@@ -0,0 +1,23 @@
+<?xml version='1.0' encoding='UTF-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>highspacingdot-deva</key>
+    <dict>
+      <key>ka-deva</key>
+      <integer>-200</integer>
+    </dict>
+    <key>ka-deva</key>
+    <dict>
+      <key>highspacingdot-deva</key>
+      <integer>-150</integer>
+      <key>ra-deva</key>
+      <integer>-250</integer>
+    </dict>
+    <key>ra-deva</key>
+    <dict>
+      <key>ka-deva</key>
+      <integer>-250</integer>
+    </dict>
+  </dict>
+</plist>
diff --git a/tests/data/SpacingCombiningTest-Regular.ufo/layercontents.plist b/tests/data/SpacingCombiningTest-Regular.ufo/layercontents.plist
new file mode 100644
index 0000000..b9c1a4f
--- /dev/null
+++ b/tests/data/SpacingCombiningTest-Regular.ufo/layercontents.plist
@@ -0,0 +1,10 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+  <array>
+    <array>
+      <string>public.default</string>
+      <string>glyphs</string>
+    </array>
+  </array>
+</plist>
diff --git a/tests/data/SpacingCombiningTest-Regular.ufo/lib.plist b/tests/data/SpacingCombiningTest-Regular.ufo/lib.plist
new file mode 100644
index 0000000..a2c97f5
--- /dev/null
+++ b/tests/data/SpacingCombiningTest-Regular.ufo/lib.plist
@@ -0,0 +1,76 @@
+<?xml version='1.0' encoding='UTF-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>com.github.googlei18n.ufo2ft.filters</key>
+    <array>
+      <dict>
+        <key>name</key>
+        <string>eraseOpenCorners</string>
+        <key>namespace</key>
+        <string>glyphsLib.filters</string>
+        <key>pre</key>
+        <true/>
+      </dict>
+    </array>
+    <key>com.schriftgestaltung.appVersion</key>
+    <string>3177</string>
+    <key>com.schriftgestaltung.customParameter.GSFont.DisplayStrings</key>
+    <array>
+      <string>कॱर</string>
+    </array>
+    <key>com.schriftgestaltung.customParameter.GSFont.disablesAutomaticAlignment</key>
+    <false/>
+    <key>com.schriftgestaltung.customParameter.GSFont.useNiceNames</key>
+    <integer>1</integer>
+    <key>com.schriftgestaltung.customParameter.GSFontMaster.customValue</key>
+    <integer>0</integer>
+    <key>com.schriftgestaltung.customParameter.GSFontMaster.customValue1</key>
+    <integer>0</integer>
+    <key>com.schriftgestaltung.customParameter.GSFontMaster.customValue2</key>
+    <integer>0</integer>
+    <key>com.schriftgestaltung.customParameter.GSFontMaster.customValue3</key>
+    <integer>0</integer>
+    <key>com.schriftgestaltung.customParameter.GSFontMaster.iconName</key>
+    <string></string>
+    <key>com.schriftgestaltung.customParameter.GSFontMaster.weightValue</key>
+    <integer>100</integer>
+    <key>com.schriftgestaltung.customParameter.GSFontMaster.widthValue</key>
+    <integer>100</integer>
+    <key>com.schriftgestaltung.fontMasterID</key>
+    <string>m01</string>
+    <key>com.schriftgestaltung.fontMasterOrder</key>
+    <integer>0</integer>
+    <key>com.schriftgestaltung.keyboardIncrement</key>
+    <integer>1</integer>
+    <key>com.schriftgestaltung.weight</key>
+    <string>Regular</string>
+    <key>com.schriftgestaltung.weightValue</key>
+    <integer>100</integer>
+    <key>com.schriftgestaltung.width</key>
+    <string>Regular</string>
+    <key>com.schriftgestaltung.widthValue</key>
+    <integer>100</integer>
+    <key>public.glyphOrder</key>
+    <array>
+      <string>ka-deva</string>
+      <string>ra-deva</string>
+      <string>space</string>
+      <string>highspacingdot-deva</string>
+    </array>
+    <key>public.openTypeCategories</key>
+    <dict>
+      <key>highspacingdot-deva</key>
+      <string>mark</string>
+    </dict>
+    <key>public.postscriptNames</key>
+    <dict>
+      <key>highspacingdot-deva</key>
+      <string>uni0971</string>
+      <key>ka-deva</key>
+      <string>uni0915</string>
+      <key>ra-deva</key>
+      <string>uni0930</string>
+    </dict>
+  </dict>
+</plist>
diff --git a/tests/data/SpacingCombiningTest-Regular.ufo/metainfo.plist b/tests/data/SpacingCombiningTest-Regular.ufo/metainfo.plist
new file mode 100644
index 0000000..7b8b34a
--- /dev/null
+++ b/tests/data/SpacingCombiningTest-Regular.ufo/metainfo.plist
@@ -0,0 +1,10 @@
+<?xml version='1.0' encoding='UTF-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>creator</key>
+    <string>com.github.fonttools.ufoLib</string>
+    <key>formatVersion</key>
+    <integer>3</integer>
+  </dict>
+</plist>
diff --git a/tests/featureWriters/kernFeatureWriter_test.py b/tests/featureWriters/kernFeatureWriter_test.py
index aa5eca7..d98a0a1 100644
--- a/tests/featureWriters/kernFeatureWriter_test.py
+++ b/tests/featureWriters/kernFeatureWriter_test.py
@@ -1,4 +1,5 @@
 import logging
+import os
 from textwrap import dedent
 
 import pytest
@@ -1259,6 +1260,39 @@ class KernFeatureWriterTest(FeatureWriterTest):
             """
         )
 
+    def test_skip_spacing_marks(self, FontClass):
+        dirname = os.path.dirname(os.path.dirname(__file__))
+        fontPath = os.path.join(dirname, "data", "SpacingCombiningTest-Regular.ufo")
+        testufo = FontClass(fontPath)
+        generated = self.writeFeatures(testufo)
+        assert dedent(str(generated)) == dedent(
+            """\
+            lookup kern_Deva {
+                @MFS_kern_Deva = [highspacingdot-deva];
+                lookupflag UseMarkFilteringSet @MFS_kern_Deva;
+                pos ka-deva ra-deva -250;
+                pos ra-deva ka-deva -250;
+            } kern_Deva;
+
+            lookup kern_Deva_marks {
+                pos highspacingdot-deva ka-deva -200;
+                pos ka-deva highspacingdot-deva -150;
+            } kern_Deva_marks;
+
+            feature dist {
+                script dev2;
+                language dflt;
+                lookup kern_Deva;
+                lookup kern_Deva_marks;
+
+                script deva;
+                language dflt;
+                lookup kern_Deva;
+                lookup kern_Deva_marks;
+            } dist;
+            """
+        )
+
 
 def test_kern_split_multi_glyph_class(FontClass):
     glyphs = {