From b627a778bf77038c0f58772b98647f9bf4bb805a Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 18 Jan 2018 13:21:08 +0000 Subject: [PATCH] Don't stop at first incompatibilty, log errors, raise at the end Based on Miguel Sousa's original PR and the following discussion: https://github.com/googlei18n/cu2qu/pull/114 Instead of raising an error at the first incompatible glyph, we let it continue (keeping the original contours unmodified when that happens), and use logging to print error messages. A new `IncompatibleFontsError` exception is raised at the end of `fonts_to_quadratic` if any glyph has incompatible number or types of segments. The exception instance has a `glyph_errors` attribute (dict) which collects all the individual IncompatibleGlyphsError keyed by glyph name. --- Lib/cu2qu/errors.py | 49 +++++++++++++++++++++++++ Lib/cu2qu/ufo.py | 40 ++++++++++++--------- tests/ufo_test.py | 87 +++++++++++++++++++++++++++++++++------------ 3 files changed, 136 insertions(+), 40 deletions(-) create mode 100644 Lib/cu2qu/errors.py diff --git a/Lib/cu2qu/errors.py b/Lib/cu2qu/errors.py new file mode 100644 index 000000000..44fdb3264 --- /dev/null +++ b/Lib/cu2qu/errors.py @@ -0,0 +1,49 @@ +from __future__ import print_function, absolute_import, division + + +class UnequalZipLengthsError(ValueError): + pass + + +class IncompatibleGlyphsError(ValueError): + + def __init__(self, glyphs): + assert len(glyphs) > 1 + self.glyphs = glyphs + names = set(repr(g.name) for g in glyphs) + if len(names) > 1: + self.combined_name = "{%s}" % ", ".join(sorted(names)) + else: + self.combined_name = names.pop() + + def __repr__(self): + return "<%s %s>" % (type(self).__name__, self.combined_name) + + +class IncompatibleSegmentNumberError(IncompatibleGlyphsError): + + def __str__(self): + return "Glyphs named %s have different number of segments" % ( + self.combined_name) + + +class IncompatibleSegmentTypesError(IncompatibleGlyphsError): + + def __init__(self, glyphs, segments): + IncompatibleGlyphsError.__init__(self, glyphs) + self.segments = segments + + def __str__(self): + from pprint import pformat + return "Glyphs named %s have incompatible segment types:\n%s" % ( + self.combined_name, pformat(self.segments)) + + +class IncompatibleFontsError(ValueError): + + def __init__(self, glyph_errors): + self.glyph_errors = glyph_errors + + def __str__(self): + return "fonts contains incompatible glyphs: %s" % ( + ", ".join(repr(g) for g in sorted(self.glyph_errors.keys()))) diff --git a/Lib/cu2qu/ufo.py b/Lib/cu2qu/ufo.py index 890581040..d7b914a50 100644 --- a/Lib/cu2qu/ufo.py +++ b/Lib/cu2qu/ufo.py @@ -31,6 +31,11 @@ from fontTools.pens.basePen import AbstractPen from cu2qu import curves_to_quadratic from cu2qu.pens import ReverseContourPen +from cu2qu.errors import ( + UnequalZipLengthsError, IncompatibleSegmentNumberError, + IncompatibleSegmentTypesError, IncompatibleGlyphsError, + IncompatibleFontsError) + __all__ = ['fonts_to_quadratic', 'font_to_quadratic'] @@ -39,16 +44,6 @@ DEFAULT_MAX_ERR = 0.001 logger = logging.getLogger(__name__) -class IncompatibleGlyphsError(ValueError): - - def __str__(self): - return ", ".join(set(repr(glyph.name) for glyph in self.args)) - - -class UnequalZipLengthsError(ValueError): - pass - - _zip = zip def zip(*args): """Ensure each argument to zip has the same length. Also make sure a list is @@ -153,7 +148,7 @@ def _glyphs_to_quadratic(glyphs, max_err, reverse_direction, stats): try: segments_by_location = zip(*[_get_segments(g) for g in glyphs]) except UnequalZipLengthsError: - raise IncompatibleGlyphsError(*glyphs) + raise IncompatibleSegmentNumberError(glyphs) if not any(segments_by_location): return False @@ -161,11 +156,12 @@ def _glyphs_to_quadratic(glyphs, max_err, reverse_direction, stats): glyphs_modified = reverse_direction new_segments_by_location = [] - for segments in segments_by_location: + incompatible = {} + for i, segments in enumerate(segments_by_location): tag = segments[0][0] if not all(s[0] == tag for s in segments[1:]): - raise IncompatibleGlyphsError(*glyphs) - if tag == 'curve': + incompatible[i] = [s[0] for s in segments] + elif tag == 'curve': segments = _segments_to_quadratic(segments, max_err, stats) glyphs_modified = True new_segments_by_location.append(segments) @@ -175,6 +171,8 @@ def _glyphs_to_quadratic(glyphs, max_err, reverse_direction, stats): for glyph, new_segments in zip(glyphs, new_segments_by_glyph): _set_segments(glyph, new_segments, reverse_direction) + if incompatible: + raise IncompatibleSegmentTypesError(glyphs, segments=incompatible) return glyphs_modified @@ -217,7 +215,7 @@ def fonts_to_quadratic( Return True if fonts were modified, else return False. - Raises IncompatibleGlyphsError if same-named glyphs from different fonts + Raises IncompatibleFontsError if same-named glyphs from different fonts have non-interpolatable outlines. """ @@ -243,6 +241,7 @@ def fonts_to_quadratic( max_errors = [f.info.unitsPerEm * max_err_em for f in fonts] modified = False + glyph_errors = {} for name in set().union(*(f.keys() for f in fonts)): glyphs = [] cur_max_errors = [] @@ -250,8 +249,15 @@ def fonts_to_quadratic( if name in font: glyphs.append(font[name]) cur_max_errors.append(error) - modified |= _glyphs_to_quadratic( - glyphs, cur_max_errors, reverse_direction, stats) + try: + modified |= _glyphs_to_quadratic( + glyphs, cur_max_errors, reverse_direction, stats) + except IncompatibleGlyphsError as exc: + logger.error(exc) + glyph_errors[name] = exc + + if glyph_errors: + raise IncompatibleFontsError(glyph_errors) if modified and dump_stats: spline_lengths = sorted(stats.keys()) diff --git a/tests/ufo_test.py b/tests/ufo_test.py index a85b4bddb..ab8123654 100644 --- a/tests/ufo_test.py +++ b/tests/ufo_test.py @@ -9,7 +9,11 @@ from cu2qu.ufo import ( glyphs_to_quadratic, glyph_to_quadratic, logger, - IncompatibleGlyphsError +) +from cu2qu.errors import ( + IncompatibleSegmentNumberError, + IncompatibleSegmentTypesError, + IncompatibleFontsError, ) from . import DATADIR @@ -116,40 +120,49 @@ class GlyphsToQuadraticTest(object): reverse_direction=True) @pytest.mark.parametrize( - "outlines", + ["outlines", "exception", "message"], [ [ [ - ('moveTo', ((0, 0),)), - ('curveTo', ((1, 1), (2, 2), (3, 3))), - ('curveTo', ((4, 4), (5, 5), (6, 6))), - ('closePath', ()), + [ + ('moveTo', ((0, 0),)), + ('curveTo', ((1, 1), (2, 2), (3, 3))), + ('curveTo', ((4, 4), (5, 5), (6, 6))), + ('closePath', ()), + ], + [ + ('moveTo', ((7, 7),)), + ('curveTo', ((8, 8), (9, 9), (10, 10))), + ('closePath', ()), + ] ], - [ - ('moveTo', ((7, 7),)), - ('curveTo', ((8, 8), (9, 9), (10, 10))), - ('closePath', ()), - ] + IncompatibleSegmentNumberError, + "have different number of segments", ], [ [ - ('moveTo', ((0, 0),)), - ('curveTo', ((1, 1), (2, 2), (3, 3))), - ('closePath', ()), + + [ + ('moveTo', ((0, 0),)), + ('curveTo', ((1, 1), (2, 2), (3, 3))), + ('closePath', ()), + ], + [ + ('moveTo', ((4, 4),)), + ('lineTo', ((5, 5),)), + ('closePath', ()), + ], ], - [ - ('moveTo', ((4, 4),)), - ('lineTo', ((5, 5),)), - ('closePath', ()), - ], - ] + IncompatibleSegmentTypesError, + "have incompatible segment types", + ], ], ids=[ "unequal-length", "different-segment-types", ] ) - def test_incompatible(self, outlines): + def test_incompatible_glyphs(self, outlines, exception, message): glyphs = [] for i, outline in enumerate(outlines): glyph = Glyph() @@ -158,9 +171,37 @@ class GlyphsToQuadraticTest(object): for operator, args in outline: getattr(pen, operator)(*args) glyphs.append(glyph) - with pytest.raises(IncompatibleGlyphsError) as excinfo: + with pytest.raises(exception) as excinfo: glyphs_to_quadratic(glyphs) - assert excinfo.match("^'glyph[0-9]+'(, 'glyph[0-9]+')*$") + assert excinfo.match(message) + + def test_incompatible_fonts(self): + font1 = Font() + font1.info.unitsPerEm = 1000 + glyph1 = font1.newGlyph("a") + pen1 = glyph1.getPen() + for operator, args in [("moveTo", ((0, 0),)), + ("lineTo", ((1, 1),)), + ("endPath", ())]: + getattr(pen1, operator)(*args) + + font2 = Font() + font2.info.unitsPerEm = 1000 + glyph2 = font2.newGlyph("a") + pen2 = glyph2.getPen() + for operator, args in [("moveTo", ((0, 0),)), + ("curveTo", ((1, 1), (2, 2), (3, 3))), + ("endPath", ())]: + getattr(pen2, operator)(*args) + + with pytest.raises(IncompatibleFontsError) as excinfo: + fonts_to_quadratic([font1, font2]) + assert excinfo.match("fonts contains incompatible glyphs: 'a'") + + assert hasattr(excinfo.value, "glyph_errors") + error = excinfo.value.glyph_errors['a'] + assert isinstance(error, IncompatibleSegmentTypesError) + assert error.segments == {1: ["line", "curve"]} def test_already_quadratic(self): glyph = Glyph()