From 222b01a40953c4fae35b38fc7d19416a29136818 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 5 Nov 2024 10:21:00 -0800 Subject: [PATCH 1/2] Red test showing failure Failing test is ``` assertion `left == right` failed left: AABB { lower: [-0.5, -0.5], upper: [-0.5, -0.5] } right: AABB { lower: [-0.5, -0.5], upper: [0.0, 0.0] } ``` --- rstar/src/aabb.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/rstar/src/aabb.rs b/rstar/src/aabb.rs index 092ff51..0229821 100644 --- a/rstar/src/aabb.rs +++ b/rstar/src/aabb.rs @@ -252,4 +252,25 @@ mod test { let aabb = AABB::from_points(&[(3., 3., 3.), (4., 4., 4.)]); assert_eq!(aabb, AABB::from_corners((3., 3., 3.), (4., 4., 4.))); } + + #[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); + } } From 12836a98fc8e04e9c0668cb4c4f273f47e430697 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 5 Nov 2024 10:22:34 -0800 Subject: [PATCH 2/2] Revert "Fix overflows applying selection iterators to empty trees by choosing a more tame value for AABB::new_empty (#162)" This reverts commit 84d12654104e783011f24267145fb6bfccd2a30e. --- rstar/src/aabb.rs | 8 ++-- .../bulk_load/bulk_load_sequential.rs | 1 - .../bulk_load/cluster_group_iterator.rs | 1 - rstar/src/algorithm/intersection_iterator.rs | 1 - rstar/src/algorithm/iterators.rs | 7 ---- rstar/src/algorithm/nearest_neighbor.rs | 4 +- rstar/src/algorithm/removal.rs | 3 +- rstar/src/algorithm/rstar.rs | 1 - rstar/src/node.rs | 1 - rstar/src/point.rs | 39 ++++++++++++------- rstar/src/rtree.rs | 3 +- 11 files changed, 33 insertions(+), 36 deletions(-) diff --git a/rstar/src/aabb.rs b/rstar/src/aabb.rs index 0229821..efc84a1 100644 --- a/rstar/src/aabb.rs +++ b/rstar/src/aabb.rs @@ -220,11 +220,11 @@ where } fn new_empty() -> AABB

