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

Adds Within trait and fixes more Contains edge cases #884

Merged
merged 2 commits into from
Aug 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions geo/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions geo/src/algorithm/contains/line_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's an example of the kind of test that was failing before this change:

<case>
<desc>P/nsL.1-5-1: a point on a non-simple LineString's start point with crossing line segments [dim(0){A.P.Int = B.nsL.Bdy.SPx}]</desc>
  <a>
    POINT(110 110)
  </a>
  <b>
    LINESTRING(110 110, 220 20, 20 20, 110 110, 220 220)
  </b>
  <test>
    <op name="relate" arg1="A" arg2="B" arg3="F0FFFF102">true</op>
  </test>
  <test><op name="contains"   arg1="A" arg2="B">false</op></test>
  <test><op name="coveredBy"  arg1="A" arg2="B">true</op></test>
  <test><op name="covers"     arg1="A" arg2="B">false</op></test>
  <test><op name="crosses"    arg1="A" arg2="B">false</op></test>
  <test><op name="disjoint"   arg1="A" arg2="B">false</op></test>
  <test><op name="equalsTopo" arg1="A" arg2="B">false</op></test>
  <test><op name="intersects" arg1="A" arg2="B">true</op></test>
  <test><op name="overlaps"   arg1="A" arg2="B">false</op></test>
  <test><op name="touches"    arg1="A" arg2="B">true</op></test>
  <test><op name="within"     arg1="A" arg2="B">false</op></test>
</case>


self.lines()
Expand Down
4 changes: 4 additions & 0 deletions geo/src/algorithm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
60 changes: 60 additions & 0 deletions geo/src/algorithm/within.rs
Original file line number Diff line number Diff line change
@@ -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));
}
}
1 change: 1 addition & 0 deletions geo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
//!
Expand Down
36 changes: 29 additions & 7 deletions jts-test-runner/src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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),

Expand All @@ -131,9 +143,6 @@ pub(crate) enum OperationInput {
#[serde(rename = "relate")]
RelateInput(RelateInput),

#[serde(rename = "contains")]
ContainsInput(ContainsInput),

#[serde(rename = "union")]
UnionInput(OverlayInput),

Expand All @@ -146,6 +155,9 @@ pub(crate) enum OperationInput {
#[serde(rename = "symdifference")]
SymDifferenceInput(OverlayInput),

#[serde(rename = "within")]
WithinInput(WithinInput),

#[serde(other)]
Unsupported,
}
Expand All @@ -161,6 +173,11 @@ pub(crate) enum Operation {
target: Geometry,
expected: bool,
},
Within {
subject: Geometry,
target: Geometry,
expected: bool,
},
ConvexHull {
subject: Geometry,
expected: Geometry,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion jts-test-runner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
36 changes: 35 additions & 1 deletion jts-test-runner/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down