Merge pull request #2223 from simoncozens/better-merge-errors

Explain merge errors in designer-friendly terms
This commit is contained in:
Simon Cozens 2021-03-15 06:27:54 +00:00 committed by GitHub
commit 4cdf3312a5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 159 additions and 62 deletions

View File

@ -1,3 +1,22 @@
from enum import Enum, auto
import textwrap
class VarLibMergeFailure(Enum):
ShouldBeConstant = "some values were different, but should have been the same"
MismatchedTypes = "data had inconsistent types"
LengthsDiffer = "a list of objects had inconsistent lengths"
KeysDiffer = "a list of objects had different keys"
InconsistentGlyphOrder = "the glyph order was inconsistent between masters"
FoundANone = "one of the values in a list was empty when it shouldn't have been"
NotANone = "one of the values in a list was not empty when it should have been"
UnsupportedFormat = "an OpenType subtable (%s) had a format I didn't expect"
InconsistentFormat = (
"an OpenType subtable (%s) had inconsistent formats between masters"
)
InconsistentExtensions = "the masters use extension lookups in inconsistent ways"
class VarLibError(Exception): class VarLibError(Exception):
"""Base exception for the varLib module.""" """Base exception for the varLib module."""
@ -9,6 +28,45 @@ class VarLibValidationError(VarLibError):
class VarLibMergeError(VarLibError): class VarLibMergeError(VarLibError):
"""Raised when input data cannot be merged into a variable font.""" """Raised when input data cannot be merged into a variable font."""
def __init__(self, merger, args):
self.merger = merger
self.args = args
def __str__(self):
cause, stack = self.args[0], self.args[1:]
fontnames = [
ttf["name"].getDebugName(1) + " " + ttf["name"].getDebugName(2)
for ttf in self.merger.ttfs
]
context = "".join(reversed(stack))
details = ""
reason = cause["reason"].value
if reason == VarLibMergeFailure.FoundANone:
offender = [x is None for x in cause["got"]].index(True)
details = (
f"\n\nThe problem is likely to be in {fontnames[offender]}:\n"
f"{stack[0]}=={cause['got']}\n"
)
elif "expected" in cause and "got" in cause:
offender = [x == cause["expected"] for x in cause["got"]].index(False)
got = cause["got"][offender]
details = (
f"\n\nThe problem is likely to be in {fontnames[offender]}:\n"
f"Expected to see {stack[0]}=={cause['expected']}, instead saw {got}\n"
)
if (
reason == VarLibMergeFailure.UnsupportedFormat
or reason == VarLibMergeFailure.InconsistentFormat
):
reason = reason % cause["subtable"]
basic = textwrap.fill(
f"Couldn't merge the fonts, because {reason}. "
f"This happened while performing the following operation: {context}",
width=78,
)
return "\n\n" + basic + details
class VarLibCFFDictMergeError(VarLibMergeError): class VarLibCFFDictMergeError(VarLibMergeError):
"""Raised when a CFF PrivateDict cannot be merged.""" """Raised when a CFF PrivateDict cannot be merged."""

View File

