Repository: googlefonts/ufo2ft
Pull request #361 (2020-02-06 18:58:21)

drop kern pairs with mixed bidi type
... as kerning across direction runs doesn't work in current implementations.
Also, we can't guess the designer's intention as to the "true" direction of these kern pairs with mixed bidi. So for now we warn and prune them. Maybe revise once we'll have defined that new "visual" hkerning.plist.

PTAL @khaledhosny 

Incidentally also this fixes https://github.com/googlefonts/ufo2ft/issues/341 because it removes the chance that feaLib will produce two PairPos2 subtables with overlapping coverages.
The split occurs when, within the same FEA lookup block definition, there are mutliple pos rules with different value record format (format A is used for LTR kern pairs [modifying only XAdvance of first glyph], format B for RTL kern pairs[modifying both XAdvance and XPlacement]). 
Instead of splitting, feaLib should fill with 0 the missing values then figure out a heuristic to store the matrix optimally (see https://github.com/fonttools/fonttools/issues/1731).






Patch:
diff --git a/Lib/ufo2ft/featureWriters/kernFeatureWriter.py b/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
index 644b207..76a95a0 100644
--- a/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
+++ b/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
@@ -99,7 +99,7 @@ DIST_ENABLED_SCRIPTS = {
     "Gong",  # Gunjala Gondi
     "Maka",  # Makasar
     # Unicode-12.0 additions
-    "Nand", # Nandinagari
+    "Nand",  # Nandinagari
 }
 
 
@@ -115,21 +115,20 @@ def unicodeScriptDirection(uv):
     return unicodedata.script_horizontal_direction(sc)
 
 
-STRONG_LTR_BIDI_TYPE = "L"
-STRONG_RTL_BIDI_TYPES = {"R", "AL"}
-LTR_NUMBER_BIDI_TYPES = {"AN", "EN"}
+RTL_BIDI_TYPES = {"R", "AL"}
+LTR_BIDI_TYPES = {"L", "AN", "EN"}
 
 
 def unicodeBidiType(uv):
-    # return "R", "L", "N" (for numbers), or None for everything else
+    """Return "R" for characters with RTL direction, or "L" for LTR (whether
+    'strong' or 'weak'), or None for neutral direction.
+    """
     char = unichr(uv)
     bidiType = unicodedata.bidirectional(char)
-    if bidiType in STRONG_RTL_BIDI_TYPES:
+    if bidiType in RTL_BIDI_TYPES:
         return "R"
-    elif bidiType == STRONG_LTR_BIDI_TYPE:
+    elif bidiType in LTR_BIDI_TYPES:
         return "L"
-    elif bidiType in LTR_NUMBER_BIDI_TYPES:
-        return "N"
     else:
         return None
 
@@ -367,7 +366,7 @@ class KernFeatureWriter(BaseFeatureWriter):
     def _makePairPosRule(pair, rtl=False):
         enumerated = pair.firstIsClass ^ pair.secondIsClass
         value = otRound(pair.value)
-        if rtl and "N" in pair.bidiTypes:
+        if rtl and "L" in pair.bidiTypes:
             # numbers are always shaped LTR even in RTL scripts
             rtl = False
         valuerecord = ast.ValueRecord(
@@ -424,10 +423,15 @@ class KernFeatureWriter(BaseFeatureWriter):
         if shouldSplit:
             # make one DFLT lookup with script-agnostic characters, and two
             # LTR/RTL lookups excluding pairs from the opposite group.
-            # We drop kerning pairs with ambiguous direction.
+            # We drop kerning pairs with ambiguous direction: i.e. those containing
+            # glyphs from scripts with different overall horizontal direction, or
+            # glyphs with incompatible bidirectional type (e.g. arabic letters vs
+            # arabic numerals).
             pairs = []
             for pair in self.context.kerning.pairs:
-                if "RTL" in pair.directions and "LTR" in pair.directions:
+                if ("RTL" in pair.directions and "LTR" in pair.directions) or (
+                    "R" in pair.bidiTypes and "L" in pair.bidiTypes
+                ):
                     self.log.warning(
                         "skipped kern pair with ambiguous direction: %r", pair
                     )


Test patch:
diff --git a/tests/featureWriters/kernFeatureWriter_test.py b/tests/featureWriters/kernFeatureWriter_test.py
index 7b0497c..c4793b2 100644
--- a/tests/featureWriters/kernFeatureWriter_test.py
+++ b/tests/featureWriters/kernFeatureWriter_test.py
@@ -874,9 +874,20 @@ class KernFeatureWriterTest(FeatureWriterTest):
 
         ufo = FontClass()
         ufo.newGlyph("A").unicode = 0x41
+        ufo.newGlyph("one").unicode = 0x31
+        ufo.newGlyph("yod-hb").unicode = 0x5D9
         ufo.newGlyph("reh-ar").unicode = 0x631
+        ufo.newGlyph("one-ar").unicode = 0x661
         ufo.newGlyph("bar").unicodes = [0x73, 0x627]
-        ufo.kerning.update({("bar", "bar"): 1, ("bar", "A"): 2, ("reh-ar", "A"): 3})
+        ufo.kerning.update(
+            {
+                ("bar", "bar"): 1,
+                ("bar", "A"): 2,
+                ("reh-ar", "A"): 3,
+                ("reh-ar", "one-ar"): 4,
+                ("yod-hb", "one"): 5,
+            }
+        )
         ufo.features.text = dedent(
             """\
             languagesystem DFLT dflt;
@@ -890,7 +901,7 @@ class KernFeatureWriterTest(FeatureWriterTest):
             generated = self.writeFeatures(ufo)
 
         assert not generated
-        assert len(caplog.records) == 3
+        assert len(caplog.records) == 5
         assert "skipped kern pair with ambiguous direction" in caplog.text
 
     def test_kern_RTL_and_DFLT_numbers(self, FontClass):