georust/rstar

Initial `AABB` can cause `from_points`/others to be wrong

Closed this issue · 3 comments

The lower bounds are wrong in this example on master:

fn main() {
    let aabb = AABB::from_points(&[(3., 3., 3.), (4., 4., 4.)]);
    println!("{:?}", aabb); // AABB { lower: (1.0, 1.0, 1.0), upper: (4.0, 4.0, 4.0) }
}

I think the problem is with recent changes to new_empty default bounds

fn new_empty<P: Point>() -> AABB<P> {

If 1 is always lower than the values provided the components of any points provided, then it will show up here. The same thing happens if the points are always lower than the default upper bound of 0:

let aabb = AABB::from_points(&[(-0.5, -0.5)]);
println!("{:?}", aabb); // AABB { lower: (-0.5, -0.5), upper: (0.0, 0.0) }

Good catch! That might be due to #162 (as of yet unreleased)

Sorry for introducing that regression. I do think we should fix from_points though instead of reverting as the current implementation does rely on the initial having min/max coordinates instead of just being empty.

I think the most straight-forward implementation would be something like

i.into_iter().map(Self::from_point).reduce(|mut lhs, rhs| {
  lhs.merge(&rhs);
  lhs
).unwrap_or(Self::new_empty())

or we could change the initial value of the fold to really use max/min-valued points?