Merge pull request AcademySoftwareFoundation#1574 from Idclip/leafman…

…ager_root_leaf_fix LeafManager LeafArray bugfix from RootNode<LeafNode>
apradhana · Feb 28, 2023 · 7edd8cd · 7edd8cd
2 parents 0cd5ed7 + cd4498b
commit 7edd8cd
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 3 deletions.
diff --git a/openvdb/openvdb/tree/LeafManager.h b/openvdb/openvdb/tree/LeafManager.h
@@ -586,7 +586,12 @@ class LeafManager
         using LeafParentT = typename CopyConstness<TreeType, NonConstLeafParentT>::Type;
 
         std::deque<LeafParentT*> leafParents;
-        mTree->getNodes(leafParents);
+        if constexpr(std::is_same<NonConstLeafParentT, RootNodeType>::value) {
+            leafParents.emplace_back(&mTree->root());
+        }
+        else {
+            mTree->getNodes(leafParents);
+        }
 
         // Compute the leaf counts for each node
 

diff --git a/openvdb/openvdb/tree/Tree.h b/openvdb/openvdb/tree/Tree.h
@@ -576,8 +576,8 @@ class Tree: public TreeBase
     /// array.reserve(tree.leafCount());//this is a fast preallocation.
     /// tree.getNodes(array);
     /// @endcode
-    template<typename ArrayT> void getNodes(ArrayT& array) { mRoot.getNodes(array); }
-    template<typename ArrayT> void getNodes(ArrayT& array) const { mRoot.getNodes(array); }
+    template<typename ArrayT> void getNodes(ArrayT& array);
+    template<typename ArrayT> void getNodes(ArrayT& array) const;
     //@}
 
     /// @brief Steals all nodes of a certain type from the tree and
@@ -1273,6 +1273,30 @@ Tree<RootNodeType>::writeBuffers(std::ostream &os, bool saveFloatAsHalf) const
 }
 
 
+template<typename RootNodeType>
+template<typename ArrayT>
+inline void
+Tree<RootNodeType>::getNodes(ArrayT& array)
+{
+    using NodeT = typename std::remove_pointer<typename ArrayT::value_type>::type;
+    static_assert(!std::is_same<NodeT, RootNodeType>::value,
+        "getNodes() does not work for the RootNode. Use Tree::root()");
+    mRoot.getNodes(array);
+}
+
+
+template<typename RootNodeType>
+template<typename ArrayT>
+inline void
+Tree<RootNodeType>::getNodes(ArrayT& array) const
+{
+    using NodeT = typename std::remove_pointer<typename ArrayT::value_type>::type;
+    static_assert(!std::is_same<NodeT, const RootNodeType>::value,
+        "getNodes() does not work for the RootNode. Use Tree::root()");
+    mRoot.getNodes(array);
+}
+
+
 template<typename RootNodeType>
 inline void
 Tree<RootNodeType>::clear()

diff --git a/openvdb/openvdb/unittest/TestLeafManager.cc b/openvdb/openvdb/unittest/TestLeafManager.cc
@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: MPL-2.0
 
 #include <openvdb/Types.h>
+#include <openvdb/TypeList.h>
 #include <openvdb/tree/LeafManager.h>
 #include <openvdb/util/CpuTimer.h>
 #include "util.h" // for unittest_util::makeSphere()
@@ -304,3 +305,39 @@ TEST_F(TestLeafManager, testReduce)
     }
     EXPECT_EQ(FloatTree::LeafNodeType::numValues(), n);
 }
+
+TEST_F(TestLeafManager, testTreeConfigurations)
+{
+    using Tree2Type = openvdb::tree::Tree<
+        openvdb::tree::RootNode<
+        openvdb::tree::LeafNode<float, 3> > >;
+    using Tree3Type = openvdb::tree::Tree3<float, 4, 3>::Type;
+    using Tree4Type = openvdb::tree::Tree4<float, 5, 4, 3>::Type;
+    using Tree5Type = openvdb::tree::Tree5<float, 5, 5, 4, 3>::Type;
+
+    using TestConfigurations = openvdb::TypeList<
+        Tree2Type, Tree3Type, Tree4Type, Tree5Type
+    >;
+
+    TestConfigurations::foreach([](auto tree) {
+        using TreeType = typename std::decay<decltype(tree)>::type;
+        using LeafNodeType = typename TreeType::LeafNodeType;
+        using LeafManagerT = openvdb::tree::LeafManager<TreeType>;
+        using ConstLeafManagerT = openvdb::tree::LeafManager<const TreeType>;
+
+        // Add 20 leaf nodes and make sure they are constructed correctly
+        constexpr openvdb::Int32 Count = 20;
+
+        const openvdb::Int32 start = -(Count/2)*openvdb::Int32(LeafNodeType::DIM);
+        const openvdb::Int32 end   =  (Count/2)*openvdb::Int32(LeafNodeType::DIM);
+        for (openvdb::Int32 idx = start; idx < end; idx+=openvdb::Int32(LeafNodeType::DIM)) {
+            tree.touchLeaf(openvdb::math::Coord(idx));
+        }
+
+        EXPECT_EQ(tree.leafCount(), Count);
+        LeafManagerT manager(tree);
+        EXPECT_EQ(manager.leafCount(), Count);
+        ConstLeafManagerT cmanager(tree);
+        EXPECT_EQ(cmanager.leafCount(), Count);
+    });
+}
diff --git a/pendingchanges/leafmanager_root_fix.txt b/pendingchanges/leafmanager_root_fix.txt
@@ -0,0 +1,7 @@
+OpenVDB:
+ - Bug Fixes:
+    - Fixed a bug with LeafManager which wouldn't correctly
+    initialize its LeafNode array for single level Tree configurations
+    i.e. RootNode<LeafNode> (bug introduced in 7.2.0)
+    [Reported by @lanwatch]
+