Merge #516

516: Round up Intersects implementations r=rmanoka a=rmanoka

This PR completes the `Intersects` implementation for all pairs of current geo-types (discounting possible bugs)!

+ Add OGC-SA descriptions to geo-types docs
+ Verify existing implementations
+ Add implementations for containers: `Geometry`, `GeometryCollection`

See [here](//georust.github.io/geo/support/intersects.html) for table describing the implementation status / map.

Interestingly, we didn't need geom-graph for this trait (presumably because this trait doesn't distinguish between interior and boundary).  I also removed any usage of `Contains` trait in this trait for easier verification of correctness.

Co-authored-by: Rajsekar Manokaran <[email protected]>
Co-authored-by: rmanoka <[email protected]>

geo-types/src/coordinate.rs

@@ -14,6 +14,13 @@ use approx::{AbsDiffEq, RelativeEq, UlpsEq};
 /// [`Add`], [`Sub`], [`Neg`], [`Zero`],
 /// [`Mul<T>`][`Mul`], and [`Div<T>`][`Div`] traits.
 ///
+/// # Semantics
+///
+/// This type does not represent any geospatial primitive,
+/// but is used in their definitions. The only requirement
+/// is that the coordinates it contains are valid numbers
+/// (for eg. not `f64::NAN`).
+///
 /// [vector space]: //en.wikipedia.org/wiki/Vector_space
 #[derive(Eq, PartialEq, Clone, Copy, Debug, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]

geo-types/src/geometry_collection.rs

@@ -6,10 +6,13 @@ use std::ops::{Index, IndexMut};
 ///
 /// It can be created from a `Vec` of Geometries, or from an Iterator which yields Geometries.
 ///
-/// Looping over this object yields its component **Geometry enum members** (_not_ the underlying geometry primitives),
-/// and it supports iteration and indexing
-/// as well as the various [`MapCoords`](algorithm/map_coords/index.html) functions, which _are_ directly
-/// applied to the underlying geometry primitives.
+/// Looping over this object yields its component **Geometry
+/// enum members** (_not_ the underlying geometry
+/// primitives), and it supports iteration and indexing as
+/// well as the various
+/// [`MapCoords`](algorithm/map_coords/index.html)
+/// functions, which _are_ directly applied to the
+/// underlying geometry primitives.
 ///
 /// # Examples
 /// ## Looping

geo-types/src/lib.rs

@@ -6,6 +6,17 @@
 //! with other `GeoRust` crates. Otherwise, the [`geo`](https://crates.io/crates/geo) crate re-exports these types and
 //! provides geospatial algorithms, while the [`geojson`](https://crates.io/crates/geojson) crate allows serialising
 //! and de-serialising `geo-types` primitives to GeoJSON.
+//!
+//! # Semantics
+//!
+//! The geospatial types provided here aim to adhere to the
+//! [OpenGIS Simple feature access][OGC-SFA] standards.
+//! Thus, the types here are inter-operable with other
+//! implementations of the standards: [JTS], [geos], etc.
+//!
+//! [OGC-SFA]: //www.ogc.org/standards/sfa
+//! [JTS]: //github.com/locationtech/jts
+//! [geos]: //trac.osgeo.org/geos
 extern crate num_traits;
 use num_traits::{Num, NumCast};
 
@@ -21,8 +32,9 @@ extern crate approx;
 
 /// The type of an x or y value of a point/coordinate.
 ///
-/// Floats (`f32` and `f64`) and Integers (`u8`, `i32` etc.) implement this. Many algorithms only
-/// make sense for Float types (like area, or length calculations).
+/// Floats (`f32` and `f64`) and Integers (`u8`, `i32` etc.)
+/// implement this. Many algorithms only make sense for
+/// Float types (like area, or length calculations).
 pub trait CoordinateType: Num + Copy + NumCast + PartialOrd {}
 // Little bit of a hack to make to make this work
 impl<T: Num + Copy + NumCast + PartialOrd> CoordinateType for T {}

geo-types/src/line.rs

@@ -1,9 +1,12 @@
 use crate::{Coordinate, CoordinateType, Point};
 
 /// A line segment made up of exactly two
-/// [`Coordinate`s](struct.Coordinate.html). The interior and
-/// boundaries are defined as with a `LineString` with the
-/// two end points.
+/// [`Coordinate`s](struct.Coordinate.html).
+///
+/// # Semantics
+///
+/// The _interior_ and _boundary_ are defined as with a
+/// `LineString` with the two end points.
 #[derive(Eq, PartialEq, Clone, Copy, Debug, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub struct Line<T>

