From 6358b47682052e4393e30945410fe8dc7c343d65 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 1 Dec 2023 19:02:16 +0000 Subject: [PATCH 1/2] [featureVars] don't overwrite FeatureVariations, append new records Currently, addFeatureVariationsRaw always deletes existing GSUB.FeatureVariations and overwrites them with the newly built records. It doesn't have to, though. If the features for which we are adding feature variations are not already "variable", we can simply append the DS-rules-generated records to the FeatureVariations. We raise an error if the existing FeatureVariations already reference the same feature tags used for the DS rules, as that could generate ambiguity/undefined logic. --- Lib/fontTools/varLib/featureVars.py | 33 ++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/Lib/fontTools/varLib/featureVars.py b/Lib/fontTools/varLib/featureVars.py index de473d976..a6beb5c7d 100644 --- a/Lib/fontTools/varLib/featureVars.py +++ b/Lib/fontTools/varLib/featureVars.py @@ -69,6 +69,14 @@ def addFeatureVariations(font, conditionalSubstitutions, featureTag="rvrn"): ) if "GSUB" not in font: font["GSUB"] = buildGSUB() + else: + existingTags = _existingVariableFeatures(font["GSUB"].table).intersection( + featureTags + ) + if existingTags: + raise VarLibError( + f"FeatureVariations already exist for feature tag(s): {existingTags}" + ) # setup lookups lookupMap = buildSubstitutionLookups( @@ -87,6 +95,16 @@ def addFeatureVariations(font, conditionalSubstitutions, featureTag="rvrn"): addFeatureVariationsRaw(font, font["GSUB"].table, conditionsAndLookups, featureTags) +def _existingVariableFeatures(table): + existingFeatureVarsTags = set() + if hasattr(table, "FeatureVariations") and table.FeatureVariations is not None: + features = table.FeatureList.FeatureRecord + for fvr in table.FeatureVariations.FeatureVariationRecord: + for ftsr in fvr.FeatureTableSubstitution.SubstitutionRecord: + existingFeatureVarsTags.add(features[ftsr.FeatureIndex].FeatureTag) + return existingFeatureVarsTags + + def _checkSubstitutionGlyphsExist(glyphNames, substitutions): referencedGlyphNames = set() for _, substitution in substitutions: @@ -349,8 +367,6 @@ def addFeatureVariationsRaw(font, table, conditionalSubstitutions, featureTag="r if table.Version < 0x00010001: table.Version = 0x00010001 # allow table.FeatureVariations - table.FeatureVariations = None # delete any existing FeatureVariations - varFeatureIndices = set() existingTags = { @@ -428,7 +444,18 @@ def addFeatureVariationsRaw(font, table, conditionalSubstitutions, featureTag="r buildFeatureVariationRecord(conditionTable, records) ) - table.FeatureVariations = buildFeatureVariations(featureVariationRecords) + if hasattr(table, "FeatureVariations") and table.FeatureVariations is not None: + if table.FeatureVariations.Version != 0x00010000: + raise VarLibError( + "Unsupported FeatureVariations table version: " + f"0x{table.FeatureVariations.Version:08x} (expected 0x00010000)." + ) + table.FeatureVariations.FeatureVariationRecord.extend(featureVariationRecords) + table.FeatureVariations.FeatureVariationCount = len( + table.FeatureVariations.FeatureVariationRecord + ) + else: + table.FeatureVariations = buildFeatureVariations(featureVariationRecords) # From e538b9c02f9fb023b7daace11f1b7241e007d32a Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Sat, 2 Dec 2023 11:18:31 +0000 Subject: [PATCH 2/2] [featureVars_test] test appending records for new features --- Tests/varLib/featureVars_test.py | 134 ++++++++++++++++++++++++++++++- 1 file changed, 133 insertions(+), 1 deletion(-) diff --git a/Tests/varLib/featureVars_test.py b/Tests/varLib/featureVars_test.py index 7a3a6650d..99f41e776 100644 --- a/Tests/varLib/featureVars_test.py +++ b/Tests/varLib/featureVars_test.py @@ -1,4 +1,136 @@ -from fontTools.varLib.featureVars import overlayFeatureVariations, overlayBox +from collections import OrderedDict +from fontTools.designspaceLib import AxisDescriptor +from fontTools.ttLib import TTFont, newTable +from fontTools import varLib +from fontTools.varLib.featureVars import ( + addFeatureVariations, + overlayFeatureVariations, + overlayBox, +) +import pytest + + +def makeVariableFont(glyphOrder, axes): + font = TTFont() + font.setGlyphOrder(glyphOrder) + font["name"] = newTable("name") + ds_axes = OrderedDict() + for axisTag, (minimum, default, maximum) in axes.items(): + axis = AxisDescriptor() + axis.name = axis.tag = axis.labelNames["en"] = axisTag + axis.minimum, axis.default, axis.maximum = minimum, default, maximum + ds_axes[axisTag] = axis + varLib._add_fvar(font, ds_axes, instances=()) + return font + + +@pytest.fixture +def varfont(): + return makeVariableFont( + [".notdef", "space", "A", "B", "A.alt", "B.alt"], + {"wght": (100, 400, 900)}, + ) + + +def test_addFeatureVariations(varfont): + assert "GSUB" not in varfont + + addFeatureVariations(varfont, [([{"wght": (0.5, 1.0)}], {"A": "A.alt"})]) + + assert "GSUB" in varfont + gsub = varfont["GSUB"].table + + assert len(gsub.ScriptList.ScriptRecord) == 1 + assert gsub.ScriptList.ScriptRecord[0].ScriptTag == "DFLT" + + assert len(gsub.FeatureList.FeatureRecord) == 1 + assert gsub.FeatureList.FeatureRecord[0].FeatureTag == "rvrn" + + assert len(gsub.LookupList.Lookup) == 1 + assert gsub.LookupList.Lookup[0].LookupType == 1 + assert len(gsub.LookupList.Lookup[0].SubTable) == 1 + assert gsub.LookupList.Lookup[0].SubTable[0].mapping == {"A": "A.alt"} + + assert gsub.FeatureVariations is not None + assert len(gsub.FeatureVariations.FeatureVariationRecord) == 1 + fvr = gsub.FeatureVariations.FeatureVariationRecord[0] + assert len(fvr.ConditionSet.ConditionTable) == 1 + cst = fvr.ConditionSet.ConditionTable[0] + assert cst.AxisIndex == 0 + assert cst.FilterRangeMinValue == 0.5 + assert cst.FilterRangeMaxValue == 1.0 + assert len(fvr.FeatureTableSubstitution.SubstitutionRecord) == 1 + ftsr = fvr.FeatureTableSubstitution.SubstitutionRecord[0] + assert ftsr.FeatureIndex == 0 + assert ftsr.Feature.LookupListIndex == [0] + + +def _substitution_features(gsub, rec_index): + fea_tags = [feature.FeatureTag for feature in gsub.FeatureList.FeatureRecord] + fea_indices = [ + gsub.FeatureVariations.FeatureVariationRecord[rec_index] + .FeatureTableSubstitution.SubstitutionRecord[i] + .FeatureIndex + for i in range( + len( + gsub.FeatureVariations.FeatureVariationRecord[ + rec_index + ].FeatureTableSubstitution.SubstitutionRecord + ) + ) + ] + return [(i, fea_tags[i]) for i in fea_indices] + + +def test_addFeatureVariations_existing_variable_feature(varfont): + assert "GSUB" not in varfont + + addFeatureVariations(varfont, [([{"wght": (0.5, 1.0)}], {"A": "A.alt"})]) + + gsub = varfont["GSUB"].table + assert len(gsub.FeatureList.FeatureRecord) == 1 + assert gsub.FeatureList.FeatureRecord[0].FeatureTag == "rvrn" + assert len(gsub.FeatureVariations.FeatureVariationRecord) == 1 + assert _substitution_features(gsub, rec_index=0) == [(0, "rvrn")] + + # can't add feature variations for an existing feature tag that already has some, + # in this case the default 'rvrn' + with pytest.raises( + varLib.VarLibError, + match=r"FeatureVariations already exist for feature tag\(s\): {'rvrn'}", + ): + addFeatureVariations(varfont, [([{"wght": (0.5, 1.0)}], {"A": "A.alt"})]) + + +def test_addFeatureVariations_new_feature(varfont): + assert "GSUB" not in varfont + + addFeatureVariations(varfont, [([{"wght": (0.5, 1.0)}], {"A": "A.alt"})]) + + gsub = varfont["GSUB"].table + assert len(gsub.FeatureList.FeatureRecord) == 1 + assert gsub.FeatureList.FeatureRecord[0].FeatureTag == "rvrn" + assert len(gsub.LookupList.Lookup) == 1 + assert len(gsub.FeatureVariations.FeatureVariationRecord) == 1 + assert _substitution_features(gsub, rec_index=0) == [(0, "rvrn")] + + # we can add feature variations for a feature tag that does not have + # any feature variations yet + addFeatureVariations( + varfont, [([{"wght": (-1.0, 0.0)}], {"B": "B.alt"})], featureTag="rclt" + ) + + assert len(gsub.FeatureList.FeatureRecord) == 2 + # Note 'rclt' is now first (index=0) in the feature list sorted by tag, and + # 'rvrn' is second (index=1) + assert gsub.FeatureList.FeatureRecord[0].FeatureTag == "rclt" + assert gsub.FeatureList.FeatureRecord[1].FeatureTag == "rvrn" + assert len(gsub.LookupList.Lookup) == 2 + assert len(gsub.FeatureVariations.FeatureVariationRecord) == 2 + # The new 'rclt' feature variation record is appended to the end; + # the feature index for 'rvrn' feature table substitution record is now 1 + assert _substitution_features(gsub, rec_index=0) == [(1, "rvrn")] + assert _substitution_features(gsub, rec_index=1) == [(0, "rclt")] def _test_linear(n):