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

Tracking issue for curve fitting robustness #279

Open
richard-uk1 opened this issue May 4, 2023 · 4 comments
Open

Tracking issue for curve fitting robustness #279

richard-uk1 opened this issue May 4, 2023 · 4 comments

Comments

@richard-uk1
Copy link
Collaborator

This is a new tracking issue for fitting robustness. There shouldn't be too many problems once #269 lands, but if there are any, please post them here.

This issue also tracks curve offset, as that is a special case of curve fitting.

@raphlinus
Copy link
Contributor

Achieving perfection here will take a lot of work, but there are two things I plan to do in the coming days which I believe will help a lot.

First, make the sample_pt_tangent() implementation in CurveOffset handle zero and near-zero derivatives in the source curve, as well as cusps caused by offsets. There's some logic for that in simplify_bezpath(), but that's at a higher level, before it gets into curve fitting.

Second, degenerate curves are manifesting as infinite recursion, it keeps subdividing (apparently sometimes to zero length) and you get either a hang or a stack overflow depending on the details. I'll add special case logic to handle the endpoints within the given accuracy of each other. You usually don't want to get there, but if you do, it should just draw a line, which is the simplest answer that meets the Fréchet bound.

@raphlinus
Copy link
Contributor

There are a number of test cases floating around, but I'll start with this one from #269, as it's clearly documented. It hangs with the _opt variant, but also generates a stack overflow (indicating infinitely recursing subdivision) in the non-opt case. Just looking at the input, it's not obvious to me where the problem is, as the source curve has perfectly fine derivatives everywhere. I'll focus on that one first, then bring in more cases as needed.

@raphlinus
Copy link
Contributor

Ok, for that particular failure it's now clear what's going on. There's some logic in CubicOffset::break_cusp called break_cusp_help which is supposed to detect the case that a cusp is near the endpoint, and it has some "magic" epsilon values (CUSP_EPS = 1e-8 is the most relevant one here) which in this case fail to trigger. Of course, this results in an extremely short segment, so I think handling that (before invoking cusp logic) will at least let the operation succeed, though the output will be less than ideal. It might also be worth thinking about more sophisticated logic for computing the epsilon - ideally you want a value large enough to avoid this kind of failure, but not so large you miss a cusp and generate incorrect output.

raphlinus added a commit that referenced this issue Jun 8, 2023
The main improvement is to try to fit a line when the chord is very short. Very short segments can occur when the cusp finding reports imprecise results, and also the main cubic fit logic is not robust when the chord is short.

I believe the simple subdivision case is now fairly robust in that it won't keep recursing. This is in spite of not having an explicit bound on recursion depth; the theory is that each subdivision reduces the parameter space in half, and at some point you'll reach the point where the chord is shorter than the given tolerance. This is true even for an adversarial break_cusp, as it's bypassed in the short chord case.

The logic for the optimized case is not as careful, in particular break_cusp is not bypassed. I'm curious whether there are still failure cases.

This is likely not the end of the numerical robustness work. In particular, break_cusp can be better tuned to not over-report cusps, and the tangent detection can be improved for cusp and near-cusp source curves. But hopefully this commit gets us to where we at least return a valid fit.

Also adds a test lightly adapted from #269.

Progress toward #269 and #279
@valadaptive
Copy link

valadaptive commented Aug 14, 2024

Not sure if this falls under the category of "robustness" or simply subpar results, but I'm trying to use kurbo's curve-simplification algorithm and there are some pretty large "bumps" in the resulting paths, of the sort described in your blog post:

image
image

These can be reproduced with this path, with an accuracy of 1.0 and an angle threshold of 10 degrees (not sure if that actually affects the algorithm in this particular case).

Tweaking the currently-hardcoded D_PENALTY_ELBOW and D_PENALTY_SLOPE values helps with this, but they need to be set to some pretty aggressive values.

This may require the development of a better metric, as you said in your blog post. There's also been a recent paper about simplifying Bezier splines that compares against kurbo's algorithm and claims an improvement; that may be worth looking into (sadly I am not smart enough to implement it).

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

3 participants