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

this contour crashes crossingsRemoved #80

Closed
mbutterick opened this issue Jan 15, 2021 · 9 comments
Closed

this contour crashes crossingsRemoved #80

mbutterick opened this issue Jan 15, 2021 · 9 comments

Comments

@mbutterick
Copy link

Screen Shot 2021-01-15 at Jan 15  1 44 57 PM

I am making a Path using this contour as a component and passing it through crossingsRemoved(). The two off-curve points are key. Once the upper point gets one unit to the right of the lower point, it triggers this assertion in addNeighbor.

@mbutterick
Copy link
Author

BTW if I disable the assert, nothing bad seems to happen, so I can’t tell if the assert is protecting against a stricter condition than necessary, or if there is some subtle logic or math error happening deeper. Considering that the error seems very sensitive to the exact relative positions of the handles, I suspect the latter.

@hfutrell
Copy link
Owner

Wow, I've never seen that assert hit before!

I am not sure why it's happening here. addNeighbor is called on a per-intersection basis and it does not appear that the path self-intersects so I'm unsure why it is called at all.

I'll try to reproduce this issue soon.

@mbutterick
Copy link
Author

mbutterick commented Jan 17, 2021

This contour also crashes crossingsRemoved — seemingly related to having four points at the same Y coordinate on the top edge. Is there a way I can print a text-based representation of a path that would make reconstructing the test case easier? (Because these errors seem to be very sensitive to point position)

@hfutrell
Copy link
Owner

hfutrell commented Jan 17, 2021

@mbutterick if you have the debugger paused at the assert, try navigating the stack up to AugmentedGraph.init(...) and type:

po path1.components.map { $0.curves }

This should print out the curves that make up the contour and I can make a test case directly from that.

@mbutterick
Copy link
Author

mbutterick commented Jan 17, 2021

Seems likely it’s a different bug so I’ll post a separate issue (see #81)

@mbutterick
Copy link
Author

Here’s a test case that produces same error. Slightly different shape on top but the problematic inflected curve at the bottom is the same.

Text:

path.components.map { $0.curves } =
[[BezierKit.CubicCurve(p0: (275.0, 656.0), p1: (325.0, 656.0), p2: (324.0, 594.0), p3: (372.0, 594.0)), BezierKit.CubicCurve(p0: (372.0, 594.0), p1: (406.0, 594.0), p2: (426.0, 616.0), p3: (426.0, 645.0)), BezierKit.CubicCurve(p0: (426.0, 645.0), p1: (426.0, 691.0), p2: (377.0, 713.0), p3: (329.0, 713.0)), BezierKit.CubicCurve(p0: (329.0, 713.0), p1: (228.0, 713.0), p2: (240.0, 656.0), p3: (275.0, 656.0))]]
path.crossingsRemoved().components.map { $0.curves } =
[[BezierKit.CubicCurve(p0: (275.0, 656.0), p1: (300.5, 656.0), p2: (312.7349, 639.8738000000001), p3: (324.96979999999996, 624.0701239999999)), BezierKit.CubicCurve(p0: (324.96979999999996, 624.0701239999999), p1: (336.7249, 608.8862), p2: (348.48, 594.0), p3: (372.0, 594.0)), BezierKit.CubicCurve(p0: (372.0, 594.0), p1: (406.0, 594.0), p2: (426.0, 616.0), p3: (426.0, 645.0)), BezierKit.CubicCurve(p0: (426.0, 645.0), p1: (426.0, 691.0), p2: (377.0, 713.0), p3: (329.0, 713.0)), BezierKit.CubicCurve(p0: (329.0, 713.0), p1: (228.0, 713.0), p2: (240.0, 656.0), p3: (275.0, 656.0))]]

Visual:
Screen Shot 2021-01-17 at 11 35 43 AM

@hfutrell
Copy link
Owner

hfutrell commented Jan 18, 2021

@mbutterick I am unfortunately not able to reproduce the issue. Here is the code I'm running trying to hit the assert with your first path:

        let mutablePath = CGMutablePath()
        mutablePath.move(to: CGPoint(x: 0, y: 62))
        mutablePath.addCurve(to: CGPoint(x: 97, y: 0),
                      control1: CGPoint(x: 50, y: 62),
                      control2: CGPoint(x: 49, y: 0))
        mutablePath.addLine(to: CGPoint(x: 78, y: 102))
        mutablePath.addLine(to: CGPoint(x: 0, y: 62))
        let path = Path(cgPath: mutablePath)
        let result = path.crossingsRemoved()
        assert(path == result)

Here is the code I'm running to try to reproduce the issue with the second path

        let points = [CGPoint(x: 275.0, y: 656.0), CGPoint(x: 325.0, y: 656.0), CGPoint(x: 324.0, y: 594.0), CGPoint(x: 372.0, y: 594.0),
                      CGPoint(x: 406.0, y: 594.0), CGPoint(x: 426.0, y: 616.0), CGPoint(x: 426.0, y: 645.0),
                      CGPoint(x: 426.0, y: 691.0), CGPoint(x: 377.0, y: 713.0), CGPoint(x: 329.0, y: 713.0),
                      CGPoint(x: 228.0, y: 713.0), CGPoint(x: 240.0, y: 656.0), CGPoint(x: 275.0, y: 656.0)]
        let path = Path(components: [PathComponent(points: points, orders: [3, 3, 3, 3])])
        let result = path.crossingsRemoved()
        assert(path == result)

Everything seems ok on my end. If you run these code snippets are things working ok on your end?

I might need more information, like what branch you are using, OS version, XCode version, hardware things are running (especially 32 vs 64 bit).

@mbutterick
Copy link
Author

Sorry, that was my error. My copy of BezierKit was a little behind the current one. I’ve updated and the issue no longer recurs.

@hfutrell
Copy link
Owner

good to know BezierKit is at least improving over time :)

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