Merge branch 'master' into prepare_v.1.2

poncateam · Dec 15, 2023 · e7d97f6 · e7d97f6
2 parents dccf9bc + cfbe035
commit e7d97f6
Show file tree

Hide file tree

Showing 9 changed files with 225 additions and 47 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -11,9 +11,11 @@ v.1.2
 This release introduces several bug fixes and minor improvements.
 
 - 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)
-    - [spatialPartitioning] Clean KdTree API (#122)
 
 - Examples
     - Fix example cuda/ponca_ssgls (#109)
@@ -22,9 +24,11 @@ This release introduces several bug fixes and minor improvements.
     - [fitting] Use variadic template for basket extensions (#85)
     - [fitting] Fix current status issue (#108)
     - [fitting] Fix sphere fit eigen solver (#112)
+    - [fitting] Fix a bug in AlgebraicSphere::primtiveGradient (#127)
     - [spatialPartitioning] Optimize memory use in knngraph queries (#104)
     - [spatialPartitioning] Fix potential compilation issues in KnnGraph (#111) 
     - [spatialPartitioning] Fix debug macros in KnnGraphRangeQuery (#121)
+    - [spatialPartitioning] Improve kdtree to_string to output data in YAML format (#125)
 
 -Docs
     - [fitting] Clarify documentation on FIT_RESULT (#108)

diff --git a/Ponca/src/Fitting/algebraicSphere.h b/Ponca/src/Fitting/algebraicSphere.h
@@ -245,11 +245,13 @@ class AlgebraicSphere : public T
      */
     PONCA_MULTIARCH inline VectorType projectDescent (const VectorType& _q, int nbIter = 16) const;
 
-    //! \brief Approximation of the scalar field gradient at \f$ \mathbf{q} (not normalized) \f$
+    /*! \brief Approximation of the scalar field gradient at \f$ \mathbf{q}\f$
+        \warning The gradient is not normalized by default */
     PONCA_MULTIARCH inline VectorType primitiveGradient (const VectorType& _q) const;
 
-    /*! \brief Approximation of the scalar field gradient at the evaluation point */
-    PONCA_MULTIARCH inline VectorType primitiveGradient () const { return m_ul.normalized(); }
+    /*! \brief Approximation of the scalar field gradient at the evaluation point
+        \warning The gradient is not normalized by default */
+    PONCA_MULTIARCH inline const VectorType& primitiveGradient () const { return m_ul; }
 
     /*!
         \brief Used to know if the fitting result to a plane

diff --git a/Ponca/src/SpatialPartitioning/KdTree/kdTree.h b/Ponca/src/SpatialPartitioning/KdTree/kdTree.h
@@ -186,7 +186,7 @@ class KdTreeBase
                                   Converter c);
 
     inline bool valid() const;
-    inline std::string to_string() const;
+    inline std::string to_string(bool verbose = false) const;
 
     // Accessors ---------------------------------------------------------------
 public:

diff --git a/Ponca/src/SpatialPartitioning/KdTree/kdTree.hpp b/Ponca/src/SpatialPartitioning/KdTree/kdTree.hpp
@@ -95,30 +95,51 @@ bool KdTreeBase<Traits>::valid() const
 }
 
 template<typename Traits>
-std::string KdTreeBase<Traits>::to_string() const
+std::string KdTreeBase<Traits>::to_string(bool verbose) const
 {
-    if (m_indices.empty()) return "";
-
     std::stringstream str;
-    str << "indices (" << index_count() << ") :\n";
-    for(IndexType i=0; i<index_count(); ++i)
+
+    str << "KdTree:";
+    str << "\n  MaxNodes: " << MAX_NODE_COUNT;
+    str << "\n  MaxPoints: " << MAX_POINT_COUNT;
+    str << "\n  MaxDepth: " << Traits::MAX_DEPTH;
+    str << "\n  PointCount: " << point_count();
+    str << "\n  SampleCount: " << index_count();
+    str << "\n  NodeCount: " << node_count();
+
+    if (!verbose)
     {
-        str << "  " << i << ": " << m_indices.operator[](i) << "\n";
+        return str.str();
     }
-    str << "nodes (" << node_count() << ") :\n";
-    for(NodeIndexType n=0; n< node_count(); ++n)
+
+    str << "\n  Samples: [";
+    static constexpr IndexType SAMPLES_PER_LINE = 10;
+    for (IndexType i = 0; i < index_count(); ++i)
     {
-        const NodeType& node = m_nodes.operator[](n);
-        if(node.is_leaf())
+        str << (i == 0 ? "" : ",");
+        str << (i % SAMPLES_PER_LINE == 0 ? "\n    " : " ");
+        str << m_indices[i];
+    }
+
+    str << "]\n  Nodes:";
+    for (NodeIndexType n = 0; n < node_count(); ++n)
+    {
+        const NodeType& node = m_nodes[n];
+        if (node.is_leaf())
         {
-            auto end = node.leaf_start() + node.leaf_size();
-            str << "  leaf: start=" << node.leaf_start() << " end=" << end << " (size=" << node.leaf_size() << ")\n";
+            str << "\n    - Type: Leaf";
+            str << "\n      Start: " << node.leaf_start();
+            str << "\n      Size: " << node.leaf_size();
         }
         else
         {
-            str << "  node: dim=" << node.inner_dim() << " split=" << node.inner_split_value() << " child=" << node.inner_first_child_id() << "\n";
+            str << "\n    - Type: Inner";
+            str << "\n      SplitDim: " << node.inner_split_dim();
+            str << "\n      SplitValue: " << node.inner_split_value();
+            str << "\n      FirstChild: " << node.inner_first_child_id();
         }
     }
+
     return str.str();
 }
 

diff --git a/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h b/Ponca/src/SpatialPartitioning/KdTree/kdTreeTraits.h
@@ -69,29 +69,49 @@ 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;
 };
 
 template <typename Index, typename Size>
 struct KdTreeDefaultLeafNode
 {
-    Index start;
-    Size size;
+    Index start{0};
+    Size size{0};
 };
 
 /*!
  * \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 <typename Index, typename NodeIndex, typename DataPoint,
-          typename LeafSize = Index>
-class KdTreeDefaultNode
+          typename LeafSize = Index,
+          typename _InnerNodeType = KdTreeDefaultInnerNode<NodeIndex, typename DataPoint::Scalar, DataPoint::Dim>,
+          typename _LeafNodeType = KdTreeDefaultLeafNode<Index, LeafSize>>
+class KdTreeCustomizableNode
 {
 private:
     using Scalar    = typename DataPoint::Scalar;
-    using LeafType  = KdTreeDefaultLeafNode<Index, LeafSize>;
-    using InnerType = KdTreeDefaultInnerNode<NodeIndex, Scalar, DataPoint::Dim>;
+    using InnerType = _InnerNodeType;
+    using LeafType  = _LeafNodeType;
 
 public:
     enum
@@ -110,10 +130,8 @@ class KdTreeDefaultNode
      * `DataPoint::VectorType`.
      */
     using AabbType = Eigen::AlignedBox<Scalar, DataPoint::Dim>;
-
-    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 +150,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,32 +168,32 @@ 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;
         }
     }
 
     /*!
      * \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 +202,52 @@ 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; }
+    [[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;
-    union
+    bool m_is_leaf{true};
+    union Data
     {
-        KdTreeDefaultLeafNode<Index, LeafSize> m_leaf;
-        KdTreeDefaultInnerNode<NodeIndex, Scalar, DataPoint::Dim> m_inner;
+        // We need an explicit constructor here, see https://stackoverflow.com/a/70428826
+        constexpr Data() : m_leaf() {}
+        // 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;
     };
+    Data data;
+};
+
+template <typename Index, typename NodeIndex, typename DataPoint,
+        typename LeafSize = Index>
+struct KdTreeDefaultNode : public KdTreeCustomizableNode<Index, NodeIndex, DataPoint, LeafSize,
+        KdTreeDefaultInnerNode<NodeIndex, typename DataPoint::Scalar, DataPoint::Dim>,
+        KdTreeDefaultLeafNode<Index, LeafSize>> {
+    using Base =  KdTreeCustomizableNode<Index, NodeIndex, DataPoint, LeafSize,
+            KdTreeDefaultInnerNode<NodeIndex, typename DataPoint::Scalar, DataPoint::Dim>,
+            KdTreeDefaultLeafNode<Index, LeafSize>>;
 };
 
 /*!
  * \brief The default traits type used by the kd-tree.
+ *
+ * \see KdTreeCustomizableNode Helper class to modify Inner/Leaf nodes without redefining a Trait class
+ *
+ * \tparam _NodeType Type used to store nodes, set by default to #KdTreeDefaultNode
  */
-template <typename _DataPoint>
+template <typename _DataPoint,
+        template <typename /*Index*/,
+                  typename /*NodeIndex*/,
+                  typename /*DataPoint*/,
+                  typename /*LeafSize*/> typename _NodeType = KdTreeDefaultNode>
 struct KdTreeDefaultTraits
 {
     enum
@@ -228,7 +277,7 @@ struct KdTreeDefaultTraits
 
     // Nodes
     using NodeIndexType = std::size_t;
-    using NodeType      = KdTreeDefaultNode<IndexType, NodeIndexType, DataPoint, LeafSizeType>;
+    using NodeType      = _NodeType<IndexType, NodeIndexType, DataPoint, LeafSizeType>;
     using NodeContainer = std::vector<NodeType>;
 };
 } // namespace Ponca
diff --git a/doc/src/ponca_module_spatialpartitioning.mdoc b/doc/src/ponca_module_spatialpartitioning.mdoc
@@ -95,7 +95,8 @@ fit.computeWithIds( myDataStructure.range_neighbors(fitInitPos, scale), vectorPo
 
   \subsection spatialpartitioning_kdtree_extending Extending KdTree
   Ponca::KdTreeBase is a customizable version of Ponca::KdTree, which can be controlled using `Traits`.
-  See KdTreeDefaultTraits for customization API.
+  See KdTreeDefaultTraits and KdTreeCustomizableNode for customization APIs. See also:
+   - `examples/cpp/ponca_customize_kdtree.cpp`
 
   \subsection spatialpartitioning_kdtree_implementation Implementation details
 

diff --git a/examples/cpp/CMakeLists.txt b/examples/cpp/CMakeLists.txt
@@ -35,12 +35,20 @@ add_dependencies(ponca-examples ponca_fit_line)
 ponca_handle_eigen_dependency(ponca_fit_line)
 
 set(ponca_neighbor_search_SRCS
-    ponca_neighbor_search.cpp
+        ponca_neighbor_search.cpp
 )
 add_executable(ponca_neighbor_search ${ponca_neighbor_search_SRCS})
 target_include_directories(ponca_neighbor_search PRIVATE ${PONCA_src_ROOT})
 add_dependencies(ponca-examples ponca_neighbor_search)
 ponca_handle_eigen_dependency(ponca_neighbor_search)
 
+set(ponca_customize_kdtree_SRCS
+        ponca_customize_kdtree.cpp
+)
+add_executable(ponca_customize_kdtree ${ponca_customize_kdtree_SRCS})
+target_include_directories(ponca_customize_kdtree PRIVATE ${PONCA_src_ROOT})
+add_dependencies(ponca-examples ponca_customize_kdtree)
+ponca_handle_eigen_dependency(ponca_customize_kdtree)
+
 add_subdirectory(pcl)
 add_subdirectory(nanoflann)