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

crossingsRemoved gets confused by coincident points #84

Open
mbutterick opened this issue Jan 21, 2021 · 3 comments
Open

crossingsRemoved gets confused by coincident points #84

mbutterick opened this issue Jan 21, 2021 · 3 comments

Comments

@mbutterick
Copy link

Without detracting from your success with #81, I discovered a case that still fails with that patch:

path.components.map { $0.curves } =
[[BezierKit.LineSegment(p0: (100.0, 585.0), p1: (100.0, 585.0)), BezierKit.LineSegment(p0: (100.0, 585.0), p1: (225.0, 585.0)), BezierKit.LineSegment(p0: (225.0, 585.0), p1: (100.0, 680.0)), BezierKit.LineSegment(p0: (100.0, 680.0), p1: (100.0, 585.0))], [BezierKit.LineSegment(p0: (260.0, 392.0), p1: (260.0, 585.0)), BezierKit.LineSegment(p0: (260.0, 585.0), p1: (160.0, 680.0)), BezierKit.LineSegment(p0: (160.0, 680.0), p1: (260.0, 392.0))]]
path.crossingsRemoved().components.map { $0.curves } =
[]

Looks like:

Screen Shot 2021-01-21 at Jan 21  12 30 48 PM

The bugaboo is that we have two points sitting atop each other at (100,585). There is no value of the discriminant that will cure this problem (so it deserves to be considered a distinct issue, I think).

In this case the “crossing” has zero area, so the “removal” (based on behavior of other crossing-removal algorithms I’ve used) should mean that the coincident points are collapsed into a single point.

For that matter, there doesn’t even need to be an actual geometric overlap for the operation to fail. A simpler example:

path.components.map { $0.curves } =
[[BezierKit.LineSegment(p0: (290.0, 467.0), p1: (290.0, 467.0)), BezierKit.LineSegment(p0: (290.0, 467.0), p1: (394.0, 467.0)), BezierKit.LineSegment(p0: (394.0, 467.0), p1: (290.0, 579.0)), BezierKit.LineSegment(p0: (290.0, 579.0), p1: (290.0, 467.0))]]
path.crossingsRemoved().components.map { $0.curves } =
[]

Screen Shot 2021-01-21 at Jan 21  12 44 53 PM

@hfutrell
Copy link
Owner

hfutrell commented Jan 21, 2021

@mbutterick this is definitely a distinct issue, thanks for opening it. As a workaround I think you'll want to remove these kinds of degeneracies before calling crossingsRemoved()

I'm undecided whether the output of the second should be equal to the input, or have the duplicate point "collapsed". But it definitely shouldn't be doing what it's doing now ...

@mbutterick
Copy link
Author

mbutterick commented Jan 21, 2021

Sure, I can work around it. FWIW as a longtime consumer of Béziers I would vote for optimizing the contour by removing extraneous points that add no information to the curve (especially because crossingsRemoved is already changing the topology of the curves)

When composing operations on Béziers (like Boolean operations or affine transformations) it’s possible to inadvertently create situations with overlapping points. If the path optimization doesn’t happen automatically, these little errors can cause unintended consequences downstream (“hey, why are there 200 points piled up here?!”)

For instance, here’s another example that is benign but not optimal: these two rectangles share an edge. crossingsRemoved correctly merges them into one contiguous area, but leaves behind two collinear points on the long edges. If I were to pass this to other Bézier operations hoping it would behave as a rectangle — let’s suppose I rotate it and scale it and rotate it some more — I might be surprised when rounding errors move the middle points out of alignment, resulting in “dents” in my supposed rectangle.

Screen Shot 2021-01-21 at Jan 21  2 33 08 PM

Screen Shot 2021-01-21 at Jan 21  2 36 40 PM

@hfutrell
Copy link
Owner

hfutrell commented Jan 29, 2021

You've given me things to think about. My caution is that degenerate or collinear curves may have some kind of domain-specific meaning and it could be frustrating for end-users to have them removed automatically. My intuition is that if a user passes in a path that doesn't self-intersect they'd expect it to pop out unmodified. Maybe there should be a separate method in BezierKit to remove these features to merge successive elements that are collinear / degenerate?

I'm glad that the case of collinear edges is actually working for you :). It took some special care to make sure that it would work at all!

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

No branches or pull requests

2 participants