Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

belluzj
Copy link

@belluzj belluzj commented Jun 3, 2021

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.


def qCurveTo(self, *points):
for pt1, pt2 in _decompose_quadratic_segment(points):
self._qCurveToOne(pt1, pt2)
self.path.originalOnCurvePoints.add(points[-1])
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize now that I could probably only keep track of the original on-curves at the end of qCurves, instead of all types of original on-curves?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, if we have to remember oncurve points, it's probably better to only do it for when we actually need it, i.e. for qCurve segments only where they can be "implied", the other segment types in pen protocol are all explicit

# TODO: figure out how to transform the original oncurve points using the matrix
# result.originalOnCurvePoints = set(
# transform_tuple_point(matrix, pt)
# for pt in self.originalOnCurvePoints
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SkMatrix has mapPoint and mapPoints (for array of SkPoints) methods:
https://api.skia.org/classSkMatrix.html#a545fc5d678f0c07c40636bc4cb699017

@anthrotype
Copy link
Member

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.

assert items[2][0] == PathVerb.QUAD
assert items[2][1] == ((2.0, 2.0), (3.0, 3.0))

# when drawn back onto a SegmentPen, the implicit on-curves are omitted
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# when drawn back onto a SegmentPen, the implicit on-curves are omitted
# when drawn back onto a SegmentPen, the explicit on-curves are preserved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants