Improve robustness of curve fitting #289
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
|
This is a substantial improvement. We’re still hitting a stack overflow with tiger that is caused when Further, if I remove all the point equality comparisons guarding cubics in the stroke converter, I get almost perfect strokes on tiger! There are a few very minor artifacts remaining that I haven’t investigated yet. |
|
Good to hear. I haven't run through tiger myself, but it sounds like that's a good set of test data, and we should try to nail down all the remaining artifacts. |
This is one possible technique for improving the robustness of parallel curve generation - a method to detect cusps and "regularize" by perturbing the control points. The implementation handles cases where the control points are coincident with the endpoints, and also simple cusps, but hasn't been explicitly tuned to handle cases where the control points are nearly co-linear. It is likely that additional tuning of the "dimension" parameter would be worthwhile. As a first approximation, it can just be the accuracy parameter.
dfrg
approved these changes
Jun 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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