Skip to content

Commit

Permalink
Fix AABB::from_points which relied on implementation details of AABB:…
Browse files Browse the repository at this point in the history
…:new_empty (#171)

- [x] I agree to follow the project's [code of
conduct](https://github.com/georust/geo/blob/master/CODE_OF_CONDUCT.md).
- [x] I added an entry to `rstar/CHANGELOG.md` if knowledge of this
change could be valuable to users.
---

This method does not just need an empty AABB, but actually requires that
the lower/upper corners use max/min values of the point coordinates.

This change therefore open-codes this to keep these invariants and the
code that relies on them close together.

A changelog entry should not be necessary as we luckily did not yet
release the regression.

Closes #170
  • Loading branch information
adamreichold authored Jun 28, 2024
1 parent 0139255 commit 7634435
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 16 deletions.
28 changes: 17 additions & 11 deletions rstar/src/aabb.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::point::{max_inline, Point, PointExt};
use crate::{Envelope, RTreeObject};
use num_traits::{One, Zero};
use num_traits::{Bounded, One, Zero};

#[cfg(feature = "serde")]
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -70,16 +70,16 @@ where
I: IntoIterator<Item = &'a P> + 'a,
P: 'a,
{
i.into_iter()
.fold(Self::new_empty(), |aabb, p| aabb.add_point(p))
}

/// Returns the AABB that contains `self` and another point.
fn add_point(&self, point: &P) -> Self {
AABB {
lower: self.lower.min_point(point),
upper: self.upper.max_point(point),
}
i.into_iter().fold(
Self {
lower: P::from_value(P::Scalar::max_value()),
upper: P::from_value(P::Scalar::min_value()),
},
|aabb, p| Self {
lower: aabb.lower.min_point(p),
upper: aabb.upper.max_point(p),
},
)
}

/// Returns the point within this AABB closest to a given point.
Expand Down Expand Up @@ -246,4 +246,10 @@ mod test {
let corner = [a[0], b[1], a[2]];
assert_eq!(aabb.min_max_dist_2(&p), corner.distance_2(&p));
}

#[test]
fn test_from_points_issue_170_regression() {
let aabb = AABB::from_points(&[(3., 3., 3.), (4., 4., 4.)]);
assert_eq!(aabb, AABB::from_corners((3., 3., 3.), (4., 4., 4.)));
}
}
4 changes: 2 additions & 2 deletions rstar/src/envelope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ pub trait Envelope: Clone + PartialEq + ::core::fmt::Debug {
///
/// # Notes
/// - While euclidean distance will be the correct choice for most use cases, any distance metric
/// fulfilling the [usual axioms](https://en.wikipedia.org/wiki/Metric_space)
/// can be used when implementing this method
/// fulfilling the [usual axioms](https://en.wikipedia.org/wiki/Metric_space)
/// can be used when implementing this method
/// - Implementers **must** ensure that the distance metric used matches that of [crate::PointDistance::distance_2]
fn distance_2(&self, point: &Self::Point) -> <Self::Point as Point>::Scalar;

Expand Down
4 changes: 2 additions & 2 deletions rstar/src/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ pub trait PointDistance: RTreeObject {
///
/// # Notes
/// - While euclidean distance will be the correct choice for most use cases, any distance metric
/// fulfilling the [usual axioms](https://en.wikipedia.org/wiki/Metric_space)
/// can be used when implementing this method
/// fulfilling the [usual axioms](https://en.wikipedia.org/wiki/Metric_space)
/// can be used when implementing this method
/// - Implementers **must** ensure that the distance metric used matches that of [crate::Envelope::distance_2]
fn distance_2(
&self,
Expand Down
2 changes: 1 addition & 1 deletion rstar/src/rtree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ where
/// # Type Parameters
/// * `T`: The type of objects stored in the r-tree.
/// * `Params`: Compile time parameters that change the r-tree's internal layout. Refer to the
/// [RTreeParams] trait for more information.
/// [RTreeParams] trait for more information.
///
/// # Defining methods generic over r-trees
/// If a library defines a method that should be generic over the r-tree type signature, make
Expand Down

0 comments on commit 7634435

Please sign in to comment.