hfutrell / BezierKit

Bezier curves and paths in Swift for building vector applications
MIT License
277 stars 26 forks source link

`crossingsRemoved` gets confused by coincident points #84

Open mbutterick opened 3 years ago

mbutterick commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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!