geo-types/src/line_string.rs

@@ -6,12 +6,18 @@ use std::ops::{Index, IndexMut};
 /// [`Coordinate`s](struct.Coordinate.html), representing a
 /// path between locations.
 ///
+/// # Semantics
+///
 /// A `LineString` is _closed_ if it is empty, or if the
 /// first and last coordinates are the same. The _boundary_
 /// of a `LineString` is empty if closed, and otherwise the
-/// end points. The interior is the (infinite) set of all
+/// end points. The _interior_ is the (infinite) set of all
 /// points along the linestring _not including_ the
-/// boundary.
+/// boundary. A `LineString` is _simple_ if it does not
+/// intersect except possibly at the first and last
+/// coordinates. A simple and closed `LineString` is a
+/// `LinearRing` as defined in the OGC-SFA (but is not a
+/// separate type here).
 ///
 /// # Validity
 ///

geo-types/src/multi_line_string.rs

@@ -2,15 +2,32 @@ use crate::{CoordinateType, LineString};
 use std::iter::FromIterator;
 
 /// A collection of
-/// [`LineString`s](line_string/struct.LineString.html). The
-/// interior and the boundary are the union of the interior
-/// or the boundary of the constituent line strings.
+/// [`LineString`s](line_string/struct.LineString.html). Can
+/// be created from a `Vec` of `LineString`s, or from an
+/// Iterator which yields `LineString`s. Iterating over this
+/// object yields the component `LineString`s.
 ///
-/// Can be created from a `Vec` of `LineString`s, or from an
-/// Iterator which yields `LineString`s.
+/// # Semantics
 ///
-/// Iterating over this objects, yields the component
-/// `LineString`s.
+/// The _boundary_ of a `MultiLineString` is obtained by
+/// applying the “mod 2” union rule: A `Point` is in the
+/// boundary of a `MultiLineString` if it is in the
+/// boundaries of an odd number of elements of the
+/// `MultiLineString`.
+///
+/// The _interior_ of a `MultiLineString` is the union of
+/// the interior, and boundary of the constituent
+/// `LineString`s, _except_ for the boundary as defined
+/// above. In other words, it is the set difference of the
+/// boundary from the union of the interior and boundary of
+/// the constituents.
+///
+/// A `MultiLineString` is _simple_ if and only if all of
+/// its elements are simple and the only intersections
+/// between any two elements occur at `Point`s that are on
+/// the boundaries of both elements. A `MultiLineString` is
+/// _closed_ if all of its elements are closed. The boundary
+/// of a closed `MultiLineString` is always empty.
 #[derive(Eq, PartialEq, Clone, Debug, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub struct MultiLineString<T>(pub Vec<LineString<T>>)

geo-types/src/multi_point.rs

@@ -1,9 +1,17 @@
 use crate::{CoordinateType, Point};
 use std::iter::FromIterator;
 
-/// A collection of [`Point`s](struct.Point.html). The
-/// interior and the boundary are the union of the interior
-/// or the boundary of the constituent points.
+/// A collection of [`Point`s](struct.Point.html). Can
+/// be created from a `Vec` of `Point`s, or from an
+/// Iterator which yields `Point`s. Iterating over this
+/// object yields the component `Point`s.
+///
+/// # Semantics
+///
+/// The _interior_ and the _boundary_ are the union of the
+/// interior and the boundary of the constituent points. In
+/// particular, the boundary of a `MultiPoint` is always
+/// empty.
 ///
 /// # Examples
 ///

geo-types/src/multi_polygon.rs

@@ -1,13 +1,27 @@
 use crate::{CoordinateType, Polygon};
 use std::iter::FromIterator;
 
-/// A collection of [`Polygon`s](struct.Polygon.html). The
-/// interior and the boundary are the union of the interior
-/// or the boundary of the constituent polygons.
+/// A collection of [`Polygon`s](struct.Polygon.html). Can
+/// be created from a `Vec` of `Polygon`s, or from an
+/// Iterator which yields `Polygon`s. Iterating over this
+/// object yields the component `Polygon`s.
 ///
-/// Can be created from a `Vec` of `Polygon`s, or `collect`ed from an Iterator which yields `Polygon`s.
+/// # Semantics
 ///
-/// Iterating over this object yields the component Polygons.
+/// The _interior_ and the _boundary_ are the union of the
+/// interior and the boundary of the constituent polygons.
+///
+/// # Validity
+///
+/// - The interiors of no two constituent polygons may intersect.
+///
+/// - The boundaries of two (distinct) constituent polygons
+/// may only intersect at finitely many points.
+///
+/// Refer to section 6.1.14 of the OGC-SFA for a formal
+/// definition of validity. Note that the validity is not
+/// enforced, but expected by the operations and
+/// predicates that operate on it.
 #[derive(Eq, PartialEq, Clone, Debug, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub struct MultiPolygon<T>(pub Vec<Polygon<T>>)

geo-types/src/point.rs

@@ -9,9 +9,11 @@ use std::ops::{Add, Div, Mul, Neg, Sub};
 /// tuples, or arrays – see the `From` impl section for a
 /// complete list.
 ///
-/// A point is _valid_ iff neither coordinate is `NaN`. The
-/// _interior_ of the point is itself (a singleton set), and
-/// its _boundary_ is empty.
+/// # Semantics
+///
+/// The _interior_ of the point is itself (a singleton set),
+/// and its _boundary_ is empty. A point is _valid_ if and
+/// only if the `Coordinate` is valid.
 ///
 /// # Examples
 ///

geo-types/src/polygon.rs

@@ -7,6 +7,8 @@ use num_traits::{Float, Signed};
 /// [`LineString`]. It may contain zero or more holes (_interior rings_), also
 /// represented by `LineString`s.
 ///
+/// # Semantics
+///
 /// The _boundary_ of the polygon is the union of the
 /// boundaries of the exterior and interiors. The interior
 /// is all the points inside the polygon (not on the
@@ -18,8 +20,29 @@ use num_traits::{Float, Signed};
 ///
 /// # Validity
 ///
-/// Besides the closed `LineString` rings guarantee, the `Polygon` structure
-/// does not enforce validity at this time. For example, it is possible to
+/// - The exterior and interior rings must be valid
+/// `LinearRing`s (see [`LineString`]).
+///
+/// - No two rings in the boundary may cross, and may
+/// intersect at a `Point` only as a tangent. In other
+/// words, the rings must be distinct, and for every pair of
+/// common points in two of the rings, there must be a
+/// neighborhood (a topological open set) around one that
+/// does not contain the other point.
+///
+/// - The closure of the interior of the `Polygon` must
+/// equal the `Polygon` itself. For instance, the exterior
+/// may not contain a spike.
+///
+/// - The interior of the polygon must be a connected
+/// point-set. That is, any two distinct points in the
+/// interior must admit a curve between these two that lies
+/// in the interior.
+///
+/// Refer to section 6.1.11.1 of the OGC-SFA for a formal
+/// definition of validity. Besides the closed `LineString`
+/// guarantee, the `Polygon` structure does not enforce
+/// validity at this time. For example, it is possible to
 /// construct a `Polygon` that has:
 ///
 /// - fewer than 3 coordinates per `LineString` ring

geo-types/src/triangle.rs

@@ -1,6 +1,9 @@
 use crate::{polygon, Coordinate, CoordinateType, Line, Polygon};
 
-/// A bounded 2D area whose three vertices are defined by `Coordinate`s.
+/// A bounded 2D area whose three vertices are defined by
+/// `Coordinate`s. The semantics and validity are that of
+/// the equivalent [`Polygon`]; in addition, the three
+/// vertices must not be collinear and they must be distinct.
 #[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub struct Triangle<T: CoordinateType>(pub Coordinate<T>, pub Coordinate<T>, pub Coordinate<T>);

geo/src/algorithm/intersects/collections.rs

@@ -0,0 +1,49 @@
+use super::Intersects;
+use crate::*;
+
+impl<T, G> Intersects<G> for Geometry<T>
+where
+    T: CoordinateType,
+    Point<T>: Intersects<G>,
+    MultiPoint<T>: Intersects<G>,
+    Line<T>: Intersects<G>,
+    LineString<T>: Intersects<G>,
+    MultiLineString<T>: Intersects<G>,
+    Triangle<T>: Intersects<G>,
+    Rect<T>: Intersects<G>,
+    Polygon<T>: Intersects<G>,
+    MultiPolygon<T>: Intersects<G>,
+{
+    fn intersects(&self, rhs: &G) -> bool {
+        match self {
+            Geometry::Point(geom) => geom.intersects(rhs),
+            Geometry::MultiPoint(geom) => geom.intersects(rhs),
+            Geometry::Line(geom) => geom.intersects(rhs),
+            Geometry::LineString(geom) => geom.intersects(rhs),
+            Geometry::MultiLineString(geom) => geom.intersects(rhs),
+            Geometry::Triangle(geom) => geom.intersects(rhs),
+            Geometry::Rect(geom) => geom.intersects(rhs),
+            Geometry::Polygon(geom) => geom.intersects(rhs),
+            Geometry::MultiPolygon(geom) => geom.intersects(rhs),
+            Geometry::GeometryCollection(geom) => geom.intersects(rhs),
+        }
+    }
+}
+symmetric_intersects_impl!(Coordinate<T>, Geometry<T>);
+symmetric_intersects_impl!(Line<T>, Geometry<T>);
+symmetric_intersects_impl!(Rect<T>, Geometry<T>);
+symmetric_intersects_impl!(Polygon<T>, Geometry<T>);
+
+impl<T, G> Intersects<G> for GeometryCollection<T>
+where
+    T: CoordinateType,
+    Geometry<T>: Intersects<G>,
+{
+    fn intersects(&self, rhs: &G) -> bool {
+        self.0.iter().any(|geom| geom.intersects(rhs))
+    }
+}
+symmetric_intersects_impl!(Coordinate<T>, GeometryCollection<T>);
+symmetric_intersects_impl!(Line<T>, GeometryCollection<T>);
+symmetric_intersects_impl!(Rect<T>, GeometryCollection<T>);
+symmetric_intersects_impl!(Polygon<T>, GeometryCollection<T>);

geo/src/algorithm/intersects/coordinate.rs

@@ -9,3 +9,13 @@ where
         self == rhs
     }
 }
+
+// The other side of this is handled via a blanket impl.
+impl<T> Intersects<Point<T>> for Coordinate<T>
+where
+    T: CoordinateType,
+{
+    fn intersects(&self, rhs: &Point<T>) -> bool {
+        self == &rhs.0
+    }
+}

geo/src/algorithm/intersects/line.rs

@@ -7,43 +7,58 @@ where
     T: HasKernel,
 {
     fn intersects(&self, rhs: &Coordinate<T>) -> bool {
+        // First we check if the point is collinear with the line.
         T::Ker::orient2d(self.start, self.end, *rhs) == Orientation::Collinear
+        // In addition, the point must have _both_ coordinates
+        // within the start and end bounds.
             && point_in_rect(*rhs, self.start, self.end)
     }
 }
-symmetric_intersects_impl!(Coordinate<T>, Line<T>, HasKernel);
-
-impl<T> Intersects<Point<T>> for Line<T>
-where
-    T: HasKernel,
-{
-    fn intersects(&self, p: &Point<T>) -> bool {
-        self.intersects(&p.0)
-    }
-}
-symmetric_intersects_impl!(Point<T>, Line<T>, HasKernel);
+symmetric_intersects_impl!(Coordinate<T>, Line<T>);
+symmetric_intersects_impl!(Line<T>, Point<T>);
 
 impl<T> Intersects<Line<T>> for Line<T>
 where
     T: HasKernel,
 {
     fn intersects(&self, line: &Line<T>) -> bool {
+        // Special case: self is equiv. to a point.
         if self.start == self.end {
             return line.intersects(&self.start);
         }
+
+        // Precondition: start and end are distinct.
+
+        // Check if orientation of rhs.{start,end} are different
+        // with respect to self.{start,end}.
         let check_1_1 = T::Ker::orient2d(self.start, self.end, line.start);
         let check_1_2 = T::Ker::orient2d(self.start, self.end, line.end);
 
         if check_1_1 != check_1_2 {
+            // Since the checks are different,
+            // rhs.{start,end} are distinct, and rhs is not
+            // collinear with self. Thus, there is exactly
+            // one point on the infinite extensions of rhs,
+            // that is collinear with self.
+
+            // By continuity, this point is not on the
+            // exterior of rhs. Now, check the same with
+            // self, rhs swapped.
+
             let check_2_1 = T::Ker::orient2d(line.start, line.end, self.start);
             let check_2_2 = T::Ker::orient2d(line.start, line.end, self.end);
 
+            // By similar argument, there is (exactly) one
+            // point on self, collinear with rhs. Thus,
+            // those two have to be same, and lies (interior
+            // or boundary, but not exterior) on both lines.
             check_2_1 != check_2_2
+
         } else if check_1_1 == Orientation::Collinear {
             // Special case: collinear line segments.
 
-            // Equivalent to the point-line intersection
-            // impl., but removes the calls to the kernel
+            // Equivalent to 4 point-line intersection
+            // checks, but removes the calls to the kernel
             // predicates.
             point_in_rect(line.start, self.start, self.end)
                 || point_in_rect(line.end, self.start, self.end)