Repository: googlefonts/ufo2ft
Pull request #711 (2023-02-20 20:30:14)

Register lookups for all defined languages
Closes https://github.com/googlefonts/ufo2ft/issues/699.

I'm not sure about:

1. Should we also consider languages defined for `DFLT`? The spec technically allows this but I don't know if it's a valid use case?
2. If the designer defined a language for e.g. the OT tag `tml2`, should it also be inserted for `taml`?


Patch:
diff --git a/Lib/ufo2ft/featureWriters/ast.py b/Lib/ufo2ft/featureWriters/ast.py
index 6fa8825..c94968e 100644
--- a/Lib/ufo2ft/featureWriters/ast.py
+++ b/Lib/ufo2ft/featureWriters/ast.py
@@ -20,15 +20,16 @@ for name in getattr(ast, "__all__", dir(ast)):
 del sys, self, name
 
 
-def getScriptLanguageSystems(feaFile):
+def getScriptLanguageSystems(feaFile, excludeDflt=True):
     """Return dictionary keyed by Unicode script code containing lists of
-    (OT_SCRIPT_TAG, [OT_LANGUAGE_TAG, ...]) tuples (excluding "DFLT").
+    (OT_SCRIPT_TAG, [OT_LANGUAGE_TAG, ...]) tuples (excluding "DFLT" by default,
+    unless excludeDflt is False).
     """
     languagesByScript = collections.OrderedDict()
     for ls in [
         st for st in feaFile.statements if isinstance(st, ast.LanguageSystemStatement)
     ]:
-        if ls.script == "DFLT":
+        if ls.script == "DFLT" and excludeDflt:
             continue
         languagesByScript.setdefault(ls.script, []).append(ls.language)
 
diff --git a/Lib/ufo2ft/featureWriters/kernFeatureWriter.py b/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
index e451a4d..7774d5f 100644
--- a/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
+++ b/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
@@ -198,6 +198,17 @@ class KernFeatureWriter(BaseFeatureWriter):
         ctx.gdefClasses = self.getGDEFGlyphClasses()
         ctx.glyphSet = self.getOrderedGlyphSet()
 
+        # Remember which languages are defined for which OT tag, as all
+        # generated kerning needs to be registered for the script's `dflt`
+        # language, but also all those the designer defined manually. Otherwise,
+        # setting any language for a script would deactivate kerning.
+        feaLanguagesByScript = ast.getScriptLanguageSystems(feaFile, excludeDflt=False)
+        ctx.feaLanguagesByScript = {
+            otTag: languages
+            for _, languageSystems in feaLanguagesByScript.items()
+            for otTag, languages in languageSystems
+        }
+
         # TODO: Also include substitution information from Designspace rules to
         # correctly set the scripts of variable substitution glyphs, maybe add
         # `glyphUnicodeMapping: dict[str, int] | None` to `BaseFeatureCompiler`?
@@ -461,27 +472,31 @@ class KernFeatureWriter(BaseFeatureWriter):
 
     def _makeFeatureBlocks(self, lookups):
         features = {}
+        feaLanguagesByScript = self.context.feaLanguagesByScript
         if "kern" in self.context.todo:
             kern = ast.FeatureBlock("kern")
-            self._registerLookups(kern, lookups)
+            self._registerLookups(kern, lookups, feaLanguagesByScript)
             if kern.statements:
                 features["kern"] = kern
         if "dist" in self.context.todo:
             dist = ast.FeatureBlock("dist")
-            self._registerLookups(dist, lookups)
+            self._registerLookups(dist, lookups, feaLanguagesByScript)
             if dist.statements:
                 features["dist"] = dist
         return features
 
     @staticmethod
     def _registerLookups(
-        feature: ast.FeatureBlock, lookups: dict[str, dict[str, ast.LookupBlock]]
+        feature: ast.FeatureBlock,
+        lookups: dict[str, dict[str, ast.LookupBlock]],
+        feaLanguagesByScript: Mapping[str, list[str]],
     ) -> None:
         # Ensure we have kerning for pure common script runs (e.g. ">1")
         isKernBlock = feature.name == "kern"
         if isKernBlock and COMMON_SCRIPT in lookups:
