From 086de222fa68f222d7bf8bde88d2cacbc756b3ae Mon Sep 17 00:00:00 2001 From: Erik van Blokland Date: Wed, 23 May 2018 13:19:23 +0200 Subject: [PATCH] WIP axes are compulsory, so don't accomodate missing axes. Raise error if document is missing axes. We know what the default location is before we start reading the masters and there is no guessing. findDefault() method: default master is at all default values of all axes, or if that fails, the master with the copyInfo flag. Warn if a location contains an axis that is not defined in the axes element. Otherwise ignore this value, no guessing. Remove rulesToFeature() function. --- Lib/fontTools/designspaceLib/__init__.py | 189 +++++++---------------- Tests/designspaceLib/designspace_test.py | 149 +++++++----------- 2 files changed, 111 insertions(+), 227 deletions(-) diff --git a/Lib/fontTools/designspaceLib/__init__.py b/Lib/fontTools/designspaceLib/__init__.py index 6d2d84d62..afc7111f1 100644 --- a/Lib/fontTools/designspaceLib/__init__.py +++ b/Lib/fontTools/designspaceLib/__init__.py @@ -6,18 +6,17 @@ import logging import os import posixpath import plistlib +import warnings + try: import xml.etree.cElementTree as ET except ImportError: import xml.etree.ElementTree as ET -# from mutatorMath.objects.location import biasFromLocations, Location """ designSpaceDocument - read and write designspace files - - axes must be defined. - - warpmap is stored in its axis element """ __all__ = [ @@ -726,8 +725,7 @@ class BaseDocReader(object): # read the axes elements, including the warp map. axes = [] if len(self.root.findall(".axes/axis"))==0: - self.guessAxes() - self._strictAxisNames = False + raise DesignSpaceDocumentError("No axes defined.") return for axisElement in self.root.findall(".axes/axis"): axisObject = self.axisDescriptorClass() @@ -750,10 +748,13 @@ class BaseDocReader(object): axisObject.labelNames[lang] = labelName self.documentObject.axes.append(axisObject) self.axisDefaults[axisObject.name] = axisObject.default + self.documentObject.defaultLoc = self.axisDefaults def _locationFromElement(self, locationElement): # mostly duplicated from readLocationElement, Needs Resolve. loc = {} + # make sure all locations start with the defaults + loc.update(self.axisDefaults) for dimensionElement in locationElement.findall(".dimension"): dimName = dimensionElement.attrib.get("name") xValue = yValue = None @@ -774,42 +775,42 @@ class BaseDocReader(object): loc[dimName] = xValue return loc - def guessAxes(self): - # Called when we have no axes element in the file. - # Look at all locations and collect the axis names and values - # assumptions: - # look for the default value on an axis from a master location - # Needs deprecation warning - allLocations = [] - minima = {} - maxima = {} - for locationElement in self.root.findall(".sources/source/location"): - allLocations.append(self._locationFromElement(locationElement)) - for locationElement in self.root.findall(".instances/instance/location"): - allLocations.append(self._locationFromElement(locationElement)) - for loc in allLocations: - for dimName, value in loc.items(): - if not isinstance(value, tuple): - value = [value] - for v in value: - if dimName not in minima: - minima[dimName] = v - continue - if minima[dimName] > v: - minima[dimName] = v - if dimName not in maxima: - maxima[dimName] = v - continue - if maxima[dimName] < v: - maxima[dimName] = v - newAxes = [] - for axisName in maxima.keys(): - a = self.axisDescriptorClass() - a.default = a.minimum = minima[axisName] - a.maximum = maxima[axisName] - a.name = axisName - a.tag, a.labelNames = tagForAxisName(axisName) - self.documentObject.axes.append(a) + # def guessAxes(self): + # # Called when we have no axes element in the file. + # # Look at all locations and collect the axis names and values + # # assumptions: + # # look for the default value on an axis from a master location + # # Needs deprecation warning + # allLocations = [] + # minima = {} + # maxima = {} + # for locationElement in self.root.findall(".sources/source/location"): + # allLocations.append(self._locationFromElement(locationElement)) + # for locationElement in self.root.findall(".instances/instance/location"): + # allLocations.append(self._locationFromElement(locationElement)) + # for loc in allLocations: + # for dimName, value in loc.items(): + # if not isinstance(value, tuple): + # value = [value] + # for v in value: + # if dimName not in minima: + # minima[dimName] = v + # continue + # if minima[dimName] > v: + # minima[dimName] = v + # if dimName not in maxima: + # maxima[dimName] = v + # continue + # if maxima[dimName] < v: + # maxima[dimName] = v + # newAxes = [] + # for axisName in maxima.keys(): + # a = self.axisDescriptorClass() + # a.default = a.minimum = minima[axisName] + # a.maximum = maxima[axisName] + # a.name = axisName + # a.tag, a.labelNames = tagForAxisName(axisName) + # self.documentObject.axes.append(a) def readSources(self): for sourceCount, sourceElement in enumerate(self.root.findall(".sources/source")): @@ -874,10 +875,8 @@ class BaseDocReader(object): for dimensionElement in locationElement.findall(".dimension"): dimName = dimensionElement.attrib.get("name") if self._strictAxisNames and dimName not in self.axisDefaults: - # In case the document contains axis definitions, - # then we should only read the axes we know about. - # However, if the document does not contain axes, - # then we need to create them after reading. + # In case the document contains no axis definitions, + warnings.warn("Location with undefined axis: \"%s\"." % dimName) continue xValue = yValue = None try: @@ -1094,6 +1093,7 @@ class DesignSpaceDocument(object): self.filename = os.path.basename(path) reader = self.readerClass(path, self) reader.read() + self.findDefault() def write(self, path): self.path = path @@ -1222,78 +1222,23 @@ class DesignSpaceDocument(object): return axisDescriptor return None - def check(self): - """ - After reading we need to make sure we have a valid designspace. - This means making repairs if things are missing - - check if we have axes and deduce them from the masters if they're missing - - that can include axes referenced in masters, instances, glyphs. - - if no default is assigned, use mutatormath to find out. - - record the default in the designspace - - report all the changes in a log - - save a "repaired" version of the doc - """ - self.checkAxes() - self.checkDefault() - - def checkDefault(self): - """ Check the sources for a copyInfo flag.""" - flaggedDefaultCandidate = None + def findDefault(self): + # new default finder + # take the master with the location at all the defaults + self.default = None + for sourceDescriptor in self.sources: + if sourceDescriptor.location == self.defaultLoc: + # we choose you! + self.default = sourceDescriptor + return sourceDescriptor + # failing that, tkae the master with the copyInfo flag for sourceDescriptor in self.sources: names = set() if sourceDescriptor.copyInfo: # we choose you! - flaggedDefaultCandidate = sourceDescriptor - mutatorDefaultCandidate = self.getMutatorDefaultCandidate() - # what are we going to do? - if flaggedDefaultCandidate is not None: - if mutatorDefaultCandidate is not None: - if mutatorDefaultCandidate.name != flaggedDefaultCandidate.name: - # warn if we have a conflict - self.logger.info("Note: conflicting default masters:\n\tUsing %s as default\n\tMutator found %s" % (flaggedDefaultCandidate.name, mutatorDefaultCandidate.name)) - self.default = flaggedDefaultCandidate - self.defaultLoc = self.default.location - else: - # we have no flagged default candidate - # let's use the one from mutator - if flaggedDefaultCandidate is None and mutatorDefaultCandidate is not None: - # we didn't have a flag, use the one selected by mutator - self.default = mutatorDefaultCandidate - self.defaultLoc = self.default.location - self.default.copyInfo = True - # now that we have a default, let's check if the axes are ok - for axisObj in self.axes: - if axisObj.name not in self.default.location: - # extend the location of the neutral master with missing default value for this axis - self.default.location[axisObj.name] = axisObj.default - else: - if axisObj.default == self.default.location.get(axisObj.name): - continue - # proposed remedy: change default value in the axisdescriptor to the value of the neutral - neutralAxisValue = self.default.location.get(axisObj.name) - # make sure this value is between the min and max - if axisObj.minimum <= neutralAxisValue <= axisObj.maximum: - # yes we can fix this - axisObj.default = neutralAxisValue - self.logger.info("Note: updating the default value of axis %s to neutral master at %3.3f" % (axisObj.name, neutralAxisValue)) - # always fit the axis dimensions to the location of the designated neutral - elif neutralAxisValue < axisObj.minimum: - axisObj.default = neutralAxisValue - axisObj.minimum = neutralAxisValue - elif neutralAxisValue > axisObj.maximum: - axisObj.maximum = neutralAxisValue - axisObj.default = neutralAxisValue - else: - # now we're in trouble, can't solve this, alert. - self.logger.info("Warning: mismatched default value for axis %s and neutral master. Master value outside of axis bounds" % (axisObj.name)) - - def getMutatorDefaultCandidate(self): - # FIXME: original implementation using MutatorMath - # masterLocations = [src.location for src in self.sources] - # mutatorBias = biasFromLocations(masterLocations, preferOrigin=False) - # for src in self.sources: - # if src.location == mutatorBias: - # return src + self.default = sourceDescriptor + return sourceDescriptor + # failing that, well, fail. We don't want to guess any more. return None def _prepAxesForBender(self): @@ -1360,7 +1305,6 @@ class DesignSpaceDocument(object): a.tag, a.labelNames = tagForAxisName(a.name) self.logger.info("CheckAxes: added a missing axis %s, %3.3f %3.3f", a.name, a.minimum, a.maximum) - def normalizeLocation(self, location): # scale this location based on the axes # accept only values for the axes that we have definitions for @@ -1439,20 +1383,3 @@ class DesignSpaceDocument(object): newConditionSets.append(newConditions) rule.conditionSets = newConditionSets - -def rulesToFeature(doc, whiteSpace="\t", newLine="\n"): - """ Showing how rules could be expressed as FDK feature text. - Speculative. Experimental. - """ - axisNames = {axis.name: axis.tag for axis in doc.axes} - axisDims = {axis.tag: (axis.minimum, axis.maximum) for axis in doc.axes} - text = [] - for rule in doc.rules: - text.append("rule %s{" % rule.name) - for cd in rule.conditions: - axisTag = axisNames.get(cd.get('name'), "****") - axisMinimum = cd.get('minimum', axisDims.get(axisTag, [0,0])[0]) - axisMaximum = cd.get('maximum', axisDims.get(axisTag, [0,0])[1]) - text.append("%s%s %f %f;" % (whiteSpace, axisTag, axisMinimum, axisMaximum)) - text.append("} %s;" % rule.name) - return newLine.join(text) diff --git a/Tests/designspaceLib/designspace_test.py b/Tests/designspaceLib/designspace_test.py index 256e45742..ed51ebe4a 100644 --- a/Tests/designspaceLib/designspace_test.py +++ b/Tests/designspaceLib/designspace_test.py @@ -31,6 +31,29 @@ def test_fill_document(tmpdir): instancePath1 = os.path.join(tmpdir, "instances", "instanceTest1.ufo") instancePath2 = os.path.join(tmpdir, "instances", "instanceTest2.ufo") doc = DesignSpaceDocument() + + # write some axes + a1 = AxisDescriptor() + a1.minimum = 0 + a1.maximum = 1000 + a1.default = 0 + a1.name = "weight" + a1.tag = "wght" + # note: just to test the element language, not an actual label name recommendations. + a1.labelNames[u'fa-IR'] = u"قطر" + a1.labelNames[u'en'] = u"Wéíght" + doc.addAxis(a1) + a2 = AxisDescriptor() + a2.minimum = 0 + a2.maximum = 1000 + a2.default = 20 + a2.name = "width" + a2.tag = "wdth" + a2.map = [(0.0, 10.0), (401.0, 66.0), (1000.0, 990.0)] + a2.hidden = True + a2.labelNames[u'fr'] = u"Chasse" + doc.addAxis(a2) + # add master 1 s1 = SourceDescriptor() s1.filename = os.path.relpath(masterPath1, os.path.dirname(testDocPath)) @@ -107,45 +130,6 @@ def test_fill_document(tmpdir): doc.filename = "suggestedFileName.designspace" doc.lib['com.coolDesignspaceApp.previewSize'] = 30 - # now we have sources and instances, but no axes yet. - doc.check() - - # Here, since the axes are not defined in the document, but instead are - # infered from the locations of the instances, we cannot guarantee the - # order in which they will be created by the `check()` method. - assert set(doc.getAxisOrder()) == set(['spooky', 'weight', 'width']) - doc.axes = [] # clear the axes - - # write some axes - a1 = AxisDescriptor() - a1.minimum = 0 - a1.maximum = 1000 - a1.default = 0 - a1.name = "weight" - a1.tag = "wght" - # note: just to test the element language, not an actual label name recommendations. - a1.labelNames[u'fa-IR'] = u"قطر" - a1.labelNames[u'en'] = u"Wéíght" - doc.addAxis(a1) - a2 = AxisDescriptor() - a2.minimum = 0 - a2.maximum = 1000 - a2.default = 20 - a2.name = "width" - a2.tag = "wdth" - a2.map = [(0.0, 10.0), (401.0, 66.0), (1000.0, 990.0)] - a2.hidden = True - a2.labelNames[u'fr'] = u"Chasse" - doc.addAxis(a2) - # add an axis that is not part of any location to see if that works - a3 = AxisDescriptor() - a3.minimum = 333 - a3.maximum = 666 - a3.default = 444 - a3.name = "spooky" - a3.tag = "SPOK" - a3.map = [(0.0, 10.0), (401.0, 66.0), (1000.0, 990.0)] - #doc.addAxis(a3) # uncomment this line to test the effects of default axes values # write some rules r1 = RuleDescriptor() r1.name = "named.rule.1" @@ -163,23 +147,11 @@ def test_fill_document(tmpdir): new = DesignSpaceDocument() new.read(testDocPath) - new.check() assert new.default.location == {'width': 20.0, 'weight': 0.0} assert new.filename == 'test.designspace' assert new.lib == doc.lib assert new.instances[0].lib == doc.instances[0].lib - # >>> for a, b in zip(doc.instances, new.instances): - # ... a.compare(b) - # >>> for a, b in zip(doc.sources, new.sources): - # ... a.compare(b) - # >>> for a, b in zip(doc.axes, new.axes): - # ... a.compare(b) - # >>> [n.mutedGlyphNames for n in new.sources] - # [['A', 'Z'], []] - # >>> doc.getFonts() - # [] - # test roundtrip for the axis attributes and data axes = {} for axis in doc.axes: @@ -197,50 +169,6 @@ def test_fill_document(tmpdir): assert a == b -def test_adjustAxisDefaultToNeutral(tmpdir): - tmpdir = str(tmpdir) - testDocPath = os.path.join(tmpdir, "testAdjustAxisDefaultToNeutral.designspace") - masterPath1 = os.path.join(tmpdir, "masters", "masterTest1.ufo") - masterPath2 = os.path.join(tmpdir, "masters", "masterTest2.ufo") - instancePath1 = os.path.join(tmpdir, "instances", "instanceTest1.ufo") - instancePath2 = os.path.join(tmpdir, "instances", "instanceTest2.ufo") - doc = DesignSpaceDocument() - # add master 1 - s1 = SourceDescriptor() - s1.filename = os.path.relpath(masterPath1, os.path.dirname(testDocPath)) - s1.name = "master.ufo1" - s1.copyInfo = True - s1.copyFeatures = True - s1.location = dict(weight=55, width=1000) - doc.addSource(s1) - # write some axes - a1 = AxisDescriptor() - a1.minimum = 0 - a1.maximum = 1000 - a1.default = 0 # the wrong value - a1.name = "weight" - a1.tag = "wght" - doc.addAxis(a1) - a2 = AxisDescriptor() - a2.minimum = -10 - a2.maximum = 10 - a2.default = 0 # the wrong value - a2.name = "width" - a2.tag = "wdth" - doc.addAxis(a2) - # write the document - doc.write(testDocPath) - assert os.path.exists(testDocPath) - # import it again - new = DesignSpaceDocument() - new.read(testDocPath) - new.check() - loc = new.default.location - for axisObj in new.axes: - n = axisObj.name - assert axisObj.default == loc.get(n) - - def test_unicodes(tmpdir): tmpdir = str(tmpdir) testDocPath = os.path.join(tmpdir, "testUnicodes.designspace") @@ -457,7 +385,7 @@ def test_handleNoAxes(tmpdir): doc.addInstance(i1) doc.write(testDocPath) - __removeAxesFromDesignSpace(testDocPath) + #__removeAxesFromDesignSpace(testDocPath) verify = DesignSpaceDocument() verify.read(testDocPath) verify.write(testDocPath2) @@ -476,8 +404,16 @@ def test_pathNameResolve(tmpdir): instancePath1 = os.path.join(tmpdir, "instances", "instanceTest1.ufo") instancePath2 = os.path.join(tmpdir, "instances", "instanceTest2.ufo") + a1 = AxisDescriptor() + a1.tag = "TAGA" + a1.name = "axisName_a" + a1.minimum = 0 + a1.maximum = 1000 + a1.default = 0 + # Case 1: filename and path are both empty. Nothing to calculate, nothing to put in the file. doc = DesignSpaceDocument() + doc.addAxis(a1) s = SourceDescriptor() s.filename = None s.path = None @@ -494,6 +430,7 @@ def test_pathNameResolve(tmpdir): # Case 2: filename is empty, path points somewhere: calculate a new filename. doc = DesignSpaceDocument() + doc.addAxis(a1) s = SourceDescriptor() s.filename = None s.path = masterPath1 @@ -510,6 +447,7 @@ def test_pathNameResolve(tmpdir): # Case 3: the filename is set, the path is None. doc = DesignSpaceDocument() + doc.addAxis(a1) s = SourceDescriptor() s.filename = "../somewhere/over/the/rainbow.ufo" s.path = None @@ -528,6 +466,7 @@ def test_pathNameResolve(tmpdir): # Case 4: the filename points to one file, the path points to another. The path takes precedence. doc = DesignSpaceDocument() + doc.addAxis(a1) s = SourceDescriptor() s.filename = "../somewhere/over/the/rainbow.ufo" s.path = masterPath1 @@ -543,6 +482,7 @@ def test_pathNameResolve(tmpdir): # Case 5: the filename is None, path has a value, update the filename doc = DesignSpaceDocument() + doc.addAxis(a1) s = SourceDescriptor() s.filename = None s.path = masterPath1 @@ -557,6 +497,7 @@ def test_pathNameResolve(tmpdir): # Case 6: the filename has a value, path has a value, update the filenames with force doc = DesignSpaceDocument() + doc.addAxis(a1) s = SourceDescriptor() s.filename = "../somewhere/over/the/rainbow.ufo" s.path = masterPath1 @@ -802,6 +743,22 @@ def test_incompleteRule(tmpdir): tmpdir = str(tmpdir) testDocPath1 = os.path.join(tmpdir, "testIncompleteRule.designspace") doc = DesignSpaceDocument() + # write some axes + a1 = AxisDescriptor() + a1.tag = "TAGA" + a1.name = "axisName_a" + a1.minimum = 0 + a1.maximum = 1000 + a1.default = 0 + doc.addAxis(a1) + a2 = AxisDescriptor() + a2.tag = "TAGB" + a2.name = "axisName_b" + a2.minimum = 0 + a2.maximum = 3000 + a2.default = 0 + doc.addAxis(a2) + r1 = RuleDescriptor() r1.name = "incomplete.rule.1" r1.conditionSets.append([