Repository: googlefonts/ufo2ft
Pull request #713 (2023-02-28 15:36:25)

Split mark and base kerning pairs
Split kerning pairs of bases against marks into

1. base-to-base only pairs, for the base lookup
2. mark-to-base, base-to-mark and mark-to-mark pairs, for the marks lookup

which should preserve the semantics of kerning exceptions (pairs modifying the effect of other pairs).

Closes https://github.com/googlefonts/ufo2ft/issues/706.

Validated with https://github.com/daltonmaag/kerning-validator/tree/16927fc10b1f4d83d0f2b029b4c1b16a8d5c7efe. It fixes the problems in Noto Serif Tamil.


Patch:
diff --git a/Lib/ufo2ft/featureWriters/kernFeatureWriter.py b/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
index 7774d5f..d24e64f 100644
--- a/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
+++ b/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
@@ -393,16 +393,53 @@ class KernFeatureWriter(BaseFeatureWriter):
             self._makeSplitScriptKernLookups(lookups, pairs)
         return lookups
 
-    def _splitBaseAndMarkPairs(self, pairs, marks):
-        basePairs, markPairs = [], []
-        if marks:
-            for pair in pairs:
-                if any(glyph in marks for glyph in pair.glyphs):
-                    markPairs.append(pair)
+    def _splitBaseAndMarkPairs(
+        self, pairs: list[KerningPair], marks: set[str]
+    ) -> tuple[list[KerningPair], list[KerningPair]]:
+        if not marks:
+            return list(pairs), []
+
+        basePairs: list[KerningPair] = []
+        markPairs: list[KerningPair] = []
+        for pair in pairs:
+            # Disentangle kerning between bases and marks by splitting a pair
+            # into a list of base-to-base pairs (basePairs) and a list of
+            # base-to-mark, mark-to-base and mark-to-mark pairs (markPairs).
+            # This ensures that "kerning exceptions" (a kerning pair modifying
+            # the effect of another) work as intended because these related
+            # pairs end up in the same list together.
+            side1Bases: tuple[str, ...] | str | None = None
+            side1Marks: tuple[str, ...] | str | None = None
+            if pair.firstIsClass:
+                side1Bases = tuple(glyph for glyph in pair.side1 if glyph not in marks)
+                side1Marks = tuple(glyph for glyph in pair.side1 if glyph in marks)
+            else:
+                if pair.side1 in marks:
+                    side1Marks = pair.side1
                 else:
-                    basePairs.append(pair)
-        else:
-            basePairs[:] = pairs
+                    side1Bases = pair.side1
+
+            side2Bases: tuple[str, ...] | str | None = None
+            side2Marks: tuple[str, ...] | str | None = None
+            if pair.secondIsClass:
+                side2Bases = tuple(glyph for glyph in pair.side2 if glyph not in marks)
+                side2Marks = tuple(glyph for glyph in pair.side2 if glyph in marks)
+            else:
+                if pair.side2 in marks:
+                    side2Marks = pair.side2
+                else:
+                    side2Bases = pair.side2
+
+            if side1Bases and side2Bases:  # base-to-base
+                basePairs.append(KerningPair(side1Bases, side2Bases, value=pair.value))
+
+            if side1Bases and side2Marks:  # base-to-mark
+                markPairs.append(KerningPair(side1Bases, side2Marks, value=pair.value))
+            if side1Marks and side2Bases:  # mark-to-base
+                markPairs.append(KerningPair(side1Marks, side2Bases, value=pair.value))
+            if side1Marks and side2Marks:  # mark-to-mark
+                markPairs.append(KerningPair(side1Marks, side2Marks, value=pair.value))
+
         return basePairs, markPairs
 
     def _makeSplitScriptKernLookups(self, lookups, pairs, ignoreMarks=True, suffix=""):


Test patch:
diff --git a/tests/featureWriters/kernFeatureWriter_test.py b/tests/featureWriters/kernFeatureWriter_test.py
index 726d6c2..aa5eca7 100644
--- a/tests/featureWriters/kernFeatureWriter_test.py
+++ b/tests/featureWriters/kernFeatureWriter_test.py
@@ -1956,6 +1956,76 @@ def test_defining_classdefs(FontClass):
     )
 
 
+def test_mark_base_kerning(FontClass):
+    """Check that kerning of bases against marks is correctly split into
+    base-only and mixed-mark-and-base lookups, to preserve the semantics of
+    kerning exceptions (pairs modifying the effect of other pairs)."""
+
+    glyphs = {
+        "aa-tamil": 0x0B86,
+        "va-tamil": 0x0BB5,
+        "aulengthmark-tamil": 0x0BD7,
+    }
+    groups = {
+        # Each group is a mix of mark and base glyph.
+        "public.kern1.e-tamil": ["aulengthmark-tamil", "va-tamil"],
+        "public.kern2.e-tamil": ["aulengthmark-tamil", "va-tamil"],
+    }
+    kerning = {
+        ("aa-tamil", "va-tamil"): -20,
+        ("aa-tamil", "public.kern2.e-tamil"): -35,
+        ("va-tamil", "aa-tamil"): -20,
+        ("public.kern1.e-tamil", "aa-tamil"): -35,
+        ("aulengthmark-tamil", "aulengthmark-tamil"): -200,
+        ("public.kern1.e-tamil", "public.kern2.e-tamil"): -100,
+    }
+    ufo = makeUFO(FontClass, glyphs, groups, kerning)
+    ufo.lib["public.openTypeCategories"] = {
+        "aulengthmark-tamil": "mark",
+    }
+
+    newFeatures = KernFeatureWriterTest.writeFeatures(ufo)
+
+    assert dedent(str(newFeatures)) == dedent(
+        """\
+        @kern1.Taml.etamil = [va-tamil];
+        @kern1.Taml.etamil_1 = [aulengthmark-tamil];
+        @kern2.Taml.etamil = [va-tamil];
+        @kern2.Taml.etamil_1 = [aulengthmark-tamil];
+
+        lookup kern_Taml {
+            lookupflag IgnoreMarks;
+            pos aa-tamil va-tamil -20;
+            pos va-tamil aa-tamil -20;
+            enum pos aa-tamil @kern2.Taml.etamil -35;
+            enum pos @kern1.Taml.etamil aa-tamil -35;
+            pos @kern1.Taml.etamil @kern2.Taml.etamil -100;
+        } kern_Taml;
+
+        lookup kern_Taml_marks {
+            pos aulengthmark-tamil aulengthmark-tamil -200;
+            enum pos aa-tamil @kern2.Taml.etamil_1 -35;
+            enum pos @kern1.Taml.etamil_1 aa-tamil -35;
+            pos @kern1.Taml.etamil_1 @kern2.Taml.etamil_1 -100;
+            pos @kern1.Taml.etamil_1 @kern2.Taml.etamil -100;
+            pos @kern1.Taml.etamil @kern2.Taml.etamil_1 -100;
+        } kern_Taml_marks;
+
+        feature dist {
+            script tml2;
+            language dflt;
+            lookup kern_Taml;
+            lookup kern_Taml_marks;
+
+            script taml;
+            language dflt;
+            lookup kern_Taml;
+            lookup kern_Taml_marks;
+        } dist;
+        """
+    )
+
+
 def test_hyphenated_duplicates(FontClass):
     """Check that kerning group names are kept separate even if their sanitized
     names are the same."""