Merge #884

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 <[email protected]>

geo/CHANGES.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+* Add `Within` trait to determine if Geometry A is completely within Geometry B
+  * <https://github.com/georust/geo/pull/884>
 * Add `Contains` impl for all remaining geometry types.
   * <https://github.com/georust/geo/pull/880>
 * Add `Scale` affine transform

geo/src/algorithm/contains/line_string.rs

@@ -16,8 +16,8 @@ where
             return false;
         }
 
-        if self.is_closed() && coord == &self.0[0] {
-            return true;
+        if coord == &self.0[0] || coord == self.0.last().unwrap() {
+            return self.is_closed();
         }
 
         self.lines()

geo/src/algorithm/mod.rs

@@ -212,5 +212,9 @@ pub use vincenty_length::VincentyLength;
 pub mod winding_order;
 pub use winding_order::Winding;
 
+/// Determine whether `Geometry` `A` is completely within by `Geometry` `B`.
+pub mod within;
+pub use within::Within;
+
 /// Planar sweep algorithm and related utils
 pub mod sweep;

geo/src/algorithm/within.rs

@@ -0,0 +1,60 @@
+use crate::algorithm::Contains;
+
+/// Tests if a geometry is completely within another geometry.
+///
+/// In other words, the [DE-9IM] intersection matrix for (Self, Rhs) is `[T*F**F***]`
+///
+/// # Examples
+///
+/// ```
+/// use geo::{point, line_string};
+/// use geo::algorithm::Within;
+///
+/// let line_string = line_string![(x: 0.0, y: 0.0), (x: 2.0, y: 4.0)];
+///
+/// assert!(point!(x: 1.0, y: 2.0).is_within(&line_string));
+///
+/// // Note that a geometry on only the *boundary* of another geometry is not considered to
+/// // be _within_ that geometry. See [`Relate`] for more information.
+/// assert!(! point!(x: 0.0, y: 0.0).is_within(&line_string));
+/// ```
+///
+/// `Within` is equivalent to [`Contains`] with the arguments swapped.
+///
+/// ```
+/// use geo::{point, line_string};
+/// use geo::algorithm::{Contains, Within};
+///
+/// let line_string = line_string![(x: 0.0, y: 0.0), (x: 2.0, y: 4.0)];
+/// let point = point!(x: 1.0, y: 2.0);
+///
+/// // These two comparisons are completely equivalent
+/// assert!(point.is_within(&line_string));
+/// assert!(line_string.contains(&point));
+/// ```
+///
+/// [DE-9IM]: https://en.wikipedia.org/wiki/DE-9IM
+pub trait Within<Other> {
+    fn is_within(&self, b: &Other) -> bool;
+}
+
+impl<G1, G2> Within<G2> for G1
+where
+    G2: Contains<G1>,
+{
+    fn is_within(&self, b: &G2) -> bool {
+        b.contains(self)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::{point, Rect};
+    #[test]
+    fn basic() {
+        let a = point!(x: 1.0, y: 2.0);
+        let b = Rect::new((0.0, 0.0), (3.0, 3.0)).to_polygon();
+        assert!(a.is_within(&b));
+    }
+}

geo/src/lib.rs

@@ -92,6 +92,7 @@
 //!   intersection, if any, between two lines.
 //! - **[`Relate`](Relate)**: Topologically relate two geometries based on
 //!   [DE-9IM](https://en.wikipedia.org/wiki/DE-9IM) semantics.
+//! - **[`Within`]**: Calculate if a geometry lies completely within another geometry.
 //!
 //! ## Winding
 //!

jts-test-runner/src/input.rs

@@ -107,6 +107,15 @@ pub struct ContainsInput {
     pub(crate) expected: bool,
 }
 
+#[derive(Debug, Deserialize)]
+pub struct WithinInput {
+    pub(crate) arg1: String,
+    pub(crate) arg2: String,
+
+    #[serde(rename = "$value", deserialize_with = "deserialize_from_str")]
+    pub(crate) expected: bool,
+}
+
 #[derive(Debug, Deserialize)]
 pub struct OverlayInput {
     pub(crate) arg1: String,
@@ -119,6 +128,9 @@ pub struct OverlayInput {
 #[derive(Debug, Deserialize)]
 #[serde(tag = "name")]
 pub(crate) enum OperationInput {
+    #[serde(rename = "contains")]
+    ContainsInput(ContainsInput),
+
     #[serde(rename = "getCentroid")]
     CentroidInput(CentroidInput),
 
@@ -131,9 +143,6 @@ pub(crate) enum OperationInput {
     #[serde(rename = "relate")]
     RelateInput(RelateInput),
 
-    #[serde(rename = "contains")]
-    ContainsInput(ContainsInput),
-
     #[serde(rename = "union")]
     UnionInput(OverlayInput),
 
@@ -146,6 +155,9 @@ pub(crate) enum OperationInput {
     #[serde(rename = "symdifference")]
     SymDifferenceInput(OverlayInput),
 
+    #[serde(rename = "within")]
+    WithinInput(WithinInput),
+
     #[serde(other)]
     Unsupported,
 }
@@ -161,6 +173,11 @@ pub(crate) enum Operation {
         target: Geometry,
         expected: bool,
     },
+    Within {
+        subject: Geometry,
+        target: Geometry,
+        expected: bool,
+    },
     ConvexHull {
         subject: Geometry,
         expected: Geometry,
@@ -230,16 +247,21 @@ impl OperationInput {
             Self::ContainsInput(input) => {
                 assert_eq!("A", input.arg1);
                 assert_eq!("B", input.arg2);
-                assert!(
-                    case.b.is_some(),
-                    "intersects test case must contain geometry b"
-                );
                 Ok(Operation::Contains {
                     subject: geometry.clone(),
                     target: case.b.clone().expect("no geometry b in case"),
                     expected: input.expected,
                 })
             }
+            Self::WithinInput(input) => {
+                assert_eq!("A", input.arg1);
+                assert_eq!("B", input.arg2);
+                Ok(Operation::Within {
+                    subject: geometry.clone(),
+                    target: case.b.clone().expect("no geometry b in case"),
+                    expected: input.expected,
+                })
+            }
             Self::UnionInput(input) => {
                 validate_boolean_op(
                     &input.arg1,

jts-test-runner/src/lib.rs

@@ -74,7 +74,7 @@ mod tests {
         //
         // We'll need to increase this number as more tests are added, but it should never be
         // decreased.
-        let expected_test_count: usize = 1728;
+        let expected_test_count: usize = 2213;
         let actual_test_count = runner.failures().len() + runner.successes().len();
         match actual_test_count.cmp(&expected_test_count) {
             Ordering::Less => {

jts-test-runner/src/runner.rs

@@ -6,7 +6,7 @@ use log::{debug, info};
 use wkt::ToWkt;
 
 use super::{input, Operation, Result};
-use geo::algorithm::{BooleanOps, Contains, HasDimensions, Intersects};
+use geo::algorithm::{BooleanOps, Contains, HasDimensions, Intersects, Within};
 use geo::geometry::*;
 use geo::GeoNum;
 
@@ -150,6 +150,40 @@ impl TestRunner {
                         self.successes.push(test_case);
                     }
                 }
+                Operation::Within {
+                    subject,
+                    target,
+                    expected,
+                } => {
+                    use geo::Relate;
+                    let relate_within_result = subject.relate(target).is_within();
+                    let within_trait_result = subject.is_within(target);
+
+                    if relate_within_result != *expected {
+                        debug!("Within failure: Relate doesn't match expected");
+                        let error_description = format!(
+                            "Within failure: expected {:?}, relate: {:?}",
+                            expected, relate_within_result
+                        );
+                        self.failures.push(TestFailure {
+                            test_case,
+                            error_description,
+                        });
+                    } else if relate_within_result != within_trait_result {
+                        debug!("Within failure: Relate doesn't match Within trait implementation");
+                        let error_description = format!(
+                            "Within failure: Relate: {:?}, Within trait: {:?}",
+                            expected, within_trait_result
+                        );
+                        self.failures.push(TestFailure {
+                            test_case,
+                            error_description,
+                        });
+                    } else {
+                        debug!("Within success: actual == expected");
+                        self.successes.push(test_case);
+                    }
+                }
                 Operation::ConvexHull { subject, expected } => {
                     use geo::prelude::ConvexHull;