Merge pull request #3360 from fonttools/fea-vars-multiple-tags

[featureVars] allow to assign same subst lookup to several feature tags
2023-12-01 18:40:35 +00:00 · 2023-12-01 18:40:35 +00:00 · 0b05cec0a3
commit 0b05cec0a3
parent ee3d7c8f80 c46478335e
5 changed files with 117 additions and 40 deletions
--- a/Doc/source/designspaceLib/xml.rst
+++ b/Doc/source/designspaceLib/xml.rst
@ -438,8 +438,8 @@ glyphname pairs: the glyphs that need to be substituted. For a rule to be trigge
      See the following issues for more information:
      `fontTools#1371 <https://github.com/fonttools/fonttools/issues/1371#issuecomment-590214572>`__
      `fontTools#2050 <https://github.com/fonttools/fonttools/issues/2050#issuecomment-678691020>`__
-   -  If you want to use a different feature altogether, e.g. ``calt``,
-      use the lib key ``com.github.fonttools.varLib.featureVarsFeatureTag``
+   -  If you want to use a different feature(s) altogether, e.g. ``calt``,
+      use the lib key ``com.github.fonttools.varLib.featureVarsFeatureTag``.

      .. code:: xml

@ -450,6 +450,9 @@ glyphname pairs: the glyphs that need to be substituted. For a rule to be trigge
               </dict>
           </lib>

+      This can also take a comma-separated list of feature tags, e.g. ``salt,ss01``,
+      if you wish the same rules to be applied with several features.
+


 ``<rule>`` element
--- a/Lib/fontTools/varLib/init.py
+++ b/Lib/fontTools/varLib/init.py
@ -52,7 +52,8 @@ from .errors import VarLibError, VarLibValidationError
 log = logging.getLogger("fontTools.varLib")

 # This is a lib key for the designspace document. The value should be
-# an OpenType feature tag, to be used as the FeatureVariations feature.
+# a comma-separated list of OpenType feature tag(s), to be used as the
+# FeatureVariations feature.
 # If present, the DesignSpace <rules processing="..."> flag is ignored.
 FEAVAR_FEATURETAG_LIB_KEY = "com.github.fonttools.varLib.featureVarsFeatureTag"

@ -781,7 +782,9 @@ def _merge_OTL(font, model, master_fonts, axisTags):
        font["GPOS"].table.remap_device_varidxes(varidx_map)


-def _add_GSUB_feature_variations(font, axes, internal_axis_supports, rules, featureTag):
+def _add_GSUB_feature_variations(
+    font, axes, internal_axis_supports, rules, featureTags
+):
    def normalize(name, value):
        return models.normalizeLocation({name: value}, internal_axis_supports)[name]

