From a675f1322e414d88da30dbcdc62024e5fa99b9e0 Mon Sep 17 00:00:00 2001 From: Nicolas Mellado Date: Thu, 14 Dec 2023 16:27:23 +0100 Subject: [PATCH 1/6] attempt to make the API easier to extend --- .../SpatialPartitioning/KdTree/kdTreeTraits.h | 75 ++++++++++++------- 1 file changed, 49 insertions(+), 26 deletions(-) diff --git a/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h b/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h index 6f3cb155..257abcb5 100644 --- a/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h +++ b/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h @@ -69,7 +69,7 @@ struct KdTreeDefaultInnerNode INDEX_BITS = sizeof(UIndex)*8 - DIM_BITS, }; - Scalar split_value; + Scalar split_value{0}; UIndex first_child_id : INDEX_BITS; UIndex split_dim : DIM_BITS; }; @@ -77,21 +77,23 @@ struct KdTreeDefaultInnerNode template struct KdTreeDefaultLeafNode { - Index start; - Size size; + Index start{0}; + Size size{0}; }; /*! * \brief The node type used by default by the kd-tree. */ template -class KdTreeDefaultNode + typename LeafSize = Index, + typename _InnerNodeType = KdTreeDefaultInnerNode, + typename _LeafNodeType = KdTreeDefaultLeafNode> +class KdTreeCustomizableNode { private: using Scalar = typename DataPoint::Scalar; - using LeafType = KdTreeDefaultLeafNode; - using InnerType = KdTreeDefaultInnerNode; + using InnerType = _InnerNodeType; + using LeafType = _LeafNodeType; public: enum @@ -110,10 +112,8 @@ class KdTreeDefaultNode * `DataPoint::VectorType`. */ using AabbType = Eigen::AlignedBox; - - KdTreeDefaultNode() = default; - bool is_leaf() const { return m_is_leaf; } + [[nodiscard]] bool is_leaf() const { return m_is_leaf; } void set_is_leaf(bool is_leaf) { m_is_leaf = is_leaf; } /*! @@ -132,8 +132,8 @@ class KdTreeDefaultNode { if (m_is_leaf) { - m_leaf.start = start; - m_leaf.size = (LeafSize)size; + data.m_leaf.start = start; + data.m_leaf.size = (LeafSize)size; } } @@ -150,9 +150,9 @@ class KdTreeDefaultNode { if (!m_is_leaf) { - m_inner.split_value = split_value; - m_inner.first_child_id = first_child_id; - m_inner.split_dim = split_dim; + data.m_inner.split_value = split_value; + data.m_inner.first_child_id = first_child_id; + data.m_inner.split_dim = split_dim; } } @@ -160,22 +160,22 @@ 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; } + [[nodiscard]] Index leaf_start() const { return data.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; } + [[nodiscard]] LeafSize leaf_size() const { return data.m_leaf.size; } /*! * \brief The position of the AABB split of the inner node. */ - Scalar inner_split_value() const { return m_inner.split_value; } + [[nodiscard]] Scalar inner_split_value() const { return data.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; } + [[nodiscard]] int inner_split_dim() const { return (int)data.m_inner.split_dim; } /*! * \brief The index of the first child of the node in the node array of the @@ -184,21 +184,44 @@ class KdTreeDefaultNode * \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; } + [[nodiscard]] Index inner_first_child_id() const { return (Index)data.m_inner.first_child_id; } + +protected: + [[nodiscard]] inline LeafType& getAsLeaf() { return data.m_leaf; } + [[nodiscard]] inline InnerType& getAsInner() { return data.m_inner; } private: - bool m_is_leaf; - union + bool m_is_leaf{true}; + union Data { - KdTreeDefaultLeafNode m_leaf; - KdTreeDefaultInnerNode m_inner; + // We need an explicit constructor here, see https://stackoverflow.com/a/70428826 + constexpr Data() : m_leaf() {} + LeafType m_leaf; + InnerType m_inner; }; + Data data; +}; + +template +struct KdTreeDefaultNode : public KdTreeCustomizableNode, + KdTreeDefaultLeafNode> { + using Base = KdTreeCustomizableNode, + KdTreeDefaultLeafNode>; }; /*! * \brief The default traits type used by the kd-tree. + * + * \tparam _NodeType Type used to store nodes, set by default to #KdTreeDefaultNode */ -template +template typename _NodeType = KdTreeDefaultNode> struct KdTreeDefaultTraits { enum @@ -228,7 +251,7 @@ struct KdTreeDefaultTraits // Nodes using NodeIndexType = std::size_t; - using NodeType = KdTreeDefaultNode; + using NodeType = _NodeType; using NodeContainer = std::vector; }; } // namespace Ponca From c4e56532ea43544972155f08d52500174abad629 Mon Sep 17 00:00:00 2001 From: Nicolas Mellado Date: Thu, 14 Dec 2023 21:04:24 +0100 Subject: [PATCH 2/6] Fix deleted destructor --- Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h b/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h index 257abcb5..adb0d81b 100644 --- a/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h +++ b/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h @@ -196,6 +196,8 @@ class KdTreeCustomizableNode { // We need an explicit constructor here, see https://stackoverflow.com/a/70428826 constexpr Data() : m_leaf() {} + constexpr Data(Data&&d) = default; + ~Data() {} LeafType m_leaf; InnerType m_inner; }; From b24dcdff3a93047857b0ec7d9444de45982effc5 Mon Sep 17 00:00:00 2001 From: Nicolas Mellado Date: Thu, 14 Dec 2023 21:11:00 +0100 Subject: [PATCH 3/6] Fix MoveInsertable requirement --- Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h b/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h index adb0d81b..f1ced56b 100644 --- a/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h +++ b/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h @@ -196,7 +196,9 @@ class KdTreeCustomizableNode { // We need an explicit constructor here, see https://stackoverflow.com/a/70428826 constexpr Data() : m_leaf() {} - constexpr Data(Data&&d) = default; + // Needed to satisfy MoveInsertable requirement https://en.cppreference.com/w/cpp/named_req/MoveInsertable + constexpr Data(const Data&d) : m_leaf(d.m_leaf) {} + ~Data() {} LeafType m_leaf; InnerType m_inner; From 5591be5c5e3c31a90e6e83279fa0cd70652d15ed Mon Sep 17 00:00:00 2001 From: Nicolas Mellado Date: Thu, 14 Dec 2023 21:42:59 +0100 Subject: [PATCH 4/6] Add missing const accessors --- Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h b/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h index f1ced56b..88363f4c 100644 --- a/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h +++ b/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h @@ -189,6 +189,8 @@ class KdTreeCustomizableNode protected: [[nodiscard]] inline LeafType& getAsLeaf() { return data.m_leaf; } [[nodiscard]] inline InnerType& getAsInner() { return data.m_inner; } + [[nodiscard]] inline const LeafType& getAsLeaf() const { return data.m_leaf; } + [[nodiscard]] inline const InnerType& getAsInner() const { return data.m_inner; } private: bool m_is_leaf{true}; From 2f5faa7685d700f1f87585763660713454314872 Mon Sep 17 00:00:00 2001 From: Nicolas Mellado Date: Thu, 14 Dec 2023 23:41:38 +0100 Subject: [PATCH 5/6] [doc] Update documentation to highlight new functionality --- .../SpatialPartitioning/KdTree/kdTreeTraits.h | 20 ++++ doc/src/ponca_module_spatialpartitioning.mdoc | 3 +- examples/cpp/CMakeLists.txt | 10 +- examples/cpp/ponca_customize_kdtree.cpp | 93 +++++++++++++++++++ 4 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 examples/cpp/ponca_customize_kdtree.cpp diff --git a/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h b/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h index 88363f4c..e96675cb 100644 --- a/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h +++ b/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h @@ -83,6 +83,24 @@ struct KdTreeDefaultLeafNode /*! * \brief The node type used by default by the kd-tree. + * + * It is possible to modify the Inner and Leaf node types by inheritance. For instance, to add a Bounding box to inner + * nodes, define a custom inner node type: + * + * \snippet ponca_customize_kdtree.cpp CustomInnerNodeDefinition + * + * Define a custom node type to use it, and expose custom data (inner/leaf node are not exposed directly): + * + * \snippet ponca_customize_kdtree.cpp CustomNodeDefinition + * + * To use in the KdTree, define a type using the custom node: + * + * \snippet ponca_customize_kdtree.cpp KdTreeTypeWithCustomNode + * + * The added attribute can be accessed + * + * \snippet ponca_customize_kdtree.cpp ReadCustomProperties + * */ template +#include +#include +#include + +struct DataPoint +{ + enum {Dim = 3}; + using Scalar = float; + using VectorType = Eigen::Vector; + inline const auto& pos() const {return m_pos;} + VectorType m_pos; +}; + +//! [CustomInnerNodeDefinition] +template > +struct MyKdTreeInnerNode : public Ponca::KdTreeDefaultInnerNode { + using AabbType = _AabbType; + AabbType m_aabb{}; +}; +//! [CustomInnerNodeDefinition] + +//! [CustomNodeDefinition] +template +struct MyKdTreeNode : Ponca::KdTreeCustomizableNode> { + + using Base = Ponca::KdTreeCustomizableNode>; + using AabbType = typename Base::AabbType; + + void configure_range(Index start, Index size, const AabbType &aabb) + { + Base::configure_range(start, size, aabb); + if (! Base::is_leaf() ) + { + Base::getAsInner().m_aabb = aabb; + } + } + [[nodiscard]] inline std::optional getAabb() const { + if (! Base::is_leaf()) + return Base::getAsInner().m_aabb; + else + return std::optional(); + } +}; +//! [CustomNodeDefinition] + +int main() +{ + // generate N random points + constexpr int N = 1e5; + std::vector points(N); + std::generate(points.begin(), points.end(), [](){ + return DataPoint{100 * DataPoint::VectorType::Random()};}); + +//! [KdTreeTypeWithCustomNode] + using CustomKdTree = Ponca::KdTreeBase>; +//! [KdTreeTypeWithCustomNode] + + // build the k-d tree + const CustomKdTree kdtree(points); + + // neighbor searches are done below from these arbitrary index and point + const int query_idx = 10; + const DataPoint::VectorType query_pt{-10.0, 0.5, 75.0}; + + // + // nearest neighbor search + // + std::cout << "the nearest neighbor of the point at index " << query_idx << " is at index " + << *kdtree.nearest_neighbor(query_idx).begin() << std::endl; + std::cout << "the nearest neighbor of the point (" << query_pt.transpose() << ") is at index " + << *kdtree.nearest_neighbor(query_pt).begin() << std::endl; + + //! [ReadCustomProperties] + auto bbox = kdtree.node_data()[0].getAabb(); + if (bbox) { + std::cout << "Root bounding box is as follows: \n" + << " Center: " << bbox->center() + << " Diagonal: " << bbox->diagonal() + << std::endl; + } + //! [ReadCustomProperties] + + return 1; +} From 10631b9fb98ea31f8cde5eb76974aab9b71ccd8c Mon Sep 17 00:00:00 2001 From: Nicolas Mellado Date: Thu, 14 Dec 2023 23:52:51 +0100 Subject: [PATCH 6/6] Update CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 90753c8d..fe1871e2 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,6 +8,7 @@ Current head (v.1.2 RC) - API - [spatialPartitioning] Optimize memory use in knngraph queries (#104) - [spatialPartitioning] Clean KdTree API (#122) + - [spatialPartitioning] Simplify node customization (#128) - [fitting] Mark `Base` type as protected instead of private in CRTP classes (#119) - [fitting] Improve KdTreeNodes API by hiding internal memory layout, improve methods naming (#120)