impl Contains for all geometries #880
Conversation
| // (Geometry::Triangle(subject), Geometry::Triangle(target)) => { subject.contains(target) == relate_actual } | ||
| _ => true, | ||
| }; | ||
| let direct_actual = subject.contains(target); |
There was a problem hiding this comment.
This might look like I'm deleting test cases, but actually it's the opposite - we now verify that Contains matches Relate::is_contains for all test cases, rather than having to specify which ones.
michaelkirk
force-pushed
the
mkirk/contains-all
branch
from
95052c8 to
4996d28
Compare
|
I found some correctness issues with this while adding more of the JTS test suite, so I'm going to close for now. (I'm actively working on it, so hopefully it won't be long before I re-open) |
michaelkirk
force-pushed
the
mkirk/contains-all
branch
from
4996d28 to
35055a9
Compare
Ok, I fixed these in #882, so once 882 is approved, this is ready for review. |
michaelkirk
force-pushed
the
mkirk/contains-all
branch
from
35055a9 to
08f1cc5
Compare
| @@ -10,6 +10,10 @@ pub use area::Area; | |||
| pub mod bearing; | |||
| pub use bearing::Bearing; | |||
|
|
|||
| /// Boolean Ops such as union, xor, difference; | |||
There was a problem hiding this comment.
Trying to keep this file alphabetically organized.
|
#882 has been merged! So this is ready for review. |
|
bors r=rmanoka |
|
Build succeeded: |
884: Adds Within trait and fixes more Contains edge cases r=frewsxcv a=michaelkirk - [x] I agree to follow the project's [code of conduct](https://github.com/georust/geo/blob/main/CODE_OF_CONDUCT.md). - [x] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users. --- ~~Based on #880, so review that first.~~ Merged! This is ready for review! The new `Within` trait is implemented in terms of `Contains`. Running the "within" tests from the JTS suite exposed some further inconsistencies with how we're handling line string edge cases. Co-authored-by: Michael Kirk <michael.code@endoftheworl.de>
CHANGES.mdif knowledge of this change could be valuable to users.Based on the test suite and correctness fixes in #882 so please review that first.
Fixes #535