+            languages = feaLanguagesByScript.get("DFLT", ["dflt"])
             ast.addLookupReferences(
-                feature, lookups[COMMON_SCRIPT].values(), "DFLT", ["dflt"]
+                feature, lookups[COMMON_SCRIPT].values(), "DFLT", languages
             )
 
         # Feature blocks use script tags to distinguish what to run for a
@@ -512,11 +527,11 @@ class KernFeatureWriter(BaseFeatureWriter):
                     if dfltScript in lookups:
                         lookupsForThisScript.extend(lookups[dfltScript].values())
                 lookupsForThisScript.extend(lookups[script].values())
-                # NOTE: We always use the `dflt` language because there is no
-                # language-specific kerning to be derived from UFO (kerning.plist)
-                # sources and we are independent of what's going on in the rest of
-                # the features.fea file.
-                ast.addLookupReferences(feature, lookupsForThisScript, tag, ["dflt"])
+                # Register the lookups for all languages defined in the feature
+                # file for the script, otherwise kerning is not applied if any
+                # language is set at all.
+                languages = feaLanguagesByScript.get(tag, ["dflt"])
+                ast.addLookupReferences(feature, lookupsForThisScript, tag, languages)
 
 
 def splitKerning(pairs, glyphScripts):


Test patch:
diff --git a/tests/featureWriters/kernFeatureWriter_test.py b/tests/featureWriters/kernFeatureWriter_test.py
index 5689664..726d6c2 100644
--- a/tests/featureWriters/kernFeatureWriter_test.py
+++ b/tests/featureWriters/kernFeatureWriter_test.py
@@ -777,11 +777,13 @@ class KernFeatureWriterTest(FeatureWriterTest):
                 language dflt;
                 lookup kern_Default;
                 lookup kern_Arab;
+                language URD;
 
                 script latn;
                 language dflt;
                 lookup kern_Default;
                 lookup kern_Latn;
+                language TRK;
             } kern;
             """
         )
@@ -901,12 +903,14 @@ class KernFeatureWriterTest(FeatureWriterTest):
                 lookup kern_Default;
                 lookup kern_Arab;
                 lookup kern_Arab_marks;
+                language URD;
 
                 script latn;
                 language dflt;
                 lookup kern_Default;
                 lookup kern_Latn;
                 lookup kern_Latn_marks;
+                language TRK;
             } kern;
             """
         )
@@ -988,6 +992,7 @@ class KernFeatureWriterTest(FeatureWriterTest):
                 language dflt;
                 lookup kern_Arab;
                 lookup kern_Arab_marks;
+                language ARA;
             } kern;
             """
         )
@@ -1988,6 +1993,51 @@ def test_hyphenated_duplicates(FontClass):
     )
 
 
+def test_dflt_language(FontClass):
+    """Check that languages defined for the special DFLT script are registered
+    as well."""
+
+    glyphs = {"a": ord("a"), "comma": ord(",")}
+    groups = {}
+    kerning = {("a", "a"): 1, ("comma", "comma"): 2}
+    features = """
+            languagesystem DFLT dflt;
+            languagesystem DFLT ZND;
+            languagesystem latn dflt;
+            languagesystem latn ANG;
+    """
+    ufo = makeUFO(FontClass, glyphs, groups, kerning, features)
+
+    newFeatures = KernFeatureWriterTest.writeFeatures(ufo)
+
+    assert dedent(str(newFeatures)) == dedent(
+        """
+        lookup kern_Latn {
+            lookupflag IgnoreMarks;
+            pos a a 1;
+        } kern_Latn;
+
+        lookup kern_Default {
+            lookupflag IgnoreMarks;
+            pos comma comma 2;
+        } kern_Default;
+
+        feature kern {
+            script DFLT;
+            language dflt;
+            lookup kern_Default;
+            language ZND;
+
+            script latn;
+            language dflt;
+            lookup kern_Default;
+            lookup kern_Latn;
+            language ANG;
+        } kern;
+        """
+    )
+
+
 if __name__ == "__main__":
     import sys