[SpatialPartitioning] Clean up new node interface and add docs

poncateam · Nov 30, 2023 · fccd053 · fccd053
1 parent 2608d84
commit fccd053
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 28 deletions.
diff --git a/Ponca/src/Common/Macro.h b/Ponca/src/Common/Macro.h
@@ -43,3 +43,12 @@
     PONCA_CRASH;                                                                \
     PONCA_MACRO_END
 
+#ifdef __has_builtin
+#if __has_builtin(__builtin_clz)
+#define PONCA_HAS_BUILTIN_CLZ 1
+#endif
+#endif
+
+#ifndef PONCA_HAS_BUILTIN_CLZ
+#define PONCA_HAS_BUILTIN_CLZ 0
+#endif
diff --git a/Ponca/src/SpatialPartitioning/KdTree/kdTree.h b/Ponca/src/SpatialPartitioning/KdTree/kdTree.h
@@ -70,8 +70,12 @@ class KdTreeBase
     enum
     {
         /*!
-         * The maximum number of points that can be stored in the kd-tree,
-         * considering the bit width of the index type.
+         * \brief The maximum number of nodes that the kd-tree can have.
+         */
+        MAX_NODE_COUNT = NodeType::MAX_COUNT,
+
+        /*!
+         * \brief The maximum number of points that can be stored in the kd-tree.
          */
         MAX_POINT_COUNT = std::size_t(2) << sizeof(IndexType)*8,
     };

diff --git a/Ponca/src/SpatialPartitioning/KdTree/kdTree.hpp b/Ponca/src/SpatialPartitioning/KdTree/kdTree.hpp
@@ -160,7 +160,13 @@ void KdTreeBase<Traits>::build_rec(NodeIndexType node_id, IndexType start, Index
     for(IndexType i=start; i<end; ++i)
         aabb.extend(m_points[m_indices[i]].pos());
 
-    node.set_is_leaf(end-start <= m_min_cell_size || level >= Traits::MAX_DEPTH);
+    node.set_is_leaf(
+        end-start <= m_min_cell_size ||
+        level >= Traits::MAX_DEPTH ||
+        // Since we add 2 nodes per inner node we need to stop if we can't add
+        // them both
+        (NodeIndexType)m_nodes.size() > MAX_NODE_COUNT - 2);
+
     node.configure_range(start, end-start, aabb);
     if (node.is_leaf())
     {

diff --git a/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h b/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h
@@ -7,19 +7,11 @@
 
 #pragma once
 
-#include <Eigen/Geometry>
+#include "../../Common/Macro.h"
 
 #include <cstddef>
 
-#ifdef __has_builtin
-#if __has_builtin(__builtin_clz)
-#define PONCA_HAS_BUILTIN_CLZ 1
-#endif
-#endif
-
-#ifndef PONCA_HAS_BUILTIN_CLZ
-#define PONCA_HAS_BUILTIN_CLZ 0
-#endif
+#include <Eigen/Geometry>
 
 namespace Ponca {
 #ifndef PARSED_WITH_DOXYGEN
@@ -50,9 +42,6 @@ template <typename NodeIndex, typename Scalar, int DIM>
 struct KdTreeDefaultInnerNode
 {
 private:
-    // We're using unsigned indices since we're using bitfields.
-    using UIndex = typename std::make_unsigned<NodeIndex>::type;
-
     enum
     {
         // The minimum bit width required to store the split dimension.
@@ -68,6 +57,9 @@ struct KdTreeDefaultInnerNode
         DIM_BITS = sizeof(unsigned int)*8 - internal::clz((unsigned int)DIM),
     };
 
+    // The node stores bitfields as unsigned indices.
+    using UIndex = typename std::make_unsigned<NodeIndex>::type;
+
 public:
     enum
     {
@@ -97,9 +89,20 @@ template <typename Index, typename NodeIndex, typename DataPoint,
 class KdTreeDefaultNode
 {
 private:
-    using Scalar = typename DataPoint::Scalar;
+    using Scalar    = typename DataPoint::Scalar;
+    using LeafType  = KdTreeDefaultLeafNode<Index, LeafSize>;
+    using InnerType = KdTreeDefaultInnerNode<NodeIndex, Scalar, DataPoint::Dim>;
 
 public:
+    enum
+    {
+        /*!
+         * \brief The maximum number of nodes that a kd-tree can have when using
+         * this node type.
+         */
+        MAX_COUNT = std::size_t(2) << InnerType::INDEX_BITS,
+    };
+
     /*!
      * \brief The type used to store node bounding boxes.
      *
@@ -109,14 +112,22 @@ class KdTreeDefaultNode
     using AabbType = Eigen::AlignedBox<Scalar, DataPoint::Dim>;
 
     KdTreeDefaultNode() = default;
-
-    /*!*/
+
     bool is_leaf() const { return m_is_leaf; }
-
-    /*!*/
     void set_is_leaf(bool is_leaf) { m_is_leaf = is_leaf; }
 
-    /*!*/
+    /*!
+     * \brief Configures the range of the node in the sample index array of the
+     * kd-tree.
+     *
+     * \see the leaf node accessors for a more detailed explanation of each
+     * argument.
+     *
+     * \note The AABB is not required by the implementation, so nodes don't
+     * have to make it available.
+     *
+     * Called after \ref set_is_leaf during kd-tree construction.
+     */
     void configure_range(Index start, Index size, const AabbType &aabb)
     {
         if (m_is_leaf)
@@ -126,7 +137,15 @@ class KdTreeDefaultNode
         }
     }
 
-    /*!*/
+    /*!
+     * \brief Configures the inner node information.
+     *
+     * \see the inner node accessors for a more detailed explanation of each
+     * argument.
+     *
+     * Called after \ref set_is_leaf and \ref configure_range during kd-tree
+     * construction.
+     */
     void configure_inner(Scalar split_value, Index first_child_id, Index split_dim)
     {
         if (!m_is_leaf)
@@ -137,19 +156,34 @@ class KdTreeDefaultNode
         }
     }
 
-    /*!*/
+    /*!
+     * \brief The start index of the range of the leaf node in the sample
+     * index array.
+     */
     Index leaf_start() const { return m_leaf.start; }
 
-    /*!*/
+    /*!
+     * \brief The size of the range of the leaf node in the sample index array.
+     */
     LeafSize leaf_size() const { return m_leaf.size; }
 
-    /*!*/
+    /*!
+     * \brief The position of the AABB split of the inner node.
+     */
     Scalar inner_split_value() const { return m_inner.split_value; }
 
-    /*!*/
+    /*!
+     * \brief Which axis the split of the AABB of the inner node was done on.
+     */
     int inner_split_dim() const { return (int)m_inner.split_dim; }
 
-    /*!*/
+    /*!
+     * \brief The index of the first child of the node in the node array of the
+     * kd-tree.
+     *
+     * \note The second child is stored directly after the first in the array
+     * (i.e. `first_child_id + 1`).
+     */
     Index inner_first_child_id() const { return (Index)m_inner.first_child_id; }
 
 private: