Skip to content

Commit

Permalink
[featureVars] Re-use FeatureVariationRecord's when possible
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
khaledhosny committed Jan 10, 2024
1 parent e3cde46 commit 1c25210
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 4 deletions.
35 changes: 31 additions & 4 deletions Lib/fontTools/varLib/featureVars.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand All @@ -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: "
Expand Down Expand Up @@ -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.
Expand Down
41 changes: 41 additions & 0 deletions Tests/feaLib/builder_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,47 @@ def test_unmarked_ignore_statement(self):
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."""
Expand Down
32 changes: 32 additions & 0 deletions Tests/varLib/featureVars_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit 1c25210

Please sign in to comment.