search

typemytype / booleanOperations

Boolean operations on paths
MIT License
38 stars 18 forks source link

Fix bug that triggers an assert for paths with an initial move. #22

Closed readroberts closed 8 years ago

anthrotype commented 8 years ago

ping @typemytype?

typemytype commented 8 years ago

He Read

I understand, booleanOperations likes to have closed shapes. But auto closing should be an option and handled nicely by the pen, instead of an extra api call. It could be solved here https://github.com/typemytype/booleanOperations/blob/master/Lib/booleanOperations/booleanGlyph.py#L30

I would like to propose BooleanGlyph(glyphObject, ignoreOpenPaths=False)

This would ignore open paths when set True and auto close open paths when set False (the default).

typemytype commented 8 years ago

see https://github.com/typemytype/booleanOperations/commit/42501dd2b9dec9807af2fe7d30845f7c1304cf23

readroberts commented 8 years ago

Hi Frederik;

The problem actually isn't about closing paths at all. Open paths are already handled by the code quite well. The problem is that in some cases, Robofont adds an unnecessary initial 'move' to an outline.The current code triggers an assert that a 'move' cannot be in the path , at line 341 in flatten.py:addPoint(). If you use flatten.py::.ContourPointDataPen to get the data, then the initial move is deleted in flatten.ContourPointDataPen.getData() before flatten.py:OutputContour.drawPoints. However, booleanGlypy.py uses robofab.pens.adapterPens.SegmentToPointPen, which does not remove the point. It is clearly a good idea to remove the unnecessary initial move pt, the question is where. I am reluctant to edit robofab.pens.adapterPens.SegmentToPointPen, so it makes more sense to me to remove it in booleanGlyph. It probably would be cleaner to subclass SegmentToPointPen and add the cleanup code there.

typemytype commented 8 years ago

Can you reproduce this, where RoboFont adds an additional move segment?

I will add a check in BooleanGlyphDataPointPen

thanks

typemytype commented 8 years ago

Ive revered it back and added a check for double points on start and end of contour

https://github.com/typemytype/booleanOperations/commit/861cc4790983300118ad2c76947519dfa3b57625

readroberts commented 8 years ago

Hi Frederik;

I have not been able to reproduce this. It happens in font data where some has used anchors a lot in UFO 1, and moved to UFO 2. This happens mostly in UFO fonts developed by Paul Hunt in our group: he has been working on fonts with a lot of accents for the last two or three years. Out of about 1000 glyphs, I have seen about 20 glyphs with the problem: enough so that I need to deal with it.

From: Frederik Berlaen notifications@github.com<mailto:notifications@github.com> Reply-To: typemytype/booleanOperations reply@reply.github.com<mailto:reply@reply.github.com> Date: Wednesday, February 24, 2016 at 2:03 AM To: typemytype/booleanOperations booleanOperations@noreply.github.com<mailto:booleanOperations@noreply.github.com> Cc: Read Roberts rroberts@adobe.com<mailto:rroberts@adobe.com> Subject: Re: [booleanOperations] Fix bug that triggers an assert for paths with an initial move. (#22)

Can you reproduce this, where RoboFont adds an additional move segment?

I will add a check in BooleanGlyphDataPointPen

thanks

— Reply to this email directly or view it on GitHubhttps://github.com/typemytype/booleanOperations/pull/22#issuecomment-188172805.

typemytype commented 8 years ago

back to auto close open paths https://github.com/typemytype/booleanOperations/commit/b7a067f245024a2571bc2b871b9fa57efd376aae