-
-
Notifications
You must be signed in to change notification settings - Fork 76
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
Existing test case with wrong result #114
Comments
Hi @bluenote10 Feel free to put the PR in even if it breaks a test case, we might be able to help debug what's going on. Also worth noting that in my #113 PR I fixed a few test cases which were incorrect so it maybe that your PR isn't actually breaking anything. Anyway put in the PR and then someone can review. Cheers |
I think this problem is caused by an inconsistency of
I think the solution to this issue is to make the two consistent: The intersection check must return a non-endpoint if and only if the To verify the hypothesis I have moved the vertical segment a little bit to the left here so that segment comparison and intersection check are consistent, and then it works fine: In conclusion, this is different to #110 which deals with the case where the endpoint falls exactly on the other segment (i.e., |
Wow good digging @bluenote10 - looking forward to checking out the PR! |
@w8r In case you are interested, I have PR for this in Rust here: 21re/rust-geo-booleanop#13 |
More very thorough work thanks @bluenote10 - I hope you are finding computational geometry an enjoyable rabbit hole because you are very good at it! |
@rowanwins Haha thanks! Yes, indeed quite a rabbit hole, but an enjoyable one. No talent really, just a fondness for tedious debugging ;). |
While preparing a fix for #110 I noticed that there is currently a test case, which tests for a wrong intersection result (input left, output right):
Also notice that the result polygon ring isn't closed. There is a similar problem when computing the union:
My solution for #110 works in all other tests but "breaks" this case, but it also doesn't fix it. I'll try and see if I can fix both. But maybe this is because the case is also affected by output edge connecting logic, and requires @rowanwins's fixes in #113.
The text was updated successfully, but these errors were encountered: