[featureVars] Re-use FeatureVariationRecord's when possible

If a FeatureVariationRecord with the same ConditionTable exists re-use
it and append FeatureTableSubstitutionRecord’s.

Without this, in the following feature code only the first lookup will
be applied since there will be two FeatureVariationRecord with the same
ConditionTable, so the first will be matched and the other will be
skipped:

    conditionset test {
        wght 600 1000;
        wdth 150 200;
    } test;

    variation ccmp test {
        sub e by a;
    } ccmp;

    variation rlig test {
        sub b by c;
    } rlig;

With this change only one FeatureVariationRecord will be created with
two FeatureTableSubstitutionRecord’s.
This commit is contained in:
خالد حسني (Khaled Hosny) 2024-01-10 19:06:26 +02:00 committed by GitHub
parent e3cde46e69
commit 1c25210360
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 104 additions and 4 deletions

View File

@ -414,6 +414,10 @@ def addFeatureVariationsRaw(font, table, conditionalSubstitutions, featureTag="r
axis.axisTag: axisIndex for axisIndex, axis in enumerate(font["fvar"].axes)
}
hasFeatureVariations = (
hasattr(table, "FeatureVariations") and table.FeatureVariations is not None
)
featureVariationRecords = []
for conditionSet, lookupIndices in conditionalSubstitutions:
conditionTable = []
@ -440,11 +444,19 @@ def addFeatureVariationsRaw(font, table, conditionalSubstitutions, featureTag="r
varFeatureIndex, combinedLookupIndices
)
)
featureVariationRecords.append(
buildFeatureVariationRecord(conditionTable, records)
)
if hasFeatureVariations and (
fvr := findFeatureVariationRecord(table.FeatureVariations, conditionTable)
):
fvr.FeatureTableSubstitution.SubstitutionRecord.extend(records)
fvr.FeatureTableSubstitution.SubstitutionCount = len(
fvr.FeatureTableSubstitution.SubstitutionRecord
)
else:
featureVariationRecords.append(
buildFeatureVariationRecord(conditionTable, records)
)
if hasattr(table, "FeatureVariations") and table.FeatureVariations is not None:
if hasFeatureVariations:
if table.FeatureVariations.Version != 0x00010000:
raise VarLibError(
"Unsupported FeatureVariations table version: "
@ -614,6 +626,21 @@ def buildConditionTable(axisIndex, filterRangeMinValue, filterRangeMaxValue):
return ct
def findFeatureVariationRecord(featureVariations, conditionTable):
"""Find a FeatureVariationRecord that has the same conditionTable."""
if featureVariations.Version != 0x00010000:
raise VarLibError(
"Unsupported FeatureVariations table version: "
f"0x{featureVariations.Version:08x} (expected 0x00010000)."
)
for fvr in featureVariations.FeatureVariationRecord:
if conditionTable == fvr.ConditionSet.ConditionTable:
return fvr
return None
def sortFeatureList(table):
"""Sort the feature list by feature tag, and remap the feature indices
elsewhere. This is needed after the feature list has been modified.

View File

@ -990,6 +990,47 @@ class BuilderTest(unittest.TestCase):
f'{name}.fea:{line}:12: Ambiguous "ignore {sub}", there should be least one marked glyph'
)
def test_conditionset_multiple_features(self):
"""Test that using the same `conditionset` for multiple features reuses the
`FeatureVariationRecord`."""
features = """
languagesystem DFLT dflt;
conditionset test {
wght 600 1000;
wdth 150 200;
} test;
variation ccmp test {
sub e by a;
} ccmp;
variation rlig test {
sub b by c;
} rlig;
"""
def make_mock_vf():
font = makeTTFont()
font["name"] = newTable("name")
addFvar(
font,
[("wght", 0, 0, 1000, "Weight"), ("wdth", 100, 100, 200, "Width")],
[],
)
del font["name"]
return font
font = make_mock_vf()
addOpenTypeFeaturesFromString(font, features)
table = font["GSUB"].table
assert table.FeatureVariations.FeatureVariationCount == 1
fvr = table.FeatureVariations.FeatureVariationRecord[0]
assert fvr.FeatureTableSubstitution.SubstitutionCount == 2
def test_condition_set_avar(self):
"""Test that the `avar` table is consulted when normalizing user-space
values."""

View File

@ -133,6 +133,38 @@ def test_addFeatureVariations_new_feature(varfont):
assert _substitution_features(gsub, rec_index=1) == [(0, "rclt")]
def test_addFeatureVariations_existing_condition(varfont):
assert "GSUB" not in varfont
# Add a feature variation for 'ccmp' feature tag with a condition
addFeatureVariations(
varfont, [([{"wght": (0.5, 1.0)}], {"A": "A.alt"})], featureTag="ccmp"
)
gsub = varfont["GSUB"].table
# Should now have one feature record, one lookup, and one feature variation record
assert len(gsub.FeatureList.FeatureRecord) == 1
assert gsub.FeatureList.FeatureRecord[0].FeatureTag == "ccmp"
assert len(gsub.LookupList.Lookup) == 1
assert len(gsub.FeatureVariations.FeatureVariationRecord) == 1
assert _substitution_features(gsub, rec_index=0) == [(0, "ccmp")]
# Add a feature variation for 'rlig' feature tag with the same condition
addFeatureVariations(
varfont, [([{"wght": (0.5, 1.0)}], {"B": "B.alt"})], featureTag="rlig"
)
# Should now have two feature records, two lookups, and one feature variation
# record, since the condition is the same for both feature variations
assert len(gsub.FeatureList.FeatureRecord) == 2
assert gsub.FeatureList.FeatureRecord[0].FeatureTag == "ccmp"
assert gsub.FeatureList.FeatureRecord[1].FeatureTag == "rlig"
assert len(gsub.LookupList.Lookup) == 2
assert len(gsub.FeatureVariations.FeatureVariationRecord) == 1
assert _substitution_features(gsub, rec_index=0) == [(0, "ccmp"), (1, "rlig")]
def _test_linear(n):
conds = []
for i in range(n):