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 geometry bug in is_straight_line_drawing #412

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

jeremy-murphy
Copy link
Contributor

@jeremy-murphy jeremy-murphy commented Dec 27, 2024

Fix the geometry bug uncovered and documented by @Hermann-SW by replacing the custom function with one from Boost.Geometry.
crosses meets the requirement described in the old code for returning true for 'intersections' that don't include endpoints or self-intersection.
I decided to remove the tolerance altogether on the assumption that it was indicative of a poor implementation.

Hermann-SW and others added 4 commits October 12, 2024 19:35
"crosses" fits the requirement as it means that the interior parts of the
geometry share some common points, not exterior parts such as
end-points, and it means "not within".
@pdimov
Copy link
Member

pdimov commented Dec 30, 2024

Why is this assigned to me?

@jeremy-murphy
Copy link
Contributor Author

Why is this assigned to me?

Sorry I meant to leave a comment that I just wanted you to give it a sanity check. I've never used the numeric_cast before, but I presume it's as simple as this.

@jeremy-murphy
Copy link
Contributor Author

I just don't really know who else to assign my own pull requests to, since I assume JZ Maddock wants to minimise his Boost workload these days.

@jeremy-murphy jeremy-murphy force-pushed the 388_is_straight_line_drawing branch from 689eb04 to 9176040 Compare January 1, 2025 22:36
@jeremy-murphy
Copy link
Contributor Author

Whoops, accidentally pushed my CMake changes here and force-pushed to get rid of them.

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.

3 participants