Repository: googlefonts/ufo2ft
Pull request #580 (2022-01-21 11:11:08)

Move unknown anchors to abvm
This changes the anchor assignment heuristic for Indic scripts. See discussion in https://github.com/fonttools/fonttools/issues/2510 and comments in code.


Patch:
diff --git a/Lib/ufo2ft/featureWriters/markFeatureWriter.py b/Lib/ufo2ft/featureWriters/markFeatureWriter.py
index 834d2f6..ba16b77 100644
--- a/Lib/ufo2ft/featureWriters/markFeatureWriter.py
+++ b/Lib/ufo2ft/featureWriters/markFeatureWriter.py
@@ -7,7 +7,6 @@ from fontTools.misc.fixedTools import otRound
 
 from ufo2ft.constants import INDIC_SCRIPTS, USE_SCRIPTS
 from ufo2ft.featureWriters import BaseFeatureWriter, ast
-from ufo2ft.fontInfoData import getAttrWithFallback
 from ufo2ft.util import classifyGlyphs, quantize, unicodeInScripts
 
 
@@ -278,10 +277,15 @@ class MarkFeatureWriter(BaseFeatureWriter):
     # @MC_top, @MC_bottom, etc.
     markClassPrefix = "MC"
 
-    # The anchor names and list of scripts for which 'abvm' and 'blwm'
-    # features are generated is the same as the one Glyphs.app uses, see:
-    # https://github.com/googlei18n/ufo2ft/issues/179
-    abvmAnchorNames = {"top", "topleft", "topright", "candra", "bindu", "candrabindu"}
+    abvmAnchorNames = {
+        "top",
+        "topleft",
+        "topright",
+        "candra",
+        "bindu",
+        "candrabindu",
+        "imatra",
+    }
     blwmAnchorNames = {"bottom", "bottomleft", "bottomright", "nukta"}
 
     scriptsUsingAbvm = set(INDIC_SCRIPTS + USE_SCRIPTS + ["Khmr"])
@@ -736,20 +740,20 @@ class MarkFeatureWriter(BaseFeatureWriter):
 
         return feature if feature.statements else None
 
-    def _getVerticalThreshold(self):
-        # anchors with unknown names whose Y coordinate is greater or equal to
-        # the line that cuts the UPEM square in half will be treated as "above
-        # base" marks, those that fall below the threshold as "below base".
-        return getAttrWithFallback(self.context.font.info, "unitsPerEm") // 2
-
     def _isAboveMark(self, anchor):
         if anchor.name in self.abvmAnchorNames:
             return True
-        if anchor.name in self.blwmAnchorNames:
+        if anchor.name in self.blwmAnchorNames or anchor.name.startswith("bottom"):
             return False
-        if anchor.y >= self.context.threshold:
-            return True
-        return False
+        # Glyphs uses (used to use?) a heuristic to guess whether an anchor
+        # should go into abvm or blwm. (See
+        # https://github.com/googlefonts/ufo2ft/issues/179#issuecomment-390391382)
+        # However, this causes issues in variable fonts where an anchor in one
+        # master is assigned to a different feature from the same anchor in
+        # another master if the Y-coordinates happen to straddle the threshold
+        # coordinate. For simplicity, we just place all unknown anchors into
+        # the abvm feature.
+        return True
 
     def _isBelowMark(self, anchor):
         return not self._isAboveMark(anchor)
@@ -838,7 +842,6 @@ class MarkFeatureWriter(BaseFeatureWriter):
                 features["mkmk"] = mkmk
         if "abvm" in todo or "blwm" in todo:
             if abvmGlyphs:
-                self.context.threshold = self._getVerticalThreshold()
                 for tag in ("abvm", "blwm"):
                     if tag not in todo:
                         continue


Test patch:
diff --git a/tests/featureWriters/markFeatureWriter_test.py b/tests/featureWriters/markFeatureWriter_test.py
index f8614b3..3ea705d 100644
--- a/tests/featureWriters/markFeatureWriter_test.py
+++ b/tests/featureWriters/markFeatureWriter_test.py
@@ -907,6 +907,7 @@ class MarkFeatureWriterTest(FeatureWriterTest):
                         ligComponent
                             <anchor NULL>
                         ligComponent
+                            <anchor 1100 499> mark @MC_bar
                             <anchor 1000 500> mark @MC_top;
                     pos ligature bar_foo
                             <anchor NULL>
@@ -914,6 +915,13 @@ class MarkFeatureWriterTest(FeatureWriterTest):
                             <anchor 600 501> mark @MC_top;
                 } abvm_mark2liga;
 
+                lookup abvm_mark2mark_bar {
+                    @MFS_abvm_mark2mark_bar = [barcomb];
+                    lookupflag UseMarkFilteringSet @MFS_abvm_mark2mark_bar;
+                    pos mark barcomb
+                        <anchor 100 440> mark @MC_bar;
+                } abvm_mark2mark_bar;
+
                 lookup abvm_mark2mark_top {
                     @MFS_abvm_mark2mark_top = [foocomb];
                     lookupflag UseMarkFilteringSet @MFS_abvm_mark2mark_top;
@@ -930,16 +938,9 @@ class MarkFeatureWriterTest(FeatureWriterTest):
                         ligComponent
                             <anchor NULL>
                         ligComponent
-                            <anchor 1100 499> mark @MC_bar;
+                            <anchor NULL>;
                 } blwm_mark2liga;
 
-                lookup blwm_mark2mark_bar {
-                    @MFS_blwm_mark2mark_bar = [barcomb];
-                    lookupflag UseMarkFilteringSet @MFS_blwm_mark2mark_bar;
-                    pos mark barcomb
-                        <anchor 100 440> mark @MC_bar;
-                } blwm_mark2mark_bar;
-
                 lookup blwm_mark2mark_bottom {
                     @MFS_blwm_mark2mark_bottom = [bazcomb];
                     lookupflag UseMarkFilteringSet @MFS_blwm_mark2mark_bottom;