Fix ray-tracing algorithm when ray passes through a vertex #526
Conversation
amatissart
changed the title
|
@amatissart Thanks for pointing out the bug; nice catch! However, I think the fix is not as simple as ignoring both endpoints. For instance, consider a simple diagonal square (1, 0) - (0, 1) - (-1, 0) - (0, -1) and we ray-trace from the origin (0, 0). In this case, the point is inside, but ignoring both endpoints will compute the predicate wrong. |
|
Crucially, the change ignores the line only if one endpoint is below the ray while the other lies on it. I will add a test case to make that more explicit. |
There was a problem hiding this comment.
@amatissart Nice, I'd missed the point about not counting segments that are not above the ray. That is indeed a very neat and nifty argument. I've suggested a few more lines of comments explaining the new logic for future readers, but otherwise the PR looks great! Thanks!
| // prevent a double counting when the ray passes | ||
| // through a vertex of the plygon | ||
| if line.start.y == coord.y { | ||
| // through a vertex of the polygon. |
There was a problem hiding this comment.
| // through a vertex of the polygon. | |
| // through a vertex of the polygon. | |
| // | |
| // The below logic handles two cases: | |
| // 1. if the ray enters/exits the polygon | |
| // at the point of intersection | |
| // 1. if the ray touches a vertex, | |
| // but doesn't enter/exit at that point |
There was a problem hiding this comment.
Thanks! I added your comment.
|
@michaelkirk May be related to the geom. graph ideas you've been looking into; any thoughts? I'll wait a couple of days to see if you or anyone else have any comments, and merge otherwise. |
|
Do you think this fix could land in a patch release of the I don't want to sound impatient 🙂. I understand that there is a lot of work going on to tackle #515. Just wondering if you have a release schedule in mind. |
|
We should definitely have a release before #515 is done. Personally, I'm hoping we can get #541 merged and released in the next couple days. If it's more urgent than that, keep in mind you can target the master branch by adding something like this to your Cargo.toml: |
|
This was just released in the 0.16.0 geo crate. |
The ray casting algorithm introduced in #511 that implements
coord_pos_relative_to_ringdoes not handle correctly the case when the ray passes through a vertex.In the following situation, the number of crossings to be counted should 1 or 3, definitely not 2:
To prevent a double counting in the general case when the ray intersects the line at one end, this PR suggests to ignore the line only if it lies below the ray.