fonttools / skia-pathops

Python bindings for the Skia library's Path Ops
https://skia.org/docs/dev/present/pathops/
BSD 3-Clause "New" or "Revised" License
47 stars 14 forks source link

WIP Keep track of on-curve points in the original path #47

Open belluzj opened 3 years ago

belluzj commented 3 years ago

Hello @anthrotype ! This is both an issue and a pull request that starts to fix the issue. Here is the problem: a designer drew a "registered" sign that is so symmetrical that the code in skia-pathops to reconstruct long quadratic curves with implicit points from the small quadratic curves that come out of Skia thought that the top extrema of the curve, which was a real on-curve point initially, was actually an implicit point and made it disappear. That then caused confusion when the designer was trying to hint the glyph in VTT. See screenshots: on the left, the glyph as it comes out of current skia-pathops, on the right as it comes out with this patch.

image image

I added a test and I'm proposing a way to fix this by remembering the coordinates of the intended on-curve points in the Path object, and then making sure that they're still on-curve after reconstructing the long qCurveTo commands.

However I didn't finish implementing this properly because I'm not sure how thorough I should be with this, e.g. right now it only works if you draw on the Path with its pen, but doesn't if you add path bits via the Path methods...

Also I haven't figured out how to use the Skia matrix transformation to transform single points (needed to keep track of "original on-curves" while decomposing components with transformations).

And also it's the first time I touch Cython code and I don't know whether I'm trashing performance with my changes.

anthrotype commented 3 years ago

one problem with keeping a set of 'originalOnCurvePoints' is that Path is a mutable object (new paths can be added, or the container can be cleared with reset/rewind methods) so they may get out of sync. I'm still thinking this through, trying to see if there's an alternative way to achive this. E.g. we might simply keep the oncurves that fall on a curve's vertical/horizontal min/max extrema where hinting instructions usually get anchored. While it may fix this particular case, but not be a general solution.