Merge pull request #3043 from fonttools/fealib-more-avar

Apply `avar` also to variable locations

Lib/fontTools/feaLib/builder.py

@@ -176,6 +176,10 @@ class Builder(object):
         self.stat_ = {}
         # for conditionsets
         self.conditionsets_ = {}
+        # We will often use exactly the same locations (i.e. the font's masters)
+        # for a large number of variable scalars. Instead of creating a model
+        # for each, let's share the models.
+        self.model_cache = {}
 
     def build(self, tables=None, debug=False):
         if self.parseTree is None:
@@ -771,7 +775,7 @@ class Builder(object):
                 gdef.remap_device_varidxes(varidx_map)
                 if "GPOS" in self.font:
                     self.font["GPOS"].table.remap_device_varidxes(varidx_map)
-            VariableScalar.clear_cache()
+            self.model_cache.clear()
         if any(
             (
                 gdef.GlyphClassDef,
@@ -1596,7 +1600,8 @@ class Builder(object):
             mapping = self.font["avar"].segments
             value = {
                 axis: tuple(
-                    piecewiseLinearMap(v, mapping[axis]) for v in condition_range
+                    piecewiseLinearMap(v, mapping[axis]) if axis in mapping else v
+                    for v in condition_range
                 )
                 for axis, condition_range in value.items()
             }
@@ -1613,8 +1618,10 @@ class Builder(object):
             deviceX = otl.buildDevice(dict(anchor.xDeviceTable))
         if anchor.yDeviceTable is not None:
             deviceY = otl.buildDevice(dict(anchor.yDeviceTable))
+        avar = self.font.get("avar")
         for dim in ("x", "y"):
-            if not isinstance(getattr(anchor, dim), VariableScalar):
+            varscalar = getattr(anchor, dim)
+            if not isinstance(varscalar, VariableScalar):
                 continue
             if getattr(anchor, dim + "DeviceTable") is not None:
                 raise FeatureLibError(
@@ -1624,9 +1631,10 @@ class Builder(object):
                 raise FeatureLibError(
                     "Can't define a variable scalar in a non-variable font", location
                 )
-            varscalar = getattr(anchor, dim)
             varscalar.axes = self.axes
-            default, index = varscalar.add_to_variation_store(self.varstorebuilder)
+            default, index = varscalar.add_to_variation_store(
+                self.varstorebuilder, self.model_cache, avar
+            )
             setattr(anchor, dim, default)
             if index is not None and index != 0xFFFFFFFF:
                 if dim == "x":
@@ -1653,8 +1661,8 @@ class Builder(object):
         if not v:
             return None
 
+        avar = self.font.get("avar")
         vr = {}
-        variable = False
         for astName, (otName, isDevice) in self._VALUEREC_ATTRS.items():
             val = getattr(v, astName, None)
             if not val:
@@ -1674,11 +1682,12 @@ class Builder(object):
                         location,
                     )
                 val.axes = self.axes
-                default, index = val.add_to_variation_store(self.varstorebuilder)
+                default, index = val.add_to_variation_store(
+                    self.varstorebuilder, self.model_cache, avar
+                )
                 vr[otName] = default
                 if index is not None and index != 0xFFFFFFFF:
                     vr[otDeviceName] = buildVarDevTable(index)
-                    variable = True
             else:
                 vr[otName] = val
 

Lib/fontTools/feaLib/variableScalar.py

@@ -1,4 +1,4 @@
-from fontTools.varLib.models import VariationModel, normalizeValue
+from fontTools.varLib.models import VariationModel, normalizeValue, piecewiseLinearMap
 
 
 def Location(loc):
@@ -8,15 +8,6 @@ def Location(loc):
 class VariableScalar:
     """A scalar with different values at different points in the designspace."""
 
-    # We will often use exactly the same locations (i.e. the font's
-    # masters) for a large number of variable scalars. Instead of
-    # creating a model for each, let's share the models.
-    model_pool = {}
-
-    @classmethod
-    def clear_cache(cls):
-        cls.model_pool = {}
-
     def __init__(self, location_value={}):
         self.values = {}
         self.axes = {}
@@ -83,29 +74,39 @@ class VariableScalar:
             # I *guess* we could interpolate one, but I don't know how.
         return self.values[key]
 
-    def value_at_location(self, location):
+    def value_at_location(self, location, model_cache=None, avar=None):
         loc = location
         if loc in self.values.keys():
             return self.values[loc]
         values = list(self.values.values())
-        return self.model.interpolateFromMasters(loc, values)
+        return self.model(model_cache, avar).interpolateFromMasters(loc, values)
 
-    @property
+    def model(self, model_cache=None, avar=None):
-    def model(self):
+        if model_cache is not None:
-        key = tuple(self.values.keys())
+            key = tuple(self.values.keys())
-        if key in self.model_pool:
+            if key in model_cache:
-            return self.model_pool[key]
+                return model_cache[key]
         locations = [dict(self._normalized_location(k)) for k in self.values.keys()]
+        if avar is not None:
+            mapping = avar.segments
+            locations = [
+                {
+                    k: piecewiseLinearMap(v, mapping[k]) if k in mapping else v
+                    for k, v in location.items()
+                }
+                for location in locations
+            ]
         m = VariationModel(locations)
-        self.model_pool[key] = m
+        if model_cache is not None:
+            model_cache[key] = m
         return m
 
-    def get_deltas_and_supports(self):
+    def get_deltas_and_supports(self, model_cache=None, avar=None):
         values = list(self.values.values())
-        return self.model.getDeltasAndSupports(values)
+        return self.model(model_cache, avar).getDeltasAndSupports(values)
 
-    def add_to_variation_store(self, store_builder):
+    def add_to_variation_store(self, store_builder, model_cache=None, avar=None):
-        deltas, supports = self.get_deltas_and_supports()
+        deltas, supports = self.get_deltas_and_supports(model_cache, avar)
         store_builder.setSupports(supports)
         index = store_builder.storeDeltas(deltas)
         return int(self.default), index

Tests/feaLib/builder_test.py

@@ -988,6 +988,7 @@ class BuilderTest(unittest.TestCase):
 
             conditionset test {
                 wght 600 1000;
+                wdth 150 200;
             } test;
 
             variation rlig test {
@@ -998,33 +999,98 @@ class BuilderTest(unittest.TestCase):
         def make_mock_vf():
             font = makeTTFont()
             font["name"] = newTable("name")
-            addFvar(font, [("wght", 0, 0, 1000, "Weight")], [])
+            addFvar(
+                font,
+                [("wght", 0, 0, 1000, "Weight"), ("wdth", 100, 100, 200, "Width")],
+                [],
+            )
             del font["name"]
             return font
 
         # Without `avar`:
         font = make_mock_vf()
         addOpenTypeFeaturesFromString(font, features)
-        assert (
+        condition_table = (
             font.tables["GSUB"]
             .table.FeatureVariations.FeatureVariationRecord[0]
-            .ConditionSet.ConditionTable[0]
+            .ConditionSet.ConditionTable
-            .FilterRangeMinValue
-            == 0.6  # user-space 600
         )
+        # user-space wdth=150 and wght=600:
+        assert condition_table[0].FilterRangeMinValue == 0.5
+        assert condition_table[1].FilterRangeMinValue == 0.6
 
-        # With `avar`, shifting the positive midpoint 0.5 a bit to the right:
+        # With `avar`, shifting the wght axis' positive midpoint 0.5 a bit to
+        # the right, but leaving the wdth axis alone:
         font = make_mock_vf()
         font["avar"] = newTable("avar")
         font["avar"].segments = {"wght": {-1.0: -1.0, 0.0: 0.0, 0.5: 0.625, 1.0: 1.0}}
         addOpenTypeFeaturesFromString(font, features)
-        assert (
+        condition_table = (
             font.tables["GSUB"]
             .table.FeatureVariations.FeatureVariationRecord[0]
-            .ConditionSet.ConditionTable[0]
+            .ConditionSet.ConditionTable
-            .FilterRangeMinValue
-            == 0.7  # user-space 600 shifted to the right,
         )
+        # user-space wdth=150 as before and wght=600 shifted to the right:
+        assert condition_table[0].FilterRangeMinValue == 0.5
+        assert condition_table[1].FilterRangeMinValue == 0.7
+
+    def test_variable_scalar_avar(self):
+        """Test that the `avar` table is consulted when normalizing user-space
+        values."""
+
+        features = """
+            languagesystem DFLT dflt;
+        
+            feature kern {
+                pos cursive one <anchor 0 (wght=200:12 wght=900:22 wdth=150,wght=900:42)> <anchor NULL>;
+                pos two <0 (wght=200:12 wght=900:22 wdth=150,wght=900:42) 0 0>;
+            } kern;
+        """
+
+        def make_mock_vf():
+            font = makeTTFont()
+            font["name"] = newTable("name")
+            addFvar(font, self.VARFONT_AXES, [])
+            del font["name"]
+            return font
+
+        def get_region(var_region_axis):
+            return (
+                var_region_axis.StartCoord,
+                var_region_axis.PeakCoord,
+                var_region_axis.EndCoord,
+            )
+
+        # Without `avar` (wght=200, wdth=100 is the default location):
+        font = make_mock_vf()
+        addOpenTypeFeaturesFromString(font, features)
+
+        var_region_list = font.tables["GDEF"].table.VarStore.VarRegionList
+        var_region_axis_wght = var_region_list.Region[0].VarRegionAxis[0]
+        var_region_axis_wdth = var_region_list.Region[0].VarRegionAxis[1]
+        assert get_region(var_region_axis_wght) == (0.0, 0.875, 0.875)
+        assert get_region(var_region_axis_wdth) == (0.0, 0.0, 0.0)
+        var_region_axis_wght = var_region_list.Region[1].VarRegionAxis[0]
+        var_region_axis_wdth = var_region_list.Region[1].VarRegionAxis[1]
+        assert get_region(var_region_axis_wght) == (0.0, 0.875, 0.875)
+        assert get_region(var_region_axis_wdth) == (0.0, 0.5, 0.5)
+
+        # With `avar`, shifting the wght axis' positive midpoint 0.5 a bit to
+        # the right, but leaving the wdth axis alone:
+        font = make_mock_vf()
+        font["avar"] = newTable("avar")
+        font["avar"].segments = {"wght": {-1.0: -1.0, 0.0: 0.0, 0.5: 0.625, 1.0: 1.0}}
+        addOpenTypeFeaturesFromString(font, features)
+
+        var_region_list = font.tables["GDEF"].table.VarStore.VarRegionList
+        var_region_axis_wght = var_region_list.Region[0].VarRegionAxis[0]
+        var_region_axis_wdth = var_region_list.Region[0].VarRegionAxis[1]
+        assert get_region(var_region_axis_wght) == (0.0, 0.90625, 0.90625)
+        assert get_region(var_region_axis_wdth) == (0.0, 0.0, 0.0)
+        var_region_axis_wght = var_region_list.Region[1].VarRegionAxis[0]
+        var_region_axis_wdth = var_region_list.Region[1].VarRegionAxis[1]
+        assert get_region(var_region_axis_wght) == (0.0, 0.90625, 0.90625)
+        assert get_region(var_region_axis_wdth) == (0.0, 0.5, 0.5)
 
 
 def generate_feature_file_test(name):