Merge pull request #1747 from justvanrossum/rclt_issue1625

[designspaceLib] [varLib] Allow FeatureVariations to be processed *after* other features

Lib/fontTools/designspaceLib/__init__.py

@@ -358,7 +358,7 @@ def getRuleDescriptor(cls):
     def __init__(self, documentPath, documentObject):
         self.path = documentPath
         self.documentObject = documentObject
-        self.documentVersion = "4.0"
+        self.documentVersion = "4.1"
         self.root = ET.Element("designspace")
         self.root.attrib['format'] = self.documentVersion
         self._axes = []     # for use by the writer only
@@ -371,7 +371,11 @@ def write(self, pretty=True, encoding="UTF-8", xml_declaration=True):
             self._addAxis(axisObject)
 
         if self.documentObject.rules:
-            self.root.append(ET.Element("rules"))
+            if getattr(self.documentObject, "rulesProcessingLast", False):
+                attributes = {"processing": "last"}
+            else:
+                attributes = {}
+            self.root.append(ET.Element("rules", attributes))
         for ruleObject in self.documentObject.rules:
             self._addRule(ruleObject)
 
@@ -675,6 +679,14 @@ def read(self):
     def readRules(self):
         # we also need to read any conditions that are outside of a condition set.
         rules = []
+        rulesElement = self.root.find(".rules")
+        if rulesElement is not None:
+            processingValue = rulesElement.attrib.get("processing", "first")
+            if processingValue not in {"first", "last"}:
+                raise DesignSpaceDocumentError(
+                    "<rules> processing attribute value is not valid: %r, "
+                    "expected 'first' or 'last'" % processingValue)
+            self.documentObject.rulesProcessingLast = processingValue == "last"
         for ruleElement in self.root.findall(".rules/rule"):
             ruleObject = self.ruleDescriptorClass()
             ruleName = ruleObject.name = ruleElement.attrib.get("name")
@@ -996,6 +1008,7 @@ def __init__(self, readerClass=None, writerClass=None):
         self.instances = []
         self.axes = []
         self.rules = []
+        self.rulesProcessingLast = False
         self.default = None         # name of the default master
 
         self.lib = {}

Lib/fontTools/varLib/__init__.py

@@ -613,7 +613,7 @@ 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):
+def _add_GSUB_feature_variations(font, axes, internal_axis_supports, rules, rulesProcessingLast):
 
 	def normalize(name, value):
 		return models.normalizeLocation(
@@ -648,7 +648,11 @@ def normalize(name, value):
 
 		conditional_subs.append((region, subs))
 
-	addFeatureVariations(font, conditional_subs)
+	if rulesProcessingLast:
+		featureTag = 'rclt'
+	else:
+		featureTag = 'rvrn'
+	addFeatureVariations(font, conditional_subs, featureTag)
 
 
 _DesignSpaceData = namedtuple(
@@ -661,6 +665,7 @@ def normalize(name, value):
 		"masters",
 		"instances",
 		"rules",
+		"rulesProcessingLast",
 	],
 )
 
@@ -761,6 +766,7 @@ def load_designspace(designspace):
 		masters,
 		instances,
 		ds.rules,
+		ds.rulesProcessingLast,
 	)
 
 
@@ -865,7 +871,7 @@ def build(designspace, master_finder=lambda s:s, exclude=[], optimize=True):
 	if 'cvar' not in exclude and 'glyf' in vf:
 		_merge_TTHinting(vf, model, master_fonts)
 	if 'GSUB' not in exclude and ds.rules:
-		_add_GSUB_feature_variations(vf, ds.axes, ds.internal_axis_supports, ds.rules)
+		_add_GSUB_feature_variations(vf, ds.axes, ds.internal_axis_supports, ds.rules, ds.rulesProcessingLast)
 	if 'CFF2' not in exclude and 'CFF ' in vf:
 		_add_CFF2(vf, model, master_fonts)
 		if "post" in vf:

Lib/fontTools/varLib/featureVars.py

@@ -11,7 +11,7 @@
 from collections import OrderedDict
 
 
