diff --git a/geo-generic-alg/src/algorithm/area.rs b/geo-generic-alg/src/algorithm/area.rs index 077a00dc8c..f16a4bb5a3 100644 --- a/geo-generic-alg/src/algorithm/area.rs +++ b/geo-generic-alg/src/algorithm/area.rs @@ -1,6 +1,7 @@ use geo_traits_ext::*; use crate::{CoordFloat, CoordNum}; +use core::borrow::Borrow; pub(crate) fn twice_signed_ring_area>( linestring: &LS, @@ -260,16 +261,6 @@ where } } -impl> AreaTrait for G -where - T: CoordFloat, -{ - crate::geometry_trait_ext_delegate_impl! { - fn signed_area_trait(&self) -> T; - fn unsigned_area_trait(&self) -> T; - } -} - impl> AreaTrait for GC where T: CoordFloat, @@ -287,6 +278,51 @@ where } } +impl> AreaTrait for G +where + T: CoordFloat, +{ + fn signed_area_trait(&self) -> T { + if self.is_collection() { + self.geometries_ext() + .map(|g_inner| g_inner.borrow().signed_area_trait()) + .fold(T::zero(), |acc, next| acc + next) + } else { + match self.as_type_ext() { + GeometryTypeExt::Point(g) => g.signed_area_trait(), + GeometryTypeExt::Line(g) => g.signed_area_trait(), + GeometryTypeExt::LineString(g) => g.signed_area_trait(), + GeometryTypeExt::Polygon(g) => g.signed_area_trait(), + GeometryTypeExt::MultiPoint(g) => g.signed_area_trait(), + GeometryTypeExt::MultiLineString(g) => g.signed_area_trait(), + GeometryTypeExt::MultiPolygon(g) => g.signed_area_trait(), + GeometryTypeExt::Rect(g) => g.signed_area_trait(), + GeometryTypeExt::Triangle(g) => g.signed_area_trait(), + } + } + } + + fn unsigned_area_trait(&self) -> T { + if self.is_collection() { + self.geometries_ext() + .map(|g_inner| g_inner.borrow().unsigned_area_trait()) + .fold(T::zero(), |acc, next| acc + next) + } else { + match self.as_type_ext() { + GeometryTypeExt::Point(g) => g.unsigned_area_trait(), + GeometryTypeExt::Line(g) => g.unsigned_area_trait(), + GeometryTypeExt::LineString(g) => g.unsigned_area_trait(), + GeometryTypeExt::Polygon(g) => g.unsigned_area_trait(), + GeometryTypeExt::MultiPoint(g) => g.unsigned_area_trait(), + GeometryTypeExt::MultiLineString(g) => g.unsigned_area_trait(), + GeometryTypeExt::MultiPolygon(g) => g.unsigned_area_trait(), + GeometryTypeExt::Rect(g) => g.unsigned_area_trait(), + GeometryTypeExt::Triangle(g) => g.unsigned_area_trait(), + } + } + } +} + #[cfg(test)] mod test { use crate::Area; diff --git a/geo-generic-alg/src/algorithm/bounding_rect.rs b/geo-generic-alg/src/algorithm/bounding_rect.rs index 8ee386ef04..d1673811a4 100644 --- a/geo-generic-alg/src/algorithm/bounding_rect.rs +++ b/geo-generic-alg/src/algorithm/bounding_rect.rs @@ -1,5 +1,6 @@ use crate::utils::{partial_max, partial_min}; use crate::{coord, geometry::*, CoordNum, GeometryCow}; +use core::borrow::Borrow; use geo_traits_ext::*; use geo_types::private_utils::get_bounding_rect; @@ -199,17 +200,6 @@ where } } -impl> BoundingRectTrait for G -where - T: CoordNum, -{ - type Output = Option>; - - crate::geometry_trait_ext_delegate_impl! { - fn bounding_rect_trait(&self) -> Self::Output; - } -} - impl> BoundingRectTrait for GC where T: CoordNum, @@ -229,6 +219,39 @@ where } } +impl> BoundingRectTrait for G +where + T: CoordNum, +{ + type Output = Option>; + + fn bounding_rect_trait(&self) -> Self::Output { + if self.is_collection() { + self.geometries_ext().fold(None, |acc, next| { + let next_bounding_rect = next.borrow().bounding_rect_trait(); + + match (acc, next_bounding_rect) { + (None, None) => None, + (Some(r), None) | (None, Some(r)) => Some(r), + (Some(r1), Some(r2)) => Some(bounding_rect_merge(r1, r2)), + } + }) + } else { + match self.as_type_ext() { + GeometryTypeExt::Point(g) => g.bounding_rect_trait().into(), + GeometryTypeExt::Line(g) => g.bounding_rect_trait().into(), + GeometryTypeExt::LineString(g) => g.bounding_rect_trait(), + GeometryTypeExt::Polygon(g) => g.bounding_rect_trait(), + GeometryTypeExt::MultiPoint(g) => g.bounding_rect_trait(), + GeometryTypeExt::MultiLineString(g) => g.bounding_rect_trait(), + GeometryTypeExt::MultiPolygon(g) => g.bounding_rect_trait(), + GeometryTypeExt::Rect(g) => g.bounding_rect_trait().into(), + GeometryTypeExt::Triangle(g) => g.bounding_rect_trait().into(), + } + } + } +} + impl BoundingRect for GeometryCow<'_, T> where T: CoordNum, @@ -379,5 +402,13 @@ mod test { ]) .bounding_rect(), ); + assert_eq!( + Some(Rect::new(coord! { x: 0., y: 0. }, coord! { x: 1., y: 2. })), + Geometry::GeometryCollection(GeometryCollection::new_from(vec![ + Geometry::Point(point! { x: 0., y: 0. }), + Geometry::Point(point! { x: 1., y: 2. }), + ])) + .bounding_rect(), + ); } } diff --git a/geo-generic-alg/src/algorithm/centroid.rs b/geo-generic-alg/src/algorithm/centroid.rs index 4bc1625426..b3a35909fc 100644 --- a/geo-generic-alg/src/algorithm/centroid.rs +++ b/geo-generic-alg/src/algorithm/centroid.rs @@ -1,3 +1,4 @@ +use core::borrow::Borrow; use std::cmp::Ordering; use geo_traits_ext::*; @@ -390,32 +391,51 @@ where { type Output = Option>; - crate::geometry_trait_ext_delegate_impl! { - /// The Centroid of a [`Geometry`] is the centroid of its enum variant - /// - /// # Examples - /// - /// ``` - /// use geo::Centroid; - /// use geo::{Geometry, Rect, point}; - /// - /// let rect = Rect::new( - /// point!(x: 0.0f32, y: 0.0), - /// point!(x: 1.0, y: 1.0), - /// ); - /// let geometry = Geometry::from(rect.clone()); - /// - /// assert_eq!( - /// Some(rect.centroid()), - /// geometry.centroid(), - /// ); - /// - /// assert_eq!( - /// Some(point!(x: 0.5, y: 0.5)), - /// geometry.centroid(), - /// ); - /// ``` - fn centroid_trait(&self) -> Self::Output; + /// The Centroid of a [`Geometry`] is the centroid of its enum variant + /// + /// # Examples + /// + /// ``` + /// use geo::Centroid; + /// use geo::{Geometry, Rect, point}; + /// + /// let rect = Rect::new( + /// point!(x: 0.0f32, y: 0.0), + /// point!(x: 1.0, y: 1.0), + /// ); + /// let geometry = Geometry::from(rect.clone()); + /// + /// assert_eq!( + /// Some(rect.centroid()), + /// geometry.centroid(), + /// ); + /// + /// assert_eq!( + /// Some(point!(x: 0.5, y: 0.5)), + /// geometry.centroid(), + /// ); + /// ``` + fn centroid_trait(&self) -> Self::Output { + if self.is_collection() { + // Handle geometry collection by computing weighted centroid + let mut operation = CentroidOperation::new(); + for g_inner in self.geometries_ext() { + operation.add_geometry(g_inner.borrow()); + } + operation.centroid() + } else { + match self.as_type_ext() { + GeometryTypeExt::Point(g) => Some(g.centroid_trait()), + GeometryTypeExt::Line(g) => Some(g.centroid_trait()), + GeometryTypeExt::LineString(g) => g.centroid_trait(), + GeometryTypeExt::Polygon(g) => g.centroid_trait(), + GeometryTypeExt::MultiPoint(g) => g.centroid_trait(), + GeometryTypeExt::MultiLineString(g) => g.centroid_trait(), + GeometryTypeExt::MultiPolygon(g) => g.centroid_trait(), + GeometryTypeExt::Rect(g) => Some(g.centroid_trait()), + GeometryTypeExt::Triangle(g) => Some(g.centroid_trait()), + } + } } } @@ -658,21 +678,26 @@ impl CentroidOperation { where G: GeometryTraitExt, { - match geometry.as_type_ext() { - GeometryTypeExt::Point(g) => { - if let Some(coord) = g.geo_coord() { - self.add_coord(coord) + if geometry.is_collection() { + for g_inner in geometry.geometries_ext() { + self.add_geometry(g_inner.borrow()); + } + } else { + match geometry.as_type_ext() { + GeometryTypeExt::Point(g) => { + if let Some(coord) = g.geo_coord() { + self.add_coord(coord) + } } + GeometryTypeExt::Line(g) => self.add_line(g), + GeometryTypeExt::LineString(g) => self.add_line_string(g), + GeometryTypeExt::Polygon(g) => self.add_polygon(g), + GeometryTypeExt::MultiPoint(g) => self.add_multi_point(g), + GeometryTypeExt::MultiLineString(g) => self.add_multi_line_string(g), + GeometryTypeExt::MultiPolygon(g) => self.add_multi_polygon(g), + GeometryTypeExt::Rect(g) => self.add_rect(g), + GeometryTypeExt::Triangle(g) => self.add_triangle(g), } - GeometryTypeExt::Line(g) => self.add_line(g), - GeometryTypeExt::LineString(g) => self.add_line_string(g), - GeometryTypeExt::Polygon(g) => self.add_polygon(g), - GeometryTypeExt::MultiPoint(g) => self.add_multi_point(g), - GeometryTypeExt::MultiLineString(g) => self.add_multi_line_string(g), - GeometryTypeExt::MultiPolygon(g) => self.add_multi_polygon(g), - GeometryTypeExt::GeometryCollection(g) => self.add_geometry_collection(g), - GeometryTypeExt::Rect(g) => self.add_rect(g), - GeometryTypeExt::Triangle(g) => self.add_triangle(g), } } diff --git a/geo-generic-alg/src/algorithm/coordinate_position.rs b/geo-generic-alg/src/algorithm/coordinate_position.rs index bd7a03d358..41310a7062 100644 --- a/geo-generic-alg/src/algorithm/coordinate_position.rs +++ b/geo-generic-alg/src/algorithm/coordinate_position.rs @@ -1,3 +1,4 @@ +use core::borrow::Borrow; use std::cmp::Ordering; use crate::geometry::*; @@ -429,6 +430,57 @@ where } } +fn geometry_calculate_coordinate_position( + g: &G, + coord: &Coord, + is_inside: &mut bool, + boundary_count: &mut usize, +) where + T: GeoNum, + G: GeometryTraitExt, +{ + if g.is_collection() { + for g_inner in g.geometries_ext() { + geometry_calculate_coordinate_position( + g_inner.borrow(), + coord, + is_inside, + boundary_count, + ); + } + } else { + match g.as_type_ext() { + GeometryTypeExt::Point(g) => { + g.calculate_coordinate_position_trait(coord, is_inside, boundary_count) + } + GeometryTypeExt::Line(g) => { + g.calculate_coordinate_position_trait(coord, is_inside, boundary_count) + } + GeometryTypeExt::LineString(g) => { + g.calculate_coordinate_position_trait(coord, is_inside, boundary_count) + } + GeometryTypeExt::Polygon(g) => { + g.calculate_coordinate_position_trait(coord, is_inside, boundary_count) + } + GeometryTypeExt::MultiPoint(g) => { + g.calculate_coordinate_position_trait(coord, is_inside, boundary_count) + } + GeometryTypeExt::MultiLineString(g) => { + g.calculate_coordinate_position_trait(coord, is_inside, boundary_count) + } + GeometryTypeExt::MultiPolygon(g) => { + g.calculate_coordinate_position_trait(coord, is_inside, boundary_count) + } + GeometryTypeExt::Rect(g) => { + g.calculate_coordinate_position_trait(coord, is_inside, boundary_count) + } + GeometryTypeExt::Triangle(g) => { + g.calculate_coordinate_position_trait(coord, is_inside, boundary_count) + } + } + } +} + impl CoordinatePositionTrait for G where T: GeoNum, @@ -436,12 +488,13 @@ where { type T = T; - crate::geometry_trait_ext_delegate_impl! { - fn calculate_coordinate_position_trait( - &self, - coord: &Coord, - is_inside: &mut bool, - boundary_count: &mut usize) -> (); + fn calculate_coordinate_position_trait( + &self, + coord: &Coord, + is_inside: &mut bool, + boundary_count: &mut usize, + ) { + geometry_calculate_coordinate_position(self, coord, is_inside, boundary_count); } } @@ -794,29 +847,46 @@ mod test { let triangle = Triangle::new((0.0, 0.0).into(), (5.0, 10.0).into(), (10.0, 0.0).into()); let rect = Rect::new((0.0, 0.0), (10.0, 10.0)); let collection = GeometryCollection::new_from(vec![triangle.into(), rect.into()]); + let geom = Geometry::GeometryCollection(collection.clone()); // outside of both assert_eq!( collection.coordinate_position(&coord! { x: 15.0, y: 15.0 }), CoordPos::Outside ); + assert_eq!( + geom.coordinate_position(&coord! { x: 15.0, y: 15.0 }), + CoordPos::Outside + ); // inside both assert_eq!( collection.coordinate_position(&coord! { x: 5.0, y: 5.0 }), CoordPos::Inside ); + assert_eq!( + geom.coordinate_position(&coord! { x: 5.0, y: 5.0 }), + CoordPos::Inside + ); // inside one, boundary of other assert_eq!( collection.coordinate_position(&coord! { x: 2.5, y: 5.0 }), CoordPos::OnBoundary ); + assert_eq!( + geom.coordinate_position(&coord! { x: 2.5, y: 5.0 }), + CoordPos::OnBoundary + ); // boundary of both assert_eq!( collection.coordinate_position(&coord! { x: 5.0, y: 10.0 }), CoordPos::Outside ); + assert_eq!( + geom.coordinate_position(&coord! { x: 5.0, y: 10.0 }), + CoordPos::Outside + ); } } diff --git a/geo-generic-alg/src/algorithm/coords_iter.rs b/geo-generic-alg/src/algorithm/coords_iter.rs index 5ffb82a4af..97b5d0df45 100644 --- a/geo-generic-alg/src/algorithm/coords_iter.rs +++ b/geo-generic-alg/src/algorithm/coords_iter.rs @@ -1003,68 +1003,101 @@ where type Scalar = T; fn coords_iter_trait(&self) -> Self::Iter<'_> { - match self.as_type_ext() { - GeometryTypeExt::Point(g) => GeometryTraitCoordsIter::Point(g.coords_iter_trait()), - GeometryTypeExt::Line(g) => GeometryTraitCoordsIter::Line(g.coords_iter_trait()), - GeometryTypeExt::LineString(g) => { - GeometryTraitCoordsIter::LineString(g.coords_iter_trait()) + if self.is_collection() { + // Boxing is likely necessary here due to heterogeneous nature + // and complexity of tracking state across different geometry types + // without significant code complexity or allocations anyway. + let mut all_coords: Vec> = Vec::new(); + for g in self.geometries_ext() { + all_coords.extend(g.borrow().coords_iter_trait()); } - GeometryTypeExt::Polygon(g) => GeometryTraitCoordsIter::Polygon(g.coords_iter_trait()), - GeometryTypeExt::MultiPoint(g) => { - GeometryTraitCoordsIter::MultiPoint(g.coords_iter_trait()) - } - GeometryTypeExt::MultiLineString(g) => { - GeometryTraitCoordsIter::MultiLineString(g.coords_iter_trait()) - } - GeometryTypeExt::MultiPolygon(g) => { - GeometryTraitCoordsIter::MultiPolygon(g.coords_iter_trait()) - } - GeometryTypeExt::GeometryCollection(g) => { - GeometryTraitCoordsIter::GeometryCollection(g.coords_iter_trait()) - } - GeometryTypeExt::Rect(g) => GeometryTraitCoordsIter::Rect(g.coords_iter_trait()), - GeometryTypeExt::Triangle(g) => { - GeometryTraitCoordsIter::Triangle(g.coords_iter_trait()) + let iter = all_coords.into_iter(); + GeometryTraitCoordsIter::GeometryCollection(iter) + } else { + match self.as_type_ext() { + GeometryTypeExt::Point(g) => GeometryTraitCoordsIter::Point(g.coords_iter_trait()), + GeometryTypeExt::Line(g) => GeometryTraitCoordsIter::Line(g.coords_iter_trait()), + GeometryTypeExt::LineString(g) => { + GeometryTraitCoordsIter::LineString(g.coords_iter_trait()) + } + GeometryTypeExt::Polygon(g) => { + GeometryTraitCoordsIter::Polygon(g.coords_iter_trait()) + } + GeometryTypeExt::MultiPoint(g) => { + GeometryTraitCoordsIter::MultiPoint(g.coords_iter_trait()) + } + GeometryTypeExt::MultiLineString(g) => { + GeometryTraitCoordsIter::MultiLineString(g.coords_iter_trait()) + } + GeometryTypeExt::MultiPolygon(g) => { + GeometryTraitCoordsIter::MultiPolygon(g.coords_iter_trait()) + } + GeometryTypeExt::Rect(g) => GeometryTraitCoordsIter::Rect(g.coords_iter_trait()), + GeometryTypeExt::Triangle(g) => { + GeometryTraitCoordsIter::Triangle(g.coords_iter_trait()) + } } } } - crate::geometry_trait_ext_delegate_impl! { - /// Return the number of coordinates in the `Geometry`. - fn coords_count_trait(&self) -> usize; + /// Return the number of coordinates in the `Geometry`. + fn coords_count_trait(&self) -> usize { + if self.is_collection() { + self.geometries_ext() + .map(|g_inner| g_inner.borrow().coords_count_trait()) + .sum() + } else { + match self.as_type_ext() { + GeometryTypeExt::Point(g) => g.coords_count_trait(), + GeometryTypeExt::Line(g) => g.coords_count_trait(), + GeometryTypeExt::LineString(g) => g.coords_count_trait(), + GeometryTypeExt::Polygon(g) => g.coords_count_trait(), + GeometryTypeExt::MultiPoint(g) => g.coords_count_trait(), + GeometryTypeExt::MultiLineString(g) => g.coords_count_trait(), + GeometryTypeExt::MultiPolygon(g) => g.coords_count_trait(), + GeometryTypeExt::Rect(g) => g.coords_count_trait(), + GeometryTypeExt::Triangle(g) => g.coords_count_trait(), + } + } } fn exterior_coords_iter_trait(&self) -> Self::ExteriorIter<'_> { - match self.as_type_ext() { - GeometryTypeExt::Point(g) => { - GeometryTraitExteriorCoordsIter::Point(g.exterior_coords_iter_trait()) - } - GeometryTypeExt::Line(g) => { - GeometryTraitExteriorCoordsIter::Line(g.exterior_coords_iter_trait()) - } - GeometryTypeExt::LineString(g) => { - GeometryTraitExteriorCoordsIter::LineString(g.exterior_coords_iter_trait()) - } - GeometryTypeExt::Polygon(g) => { - GeometryTraitExteriorCoordsIter::Polygon(g.exterior_coords_iter_trait()) - } - GeometryTypeExt::MultiPoint(g) => { - GeometryTraitExteriorCoordsIter::MultiPoint(g.exterior_coords_iter_trait()) + if self.is_collection() { + let mut all_coords: Vec> = Vec::new(); + for g in self.geometries_ext() { + all_coords.extend(g.borrow().exterior_coords_iter_trait()); } - GeometryTypeExt::MultiLineString(g) => { - GeometryTraitExteriorCoordsIter::MultiLineString(g.exterior_coords_iter_trait()) - } - GeometryTypeExt::MultiPolygon(g) => { - GeometryTraitExteriorCoordsIter::MultiPolygon(g.exterior_coords_iter_trait()) - } - GeometryTypeExt::GeometryCollection(g) => { - GeometryTraitExteriorCoordsIter::GeometryCollection(g.exterior_coords_iter_trait()) - } - GeometryTypeExt::Rect(g) => { - GeometryTraitExteriorCoordsIter::Rect(g.exterior_coords_iter_trait()) - } - GeometryTypeExt::Triangle(g) => { - GeometryTraitExteriorCoordsIter::Triangle(g.exterior_coords_iter_trait()) + let iter = all_coords.into_iter(); + GeometryTraitExteriorCoordsIter::GeometryCollection(iter) + } else { + match self.as_type_ext() { + GeometryTypeExt::Point(g) => { + GeometryTraitExteriorCoordsIter::Point(g.exterior_coords_iter_trait()) + } + GeometryTypeExt::Line(g) => { + GeometryTraitExteriorCoordsIter::Line(g.exterior_coords_iter_trait()) + } + GeometryTypeExt::LineString(g) => { + GeometryTraitExteriorCoordsIter::LineString(g.exterior_coords_iter_trait()) + } + GeometryTypeExt::Polygon(g) => { + GeometryTraitExteriorCoordsIter::Polygon(g.exterior_coords_iter_trait()) + } + GeometryTypeExt::MultiPoint(g) => { + GeometryTraitExteriorCoordsIter::MultiPoint(g.exterior_coords_iter_trait()) + } + GeometryTypeExt::MultiLineString(g) => { + GeometryTraitExteriorCoordsIter::MultiLineString(g.exterior_coords_iter_trait()) + } + GeometryTypeExt::MultiPolygon(g) => { + GeometryTraitExteriorCoordsIter::MultiPolygon(g.exterior_coords_iter_trait()) + } + GeometryTypeExt::Rect(g) => { + GeometryTraitExteriorCoordsIter::Rect(g.exterior_coords_iter_trait()) + } + GeometryTypeExt::Triangle(g) => { + GeometryTraitExteriorCoordsIter::Triangle(g.exterior_coords_iter_trait()) + } } } } @@ -1162,9 +1195,7 @@ where as CoordsIterTrait>::Iter<'a>, ), MultiPolygon( as CoordsIterTrait>::Iter<'a>), - GeometryCollection( - as CoordsIterTrait>::Iter<'a>, - ), + GeometryCollection(std::vec::IntoIter>), Rect( as CoordsIterTrait>::Iter<'a>), Triangle( as CoordsIterTrait>::Iter<'a>), } @@ -1223,11 +1254,7 @@ where MultiPolygon( as CoordsIterTrait>::ExteriorIter<'a>, ), - GeometryCollection( - as CoordsIterTrait>::ExteriorIter< - 'a, - >, - ), + GeometryCollection(std::vec::IntoIter>), Rect( as CoordsIterTrait>::ExteriorIter<'a>), Triangle( as CoordsIterTrait>::ExteriorIter<'a>), } @@ -1396,13 +1423,16 @@ mod test { let (polygon, mut coords) = create_polygon(); expected_coords.append(&mut coords); - let actual_coords = GeometryCollection::new_from(vec![ + let collection = GeometryCollection::new_from(vec![ Geometry::LineString(line_string), Geometry::Polygon(polygon), - ]) - .coords_iter() - .collect::>(); + ]); + let geom = Geometry::GeometryCollection(collection.clone()); + + let actual_coords = collection.coords_iter().collect::>(); + assert_eq!(expected_coords, actual_coords); + let actual_coords = geom.coords_iter().collect::>(); assert_eq!(expected_coords, actual_coords); } diff --git a/geo-generic-alg/src/algorithm/dimensions.rs b/geo-generic-alg/src/algorithm/dimensions.rs index 46d372ecd4..78f2cd1659 100644 --- a/geo-generic-alg/src/algorithm/dimensions.rs +++ b/geo-generic-alg/src/algorithm/dimensions.rs @@ -1,3 +1,4 @@ +use core::borrow::Borrow; use geo_traits_ext::*; use crate::Orientation::Collinear; @@ -166,10 +167,82 @@ impl HasDimensionsTrait for G where G: GeometryTraitExt, { - crate::geometry_trait_ext_delegate_impl! { - fn is_empty_trait(&self) -> bool; - fn dimensions_trait(&self) -> Dimensions; - fn boundary_dimensions_trait(&self) -> Dimensions; + fn is_empty_trait(&self) -> bool { + if self.is_collection() { + if self.num_geometries_ext() == 0 { + true + } else { + self.geometries_ext() + .all(|g_inner| g_inner.borrow().is_empty_trait()) + } + } else { + match self.as_type_ext() { + GeometryTypeExt::Point(g) => g.is_empty_trait(), + GeometryTypeExt::Line(g) => g.is_empty_trait(), + GeometryTypeExt::LineString(g) => g.is_empty_trait(), + GeometryTypeExt::Polygon(g) => g.is_empty_trait(), + GeometryTypeExt::MultiPoint(g) => g.is_empty_trait(), + GeometryTypeExt::MultiLineString(g) => g.is_empty_trait(), + GeometryTypeExt::MultiPolygon(g) => g.is_empty_trait(), + GeometryTypeExt::Rect(g) => g.is_empty_trait(), + GeometryTypeExt::Triangle(g) => g.is_empty_trait(), + } + } + } + + fn dimensions_trait(&self) -> Dimensions { + if self.is_collection() { + let mut max = Dimensions::Empty; + for geom in self.geometries_ext() { + let dimensions = geom.borrow().dimensions_trait(); + if dimensions == Dimensions::TwoDimensional { + // short-circuit since we know none can be larger + return Dimensions::TwoDimensional; + } + max = max.max(dimensions); + } + max + } else { + match self.as_type_ext() { + GeometryTypeExt::Point(g) => g.dimensions_trait(), + GeometryTypeExt::Line(g) => g.dimensions_trait(), + GeometryTypeExt::LineString(g) => g.dimensions_trait(), + GeometryTypeExt::Polygon(g) => g.dimensions_trait(), + GeometryTypeExt::MultiPoint(g) => g.dimensions_trait(), + GeometryTypeExt::MultiLineString(g) => g.dimensions_trait(), + GeometryTypeExt::MultiPolygon(g) => g.dimensions_trait(), + GeometryTypeExt::Rect(g) => g.dimensions_trait(), + GeometryTypeExt::Triangle(g) => g.dimensions_trait(), + } + } + } + + fn boundary_dimensions_trait(&self) -> Dimensions { + if self.is_collection() { + let mut max = Dimensions::Empty; + for geom in self.geometries_ext() { + let d = geom.borrow().boundary_dimensions_trait(); + + if d == Dimensions::OneDimensional { + return Dimensions::OneDimensional; + } + + max = max.max(d); + } + max + } else { + match self.as_type_ext() { + GeometryTypeExt::Point(g) => g.boundary_dimensions_trait(), + GeometryTypeExt::Line(g) => g.boundary_dimensions_trait(), + GeometryTypeExt::LineString(g) => g.boundary_dimensions_trait(), + GeometryTypeExt::Polygon(g) => g.boundary_dimensions_trait(), + GeometryTypeExt::MultiPoint(g) => g.boundary_dimensions_trait(), + GeometryTypeExt::MultiLineString(g) => g.boundary_dimensions_trait(), + GeometryTypeExt::MultiPolygon(g) => g.boundary_dimensions_trait(), + GeometryTypeExt::Rect(g) => g.boundary_dimensions_trait(), + GeometryTypeExt::Triangle(g) => g.boundary_dimensions_trait(), + } + } } } @@ -415,7 +488,7 @@ where // short-circuit since we know none can be larger return Dimensions::TwoDimensional; } - max = max.max(dimensions) + max = max.max(dimensions); } max } diff --git a/geo-generic-alg/src/algorithm/euclidean_length.rs b/geo-generic-alg/src/algorithm/euclidean_length.rs index 4640289dd2..4c83232729 100644 --- a/geo-generic-alg/src/algorithm/euclidean_length.rs +++ b/geo-generic-alg/src/algorithm/euclidean_length.rs @@ -1,3 +1,4 @@ +use core::borrow::Borrow; use std::iter::Sum; use crate::CoordFloat; @@ -169,18 +170,7 @@ where // Linear geometries (lines, linestrings) will contribute their actual length // Non-linear geometries (points, polygons) will contribute zero self.geometries_ext() - .map(|g| match g.as_type_ext() { - GeometryTypeExt::Point(_) => T::zero(), - GeometryTypeExt::Line(line) => line.euclidean_length_trait(), - GeometryTypeExt::LineString(ls) => ls.euclidean_length_trait(), - GeometryTypeExt::Polygon(_) => T::zero(), - GeometryTypeExt::MultiPoint(_) => T::zero(), - GeometryTypeExt::MultiLineString(mls) => mls.euclidean_length_trait(), - GeometryTypeExt::MultiPolygon(_) => T::zero(), - GeometryTypeExt::GeometryCollection(gc) => gc.euclidean_length_trait(), - GeometryTypeExt::Rect(_) => T::zero(), - GeometryTypeExt::Triangle(_) => T::zero(), - }) + .map(|g| g.euclidean_length_trait()) .fold(T::zero(), |acc, next| acc + next) } } @@ -190,8 +180,24 @@ impl> EuclideanLengthTrait for G where T: CoordFloat + Sum, { - crate::geometry_trait_ext_delegate_impl! { - fn euclidean_length_trait(&self) -> T; + fn euclidean_length_trait(&self) -> T { + if self.is_collection() { + self.geometries_ext() + .map(|g_inner| g_inner.borrow().euclidean_length_trait()) + .fold(T::zero(), |acc, next| acc + next) + } else { + match self.as_type_ext() { + GeometryTypeExt::Point(_) => T::zero(), + GeometryTypeExt::Line(line) => line.euclidean_length_trait(), + GeometryTypeExt::LineString(ls) => ls.euclidean_length_trait(), + GeometryTypeExt::Polygon(_) => T::zero(), + GeometryTypeExt::MultiPoint(_) => T::zero(), + GeometryTypeExt::MultiLineString(mls) => mls.euclidean_length_trait(), + GeometryTypeExt::MultiPolygon(_) => T::zero(), + GeometryTypeExt::Rect(_) => T::zero(), + GeometryTypeExt::Triangle(_) => T::zero(), + } + } } } @@ -371,6 +377,11 @@ mod test { 2.8284271247461903, epsilon = 1e-10 ); + assert_relative_eq!( + Geometry::GeometryCollection(collection).euclidean_length(), + 2.8284271247461903, + epsilon = 1e-10 + ); } // Individual test functions matching pytest parametrized scenarios diff --git a/geo-generic-alg/src/algorithm/intersects/collections.rs b/geo-generic-alg/src/algorithm/intersects/collections.rs index 7114865469..26103e8baa 100644 --- a/geo-generic-alg/src/algorithm/intersects/collections.rs +++ b/geo-generic-alg/src/algorithm/intersects/collections.rs @@ -1,8 +1,8 @@ +use core::borrow::Borrow; use geo_traits_ext::*; use super::has_disjoint_bboxes; use super::IntersectsTrait; -use crate::geometry_trait_ext_delegate_impl; use crate::GeoNum; macro_rules! impl_intersects_geometry { @@ -13,8 +13,23 @@ macro_rules! impl_intersects_geometry { LHS: GeometryTraitExt, RHS: $rhs_type, { - geometry_trait_ext_delegate_impl! { - fn intersects_trait(&self, rhs: &RHS) -> bool; + fn intersects_trait(&self, rhs: &RHS) -> bool { + if self.is_collection() { + self.geometries_ext() + .any(|lhs_inner| lhs_inner.borrow().intersects_trait(rhs)) + } else { + match self.as_type_ext() { + GeometryTypeExt::Point(g) => g.intersects_trait(rhs), + GeometryTypeExt::Line(g) => g.intersects_trait(rhs), + GeometryTypeExt::LineString(g) => g.intersects_trait(rhs), + GeometryTypeExt::Polygon(g) => g.intersects_trait(rhs), + GeometryTypeExt::MultiPoint(g) => g.intersects_trait(rhs), + GeometryTypeExt::MultiLineString(g) => g.intersects_trait(rhs), + GeometryTypeExt::MultiPolygon(g) => g.intersects_trait(rhs), + GeometryTypeExt::Rect(g) => g.intersects_trait(rhs), + GeometryTypeExt::Triangle(g) => g.intersects_trait(rhs), + } + } } } }; diff --git a/geo-generic-alg/src/algorithm/intersects/mod.rs b/geo-generic-alg/src/algorithm/intersects/mod.rs index 45a71b21a6..bae78135e6 100644 --- a/geo-generic-alg/src/algorithm/intersects/mod.rs +++ b/geo-generic-alg/src/algorithm/intersects/mod.rs @@ -136,7 +136,7 @@ where #[cfg(test)] mod test { - use geo_types::Coord; + use geo_types::{Coord, GeometryCollection}; use crate::Intersects; use crate::{ @@ -549,6 +549,26 @@ mod test { assert!(b.intersects(&a)); } + #[test] + fn test_geom_collection_collection() { + let collection0 = Geometry::GeometryCollection(GeometryCollection::new_from(vec![ + Geometry::Point(Point::new(0., 0.)), + Geometry::Point(Point::new(1., 1.)), + ])); + let collection1 = Geometry::GeometryCollection(GeometryCollection::new_from(vec![ + Geometry::Point(Point::new(0., 0.)), + Geometry::Point(Point::new(2., 2.)), + ])); + let collection2 = Geometry::GeometryCollection(GeometryCollection::new_from(vec![ + Geometry::Point(Point::new(3., 3.)), + Geometry::Point(Point::new(4., 4.)), + ])); + assert!(collection0.intersects(&collection1)); + assert!(collection1.intersects(&collection0)); + assert!(!collection0.intersects(&collection2)); + assert!(!collection2.intersects(&collection0)); + } + #[test] fn compile_test_geom_geom() { // This test should check existence of all diff --git a/geo-generic-alg/src/algorithm/line_measures/length.rs b/geo-generic-alg/src/algorithm/line_measures/length.rs index 28ee07460f..41d08ef452 100644 --- a/geo-generic-alg/src/algorithm/line_measures/length.rs +++ b/geo-generic-alg/src/algorithm/line_measures/length.rs @@ -2,6 +2,7 @@ use super::Distance; use crate::{CoordFloat, Line, LineString, MultiLineString, Point}; use geo_traits::{CoordTrait, PolygonTrait}; use geo_traits_ext::*; +use std::borrow::Borrow; /// Calculate the length of a geometry using a given [metric space](crate::algorithm::line_measures::metric_spaces). /// @@ -390,7 +391,28 @@ where { fn length_trait(&self, metric_space: &impl Distance, Point>) -> F { self.geometries_ext() - .map(|g| match g.as_type_ext() { + .map(|g| g.borrow().length_trait(metric_space)) + .fold(F::zero(), |acc, next| acc + next) + } + + fn perimeter_trait(&self, metric_space: &impl Distance, Point>) -> F { + self.geometries_ext() + .map(|g| g.borrow().perimeter_trait(metric_space)) + .fold(F::zero(), |acc, next| acc + next) + } +} + +impl> LengthMeasurableTrait for G +where + F: CoordFloat, +{ + fn length_trait(&self, metric_space: &impl Distance, Point>) -> F { + if self.is_collection() { + self.geometries_ext() + .map(|g_inner| g_inner.borrow().length_trait(metric_space)) + .fold(F::zero(), |acc, next| acc + next) + } else { + match self.as_type_ext() { GeometryTypeExt::Point(_) => F::zero(), GeometryTypeExt::Line(line) => line.length_trait(metric_space), GeometryTypeExt::LineString(ls) => ls.length_trait(metric_space), @@ -398,16 +420,19 @@ where GeometryTypeExt::MultiPoint(_) => F::zero(), GeometryTypeExt::MultiLineString(mls) => mls.length_trait(metric_space), GeometryTypeExt::MultiPolygon(_) => F::zero(), - GeometryTypeExt::GeometryCollection(gc) => gc.length_trait(metric_space), GeometryTypeExt::Rect(_) => F::zero(), GeometryTypeExt::Triangle(_) => F::zero(), - }) - .fold(F::zero(), |acc, next| acc + next) + } + } } fn perimeter_trait(&self, metric_space: &impl Distance, Point>) -> F { - self.geometries_ext() - .map(|g| match g.as_type_ext() { + if self.is_collection() { + self.geometries_ext() + .map(|g_inner| g_inner.borrow().perimeter_trait(metric_space)) + .fold(F::zero(), |acc, next| acc + next) + } else { + match self.as_type_ext() { GeometryTypeExt::Point(_) => F::zero(), GeometryTypeExt::Line(_) => F::zero(), // 1D geometry - no perimeter GeometryTypeExt::LineString(_) => F::zero(), // 1D geometry - no perimeter @@ -415,30 +440,10 @@ where GeometryTypeExt::MultiPoint(_) => F::zero(), GeometryTypeExt::MultiLineString(_) => F::zero(), // 1D geometry - no perimeter GeometryTypeExt::MultiPolygon(mp) => mp.perimeter_trait(metric_space), - GeometryTypeExt::GeometryCollection(gc) => gc.perimeter_trait(metric_space), GeometryTypeExt::Rect(rect) => rect.perimeter_trait(metric_space), GeometryTypeExt::Triangle(triangle) => triangle.perimeter_trait(metric_space), - }) - .fold(F::zero(), |acc, next| acc + next) - } -} - -impl> LengthMeasurableTrait for G -where - F: CoordFloat, -{ - // This macro delegates the `length_trait` method to the appropriate geometry variant. - // It is critical for WKB (Well-Known Binary) compatibility, ensuring that trait methods - // are correctly dispatched for all geometry types when deserializing from WKB. - crate::geometry_trait_ext_delegate_impl! { - fn length_trait(&self, metric_space: &impl Distance, Point>) -> F; - } - - // This macro delegates the `perimeter_trait` method to the appropriate geometry variant. - // It is critical for WKB (Well-Known Binary) compatibility, ensuring that trait methods - // are correctly dispatched for all geometry types when deserializing from WKB. - crate::geometry_trait_ext_delegate_impl! { - fn perimeter_trait(&self, metric_space: &impl Distance, Point>) -> F; + } + } } } @@ -688,6 +693,13 @@ mod tests { 2.8284271247461903, // 2*sqrt(2) only from linestrings epsilon = 1e-10 ); + + // GEOMETRY representation of GEOMETRYCOLLECTION + assert_relative_eq!( + Geometry::GeometryCollection(collection.clone()).length_ext(&Euclidean), + 2.8284271247461903, // 2*sqrt(2) only from linestrings + epsilon = 1e-10 + ); } #[test] @@ -776,6 +788,13 @@ mod tests { 4.0, // only polygon perimeter counts epsilon = 1e-10 ); + + // GEOMETRY representation of GEOMETRYCOLLECTION + assert_relative_eq!( + Geometry::GeometryCollection(collection).perimeter_ext(&Euclidean), + 4.0, // only polygon perimeter counts + epsilon = 1e-10 + ); } #[test] diff --git a/geo-generic-alg/src/algorithm/line_measures/metric_spaces/euclidean/distance.rs b/geo-generic-alg/src/algorithm/line_measures/metric_spaces/euclidean/distance.rs index f0301c3aa6..9133447fc8 100644 --- a/geo-generic-alg/src/algorithm/line_measures/metric_spaces/euclidean/distance.rs +++ b/geo-generic-alg/src/algorithm/line_measures/metric_spaces/euclidean/distance.rs @@ -2,8 +2,8 @@ use super::{Distance, Euclidean}; use crate::algorithm::Intersects; use crate::geometry::*; use crate::{Coord, CoordFloat, GeoFloat, Point}; -use geo_traits::to_geo::ToGeoGeometry; use num_traits::{Bounded, Float}; +use std::borrow::Borrow; // Import all the utility functions from utils module use super::utils::{ @@ -391,12 +391,13 @@ pub trait DistanceExt { macro_rules! impl_distance_ext_for_polygonlike_geometry_trait { ($polygonlike_trait:ident, $polygonlike_tag:ident, [$(($geometry_trait:ident, $geometry_tag:ident)),*]) => { // Self-to-self distance implementation - impl> - GenericDistanceTrait for P + impl GenericDistanceTrait for LHS where F: GeoFloat, + LHS: $polygonlike_trait, + RHS: $polygonlike_trait, { - fn generic_distance_trait(&self, rhs: &P) -> F { + fn generic_distance_trait(&self, rhs: &RHS) -> F { let poly1 = self.to_polygon(); let poly2 = rhs.to_polygon(); distance_polygon_to_polygon_generic(&poly1, &poly2) @@ -494,11 +495,13 @@ macro_rules! impl_polygonlike_to_geometry_distance { /// Follows the same pattern as impl_euclidean_distance_for_iter_geometry! macro_rules! impl_distance_ext_for_iter_geometry_trait { ($iter_trait:ident, $iter_tag:ident, $member_method:ident) => { - impl> GenericDistanceTrait for I + impl GenericDistanceTrait for LHS where F: GeoFloat, + LHS: $iter_trait, + RHS: $iter_trait, { - fn generic_distance_trait(&self, rhs: &I) -> F { + fn generic_distance_trait(&self, rhs: &RHS) -> F { let mut min_dist: F = Float::max_value(); for member1 in self.$member_method() { for member2 in rhs.$member_method() { @@ -664,11 +667,13 @@ where // └────────────────────────────────────────────────────────────┘ // Point-to-Point direct distance implementation -impl> GenericDistanceTrait for P +impl GenericDistanceTrait for LHS where F: GeoFloat, + LHS: PointTraitExt, + RHS: PointTraitExt, { - fn generic_distance_trait(&self, rhs: &P) -> F { + fn generic_distance_trait(&self, rhs: &RHS) -> F { distance_point_to_point_generic(self, rhs) } } @@ -747,11 +752,13 @@ symmetric_distance_ext_trait_impl!(GeoFloat, LineTraitExt, LineTag, CoordTraitEx symmetric_distance_ext_trait_impl!(GeoFloat, LineTraitExt, LineTag, PointTraitExt, PointTag); // Line-to-Line direct distance implementation -impl> GenericDistanceTrait for L +impl GenericDistanceTrait for LHS where F: GeoFloat, + LHS: LineTraitExt, + RHS: LineTraitExt, { - fn generic_distance_trait(&self, rhs: &L) -> F { + fn generic_distance_trait(&self, rhs: &RHS) -> F { distance_line_to_line_generic(self, rhs) } } @@ -1140,7 +1147,7 @@ impl_distance_geometry_collection_from_geometry!(MultiLineStringTraitExt, MultiL impl_distance_geometry_collection_from_geometry!(MultiPolygonTraitExt, MultiPolygonTag); impl_distance_geometry_collection_from_geometry!(RectTraitExt, RectTag); impl_distance_geometry_collection_from_geometry!(TriangleTraitExt, TriangleTag); -// Manual implementation for GeometryCollection to GeometryCollection to avoid infinite recursion + impl GenericDistanceTrait for LHS where F: GeoFloat, @@ -1148,17 +1155,10 @@ where RHS: GeometryCollectionTraitExt, { fn generic_distance_trait(&self, rhs: &RHS) -> F { - use num_traits::Bounded; - let mut min_distance = ::max_value(); - for lhs_geom in self.geometries_ext() { for rhs_geom in rhs.geometries_ext() { - // Convert to concrete types for this specific case only - // This avoids the trait bound complexity while still using generic traits everywhere else - let lhs_concrete = lhs_geom.to_geometry(); - let rhs_concrete = rhs_geom.to_geometry(); - let distance = Euclidean.distance(&lhs_concrete, &rhs_concrete); + let distance = lhs_geom.distance_ext(&rhs_geom); min_distance = min_distance.min(distance); // Early exit optimization @@ -1250,7 +1250,44 @@ macro_rules! impl_distance_geometry_to_type { RHS: $rhs_type, { fn generic_distance_trait(&self, rhs: &RHS) -> F { - self.distance_ext(rhs) + if self.is_collection() { + let mut min_distance = ::max_value(); + for lhs_geom in self.geometries_ext() { + let lhs_geom = lhs_geom.borrow(); + let distance = lhs_geom.generic_distance_trait(rhs); + min_distance = min_distance.min(distance); + + // Early exit optimization + if distance == F::zero() { + return F::zero(); + } + } + min_distance + } else { + match self.as_type_ext() { + geo_traits_ext::GeometryTypeExt::Point(g) => g.generic_distance_trait(rhs), + geo_traits_ext::GeometryTypeExt::Line(g) => g.generic_distance_trait(rhs), + geo_traits_ext::GeometryTypeExt::LineString(g) => { + g.generic_distance_trait(rhs) + } + geo_traits_ext::GeometryTypeExt::Polygon(g) => { + g.generic_distance_trait(rhs) + } + geo_traits_ext::GeometryTypeExt::MultiPoint(g) => { + g.generic_distance_trait(rhs) + } + geo_traits_ext::GeometryTypeExt::MultiLineString(g) => { + g.generic_distance_trait(rhs) + } + geo_traits_ext::GeometryTypeExt::MultiPolygon(g) => { + g.generic_distance_trait(rhs) + } + geo_traits_ext::GeometryTypeExt::Rect(g) => g.generic_distance_trait(rhs), + geo_traits_ext::GeometryTypeExt::Triangle(g) => { + g.generic_distance_trait(rhs) + } + } + } } } }; @@ -1267,44 +1304,112 @@ impl_distance_geometry_to_type!(RectTraitExt, RectTag); impl_distance_geometry_to_type!(TriangleTraitExt, TriangleTag); impl_distance_geometry_to_type!(GeometryCollectionTraitExt, GeometryCollectionTag); -impl> GenericDistanceTrait for G +symmetric_distance_ext_trait_impl!( + GeoFloat, + PointTraitExt, + PointTag, + GeometryTraitExt, + GeometryTag +); +symmetric_distance_ext_trait_impl!( + GeoFloat, + LineTraitExt, + LineTag, + GeometryTraitExt, + GeometryTag +); +symmetric_distance_ext_trait_impl!( + GeoFloat, + LineStringTraitExt, + LineStringTag, + GeometryTraitExt, + GeometryTag +); +symmetric_distance_ext_trait_impl!( + GeoFloat, + PolygonTraitExt, + PolygonTag, + GeometryTraitExt, + GeometryTag +); +symmetric_distance_ext_trait_impl!( + GeoFloat, + MultiPointTraitExt, + MultiPointTag, + GeometryTraitExt, + GeometryTag +); +symmetric_distance_ext_trait_impl!( + GeoFloat, + MultiLineStringTraitExt, + MultiLineStringTag, + GeometryTraitExt, + GeometryTag +); +symmetric_distance_ext_trait_impl!( + GeoFloat, + MultiPolygonTraitExt, + MultiPolygonTag, + GeometryTraitExt, + GeometryTag +); +symmetric_distance_ext_trait_impl!( + GeoFloat, + RectTraitExt, + RectTag, + GeometryTraitExt, + GeometryTag +); +symmetric_distance_ext_trait_impl!( + GeoFloat, + TriangleTraitExt, + TriangleTag, + GeometryTraitExt, + GeometryTag +); +symmetric_distance_ext_trait_impl!( + GeoFloat, + GeometryCollectionTraitExt, + GeometryCollectionTag, + GeometryTraitExt, + GeometryTag +); + +impl GenericDistanceTrait for LHS where F: GeoFloat, + LHS: GeometryTraitExt, + RHS: GeometryTraitExt, { - fn generic_distance_trait(&self, rhs: &G) -> F { - use geo_traits_ext::GeometryTypeExt; - - // This macro generates the entire match statement - macro_rules! generate_distance_match { - ($($left:ident => [$($right:ident),+]),+ $(,)?) => { - match (self.as_type_ext(), rhs.as_type_ext()) { - $($( - (GeometryTypeExt::$left(left), GeometryTypeExt::$right(right)) => { - left.distance_ext(right) - }, - )+)+ - _ => { - let g1 = self.to_geometry(); - let g2 = rhs.to_geometry(); - Euclidean.distance(&g1, &g2) - } + fn generic_distance_trait(&self, rhs: &RHS) -> F { + if self.is_collection() { + let mut min_distance = ::max_value(); + for lhs_geom in self.geometries_ext() { + let lhs_geom = lhs_geom.borrow(); + let distance = lhs_geom.generic_distance_trait(rhs); + min_distance = min_distance.min(distance); + + // Early exit optimization + if distance == F::zero() { + return F::zero(); } - }; + } + min_distance + } else { + match self.as_type_ext() { + geo_traits_ext::GeometryTypeExt::Point(g) => g.generic_distance_trait(rhs), + geo_traits_ext::GeometryTypeExt::Line(g) => g.generic_distance_trait(rhs), + geo_traits_ext::GeometryTypeExt::LineString(g) => g.generic_distance_trait(rhs), + geo_traits_ext::GeometryTypeExt::Polygon(g) => g.generic_distance_trait(rhs), + geo_traits_ext::GeometryTypeExt::MultiPoint(g) => g.generic_distance_trait(rhs), + geo_traits_ext::GeometryTypeExt::MultiLineString(g) => { + g.generic_distance_trait(rhs) + } + geo_traits_ext::GeometryTypeExt::MultiPolygon(g) => g.generic_distance_trait(rhs), + geo_traits_ext::GeometryTypeExt::Rect(g) => g.generic_distance_trait(rhs), + geo_traits_ext::GeometryTypeExt::Triangle(g) => g.generic_distance_trait(rhs), + } } - - // Generate the match with explicit left => [right types] mappings - generate_distance_match!( - Point => [Point, Line, LineString, Polygon, Triangle, Rect, MultiPoint, MultiLineString, MultiPolygon, GeometryCollection], - Line => [Point, Line, LineString, Polygon, Triangle, Rect, MultiPoint, MultiLineString, MultiPolygon, GeometryCollection], - LineString => [Point, Line, LineString, Polygon, Triangle, Rect, MultiPoint, MultiLineString, MultiPolygon, GeometryCollection], - Polygon => [Point, Line, LineString, Polygon, Triangle, Rect, MultiPoint, MultiLineString, MultiPolygon, GeometryCollection], - Triangle => [Point, Line, LineString, Polygon, Triangle, Rect, MultiPoint, MultiLineString, MultiPolygon, GeometryCollection], - Rect => [Point, Line, LineString, Polygon, Triangle, Rect, MultiPoint, MultiLineString, MultiPolygon, GeometryCollection], - MultiPoint => [Point, Line, LineString, Polygon, Triangle, Rect, MultiPoint, MultiLineString, MultiPolygon, GeometryCollection], - MultiLineString => [Point, Line, LineString, Polygon, Triangle, Rect, MultiPoint, MultiLineString, MultiPolygon, GeometryCollection], - MultiPolygon => [Point, Line, LineString, Polygon, Triangle, Rect, MultiPoint, MultiLineString, MultiPolygon, GeometryCollection], - GeometryCollection => [Point, Line, LineString, Polygon, Triangle, Rect, MultiPoint, MultiLineString, MultiPolygon], - ) } } @@ -2445,7 +2550,6 @@ mod tests { #[test] fn test_original_issue_verification() { - // This test verifies the fix for the segmentation fault issue reported in SedonaDB: let point = Point::new(0.0, 0.0); let linestring = LineString::from(vec![(0.0, 0.0), (1.0, 1.0)]); @@ -2459,12 +2563,6 @@ mod tests { Geometry::LineString(linestring), ]); - // Before our fix: This would cause infinite recursion because: - // 1. GeometryCollection calls distance_ext on each geometry - // 2. When geometry is another GeometryCollection, it calls distance_ext again - // 3. This triggers the same GeometryCollection implementation → infinite recursion - // 4. Stack overflow → segmentation fault - // Test the concrete Distance API let distance = Euclidean.distance(&gc1, &gc2); assert_eq!( @@ -2472,7 +2570,7 @@ mod tests { "Distance between identical GeometryCollections should be 0" ); - // Test the generic distance_ext API directly (this should trigger the problematic path) + // Test the generic distance_ext API directly use crate::line_measures::DistanceExt; let distance_ext = gc1.distance_ext(&gc2); assert_eq!(distance_ext, 0.0, "Generic distance should also be 0"); @@ -2493,9 +2591,19 @@ mod tests { Geometry::LineString(linestring), ]); - // Force the generic trait path by calling distance_ext on trait objects let distance_result = gc1.distance_ext(&gc2); assert_eq!(distance_result, 0.0); + + let geom_gc1 = Geometry::GeometryCollection(gc1.clone()); + let geom_gc2 = Geometry::GeometryCollection(gc2.clone()); + let distance_result = geom_gc1.distance_ext(&geom_gc2); + assert_eq!(distance_result, 0.0); + + let distance_result = geom_gc1.distance_ext(&gc2); + assert_eq!(distance_result, 0.0); + + let distance_result = gc1.distance_ext(&geom_gc2); + assert_eq!(distance_result, 0.0); } } } diff --git a/geo-generic-alg/src/algorithm/map_coords.rs b/geo-generic-alg/src/algorithm/map_coords.rs index 1f37eacff3..68624825bd 100644 --- a/geo-generic-alg/src/algorithm/map_coords.rs +++ b/geo-generic-alg/src/algorithm/map_coords.rs @@ -27,6 +27,7 @@ pub(crate) use crate::geometry::*; pub(crate) use crate::CoordNum; +use core::borrow::Borrow; use geo_traits_ext::*; use Coord; @@ -590,21 +591,29 @@ where type Output = Geometry; fn map_coords_trait(&self, func: impl Fn(Coord) -> Coord + Copy) -> Self::Output { - match self.as_type_ext() { - GeometryTypeExt::Point(x) => Geometry::Point(x.map_coords_trait(func)), - GeometryTypeExt::Line(x) => Geometry::Line(x.map_coords_trait(func)), - GeometryTypeExt::LineString(x) => Geometry::LineString(x.map_coords_trait(func)), - GeometryTypeExt::Polygon(x) => Geometry::Polygon(x.map_coords_trait(func)), - GeometryTypeExt::MultiPoint(x) => Geometry::MultiPoint(x.map_coords_trait(func)), - GeometryTypeExt::MultiLineString(x) => { - Geometry::MultiLineString(x.map_coords_trait(func)) - } - GeometryTypeExt::MultiPolygon(x) => Geometry::MultiPolygon(x.map_coords_trait(func)), - GeometryTypeExt::GeometryCollection(x) => { - Geometry::GeometryCollection(x.map_coords_trait(func)) + if self.is_collection() { + let collection = GeometryCollection::new_from( + self.geometries_ext() + .map(|g| g.borrow().map_coords(func)) + .collect(), + ); + Geometry::GeometryCollection(collection) + } else { + match self.as_type_ext() { + GeometryTypeExt::Point(x) => Geometry::Point(x.map_coords_trait(func)), + GeometryTypeExt::Line(x) => Geometry::Line(x.map_coords_trait(func)), + GeometryTypeExt::LineString(x) => Geometry::LineString(x.map_coords_trait(func)), + GeometryTypeExt::Polygon(x) => Geometry::Polygon(x.map_coords_trait(func)), + GeometryTypeExt::MultiPoint(x) => Geometry::MultiPoint(x.map_coords_trait(func)), + GeometryTypeExt::MultiLineString(x) => { + Geometry::MultiLineString(x.map_coords_trait(func)) + } + GeometryTypeExt::MultiPolygon(x) => { + Geometry::MultiPolygon(x.map_coords_trait(func)) + } + GeometryTypeExt::Rect(x) => Geometry::Rect(x.map_coords_trait(func)), + GeometryTypeExt::Triangle(x) => Geometry::Triangle(x.map_coords_trait(func)), } - GeometryTypeExt::Rect(x) => Geometry::Rect(x.map_coords_trait(func)), - GeometryTypeExt::Triangle(x) => Geometry::Triangle(x.map_coords_trait(func)), } } @@ -612,27 +621,35 @@ where &self, func: impl Fn(Coord) -> Result, E> + Copy, ) -> Result { - match self.as_type_ext() { - GeometryTypeExt::Point(x) => Ok(Geometry::Point(x.try_map_coords_trait(func)?)), - GeometryTypeExt::Line(x) => Ok(Geometry::Line(x.try_map_coords_trait(func)?)), - GeometryTypeExt::LineString(x) => { - Ok(Geometry::LineString(x.try_map_coords_trait(func)?)) - } - GeometryTypeExt::Polygon(x) => Ok(Geometry::Polygon(x.try_map_coords_trait(func)?)), - GeometryTypeExt::MultiPoint(x) => { - Ok(Geometry::MultiPoint(x.try_map_coords_trait(func)?)) - } - GeometryTypeExt::MultiLineString(x) => { - Ok(Geometry::MultiLineString(x.try_map_coords_trait(func)?)) - } - GeometryTypeExt::MultiPolygon(x) => { - Ok(Geometry::MultiPolygon(x.try_map_coords_trait(func)?)) - } - GeometryTypeExt::GeometryCollection(x) => { - Ok(Geometry::GeometryCollection(x.try_map_coords_trait(func)?)) + if self.is_collection() { + let geoms = self + .geometries_ext() + .map(|g| g.borrow().try_map_coords(func)) + .collect::, E>>()?; + let collection = GeometryCollection::new_from(geoms); + Ok(Geometry::GeometryCollection(collection)) + } else { + match self.as_type_ext() { + GeometryTypeExt::Point(x) => Ok(Geometry::Point(x.try_map_coords_trait(func)?)), + GeometryTypeExt::Line(x) => Ok(Geometry::Line(x.try_map_coords_trait(func)?)), + GeometryTypeExt::LineString(x) => { + Ok(Geometry::LineString(x.try_map_coords_trait(func)?)) + } + GeometryTypeExt::Polygon(x) => Ok(Geometry::Polygon(x.try_map_coords_trait(func)?)), + GeometryTypeExt::MultiPoint(x) => { + Ok(Geometry::MultiPoint(x.try_map_coords_trait(func)?)) + } + GeometryTypeExt::MultiLineString(x) => { + Ok(Geometry::MultiLineString(x.try_map_coords_trait(func)?)) + } + GeometryTypeExt::MultiPolygon(x) => { + Ok(Geometry::MultiPolygon(x.try_map_coords_trait(func)?)) + } + GeometryTypeExt::Rect(x) => Ok(Geometry::Rect(x.try_map_coords_trait(func)?)), + GeometryTypeExt::Triangle(x) => { + Ok(Geometry::Triangle(x.try_map_coords_trait(func)?)) + } } - GeometryTypeExt::Rect(x) => Ok(Geometry::Rect(x.try_map_coords_trait(func)?)), - GeometryTypeExt::Triangle(x) => Ok(Geometry::Triangle(x.try_map_coords_trait(func)?)), } } } @@ -1042,13 +1059,19 @@ mod test { let line1 = Geometry::LineString(LineString::from(vec![(0., 0.), (1., 2.)])); let gc = GeometryCollection::new_from(vec![p1, line1]); + let expected = GeometryCollection::new_from(vec![ + Geometry::Point(Point::new(20., 110.)), + Geometry::LineString(LineString::from(vec![(10., 100.), (11., 102.)])), + ]); assert_eq!( gc.map_coords(|Coord { x, y }| (x + 10., y + 100.).into()), - GeometryCollection::new_from(vec![ - Geometry::Point(Point::new(20., 110.)), - Geometry::LineString(LineString::from(vec![(10., 100.), (11., 102.)])), - ]) + expected + ); + assert_eq!( + Geometry::GeometryCollection(gc) + .map_coords(|Coord { x, y }| (x + 10., y + 100.).into()), + Geometry::GeometryCollection(expected) ); } diff --git a/geo-generic-alg/src/types.rs b/geo-generic-alg/src/types.rs index d7a905645f..affda72acc 100644 --- a/geo-generic-alg/src/types.rs +++ b/geo-generic-alg/src/types.rs @@ -156,38 +156,3 @@ macro_rules! __geometry_delegate_impl_helper { )+ }; } - -#[macro_export] -macro_rules! geometry_trait_ext_delegate_impl { - ($($a:tt)*) => { $crate::__geometry_trait_ext_delegate_impl_helper!{ GeometryTypeExt, $($a)* } } -} - -#[doc(hidden)] -#[macro_export] -macro_rules! __geometry_trait_ext_delegate_impl_helper { - ( - $enum:ident, - $( - $(#[$outer:meta])* - fn $func_name: ident(&$($self_life:lifetime)?self $(, $arg_name: ident: $arg_type: ty)*) -> $return: ty; - )+ - ) => { - $( - $(#[$outer])* - fn $func_name(&$($self_life)? self, $($arg_name: $arg_type),*) -> $return { - match self.as_type_ext() { - $enum::Point(g) => g.$func_name($($arg_name),*).into(), - $enum::Line(g) => g.$func_name($($arg_name),*).into(), - $enum::LineString(g) => g.$func_name($($arg_name),*).into(), - $enum::Polygon(g) => g.$func_name($($arg_name),*).into(), - $enum::MultiPoint(g) => g.$func_name($($arg_name),*).into(), - $enum::MultiLineString(g) => g.$func_name($($arg_name),*).into(), - $enum::MultiPolygon(g) => g.$func_name($($arg_name),*).into(), - $enum::GeometryCollection(g) => g.$func_name($($arg_name),*).into(), - $enum::Rect(g) => g.$func_name($($arg_name),*).into(), - $enum::Triangle(g) => g.$func_name($($arg_name),*).into(), - } - } - )+ - }; -} diff --git a/geo-generic-tests/src/simple/geometry.rs b/geo-generic-tests/src/simple/geometry.rs index d1b20f7e87..bcb162754a 100644 --- a/geo-generic-tests/src/simple/geometry.rs +++ b/geo-generic-tests/src/simple/geometry.rs @@ -173,8 +173,29 @@ impl GeometryTrait for SimpleGeometry { } } -impl GeometryTraitExt for &SimpleGeometry { +impl<'a, T: CoordNum> GeometryTraitExt for &'a SimpleGeometry { forward_geometry_trait_ext_funcs!(T); + + type InnerGeometryRef<'b> + = &'a SimpleGeometry + where + Self: 'b; + + fn geometry_ext(&self, _i: usize) -> Option> { + None + } + + unsafe fn geometry_unchecked_ext(&self, _i: usize) -> Self::InnerGeometryRef<'_> { + unimplemented!() + } + + fn geometries_ext(&self) -> impl Iterator> { + unimplemented!(); + + // For making the type checker happy + #[allow(unreachable_code)] + core::iter::empty() + } } impl GeoTraitExtWithTypeTag for &SimpleGeometry { @@ -183,6 +204,27 @@ impl GeoTraitExtWithTypeTag for &SimpleGeometry { impl GeometryTraitExt for SimpleGeometry { forward_geometry_trait_ext_funcs!(T); + + type InnerGeometryRef<'a> + = &'a SimpleGeometry + where + Self: 'a; + + fn geometry_ext(&self, _i: usize) -> Option> { + None + } + + unsafe fn geometry_unchecked_ext(&self, _i: usize) -> Self::InnerGeometryRef<'_> { + unimplemented!() + } + + fn geometries_ext(&self) -> impl Iterator> { + unimplemented!(); + + // For making the type checker happy + #[allow(unreachable_code)] + core::iter::empty() + } } impl GeoTraitExtWithTypeTag for SimpleGeometry { diff --git a/geo-generic-tests/src/wkb/reader/geometry.rs b/geo-generic-tests/src/wkb/reader/geometry.rs index a434d07b11..343b464a84 100644 --- a/geo-generic-tests/src/wkb/reader/geometry.rs +++ b/geo-generic-tests/src/wkb/reader/geometry.rs @@ -13,8 +13,8 @@ use crate::wkb::reader::{ }; use crate::wkb::Endianness; use geo_traits::{ - Dimensions, GeometryTrait, GeometryType, UnimplementedLine, UnimplementedRect, - UnimplementedTriangle, + Dimensions, GeometryCollectionTrait, GeometryTrait, GeometryType, UnimplementedLine, + UnimplementedRect, UnimplementedTriangle, }; use super::linearring::WKBLinearRing; @@ -263,8 +263,34 @@ impl<'a> GeometryTrait for &Wkb<'a> { } } -impl GeometryTraitExt for Wkb<'_> { +impl<'a> GeometryTraitExt for Wkb<'a> { forward_geometry_trait_ext_funcs!(f64); + + type InnerGeometryRef<'b> + = &'b Wkb<'a> + where + Self: 'b; + + fn geometry_ext(&self, i: usize) -> Option> { + let GeometryType::GeometryCollection(gc) = self.as_type() else { + return None; + }; + gc.geometry(i) + } + + unsafe fn geometry_unchecked_ext(&self, i: usize) -> Self::InnerGeometryRef<'_> { + let GeometryType::GeometryCollection(gc) = self.as_type() else { + panic!("Called geometry_unchecked_ext on a non-GeometryCollection geometry"); + }; + gc.geometry_unchecked(i) + } + + fn geometries_ext(&self) -> impl Iterator> { + let GeometryType::GeometryCollection(gc) = self.as_type() else { + panic!("Called geometries_ext on a non-GeometryCollection geometry"); + }; + gc.geometries() + } } impl<'a, 'b> GeometryTraitExt for &'b Wkb<'a> @@ -272,6 +298,26 @@ where 'a: 'b, { forward_geometry_trait_ext_funcs!(f64); + + type InnerGeometryRef<'c> + = &'b Wkb<'a> + where + Self: 'c; + + fn geometry_ext(&self, i: usize) -> Option> { + let g = *self; + g.geometry_ext(i) + } + + unsafe fn geometry_unchecked_ext(&self, i: usize) -> Self::InnerGeometryRef<'_> { + let g = *self; + g.geometry_unchecked_ext(i) + } + + fn geometries_ext(&self) -> impl Iterator> { + let g = *self; + g.geometries_ext() + } } impl GeoTraitExtWithTypeTag for Wkb<'_> { diff --git a/geo-traits-ext/src/geometry.rs b/geo-traits-ext/src/geometry.rs index 4c14c7a0af..d077aefcb5 100644 --- a/geo-traits-ext/src/geometry.rs +++ b/geo-traits-ext/src/geometry.rs @@ -1,5 +1,7 @@ // Extend GeometryTrait traits for the `geo-traits` crate +use core::{borrow::Borrow, panic}; + use geo_traits::*; use geo_types::*; @@ -46,11 +48,36 @@ where where Self: 'a; - type GeometryCollectionTypeExt<'a>: 'a - + GeometryCollectionTraitExt::T> + // Note that we don't have a GeometryCollectionTypeExt here, because it would introduce recursive GATs + // such as G::GeometryCollectionTypeExt::GeometryTypeExt::GeometryCollectionTypeExt::... and easily + // trigger a Rust compiler bug: https://github.com/rust-lang/rust/issues/128887 and https://github.com/rust-lang/rust/issues/131960. + // See also https://github.com/geoarrow/geoarrow-rs/issues/1339. + // + // Although this could be worked around by not implementing generic functions using trait-based approach and use + // function-based approach instead, see https://github.com/geoarrow/geoarrow-rs/pull/956 and https://github.com/georust/wkb/pull/77, + // we are not certain if there will be other issues caused by recursive GATs in the future. So we decided to completely get rid + // of recursive GATs. + + type InnerGeometryRef<'a>: 'a + Borrow where Self: 'a; + /// Returns true if this geometry is a GeometryCollection + fn is_collection(&self) -> bool { + matches!(self.as_type(), GeometryType::GeometryCollection(_)) + } + + /// Returns the number of geometries inside this GeometryCollection + fn num_geometries_ext(&self) -> usize { + let GeometryType::GeometryCollection(gc) = self.as_type() else { + panic!("Not a GeometryCollection"); + }; + gc.num_geometries() + } + + /// Cast this geometry to a [`GeometryTypeExt`] enum, which allows for downcasting to a specific + /// type. This does not work when the geometry is a GeometryCollection. Please use `is_collection` + /// to check if the geometry is NOT a GeometryCollection first before calling this method. fn as_type_ext( &self, ) -> GeometryTypeExt< @@ -61,15 +88,32 @@ where Self::MultiPointTypeExt<'_>, Self::MultiLineStringTypeExt<'_>, Self::MultiPolygonTypeExt<'_>, - Self::GeometryCollectionTypeExt<'_>, Self::RectTypeExt<'_>, Self::TriangleTypeExt<'_>, Self::LineTypeExt<'_>, >; + + /// Returns a geometry by index, or None if the index is out of bounds. This method only works with + /// GeometryCollection. Please use `is_collection` to check if the geometry is a GeometryCollection first before + /// calling this method. + fn geometry_ext(&self, i: usize) -> Option>; + + /// Returns a geometry by index without bounds checking. This method only works with GeometryCollection. + /// Please use `is_collection` to check if the geometry is a GeometryCollection first before calling this method. + /// + /// # Safety + /// The caller must ensure that `i` is a valid index less than the number of geometries. + /// Otherwise, this function may cause undefined behavior. + unsafe fn geometry_unchecked_ext(&self, i: usize) -> Self::InnerGeometryRef<'_>; + + /// Returns an iterator over the geometries in this GeometryCollection. This method only works with + /// GeometryCollection. Please use `is_collection` to check if the geometry is a GeometryCollection first before + /// calling this method. + fn geometries_ext(&self) -> impl Iterator>; } #[derive(Debug)] -pub enum GeometryTypeExt<'a, P, LS, Y, MP, ML, MY, GC, R, TT, L> +pub enum GeometryTypeExt<'a, P, LS, Y, MP, ML, MY, R, TT, L> where P: PointTraitExt, LS: LineStringTraitExt, @@ -77,7 +121,6 @@ where MP: MultiPointTraitExt, ML: MultiLineStringTraitExt, MY: MultiPolygonTraitExt, - GC: GeometryCollectionTraitExt, R: RectTraitExt, TT: TriangleTraitExt, L: LineTraitExt, @@ -87,7 +130,6 @@ where ::T: CoordNum, ::T: CoordNum, ::T: CoordNum, - ::T: CoordNum, ::T: CoordNum, ::T: CoordNum, ::T: CoordNum, @@ -98,7 +140,6 @@ where MultiPoint(&'a MP), MultiLineString(&'a ML), MultiPolygon(&'a MY), - GeometryCollection(&'a GC), Rect(&'a R), Triangle(&'a TT), Line(&'a L), @@ -137,11 +178,6 @@ macro_rules! forward_geometry_trait_ext_funcs { where Self: '__g_inner; - type GeometryCollectionTypeExt<'__g_inner> - = ::GeometryCollectionType<'__g_inner> - where - Self: '__g_inner; - type RectTypeExt<'__g_inner> = ::RectType<'__g_inner> where @@ -167,7 +203,6 @@ macro_rules! forward_geometry_trait_ext_funcs { Self::MultiPointTypeExt<'_>, Self::MultiLineStringTypeExt<'_>, Self::MultiPolygonTypeExt<'_>, - Self::GeometryCollectionTypeExt<'_>, Self::RectTypeExt<'_>, Self::TriangleTypeExt<'_>, Self::LineTypeExt<'_>, @@ -179,7 +214,9 @@ macro_rules! forward_geometry_trait_ext_funcs { GeometryType::MultiPoint(mp) => GeometryTypeExt::MultiPoint(mp), GeometryType::MultiLineString(mls) => GeometryTypeExt::MultiLineString(mls), GeometryType::MultiPolygon(mp) => GeometryTypeExt::MultiPolygon(mp), - GeometryType::GeometryCollection(gc) => GeometryTypeExt::GeometryCollection(gc), + GeometryType::GeometryCollection(_) => { + panic!("GeometryCollection is not supported in GeometryTraitExt::as_type_ext") + } GeometryType::Rect(r) => GeometryTypeExt::Rect(r), GeometryType::Triangle(t) => GeometryTypeExt::Triangle(t), GeometryType::Line(l) => GeometryTypeExt::Line(l), @@ -193,17 +230,63 @@ where T: CoordNum, { forward_geometry_trait_ext_funcs!(T); + + type InnerGeometryRef<'a> + = &'a Geometry + where + Self: 'a; + + fn geometry_ext(&self, i: usize) -> Option<&Geometry> { + let GeometryType::GeometryCollection(gc) = self.as_type() else { + panic!("Not a GeometryCollection"); + }; + gc.geometry(i) + } + + unsafe fn geometry_unchecked_ext(&self, i: usize) -> &Geometry { + let GeometryType::GeometryCollection(gc) = self.as_type() else { + panic!("Not a GeometryCollection"); + }; + gc.geometry_unchecked(i) + } + + fn geometries_ext(&self) -> impl Iterator> { + let GeometryType::GeometryCollection(gc) = self.as_type() else { + panic!("Not a GeometryCollection"); + }; + gc.geometries() + } } impl GeoTraitExtWithTypeTag for Geometry { type Tag = GeometryTag; } -impl GeometryTraitExt for &Geometry +impl<'a, T> GeometryTraitExt for &'a Geometry where T: CoordNum, { forward_geometry_trait_ext_funcs!(T); + + type InnerGeometryRef<'b> + = &'a Geometry + where + Self: 'b; + + fn geometry_ext(&self, i: usize) -> Option<&'a Geometry> { + let g = *self; + g.geometry_ext(i) + } + + unsafe fn geometry_unchecked_ext(&self, i: usize) -> &'a Geometry { + let g = *self; + g.geometry_unchecked_ext(i) + } + + fn geometries_ext(&self) -> impl Iterator> { + let g = *self; + g.geometries_ext() + } } impl GeoTraitExtWithTypeTag for &Geometry { @@ -215,6 +298,27 @@ where T: CoordNum, { forward_geometry_trait_ext_funcs!(T); + + type InnerGeometryRef<'a> + = &'a UnimplementedGeometry + where + Self: 'a; + + fn geometry_ext(&self, _i: usize) -> Option> { + unimplemented!() + } + + unsafe fn geometry_unchecked_ext(&self, _i: usize) -> Self::InnerGeometryRef<'_> { + unimplemented!() + } + + fn geometries_ext(&self) -> impl Iterator> { + unimplemented!(); + + // For making the type checker happy + #[allow(unreachable_code)] + core::iter::empty() + } } impl GeoTraitExtWithTypeTag for UnimplementedGeometry {