booleanOperations
booleanOperations copied to clipboard
WIP: Attempt to fix issue with segments disappearing
This is an attempt at fixing https://github.com/googlei18n/fontmake/issues/180
@typemytype could you please have a look?
it appears like when a curve segment is "flat", i.e. the offcurves are on the same line connecting the previous and current on-curve, and it happens to be the last segment in the output contour, sometimes it can end up being dropped altogether, as shown in the above mentioned fontmake issue.
The glyph in question, which prompted this bug, is the following:
<?xml version="1.0" encoding="UTF-8"?>
<glyph name="uniA73A" format="2">
<advance width="797"/>
<unicode hex="A73A"/>
<outline>
<contour>
<point x="0" y="0" type="line"/>
<point x="29" y="0" type="line"/>
<point x="245" y="593" type="line" smooth="yes"/>
<point x="255" y="620"/>
<point x="267" y="652"/>
<point x="277" y="682" type="curve"/>
<point x="289" y="645"/>
<point x="299" y="617"/>
<point x="307" y="594" type="curve" smooth="yes"/>
<point x="515" y="0" type="line"/>
<point x="544" y="0" type="line"/>
<point x="797" y="714" type="line"/>
<point x="767" y="714" type="line"/>
<point x="567" y="143" type="line" smooth="yes"/>
<point x="554" y="107"/>
<point x="542" y="73"/>
<point x="530" y="40" type="curve"/>
<point x="517" y="76"/>
<point x="504" y="112"/>
<point x="491" y="148" type="curve" smooth="yes"/>
<point x="291" y="716" type="line"/>
<point x="265" y="716" type="line"/>
</contour>
<contour>
<point x="146" y="344" type="line"/>
<point x="655" y="344" type="line"/>
<point x="655" y="369" type="line"/>
<point x="146" y="369" type="line"/>
</contour>
</outline>
</glyph>
The mergeFirstSegments
logic in the (extremely complicated) OutputContour.reCurveSubSegments
method seems to be responsible for popping up the last segment.
https://github.com/typemytype/booleanOperations/blob/master/Lib/booleanOperations/flatten.py#L765-L803
If the segmentType of such "flat" curves is set to be "line" (because that's what they actually are), then they will be kept.
I hope that makes sense.
this other issue seems to be related to that mergeFirstSegments
issue as well:
https://github.com/googlei18n/fontmake/issues/181#issuecomment-258485735
The patch in the current PR does not fix the issue with the latter glyph, so I'm no longer sure if it's the correct one.
Only if I comment out the following tow lines (i.e. completely disabling the mergeFirstSegments
thing) I can get the segment to not disappear:
https://github.com/typemytype/booleanOperations/blob/master/Lib/booleanOperations/flatten.py#L801-L802
I still can't understand what is the rationale for that piece of code.
BTW, this is the glyph in question:
<?xml version="1.0" encoding="UTF-8"?>
<glyph name="A" format="2">
<advance width="500"/>
<unicode hex="0041"/>
<outline>
<contour>
<point x="296" y="-467" type="curve" smooth="yes"/>
<point x="1542" y="-465" type="line"/>
<point x="1396" y="-393" type="line"/>
<point x="292" y="-395" type="line" smooth="yes"/>
<point x="214" y="-395"/>
<point x="181" y="-349"/>
<point x="181" y="-269" type="curve" smooth="yes"/>
<point x="181" y="806" type="line" smooth="yes"/>
<point x="181" y="885"/>
<point x="214" y="931"/>
<point x="292" y="931" type="curve" smooth="yes"/>
<point x="1531" y="931" type="line" smooth="yes"/>
<point x="1607" y="931"/>
<point x="1642" y="885"/>
<point x="1642" y="806" type="curve" smooth="yes"/>
<point x="1642" y="760" type="line"/>
<point x="1731" y="760" type="line"/>
<point x="1731" y="789" type="line" smooth="yes"/>
<point x="1731" y="932"/>
<point x="1663" y="1003"/>
<point x="1526" y="1003" type="curve" smooth="yes"/>
<point x="296" y="1003" type="line" smooth="yes"/>
<point x="159" y="1003"/>
<point x="91" y="932"/>
<point x="91" y="789" type="curve" smooth="yes"/>
<point x="91" y="-253" type="line" smooth="yes"/>
<point x="91" y="-396"/>
<point x="159" y="-467"/>
</contour>
<contour>
<point x="1542" y="-465" type="curve" smooth="yes"/>
<point x="1658" y="-465"/>
<point x="1729" y="-381"/>
<point x="1729" y="-282" type="curve" smooth="yes"/>
<point x="1729" y="-177"/>
<point x="1661" y="-90"/>
<point x="1545" y="-90" type="curve" smooth="yes"/>
<point x="1433" y="-90"/>
<point x="1362" y="-171"/>
<point x="1362" y="-278" type="curve" smooth="yes"/>
<point x="1362" y="-385"/>
<point x="1432" y="-465"/>
</contour>
<contour>
<point x="1546" y="-398" type="curve" smooth="yes"/>
<point x="1482" y="-398"/>
<point x="1441" y="-348"/>
<point x="1441" y="-281" type="curve" smooth="yes"/>
<point x="1441" y="-208"/>
<point x="1481" y="-157"/>
<point x="1546" y="-157" type="curve" smooth="yes"/>
<point x="1610" y="-157"/>
<point x="1649" y="-210"/>
<point x="1649" y="-283" type="curve" smooth="yes"/>
<point x="1649" y="-350"/>
<point x="1606" y="-398"/>
</contour>
</outline>
</glyph>
from the git log, I can see @readroberts also contributed to this section of the code where it merges the first and last segments when the last segment is a curve: cd59abbb8bb9cb5fe0756dcceaec41270f9c2d14
I wonder if he could also shed some light on this bug.
I will look at this in the next few days - can't immediately remember what that function is for, although I certainly added it for cases that are otherwise bugs.
@readroberts : is this fixed?
No, I have not yet taken the time to review this. I expect to do so next week.