@ -812,7 +815,7 @@ def _add_GSUB_feature_variations(font, axes, internal_axis_supports, rules, feat

        conditional_subs.append((region, subs))

-    addFeatureVariations(font, conditional_subs, featureTag)
+    addFeatureVariations(font, conditional_subs, featureTags)


 _DesignSpaceData = namedtuple(
@ -1204,11 +1207,9 @@ def build(
    if "cvar" not in exclude and "glyf" in vf:
        _merge_TTHinting(vf, model, master_fonts)
    if "GSUB" not in exclude and ds.rules:
-        featureTag = ds.lib.get(
-            FEAVAR_FEATURETAG_LIB_KEY, "rclt" if ds.rulesProcessingLast else "rvrn"
-        )
+        featureTags = _feature_variations_tags(ds)
        _add_GSUB_feature_variations(
-            vf, ds.axes, ds.internal_axis_supports, ds.rules, featureTag
+            vf, ds.axes, ds.internal_axis_supports, ds.rules, featureTags
        )
    if "CFF2" not in exclude and ("CFF " in vf or "CFF2" in vf):
        _add_CFF2(vf, model, master_fonts)
@ -1299,6 +1300,14 @@ class MasterFinder(object):
        return os.path.normpath(path)


+def _feature_variations_tags(ds):
+    raw_tags = ds.lib.get(
+        FEAVAR_FEATURETAG_LIB_KEY,
+        "rclt" if ds.rulesProcessingLast else "rvrn",
+    )
+    return sorted({t.strip() for t in raw_tags.split(",")})
+
+
 def main(args=None):
    """Build variable fonts from a designspace file and masters"""
    from argparse import ArgumentParser
--- a/Lib/fontTools/varLib/featureVars.py
+++ b/Lib/fontTools/varLib/featureVars.py
@ -43,9 +43,18 @@ def addFeatureVariations(font, conditionalSubstitutions, featureTag="rvrn"):
    # ... ]
    # >>> addFeatureVariations(f, condSubst)
    # >>> f.save(dstPath)
+
+    The `featureTag` parameter takes either a str or a iterable of str (the single str
+    is kept for backwards compatibility), and defines which feature(s) will be
+    associated with the feature variations.
+    Note, if this is "rvrn", then the substitution lookup will be inserted at the
+    beginning of the lookup list so that it is processed before others, otherwise
+    for any other feature tags it will be appended last.
    """

-    processLast = featureTag != "rvrn"
+    # process first when "rvrn" is the only listed tag
+    featureTags = [featureTag] if isinstance(featureTag, str) else sorted(featureTag)
+    processLast = "rvrn" not in featureTags or len(featureTags) > 1

    _checkSubstitutionGlyphsExist(
        glyphNames=set(font.getGlyphOrder()),
@ -75,7 +84,7 @@ def addFeatureVariations(font, conditionalSubstitutions, featureTag="rvrn"):
            (conditionSet, [lookupMap[s] for s in substitutions])
        )

-    addFeatureVariationsRaw(font, font["GSUB"].table, conditionsAndLookups, featureTag)
+    addFeatureVariationsRaw(font, font["GSUB"].table, conditionsAndLookups, featureTags)


 def _checkSubstitutionGlyphsExist(glyphNames, substitutions):
@ -324,13 +333,16 @@ def addFeatureVariationsRaw(font, table, conditionalSubstitutions, featureTag="r
    """Low level implementation of addFeatureVariations that directly
    models the possibilities of the FeatureVariations table."""

-    processLast = featureTag != "rvrn"
+    featureTags = [featureTag] if isinstance(featureTag, str) else sorted(featureTag)
+    processLast = "rvrn" not in featureTags or len(featureTags) > 1

    #
-    # if there is no <featureTag> feature:
+    # if a <featureTag> feature is not present:
    #     make empty <featureTag> feature
    #     sort features, get <featureTag> feature index
    #     add <featureTag> feature to all scripts
+    # if a <featureTag> feature is present:
+    #     reuse <featureTag> feature index
    # make lookups
    # add feature variations
    #
@ -339,31 +351,48 @@ def addFeatureVariationsRaw(font, table, conditionalSubstitutions, featureTag="r

    table.FeatureVariations = None  # delete any existing FeatureVariations

-    varFeatureIndices = []
-    for index, feature in enumerate(table.FeatureList.FeatureRecord):
-        if feature.FeatureTag == featureTag:
-            varFeatureIndices.append(index)
+    varFeatureIndices = set()

-    if not varFeatureIndices:
-        varFeature = buildFeatureRecord(featureTag, [])
-        table.FeatureList.FeatureRecord.append(varFeature)
+    existingTags = {
+        feature.FeatureTag
+        for feature in table.FeatureList.FeatureRecord
+        if feature.FeatureTag in featureTags
+    }
+
+    newTags = set(featureTags) - existingTags
+    if newTags:
+        varFeatures = []
+        for featureTag in sorted(newTags):
+            varFeature = buildFeatureRecord(featureTag, [])
+            table.FeatureList.FeatureRecord.append(varFeature)
+            varFeatures.append(varFeature)
        table.FeatureList.FeatureCount = len(table.FeatureList.FeatureRecord)

        sortFeatureList(table)
-        varFeatureIndex = table.FeatureList.FeatureRecord.index(varFeature)

-        for scriptRecord in table.ScriptList.ScriptRecord:
-            if scriptRecord.Script.DefaultLangSys is None:
-                raise VarLibError(
-                    "Feature variations require that the script "
-                    f"'{scriptRecord.ScriptTag}' defines a default language system."
-                )
-            langSystems = [lsr.LangSys for lsr in scriptRecord.Script.LangSysRecord]
-            for langSys in [scriptRecord.Script.DefaultLangSys] + langSystems:
-                langSys.FeatureIndex.append(varFeatureIndex)
-                langSys.FeatureCount = len(langSys.FeatureIndex)
+        for varFeature in varFeatures:
+            varFeatureIndex = table.FeatureList.FeatureRecord.index(varFeature)

-        varFeatureIndices = [varFeatureIndex]
+            for scriptRecord in table.ScriptList.ScriptRecord:
+                if scriptRecord.Script.DefaultLangSys is None:
+                    raise VarLibError(
+                        "Feature variations require that the script "
+                        f"'{scriptRecord.ScriptTag}' defines a default language system."
+                    )
+                langSystems = [lsr.LangSys for lsr in scriptRecord.Script.LangSysRecord]
+                for langSys in [scriptRecord.Script.DefaultLangSys] + langSystems:
+                    langSys.FeatureIndex.append(varFeatureIndex)
+                    langSys.FeatureCount = len(langSys.FeatureIndex)
+            varFeatureIndices.add(varFeatureIndex)
+
+    if existingTags:
+        # indices may have changed if we inserted new features and sorted feature list
+        # so we must do this after the above
+        varFeatureIndices.update(
+            index
+            for index, feature in enumerate(table.FeatureList.FeatureRecord)
+            if feature.FeatureTag in existingTags
+        )

    axisIndices = {
        axis.axisTag: axisIndex for axisIndex, axis in enumerate(font["fvar"].axes)
@ -380,7 +409,7 @@ def addFeatureVariationsRaw(font, table, conditionalSubstitutions, featureTag="r
            ct = buildConditionTable(axisIndices[axisTag], minValue, maxValue)
            conditionTable.append(ct)
        records = []
-        for varFeatureIndex in varFeatureIndices:
+        for varFeatureIndex in sorted(varFeatureIndices):
            existingLookupIndices = table.FeatureList.FeatureRecord[
                varFeatureIndex
            ].Feature.LookupListIndex
--- a/Tests/varLib/data/FeatureVarsCustomTag.designspace
+++ b/Tests/varLib/data/FeatureVarsCustomTag.designspace
@ -71,7 +71,7 @@
    <lib>
        <dict>
            <key>com.github.fonttools.varLib.featureVarsFeatureTag</key>
-            <string>calt</string>
+            <string>rclt,calt</string>
        </dict>
    </lib>
 </designspace>
--- a/Tests/varLib/data/test_results/FeatureVarsCustomTag.ttx
+++ b/Tests/varLib/data/test_results/FeatureVarsCustomTag.ttx
@ -33,21 +33,28 @@
        <Script>
          <DefaultLangSys>
            <ReqFeatureIndex value="65535"/>
-            <!-- FeatureCount=1 -->
+            <!-- FeatureCount=2 -->
            <FeatureIndex index="0" value="0"/>
+            <FeatureIndex index="1" value="1"/>
          </DefaultLangSys>
          <!-- LangSysCount=0 -->
        </Script>
      </ScriptRecord>
    </ScriptList>
    <FeatureList>
-      <!-- FeatureCount=1 -->
+      <!-- FeatureCount=2 -->
      <FeatureRecord index="0">
        <FeatureTag value="calt"/>
        <Feature>
          <!-- LookupCount=0 -->
        </Feature>
      </FeatureRecord>
+      <FeatureRecord index="1">
+        <FeatureTag value="rclt"/>
+        <Feature>
+          <!-- LookupCount=0 -->
+        </Feature>
+      </FeatureRecord>
    </FeatureList>
    <LookupList>
      <!-- LookupCount=3 -->
@ -95,7 +102,7 @@
        </ConditionSet>
        <FeatureTableSubstitution>
          <Version value="0x00010000"/>
-          <!-- SubstitutionCount=1 -->
+          <!-- SubstitutionCount=2 -->
          <SubstitutionRecord index="0">
            <FeatureIndex value="0"/>
            <Feature>
@ -104,6 +111,14 @@
              <LookupListIndex index="1" value="1"/>
            </Feature>
          </SubstitutionRecord>
+          <SubstitutionRecord index="1">
+            <FeatureIndex value="1"/>
+            <Feature>
+              <!-- LookupCount=2 -->
+              <LookupListIndex index="0" value="0"/>
+              <LookupListIndex index="1" value="1"/>
+            </Feature>
+          </SubstitutionRecord>
        </FeatureTableSubstitution>
      </FeatureVariationRecord>
      <FeatureVariationRecord index="1">
@ -122,7 +137,7 @@
        </ConditionSet>
        <FeatureTableSubstitution>
          <Version value="0x00010000"/>
-          <!-- SubstitutionCount=1 -->
+          <!-- SubstitutionCount=2 -->
          <SubstitutionRecord index="0">
            <FeatureIndex value="0"/>
            <Feature>
@ -130,6 +145,13 @@
              <LookupListIndex index="0" value="2"/>
            </Feature>
          </SubstitutionRecord>
+          <SubstitutionRecord index="1">
+            <FeatureIndex value="1"/>
+            <Feature>
+              <!-- LookupCount=1 -->
+              <LookupListIndex index="0" value="2"/>
+            </Feature>
+          </SubstitutionRecord>
        </FeatureTableSubstitution>
      </FeatureVariationRecord>
      <FeatureVariationRecord index="2">
@ -143,7 +165,7 @@
        </ConditionSet>
        <FeatureTableSubstitution>
          <Version value="0x00010000"/>
-          <!-- SubstitutionCount=1 -->
+          <!-- SubstitutionCount=2 -->
          <SubstitutionRecord index="0">
            <FeatureIndex value="0"/>
            <Feature>
@ -151,6 +173,13 @@
              <LookupListIndex index="0" value="1"/>
            </Feature>
          </SubstitutionRecord>
+          <SubstitutionRecord index="1">
+            <FeatureIndex value="1"/>
+            <Feature>
+              <!-- LookupCount=1 -->
+              <LookupListIndex index="0" value="1"/>
+            </Feature>
+          </SubstitutionRecord>
        </FeatureTableSubstitution>
      </FeatureVariationRecord>
      <FeatureVariationRecord index="3">
@ -164,7 +193,7 @@
        </ConditionSet>
        <FeatureTableSubstitution>
          <Version value="0x00010000"/>
-          <!-- SubstitutionCount=1 -->
+          <!-- SubstitutionCount=2 -->
          <SubstitutionRecord index="0">
            <FeatureIndex value="0"/>
            <Feature>
@ -172,6 +201,13 @@
              <LookupListIndex index="0" value="0"/>
            </Feature>
          </SubstitutionRecord>
+          <SubstitutionRecord index="1">
+            <FeatureIndex value="1"/>
+            <Feature>
+              <!-- LookupCount=1 -->
+              <LookupListIndex index="0" value="0"/>
+            </Feature>
+          </SubstitutionRecord>
        </FeatureTableSubstitution>
      </FeatureVariationRecord>
    </FeatureVariations>