Breaking renaming change, minor code cleanup, and improve doc comments

Author: jbuckmccready

CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to the static_aabb2d_index crate will be documented in this
 
 ## Unreleased
 
+### Added ⭐
+
+- Added `item_indices` method to get a slice over the indices for all item boxes added.
+
 ### Changed 🔧
 
 - ⚠️ BREAKING: Index now supports being empty (no longer errors when building the index if item
@@ -14,6 +18,9 @@ All notable changes to the static_aabb2d_index crate will be documented in this
   `visit_query_with_stack` function.
 - ⚠️ BREAKING: fixed inconsistency in `visit_neighbors` function to return break result the same as
   the `visit_neighbors_with_queue` function.
+- ⚠️ BREAKING: renamed `map_all_boxes_index` function to `all_box_indices` and changed signature to
+  return a slice rather than indexing into a slice internally.
+- Improved doc comments
 
 ## 0.7.1 - 2023-02-22
 

examples/index_tree_structure.rs

@@ -58,21 +58,21 @@ fn main() {
     // dbg!(index.indices_map().len());
     // dbg!(index.indices_map()[125]);
 
-    // in order to access the actual tree node relations the map_all_boxes_index method can be used
+    // in order to access the actual tree node relations the all_box_indices method can be used
     // the indices_map will return the start index of the children for a given node index
     // or map back to the added item index in the case that the node has no children
 
     // going from all_boxes index to the index the item was originally added
     for i in 0..index.count() {
-        let added_item_index = index.map_all_boxes_index(i);
+        let added_item_index = index.all_box_indices()[i];
         assert_eq!(input_boxes[added_item_index], all_boxes[i]);
     }
 
     // finding children node start index for a node
     // note: accessing index past the item count so it must be a node with children
     let parent_node_index = index.count() + 5;
-    let children_start_index = index.map_all_boxes_index(parent_node_index);
-    let children_end_index = index.map_all_boxes_index(parent_node_index + 1);
+    let children_start_index = index.all_box_indices()[parent_node_index];
+    let children_end_index = index.all_box_indices()[parent_node_index + 1];
 
     let child_indices: Vec<usize> = (children_start_index..children_end_index).collect();
     // all child boxes should be contained by their parent
@@ -85,12 +85,12 @@ fn main() {
     // here we loop through all the parent nodes and check that their child boxes are within
     // the parent nodes extents and handle the case of the root node
     for parent_node_index in index.count()..all_boxes.len() - 1 {
-        let children_start_index = index.map_all_boxes_index(parent_node_index);
+        let children_start_index = index.all_box_indices()[parent_node_index];
         let children_end_index = if parent_node_index == all_boxes.len() - 1 {
             // root node, all_boxes length is the end
             all_boxes.len()
         } else {
-            index.map_all_boxes_index(parent_node_index + 1)
+            index.all_box_indices()[parent_node_index + 1]
         };
 
         // all child boxes should be contained by their parent

src/static_aabb2d_index.rs

@@ -1,9 +1,6 @@
 use fmt::Debug;
 use std::fmt;
-use std::{
-    cmp::{max, min},
-    collections::BinaryHeap,
-};
+use std::{cmp::min, collections::BinaryHeap};
 
 use crate::{try_control, ControlFlow, IndexableNum, NeighborVisitor, QueryVisitor, AABB};
 
@@ -62,7 +59,7 @@ where
 /// Static/fixed size indexing data structure for two dimensional axis aligned bounding boxes.
 ///
 /// The index allows for fast construction and fast querying but cannot be modified after creation.
-/// This type is constructed from a [StaticAABB2DIndexBuilder].
+/// This type is constructed from a [`StaticAABB2DIndexBuilder`].
 ///
 /// 2D axis aligned bounding boxes are represented by two extent points (four values):
 /// (min_x, min_y), (max_x, max_y).
@@ -157,7 +154,7 @@ where
             };
         }
 
-        let node_size = min(max(node_size, 2), 65535);
+        let node_size = node_size.clamp(2, 65535);
 
         let mut n = num_items;
         let mut num_nodes = num_items;
@@ -186,19 +183,19 @@ where
         }
     }
 
-    /// Construct a new [StaticAABB2DIndexBuilder] to fit exactly the specified `count` number of
+    /// Construct a new [`StaticAABB2DIndexBuilder`] to fit exactly the specified `count` number of
     /// items.
     #[inline]
     pub fn new(count: usize) -> Self {
         StaticAABB2DIndexBuilder::init(count, 16)
     }
 
