Fixed sorting for integer types and 2.0.0 release

- Use f64 for creating hilbert coordinate values (fixes issue when using
  integer numeric types)
- Implement IndexableNum trait for i8, u8, and i16 types
- Rearrange hilbert coordinate value math expressions for performance
  improvement
- Added expected indices order test
- 2.0.0 version bump and change notes

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 🔧

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

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);

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<f64> = 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<StaticAABB2DIndex<T>, StaticAABB2DIndexBuildError> {
         if self.pos != self.num_items {
@@ -362,33 +362,55 @@ 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<f64, StaticAABB2DIndexBuildError> {
+            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
         // of the entire set of bounding boxes maps to n - 1 our 2d space is x: [0 -> n-1] and
         // y: [0 -> n-1], our 1d hilbert curve value space is d: [0 -> n^2 - 1]
         let mut hilbert_values: Vec<u32> = 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));
         }
 

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();