Revert back to min/max representation of empty AABB

rstar-benches/Cargo.toml

@@ -1,5 +1,6 @@
 [package]
 name = "rstar-benches"
+edition = "2015"
 version = "0.1.1"
 authors = ["Stefan Altmayer <[email protected]>", "The Georust Developers <[email protected]>"]
 

rstar/CHANGELOG.md

@@ -1,3 +1,8 @@
+# Unreleased
+
+## Changed
+- Reverted the change to `AABB::new_empty` while still avoiding overflow panics applying selections on empty trees ([PR](https://github.com/georust/rstar/pull/184)
+
 # 0.12.1
 
 ## Added

rstar/src/aabb.rs

@@ -106,7 +106,12 @@ where
     type Point = P;
 
     fn new_empty() -> Self {
-        new_empty()
+        let max = P::Scalar::max_value();
+        let min = P::Scalar::min_value();
+        Self {
+            lower: P::from_value(max),
+            upper: P::from_value(min),
+        }
     }
 
     fn contains_point(&self, point: &P) -> bool {
@@ -219,21 +224,33 @@ where
     }
 }
 
-fn new_empty<P: Point>() -> AABB<P> {
-    let one = P::Scalar::one();
-    let zero = P::Scalar::zero();
-    AABB {
-        lower: P::from_value(one),
-        upper: P::from_value(zero),
-    }
-}
-
 #[cfg(test)]
 mod test {
     use super::AABB;
     use crate::envelope::Envelope;
     use crate::object::PointDistance;
 
+    #[test]
+    fn empty_rect() {
+        let empty = AABB::<[f32; 2]>::new_empty();
+
+        let other = AABB::from_corners([1.0, 1.0], [1.0, 1.0]);
+        let subject = empty.merged(&other);
+        assert_eq!(other, subject);
+
+        let other = AABB::from_corners([0.0, 0.0], [0.0, 0.0]);
+        let subject = empty.merged(&other);
+        assert_eq!(other, subject);
+
+        let other = AABB::from_corners([0.5, 0.5], [0.5, 0.5]);
+        let subject = empty.merged(&other);
+        assert_eq!(other, subject);
+
+        let other = AABB::from_corners([-0.5, -0.5], [-0.5, -0.5]);
+        let subject = empty.merged(&other);
+        assert_eq!(other, subject);
+    }
+
     /// Test that min_max_dist_2 is identical to distance_2 for the equivalent
     /// min max corner of the AABB. This is necessary to prevent optimizations
     /// from inadvertently changing floating point order of operations.

rstar/src/algorithm/iterators.rs

@@ -53,11 +53,12 @@ where
     Func: SelectionFunction<T>,
 {
     pub(crate) fn new(root: &'a ParentNode<T>, func: Func) -> Self {
-        let current_nodes = if func.should_unpack_parent(&root.envelope()) {
-            root.children.iter().collect()
-        } else {
-            SmallVec::new()
-        };
+        let current_nodes =
+            if !root.children.is_empty() && func.should_unpack_parent(&root.envelope()) {
+                root.children.iter().collect()
+            } else {
+                SmallVec::new()
+            };
 
         SelectionIterator {
             func,
@@ -135,7 +136,7 @@ where
         ControlFlow::Continue(())
     }
 
-    if func.should_unpack_parent(&root.envelope()) {
+    if !root.children.is_empty() && func.should_unpack_parent(&root.envelope()) {
         inner(root, &mut Args { func, visitor })?;
     }
 
@@ -158,11 +159,12 @@ where
     Func: SelectionFunction<T>,
 {
     pub(crate) fn new(root: &'a mut ParentNode<T>, func: Func) -> Self {
-        let current_nodes = if func.should_unpack_parent(&root.envelope()) {
-            root.children.iter_mut().collect()
-        } else {
-            SmallVec::new()
-        };
+        let current_nodes =
+            if !root.children.is_empty() && func.should_unpack_parent(&root.envelope()) {
+                root.children.iter_mut().collect()
+            } else {
+                SmallVec::new()
+            };
 
         SelectionIteratorMut {
             func,
@@ -240,7 +242,7 @@ where
         ControlFlow::Continue(())
     }
 
-    if func.should_unpack_parent(&root.envelope()) {
+    if !root.children.is_empty() && func.should_unpack_parent(&root.envelope()) {
         inner(root, &mut Args { func, visitor })?;
     }
 
@@ -408,8 +410,10 @@ mod test {
 
     #[test]
     fn test_locate_within_distance_on_empty_tree() {
-        let tree: RTree<[i64; 3]> = RTree::new();
+        let tree: RTree<[f64; 3]> = RTree::new();
+        tree.locate_within_distance([0.0, 0.0, 0.0], 10.0);
 
+        let tree: RTree<[i64; 3]> = RTree::new();
         tree.locate_within_distance([0, 0, 0], 10);
     }
 }

rstar/src/node.rs

@@ -103,6 +103,7 @@ where
     }
 
     #[cfg(test)]
+    #[allow(missing_docs)]
     pub fn sanity_check<Params>(&self, check_max_size: bool) -> Option<usize>
     where
         Params: RTreeParams,

rstar/src/point.rs

@@ -14,20 +14,16 @@ use num_traits::{Bounded, Num, Signed, Zero};
 /// # Example
 /// ```
 /// # extern crate num_traits;
-/// use num_traits::{Num, One, Signed, Zero};
+/// use num_traits::{Bounded, Num, Signed};
 ///
 /// #[derive(Clone, Copy, PartialEq, PartialOrd, Debug)]
 /// struct MyFancyNumberType(f32);
 ///
-/// impl Zero for MyFancyNumberType {
+/// impl num_traits::Bounded for MyFancyNumberType {
 ///   // ... details hidden ...
-/// # fn zero() -> Self { MyFancyNumberType(Zero::zero()) }
-/// # fn is_zero(&self) -> bool { unimplemented!() }
-/// }
-///
-/// impl One for MyFancyNumberType {
-///   // ... details hidden ...
-/// # fn one() -> Self { MyFancyNumberType(One::one()) }
+/// # fn min_value() -> Self { Self(Bounded::min_value()) }
+/// #
+/// # fn max_value() -> Self { Self(Bounded::max_value()) }
 /// }
 ///
 /// impl Signed for MyFancyNumberType {
@@ -58,9 +54,13 @@ use num_traits::{Bounded, Num, Signed, Zero};
 /// rtree.insert([MyFancyNumberType(0.0), MyFancyNumberType(0.0)]);
 /// # }
 ///
-/// # impl num_traits::Bounded for MyFancyNumberType {
-/// #   fn min_value() -> Self { unimplemented!() }
-/// #   fn max_value() -> Self { unimplemented!() }
+/// # impl num_traits::Zero for MyFancyNumberType {
+/// #   fn zero() -> Self { unimplemented!() }
+/// #   fn is_zero(&self) -> bool { unimplemented!() }
+/// # }
+/// #
+/// # impl num_traits::One for MyFancyNumberType {
+/// #   fn one() -> Self { unimplemented!() }
 /// # }
 /// #
 /// # impl core::ops::Mul for MyFancyNumberType {