{ - let one = P::Scalar::one(); - let zero = P::Scalar::zero(); + let max = P::Scalar::max_value(); + let min = P::Scalar::min_value(); AABB { - lower: P::from_value(one), - upper: P::from_value(zero), + lower: P::from_value(max), + upper: P::from_value(min), } } diff --git a/rstar/src/algorithm/bulk_load/bulk_load_sequential.rs b/rstar/src/algorithm/bulk_load/bulk_load_sequential.rs index ca75d8a..b45d05f 100644 --- a/rstar/src/algorithm/bulk_load/bulk_load_sequential.rs +++ b/rstar/src/algorithm/bulk_load/bulk_load_sequential.rs @@ -4,7 +4,6 @@ use crate::object::RTreeObject; use crate::params::RTreeParams; use crate::point::Point; -#[cfg(not(test))] use alloc::{vec, vec::Vec}; #[allow(unused_imports)] // Import is required when building without std diff --git a/rstar/src/algorithm/bulk_load/cluster_group_iterator.rs b/rstar/src/algorithm/bulk_load/cluster_group_iterator.rs index 6200442..ef6b9c9 100644 --- a/rstar/src/algorithm/bulk_load/cluster_group_iterator.rs +++ b/rstar/src/algorithm/bulk_load/cluster_group_iterator.rs @@ -1,6 +1,5 @@ use crate::{Envelope, Point, RTreeObject, RTreeParams}; -#[cfg(not(test))] use alloc::vec::Vec; #[allow(unused_imports)] // Import is required when building without std diff --git a/rstar/src/algorithm/intersection_iterator.rs b/rstar/src/algorithm/intersection_iterator.rs index 3be3c55..19a22c1 100644 --- a/rstar/src/algorithm/intersection_iterator.rs +++ b/rstar/src/algorithm/intersection_iterator.rs @@ -4,7 +4,6 @@ use crate::RTreeNode; use crate::RTreeNode::*; use crate::RTreeObject; -#[cfg(not(test))] use alloc::vec::Vec; use core::mem::take; diff --git a/rstar/src/algorithm/iterators.rs b/rstar/src/algorithm/iterators.rs index 5a241f0..64b3065 100644 --- a/rstar/src/algorithm/iterators.rs +++ b/rstar/src/algorithm/iterators.rs @@ -405,11 +405,4 @@ mod test { assert!(located.contains(point)); } } - - #[test] - fn test_locate_within_distance_on_empty_tree() { - let tree: RTree<[i64; 3]> = RTree::new(); - - tree.locate_within_distance([0, 0, 0], 10); - } } diff --git a/rstar/src/algorithm/nearest_neighbor.rs b/rstar/src/algorithm/nearest_neighbor.rs index 698b26d..155ed5c 100644 --- a/rstar/src/algorithm/nearest_neighbor.rs +++ b/rstar/src/algorithm/nearest_neighbor.rs @@ -2,9 +2,7 @@ use crate::node::{ParentNode, RTreeNode}; use crate::point::{min_inline, Point}; use crate::{Envelope, PointDistance, RTreeObject}; -use alloc::collections::BinaryHeap; -#[cfg(not(test))] -use alloc::{vec, vec::Vec}; +use alloc::{collections::BinaryHeap, vec, vec::Vec}; use core::mem::replace; use heapless::binary_heap as static_heap; use num_traits::Bounded; diff --git a/rstar/src/algorithm/removal.rs b/rstar/src/algorithm/removal.rs index 0bf5baa..6788ae3 100644 --- a/rstar/src/algorithm/removal.rs +++ b/rstar/src/algorithm/removal.rs @@ -6,7 +6,6 @@ use crate::object::RTreeObject; use crate::params::RTreeParams; use crate::{Envelope, RTree}; -#[cfg(not(test))] use alloc::{vec, vec::Vec}; #[allow(unused_imports)] // Import is required when building without std @@ -253,7 +252,7 @@ mod test { use crate::point::PointExt; use crate::primitives::Line; use crate::test_utilities::{create_random_points, create_random_rectangles, SEED_1, SEED_2}; - use crate::AABB; + use crate::{RTree, AABB}; use super::*; diff --git a/rstar/src/algorithm/rstar.rs b/rstar/src/algorithm/rstar.rs index 9127037..392242b 100644 --- a/rstar/src/algorithm/rstar.rs +++ b/rstar/src/algorithm/rstar.rs @@ -5,7 +5,6 @@ use crate::params::{InsertionStrategy, RTreeParams}; use crate::point::{Point, PointExt}; use crate::rtree::RTree; -#[cfg(not(test))] use alloc::vec::Vec; use num_traits::{Bounded, Zero}; diff --git a/rstar/src/node.rs b/rstar/src/node.rs index 3498a26..c126285 100644 --- a/rstar/src/node.rs +++ b/rstar/src/node.rs @@ -2,7 +2,6 @@ use crate::envelope::Envelope; use crate::object::RTreeObject; use crate::params::RTreeParams; -#[cfg(not(test))] use alloc::vec::Vec; #[cfg(feature = "serde")] diff --git a/rstar/src/point.rs b/rstar/src/point.rs index b162a4f..7763c82 100644 --- a/rstar/src/point.rs +++ b/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 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 { MyFancyNumberType(Bounded::min_value()) } +/// # +/// # fn max_value() -> Self { MyFancyNumberType(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 { @@ -202,7 +202,13 @@ pub trait PointExt: Point { other: &Self, mut f: impl FnMut(Self::Scalar, Self::Scalar) -> bool, ) -> bool { - (0..Self::DIMENSIONS).all(|i| f(self.nth(i), other.nth(i))) + // TODO: Maybe do this by proper iteration + for i in 0..Self::DIMENSIONS { + if !f(self.nth(i), other.nth(i)) { + return false; + } + } + true } /// Returns the dot product of `self` and `rhs`. @@ -219,7 +225,12 @@ pub trait PointExt: Point { /// /// After applying the closure to every component of the Point, fold() returns the accumulator. fn fold(&self, start_value: T, mut f: impl FnMut(T, Self::Scalar) -> T) -> T { - (0..Self::DIMENSIONS).fold(start_value, |accumulated, i| f(accumulated, self.nth(i))) + let mut accumulated = start_value; + // TODO: Maybe do this by proper iteration + for i in 0..Self::DIMENSIONS { + accumulated = f(accumulated, self.nth(i)); + } + accumulated } /// Returns a Point with every component set to `value`. diff --git a/rstar/src/rtree.rs b/rstar/src/rtree.rs index 862afa9..36b1420 100644 --- a/rstar/src/rtree.rs +++ b/rstar/src/rtree.rs @@ -1,9 +1,11 @@ use crate::algorithm::bulk_load; +use crate::algorithm::intersection_iterator::IntersectionIterator; use crate::algorithm::iterators::*; use crate::algorithm::nearest_neighbor; use crate::algorithm::nearest_neighbor::NearestNeighborDistance2Iterator; use crate::algorithm::nearest_neighbor::NearestNeighborIterator; use crate::algorithm::removal; +use crate::algorithm::removal::DrainIterator; use crate::algorithm::selection_functions::*; use crate::envelope::Envelope; use crate::node::ParentNode; @@ -12,7 +14,6 @@ use crate::params::{verify_parameters, DefaultParams, InsertionStrategy, RTreePa use crate::Point; use core::ops::ControlFlow; -#[cfg(not(test))] use alloc::vec::Vec; #[cfg(feature = "serde")]