-    /// Construct a new [StaticAABB2DIndexBuilder] to fit exactly the specified `count` number of
+    /// Construct a new [`StaticAABB2DIndexBuilder`] to fit exactly the specified `count` number of
     /// items and use `node_size` for the index tree shape.
     ///
     /// Each node in the index tree has a maximum size which may be adjusted by `node_size` for
     /// performance reasons, however the default value of 16 when calling
-    /// `StaticAABB2DIndexBuilder::new` is tested to be optimal in most cases.
+    /// [`StaticAABB2DIndexBuilder::new`] is tested to be optimal in most cases.
     ///
     /// If `node_size` is less than 2 then 2 is used, if `node_size` is greater than 65535 then
     /// 65535 is used.
@@ -212,7 +209,8 @@ where
     ///
     /// For performance reasons the sanity checks of `min_x <= max_x` and `min_y <= max_y` are only
     /// debug asserted. If an invalid box is added it may lead to a panic or unexpected behavior
-    /// from the constructed [StaticAABB2DIndex].
+    /// from the constructed [`StaticAABB2DIndex`].
+    #[inline]
     pub fn add(&mut self, min_x: T, min_y: T, max_x: T, max_y: T) -> &mut Self {
         // catch adding past num_items (error will be returned when build is called)
         if self.pos >= self.num_items {
@@ -231,13 +229,13 @@ where
         self
     }
 
-    /// Build the [StaticAABB2DIndex] with the boxes that have been added.
+    /// Build the [`StaticAABB2DIndex`] with the boxes that have been added.
     ///
     /// 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.
+    /// 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
-    /// [StaticAABB2DIndexBuildError::NumericCastError] will be returned.
+    /// [`StaticAABB2DIndexBuildError::NumericCastError`] will be returned.
     pub fn build(mut self) -> Result<StaticAABB2DIndex<T>, StaticAABB2DIndexBuildError> {
         if self.pos != self.num_items {
             return Err(StaticAABB2DIndexBuildError::ItemCountError {
@@ -379,7 +377,7 @@ where
 /// Maps 2d space to 1d hilbert curve space.
 ///
 /// 2d space is `x: [0 -> n-1]` and `y: [0 -> n-1]`, 1d hilbert curve value space is
-/// `d: [0 -> n^2 - 1]`, where n = 2^16, so `x` and `y` must be between 0 and [u16::MAX]
+/// `d: [0 -> n^2 - 1]`, where n = 2^16, so `x` and `y` must be between 0 and [`u16::MAX`]
 /// (65535 or 2^16 - 1).
 pub fn hilbert_xy_to_index(x: u16, y: u16) -> u32 {
     let x = x as u32;
@@ -762,13 +760,15 @@ where
 
 /// Type alias for priority queue used for nearest neighbor searches.
 ///
-/// See: [StaticAABB2DIndex::visit_neighbors_with_queue].
+/// See: [`StaticAABB2DIndex::visit_neighbors_with_queue`].
 pub type NeighborPriorityQueue<T> = BinaryHeap<NeighborsState<T>>;
 
 /// Holds state for priority queue used in nearest neighbors query.
 ///
 /// Note this type is public for use in passing in an existing priority queue but
 /// all fields and constructor are private for internal use only.
+///
+/// See also: [`StaticAABB2DIndex::visit_neighbors_with_queue`].
 #[derive(Debug, Copy, Clone, PartialEq)]
 pub struct NeighborsState<T>
 where
@@ -828,18 +828,18 @@ where
         self.boxes.last().copied()
     }
 
-    /// Gets the total count of items that were added to the index.
+    /// Gets the total count of items that were added to the index during construction.
     #[inline]
     pub fn count(&self) -> usize {
         self.num_items
     }
 
-    /// Queries the index, returning a collection of indexes to items that overlap with the bounding
+    /// Queries the index, returning a collection of indices to items that overlap with the bounding
     /// box given.
     ///
     /// `min_x`, `min_y`, `max_x`, and `max_y` represent the bounding box to use for the query.
     /// Indexes returned match with the order items were added to the index using
-    /// [StaticAABB2DIndexBuilder::add].
+    /// [`StaticAABB2DIndexBuilder::add`].
     #[inline]
     pub fn query(&self, min_x: T, min_y: T, max_x: T, max_y: T) -> Vec<usize> {
         let mut results = Vec::new();
@@ -850,8 +850,8 @@ where
         results
     }
 
-    /// The same as [StaticAABB2DIndex::query] but instead of returning a [Vec] of results a lazy
-    /// iterator is returned which yields the results.
+    /// The same as [`StaticAABB2DIndex::query`] but instead of returning a [`Vec`] of results a
+    /// iterator is returned which yields the results by lazily querying the index.
     ///
     /// # Examples
     /// ```
@@ -877,8 +877,9 @@ where
         QueryIterator::<'a, T>::new(self, min_x, min_y, max_x, max_y)
     }
 
-    /// The same as [StaticAABB2DIndex::query_iter] but allows using an existing buffer for stack
-    /// traversal.
+    /// The same as [`StaticAABB2DIndex::query_iter`] but allows using an existing buffer for stack
+    /// traversal. This is useful for performance when many queries will be done repeatedly to avoid
+    /// allocating a new stack for each query (this is for performance benefit only).
     #[inline]
     pub fn query_iter_with_stack<'a>(
         &'a self,
@@ -891,9 +892,13 @@ where
         QueryIteratorStackRef::<'a, T>::new(self, stack, min_x, min_y, max_x, max_y)
     }
 
-    /// Same as [StaticAABB2DIndex::query] but instead of returning a collection of indexes a
+    /// Same as [`StaticAABB2DIndex::query`] but instead of returning a collection of indices a
     /// `visitor` function is called for each index that would be returned.  The `visitor` returns a
     /// control flow indicating whether to continue visiting or break.
+    ///
+    /// The [`ControlFlow`] and [`QueryVisitor`] traits are implemented to allow passing in a
+    /// function [`FnMut`] visitor that returns no value (all results will be visited ) or a
+    /// [`ControlFlow`] to break early.
     #[inline]
     pub fn visit_query<V, C>(&self, min_x: T, min_y: T, max_x: T, max_y: T, visitor: &mut V) -> C
     where
@@ -908,16 +913,24 @@ where
         self.visit_query_with_stack_impl(min_x, min_y, max_x, max_y, visitor, &mut stack)
     }
 
-    /// Returns all the item [AABB] that were added to the index by [StaticAABB2DIndexBuilder::add].
+    /// Returns all the item [`AABB`] that were added to the index by
+    /// [`StaticAABB2DIndexBuilder::add`] during construction.
     ///
-    /// Use [StaticAABB2DIndex::map_all_boxes_index] to map a box back to the original index
-    /// position it was added.
+    /// Use [`StaticAABB2DIndex::item_indices`] or [`StaticAABB2DIndex::all_box_indices`] to map a
+    /// box's positional index to the original index position the item was added.
     #[inline]
     pub fn item_boxes(&self) -> &[AABB<T>] {
         &self.boxes[0..self.num_items]
     }
 
-    /// Gets the node size used for the [StaticAABB2DIndex].
+    /// Used to map an item box index position from [`StaticAABB2DIndex::item_boxes`] back to the
+    /// original index position the item was added.
+    #[inline]
+    pub fn item_indices(&self) -> &[usize] {
+        &self.indices[0..self.num_items]
+    }
+
+    /// Gets the node size used for the index.
     ///
     /// The node size is the maximum number of boxes stored as children of each node in the index
     /// tree.
@@ -926,40 +939,38 @@ where
         self.node_size
     }
 
-    /// Gets the level bounds for all the boxes in the [StaticAABB2DIndex].
+    /// Gets the level bounds for all the boxes in the index.
     ///
-    /// The level bounds are the index positions in [StaticAABB2DIndex::all_boxes] where a change in
-    /// the level of the index tree occurs.
+    /// The level bounds are the index positions in [`StaticAABB2DIndex::all_boxes`] where a change
+    /// in the level of the index tree occurs.
     #[inline]
     pub fn level_bounds(&self) -> &[usize] {
         &self.level_bounds
     }
 
-    /// Gets all the bounding boxes for the [StaticAABB2DIndex].
+    /// Gets all the bounding boxes for the index.
     ///
     /// The boxes are ordered from the bottom of the tree up, so from 0 to
-    /// [StaticAABB2DIndex::count] are all the item bounding boxes. Use
-    /// [StaticAABB2DIndex::map_all_boxes_index] to map a box back to the original index position it
+    /// [`StaticAABB2DIndex::count`] are all the item bounding boxes. Use
+    /// [`StaticAABB2DIndex::all_box_indices`] to map a box back to the original index position it
     /// was added or find the start position for the children of a node box.
     #[inline]
     pub fn all_boxes(&self) -> &[AABB<T>] {
         &self.boxes
     }
 
-    /// Gets the original item index position (from the time it was added) from a
-    /// [StaticAABB2DIndex::all_boxes] slice index position.
-    ///
-    /// If `all_boxes_index` is greater than [StaticAABB2DIndex::count] then it will return the
-    /// [StaticAABB2DIndex::all_boxes] starting index of the node's children boxes.
-    /// See the index_tree_structure.rs example for more information.
+    /// Used to map an item box index position from [`StaticAABB2DIndex::all_boxes`] back to the
+    /// original index position the item was added. Or if indexing past [`StaticAABB2DIndex::count`]
+    /// it will yield the [`StaticAABB2DIndex::all_boxes`] starting index of the node's children
+    /// boxes. See the `index_tree_structure.rs` example for more information.
     #[inline]
-    pub fn map_all_boxes_index(&self, all_boxes_index: usize) -> usize {
-        self.indices[all_boxes_index]
+    pub fn all_box_indices(&self) -> &[usize] {
+        &self.indices
     }
 
-    /// Same as [StaticAABB2DIndex::query] but accepts an existing [Vec] to be used as a stack
-    /// buffer when performing the query to avoid the need for allocation (this is for performance
-    /// benefit only).
+    /// Same as [`StaticAABB2DIndex::query`] but allows using an existing buffer for stack
+    /// traversal. This is useful for performance when many queries will be done repeatedly to avoid
+    /// allocating a new stack for each query (this is for performance benefit only).
     #[inline]
     pub fn query_with_stack(
         &self,
@@ -977,9 +988,9 @@ where
         results
     }
 
-    /// Same as [StaticAABB2DIndex::visit_query] but accepts an existing [Vec] to be used as a stack
-    /// buffer when performing the query to avoid the need for allocation (this is for performance
-    /// benefit only).
+    /// Same as [`StaticAABB2DIndex::visit_query`] but allows using an existing buffer for stack
+    /// traversal. This is useful for performance when many queries will be done repeatedly to avoid
+    /// allocating a new stack for each query (this is for performance benefit only).
     #[inline]
     pub fn visit_query_with_stack<V, C>(
         &self,
@@ -1062,8 +1073,8 @@ where
     /// * Because distances are squared (`dx * dx + dy * dy`) be cautious of smaller numeric types
     ///   overflowing (e.g. it's easy to overflow an i32 with squared distances).
     /// * If the point is inside of an item's bounding box then the euclidean distance is 0.
-    /// * If repeatedly calling this method then [StaticAABB2DIndex::visit_neighbors_with_queue] can
-    ///   be used to avoid repeated allocations for the priority queue used internally.
+    /// * If repeatedly calling this method then [`StaticAABB2DIndex::visit_neighbors_with_queue`]
+    ///   can be used to avoid repeated allocations for the priority queue used internally.
     #[inline]
     pub fn visit_neighbors<V, C>(&self, x: T, y: T, visitor: &mut V) -> C
     where
@@ -1078,7 +1089,7 @@ where
         self.visit_neighbors_with_queue_impl(x, y, visitor, &mut queue)
     }
 
-    /// Works the same as [StaticAABB2DIndex::visit_neighbors] but accepts an existing binary heap
+    /// Works the same as [`StaticAABB2DIndex::visit_neighbors`] but accepts an existing binary heap
     /// to be used as a priority queue to avoid allocations.
     #[inline]
     pub fn visit_neighbors_with_queue<V, C>(

tests/test.rs

@@ -212,19 +212,19 @@ fn many_tree_levels() {
 
     let all_boxes = index.all_boxes();
 
-    // map_all_boxes_index should map back to original aabb index
+    // all_box_indices should map back to original aabb index
     for (i, curr_box) in all_boxes.iter().enumerate().take(index.count()) {
-        let added_item_index = index.map_all_boxes_index(i);
+        let added_item_index = index.all_box_indices()[i];
         assert_eq!(&input_boxes[added_item_index], curr_box);
     }
 
-    // map_all_boxes_index should get child start index
+    // all_box_indices should get child start index
     for parent_node_index in index.count()..all_boxes.len() - 1 {
-        let children_start_index = index.map_all_boxes_index(parent_node_index);
+        let children_start_index = index.all_box_indices()[parent_node_index];
         let children_end_index = if parent_node_index == all_boxes.len() - 1 {
             all_boxes.len()
         } else {
-            index.map_all_boxes_index(parent_node_index + 1)
+            index.all_box_indices()[parent_node_index + 1]
         };
 
         // all child boxes should be contained by their parent