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

Improve cusp detection for strokes #318

Merged
merged 1 commit into from
Oct 19, 2023
Merged

Improve cusp detection for strokes #318

merged 1 commit into from
Oct 19, 2023

Conversation

raphlinus
Copy link
Contributor

The cusp detection was failing because a comparison wasn't inclusive enough. In particular, it was trying to compute whether curvature at the point exceeded 1/dimension, without doing division, but both values in the comparison become zero when the derivative vanishes.

Also avoids infinite recursion when break_cusp reports a cusp within ulp of an endpoint. This is supposed to be a violation of the contract, but it's not hard to see how it can happen.

Includes test cases adapted from vello#388

The cusp detection was failing because a comparison wasn't inclusive enough. In particular, it was trying to compute whether curvature at the point exceeded 1/dimension, without doing division, but both values in the comparison become zero when the derivative vanishes.

Also avoids infinite recursion when break_cusp reports a cusp within ulp of an endpoint. This is supposed to be a violation of the contract, but it's not hard to see how it can happen.

Includes test cases adapted from vello#388
Copy link

@armansito armansito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, and I confirmed that the tests in vello#388 render correctly with this change.

@raphlinus raphlinus merged commit 3a40570 into main Oct 19, 2023
14 checks passed
@raphlinus raphlinus deleted the yet_more_robustness branch October 24, 2023 14:44
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

Successfully merging this pull request may close these issues.

2 participants