Merge pull request #962 from googlefonts/fix-dangling-dist

Fix dangling lookups when feature block already exists

Author: anthrotype

Lib/ufo2ft/featureWriters/kernFeatureWriter.py

@@ -297,18 +297,32 @@ def _write(self):
         classDefs = self.context.kerning.classDefs
         newClassDefs = [c for _, c in sorted(classDefs.items())]
 
+        featureBlocks = [features[tag] for tag in ["kern", "dist"] if tag in features]
+
+        # Collect only the lookups that are referenced by the features we're
+        # writing, to avoid inserting unreferenced/dangling lookups when a
+        # feature block already exists in the feature file.
+        referencedLookups = {
+            statement.lookup
+            for feature in featureBlocks
+            for statement in feature.statements
+            if isinstance(statement, ast.LookupReferenceStatement)
+        }
+
         lookupGroups = []
         for _, lookupGroup in sorted(lookups.items()):
             lookupGroups.extend(
-                lkp for lkp in lookupGroup.values() if lkp not in lookupGroups
+                lkp
+                for lkp in lookupGroup.values()
+                if lkp in referencedLookups and lkp not in lookupGroups
             )
 
         # NOTE: We don't write classDefs because we literalise all classes.
         self._insert(
             feaFile=feaFile,
             classDefs=newClassDefs,
             lookups=lookupGroups,
-            features=[features[tag] for tag in ["kern", "dist"] if tag in features],
+            features=featureBlocks,
         )
         return True
 

Lib/ufo2ft/featureWriters/kernFeatureWriter2.py

@@ -291,17 +291,31 @@ def _write(self):
         classDefs = self.context.kerning.classDefs
         newClassDefs = [c for _, c in sorted(classDefs.items())]
 
+        featureBlocks = [features[tag] for tag in ["kern", "dist"] if tag in features]
+
+        # Collect only the lookups that are referenced by the features we're
+        # writing, to avoid inserting unreferenced/dangling lookups when a
+        # feature block already exists in the feature file.
+        referencedLookups = {
+            statement.lookup
+            for feature in featureBlocks
+            for statement in feature.statements
+            if isinstance(statement, ast.LookupReferenceStatement)
+        }
+
         lookupGroups = []
         for _, lookupGroup in sorted(lookups.items(), key=lambda x: x[0].value):
             lookupGroups.extend(
-                lkp for lkp in lookupGroup.values() if lkp not in lookupGroups
+                lkp
+                for lkp in lookupGroup.values()
+                if lkp in referencedLookups and lkp not in lookupGroups
             )
 
         self._insert(
             feaFile=feaFile,
             classDefs=newClassDefs,
             lookups=lookupGroups,
-            features=[features[tag] for tag in ["kern", "dist"] if tag in features],
+            features=featureBlocks,
         )
         return True
 

tests/featureWriters/kernFeatureWriter_test.py

@@ -2279,6 +2279,60 @@ def test_dflt_language(FontClass):
     )
 
 
+def test_skip_existing_feature_no_dangling_lookups(FontClass):
+    """Check that when a feature block already exists (e.g. dist), the lookups
+    that would have been generated for that feature are not written to the
+    feature file, thus ensuring no dangling/unreferenced lookups.
+
+    See: https://github.com/googlefonts/ufo2ft/issues/960
+    """
+    # Create glyphs for both kern (Latin) and dist (Kannada)
+    glyphs = {
+        "a": ord("a"),
+        "b": ord("b"),
+        "aaMatra_kannada": 0x0CBE,
+        "ailength_kannada": 0xCD6,
+    }
+    groups = {
+        "public.kern1.KND_aaMatra_R": ["aaMatra_kannada"],
+        "public.kern2.KND_ailength_L": ["aaMatra_kannada"],
+    }
+    kerning = {
+        ("a", "b"): 10,  # This will go to kern feature
+        ("public.kern1.KND_aaMatra_R", "public.kern2.KND_ailength_L"): 34,  # dist
+    }
+    # Features with existing dist block: this should cause dist to be skipped
+    features = dedent(
+        """\
+        languagesystem DFLT dflt;
+        languagesystem latn dflt;
+        languagesystem knda dflt;
+        languagesystem knd2 dflt;
+
+        feature dist {
+            # manual dist feature
+            pos aaMatra_kannada ailength_kannada 50;
+        } dist;
+        """
+    )
+
+    ufo = makeUFO(FontClass, glyphs, groups, kerning, features)
+
+    writer = KernFeatureWriter()
+    feaFile = parseLayoutFeatures(ufo)
+    writer.write(ufo, feaFile)
+
+    # Get all lookups in the generated feature file
+    lookups = getLookups(feaFile)
+    lookup_names = [lkp.name for lkp in lookups]
+
+    # Expect kern lookup for Latin, but NOT dist lookup for Kannada
+    assert "kern_Latn" in lookup_names
+    assert (
+        "kern_Knda" not in lookup_names
+    ), "kern_Knda lookup should not be generated when dist feature exists"
+
+
 if __name__ == "__main__":
     import sys