From 73e7aad1c3e83d7c2e80eb9ce2ca6c97da248215 Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Wed, 15 Mar 2023 16:18:47 +0000 Subject: [PATCH 1/5] Guard against missing avar entries --- Lib/fontTools/feaLib/builder.py | 3 ++- Tests/feaLib/builder_test.py | 28 ++++++++++++++++++---------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/Lib/fontTools/feaLib/builder.py b/Lib/fontTools/feaLib/builder.py index bee3711fb..0aa7a819c 100644 --- a/Lib/fontTools/feaLib/builder.py +++ b/Lib/fontTools/feaLib/builder.py @@ -1596,7 +1596,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() } diff --git a/Tests/feaLib/builder_test.py b/Tests/feaLib/builder_test.py index 885e82384..abf4965b1 100644 --- a/Tests/feaLib/builder_test.py +++ b/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,40 @@ 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] - .FilterRangeMinValue - == 0.6 # user-space 600 + .ConditionSet.ConditionTable ) + # 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] - .FilterRangeMinValue - == 0.7 # user-space 600 shifted to the right, + .ConditionSet.ConditionTable ) + # 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 generate_feature_file_test(name): From a993247e47f04f867a3334de43b7db839298951f Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Wed, 15 Mar 2023 16:19:12 +0000 Subject: [PATCH 2/5] Remove unused variable --- Lib/fontTools/feaLib/builder.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/fontTools/feaLib/builder.py b/Lib/fontTools/feaLib/builder.py index 0aa7a819c..ec67bc76c 100644 --- a/Lib/fontTools/feaLib/builder.py +++ b/Lib/fontTools/feaLib/builder.py @@ -1655,7 +1655,6 @@ class Builder(object): return None vr = {} - variable = False for astName, (otName, isDevice) in self._VALUEREC_ATTRS.items(): val = getattr(v, astName, None) if not val: @@ -1679,7 +1678,6 @@ class Builder(object): vr[otName] = default if index is not None and index != 0xFFFFFFFF: vr[otDeviceName] = buildVarDevTable(index) - variable = True else: vr[otName] = val From cf43ff5d2253e62fa9240c6643d1b45e61e06e81 Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Wed, 15 Mar 2023 16:19:39 +0000 Subject: [PATCH 3/5] Apply avar to variable locations --- Lib/fontTools/feaLib/builder.py | 8 +++- Lib/fontTools/feaLib/variableScalar.py | 26 ++++++++---- Tests/feaLib/builder_test.py | 58 ++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 11 deletions(-) diff --git a/Lib/fontTools/feaLib/builder.py b/Lib/fontTools/feaLib/builder.py index ec67bc76c..c785f2e76 100644 --- a/Lib/fontTools/feaLib/builder.py +++ b/Lib/fontTools/feaLib/builder.py @@ -1614,6 +1614,7 @@ 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): continue @@ -1627,7 +1628,9 @@ class Builder(object): ) 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, avar + ) setattr(anchor, dim, default) if index is not None and index != 0xFFFFFFFF: if dim == "x": @@ -1654,6 +1657,7 @@ class Builder(object): if not v: return None + avar = self.font.get("avar") vr = {} for astName, (otName, isDevice) in self._VALUEREC_ATTRS.items(): val = getattr(v, astName, None) @@ -1674,7 +1678,7 @@ 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, avar) vr[otName] = default if index is not None and index != 0xFFFFFFFF: vr[otDeviceName] = buildVarDevTable(index) diff --git a/Lib/fontTools/feaLib/variableScalar.py b/Lib/fontTools/feaLib/variableScalar.py index 35847e9de..8cbc2d22a 100644 --- a/Lib/fontTools/feaLib/variableScalar.py +++ b/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): @@ -83,29 +83,37 @@ 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, 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(avar).interpolateFromMasters(loc, values) - @property - def model(self): + def model(self, avar=None): key = tuple(self.values.keys()) if key in self.model_pool: return self.model_pool[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 return m - def get_deltas_and_supports(self): + def get_deltas_and_supports(self, avar=None): values = list(self.values.values()) - return self.model.getDeltasAndSupports(values) + return self.model(avar).getDeltasAndSupports(values) - def add_to_variation_store(self, store_builder): - deltas, supports = self.get_deltas_and_supports() + def add_to_variation_store(self, store_builder, avar=None): + deltas, supports = self.get_deltas_and_supports(avar) store_builder.setSupports(supports) index = store_builder.storeDeltas(deltas) return int(self.default), index diff --git a/Tests/feaLib/builder_test.py b/Tests/feaLib/builder_test.py index abf4965b1..5fbbfaf52 100644 --- a/Tests/feaLib/builder_test.py +++ b/Tests/feaLib/builder_test.py @@ -1034,6 +1034,64 @@ class BuilderTest(unittest.TestCase): 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 ; + 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): return lambda self: self.check_feature_file(name) From ac0361fe31a290bc52f30982feb177ec27e807a7 Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Thu, 16 Mar 2023 11:27:06 +0000 Subject: [PATCH 4/5] Move VariableScalar cache into Builder --- Lib/fontTools/feaLib/builder.py | 16 +++++++++----- Lib/fontTools/feaLib/variableScalar.py | 29 +++++++++----------------- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/Lib/fontTools/feaLib/builder.py b/Lib/fontTools/feaLib/builder.py index c785f2e76..71b1b209a 100644 --- a/Lib/fontTools/feaLib/builder.py +++ b/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, @@ -1616,7 +1620,8 @@ class Builder(object): 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( @@ -1626,10 +1631,9 @@ 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, avar + self.varstorebuilder, self.model_cache, avar ) setattr(anchor, dim, default) if index is not None and index != 0xFFFFFFFF: @@ -1678,7 +1682,9 @@ class Builder(object): location, ) val.axes = self.axes - default, index = val.add_to_variation_store(self.varstorebuilder, avar) + 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) diff --git a/Lib/fontTools/feaLib/variableScalar.py b/Lib/fontTools/feaLib/variableScalar.py index 8cbc2d22a..94b8eee8a 100644 --- a/Lib/fontTools/feaLib/variableScalar.py +++ b/Lib/fontTools/feaLib/variableScalar.py @@ -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,17 +74,17 @@ class VariableScalar: # I *guess* we could interpolate one, but I don't know how. return self.values[key] - def value_at_location(self, location, avar=None): + def value_at_location(self, location, model_cache, avar=None): loc = location if loc in self.values.keys(): return self.values[loc] values = list(self.values.values()) - return self.model(avar).interpolateFromMasters(loc, values) + return self.model(model_cache, avar).interpolateFromMasters(loc, values) - def model(self, avar=None): + def model(self, model_cache, avar=None): key = tuple(self.values.keys()) - if key in self.model_pool: - return self.model_pool[key] + if key in model_cache: + return model_cache[key] locations = [dict(self._normalized_location(k)) for k in self.values.keys()] if avar is not None: mapping = avar.segments @@ -105,15 +96,15 @@ class VariableScalar: for location in locations ] m = VariationModel(locations) - self.model_pool[key] = m + model_cache[key] = m return m - def get_deltas_and_supports(self, avar=None): + def get_deltas_and_supports(self, model_cache, avar=None): values = list(self.values.values()) - return self.model(avar).getDeltasAndSupports(values) + return self.model(model_cache, avar).getDeltasAndSupports(values) - def add_to_variation_store(self, store_builder, avar=None): - deltas, supports = self.get_deltas_and_supports(avar) + def add_to_variation_store(self, store_builder, model_cache, avar=None): + deltas, supports = self.get_deltas_and_supports(model_cache, avar) store_builder.setSupports(supports) index = store_builder.storeDeltas(deltas) return int(self.default), index From 69b1752d806575a39b9ffb25e448bf818159f5a9 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 16 Mar 2023 12:03:33 +0000 Subject: [PATCH 5/5] variableScalar: make model_cache parameter optional --- Lib/fontTools/feaLib/variableScalar.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/Lib/fontTools/feaLib/variableScalar.py b/Lib/fontTools/feaLib/variableScalar.py index 94b8eee8a..c97b43542 100644 --- a/Lib/fontTools/feaLib/variableScalar.py +++ b/Lib/fontTools/feaLib/variableScalar.py @@ -74,17 +74,18 @@ class VariableScalar: # I *guess* we could interpolate one, but I don't know how. return self.values[key] - def value_at_location(self, location, model_cache, avar=None): + 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(model_cache, avar).interpolateFromMasters(loc, values) - def model(self, model_cache, avar=None): - key = tuple(self.values.keys()) - if key in model_cache: - return model_cache[key] + def model(self, model_cache=None, avar=None): + if model_cache is not None: + key = tuple(self.values.keys()) + if key in model_cache: + return model_cache[key] locations = [dict(self._normalized_location(k)) for k in self.values.keys()] if avar is not None: mapping = avar.segments @@ -96,14 +97,15 @@ class VariableScalar: for location in locations ] m = VariationModel(locations) - model_cache[key] = m + if model_cache is not None: + model_cache[key] = m return m - def get_deltas_and_supports(self, model_cache, avar=None): + def get_deltas_and_supports(self, model_cache=None, avar=None): values = list(self.values.values()) return self.model(model_cache, avar).getDeltasAndSupports(values) - def add_to_variation_store(self, store_builder, model_cache, avar=None): + def add_to_variation_store(self, store_builder, model_cache=None, avar=None): deltas, supports = self.get_deltas_and_supports(model_cache, avar) store_builder.setSupports(supports) index = store_builder.storeDeltas(deltas)