@ -14,7 +14,7 @@ from fontTools.varLib.varStore import VarStoreInstancer
from functools import reduce from functools import reduce
from fontTools.otlLib.builder import buildSinglePos from fontTools.otlLib.builder import buildSinglePos
from .errors import VarLibMergeError from .errors import VarLibMergeError, VarLibMergeFailure
class Merger(object): class Merger(object):
@ -69,7 +69,10 @@ class Merger(object):
item.ensureDecompiled() item.ensureDecompiled()
keys = sorted(vars(out).keys()) keys = sorted(vars(out).keys())
if not all(keys == sorted(vars(v).keys()) for v in lst): if not all(keys == sorted(vars(v).keys()) for v in lst):
raise VarLibMergeError((keys, [sorted(vars(v).keys()) for v in lst])) raise VarLibMergeError(self, ({
"reason": VarLibMergeFailure.KeysDiffer,
"expected": keys,
"got": [sorted(vars(v).keys()) for v in lst]},))
mergers = self.mergersFor(out) mergers = self.mergersFor(out)
defaultMerger = mergers.get('*', self.__class__.mergeThings) defaultMerger = mergers.get('*', self.__class__.mergeThings)
try: try:
@ -85,7 +88,10 @@ class Merger(object):
def mergeLists(self, out, lst): def mergeLists(self, out, lst):
if not allEqualTo(out, lst, len): if not allEqualTo(out, lst, len):
raise VarLibMergeError((len(out), [len(v) for v in lst])) raise VarLibMergeError(self, ({
"reason": VarLibMergeFailure.LengthsDiffer,
"expected": len(out),
"got": [len(x) for x in lst]},))
for i,(value,values) in enumerate(zip(out, zip(*lst))): for i,(value,values) in enumerate(zip(out, zip(*lst))):
try: try:
self.mergeThings(value, values) self.mergeThings(value, values)
@ -94,9 +100,11 @@ class Merger(object):
raise raise
def mergeThings(self, out, lst): def mergeThings(self, out, lst):
try:
if not allEqualTo(out, lst, type): if not allEqualTo(out, lst, type):
raise VarLibMergeError((out, lst)) raise VarLibMergeError(self, ({
"reason": VarLibMergeFailure.MismatchedTypes,
"expected": type(out),
"got": [type(x) for x in lst]},))
mergerFunc = self.mergersFor(out).get(None, None) mergerFunc = self.mergersFor(out).get(None, None)
if mergerFunc is not None: if mergerFunc is not None:
mergerFunc(self, out, lst) mergerFunc(self, out, lst)
@ -106,13 +114,13 @@ class Merger(object):
self.mergeLists(out, lst) self.mergeLists(out, lst)
else: else:
if not allEqualTo(out, lst): if not allEqualTo(out, lst):
raise VarLibMergeError((out, lst)) raise VarLibMergeError(self, ({
except Exception as e: "reason": VarLibMergeFailure.ShouldBeConstant,
e.args = e.args + (type(out).__name__,) "expected": out,
raise "got": lst},))
def mergeTables(self, font, master_ttfs, tableTags): def mergeTables(self, font, master_ttfs, tableTags):
self.ttfs = master_ttfs # For error reporting
for tag in tableTags: for tag in tableTags:
if tag not in font: continue if tag not in font: continue
self.mergeThings(font[tag], [m[tag] if tag in m else None self.mergeThings(font[tag], [m[tag] if tag in m else None
@ -128,7 +136,11 @@ class AligningMerger(Merger):
def merge(merger, self, lst): def merge(merger, self, lst):
if self is None: if self is None:
if not allNone(lst): if not allNone(lst):
raise VarLibMergeError(lst) raise VarLibMergeError(self, ({
"reason": VarLibMergeFailure.NotANone,
"expected": None,
"got": lst
},))
return return
lst = [l.classDefs for l in lst] lst = [l.classDefs for l in lst]
@ -141,7 +153,11 @@ def merge(merger, self, lst):
for k in allKeys: for k in allKeys:
allValues = nonNone(l.get(k) for l in lst) allValues = nonNone(l.get(k) for l in lst)
if not allEqual(allValues): if not allEqual(allValues):
raise VarLibMergeError(allValues) raise VarLibMergeError(self, ({
"reason": VarLibMergeFailure.ShouldBeConstant,
"expected": allValues[0],
"got": lst,
}, "."+k))
if not allValues: if not allValues:
self[k] = None self[k] = None
else: else:
@ -178,7 +194,9 @@ def _merge_GlyphOrders(font, lst, values_lst=None, default=None):
order = sorted(combined, key=sortKey) order = sorted(combined, key=sortKey)
# Make sure all input glyphsets were in proper order # Make sure all input glyphsets were in proper order
if not all(sorted(vs, key=sortKey) == vs for vs in lst): if not all(sorted(vs, key=sortKey) == vs for vs in lst):
raise VarLibMergeError("Glyph order inconsistent across masters.") raise VarLibMergeError(self, ({
"reason" : VarLibMergeFailure.InconsistentGlyphOrder
},))
del combined del combined
paddedValues = None paddedValues = None
@ -205,10 +223,10 @@ def _Lookup_SinglePos_get_effective_value(subtables, glyph):
elif self.Format == 2: elif self.Format == 2:
return self.Value[self.Coverage.glyphs.index(glyph)] return self.Value[self.Coverage.glyphs.index(glyph)]
else: else:
raise VarLibMergeError( raise VarLibMergeError(self, ({
"Cannot retrieve effective value for SinglePos lookup, unsupported " "reason": VarLibMergeFailure.UnsupportedFormat,
f"format {self.Format}." "subtable": "single positioning lookup"
) },))
return None return None
def _Lookup_PairPos_get_effective_value_pair(subtables, firstGlyph, secondGlyph): def _Lookup_PairPos_get_effective_value_pair(subtables, firstGlyph, secondGlyph):
@ -230,17 +248,21 @@ def _Lookup_PairPos_get_effective_value_pair(subtables, firstGlyph, secondGlyph)
klass2 = self.ClassDef2.classDefs.get(secondGlyph, 0) klass2 = self.ClassDef2.classDefs.get(secondGlyph, 0)
return self.Class1Record[klass1].Class2Record[klass2] return self.Class1Record[klass1].Class2Record[klass2]
else: else:
raise VarLibMergeError( raise VarLibMergeError(self, ({
"Cannot retrieve effective value pair for PairPos lookup, unsupported " "reason": VarLibMergeFailure.UnsupportedFormat,
f"format {self.Format}." "subtable": "pair positioning lookup"
) },))
return None return None
@AligningMerger.merger(ot.SinglePos) @AligningMerger.merger(ot.SinglePos)
def merge(merger, self, lst): def merge(merger, self, lst):
self.ValueFormat = valueFormat = reduce(int.__or__, [l.ValueFormat for l in lst], 0) self.ValueFormat = valueFormat = reduce(int.__or__, [l.ValueFormat for l in lst], 0)
if not (len(lst) == 1 or (valueFormat & ~0xF == 0)): if not (len(lst) == 1 or (valueFormat & ~0xF == 0)):
raise VarLibMergeError(f"SinglePos format {valueFormat} is unsupported.") raise VarLibMergeError(self, ({
"reason": VarLibMergeFailure.UnsupportedFormat,
"subtable": "single positioning lookup"
},))
# If all have same coverage table and all are format 1, # If all have same coverage table and all are format 1,
coverageGlyphs = self.Coverage.glyphs coverageGlyphs = self.Coverage.glyphs
@ -526,9 +548,10 @@ def merge(merger, self, lst):
elif self.Format == 2: elif self.Format == 2:
_PairPosFormat2_merge(self, lst, merger) _PairPosFormat2_merge(self, lst, merger)
else: else:
raise VarLibMergeError( raise VarLibMergeError(self, ({
f"Cannot merge PairPos lookup, unsupported format {self.Format}." "reason": VarLibMergeFailure.UnsupportedFormat,
) "subtable": "pair positioning lookup"
},))
del merger.valueFormat1, merger.valueFormat2 del merger.valueFormat1, merger.valueFormat2
@ -594,7 +617,7 @@ def _MarkBasePosFormat1_merge(self, lst, merger, Mark='Mark', Base='Base'):
# input masters. # input masters.
if not allEqual(allClasses): if not allEqual(allClasses):
raise VarLibMergeError(allClasses) raise VarLibMergeError(self, allClasses)
if not allClasses: if not allClasses:
rec = None rec = None
else: else:
@ -644,30 +667,34 @@ def _MarkBasePosFormat1_merge(self, lst, merger, Mark='Mark', Base='Base'):
@AligningMerger.merger(ot.MarkBasePos) @AligningMerger.merger(ot.MarkBasePos)
def merge(merger, self, lst): def merge(merger, self, lst):
if not allEqualTo(self.Format, (l.Format for l in lst)): if not allEqualTo(self.Format, (l.Format for l in lst)):
raise VarLibMergeError( raise VarLibMergeError(self, ({
f"MarkBasePos formats inconsistent across masters, " "reason": VarLibMergeFailure.InconsistentFormats,
f"expected {self.Format} but got {[l.Format for l in lst]}." "subtable": "mark-to-base positioning lookup",
) "expected": self.Format,
"got": [l.Format for l in lst]},))
if self.Format == 1: if self.Format == 1:
_MarkBasePosFormat1_merge(self, lst, merger) _MarkBasePosFormat1_merge(self, lst, merger)
else: else:
raise VarLibMergeError( raise VarLibMergeError(self, ({
f"Cannot merge MarkBasePos lookup, unsupported format {self.Format}." "reason": VarLibMergeFailure.UnsupportedFormat,
) "subtable": "mark-to-base positioning lookup",
}))
@AligningMerger.merger(ot.MarkMarkPos) @AligningMerger.merger(ot.MarkMarkPos)
def merge(merger, self, lst): def merge(merger, self, lst):
if not allEqualTo(self.Format, (l.Format for l in lst)): if not allEqualTo(self.Format, (l.Format for l in lst)):
raise VarLibMergeError( raise VarLibMergeError(self, ({
f"MarkMarkPos formats inconsistent across masters, " "reason": VarLibMergeFailure.InconsistentFormats,
f"expected {self.Format} but got {[l.Format for l in lst]}." "subtable": "mark-to-mark positioning lookup",
) "expected": self.Format,
"got": [l.Format for l in lst]},))
if self.Format == 1: if self.Format == 1:
_MarkBasePosFormat1_merge(self, lst, merger, 'Mark1', 'Mark2') _MarkBasePosFormat1_merge(self, lst, merger, 'Mark1', 'Mark2')
else: else:
raise VarLibMergeError( raise VarLibMergeError(self, ({
f"Cannot merge MarkMarkPos lookup, unsupported format {self.Format}." "reason": VarLibMergeFailure.UnsupportedFormat,
) "subtable": "mark-to-mark positioning lookup",
}))
def _PairSet_flatten(lst, font): def _PairSet_flatten(lst, font):
@ -797,15 +824,15 @@ def merge(merger, self, lst):
continue continue
if sts[0].__class__.__name__.startswith('Extension'): if sts[0].__class__.__name__.startswith('Extension'):
if not allEqual([st.__class__ for st in sts]): if not allEqual([st.__class__ for st in sts]):
raise VarLibMergeError( raise VarLibMergeError(self, ({
"Use of extensions inconsistent between masters: " "reason": VarLibMergeFailure.InconsistentExtensions,
f"{[st.__class__.__name__ for st in sts]}." "expected": "Extension",
) "got": [st.__class__.__name__ for st in sts]
},))
if not allEqual([st.ExtensionLookupType for st in sts]): if not allEqual([st.ExtensionLookupType for st in sts]):
raise VarLibMergeError( raise VarLibMergeError(self, ({
"Extension lookup type differs between masters: " "reason": VarLibMergeFailure.InconsistentExtensions,
f"{[st.ExtensionLookupType for st in sts]}." },))
)
l.LookupType = sts[0].ExtensionLookupType l.LookupType = sts[0].ExtensionLookupType
new_sts = [st.ExtSubTable for st in sts] new_sts = [st.ExtSubTable for st in sts]
del sts[:] del sts[:]
@ -1034,7 +1061,10 @@ class VariationMerger(AligningMerger):
if None in lst: if None in lst:
if allNone(lst): if allNone(lst):
if out is not None: if out is not None:
raise VarLibMergeError((out, lst)) raise VarLibMergeError(self, ({
"reason": VarLibMergeFailure.FoundANone,
"got": lst
},))
return return
masterModel = self.model masterModel = self.model
model, lst = masterModel.getSubModel(lst) model, lst = masterModel.getSubModel(lst)
@ -1055,7 +1085,10 @@ def buildVarDevTable(store_builder, master_values):
@VariationMerger.merger(ot.BaseCoord) @VariationMerger.merger(ot.BaseCoord)
def merge(merger, self, lst): def merge(merger, self, lst):
if self.Format != 1: if self.Format != 1:
raise VarLibMergeError(f"BaseCoord format {self.Format} unsupported.") raise VarLibMergeError(self, ({
"cause": VarLibMergeFailure.UnsupportedFormat,
"subtable": "a baseline coordinate"
},))
self.Coordinate, DeviceTable = buildVarDevTable(merger.store_builder, [a.Coordinate for a in lst]) self.Coordinate, DeviceTable = buildVarDevTable(merger.store_builder, [a.Coordinate for a in lst])
if DeviceTable: if DeviceTable:
self.Format = 3 self.Format = 3
@ -1064,7 +1097,10 @@ def merge(merger, self, lst):
@VariationMerger.merger(ot.CaretValue) @VariationMerger.merger(ot.CaretValue)
def merge(merger, self, lst): def merge(merger, self, lst):
if self.Format != 1: if self.Format != 1:
raise VarLibMergeError(f"CaretValue format {self.Format} unsupported.") raise VarLibMergeError(self, ({
"cause": VarLibMergeFailure.UnsupportedFormat,
"subtable": "a caret"
},))
self.Coordinate, DeviceTable = buildVarDevTable(merger.store_builder, [a.Coordinate for a in lst]) self.Coordinate, DeviceTable = buildVarDevTable(merger.store_builder, [a.Coordinate for a in lst])
if DeviceTable: if DeviceTable:
self.Format = 3 self.Format = 3
@ -1073,7 +1109,10 @@ def merge(merger, self, lst):
@VariationMerger.merger(ot.Anchor) @VariationMerger.merger(ot.Anchor)
def merge(merger, self, lst): def merge(merger, self, lst):
if self.Format != 1: if self.Format != 1:
raise VarLibMergeError(f"Anchor format {self.Format} unsupported.") raise VarLibMergeError(self, ({
"cause": VarLibMergeFailure.UnsupportedFormat,
"subtable": "an anchor"
},))
self.XCoordinate, XDeviceTable = buildVarDevTable(merger.store_builder, [a.XCoordinate for a in lst]) self.XCoordinate, XDeviceTable = buildVarDevTable(merger.store_builder, [a.XCoordinate for a in lst])
self.YCoordinate, YDeviceTable = buildVarDevTable(merger.store_builder, [a.YCoordinate for a in lst]) self.YCoordinate, YDeviceTable = buildVarDevTable(merger.store_builder, [a.YCoordinate for a in lst])
if XDeviceTable or YDeviceTable: if XDeviceTable or YDeviceTable: