From 5413ddc6909ec40db38ba8a8092f07215aae4d40 Mon Sep 17 00:00:00 2001 From: Rajsekar Manokaran Date: Sat, 3 Oct 2020 23:25:56 +0530 Subject: [PATCH 1/5] Confirm to OGC-SFA standards + semantics of geo-types + validity of geo-types --- geo-types/src/coordinate.rs | 7 +++++++ geo-types/src/geometry_collection.rs | 11 ++++++---- geo-types/src/lib.rs | 16 ++++++++++++-- geo-types/src/line.rs | 9 +++++--- geo-types/src/line_string.rs | 10 +++++++-- geo-types/src/multi_line_string.rs | 31 +++++++++++++++++++++------- geo-types/src/multi_point.rs | 14 ++++++++++--- geo-types/src/multi_polygon.rs | 24 ++++++++++++++++----- geo-types/src/point.rs | 8 ++++--- geo-types/src/polygon.rs | 27 ++++++++++++++++++++++-- geo-types/src/triangle.rs | 5 ++++- 11 files changed, 130 insertions(+), 32 deletions(-) diff --git a/geo-types/src/coordinate.rs b/geo-types/src/coordinate.rs index cf7073f12..fdab7ea5d 100644 --- a/geo-types/src/coordinate.rs +++ b/geo-types/src/coordinate.rs @@ -14,6 +14,13 @@ use approx::{AbsDiffEq, RelativeEq, UlpsEq}; /// [`Add`], [`Sub`], [`Neg`], [`Zero`], /// [`Mul`][`Mul`], and [`Div`][`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))] diff --git a/geo-types/src/geometry_collection.rs b/geo-types/src/geometry_collection.rs index 4613725de..f78b3042c 100644 --- a/geo-types/src/geometry_collection.rs +++ b/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 diff --git a/geo-types/src/lib.rs b/geo-types/src/lib.rs index 2ad2c7b87..e51234371 100644 --- a/geo-types/src/lib.rs +++ b/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 CoordinateType for T {} diff --git a/geo-types/src/line.rs b/geo-types/src/line.rs index 4b4d91efe..a5815db84 100644 --- a/geo-types/src/line.rs +++ b/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 diff --git a/geo-types/src/line_string.rs b/geo-types/src/line_string.rs index 6d3ee2786..746eb86de 100644 --- a/geo-types/src/line_string.rs +++ b/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 /// diff --git a/geo-types/src/multi_line_string.rs b/geo-types/src/multi_line_string.rs index fe93b4970..f9d8590d8 100644 --- a/geo-types/src/multi_line_string.rs +++ b/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 +/// objects, 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(pub Vec>) diff --git a/geo-types/src/multi_point.rs b/geo-types/src/multi_point.rs index 3db979b9b..1e2a0e279 100644 --- a/geo-types/src/multi_point.rs +++ b/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 +/// objects, 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 a `MultiPoint` is always +/// empty. /// /// # Examples /// diff --git a/geo-types/src/multi_polygon.rs b/geo-types/src/multi_polygon.rs index db816ae90..0b5502bd0 100644 --- a/geo-types/src/multi_polygon.rs +++ b/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 +/// objects, 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 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(pub Vec>) diff --git a/geo-types/src/point.rs b/geo-types/src/point.rs index 4a4890fe8..2f5010e62 100644 --- a/geo-types/src/point.rs +++ b/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 /// diff --git a/geo-types/src/polygon.rs b/geo-types/src/polygon.rs index d8da9235f..1310fb415 100644 --- a/geo-types/src/polygon.rs +++ b/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 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 diff --git a/geo-types/src/triangle.rs b/geo-types/src/triangle.rs index 2304e9c89..7d49d2b1f 100644 --- a/geo-types/src/triangle.rs +++ b/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 be collinear (and distinct). #[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub struct Triangle(pub Coordinate, pub Coordinate, pub Coordinate); From 5f877c48dbe59535885c420ceb80de0af6b38060 Mon Sep 17 00:00:00 2001 From: Rajsekar Manokaran Date: Sat, 3 Oct 2020 23:27:26 +0530 Subject: [PATCH 2/5] Clean-up Intersects + simplify symmetric impl. macro + add macro intersects with rhs=pt impl from rhs=coord + verify for coord (hence point, multipoint) + verify for line (hence linestring, multilinestring) + verify for polygon (hence multipolygon, triangle) + add remaining for rect --- geo/src/algorithm/intersects/coordinate.rs | 3 ++ geo/src/algorithm/intersects/line.rs | 41 +++++++++----- geo/src/algorithm/intersects/line_string.rs | 59 +++++++-------------- geo/src/algorithm/intersects/mod.rs | 40 ++++++++++++-- geo/src/algorithm/intersects/point.rs | 27 ++++------ geo/src/algorithm/intersects/polygon.rs | 54 +++++-------------- geo/src/algorithm/intersects/rect.rs | 39 ++++++++++++++ geo/src/algorithm/intersects/triangle.rs | 10 ++-- 8 files changed, 150 insertions(+), 123 deletions(-) diff --git a/geo/src/algorithm/intersects/coordinate.rs b/geo/src/algorithm/intersects/coordinate.rs index 037832cc6..50e2becdc 100644 --- a/geo/src/algorithm/intersects/coordinate.rs +++ b/geo/src/algorithm/intersects/coordinate.rs @@ -9,3 +9,6 @@ where self == rhs } } + +// The other side of this is handled via a blanket impl. +rhs_pt_from_coord_intersects_impl!(Coordinate); diff --git a/geo/src/algorithm/intersects/line.rs b/geo/src/algorithm/intersects/line.rs index b08a2c663..993cc0d9c 100644 --- a/geo/src/algorithm/intersects/line.rs +++ b/geo/src/algorithm/intersects/line.rs @@ -7,43 +7,58 @@ where T: HasKernel, { fn intersects(&self, rhs: &Coordinate) -> 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, Line, HasKernel); - -impl Intersects> for Line -where - T: HasKernel, -{ - fn intersects(&self, p: &Point) -> bool { - self.intersects(&p.0) - } -} -symmetric_intersects_impl!(Point, Line, HasKernel); +symmetric_intersects_impl!(Coordinate, Line); +symmetric_intersects_impl!(Line, Point); impl Intersects> for Line where T: HasKernel, { fn intersects(&self, line: &Line) -> 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) diff --git a/geo/src/algorithm/intersects/line_string.rs b/geo/src/algorithm/intersects/line_string.rs index 2cdfb2546..454c4e1f6 100644 --- a/geo/src/algorithm/intersects/line_string.rs +++ b/geo/src/algorithm/intersects/line_string.rs @@ -1,53 +1,32 @@ use super::Intersects; -use crate::kernels::*; use crate::*; -impl Intersects> for LineString +// Blanket implementation using self.lines().any(). +impl Intersects for LineString where - T: HasKernel, + T: CoordinateType, + Line: Intersects, { - fn intersects(&self, coord: &Coordinate) -> bool { - self.lines().any(|l| coord.intersects(&l)) + fn intersects(&self, geom: &G) -> bool { + // No need to loop if either self or rhs is empty. + if self.0.is_empty() { + false + } else { + self.lines().any(|l| l.intersects(geom)) + } } } -symmetric_intersects_impl!(Coordinate, LineString, HasKernel); +symmetric_intersects_impl!(Coordinate, LineString); +symmetric_intersects_impl!(Line, LineString); -impl Intersects> for LineString -where - T: HasKernel, -{ - fn intersects(&self, point: &Point) -> bool { - self.intersects(&point.0) - } -} -symmetric_intersects_impl!(Point, LineString, HasKernel); -impl Intersects> for LineString +// Blanket implementation from LineString +impl Intersects for MultiLineString where - T: HasKernel, + T: CoordinateType, + LineString: Intersects, { - fn intersects(&self, line: &Line) -> bool { - self.lines().any(|l| line.intersects(&l)) - } -} -symmetric_intersects_impl!(Line, LineString, HasKernel); - -impl Intersects> for LineString -where - T: HasKernel, -{ - fn intersects(&self, linestring: &LineString) -> bool { - if self.0.is_empty() || linestring.0.is_empty() { - return false; - } - for a in self.lines() { - for b in linestring.lines() { - if a.intersects(&b) { - return true; - } - } - } - - false + fn intersects(&self, rhs: &G) -> bool { + self.0.iter().any(|p| p.intersects(rhs)) } } diff --git a/geo/src/algorithm/intersects/mod.rs b/geo/src/algorithm/intersects/mod.rs index 7c145f03e..2e3c440d9 100644 --- a/geo/src/algorithm/intersects/mod.rs +++ b/geo/src/algorithm/intersects/mod.rs @@ -41,14 +41,21 @@ pub trait Intersects { // Since `Intersects` is symmetric, we use a macro to // implement `T: Intersects` if `S: Intersects` is -// available. As a convention, we provide explicit impl. +// available. +// +// As a convention, we typically provide explicit impl. // whenever the Rhs is a "simpler geometry" than the target -// type, and use the macro for the reverse impl. +// type, and use the macro for the reverse impl. However, +// when there is a blanket implementations (eg. Point from +// Coordinate, MultiPoint from Point), we need to provide +// the reverse (where Self is "simpler" than Rhs). #[macro_use] macro_rules! symmetric_intersects_impl { - ($t:ty, $k:ty, $bounds:tt $(+ $more_bounds:tt )*) => { + ($t:ty, $k:ty) => { impl $crate::algorithm::intersects::Intersects<$k> for $t - where T: $bounds $(+ $more_bounds)* + where + $k: $crate::algorithm::intersects::Intersects<$t>, + T: CoordinateType, { fn intersects(&self, rhs: &$k) -> bool { rhs.intersects(self) @@ -58,6 +65,31 @@ macro_rules! symmetric_intersects_impl { }; } +// Macro to delegate implementation to field `0` of rhs. +// Used for `Intersects>` from corresponding +// `Coordinate` impl. This cannot be a blanket impl. +// because it would conflict with blanket impl. of Point. +#[macro_use] +macro_rules! rhs_pt_from_coord_intersects_impl { + ($t:ty) => { + impl $crate::algorithm::intersects::Intersects> for $t + where + T: CoordinateType, + $t: $crate::algorithm::intersects::Intersects> + { + fn intersects(&self, rhs: &Point) -> bool { + self.intersects(&rhs.0) + } + + } + }; +} + +// Macro to delegate implementation to any() of an iterator. +// Used for `LineString`, `Intersects>` from corresponding +// `Coordinate` impl. This cannot be a blanket impl. +// because it would conflict with blanket impl. of Point. + mod coordinate; mod line; mod line_string; diff --git a/geo/src/algorithm/intersects/point.rs b/geo/src/algorithm/intersects/point.rs index 2d0a2f952..0b5a165b2 100644 --- a/geo/src/algorithm/intersects/point.rs +++ b/geo/src/algorithm/intersects/point.rs @@ -1,26 +1,18 @@ use super::Intersects; -use crate::kernels::*; use crate::*; -impl Intersects> for Point +// Blanket implementation from Coordinate +impl Intersects for Point where T: CoordinateType, + Coordinate: Intersects, { - fn intersects(&self, rhs: &Coordinate) -> bool { + fn intersects(&self, rhs: &G) -> bool { self.0.intersects(rhs) } } -symmetric_intersects_impl!(Coordinate, Point, CoordinateType); - -impl Intersects> for Point -where - T: CoordinateType, -{ - fn intersects(&self, rhs: &Point) -> bool { - self.intersects(&rhs.0) - } -} +// Blanket implementation from Point impl Intersects for MultiPoint where T: CoordinateType, @@ -30,8 +22,7 @@ where self.0.iter().any(|p| p.intersects(rhs)) } } -symmetric_intersects_impl!(Coordinate, MultiPoint, CoordinateType); -symmetric_intersects_impl!(Point, MultiPoint, CoordinateType); -symmetric_intersects_impl!(Line, MultiPoint, HasKernel); -symmetric_intersects_impl!(LineString, MultiPoint, HasKernel); -symmetric_intersects_impl!(Polygon, MultiPoint, HasKernel); + +symmetric_intersects_impl!(Coordinate, MultiPoint); +symmetric_intersects_impl!(Line, MultiPoint); +symmetric_intersects_impl!(Polygon, MultiPoint); diff --git a/geo/src/algorithm/intersects/polygon.rs b/geo/src/algorithm/intersects/polygon.rs index 0f082a5ca..b07613a79 100644 --- a/geo/src/algorithm/intersects/polygon.rs +++ b/geo/src/algorithm/intersects/polygon.rs @@ -1,5 +1,4 @@ use super::Intersects; -use crate::contains::Contains; use crate::kernels::*; use crate::utils::{coord_pos_relative_to_ring, CoordPos}; use crate::*; @@ -16,17 +15,8 @@ where .all(|int| coord_pos_relative_to_ring(*p, int) != CoordPos::Inside) } } -symmetric_intersects_impl!(Coordinate, Polygon, HasKernel); - -impl Intersects> for Polygon -where - T: HasKernel, -{ - fn intersects(&self, p: &Point) -> bool { - self.intersects(&p.0) - } -} -symmetric_intersects_impl!(Point, Polygon, HasKernel); +symmetric_intersects_impl!(Coordinate, Polygon); +symmetric_intersects_impl!(Polygon, Point); impl Intersects> for Polygon where @@ -35,32 +25,13 @@ where fn intersects(&self, line: &Line) -> bool { self.exterior().intersects(line) || self.interiors().iter().any(|inner| inner.intersects(line)) - || self.contains(&line.start) - || self.contains(&line.end) - } -} -symmetric_intersects_impl!(Line, Polygon, HasKernel); - -impl Intersects> for Polygon -where - T: HasKernel, -{ - fn intersects(&self, linestring: &LineString) -> bool { - // line intersects inner or outer polygon edge - if self.exterior().intersects(linestring) - || self - .interiors() - .iter() - .any(|inner| inner.intersects(linestring)) - { - true - } else { - // or if it's contained in the polygon - linestring.0.iter().any(|c| self.contains(c)) - } + || self.intersects(&line.start) + || self.intersects(&line.end) } } -symmetric_intersects_impl!(LineString, Polygon, HasKernel); +symmetric_intersects_impl!(Line, Polygon); +symmetric_intersects_impl!(Polygon, LineString); +symmetric_intersects_impl!(Polygon, MultiLineString); impl Intersects> for Polygon where @@ -70,7 +41,7 @@ where self.intersects(&rect.clone().to_polygon()) } } -symmetric_intersects_impl!(Rect, Polygon, HasKernel); +symmetric_intersects_impl!(Rect, Polygon); impl Intersects> for Polygon where @@ -97,8 +68,7 @@ where } } -symmetric_intersects_impl!(Point, MultiPolygon, HasKernel); -symmetric_intersects_impl!(Line, MultiPolygon, HasKernel); -symmetric_intersects_impl!(LineString, MultiPolygon, HasKernel); -symmetric_intersects_impl!(Polygon, MultiPolygon, HasKernel); -symmetric_intersects_impl!(Rect, MultiPolygon, HasKernel); +symmetric_intersects_impl!(Point, MultiPolygon); +symmetric_intersects_impl!(Line, MultiPolygon); +symmetric_intersects_impl!(Rect, MultiPolygon); +symmetric_intersects_impl!(Polygon, MultiPolygon); diff --git a/geo/src/algorithm/intersects/rect.rs b/geo/src/algorithm/intersects/rect.rs index 8078be842..aa8fbfe43 100644 --- a/geo/src/algorithm/intersects/rect.rs +++ b/geo/src/algorithm/intersects/rect.rs @@ -1,6 +1,24 @@ use super::{value_in_range, Intersects}; +use crate::kernels::*; use crate::*; +impl Intersects> for Rect +where + T: CoordinateType, +{ + fn intersects(&self, rhs: &Coordinate) -> bool { + // Funnily, we don't use point_in_rect, as we know + // self.min <= self.max. + let bound_1 = self.min(); + let bound_2 = self.max(); + value_in_range(rhs.x, bound_1.x, bound_2.x) + && value_in_range(rhs.y, bound_1.y, bound_2.y) + } +} +symmetric_intersects_impl!(Coordinate, Rect); +symmetric_intersects_impl!(Rect, Point); +symmetric_intersects_impl!(Rect, MultiPoint); + impl Intersects> for Rect where T: CoordinateType, @@ -15,3 +33,24 @@ where x_overlap && y_overlap } } + +// Same logic as Polygon: Intersects>, but avoid +// an allocation. +impl Intersects> for Rect +where + T: HasKernel, +{ + fn intersects(&self, rhs: &Line) -> bool { + let lt = self.min(); + let rb = self.max(); + let lb = Coordinate::from((lt.x, rb.y)); + let rt = Coordinate::from((rb.x, lt.y)); + // If either rhs.{start,end} lies inside Rect, then true + self.intersects(&rhs.start) + || self.intersects(&rhs.end) + || Line::new(lt, rt).intersects(rhs) + || Line::new(rt, rb).intersects(rhs) + || Line::new(lb, rb).intersects(rhs) + || Line::new(lt, lb).intersects(rhs) + } +} diff --git a/geo/src/algorithm/intersects/triangle.rs b/geo/src/algorithm/intersects/triangle.rs index 2f2d7fe60..8f116bab9 100644 --- a/geo/src/algorithm/intersects/triangle.rs +++ b/geo/src/algorithm/intersects/triangle.rs @@ -1,5 +1,4 @@ use super::Intersects; -use crate::kernels::*; use crate::*; impl Intersects for Triangle @@ -11,8 +10,7 @@ where self.clone().to_polygon().intersects(rhs) } } -symmetric_intersects_impl!(Coordinate, Triangle, HasKernel); -symmetric_intersects_impl!(Point, Triangle, HasKernel); -symmetric_intersects_impl!(Line, Triangle, HasKernel); -symmetric_intersects_impl!(LineString, Triangle, HasKernel); -symmetric_intersects_impl!(Polygon, Triangle, HasKernel); +symmetric_intersects_impl!(Coordinate, Triangle); +symmetric_intersects_impl!(Line, Triangle); +symmetric_intersects_impl!(Rect, Triangle); +symmetric_intersects_impl!(Polygon, Triangle); From f7b6527ea224847bbe9cac422f72990df9477a0d Mon Sep 17 00:00:00 2001 From: Rajsekar Manokaran Date: Mon, 5 Oct 2020 23:20:05 +0530 Subject: [PATCH 3/5] Complete rest of Intersects + add blanket impl for Geometry, Geom.Coll. --- geo/src/algorithm/intersects/collections.rs | 49 +++++++++++++++++++++ geo/src/algorithm/intersects/coordinate.rs | 9 +++- geo/src/algorithm/intersects/line_string.rs | 7 +-- geo/src/algorithm/intersects/mod.rs | 27 +----------- 4 files changed, 60 insertions(+), 32 deletions(-) create mode 100644 geo/src/algorithm/intersects/collections.rs diff --git a/geo/src/algorithm/intersects/collections.rs b/geo/src/algorithm/intersects/collections.rs new file mode 100644 index 000000000..73cfaade8 --- /dev/null +++ b/geo/src/algorithm/intersects/collections.rs @@ -0,0 +1,49 @@ +use super::Intersects; +use crate::*; + +impl Intersects for Geometry +where + T: CoordinateType, + Point: Intersects, + MultiPoint: Intersects, + Line: Intersects, + LineString: Intersects, + MultiLineString: Intersects, + Triangle: Intersects, + Rect: Intersects, + Polygon: Intersects, + MultiPolygon: Intersects, +{ + 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, Geometry); +symmetric_intersects_impl!(Line, Geometry); +symmetric_intersects_impl!(Rect, Geometry); +symmetric_intersects_impl!(Polygon, Geometry); + +impl Intersects for GeometryCollection +where + T: CoordinateType, + Geometry: Intersects, +{ + fn intersects(&self, rhs: &G) -> bool { + self.0.iter().any(|geom| geom.intersects(rhs)) + } +} +symmetric_intersects_impl!(Coordinate, GeometryCollection); +symmetric_intersects_impl!(Line, GeometryCollection); +symmetric_intersects_impl!(Rect, GeometryCollection); +symmetric_intersects_impl!(Polygon, GeometryCollection); diff --git a/geo/src/algorithm/intersects/coordinate.rs b/geo/src/algorithm/intersects/coordinate.rs index 50e2becdc..c25798b58 100644 --- a/geo/src/algorithm/intersects/coordinate.rs +++ b/geo/src/algorithm/intersects/coordinate.rs @@ -11,4 +11,11 @@ where } // The other side of this is handled via a blanket impl. -rhs_pt_from_coord_intersects_impl!(Coordinate); +impl Intersects> for Coordinate +where + T: CoordinateType, +{ + fn intersects(&self, rhs: &Point) -> bool { + self == &rhs.0 + } +} diff --git a/geo/src/algorithm/intersects/line_string.rs b/geo/src/algorithm/intersects/line_string.rs index 454c4e1f6..19b6d8c45 100644 --- a/geo/src/algorithm/intersects/line_string.rs +++ b/geo/src/algorithm/intersects/line_string.rs @@ -8,12 +8,7 @@ where Line: Intersects, { fn intersects(&self, geom: &G) -> bool { - // No need to loop if either self or rhs is empty. - if self.0.is_empty() { - false - } else { - self.lines().any(|l| l.intersects(geom)) - } + self.lines().any(|l| l.intersects(geom)) } } symmetric_intersects_impl!(Coordinate, LineString); diff --git a/geo/src/algorithm/intersects/mod.rs b/geo/src/algorithm/intersects/mod.rs index 2e3c440d9..f129ad117 100644 --- a/geo/src/algorithm/intersects/mod.rs +++ b/geo/src/algorithm/intersects/mod.rs @@ -65,31 +65,6 @@ macro_rules! symmetric_intersects_impl { }; } -// Macro to delegate implementation to field `0` of rhs. -// Used for `Intersects>` from corresponding -// `Coordinate` impl. This cannot be a blanket impl. -// because it would conflict with blanket impl. of Point. -#[macro_use] -macro_rules! rhs_pt_from_coord_intersects_impl { - ($t:ty) => { - impl $crate::algorithm::intersects::Intersects> for $t - where - T: CoordinateType, - $t: $crate::algorithm::intersects::Intersects> - { - fn intersects(&self, rhs: &Point) -> bool { - self.intersects(&rhs.0) - } - - } - }; -} - -// Macro to delegate implementation to any() of an iterator. -// Used for `LineString`, `Intersects>` from corresponding -// `Coordinate` impl. This cannot be a blanket impl. -// because it would conflict with blanket impl. of Point. - mod coordinate; mod line; mod line_string; @@ -97,6 +72,8 @@ mod point; mod polygon; mod rect; mod triangle; +mod collections; + // Helper function to check value lies between min and max. // Only makes sense if min <= max (or always false) From ee5314f9506fbdebe98c511861240ff952dd8daa Mon Sep 17 00:00:00 2001 From: rmanoka Date: Tue, 6 Oct 2020 11:00:05 +0530 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Michael Kirk --- geo-types/src/multi_line_string.rs | 2 +- geo-types/src/multi_point.rs | 4 ++-- geo-types/src/multi_polygon.rs | 4 ++-- geo-types/src/polygon.rs | 2 +- geo-types/src/triangle.rs | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/geo-types/src/multi_line_string.rs b/geo-types/src/multi_line_string.rs index f9d8590d8..0243ef777 100644 --- a/geo-types/src/multi_line_string.rs +++ b/geo-types/src/multi_line_string.rs @@ -5,7 +5,7 @@ use std::iter::FromIterator; /// [`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 -/// objects, yields the component `LineString`s. +/// object yields the component `LineString`s. /// /// # Semantics /// diff --git a/geo-types/src/multi_point.rs b/geo-types/src/multi_point.rs index 1e2a0e279..bdd4f0b0c 100644 --- a/geo-types/src/multi_point.rs +++ b/geo-types/src/multi_point.rs @@ -4,13 +4,13 @@ use std::iter::FromIterator; /// 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 -/// objects, yields the component `Point`s. +/// 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 a `MultiPoint` is always +/// particular, the boundary of a `MultiPoint` is always /// empty. /// /// # Examples diff --git a/geo-types/src/multi_polygon.rs b/geo-types/src/multi_polygon.rs index 0b5502bd0..96c225182 100644 --- a/geo-types/src/multi_polygon.rs +++ b/geo-types/src/multi_polygon.rs @@ -4,7 +4,7 @@ use std::iter::FromIterator; /// 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 -/// objects, yields the component `Polygon`s. +/// object yields the component `Polygon`s. /// /// # Semantics /// @@ -18,7 +18,7 @@ use std::iter::FromIterator; /// - The boundaries of two (distinct) constituent polygons /// may only intersect at finitely many points. /// -/// Refer section 6.1.14 of the OGC-SFA for a formal +/// 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. diff --git a/geo-types/src/polygon.rs b/geo-types/src/polygon.rs index 1310fb415..163f2573c 100644 --- a/geo-types/src/polygon.rs +++ b/geo-types/src/polygon.rs @@ -39,7 +39,7 @@ use num_traits::{Float, Signed}; /// interior must admit a curve between these two that lies /// in the interior. /// -/// Refer section 6.1.11.1 of the OGC-SFA for a formal +/// 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 diff --git a/geo-types/src/triangle.rs b/geo-types/src/triangle.rs index 7d49d2b1f..508648f87 100644 --- a/geo-types/src/triangle.rs +++ b/geo-types/src/triangle.rs @@ -3,7 +3,7 @@ use crate::{polygon, Coordinate, CoordinateType, Line, Polygon}; /// 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 be collinear (and distinct). +/// 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(pub Coordinate, pub Coordinate, pub Coordinate); From 11ac854c98a8c90088482d746f5870a35558e989 Mon Sep 17 00:00:00 2001 From: Rajsekar Manokaran Date: Tue, 6 Oct 2020 11:07:42 +0530 Subject: [PATCH 5/5] Add missing impls + missed Line-Rect intersection + added Geom-Geom test to verify impl of all combination --- geo/src/algorithm/intersects/mod.rs | 10 +++++++++- geo/src/algorithm/intersects/rect.rs | 1 + 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/geo/src/algorithm/intersects/mod.rs b/geo/src/algorithm/intersects/mod.rs index f129ad117..b044d64a3 100644 --- a/geo/src/algorithm/intersects/mod.rs +++ b/geo/src/algorithm/intersects/mod.rs @@ -113,7 +113,7 @@ where #[cfg(test)] mod test { use crate::algorithm::intersects::Intersects; - use crate::{line_string, polygon, Coordinate, Line, LineString, Point, Polygon, Rect}; + use crate::{line_string, polygon, Geometry, Coordinate, Line, LineString, Point, Polygon, Rect}; /// Tests: intersection LineString and LineString #[test] @@ -567,4 +567,12 @@ mod test { assert!(a.intersects(&b)); assert!(b.intersects(&a)); } + + #[test] + fn compile_test_geom_geom() { + // This test should check existance of all + // combinations of geometry types. + let geom: Geometry<_> = Line::from([(0.5, 0.5), (2., 1.)]).into(); + assert!(geom.intersects(&geom)); + } } diff --git a/geo/src/algorithm/intersects/rect.rs b/geo/src/algorithm/intersects/rect.rs index aa8fbfe43..a99e513f3 100644 --- a/geo/src/algorithm/intersects/rect.rs +++ b/geo/src/algorithm/intersects/rect.rs @@ -54,3 +54,4 @@ where || Line::new(lt, lb).intersects(rhs) } } +symmetric_intersects_impl!(Line, Rect);