From 6e1cba3691b1a89321f3ea2be4bff6885f8e5b65 Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Wed, 24 Jul 2019 09:39:50 -0400 Subject: [PATCH 1/2] Make geo-types Rect fields private to force users to use constructor --- geo-types/src/lib.rs | 8 ++-- geo-types/src/line.rs | 2 +- geo-types/src/line_string.rs | 4 +- geo-types/src/polygon.rs | 10 ++-- geo-types/src/rect.rs | 44 ++++++++++++++---- geo/src/algorithm/area.rs | 18 ++++---- geo/src/algorithm/bounding_rect.rs | 72 ++++++++++++++--------------- geo/src/algorithm/centroid.rs | 12 ++--- geo/src/algorithm/contains.rs | 42 ++++++++--------- geo/src/algorithm/intersects.rs | 74 +++++++++++++++--------------- geo/src/algorithm/map_coords.rs | 30 +++++------- geo/src/algorithm/simplifyvw.rs | 4 +- 12 files changed, 171 insertions(+), 149 deletions(-) diff --git a/geo-types/src/lib.rs b/geo-types/src/lib.rs index b777db5a0..f9ad9cd66 100644 --- a/geo-types/src/lib.rs +++ b/geo-types/src/lib.rs @@ -151,10 +151,10 @@ mod test { #[test] fn test_rects() { - let r = Rect { - min: Coordinate { x: -1., y: -1. }, - max: Coordinate { x: 1., y: 1. }, - }; + let r = Rect::new( + Coordinate { x: -1., y: -1. }, + Coordinate { x: 1., y: 1. }, + ); let p: Polygon<_> = r.into(); assert_eq!( p, diff --git a/geo-types/src/line.rs b/geo-types/src/line.rs index 74fbe2c1e..ec7adc9b4 100644 --- a/geo-types/src/line.rs +++ b/geo-types/src/line.rs @@ -169,7 +169,7 @@ where fn envelope(&self) -> Self::Envelope { let bounding_rect = crate::private_utils::line_bounding_rect(*self); - ::rstar::AABB::from_corners(bounding_rect.min.into(), bounding_rect.max.into()) + ::rstar::AABB::from_corners(bounding_rect.min().into(), bounding_rect.max().into()) } } diff --git a/geo-types/src/line_string.rs b/geo-types/src/line_string.rs index f43f3345c..5562be08b 100644 --- a/geo-types/src/line_string.rs +++ b/geo-types/src/line_string.rs @@ -220,8 +220,8 @@ where Point::new(Bounded::max_value(), Bounded::max_value()), ), Some(b) => ::rstar::AABB::from_corners( - Point::new(b.min.x, b.min.y), - Point::new(b.max.x, b.max.y), + Point::new(b.min().x, b.min().y), + Point::new(b.max().x, b.max().y), ), } } diff --git a/geo-types/src/polygon.rs b/geo-types/src/polygon.rs index 34698b656..bd7bb64d1 100644 --- a/geo-types/src/polygon.rs +++ b/geo-types/src/polygon.rs @@ -457,11 +457,11 @@ impl From> for Polygon { fn from(r: Rect) -> Polygon { Polygon::new( vec![ - (r.min.x, r.min.y), - (r.max.x, r.min.y), - (r.max.x, r.max.y), - (r.min.x, r.max.y), - (r.min.x, r.min.y), + (r.min().x, r.min().y), + (r.max().x, r.min().y), + (r.max().x, r.max().y), + (r.min().x, r.max().y), + (r.min().x, r.min().y), ] .into(), Vec::new(), diff --git a/geo-types/src/rect.rs b/geo-types/src/rect.rs index 029b2f8a5..da5d81812 100644 --- a/geo-types/src/rect.rs +++ b/geo-types/src/rect.rs @@ -7,8 +7,8 @@ pub struct Rect where T: CoordinateType, { - pub min: Coordinate, - pub max: Coordinate, + min: Coordinate, + max: Coordinate, } impl Rect { @@ -39,20 +39,48 @@ impl Rect { { let (min, max) = (min.into(), max.into()); - assert!( - min.x <= max.x && min.y <= max.y, - "Failed to create the Rect type: 'min' coordinate's x/y value must be smaller or equal to the 'max' x/y value" - ); + Self::assert_valid_bounds(min, max); Rect { min, max } } + pub fn min(self) -> Coordinate { + self.min + } + + pub fn set_min(&mut self, min: C) + where + C: Into>, + { + self.min = min.into(); + Self::assert_valid_bounds(self.min, self.max); + } + + pub fn max(self) -> Coordinate { + self.max + } + + pub fn set_max(&mut self, max: C) + where + C: Into>, + { + self.max = max.into(); + Self::assert_valid_bounds(self.min, self.max); + } + pub fn width(self) -> T { - self.max.x - self.min.x + self.max().x - self.min().x } pub fn height(self) -> T { - self.max.y - self.min.y + self.max().y - self.min().y + } + + fn assert_valid_bounds(min: Coordinate, max: Coordinate) { + assert!( + min.x <= max.x && min.y <= max.y, + "Failed to create the Rect type: 'min' coordinate's x/y value must be smaller or equal to the 'max' x/y value" + ); } } diff --git a/geo/src/algorithm/area.rs b/geo/src/algorithm/area.rs index f62f485ce..b6edc7953 100644 --- a/geo/src/algorithm/area.rs +++ b/geo/src/algorithm/area.rs @@ -80,7 +80,7 @@ where T: CoordinateType, { fn area(&self) -> T { - (self.max.x - self.min.x) * (self.max.y - self.min.y) + self.width() * self.height() } } @@ -126,16 +126,16 @@ mod test { } #[test] fn rectangle_test() { - let rect1: Rect = Rect { - min: Coordinate { x: 10., y: 30. }, - max: Coordinate { x: 20., y: 40. }, - }; + let rect1: Rect = Rect::new( + Coordinate { x: 10., y: 30. }, + Coordinate { x: 20., y: 40. }, + ); assert_relative_eq!(rect1.area(), 100.); - let rect2: Rect = Rect { - min: Coordinate { x: 10, y: 30 }, - max: Coordinate { x: 20, y: 40 }, - }; + let rect2: Rect = Rect::new( + Coordinate { x: 10, y: 30 }, + Coordinate { x: 20, y: 40 }, + ); assert_eq!(rect2.area(), 100); } #[test] diff --git a/geo/src/algorithm/bounding_rect.rs b/geo/src/algorithm/bounding_rect.rs index fb43679f8..abc65d1e4 100644 --- a/geo/src/algorithm/bounding_rect.rs +++ b/geo/src/algorithm/bounding_rect.rs @@ -23,10 +23,10 @@ pub trait BoundingRect { /// let linestring = LineString::from(vec); /// let bounding_rect = linestring.bounding_rect().unwrap(); /// - /// assert_eq!(40.02f64, bounding_rect.min.x); - /// assert_eq!(42.02f64, bounding_rect.max.x); - /// assert_eq!(116.34, bounding_rect.min.y); - /// assert_eq!(118.34, bounding_rect.max.y); + /// assert_eq!(40.02f64, bounding_rect.min().x); + /// assert_eq!(42.02f64, bounding_rect.max().x); + /// assert_eq!(116.34, bounding_rect.min().y); + /// assert_eq!(118.34, bounding_rect.max().y); /// ``` /// fn bounding_rect(&self) -> Self::Output; @@ -57,10 +57,10 @@ where let b = self.end; let (xmin, xmax) = if a.x <= b.x { (a.x, b.x) } else { (b.x, a.x) }; let (ymin, ymax) = if a.y <= b.y { (a.y, b.y) } else { (b.y, a.y) }; - Rect { - min: Coordinate { x: xmin, y: ymin }, - max: Coordinate { x: xmax, y: ymax }, - } + Rect::new( + Coordinate { x: xmin, y: ymin }, + Coordinate { x: xmax, y: ymax }, + ) } } @@ -165,16 +165,16 @@ mod test { #[test] fn linestring_one_point_test() { let linestring = line_string![(x: 40.02f64, y: 116.34)]; - let bounding_rect = Rect { - min: Coordinate { + let bounding_rect = Rect::new( + Coordinate { x: 40.02f64, y: 116.34, }, - max: Coordinate { + Coordinate { x: 40.02, y: 116.34, }, - }; + ); assert_eq!(bounding_rect, linestring.bounding_rect().unwrap()); } #[test] @@ -185,10 +185,10 @@ mod test { (x: -3., y: -3.), (x: -4., y: 4.) ]; - let bounding_rect = Rect { - min: Coordinate { x: -4., y: -3. }, - max: Coordinate { x: 2., y: 4. }, - }; + let bounding_rect = Rect::new( + Coordinate { x: -4., y: -3. }, + Coordinate { x: 2., y: 4. }, + ); assert_eq!(bounding_rect, linestring.bounding_rect().unwrap()); } #[test] @@ -199,19 +199,19 @@ mod test { line_string![(x: 1., y: 1.), (x: 1., y: -60.)], line_string![(x: 1., y: 1.), (x: 1., y: 70.)], ]); - let bounding_rect = Rect { - min: Coordinate { x: -40., y: -60. }, - max: Coordinate { x: 50., y: 70. }, - }; + let bounding_rect = Rect::new( + Coordinate { x: -40., y: -60. }, + Coordinate { x: 50., y: 70. }, + ); assert_eq!(bounding_rect, multiline.bounding_rect().unwrap()); } #[test] fn multipoint_test() { let multipoint = MultiPoint::from(vec![(1., 1.), (2., -2.), (-3., -3.), (-4., 4.)]); - let bounding_rect = Rect { - min: Coordinate { x: -4., y: -3. }, - max: Coordinate { x: 2., y: 4. }, - }; + let bounding_rect = Rect::new( + Coordinate { x: -4., y: -3. }, + Coordinate { x: 2., y: 4. }, + ); assert_eq!(bounding_rect, multipoint.bounding_rect().unwrap()); } #[test] @@ -234,10 +234,10 @@ mod test { polygon![(x: 0., y: 0.), (x: 5., y: 0.), (x: 0., y: 80.), (x: 0., y: 0.)], polygon![(x: 0., y: 0.), (x: -60., y: 0.), (x: 0., y: 6.), (x: 0., y: 0.)], ]); - let bounding_rect = Rect { - min: Coordinate { x: -60., y: -70. }, - max: Coordinate { x: 50., y: 80. }, - }; + let bounding_rect = Rect::new( + Coordinate { x: -60., y: -70. }, + Coordinate { x: 50., y: 80. }, + ); assert_eq!(bounding_rect, mpoly.bounding_rect().unwrap()); } #[test] @@ -246,17 +246,17 @@ mod test { let line2 = Line::new(Coordinate { x: 2., y: 3. }, Coordinate { x: 0., y: 1. }); assert_eq!( line1.bounding_rect(), - Rect { - min: Coordinate { x: 0., y: 1. }, - max: Coordinate { x: 2., y: 3. }, - } + Rect::new( + Coordinate { x: 0., y: 1. }, + Coordinate { x: 2., y: 3. }, + ) ); assert_eq!( line2.bounding_rect(), - Rect { - min: Coordinate { x: 0., y: 1. }, - max: Coordinate { x: 2., y: 3. }, - } + Rect::new( + Coordinate { x: 0., y: 1. }, + Coordinate { x: 2., y: 3. }, + ) ); } } diff --git a/geo/src/algorithm/centroid.rs b/geo/src/algorithm/centroid.rs index 6a316cf8e..7e5765b6d 100644 --- a/geo/src/algorithm/centroid.rs +++ b/geo/src/algorithm/centroid.rs @@ -251,8 +251,8 @@ where fn centroid(&self) -> Self::Output { let two = T::one() + T::one(); Point::new( - (self.max.x + self.min.x) / two, - (self.max.y + self.min.y) / two, + (self.max().x + self.min().x) / two, + (self.max().y + self.min().y) / two, ) } } @@ -533,10 +533,10 @@ mod test { } #[test] fn bounding_rect_test() { - let bounding_rect = Rect { - min: Coordinate { x: 0., y: 50. }, - max: Coordinate { x: 4., y: 100. }, - }; + let bounding_rect = Rect::new( + Coordinate { x: 0., y: 50. }, + Coordinate { x: 4., y: 100. }, + ); let point = Point(Coordinate { x: 2., y: 75. }); assert_eq!(point, bounding_rect.centroid()); } diff --git a/geo/src/algorithm/contains.rs b/geo/src/algorithm/contains.rs index 1d1bcca43..7e785ae57 100644 --- a/geo/src/algorithm/contains.rs +++ b/geo/src/algorithm/contains.rs @@ -233,7 +233,7 @@ where T: CoordinateType, { fn contains(&self, p: &Point) -> bool { - p.x() >= self.min.x && p.x() <= self.max.x && p.y() >= self.min.y && p.y() <= self.max.y + p.x() >= self.min().x && p.x() <= self.max().x && p.y() >= self.min().y && p.y() <= self.max().y } } @@ -243,10 +243,10 @@ where { fn contains(&self, bounding_rect: &Rect) -> bool { // All points of LineString must be in the polygon ? - self.min.x <= bounding_rect.min.x - && self.max.x >= bounding_rect.max.x - && self.min.y <= bounding_rect.min.y - && self.max.y >= bounding_rect.max.y + self.min().x <= bounding_rect.min().x + && self.max().x >= bounding_rect.max().x + && self.min().y <= bounding_rect.min().y + && self.max().y >= bounding_rect.max().y } } @@ -498,14 +498,14 @@ mod test { } #[test] fn bounding_rect_in_inner_bounding_rect_test() { - let bounding_rect_xl = Rect { - min: Coordinate { x: -100., y: -200. }, - max: Coordinate { x: 100., y: 200. }, - }; - let bounding_rect_sm = Rect { - min: Coordinate { x: -10., y: -20. }, - max: Coordinate { x: 10., y: 20. }, - }; + let bounding_rect_xl = Rect::new( + Coordinate { x: -100., y: -200. }, + Coordinate { x: 100., y: 200. }, + ); + let bounding_rect_sm = Rect::new( + Coordinate { x: -10., y: -20. }, + Coordinate { x: 10., y: 20. }, + ); assert_eq!(true, bounding_rect_xl.contains(&bounding_rect_sm)); assert_eq!(false, bounding_rect_sm.contains(&bounding_rect_xl)); } @@ -594,17 +594,17 @@ mod test { #[test] fn integer_bounding_rects() { let p: Point = Point::new(10, 20); - let bounding_rect: Rect = Rect { - min: Coordinate { x: 0, y: 0 }, - max: Coordinate { x: 100, y: 100 }, - }; + let bounding_rect: Rect = Rect::new( + Coordinate { x: 0, y: 0 }, + Coordinate { x: 100, y: 100 }, + ); assert!(bounding_rect.contains(&p)); assert!(!bounding_rect.contains(&Point::new(-10, -10))); - let smaller_bounding_rect: Rect = Rect { - min: Coordinate { x: 10, y: 10 }, - max: Coordinate { x: 20, y: 20 }, - }; + let smaller_bounding_rect: Rect = Rect::new( + Coordinate { x: 10, y: 10 }, + Coordinate { x: 20, y: 20 }, + ); assert!(bounding_rect.contains(&smaller_bounding_rect)); } diff --git a/geo/src/algorithm/intersects.rs b/geo/src/algorithm/intersects.rs index a56d9a4d3..b134cde4b 100644 --- a/geo/src/algorithm/intersects.rs +++ b/geo/src/algorithm/intersects.rs @@ -209,10 +209,10 @@ where if bounding_rect.contains(self) { false } else { - (self.min.x >= bounding_rect.min.x && self.min.x <= bounding_rect.max.x - || self.max.x >= bounding_rect.min.x && self.max.x <= bounding_rect.max.x) - && (self.min.y >= bounding_rect.min.y && self.min.y <= bounding_rect.max.y - || self.max.y >= bounding_rect.min.y && self.max.y <= bounding_rect.max.y) + (self.min().x >= bounding_rect.min().x && self.min().x <= bounding_rect.max().x + || self.max().x >= bounding_rect.min().x && self.max().x <= bounding_rect.max().x) + && (self.min().y >= bounding_rect.min().y && self.min().y <= bounding_rect.max().y + || self.max().y >= bounding_rect.min().y && self.max().y <= bounding_rect.max().y) } } } @@ -233,11 +233,11 @@ where fn intersects(&self, bounding_rect: &Rect) -> bool { let p = Polygon::new( LineString::from(vec![ - (bounding_rect.min.x, bounding_rect.min.y), - (bounding_rect.min.x, bounding_rect.max.y), - (bounding_rect.max.x, bounding_rect.max.y), - (bounding_rect.max.x, bounding_rect.min.y), - (bounding_rect.min.x, bounding_rect.min.y), + (bounding_rect.min().x, bounding_rect.min().y), + (bounding_rect.min().x, bounding_rect.max().y), + (bounding_rect.max().x, bounding_rect.max().y), + (bounding_rect.max().x, bounding_rect.min().y), + (bounding_rect.min().x, bounding_rect.min().y), ]), vec![], ); @@ -487,22 +487,22 @@ mod test { (7., 4.), ])], ); - let b1 = Rect { - min: Coordinate { x: 11., y: 1. }, - max: Coordinate { x: 13., y: 2. }, - }; - let b2 = Rect { - min: Coordinate { x: 2., y: 2. }, - max: Coordinate { x: 8., y: 5. }, - }; - let b3 = Rect { - min: Coordinate { x: 8., y: 5. }, - max: Coordinate { x: 10., y: 6. }, - }; - let b4 = Rect { - min: Coordinate { x: 1., y: 1. }, - max: Coordinate { x: 3., y: 3. }, - }; + let b1 = Rect::new( + Coordinate { x: 11., y: 1. }, + Coordinate { x: 13., y: 2. }, + ); + let b2 = Rect::new( + Coordinate { x: 2., y: 2. }, + Coordinate { x: 8., y: 5. }, + ); + let b3 = Rect::new( + Coordinate { x: 8., y: 5. }, + Coordinate { x: 10., y: 6. }, + ); + let b4 = Rect::new( + Coordinate { x: 1., y: 1. }, + Coordinate { x: 3., y: 3. }, + ); // overlaps assert!(poly.intersects(&b1)); // contained in exterior, overlaps with hole @@ -519,18 +519,18 @@ mod test { } #[test] fn bounding_rect_test() { - let bounding_rect_xl = Rect { - min: Coordinate { x: -100., y: -200. }, - max: Coordinate { x: 100., y: 200. }, - }; - let bounding_rect_sm = Rect { - min: Coordinate { x: -10., y: -20. }, - max: Coordinate { x: 10., y: 20. }, - }; - let bounding_rect_s2 = Rect { - min: Coordinate { x: 0., y: 0. }, - max: Coordinate { x: 20., y: 30. }, - }; + let bounding_rect_xl = Rect::new( + Coordinate { x: -100., y: -200. }, + Coordinate { x: 100., y: 200. }, + ); + let bounding_rect_sm = Rect::new( + Coordinate { x: -10., y: -20. }, + Coordinate { x: 10., y: 20. }, + ); + let bounding_rect_s2 = Rect::new( + Coordinate { x: 0., y: 0. }, + Coordinate { x: 20., y: 30. }, + ); assert_eq!(false, bounding_rect_xl.intersects(&bounding_rect_sm)); assert_eq!(false, bounding_rect_sm.intersects(&bounding_rect_xl)); assert_eq!(true, bounding_rect_sm.intersects(&bounding_rect_s2)); diff --git a/geo/src/algorithm/map_coords.rs b/geo/src/algorithm/map_coords.rs index 3f590611f..1ff4de641 100644 --- a/geo/src/algorithm/map_coords.rs +++ b/geo/src/algorithm/map_coords.rs @@ -421,10 +421,9 @@ impl MapCoords for Rect { type Output = Rect; fn map_coords(&self, func: &dyn Fn(&(T, T)) -> (NT, NT)) -> Self::Output { - let new_min = func(&(self.min.x, self.min.y)); - let new_max = func(&(self.max.x, self.max.y)); + let new_min = func(&self.min().x_y()); + let new_max = func(&self.max().x_y()); - // using constructor to enforce soundness of the obtained coordinates Rect::new( Coordinate { x: new_min.0, @@ -445,8 +444,8 @@ impl TryMapCoords for Rect { &self, func: &dyn Fn(&(T, T)) -> Result<(NT, NT), Error>, ) -> Result { - let new_min = func(&(self.min.x, self.min.y))?; - let new_max = func(&(self.max.x, self.max.y))?; + let new_min = func(&(self.min().x, self.min().y))?; + let new_max = func(&(self.max().x, self.max().y))?; Ok(Rect::new( Coordinate { @@ -463,17 +462,12 @@ impl TryMapCoords for Rect { impl MapCoordsInplace for Rect { fn map_coords_inplace(&mut self, func: &dyn Fn(&(T, T)) -> (T, T)) { - let new_min = func(&(self.min.x, self.min.y)); - let new_max = func(&(self.max.x, self.max.y)); + let new_min = func(&self.min().x_y()); + let new_max = func(&self.max().x_y()); - // make sure the min coordinates are always smaller than the max coordinates, otherwise - // we won't have a "regular" rectangle that many of the methods assumes. - assert!(new_min.0 < new_max.0 && new_min.1 < new_max.1); + let mut new_rect = Rect::new(new_min, new_max); - self.min.x = new_min.0; - self.min.y = new_min.1; - self.max.x = new_max.0; - self.max.y = new_max.1; + ::std::mem::swap(self, &mut new_rect); } } @@ -502,8 +496,8 @@ mod test { fn rect_inplace() { let mut rect = Rect::new((10, 10), (20, 20)); rect.map_coords_inplace(&|&(x, y)| (x + 10, y + 20)); - assert_eq!(rect.min, Coordinate { x: 20, y: 30 }); - assert_eq!(rect.max, Coordinate { x: 30, y: 40 }); + assert_eq!(rect.min(), Coordinate { x: 20, y: 30 }); + assert_eq!(rect.max(), Coordinate { x: 30, y: 40 }); } #[test] @@ -523,8 +517,8 @@ mod test { fn rect_map_coords() { let rect = Rect::new((10, 10), (20, 20)); let another_rect = rect.map_coords(&|&(x, y)| (x + 10, y + 20)); - assert_eq!(another_rect.min, Coordinate { x: 20, y: 30 }); - assert_eq!(another_rect.max, Coordinate { x: 30, y: 40 }); + assert_eq!(another_rect.min(), Coordinate { x: 20, y: 30 }); + assert_eq!(another_rect.max(), Coordinate { x: 30, y: 40 }); } #[test] diff --git a/geo/src/algorithm/simplifyvw.rs b/geo/src/algorithm/simplifyvw.rs index baef23b3e..15fc0db81 100644 --- a/geo/src/algorithm/simplifyvw.rs +++ b/geo/src/algorithm/simplifyvw.rs @@ -383,8 +383,8 @@ where orig[triangle.right], ) .bounding_rect(); - let br = Point::new(bounding_rect.min.x, bounding_rect.min.y); - let tl = Point::new(bounding_rect.max.x, bounding_rect.max.y); + let br = Point::new(bounding_rect.min().x, bounding_rect.min().y); + let tl = Point::new(bounding_rect.max().x, bounding_rect.max().y); tree.locate_in_envelope_intersecting(&rstar::AABB::from_corners(br, tl)) .any(|c| { // triangle start point, end point From bf77d874962d5a5c2fb5a82bd5f368fbc774f3c2 Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Wed, 24 Jul 2019 21:43:15 -0400 Subject: [PATCH 2/2] fix --- geo-types/src/rect.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/geo-types/src/rect.rs b/geo-types/src/rect.rs index da5d81812..4c64f9584 100644 --- a/geo-types/src/rect.rs +++ b/geo-types/src/rect.rs @@ -30,8 +30,8 @@ impl Rect { /// Coordinate { x: 10., y: 20. }, /// ); /// - /// assert_eq!(rect.min, Coordinate { x: 0., y: 0. }); - /// assert_eq!(rect.max, Coordinate { x: 10., y: 20. }); + /// assert_eq!(rect.min(), Coordinate { x: 0., y: 0. }); + /// assert_eq!(rect.max(), Coordinate { x: 10., y: 20. }); /// ``` pub fn new(min: C, max: C) -> Rect where