diff --git a/Lib/fontTools/designspaceLib/__init__.py b/Lib/fontTools/designspaceLib/__init__.py index 0c58de254..9859e43f3 100644 --- a/Lib/fontTools/designspaceLib/__init__.py +++ b/Lib/fontTools/designspaceLib/__init__.py @@ -358,7 +358,7 @@ class BaseDocWriter(object): 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 @@ class BaseDocWriter(object): 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 @@ class BaseDocReader(LogMixin): 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( + " 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 @@ class DesignSpaceDocument(LogMixin, AsDictMixin): self.instances = [] self.axes = [] self.rules = [] + self.rulesProcessingLast = False self.default = None # name of the default master self.lib = {} diff --git a/Lib/fontTools/varLib/__init__.py b/Lib/fontTools/varLib/__init__.py index 59d060720..2dfe9e12c 100644 --- a/Lib/fontTools/varLib/__init__.py +++ b/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 _add_GSUB_feature_variations(font, axes, internal_axis_supports, rules): 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 @@ _DesignSpaceData = namedtuple( "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: diff --git a/Lib/fontTools/varLib/featureVars.py b/Lib/fontTools/varLib/featureVars.py index 3a7102a3d..287a885d7 100644 --- a/Lib/fontTools/varLib/featureVars.py +++ b/Lib/fontTools/varLib/featureVars.py @@ -11,7 +11,7 @@ from fontTools.otlLib.builder import buildLookup, buildSingleSubstSubtable 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 feature: + # make empty feature + # sort features, get feature index + # add 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) diff --git a/Tests/designspaceLib/data/test.designspace b/Tests/designspaceLib/data/test.designspace index 901a0ee06..e12f1568a 100644 --- a/Tests/designspaceLib/data/test.designspace +++ b/Tests/designspaceLib/data/test.designspace @@ -1,5 +1,5 @@ - + Wéíght @@ -13,7 +13,7 @@ - + diff --git a/Tests/designspaceLib/designspace_test.py b/Tests/designspaceLib/designspace_test.py index 7f0756bef..d134e04c7 100644 --- a/Tests/designspaceLib/designspace_test.py +++ b/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 diff --git a/Tests/varLib/data/FeatureVars.designspace b/Tests/varLib/data/FeatureVars.designspace index 9c958a5ac..902fade1b 100644 --- a/Tests/varLib/data/FeatureVars.designspace +++ b/Tests/varLib/data/FeatureVars.designspace @@ -6,7 +6,7 @@ Contrast - + diff --git a/Tests/varLib/data/test_results/FeatureVars.ttx b/Tests/varLib/data/test_results/FeatureVars.ttx index 93ad795f7..18d90aa2b 100644 --- a/Tests/varLib/data/test_results/FeatureVars.ttx +++ b/Tests/varLib/data/test_results/FeatureVars.ttx @@ -43,7 +43,7 @@ - + diff --git a/Tests/varLib/data/test_results/FeatureVars_rclt.ttx b/Tests/varLib/data/test_results/FeatureVars_rclt.ttx new file mode 100644 index 000000000..a9a998f48 --- /dev/null +++ b/Tests/varLib/data/test_results/FeatureVars_rclt.ttx @@ -0,0 +1,249 @@ + + + + + + + + wght + 0x0 + 0.0 + 368.0 + 1000.0 + 256 + + + + + cntr + 0x0 + 0.0 + 0.0 + 100.0 + 257 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/varLib/varLib_test.py b/Tests/varLib/varLib_test.py index 442b1aabe..f7289e72b 100644 --- a/Tests/varLib/varLib_test.py +++ b/Tests/varLib/varLib_test.py @@ -7,6 +7,7 @@ from fontTools.varLib import set_default_weight_width_slant from fontTools.designspaceLib import ( DesignSpaceDocumentError, DesignSpaceDocument, SourceDescriptor, ) +from fontTools.feaLib.builder import addOpenTypeFeaturesFromString import difflib import os import shutil @@ -107,7 +108,8 @@ class BuildTest(unittest.TestCase): return font, savepath def _run_varlib_build_test(self, designspace_name, font_name, tables, - expected_ttx_name, save_before_dump=False): + expected_ttx_name, save_before_dump=False, + post_process_master=None): suffix = '.ttf' ds_path = self.get_test_input(designspace_name + '.designspace') ufo_dir = self.get_test_input('master_ufo') @@ -116,7 +118,9 @@ class BuildTest(unittest.TestCase): self.temp_dir() ttx_paths = self.get_file_list(ttx_dir, '.ttx', font_name + '-') for path in ttx_paths: - self.compile_font(path, suffix, self.tempdir) + font, savepath = self.compile_font(path, suffix, self.tempdir) + if post_process_master is not None: + post_process_master(font, savepath) finder = lambda s: s.replace(ufo_dir, self.tempdir).replace('.ufo', suffix) varfont, model, _ = build(ds_path, finder) @@ -213,6 +217,47 @@ class BuildTest(unittest.TestCase): save_before_dump=True, ) + def test_varlib_build_feature_variations_with_existing_rclt(self): + """Designspace file contains element, used to build GSUB + FeatureVariations table. is specified to do its OT processing + "last", so a 'rclt' feature will be used or created. This test covers + the case when a 'rclt' already exists in the masters. + + We dynamically add a 'rclt' feature to an existing set of test + masters, to avoid adding more test data. + + The multiple languages are done to verify whether multiple existing + 'rclt' features are updated correctly. + """ + def add_rclt(font, savepath): + features = """ + languagesystem DFLT dflt; + languagesystem latn dflt; + languagesystem latn NLD; + + feature rclt { + script latn; + language NLD; + lookup A { + sub uni0041 by uni0061; + } A; + language dflt; + lookup B { + sub uni0041 by uni0061; + } B; + } rclt; + """ + addOpenTypeFeaturesFromString(font, features) + font.save(savepath) + self._run_varlib_build_test( + designspace_name="FeatureVars", + font_name="TestFamily", + tables=["fvar", "GSUB"], + expected_ttx_name="FeatureVars_rclt", + save_before_dump=True, + post_process_master=add_rclt, + ) + def test_varlib_gvar_explicit_delta(self): """The variable font contains a composite glyph odieresis which does not need a gvar entry, because all its deltas are 0, but it must be added