From fd935b238f096b9982a20ad832379c363128a399 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Thu, 31 Oct 2024 16:00:29 -0700 Subject: [PATCH 1/4] InterpolatePoint: handle some reasonable edge cases, document current behavior in tests. --- geo/CHANGES.md | 1 + .../line_measures/interpolate_point.rs | 156 ++++++++++++++++++ .../line_measures/metric_spaces/geodesic.rs | 10 ++ .../line_measures/metric_spaces/haversine.rs | 6 + 4 files changed, 173 insertions(+) diff --git a/geo/CHANGES.md b/geo/CHANGES.md index ad1f414b0..b9961991c 100644 --- a/geo/CHANGES.md +++ b/geo/CHANGES.md @@ -2,6 +2,7 @@ ## Unreleased +* Improve handling of InterploatePoint with collapsed Line ## 0.29.0 - 2024.10.30 diff --git a/geo/src/algorithm/line_measures/interpolate_point.rs b/geo/src/algorithm/line_measures/interpolate_point.rs index d53bc226e..9f653b966 100644 --- a/geo/src/algorithm/line_measures/interpolate_point.rs +++ b/geo/src/algorithm/line_measures/interpolate_point.rs @@ -32,3 +32,159 @@ pub trait InterpolatePoint { include_ends: bool, ) -> impl Iterator>; } + +#[cfg(test)] +mod tests { + use crate::{Euclidean, Geodesic, Haversine, InterpolatePoint, Point, Rhumb}; + + #[test] + fn point_at_ratio_between_line_ends() { + let start = Point::new(0.0, 0.0); + let end = Point::new(1.0, 1.0); + + let ratio = 0.0; + assert_eq!(Haversine::point_at_ratio_between(start, end, ratio), start); + assert_eq!(Euclidean::point_at_ratio_between(start, end, ratio), start); + assert_eq!(Geodesic::point_at_ratio_between(start, end, ratio), start); + assert_eq!(Rhumb::point_at_ratio_between(start, end, ratio), start); + + let ratio = 1.0; + assert_eq!(Haversine::point_at_ratio_between(start, end, ratio), end); + assert_eq!(Euclidean::point_at_ratio_between(start, end, ratio), end); + assert_eq!(Geodesic::point_at_ratio_between(start, end, ratio), end); + assert_eq!(Rhumb::point_at_ratio_between(start, end, ratio), end); + } + + mod degenerate { + use super::*; + + #[test] + fn point_at_ratio_between_collapsed_line() { + let start = Point::new(1.0, 1.0); + + let ratio = 0.0; + assert_eq!( + Haversine::point_at_ratio_between(start, start, ratio), + start + ); + assert_eq!( + Euclidean::point_at_ratio_between(start, start, ratio), + start + ); + assert_eq!(Geodesic::point_at_ratio_between(start, start, ratio), start); + assert_eq!(Rhumb::point_at_ratio_between(start, start, ratio), start); + + let ratio = 0.5; + assert_eq!( + Haversine::point_at_ratio_between(start, start, ratio), + start + ); + assert_eq!( + Euclidean::point_at_ratio_between(start, start, ratio), + start + ); + assert_eq!(Geodesic::point_at_ratio_between(start, start, ratio), start); + assert_eq!(Rhumb::point_at_ratio_between(start, start, ratio), start); + + let ratio = 1.0; + assert_eq!( + Haversine::point_at_ratio_between(start, start, ratio), + start + ); + assert_eq!( + Euclidean::point_at_ratio_between(start, start, ratio), + start + ); + assert_eq!(Geodesic::point_at_ratio_between(start, start, ratio), start); + assert_eq!(Rhumb::point_at_ratio_between(start, start, ratio), start); + } + + #[test] + fn point_at_distance_between_collapsed_line() { + // This method just documents existing behavior. I don't think our current behavior + // is especially useful, but we might consider handling it uniformly one day. + let start: Point = Point::new(1.0, 1.0); + + let distance = 0.0; + assert_eq!( + Haversine::point_at_distance_between(start, start, distance), + start + ); + + let euclidean_result = Euclidean::point_at_distance_between(start, start, distance); + assert!(euclidean_result.x().is_nan()); + assert!(euclidean_result.y().is_nan()); + assert_eq!( + Geodesic::point_at_distance_between(start, start, distance), + start + ); + assert_eq!( + Rhumb::point_at_distance_between(start, start, distance), + start + ); + + let distance = 100000.0; + let due_north = Point::new(1.0, 1.9); + let due_south = Point::new(1.0, 0.1); + assert_relative_eq!( + Haversine::point_at_distance_between(start, start, distance), + due_north, + epsilon = 1.0e-1 + ); + let euclidean_result = Euclidean::point_at_distance_between(start, start, distance); + assert!(euclidean_result.x().is_nan()); + assert!(euclidean_result.y().is_nan()); + assert_relative_eq!( + Geodesic::point_at_distance_between(start, start, distance), + due_south, + epsilon = 1.0e-1 + ); + assert_relative_eq!( + Rhumb::point_at_distance_between(start, start, distance), + due_north, + epsilon = 1.0e-1 + ); + } + + #[test] + fn points_along_collapsed_line() { + let start = Point::new(1.0, 1.0); + + let max_distance = 1.0; + + let include_ends = true; + let points: Vec<_> = + Haversine::points_along_line(start, start, max_distance, include_ends).collect(); + assert_eq!(points, vec![start, start]); + + let points: Vec<_> = + Euclidean::points_along_line(start, start, max_distance, include_ends).collect(); + assert_eq!(points, vec![start, start]); + + let points: Vec<_> = + Geodesic::points_along_line(start, start, max_distance, include_ends).collect(); + assert_eq!(points, vec![start, start]); + + let points: Vec<_> = + Rhumb::points_along_line(start, start, max_distance, include_ends).collect(); + assert_eq!(points, vec![start, start]); + + let include_ends = false; + let points: Vec<_> = + Haversine::points_along_line(start, start, max_distance, include_ends).collect(); + assert_eq!(points, vec![]); + + let points: Vec<_> = + Euclidean::points_along_line(start, start, max_distance, include_ends).collect(); + assert_eq!(points, vec![]); + + let points: Vec<_> = + Geodesic::points_along_line(start, start, max_distance, include_ends).collect(); + assert_eq!(points, vec![]); + + let points: Vec<_> = + Rhumb::points_along_line(start, start, max_distance, include_ends).collect(); + assert_eq!(points, vec![]); + } + } +} diff --git a/geo/src/algorithm/line_measures/metric_spaces/geodesic.rs b/geo/src/algorithm/line_measures/metric_spaces/geodesic.rs index 6e9ccb874..89335ca87 100644 --- a/geo/src/algorithm/line_measures/metric_spaces/geodesic.rs +++ b/geo/src/algorithm/line_measures/metric_spaces/geodesic.rs @@ -168,6 +168,9 @@ impl InterpolatePoint for Geodesic { end: Point, meters_from_start: f64, ) -> Point { + if meters_from_start == 0.0 { + return start; + } let bearing = Self::bearing(start, end); Self::destination(start, bearing, meters_from_start) } @@ -205,6 +208,13 @@ impl InterpolatePoint for Geodesic { end: Point, ratio_from_start: f64, ) -> Point { + if start == end || ratio_from_start == 0.0 { + return start; + } + if ratio_from_start == 1.0 { + return end; + } + let g = geographiclib_rs::Geodesic::wgs84(); let (total_distance, azi1, _azi2, _a12) = g.inverse(start.y(), start.x(), end.y(), end.x()); let distance = total_distance * ratio_from_start; diff --git a/geo/src/algorithm/line_measures/metric_spaces/haversine.rs b/geo/src/algorithm/line_measures/metric_spaces/haversine.rs index 971e7504e..ac4b02479 100644 --- a/geo/src/algorithm/line_measures/metric_spaces/haversine.rs +++ b/geo/src/algorithm/line_measures/metric_spaces/haversine.rs @@ -204,6 +204,12 @@ impl InterpolatePoint for Haversine { /// /// [great circle]: https://en.wikipedia.org/wiki/Great_circle fn point_at_ratio_between(start: Point, end: Point, ratio_from_start: F) -> Point { + if start == end || ratio_from_start == F::zero() { + return start; + } + if ratio_from_start == F::one() { + return end; + } let calculation = HaversineIntermediateFillCalculation::new(start, end); calculation.point_at_ratio(ratio_from_start) } From 6e5b7195b7e05deebcd55f60c503602c55a91bb0 Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Thu, 31 Oct 2024 22:00:58 -0400 Subject: [PATCH 2/4] Allow configuring of the `i_overlay` dep, Rayon transitive dep, and document more features. --- .github/workflows/test.yml | 2 +- geo/Cargo.toml | 5 +++-- geo/src/algorithm/bool_ops/mod.rs | 2 ++ geo/src/algorithm/mod.rs | 8 +++++++- geo/src/algorithm/triangulate_earcut.rs | 2 ++ geo/src/lib.rs | 26 ++++++++++++++++++++----- 6 files changed, 36 insertions(+), 9 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ddf68ef01..cdbf06f9b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -116,7 +116,7 @@ jobs: uses: actions/checkout@v3 - run: cargo check --all-targets --no-default-features # we don't want to test `proj-network` because it only enables the `proj` feature - - run: cargo test --features "use-proj use-serde" + - run: cargo test --features "use-proj use-serde i_overlay earcutr multithreading" geo_traits: name: geo-traits diff --git a/geo/Cargo.toml b/geo/Cargo.toml index c8cc5f188..9c29776ce 100644 --- a/geo/Cargo.toml +++ b/geo/Cargo.toml @@ -13,10 +13,11 @@ rust-version = "1.75" categories = ["science::geo"] [features] -default = ["earcutr", "spade"] +default = ["earcutr", "spade", "i_overlay", "multithreading"] use-proj = ["proj"] proj-network = ["use-proj", "proj/network"] use-serde = ["serde", "geo-types/serde"] +multithreading = ["i_overlay/allow_multithreading"] [dependencies] earcutr = { version = "0.4.2", optional = true } @@ -30,7 +31,7 @@ proj = { version = "0.27.0", optional = true } robust = "1.1.0" rstar = "0.12.0" serde = { version = "1.0", optional = true, features = ["derive"] } -i_overlay = "1.7.2" +i_overlay = { version = "1.7.2", optional = true } [dev-dependencies] approx = ">= 0.4.0, < 0.6.0" diff --git a/geo/src/algorithm/bool_ops/mod.rs b/geo/src/algorithm/bool_ops/mod.rs index 16287b049..aa5abdf02 100644 --- a/geo/src/algorithm/bool_ops/mod.rs +++ b/geo/src/algorithm/bool_ops/mod.rs @@ -8,6 +8,8 @@ use crate::geometry::{LineString, MultiLineString, MultiPolygon, Polygon}; /// Boolean Operations on geometry. /// +/// Requires the `"i_overlay"` feature, which is enabled by default. +/// /// Boolean operations are set operations on geometries considered as a subset /// of the 2-D plane. The operations supported are: intersection, union, xor or /// symmetric difference, and set-difference on pairs of 2-D geometries and diff --git a/geo/src/algorithm/mod.rs b/geo/src/algorithm/mod.rs index 344084dc3..b47f1c56d 100644 --- a/geo/src/algorithm/mod.rs +++ b/geo/src/algorithm/mod.rs @@ -6,8 +6,12 @@ pub use kernels::{Kernel, Orientation}; pub mod area; pub use area::Area; -/// Boolean Ops such as union, xor, difference; +/// Boolean Ops such as union, xor, difference. +/// +/// Requires the `"i_overlay"` feature, which is enabled by default. +#[cfg(feature = "i_overlay")] pub mod bool_ops; +#[cfg(feature = "i_overlay")] pub use bool_ops::{BooleanOps, OpType}; /// Calculate the bounding rectangle of a `Geometry`. @@ -254,6 +258,8 @@ pub mod translate; pub use translate::Translate; /// Triangulate polygons using an [ear-cutting algorithm](https://www.geometrictools.com/Documentation/TriangulationByEarClipping.pdf). +/// +/// Requires the `"earcutr"` feature. #[cfg(feature = "earcutr")] pub mod triangulate_earcut; #[cfg(feature = "earcutr")] diff --git a/geo/src/algorithm/triangulate_earcut.rs b/geo/src/algorithm/triangulate_earcut.rs index d4d23df32..586f3cb3d 100644 --- a/geo/src/algorithm/triangulate_earcut.rs +++ b/geo/src/algorithm/triangulate_earcut.rs @@ -1,6 +1,8 @@ use crate::{coord, CoordFloat, CoordsIter, Polygon, Triangle}; /// Triangulate polygons using an [ear-cutting algorithm](https://www.geometrictools.com/Documentation/TriangulationByEarClipping.pdf). +/// +/// Requires the `"earcutr"` feature, which is enabled by default. pub trait TriangulateEarcut { /// # Examples /// diff --git a/geo/src/lib.rs b/geo/src/lib.rs index 11cf8bc4a..ff5bdf94a 100644 --- a/geo/src/lib.rs +++ b/geo/src/lib.rs @@ -67,7 +67,7 @@ //! //! ## Boolean Operations //! -//! - **[`BooleanOps`]**: combine or split (Multi)Polygons using intersecton, union, xor, or difference operations +//! - **[`BooleanOps`]**: combine or split (Multi)Polygons using intersecton, union, xor, or difference operations. Requires the `"i_overlay"` feature, which is enabled by default. //! //! ## Outlier Detection //! @@ -112,7 +112,7 @@ //! //! ## Triangulation //! -//! - **[`TriangulateEarcut`](triangulate_earcut)**: Triangulate polygons using the earcut algorithm (requires the `earcutr` feature). +//! - **[`TriangulateEarcut`](triangulate_earcut)**: Triangulate polygons using the earcut algorithm. Requires the `"earcutr"` feature, which is enabled by default. //! //! ## Winding //! @@ -177,9 +177,25 @@ //! //! The following optional [Cargo features] are available: //! -//! - `proj-network`: Enables [network grid] support for the [`proj` crate]. After enabling this feature, [further configuration][proj crate file download] is required to use the network grid -//! - `use-proj`: Enables coordinate conversion and transformation of `Point` geometries using the [`proj` crate] -//! - `use-serde`: Allows geometry types to be serialized and deserialized with [Serde] +//! - `i_overlay`: +//! - Enables the `i_overlay` crate, which provides boolean operations on geometries. +//! - ☑ Enabled by default. +//! - `earcutr`: +//! - Enables the `earcutr` crate, which provides triangulation of polygons using the earcut algorithm. +//! - ☑ Enabled by default. +//! - `proj-network`: +//! - Enables [network grid] support for the [`proj` crate]. +//! - After enabling this feature, [further configuration][proj crate file download] is required to use the network grid. +//! - ☐ Disabled by default. +//! - `use-proj`: +//! - Enables coordinate conversion and transformation of `Point` geometries using the [`proj` crate] +//! - ☐ Disabled by default. +//! - `use-serde`: +//! - Allows geometry types to be serialized and deserialized with [Serde]. +//! - ☐ Disabled by default. +//! - `multithreading`: +//! - Enables multithreading support for the `i_overlay` crate. +//! - ☑ Enabled by default. //! //! # Ecosystem //! From 6e2303904d3b3470e683f194a8e21cbe3786fa67 Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Fri, 1 Nov 2024 12:21:52 -0400 Subject: [PATCH 3/4] Update Cargo.toml --- geo/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/geo/Cargo.toml b/geo/Cargo.toml index 9c29776ce..dd6b8ebb0 100644 --- a/geo/Cargo.toml +++ b/geo/Cargo.toml @@ -31,7 +31,7 @@ proj = { version = "0.27.0", optional = true } robust = "1.1.0" rstar = "0.12.0" serde = { version = "1.0", optional = true, features = ["derive"] } -i_overlay = { version = "1.7.2", optional = true } +i_overlay = { version = "1.7.2", optional = true, default-features = false } [dev-dependencies] approx = ">= 0.4.0, < 0.6.0" From c85ed96fec60fd10bd452e49d461c6856c3103b5 Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Fri, 1 Nov 2024 14:54:31 -0400 Subject: [PATCH 4/4] apply changes --- .github/workflows/test.yml | 2 +- geo/Cargo.toml | 4 ++-- geo/src/algorithm/bool_ops/mod.rs | 2 -- geo/src/algorithm/mod.rs | 4 ---- geo/src/lib.rs | 5 +---- 5 files changed, 4 insertions(+), 13 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cdbf06f9b..598c60b2c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -116,7 +116,7 @@ jobs: uses: actions/checkout@v3 - run: cargo check --all-targets --no-default-features # we don't want to test `proj-network` because it only enables the `proj` feature - - run: cargo test --features "use-proj use-serde i_overlay earcutr multithreading" + - run: cargo test --features "use-proj use-serde earcutr multithreading" geo_traits: name: geo-traits diff --git a/geo/Cargo.toml b/geo/Cargo.toml index 9c29776ce..b9a63b277 100644 --- a/geo/Cargo.toml +++ b/geo/Cargo.toml @@ -13,7 +13,7 @@ rust-version = "1.75" categories = ["science::geo"] [features] -default = ["earcutr", "spade", "i_overlay", "multithreading"] +default = ["earcutr", "spade", "multithreading"] use-proj = ["proj"] proj-network = ["use-proj", "proj/network"] use-serde = ["serde", "geo-types/serde"] @@ -31,7 +31,7 @@ proj = { version = "0.27.0", optional = true } robust = "1.1.0" rstar = "0.12.0" serde = { version = "1.0", optional = true, features = ["derive"] } -i_overlay = { version = "1.7.2", optional = true } +i_overlay = "1.7.2" [dev-dependencies] approx = ">= 0.4.0, < 0.6.0" diff --git a/geo/src/algorithm/bool_ops/mod.rs b/geo/src/algorithm/bool_ops/mod.rs index aa5abdf02..16287b049 100644 --- a/geo/src/algorithm/bool_ops/mod.rs +++ b/geo/src/algorithm/bool_ops/mod.rs @@ -8,8 +8,6 @@ use crate::geometry::{LineString, MultiLineString, MultiPolygon, Polygon}; /// Boolean Operations on geometry. /// -/// Requires the `"i_overlay"` feature, which is enabled by default. -/// /// Boolean operations are set operations on geometries considered as a subset /// of the 2-D plane. The operations supported are: intersection, union, xor or /// symmetric difference, and set-difference on pairs of 2-D geometries and diff --git a/geo/src/algorithm/mod.rs b/geo/src/algorithm/mod.rs index b47f1c56d..6f92320cf 100644 --- a/geo/src/algorithm/mod.rs +++ b/geo/src/algorithm/mod.rs @@ -7,11 +7,7 @@ pub mod area; pub use area::Area; /// Boolean Ops such as union, xor, difference. -/// -/// Requires the `"i_overlay"` feature, which is enabled by default. -#[cfg(feature = "i_overlay")] pub mod bool_ops; -#[cfg(feature = "i_overlay")] pub use bool_ops::{BooleanOps, OpType}; /// Calculate the bounding rectangle of a `Geometry`. diff --git a/geo/src/lib.rs b/geo/src/lib.rs index ff5bdf94a..88bf61aa8 100644 --- a/geo/src/lib.rs +++ b/geo/src/lib.rs @@ -67,7 +67,7 @@ //! //! ## Boolean Operations //! -//! - **[`BooleanOps`]**: combine or split (Multi)Polygons using intersecton, union, xor, or difference operations. Requires the `"i_overlay"` feature, which is enabled by default. +//! - **[`BooleanOps`]**: combine or split (Multi)Polygons using intersecton, union, xor, or difference operations. //! //! ## Outlier Detection //! @@ -177,9 +177,6 @@ //! //! The following optional [Cargo features] are available: //! -//! - `i_overlay`: -//! - Enables the `i_overlay` crate, which provides boolean operations on geometries. -//! - ☑ Enabled by default. //! - `earcutr`: //! - Enables the `earcutr` crate, which provides triangulation of polygons using the earcut algorithm. //! - ☑ Enabled by default.