Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix empty rect behavior by reverting 84d12654104e783011f24267145fb6bfccd2a30e #181

Closed
wants to merge 2 commits into from
Closed

Fix empty rect behavior by reverting 84d12654104e783011f24267145fb6bfccd2a30e #181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
222b01a
Red test showing failure
michaelkirk Nov 5, 2024
12836a9
Revert "Fix overflows applying selection iterators to empty trees by …
michaelkirk Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions rstar/src/aabb.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,11 @@ where
}

fn new_empty<P: Point>() -> AABB<P> {
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),
}
}

Expand Down Expand Up @@ -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]);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other cases pass, but this one fails with:

```
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] }
```

let subject = empty.merged(&other);
assert_eq!(other, subject);
}
}
1 change: 0 additions & 1 deletion rstar/src/algorithm/bulk_load/bulk_load_sequential.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion rstar/src/algorithm/bulk_load/cluster_group_iterator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down
1 change: 0 additions & 1 deletion rstar/src/algorithm/intersection_iterator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use crate::RTreeNode;
use crate::RTreeNode::*;
use crate::RTreeObject;

#[cfg(not(test))]
use alloc::vec::Vec;
use core::mem::take;

Expand Down
7 changes: 0 additions & 7 deletions rstar/src/algorithm/iterators.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
4 changes: 1 addition & 3 deletions rstar/src/algorithm/nearest_neighbor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions rstar/src/algorithm/removal.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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::*;

Expand Down
1 change: 0 additions & 1 deletion rstar/src/algorithm/rstar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down
1 change: 0 additions & 1 deletion rstar/src/node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
39 changes: 25 additions & 14 deletions rstar/src/point.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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`.
Expand All @@ -219,7 +225,12 @@ pub trait PointExt: Point {
///
/// After applying the closure to every component of the Point, fold() returns the accumulator.
fn fold<T>(&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`.
Expand Down
3 changes: 2 additions & 1 deletion rstar/src/rtree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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")]
Expand Down