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

Fix edge case for mindist #334

Merged
merged 1 commit into from
Jun 28, 2024
Merged

Conversation

platlas
Copy link
Collaborator

@platlas platlas commented Feb 12, 2024

This is fix for #218.

Depends on orientation of curves, function fails if curves have different order. Check, detecting end-points as closest points, returns false positives here. This is caused by small typo in implementation where check takes order of first curve instead of order from second curve.

PR consist of test case from #218 and replacement of n order to m in that specific check.

@platlas platlas linked an issue Feb 12, 2024 that may be closed by this pull request
Copy link
Contributor

@raphlinus raphlinus left a comment

Choose a reason for hiding this comment

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

I didn't analyze the logic deeply, but I agree this is a typo, and the test case seems to cover it. Thanks!

(I don't remember if you have commit access, but if not, let me know. We have a policy of author-commits)

@platlas platlas merged commit c9843e8 into linebender:main Jun 28, 2024
14 checks passed
@platlas platlas deleted the mindist_edge_case branch June 28, 2024 21:51
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.

mindist doesn't actually work for lines
2 participants