-def addFeatureVariations(font, conditionalSubstitutions):
+def addFeatureVariations(font, conditionalSubstitutions, featureTag='rvrn'):
     """Add conditional substitutions to a Variable Font.
 
     The `conditionalSubstitutions` argument is a list of (Region, Substitutions)
@@ -43,7 +43,8 @@ def addFeatureVariations(font, conditionalSubstitutions):
     """
 
     addFeatureVariationsRaw(font,
-                            overlayFeatureVariations(conditionalSubstitutions))
+                            overlayFeatureVariations(conditionalSubstitutions),
+                            featureTag)
 
 def overlayFeatureVariations(conditionalSubstitutions):
     """Compute overlaps between all conditional substitutions.
@@ -255,15 +256,15 @@ def cleanupBox(box):
 # Low level implementation
 #
 
-def addFeatureVariationsRaw(font, conditionalSubstitutions):
+def addFeatureVariationsRaw(font, conditionalSubstitutions, featureTag='rvrn'):
     """Low level implementation of addFeatureVariations that directly
     models the possibilities of the FeatureVariations table."""
 
     #
-    # assert there is no 'rvrn' feature
-    # make dummy 'rvrn' feature with no lookups
-    # sort features, get 'rvrn' feature index
-    # add 'rvrn' feature to all scripts
+    # if there is no <featureTag> feature:
+    #     make empty <featureTag> feature
+    #     sort features, get <featureTag> feature index
+    #     add <featureTag> feature to all scripts
     # make lookups
     # add feature variations
     #
@@ -278,20 +279,25 @@ def addFeatureVariationsRaw(font, conditionalSubstitutions):
 
     gsub.FeatureVariations = None  # delete any existing FeatureVariations
 
-    for feature in gsub.FeatureList.FeatureRecord:
-        assert feature.FeatureTag != 'rvrn'
+    varFeatureIndices = []
+    for index, feature in enumerate(gsub.FeatureList.FeatureRecord):
+        if feature.FeatureTag == featureTag:
+            varFeatureIndices.append(index)
 
-    rvrnFeature = buildFeatureRecord('rvrn', [])
-    gsub.FeatureList.FeatureRecord.append(rvrnFeature)
-    gsub.FeatureList.FeatureCount = len(gsub.FeatureList.FeatureRecord)
+    if not varFeatureIndices:
+        varFeature = buildFeatureRecord(featureTag, [])
+        gsub.FeatureList.FeatureRecord.append(varFeature)
+        gsub.FeatureList.FeatureCount = len(gsub.FeatureList.FeatureRecord)
 
-    sortFeatureList(gsub)
-    rvrnFeatureIndex = gsub.FeatureList.FeatureRecord.index(rvrnFeature)
+        sortFeatureList(gsub)
+        varFeatureIndex = gsub.FeatureList.FeatureRecord.index(varFeature)
 
-    for scriptRecord in gsub.ScriptList.ScriptRecord:
-        langSystems = [lsr.LangSys for lsr in scriptRecord.Script.LangSysRecord]
-        for langSys in [scriptRecord.Script.DefaultLangSys] + langSystems:
-            langSys.FeatureIndex.append(rvrnFeatureIndex)
+        for scriptRecord in gsub.ScriptList.ScriptRecord:
+            langSystems = [lsr.LangSys for lsr in scriptRecord.Script.LangSysRecord]
+            for langSys in [scriptRecord.Script.DefaultLangSys] + langSystems:
+                langSys.FeatureIndex.append(varFeatureIndex)
+
+        varFeatureIndices = [varFeatureIndex]
 
     # setup lookups
 
@@ -311,8 +317,11 @@ def addFeatureVariationsRaw(font, conditionalSubstitutions):
             conditionTable.append(ct)
 
         lookupIndices = [lookupMap[subst] for subst in substitutions]
-        record = buildFeatureTableSubstitutionRecord(rvrnFeatureIndex, lookupIndices)
-        featureVariationRecords.append(buildFeatureVariationRecord(conditionTable, [record]))
+        records = []
+        for varFeatureIndex in varFeatureIndices:
+            existingLookupIndices = gsub.FeatureList.FeatureRecord[varFeatureIndex].Feature.LookupListIndex
+            records.append(buildFeatureTableSubstitutionRecord(varFeatureIndex, existingLookupIndices + lookupIndices))
+        featureVariationRecords.append(buildFeatureVariationRecord(conditionTable, records))
 
     gsub.FeatureVariations = buildFeatureVariations(featureVariationRecords)
 

Tests/designspaceLib/data/test.designspace

@@ -1,5 +1,5 @@
 <?xml version='1.0' encoding='UTF-8'?>
-<designspace format="4.0">
+<designspace format="4.1">
   <axes>
     <axis tag="wght" name="weight" minimum="0" maximum="1000" default="0">
       <labelname xml:lang="en">Wéíght</labelname>
@@ -13,7 +13,7 @@
       <map input="1000" output="990"/>
     </axis>
   </axes>
-  <rules>
+  <rules processing="last">
     <rule name="named.rule.1">
       <conditionset>
         <condition name="axisName_a" minimum="0" maximum="1"/>

Tests/designspaceLib/designspace_test.py

@@ -49,6 +49,7 @@ def test_fill_document(tmpdir):
     instancePath1 = os.path.join(tmpdir, "instances", "instanceTest1.ufo")
     instancePath2 = os.path.join(tmpdir, "instances", "instanceTest2.ufo")
     doc = DesignSpaceDocument()
+    doc.rulesProcessingLast = True
 
     # write some axes
     a1 = AxisDescriptor()
@@ -698,6 +699,7 @@ def test_rulesDocument(tmpdir):
     testDocPath = os.path.join(tmpdir, "testRules.designspace")
     testDocPath2 = os.path.join(tmpdir, "testRules_roundtrip.designspace")
     doc = DesignSpaceDocument()
+    doc.rulesProcessingLast = True
     a1 = AxisDescriptor()
     a1.minimum = 0
     a1.maximum = 1000
@@ -741,6 +743,7 @@ def test_rulesDocument(tmpdir):
     _addUnwrappedCondition(testDocPath)
     doc2 = DesignSpaceDocument()
     doc2.read(testDocPath)
+    assert doc2.rulesProcessingLast
     assert len(doc2.axes) == 2
     assert len(doc2.rules) == 1
     assert len(doc2.rules[0].conditionSets) == 2

Tests/varLib/data/FeatureVars.designspace

@@ -6,7 +6,7 @@
             <labelname xml:lang="en">Contrast</labelname>
         </axis>
     </axes>
-    <rules>
+    <rules processing="last">
         <rule name="dollar-stroke">
             <conditionset>
                 <condition name="weight" minimum="500" /> <!-- intentionally omitted maximum -->

Tests/varLib/data/test_results/FeatureVars.ttx

@@ -43,7 +43,7 @@
     <FeatureList>
       <!-- FeatureCount=1 -->
       <FeatureRecord index="0">
-        <FeatureTag value="rvrn"/>
+        <FeatureTag value="rclt"/>
         <Feature>
           <!-- LookupCount=0 -->
         </Feature>