From d2ea72cbec757db7bda643fe1268bbb76a490367 Mon Sep 17 00:00:00 2001 From: justvanrossum Date: Thu, 17 Oct 2019 20:01:05 +0200 Subject: [PATCH 01/10] addFeatureVariations(): allow the feature tag to be specified (default to 'rvrn'); allow said feature to already exist, in which case we append new lookup indices to the existing feature --- Lib/fontTools/varLib/featureVars.py | 46 +++++++++++++++++------------ 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/Lib/fontTools/varLib/featureVars.py b/Lib/fontTools/varLib/featureVars.py index 3a7102a3d7..803300b1ba 100644 --- a/Lib/fontTools/varLib/featureVars.py +++ b/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 feature: + # make empty feature + # sort features, get feature index + # add feature to all scripts # make lookups # add feature variations # @@ -278,20 +279,27 @@ def addFeatureVariationsRaw(font, conditionalSubstitutions): gsub.FeatureVariations = None # delete any existing FeatureVariations - for feature in gsub.FeatureList.FeatureRecord: - assert feature.FeatureTag != 'rvrn' + varFeature = None + for index, feature in enumerate(gsub.FeatureList.FeatureRecord): + if feature.FeatureTag == featureTag: + varFeature = feature + varFeatureIndex = index + existingLookupIndices = feature.Feature.LookupListIndex + break - rvrnFeature = buildFeatureRecord('rvrn', []) - gsub.FeatureList.FeatureRecord.append(rvrnFeature) - gsub.FeatureList.FeatureCount = len(gsub.FeatureList.FeatureRecord) + if varFeature is None: + existingLookupIndices = [] + 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) # setup lookups @@ -311,7 +319,7 @@ def addFeatureVariationsRaw(font, conditionalSubstitutions): conditionTable.append(ct) lookupIndices = [lookupMap[subst] for subst in substitutions] - record = buildFeatureTableSubstitutionRecord(rvrnFeatureIndex, lookupIndices) + record = buildFeatureTableSubstitutionRecord(varFeatureIndex, existingLookupIndices + lookupIndices) featureVariationRecords.append(buildFeatureVariationRecord(conditionTable, [record])) gsub.FeatureVariations = buildFeatureVariations(featureVariationRecords) From db04262bedf3a0ca34de8ffa8d7870c5ec4d265e Mon Sep 17 00:00:00 2001 From: justvanrossum Date: Thu, 17 Oct 2019 20:46:59 +0200 Subject: [PATCH 02/10] support multiple features --- Lib/fontTools/varLib/featureVars.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/Lib/fontTools/varLib/featureVars.py b/Lib/fontTools/varLib/featureVars.py index 803300b1ba..287a885d75 100644 --- a/Lib/fontTools/varLib/featureVars.py +++ b/Lib/fontTools/varLib/featureVars.py @@ -279,16 +279,12 @@ def addFeatureVariationsRaw(font, conditionalSubstitutions, featureTag='rvrn'): gsub.FeatureVariations = None # delete any existing FeatureVariations - varFeature = None + varFeatureIndices = [] for index, feature in enumerate(gsub.FeatureList.FeatureRecord): if feature.FeatureTag == featureTag: - varFeature = feature - varFeatureIndex = index - existingLookupIndices = feature.Feature.LookupListIndex - break + varFeatureIndices.append(index) - if varFeature is None: - existingLookupIndices = [] + if not varFeatureIndices: varFeature = buildFeatureRecord(featureTag, []) gsub.FeatureList.FeatureRecord.append(varFeature) gsub.FeatureList.FeatureCount = len(gsub.FeatureList.FeatureRecord) @@ -301,6 +297,8 @@ def addFeatureVariationsRaw(font, conditionalSubstitutions, featureTag='rvrn'): for langSys in [scriptRecord.Script.DefaultLangSys] + langSystems: langSys.FeatureIndex.append(varFeatureIndex) + varFeatureIndices = [varFeatureIndex] + # setup lookups # turn substitution dicts into tuples of tuples, so they are hashable @@ -319,8 +317,11 @@ def addFeatureVariationsRaw(font, conditionalSubstitutions, featureTag='rvrn'): conditionTable.append(ct) lookupIndices = [lookupMap[subst] for subst in substitutions] - record = buildFeatureTableSubstitutionRecord(varFeatureIndex, existingLookupIndices + 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) From d42b7d74cae2bfc50948d1beef748761a5b64d57 Mon Sep 17 00:00:00 2001 From: justvanrossum Date: Fri, 18 Oct 2019 13:20:28 +0200 Subject: [PATCH 03/10] Implement attribute 'processing' according to spec update #1750, as well as the related doc.rulesProcessingLast flag --- Lib/fontTools/designspaceLib/__init__.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/Lib/fontTools/designspaceLib/__init__.py b/Lib/fontTools/designspaceLib/__init__.py index 0c58de2541..a30c271066 100644 --- a/Lib/fontTools/designspaceLib/__init__.py +++ b/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,12 @@ 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(" processing attribute value is not valid: %r, expected 'first' or 'last'") + 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 +1006,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 = {} From 0ee3f01f0c44bcfe21f77a884aca4ab1fae1e36b Mon Sep 17 00:00:00 2001 From: justvanrossum Date: Fri, 18 Oct 2019 13:21:06 +0200 Subject: [PATCH 04/10] use designspace.rulesProcessingLast flag to determine whether we should use 'rvrn' or 'rclt' --- Lib/fontTools/varLib/__init__.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Lib/fontTools/varLib/__init__.py b/Lib/fontTools/varLib/__init__.py index 59d0607201..2e93452206 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 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, + getattr(ds, "rulesProcessingLast", False), ) @@ -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: From f9b04572f614082ac3734f3933ad79dd8f5049e7 Mon Sep 17 00:00:00 2001 From: justvanrossum Date: Fri, 18 Oct 2019 13:24:32 +0200 Subject: [PATCH 05/10] fix error message, reformatted long line --- Lib/fontTools/designspaceLib/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/fontTools/designspaceLib/__init__.py b/Lib/fontTools/designspaceLib/__init__.py index a30c271066..9859e43f3a 100644 --- a/Lib/fontTools/designspaceLib/__init__.py +++ b/Lib/fontTools/designspaceLib/__init__.py @@ -683,7 +683,9 @@ def readRules(self): 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'") + 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() From f4925fff4447d221d0c262041e8bb5536d903f94 Mon Sep 17 00:00:00 2001 From: justvanrossum Date: Fri, 18 Oct 2019 13:33:29 +0200 Subject: [PATCH 06/10] perform some testing with and doc.rulesProcessingLast --- Tests/designspaceLib/data/test.designspace | 4 ++-- Tests/designspaceLib/designspace_test.py | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Tests/designspaceLib/data/test.designspace b/Tests/designspaceLib/data/test.designspace index 901a0ee064..e12f1568a3 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 7f0756befe..d134e04c7e 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 From 4322a3f95d572e56b6134c996f27754c77476313 Mon Sep 17 00:00:00 2001 From: justvanrossum Date: Fri, 18 Oct 2019 13:36:02 +0200 Subject: [PATCH 07/10] more testing of rules-processing-last --- Tests/varLib/data/FeatureVars.designspace | 2 +- Tests/varLib/data/test_results/FeatureVars.ttx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/varLib/data/FeatureVars.designspace b/Tests/varLib/data/FeatureVars.designspace index 9c958a5acd..902fade1bd 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 93ad795f79..18d90aa2b9 100644 --- a/Tests/varLib/data/test_results/FeatureVars.ttx +++ b/Tests/varLib/data/test_results/FeatureVars.ttx @@ -43,7 +43,7 @@ - + From 7bfeb1ab975efaad3c1e7c66d4bad616b8787a11 Mon Sep 17 00:00:00 2001 From: justvanrossum Date: Sat, 19 Oct 2019 11:17:26 +0200 Subject: [PATCH 08/10] assume ds is a DesignSpaceDocument instance and therefore is known to have the rulesProcessingLast attribute --- Lib/fontTools/varLib/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/fontTools/varLib/__init__.py b/Lib/fontTools/varLib/__init__.py index 2e93452206..2dfe9e12c4 100644 --- a/Lib/fontTools/varLib/__init__.py +++ b/Lib/fontTools/varLib/__init__.py @@ -766,7 +766,7 @@ def load_designspace(designspace): masters, instances, ds.rules, - getattr(ds, "rulesProcessingLast", False), + ds.rulesProcessingLast, ) From e2bac99eb8c7e89bafcac7be8a866cc709335ca2 Mon Sep 17 00:00:00 2001 From: justvanrossum Date: Sun, 20 Oct 2019 09:19:40 +0200 Subject: [PATCH 09/10] Test adding FeatureVariations with an existing 'rclt' feature --- Tests/varLib/varLib_test.py | 49 +++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/Tests/varLib/varLib_test.py b/Tests/varLib/varLib_test.py index 442b1aabe8..f7289e72be 100644 --- a/Tests/varLib/varLib_test.py +++ b/Tests/varLib/varLib_test.py @@ -7,6 +7,7 @@ from fontTools.designspaceLib import ( DesignSpaceDocumentError, DesignSpaceDocument, SourceDescriptor, ) +from fontTools.feaLib.builder import addOpenTypeFeaturesFromString import difflib import os import shutil @@ -107,7 +108,8 @@ def compile_font(self, path, suffix, temp_dir): 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 @@ def _run_varlib_build_test(self, designspace_name, font_name, tables, 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 @@ def test_varlib_build_feature_variations(self): 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 From 76b8517102b0dc0c62c1411a48a738124496976e Mon Sep 17 00:00:00 2001 From: justvanrossum Date: Sun, 20 Oct 2019 09:25:50 +0200 Subject: [PATCH 10/10] ooops, forgot to add the expected results file --- .../data/test_results/FeatureVars_rclt.ttx | 249 ++++++++++++++++++ 1 file changed, 249 insertions(+) create mode 100644 Tests/varLib/data/test_results/FeatureVars_rclt.ttx 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 0000000000..a9a998f48d --- /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 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +