[feaLib] Don’t modify variable anchors in place (#3717)

When passing a parsed feature file that has variable anchors to
addOpenTypeFeatures(), builder would modify the anchors in place and
discard the variations, which break any subsequent use of the feature
file.

I encountered this building a font that has variable cursing anchors
with ufo2ft. The cursFeatureWriter would write the variable anchors, but
then when kernFeatreWriter compiles the file to get GSUB closure, the
variation would be dropped from the anchors, and later when when the
feature data is compiled into the font, the anchors would be compiled
without variations.

Lib/fontTools/feaLib/builder.py

@@ -1658,38 +1658,31 @@ class Builder(object):
 
         return default, device
 
+    def makeAnchorPos(self, varscalar, deviceTable, location):
+        device = None
+        if not isinstance(varscalar, VariableScalar):
+            if deviceTable is not None:
+                device = otl.buildDevice(dict(deviceTable))
+            return varscalar, device
+        default, device = self.makeVariablePos(location, varscalar)
+        if device is not None and deviceTable is not None:
+            raise FeatureLibError(
+                "Can't define a device coordinate and variable scalar", location
+            )
+        return default, device
+
     def makeOpenTypeAnchor(self, location, anchor):
         """ast.Anchor --> otTables.Anchor"""
         if anchor is None:
             return None
-        variable = False
         deviceX, deviceY = None, None
         if anchor.xDeviceTable is not None:
             deviceX = otl.buildDevice(dict(anchor.xDeviceTable))
         if anchor.yDeviceTable is not None:
             deviceY = otl.buildDevice(dict(anchor.yDeviceTable))
-        for dim in ("x", "y"):
+        x, deviceX = self.makeAnchorPos(anchor.x, anchor.xDeviceTable, location)
-            varscalar = getattr(anchor, dim)
+        y, deviceY = self.makeAnchorPos(anchor.y, anchor.yDeviceTable, location)
-            if not isinstance(varscalar, VariableScalar):
+        otlanchor = otl.buildAnchor(x, y, anchor.contourpoint, deviceX, deviceY)
-                continue
-            if getattr(anchor, dim + "DeviceTable") is not None:
-                raise FeatureLibError(
-                    "Can't define a device coordinate and variable scalar", location
-                )
-            default, device = self.makeVariablePos(location, varscalar)
-            setattr(anchor, dim, default)
-            if device is not None:
-                if dim == "x":
-                    deviceX = device
-                else:
-                    deviceY = device
-                variable = True
-
-        otlanchor = otl.buildAnchor(
-            anchor.x, anchor.y, anchor.contourpoint, deviceX, deviceY
-        )
-        if variable:
-            otlanchor.Format = 3
         return otlanchor
 
     _VALUEREC_ATTRS = {

Tests/feaLib/builder_test.py

@@ -12,6 +12,7 @@ from fontTools.feaLib.lexer import Lexer
 from fontTools.fontBuilder import addFvar
 import difflib
 from io import StringIO
+from textwrap import dedent
 import os
 import re
 import shutil
@@ -1160,6 +1161,22 @@ class BuilderTest(unittest.TestCase):
         var_region_axis = var_region_list.Region[0].VarRegionAxis[0]
         assert self.get_region(var_region_axis) == (0.0, 0.875, 1.0)
 
+    def test_variable_anchors_round_trip(self):
+        """Test that calling `addOpenTypeFeatures` with parsed feature file does
+        not discard variations from variable anchors."""
+        features = """\
+            feature curs {
+                pos cursive one <anchor 0 (wdth=100,wght=200:12 wdth=150,wght=900:42)> <anchor NULL>;
+            } curs;
+        """
+
+        f = StringIO(features)
+        feafile = Parser(f).parse()
+
+        font = self.make_mock_vf()
+        addOpenTypeFeatures(font, feafile)
+        assert dedent(str(feafile)) == dedent(features)
+
 
 def generate_feature_file_test(name):
     return lambda self: self.check_feature_file(name)