I looked at this again and got suspicious becuase I noticed that the number of pen commands increased from 4 to 6, whereas I only expected it to be 5 (4+1) in the outputImpliedClosingLine=True case.
Turns out we are adding an extra duplicate lineTo right after moveTo, which I think is not correct.
The PointToSegmentPen translates between PointPen and (Segment)Pen
protocol.
In the SegmentPen protocol, closed contours always imply a final 'lineTo'
segment from the last oncurve point to the starting point.
So the PointToSegmentPen omits the final 'lineTo' segment for closed
contours -- unless the option 'outputImpliedClosingLine' is True
(it is False by default, and defcon.Glyph.draw method initializes the
converter pen without this option).
However, if the last oncurve point is on a "line" segment and has same
coordinates as the starting point of a closed contour, the converter pen must
always output the closing 'lineTo' explicitly (regardless of the value of the
'outputImpliedClosingLine' option) in order to disambiguate this case from
the implied closing 'lineTo'.
If it doesn't do that, a duplicate 'line' point at the end of a closed
contour gets lost in the conversion.
See https://github.com/googlefonts/fontmake/issues/572.
This test is not normally run. It is skipped when ufoLib is not importable,
as it's the case from the test runner's virtual environment.
The only reason it exists is so that I could check that the
ReverseContourPen I was writing behaves the same as using ufoLib's
ReverseContourPointPen when the latter is run through the
SegmentToPointPen and PointToSegmentPen converters.
The two methods for reversing contours now diverge since
https://github.com/fonttools/fonttools/pull/1080,
but only nominally because both produce effectively the same results.
The only difference is that, when using the point pens with
outputImpliedClosingLine=True, the closing lineTo is always there even
when it's redundant. In our implmentation, we only output the closing
lineTo when it is the same as moveTo (this was necessary in order to
fix https://github.com/googlei18n/cu2qu/issues/51)
Nevertheless, the total number of points is the same in both cases.
Maybe this test should not be here, as it's testing functionalities
from a different package.
Closes https://github.com/fonttools/fonttools/issues/1081
One way to work around https://github.com/googlei18n/cu2qu/issues/51
when using the ufoLib's ReverseConturPointPen via the converter pens
would be to pass the outputImpliedClosingLine=True argument
(False by default) to the PointToSegmentPen.
This way, all the final lineTos are explicitly outputted, even when
they are redundant and could be implied (i.e. when they are not
duplicate points).
Note that this test is skipped by the CI, because ufoLib is not a
dependency of fonttools; you can run locally if you wish.
