diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d1b2ed..1e5319f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,30 @@ All notable changes to the static_aabb2d_index crate will be documented in this file. +## 2.0.0 - 2023-09-04 + +### Fixed 🐛 + +- Fixed building index properly for integer types. Previously due to integer division truncation + and the way the hilbert coordinate values were computed the index was not properly + sorted/structured when using integer values (this lead to a less optimal tree for querying, + despite everything still querying properly). Hilbert coordinate values are now computed using + `f64` to properly scale and work in all integer cases. This improves performance of the index when + using integers. + +### Changed 🔧 + +- ⚠️ MINOR BREAKING: numeric type used is now required to successfully cast to a `f64` (previously + expected to cast to/from `u16`). This is a very minor breaking change, and only arises when using + a non-builtin numeric type that for whatever reason cannot cast to a `f64` but is able to cast + to/from a `u16`. Major version number was bumped since this is technically a breaking change for + a weird edge use case. +- Numeric type used no longer requires casting to/from a `u16`. +- Rearranged math expressions when building hilbert coordinate values for performance improvement + (~5%). +- Implemented the `IndexableNum` trait for `i8`, `u8`, and `i16` types (those types are now + supported, this came as part of the fix for integer types). + ## 1.1.0 - 2023-08-31 ### Changed 🔧 diff --git a/Cargo.toml b/Cargo.toml index a145484..ece429f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ keywords = ["algorithm", "2d", "computational", "geometry", "spatial"] license = "MIT OR Apache-2.0" name = "static_aabb2d_index" repository = "https://github.com/jbuckmccready/static_aabb2d_index" -version = "1.1.0" +version = "2.0.0" [features] # allows for some selective use of unsafe for performance gains diff --git a/src/core.rs b/src/core.rs index afc49ad..cc22994 100644 --- a/src/core.rs +++ b/src/core.rs @@ -51,8 +51,9 @@ macro_rules! impl_indexable_num_for_ord_type { } // impl for all supported built in types -// note that other builtin primitive numbers are not supported -// since the type must cast to/from u16 to be supported +impl_indexable_num_for_ord_type!(i8); +impl_indexable_num_for_ord_type!(u8); +impl_indexable_num_for_ord_type!(i16); impl_indexable_num_for_ord_type!(u16); impl_indexable_num_for_ord_type!(i32); impl_indexable_num_for_ord_type!(u32); diff --git a/src/static_aabb2d_index.rs b/src/static_aabb2d_index.rs index c1698c8..7258291 100644 --- a/src/static_aabb2d_index.rs +++ b/src/static_aabb2d_index.rs @@ -1,4 +1,5 @@ use fmt::Debug; +use num_traits::ToPrimitive; use std::fmt; use std::{cmp::min, collections::BinaryHeap}; @@ -15,7 +16,7 @@ pub enum StaticAABB2DIndexBuildError { /// The number of items that were expected (set at construction). expected: usize, }, - /// Error for the case when the numeric type T used for the index fails to cast to/from u16. + /// Error for the case when the numeric type T used for the index fails to cast to f64. NumericCastError, } @@ -30,10 +31,9 @@ impl fmt::Display for StaticAABB2DIndexBuildError { (added: {}, expected: {})", added, expected ), - StaticAABB2DIndexBuildError::NumericCastError => write!( - f, - "numeric cast of type T to/from u16 failed (may be due to overflow/underflow)" - ), + StaticAABB2DIndexBuildError::NumericCastError => { + write!(f, "numeric type T used for index failed to cast to f64") + } } } } @@ -76,7 +76,7 @@ where /// builder.add(0.0, 0.0, 1.0, 3.0); /// builder.add(4.0, 2.0, 16.0, 8.0); /// // note build may return an error if the number of added boxes does not equal the static size -/// // given at the time the builder was created or the type used fails to cast to/from a u16 +/// // given at the time the builder was created or the type used fails to cast to a f64 /// let index: StaticAABB2DIndex = builder.build().unwrap(); /// // query the created index (min_x, min_y, max_x, max_y) /// let query_results = index.query(-1.0, -1.0, -0.5, -0.5); @@ -290,7 +290,7 @@ where /// If the number of added items does not match the count given at the time the builder was /// created then a [`StaticAABB2DIndexBuildError::ItemCountError`] will be returned. /// - /// If the numeric type T fails to cast to/from a u16 for any reason then a + /// If the numeric type T fails to cast to a f64 for any reason then a /// [`StaticAABB2DIndexBuildError::NumericCastError`] will be returned. pub fn build(mut self) -> Result, StaticAABB2DIndexBuildError> { if self.pos != self.num_items { @@ -362,12 +362,41 @@ where }); } - let width = max_x - min_x; - let height = max_y - min_y; + // helper function to cast T to f64 + let cast_to_f64 = |x: T| -> Result { + x.to_f64() + .ok_or(StaticAABB2DIndexBuildError::NumericCastError) + }; + + let width = cast_to_f64(max_x - min_x)?; + let height = cast_to_f64(max_y - min_y)?; + let extent_min_x = cast_to_f64(min_x)?; + let extent_min_y = cast_to_f64(min_y)?; // hilbert max input value for x and y - let hilbert_max = T::from(u16::MAX).ok_or(StaticAABB2DIndexBuildError::NumericCastError)?; - let two = T::from(2u16).ok_or(StaticAABB2DIndexBuildError::NumericCastError)?; + let hilbert_max = u16::MAX as f64; + let scaled_width = hilbert_max / width; + let scaled_height = hilbert_max / height; + + // helper function to build hilbert coordinate value from AABB + fn hilbert_coord(scaled_extent: f64, aabb_min: f64, aabb_max: f64, extent_min: f64) -> u16 { + let value = scaled_extent * (0.5 * (aabb_min + aabb_max) - extent_min); + // this should successfully convert to u16 since scaled_extent should be between 0 and + // u16::MAX and the coefficient should be between 0.0 and 1.0, but in the case of + // positive/negative infinity (width or height is 0.0) or NAN (inputs contain NAN) we + // want to continue + value.to_u16().unwrap_or( + // saturate + if value > u16::MAX as f64 { + u16::MAX + } else if value < u16::MIN as f64 { + u16::MIN + } else { + // NAN + 0 + }, + ) + } // mapping the x and y coordinates of the center of the item boxes to values in the range // [0 -> n - 1] such that the min of the entire set of bounding boxes maps to 0 and the max @@ -375,20 +404,13 @@ where // y: [0 -> n-1], our 1d hilbert curve value space is d: [0 -> n^2 - 1] let mut hilbert_values: Vec = Vec::with_capacity(self.num_items); for aabb in item_boxes.iter() { - let x = if width == T::zero() { - 0 - } else { - (hilbert_max * ((aabb.min_x + aabb.max_x) / two - min_x) / width) - .to_u16() - .ok_or(StaticAABB2DIndexBuildError::NumericCastError)? - }; - let y = if height == T::zero() { - 0 - } else { - (hilbert_max * ((aabb.min_y + aabb.max_y) / two - min_y) / height) - .to_u16() - .ok_or(StaticAABB2DIndexBuildError::NumericCastError)? - }; + let aabb_min_x = cast_to_f64(aabb.min_x)?; + let aabb_min_y = cast_to_f64(aabb.min_y)?; + let aabb_max_x = cast_to_f64(aabb.max_x)?; + let aabb_max_y = cast_to_f64(aabb.max_y)?; + + let x = hilbert_coord(scaled_width, aabb_min_x, aabb_max_x, extent_min_x); + let y = hilbert_coord(scaled_height, aabb_min_y, aabb_max_y, extent_min_y); hilbert_values.push(hilbert_xy_to_index(x, y)); } diff --git a/tests/test.rs b/tests/test.rs index 592d2f6..5cd7351 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -246,6 +246,19 @@ fn total_extents() { assert_eq!(total_bounds.max_y, 95); } +#[test] +fn expected_indices_order() { + let index = create_test_index(); + let expected_indices = &[ + 95, 92, 87, 70, 67, 64, 55, 52, 49, 43, 40, 11, 26, 19, 44, 9, 59, 84, 77, 39, 6, 75, 80, + 18, 23, 62, 58, 88, 86, 27, 90, 0, 73, 7, 37, 30, 13, 14, 48, 17, 56, 79, 25, 38, 85, 76, + 91, 66, 24, 33, 21, 3, 99, 16, 54, 28, 29, 68, 50, 31, 22, 72, 78, 83, 53, 89, 51, 93, 81, + 20, 8, 96, 4, 63, 74, 5, 47, 32, 10, 98, 61, 82, 57, 97, 65, 35, 41, 2, 45, 46, 36, 42, 69, + 34, 1, 60, 15, 94, 12, 71, 0, 16, 32, 48, 64, 80, 96, 100, + ]; + assert_eq!(index.all_box_indices(), expected_indices); +} + #[test] fn query() { let index = create_test_index();