Merge pull request #2995 from fonttools/reverse-output-implied-line

reverseContourPen_test: check outputImpliedClosingLine works as expected
This commit is contained in:
Behdad Esfahbod 2023-02-17 10:43:28 -07:00 committed by GitHub
commit 97ed3a61cd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 221 additions and 36 deletions

View File

@ -62,7 +62,7 @@ def reversedContour(contour, outputImpliedClosingLine=False):
if closed:
# for closed paths, we keep the starting point
yield firstType, firstPts
if outputImpliedClosingLine or firstOnCurve != lastOnCurve:
if firstOnCurve != lastOnCurve:
# emit an implied line between the last and first points
yield "lineTo", (lastOnCurve,)
contour[-1] = (lastType, tuple(lastPts[:-1]) + (firstOnCurve,))

View File

@ -12,6 +12,7 @@ TEST_DATA = [
("lineTo", ((3, 3),)), # last not on move, line is implied
("closePath", ()),
],
False, # outputImpliedClosingLine
[
("moveTo", ((0, 0),)),
("lineTo", ((3, 3),)),
@ -25,30 +26,85 @@ TEST_DATA = [
("moveTo", ((0, 0),)),
("lineTo", ((1, 1),)),
("lineTo", ((2, 2),)),
("lineTo", ((0, 0),)), # last on move, no implied line
("lineTo", ((3, 3),)), # last line does not overlap move...
("closePath", ()),
],
True, # outputImpliedClosingLine
[
("moveTo", ((0, 0),)),
("lineTo", ((3, 3),)),
("lineTo", ((2, 2),)),
("lineTo", ((1, 1),)),
("lineTo", ((0, 0),)), # ... but closing line is NOT implied
("closePath", ()),
],
),
(
[
("moveTo", ((0, 0),)),
("lineTo", ((0, 0),)),
("lineTo", ((1, 1),)),
("lineTo", ((2, 2),)),
("lineTo", ((0, 0),)), # last line overlaps move, explicit line
("closePath", ()),
],
False,
[
("moveTo", ((0, 0),)),
("lineTo", ((2, 2),)),
("lineTo", ((1, 1),)),
("lineTo", ((0, 0),)),
("lineTo", ((0, 0),)),
("closePath", ()), # closing line implied
],
),
(
[
("moveTo", ((0, 0),)),
("lineTo", ((1, 1),)),
("lineTo", ((2, 2),)),
("lineTo", ((0, 0),)), # last line overlaps move...
("closePath", ()),
],
True,
[
("moveTo", ((0, 0),)),
("lineTo", ((2, 2),)),
("lineTo", ((1, 1),)),
("lineTo", ((0, 0),)), # ... but line is NOT implied
("closePath", ()),
],
),
(
[
("moveTo", ((0, 0),)),
("lineTo", ((0, 0),)), # duplicate lineTo following moveTo
("lineTo", ((1, 1),)),
("lineTo", ((2, 2),)),
("closePath", ()),
],
False,
[
("moveTo", ((0, 0),)),
("lineTo", ((2, 2),)),
("lineTo", ((1, 1),)),
("lineTo", ((0, 0),)), # extra explicit lineTo is always emitted to
("lineTo", ((0, 0),)), # disambiguate from an implicit closing line
("closePath", ()),
],
),
(
[
("moveTo", ((0, 0),)),
("lineTo", ((0, 0),)), # duplicate lineTo following moveTo
("lineTo", ((1, 1),)),
("lineTo", ((2, 2),)),
("closePath", ()),
],
True,
[
("moveTo", ((0, 0),)),
("lineTo", ((2, 2),)),
("lineTo", ((1, 1),)),
("lineTo", ((0, 0),)), # duplicate lineTo is retained also in this case,
("lineTo", ((0, 0),)), # same result as with outputImpliedClosingLine=False
("closePath", ()),
],
),
@ -58,21 +114,37 @@ TEST_DATA = [
("lineTo", ((1, 1),)),
("closePath", ()),
],
False,
[
("moveTo", ((0, 0),)),
("lineTo", ((1, 1),)),
("closePath", ()),
],
),
(
[
("moveTo", ((0, 0),)),
("lineTo", ((1, 1),)),
("closePath", ()),
],
True,
[
("moveTo", ((0, 0),)),
("lineTo", ((1, 1),)),
("lineTo", ((0, 0),)),
("closePath", ()),
],
),
(
[
("moveTo", ((0, 0),)),
("curveTo", ((1, 1), (2, 2), (3, 3))),
("curveTo", ((4, 4), (5, 5), (0, 0))),
("curveTo", ((4, 4), (5, 5), (0, 0))), # closed curveTo overlaps moveTo
("closePath", ()),
],
False,
[
("moveTo", ((0, 0),)),
("moveTo", ((0, 0),)), # no extra lineTo added here
("curveTo", ((5, 5), (4, 4), (3, 3))),
("curveTo", ((2, 2), (1, 1), (0, 0))),
("closePath", ()),
@ -82,12 +154,44 @@ TEST_DATA = [
[
("moveTo", ((0, 0),)),
("curveTo", ((1, 1), (2, 2), (3, 3))),
("curveTo", ((4, 4), (5, 5), (6, 6))),
("curveTo", ((4, 4), (5, 5), (0, 0))), # closed curveTo overlaps moveTo
("closePath", ()),
],
True,
[
("moveTo", ((0, 0),)), # no extra lineTo added here, same as preceding
("curveTo", ((5, 5), (4, 4), (3, 3))),
("curveTo", ((2, 2), (1, 1), (0, 0))),
("closePath", ()),
],
),
(
[
("moveTo", ((0, 0),)),
("lineTo", ((6, 6),)), # implied line
("curveTo", ((1, 1), (2, 2), (3, 3))),
("curveTo", ((4, 4), (5, 5), (6, 6))), # closed curve not overlapping move
("closePath", ()),
],
False,
[
("moveTo", ((0, 0),)),
("lineTo", ((6, 6),)), # the previously implied line
("curveTo", ((5, 5), (4, 4), (3, 3))),
("curveTo", ((2, 2), (1, 1), (0, 0))),
("closePath", ()),
],
),
(
[
("moveTo", ((0, 0),)),
("curveTo", ((1, 1), (2, 2), (3, 3))),
("curveTo", ((4, 4), (5, 5), (6, 6))), # closed curve not overlapping move
("closePath", ()),
],
True,
[
("moveTo", ((0, 0),)),
("lineTo", ((6, 6),)), # the previously implied line (same as above)
("curveTo", ((5, 5), (4, 4), (3, 3))),
("curveTo", ((2, 2), (1, 1), (0, 0))),
("closePath", ()),
@ -101,6 +205,7 @@ TEST_DATA = [
("curveTo", ((5, 5), (6, 6), (7, 7))),
("closePath", ()),
],
False,
[
("moveTo", ((0, 0),)),
("lineTo", ((7, 7),)),
@ -112,12 +217,31 @@ TEST_DATA = [
(
[
("moveTo", ((0, 0),)),
("qCurveTo", ((1, 1), (2, 2))),
("qCurveTo", ((3, 3), (0, 0))),
("lineTo", ((1, 1),)), # this line...
("curveTo", ((2, 2), (3, 3), (4, 4))),
("curveTo", ((5, 5), (6, 6), (7, 7))),
("closePath", ()),
],
True,
[
("moveTo", ((0, 0),)),
("lineTo", ((7, 7),)),
("curveTo", ((6, 6), (5, 5), (4, 4))),
("curveTo", ((3, 3), (2, 2), (1, 1))),
("lineTo", ((0, 0),)), # ... does NOT become implied
("closePath", ()),
],
),
(
[
("moveTo", ((0, 0),)),
("qCurveTo", ((1, 1), (2, 2))),
("qCurveTo", ((3, 3), (0, 0))), # closed qCurve overlaps move
("closePath", ()),
],
False,
[
("moveTo", ((0, 0),)), # no extra lineTo added here
("qCurveTo", ((3, 3), (2, 2))),
("qCurveTo", ((1, 1), (0, 0))),
("closePath", ()),
@ -127,12 +251,44 @@ TEST_DATA = [
[
("moveTo", ((0, 0),)),
("qCurveTo", ((1, 1), (2, 2))),
("qCurveTo", ((3, 3), (4, 4))),
("qCurveTo", ((3, 3), (0, 0))), # closed qCurve overlaps move
("closePath", ()),
],
True, # <--
[
("moveTo", ((0, 0),)), # no extra lineTo added here, same as above
("qCurveTo", ((3, 3), (2, 2))),
("qCurveTo", ((1, 1), (0, 0))),
("closePath", ()),
],
),
(
[
("moveTo", ((0, 0),)),
("lineTo", ((4, 4),)),
("qCurveTo", ((1, 1), (2, 2))),
("qCurveTo", ((3, 3), (4, 4))), # closed qCurve not overlapping move
("closePath", ()),
],
False,
[
("moveTo", ((0, 0),)),
("lineTo", ((4, 4),)), # the previously implied line
("qCurveTo", ((3, 3), (2, 2))),
("qCurveTo", ((1, 1), (0, 0))),
("closePath", ()),
],
),
(
[
("moveTo", ((0, 0),)),
("qCurveTo", ((1, 1), (2, 2))),
("qCurveTo", ((3, 3), (4, 4))), # closed qCurve not overlapping move
("closePath", ()),
],
True,
[
("moveTo", ((0, 0),)),
("lineTo", ((4, 4),)), # the previously implied line (same as above)
("qCurveTo", ((3, 3), (2, 2))),
("qCurveTo", ((1, 1), (0, 0))),
("closePath", ()),
@ -145,6 +301,7 @@ TEST_DATA = [
("qCurveTo", ((2, 2), (3, 3))),
("closePath", ()),
],
False,
[
("moveTo", ((0, 0),)),
("lineTo", ((3, 3),)),
@ -154,14 +311,16 @@ TEST_DATA = [
),
(
[("addComponent", ("a", (1, 0, 0, 1, 0, 0)))],
False,
[("addComponent", ("a", (1, 0, 0, 1, 0, 0)))],
),
([], []),
([], False, []),
(
[
("moveTo", ((0, 0),)),
("endPath", ()),
],
False,
[
("moveTo", ((0, 0),)),
("endPath", ()),
@ -172,6 +331,7 @@ TEST_DATA = [
("moveTo", ((0, 0),)),
("closePath", ()),
],
False,
[
("moveTo", ((0, 0),)),
("endPath", ()), # single-point paths is always open
@ -179,10 +339,12 @@ TEST_DATA = [
),
(
[("moveTo", ((0, 0),)), ("lineTo", ((1, 1),)), ("endPath", ())],
False,
[("moveTo", ((1, 1),)), ("lineTo", ((0, 0),)), ("endPath", ())],
),
(
[("moveTo", ((0, 0),)), ("curveTo", ((1, 1), (2, 2), (3, 3))), ("endPath", ())],
False,
[("moveTo", ((3, 3),)), ("curveTo", ((2, 2), (1, 1), (0, 0))), ("endPath", ())],
),
(
@ -192,6 +354,7 @@ TEST_DATA = [
("lineTo", ((4, 4),)),
("endPath", ()),
],
False,
[
("moveTo", ((4, 4),)),
("lineTo", ((3, 3),)),
@ -206,6 +369,7 @@ TEST_DATA = [
("curveTo", ((2, 2), (3, 3), (4, 4))),
("endPath", ()),
],
False,
[
("moveTo", ((4, 4),)),
("curveTo", ((3, 3), (2, 2), (1, 1))),
@ -215,10 +379,12 @@ TEST_DATA = [
),
(
[("qCurveTo", ((0, 0), (1, 1), (2, 2), None)), ("closePath", ())],
False,
[("qCurveTo", ((0, 0), (2, 2), (1, 1), None)), ("closePath", ())],
),
(
[("qCurveTo", ((0, 0), (1, 1), (2, 2), None)), ("endPath", ())],
False,
[
("qCurveTo", ((0, 0), (2, 2), (1, 1), None)),
("closePath", ()), # this is always "closed"
@ -237,6 +403,7 @@ TEST_DATA = [
("qCurveTo", ((449, -3), (649, -3), (848, 171), (848, 348))),
("closePath", ()),
],
False,
[
("moveTo", ((848, 348),)),
("qCurveTo", ((848, 171), (649, -3), (449, -3), (449, -3))),
@ -260,6 +427,7 @@ TEST_DATA = [
("lineTo", ((0, 651),)),
("closePath", ()),
],
False,
[
("moveTo", ((0, 651),)),
("lineTo", ((0, 651),)),
@ -268,13 +436,32 @@ TEST_DATA = [
("closePath", ()),
],
),
(
[
("moveTo", ((0, 651),)),
("lineTo", ((0, 101),)),
("lineTo", ((0, 101),)),
("lineTo", ((0, 651),)),
("lineTo", ((0, 651),)),
("closePath", ()),
],
True,
[
("moveTo", ((0, 651),)),
("lineTo", ((0, 651),)),
("lineTo", ((0, 101),)),
("lineTo", ((0, 101),)),
("lineTo", ((0, 651),)), # closing line not implied
("closePath", ()),
],
),
]
@pytest.mark.parametrize("contour, expected", TEST_DATA)
def test_reverse_pen(contour, expected):
@pytest.mark.parametrize("contour, outputImpliedClosingLine, expected", TEST_DATA)
def test_reverse_pen(contour, outputImpliedClosingLine, expected):
recpen = RecordingPen()
revpen = ReverseContourPen(recpen)
revpen = ReverseContourPen(recpen, outputImpliedClosingLine)
for operator, operands in contour:
getattr(revpen, operator)(*operands)
assert recpen.value == expected
@ -288,7 +475,13 @@ def test_reverse_pen_outputImpliedClosingLine():
revpen.lineTo((0, 10))
revpen.lineTo((0, 0))
revpen.closePath()
assert len(recpen.value) == 4
assert recpen.value == [
("moveTo", ((0, 0),)),
("lineTo", ((0, 10),)),
("lineTo", ((10, 0),)),
# ("lineTo", ((0, 0),)), # implied
("closePath", ()),
]
recpen = RecordingPen()
revpen = ReverseContourPen(recpen, outputImpliedClosingLine=True)
@ -297,11 +490,17 @@ def test_reverse_pen_outputImpliedClosingLine():
revpen.lineTo((0, 10))
revpen.lineTo((0, 0))
revpen.closePath()
assert len(recpen.value) == 6
assert recpen.value == [
("moveTo", ((0, 0),)),
("lineTo", ((0, 10),)),
("lineTo", ((10, 0),)),
("lineTo", ((0, 0),)), # not implied
("closePath", ()),
]
@pytest.mark.parametrize("contour, expected", TEST_DATA)
def test_reverse_point_pen(contour, expected):
@pytest.mark.parametrize("contour, outputImpliedClosingLine, expected", TEST_DATA)
def test_reverse_point_pen(contour, outputImpliedClosingLine, expected):
from fontTools.ufoLib.pointPen import (
ReverseContourPointPen,
PointToSegmentPen,
@ -309,24 +508,10 @@ def test_reverse_point_pen(contour, expected):
)
recpen = RecordingPen()
pt2seg = PointToSegmentPen(recpen, outputImpliedClosingLine=True)
pt2seg = PointToSegmentPen(recpen, outputImpliedClosingLine)
revpen = ReverseContourPointPen(pt2seg)
seg2pt = SegmentToPointPen(revpen)
for operator, operands in contour:
getattr(seg2pt, operator)(*operands)
# for closed contours that have a lineTo following the moveTo,
# and whose points don't overlap, our current implementation diverges
# from the ReverseContourPointPen as wrapped by ufoLib's pen converters.
# In the latter case, an extra lineTo is added because of
# outputImpliedClosingLine=True. This is redundant but not incorrect,
# as the number of points is the same in both.
if (
contour
and contour[-1][0] == "closePath"
and contour[1][0] == "lineTo"
and contour[1][1] != contour[0][1]
):
expected = expected[:-1] + [("lineTo", contour[0][1])] + expected[-1:]
assert recpen.value == expected