diff --git a/geo/CHANGES.md b/geo/CHANGES.md index d032c522e..4d776d9bb 100644 --- a/geo/CHANGES.md +++ b/geo/CHANGES.md @@ -2,6 +2,9 @@ ## Unreleased +- Fix crash in `BoolOps` by updating `i_overlay` to 1.9.0. + - + ## 0.29.2 - 2024.11.15 - Pin `i_overlay` to < 1.8.0 to work around [recursion bug](https://github.com/georust/geo/issues/1270). diff --git a/geo/Cargo.toml b/geo/Cargo.toml index 005a55386..1c40d91ff 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, < 1.8.0", default-features = false } +i_overlay = { version = "1.9.0, < 1.10.0", default-features = false } [dev-dependencies] approx = ">= 0.4.0, < 0.6.0" diff --git a/geo/src/algorithm/bool_ops/i_overlay_integration.rs b/geo/src/algorithm/bool_ops/i_overlay_integration.rs index bb78254a7..2ad867a13 100644 --- a/geo/src/algorithm/bool_ops/i_overlay_integration.rs +++ b/geo/src/algorithm/bool_ops/i_overlay_integration.rs @@ -1,290 +1,71 @@ use crate::geometry::Coord; use crate::GeoNum; -use i_overlay::core::fill_rule::FillRule; -use i_overlay::core::overlay::ShapeType; -use i_overlay::core::overlay_rule::OverlayRule; -use i_overlay::string::clip::ClipRule; +use i_overlay::i_float::float::compatible::FloatPointCompatible; +use i_overlay::i_float::float::number::FloatNumber; -pub trait BoolOpsCoord: Copy { - fn new(x: T, y: T) -> Self; - fn x(&self) -> T; - fn y(&self) -> T; -} - -/// A geometry coordinate number suitable for performing geometric boolean operations. -pub trait BoolOpsNum: GeoNum { - type CoordType: BoolOpsCoord; - type OverlayType: BoolOpsOverlay; - type StringOverlayType: BoolOpsStringOverlay; - - fn to_bops_coord(geo_coord: Coord) -> Self::CoordType { - Self::CoordType::new(geo_coord.x, geo_coord.y) - } - - fn to_geo_coord(bops_coord: Self::CoordType) -> Coord { - Coord { - x: bops_coord.x(), - y: bops_coord.y(), - } - } -} - -pub trait BoolOpsOverlay { - type CoordType; - type OverlayGraph: BoolOpsOverlayGraph; - fn new() -> Self; - fn add_path(&mut self, path: Vec, shape_type: ShapeType); - fn into_graph(self, fill_rule: FillRule) -> Self::OverlayGraph; -} - -pub(super) trait BoolOpsOverlayGraph { - type CoordType; - fn extract_shapes(&self, overlay_rule: OverlayRule) -> Vec>>; -} - -pub trait BoolOpsStringOverlay { - type CoordType; - type StringGraph: BoolOpsStringGraph; - fn new() -> Self; - fn add_shape_path(&mut self, path: Vec); - fn add_string_line(&mut self, path: [Self::CoordType; 2]); - fn into_graph(self, fill_rule: FillRule) -> Self::StringGraph; -} - -pub(super) trait BoolOpsStringGraph { - type CoordType; - fn clip_string_lines(&self, clip_rule: ClipRule) -> Vec>; -} - -mod f64 { - use super::{ClipRule, FillRule, OverlayRule, ShapeType}; - use i_overlay::f64::{ - graph::F64OverlayGraph, - overlay::F64Overlay, - string::{F64StringGraph, F64StringOverlay}, - }; - use i_overlay::i_float::f64_point::F64Point; - - impl super::BoolOpsNum for f64 { - type CoordType = F64Point; - type OverlayType = F64Overlay; - type StringOverlayType = F64StringOverlay; - } - - impl super::BoolOpsCoord for F64Point { - #[inline] - fn new(x: f64, y: f64) -> Self { - Self::new(x, y) - } - - #[inline] - fn x(&self) -> f64 { - self.x - } - - #[inline] - fn y(&self) -> f64 { - self.y - } - } - - impl super::BoolOpsOverlay for F64Overlay { - type CoordType = F64Point; - type OverlayGraph = F64OverlayGraph; - - #[inline] - fn new() -> Self { - Self::new() - } - - #[inline] - fn add_path(&mut self, path: Vec, shape_type: ShapeType) { - self.add_path(path, shape_type) - } - - #[inline] - fn into_graph(self, fill_rule: FillRule) -> Self::OverlayGraph { - self.into_graph(fill_rule) - } - } - - impl super::BoolOpsOverlayGraph for F64OverlayGraph { - type CoordType = F64Point; - - #[inline] - fn extract_shapes(&self, overlay_rule: OverlayRule) -> Vec>> { - self.extract_shapes(overlay_rule) - } - } - - impl super::BoolOpsStringOverlay for F64StringOverlay { - type CoordType = F64Point; - type StringGraph = F64StringGraph; - - #[inline] - fn new() -> Self { - Self::new() - } +/// A geometry coordinate scalar suitable for performing geometric boolean operations. +pub trait BoolOpsNum: GeoNum + FloatNumber {} +impl BoolOpsNum for T {} - #[inline] - fn add_shape_path(&mut self, path: Vec) { - self.add_shape_path(path) - } - - #[inline] - fn add_string_line(&mut self, path: [Self::CoordType; 2]) { - self.add_string_line(path) - } +/// New type for `Coord` that implements `FloatPointCompatible` for `BoolOpsNum` to +/// circumvent orphan rule, since Coord is defined in geo_types. +#[derive(Copy, Clone, Debug)] +pub struct BoolOpsCoord(pub(crate) Coord); - #[inline] - fn into_graph(self, fill_rule: FillRule) -> Self::StringGraph { - self.into_graph(fill_rule) - } +impl FloatPointCompatible for BoolOpsCoord { + fn from_xy(x: T, y: T) -> Self { + Self(Coord { x, y }) } - impl super::BoolOpsStringGraph for F64StringGraph { - type CoordType = F64Point; - - #[inline] - fn clip_string_lines(&self, clip_rule: ClipRule) -> Vec> { - self.clip_string_lines(clip_rule) - } + fn x(&self) -> T { + self.0.x } -} - -mod f32 { - use i_overlay::core::fill_rule::FillRule; - use i_overlay::core::overlay::ShapeType; - use i_overlay::core::overlay_rule::OverlayRule; - use i_overlay::f32::graph::F32OverlayGraph; - use i_overlay::f32::overlay::F32Overlay; - use i_overlay::f32::string::{F32StringGraph, F32StringOverlay}; - use i_overlay::i_float::f32_point::F32Point; - use i_overlay::string::clip::ClipRule; - impl super::BoolOpsNum for f32 { - type CoordType = F32Point; - type OverlayType = F32Overlay; - type StringOverlayType = F32StringOverlay; - } - - impl super::BoolOpsCoord for F32Point { - #[inline] - fn new(x: f32, y: f32) -> Self { - Self::new(x, y) - } - #[inline] - fn x(&self) -> f32 { - self.x - } - #[inline] - fn y(&self) -> f32 { - self.y - } - } - - impl super::BoolOpsOverlay for F32Overlay { - type CoordType = F32Point; - type OverlayGraph = F32OverlayGraph; - - #[inline] - fn new() -> Self { - Self::new() - } - - #[inline] - fn add_path(&mut self, path: Vec, shape_type: ShapeType) { - self.add_path(path, shape_type) - } - - #[inline] - fn into_graph(self, fill_rule: FillRule) -> Self::OverlayGraph { - self.into_graph(fill_rule) - } - } - - impl super::BoolOpsOverlayGraph for F32OverlayGraph { - type CoordType = F32Point; - - #[inline] - fn extract_shapes(&self, overlay_rule: OverlayRule) -> Vec>> { - self.extract_shapes(overlay_rule) - } - } - - impl super::BoolOpsStringOverlay for F32StringOverlay { - type CoordType = F32Point; - type StringGraph = F32StringGraph; - - #[inline] - fn new() -> Self { - Self::new() - } - - #[inline] - fn add_shape_path(&mut self, path: Vec) { - self.add_shape_path(path) - } - - #[inline] - fn add_string_line(&mut self, path: [Self::CoordType; 2]) { - self.add_string_line(path) - } - - #[inline] - fn into_graph(self, fill_rule: FillRule) -> Self::StringGraph { - self.into_graph(fill_rule) - } - } - - impl super::BoolOpsStringGraph for F32StringGraph { - type CoordType = F32Point; - - #[inline] - fn clip_string_lines(&self, clip_rule: ClipRule) -> Vec> { - self.clip_string_lines(clip_rule) - } + fn y(&self) -> T { + self.0.y } } pub(super) mod convert { use super::super::OpType; - use super::{BoolOpsNum, OverlayRule}; + use super::BoolOpsNum; + use crate::bool_ops::i_overlay_integration::BoolOpsCoord; use crate::geometry::{LineString, MultiLineString, MultiPolygon, Polygon}; + use i_overlay::core::overlay_rule::OverlayRule; - pub fn line_string_from_path(path: Vec) -> LineString { - let coords = path.into_iter().map(T::to_geo_coord); - LineString(coords.collect()) + pub fn line_string_from_path(path: Vec>) -> LineString { + let coords = path.into_iter().map(|bops_coord| bops_coord.0).collect(); + LineString(coords) } pub fn multi_line_string_from_paths( - paths: Vec>, + paths: Vec>>, ) -> MultiLineString { let line_strings = paths.into_iter().map(|p| line_string_from_path(p)); MultiLineString(line_strings.collect()) } - pub fn polygon_from_shape(shape: Vec>) -> Polygon { + pub fn polygon_from_shape(shape: Vec>>) -> Polygon { let mut rings = shape.into_iter().map(|p| line_string_from_path(p)); let exterior = rings.next().unwrap_or(LineString::new(vec![])); Polygon::new(exterior, rings.collect()) } pub fn multi_polygon_from_shapes( - shapes: Vec>>, + shapes: Vec>>>, ) -> MultiPolygon { let polygons = shapes.into_iter().map(|s| polygon_from_shape(s)); MultiPolygon(polygons.collect()) } - pub fn ring_to_shape_path(line_string: &LineString) -> Vec { + pub fn ring_to_shape_path(line_string: &LineString) -> Vec> { if line_string.0.is_empty() { return vec![]; } // In geo, Polygon rings are explicitly closed LineStrings — their final coordinate is the same as their first coordinate, // however in i_overlay, shape paths are implicitly closed, so we skip the last coordinate. let coords = &line_string.0[..line_string.0.len() - 1]; - coords.iter().copied().map(T::to_bops_coord).collect() + coords.iter().copied().map(BoolOpsCoord).collect() } impl From for OverlayRule { @@ -308,7 +89,7 @@ mod tests { #[test] fn two_empty_polygons() { let p1: Polygon = wkt!(POLYGON EMPTY); - let p2 = wkt!(POLYGON EMPTY); + let p2: Polygon = wkt!(POLYGON EMPTY); assert_eq!(&p1.union(&p2), &wkt!(MULTIPOLYGON EMPTY)); assert_eq!(&p1.intersection(&p2), &wkt!(MULTIPOLYGON EMPTY)); } @@ -316,7 +97,7 @@ mod tests { #[test] fn one_empty_polygon() { let p1: Polygon = wkt!(POLYGON((0. 0., 0. 1., 1. 1., 1. 0., 0. 0.))); - let p2 = wkt!(POLYGON EMPTY); + let p2: Polygon = wkt!(POLYGON EMPTY); assert_eq!(&p1.union(&p2), &MultiPolygon(vec![p1.clone()])); assert_eq!(&p1.intersection(&p2), &wkt!(MULTIPOLYGON EMPTY)); } diff --git a/geo/src/algorithm/bool_ops/mod.rs b/geo/src/algorithm/bool_ops/mod.rs index 16287b049..e0debd80a 100644 --- a/geo/src/algorithm/bool_ops/mod.rs +++ b/geo/src/algorithm/bool_ops/mod.rs @@ -2,6 +2,14 @@ mod i_overlay_integration; #[cfg(test)] mod tests; +use crate::bool_ops::i_overlay_integration::convert::{ + multi_polygon_from_shapes, ring_to_shape_path, +}; +use crate::bool_ops::i_overlay_integration::BoolOpsCoord; +use i_overlay::core::fill_rule::FillRule; +use i_overlay::float::clip::FloatClip; +use i_overlay::float::single::SingleFloatOverlay; +use i_overlay::string::clip::ClipRule; pub use i_overlay_integration::BoolOpsNum; use crate::geometry::{LineString, MultiLineString, MultiPolygon, Polygon}; @@ -43,22 +51,10 @@ pub trait BooleanOps { other: &impl BooleanOps, op: OpType, ) -> MultiPolygon { - use i_overlay::core::fill_rule::FillRule; - use i_overlay::core::overlay::ShapeType; - use i_overlay_integration::{convert, BoolOpsOverlay, BoolOpsOverlayGraph}; - let mut overlay = ::OverlayType::new(); - - for ring in self.rings() { - overlay.add_path(convert::ring_to_shape_path(ring), ShapeType::Subject); - } - for ring in other.rings() { - overlay.add_path(convert::ring_to_shape_path(ring), ShapeType::Clip); - } - - let graph = overlay.into_graph(FillRule::EvenOdd); - let shapes = graph.extract_shapes(op.into()); - - convert::multi_polygon_from_shapes(shapes) + let subject = self.rings().map(ring_to_shape_path).collect::>(); + let clip = other.rings().map(ring_to_shape_path).collect::>(); + let shapes = subject.overlay(&clip, op.into(), FillRule::EvenOdd); + multi_polygon_from_shapes(shapes) } fn intersection( @@ -89,31 +85,19 @@ pub trait BooleanOps { multi_line_string: &MultiLineString, invert: bool, ) -> MultiLineString { - use i_overlay::core::fill_rule::FillRule; - use i_overlay::string::clip::ClipRule; - use i_overlay_integration::{convert, BoolOpsStringGraph, BoolOpsStringOverlay}; - - let mut overlay = ::StringOverlayType::new(); + let subject: Vec> = multi_line_string + .iter() + .map(|line_string| line_string.coords().map(|c| BoolOpsCoord(*c)).collect()) + .collect(); - for ring in self.rings() { - overlay.add_shape_path(convert::ring_to_shape_path(ring)); - } - for line_string in multi_line_string { - for line in line_string.lines() { - let line = [ - Self::Scalar::to_bops_coord(line.start), - Self::Scalar::to_bops_coord(line.end), - ]; - overlay.add_string_line(line) - } - } + let clip = self.rings().map(ring_to_shape_path).collect::>(); - let graph = overlay.into_graph(FillRule::EvenOdd); - let paths = graph.clip_string_lines(ClipRule { + let clip_rule = ClipRule { invert, boundary_included: true, - }); - convert::multi_line_string_from_paths(paths) + }; + let paths = subject.clip_by(&clip, FillRule::EvenOdd, clip_rule); + i_overlay_integration::convert::multi_line_string_from_paths(paths) } } @@ -129,7 +113,7 @@ impl BooleanOps for Polygon { type Scalar = T; fn rings(&self) -> impl Iterator> { - std::iter::once(self.exterior()).chain(self.interiors().iter()) + std::iter::once(self.exterior()).chain(self.interiors()) } } @@ -137,6 +121,6 @@ impl BooleanOps for MultiPolygon { type Scalar = T; fn rings(&self) -> impl Iterator> { - self.0.iter().flat_map(|p| p.rings()) + self.iter().flat_map(BooleanOps::rings) } } diff --git a/geo/src/algorithm/bool_ops/tests.rs b/geo/src/algorithm/bool_ops/tests.rs index 4fac6d0e6..ffc4c059a 100644 --- a/geo/src/algorithm/bool_ops/tests.rs +++ b/geo/src/algorithm/bool_ops/tests.rs @@ -1,5 +1,6 @@ use super::BooleanOps; use crate::{wkt, Convert, MultiPolygon, Relate}; +use wkt::ToWkt; #[test] fn jts_test_overlay_la_1() { @@ -50,7 +51,12 @@ fn jts_test_overlay_la_1() { .convert(); let im = actual.relate(&expected); - assert!(im.is_equal_topo()); + assert!( + im.is_equal_topo(), + "actual: {:#?}, expected: {:#?}", + actual.wkt_string(), + expected.wkt_string() + ); } mod gh_issues { @@ -181,7 +187,7 @@ mod gh_issues { wkt!(POLYGON((3.3493652 -55.80127,171.22443 -300.,195.83728 -300.,-46.650635 30.80127,3.3493652 -55.80127))), ]; - let mut multi = MultiPolygon::new(Vec::new()); + let mut multi: MultiPolygon = MultiPolygon::new(Vec::new()); for poly in polygons { multi = multi.union(&MultiPolygon::new(vec![poly])); }