Skip to content

Commit 5a0519d

Browse files
IdclipIdclip
authored
Merge pull request #1575 from Idclip/leaf_node_bool_topology_fix
Fixed a bug with LeafNodeBool TopologyCopy constructor
2 parents 2d7ba1c + 7d88a61 commit 5a0519d

File tree

3 files changed

+153
-20
lines changed

3 files changed

+153
-20
lines changed

‎openvdb/openvdb/tree/LeafNodeBool.h‎

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,35 @@ class LeafNode<bool, Log2Dim>
7979
template<typename OtherValueType>
8080
explicit LeafNode(const LeafNode<OtherValueType, Log2Dim>& other);
8181

82-
/// Topology copy constructor
82+
/// @brief Deprecated topology copy constructor
83+
/// @note This constructor initialises the bool buffer to the ValueMask
84+
/// states (i.e. value will be true if the active state is on and
85+
/// vice-versa). This is not really a "TopologyCopy" and is therefor
86+
/// deprecated. Use the explicit mask/buffer constructor instead:
87+
/// @code
88+
/// // build new leaf node with the mask of 'a', but with the mask of
89+
/// // 'b' as the the new value buffer.
90+
/// const LeafNode a = ... ;
91+
/// const LeafNode b = ... ;
92+
/// const LeafNode copy(a.origin(), /*mask=*/a.getValueMask(),
93+
/// /*buff=*/b.getValueMask());
94+
/// @endcode
8395
template<typename ValueType>
96+
OPENVDB_DEPRECATED_MESSAGE("Use LeafNodeBool component constructor.")
8497
LeafNode(const LeafNode<ValueType, Log2Dim>& other, TopologyCopy);
8598

99+
/// @brief Construct a LeafNodeBool with its individual components
100+
/// @param xyz Leaf origin
101+
/// @param mask The ValueMask to copy
102+
/// @param buff The LeafBuffer to copy (also constructable from a ValueMask)
103+
/// @param trans Transient data (ignored until ABI 9)
104+
LeafNode(const Coord& xyz,
105+
const NodeMaskType& mask,
106+
const Buffer& buff,
107+
const Index32 trans = 0);
108+
86109
//@{
87-
/// @brief Topology copy constructor
88-
/// @note This variant exists mainly to enable template instantiation.
110+
/// @brief Topology copy constructors
89111
template<typename ValueType>
90112
LeafNode(const LeafNode<ValueType, Log2Dim>& other, bool offValue, bool onValue, TopologyCopy);
91113
template<typename ValueType>
@@ -831,20 +853,41 @@ LeafNode<bool, Log2Dim>::LeafNode(const LeafNode<ValueT, Log2Dim>& other, Topolo
831853
{
832854
}
833855

856+
template<Index Log2Dim>
857+
inline
858+
LeafNode<bool, Log2Dim>::LeafNode(const Coord& xyz,
859+
const NodeMaskType& mask,
860+
const Buffer& buff,
861+
#if OPENVDB_ABI_VERSION_NUMBER < 9
862+
[[maybe_unused]]
863+
#endif
864+
const Index32 trans)
865+
: mValueMask(mask)
866+
, mBuffer(buff)
867+
, mOrigin(xyz & (~(DIM - 1)))
868+
#if OPENVDB_ABI_VERSION_NUMBER >= 9
869+
, mTransientData(trans)
870+
#endif
871+
{
872+
}
834873

835874
template<Index Log2Dim>
836875
template<typename ValueT>
837876
inline
838877
LeafNode<bool, Log2Dim>::LeafNode(const LeafNode<ValueT, Log2Dim>& other,
839878
bool offValue, bool onValue, TopologyCopy)
840879
: mValueMask(other.valueMask())
841-
, mBuffer(other.valueMask())
880+
, mBuffer(offValue)
842881
, mOrigin(other.origin())
843882
#if OPENVDB_ABI_VERSION_NUMBER >= 9
844883
, mTransientData(other.mTransientData)
845884
#endif
846885
{
847-
if (offValue) { if (!onValue) mBuffer.mData.toggle(); else mBuffer.mData.setOn(); }
886+
for (Index i = 0; i < SIZE; ++i) {
887+
if (mValueMask.isOn(i)) {
888+
mBuffer.setValue(i, onValue);
889+
}
890+
}
848891
}
849892

850893

‎openvdb/openvdb/unittest/TestLeafBool.cc‎

Lines changed: 100 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -307,30 +307,115 @@ TEST_F(TestLeafBool, testIO)
307307
}
308308

309309

310-
TEST_F(TestLeafBool, testTopologyCopy)
310+
TEST_F(TestLeafBool, testConstructors)
311311
{
312312
using openvdb::Coord;
313313

314-
// LeafNode<float, Log2Dim> having the same Log2Dim as LeafType
315-
typedef LeafType::ValueConverter<float>::Type FloatLeafType;
314+
{ // Test constructor which takes a value mask and buffer accepts 2 value masks
315+
LeafType a, b;
316+
a.setValueOn(1, true);
317+
a.setValueOn(2, false);
318+
a.setValueOn(3, true);
319+
a.setValueOn(4, false);
320+
321+
b.setValueOn(1, false);
322+
b.setValueOff(2, true);
323+
b.setValueOn(3, false);
324+
b.setValueOn(4, true);
325+
b.setValueOff(5, true);
326+
b.setValueOff(6, false);
327+
const LeafType c(Coord(12,4,25), /*mask=*/a.getValueMask(), /*buff=*/b.getValueMask());
328+
EXPECT_EQ(c.origin(), Coord(8,0,24));
329+
EXPECT_EQ(c.getValueMask(), a.getValueMask());
330+
EXPECT_EQ(c.buffer(), b.getValueMask());
331+
332+
EXPECT_TRUE(c.isValueOn(1));
333+
EXPECT_TRUE(c.isValueOn(2));
334+
EXPECT_TRUE(c.isValueOn(3));
335+
EXPECT_TRUE(c.isValueOn(4));
336+
EXPECT_TRUE(!c.isValueOn(5));
337+
EXPECT_TRUE(!c.isValueOn(6));
338+
339+
EXPECT_EQ(c.getValue(1), true);
340+
EXPECT_EQ(c.getValue(2), false);
341+
EXPECT_EQ(c.getValue(3), true);
342+
EXPECT_EQ(c.getValue(4), true);
343+
EXPECT_EQ(c.getValue(5), false);
344+
EXPECT_EQ(c.getValue(6), false);
345+
}
346+
347+
{ // Test copy constructor with new buffer values
348+
LeafType a;
349+
a.setValueOn(1, true);
350+
a.setValueOn(2, false);
351+
a.setValueOn(3, true);
352+
a.setValueOn(4, false);
353+
a.setValueOff(5, true);
354+
a.setValueOff(6, false);
355+
356+
const LeafType b(a, /*off=*/true, /*on=*/false, openvdb::TopologyCopy());
357+
EXPECT_EQ(b.getValueMask(), a.getValueMask());
358+
EXPECT_EQ(b.getValue(1), false);
359+
EXPECT_EQ(b.getValue(2), false);
360+
EXPECT_EQ(b.getValue(3), false);
361+
EXPECT_EQ(b.getValue(4), false);
362+
EXPECT_TRUE(b.isValueOn(1));
363+
EXPECT_TRUE(b.isValueOn(2));
364+
EXPECT_TRUE(b.isValueOn(3));
365+
EXPECT_TRUE(b.isValueOn(4));
366+
for (openvdb::Index i = 5; i < LeafType::SIZE; ++i) {
367+
EXPECT_EQ(b.getValue(i), true);
368+
EXPECT_TRUE(!b.isValueOn(i));
369+
}
370+
}
316371

317-
FloatLeafType fleaf(Coord(10, 20, 30), /*background=*/-1.0);
318-
std::set<Coord> coords;
319-
for (openvdb::Index n = 0; n < fleaf.numValues(); n += 10) {
320-
Coord xyz = fleaf.offsetToGlobalCoord(n);
321-
fleaf.setValueOn(xyz, float(n));
322-
coords.insert(xyz);
372+
{ // Test copy constructor with background value
373+
LeafType a;
374+
a.setValueOn(1, true);
375+
a.setValueOn(2, false);
376+
a.setValueOn(3, true);
377+
a.setValueOn(4, false);
378+
a.setValueOff(5, true);
379+
a.setValueOff(6, false);
380+
381+
const LeafType b(a, /*background*/true, openvdb::TopologyCopy());
382+
EXPECT_EQ(b.getValueMask(), a.getValueMask());
383+
EXPECT_TRUE(b.isValueOn(1));
384+
EXPECT_TRUE(b.isValueOn(2));
385+
EXPECT_TRUE(b.isValueOn(3));
386+
EXPECT_TRUE(b.isValueOn(4));
387+
// All values are background
388+
for (openvdb::Index i = 0; i < LeafType::SIZE; ++i) {
389+
EXPECT_EQ(b.getValue(i), true);
390+
}
323391
}
324392

325-
LeafType leaf(fleaf, openvdb::TopologyCopy());
326-
EXPECT_EQ(fleaf.onVoxelCount(), leaf.onVoxelCount());
393+
{
394+
// LeafNode<float, Log2Dim> having the same Log2Dim as LeafType
395+
typedef LeafType::ValueConverter<float>::Type FloatLeafType;
396+
397+
FloatLeafType fleaf(Coord(10, 20, 30), /*background=*/-1.0);
398+
std::set<Coord> coords;
399+
for (openvdb::Index n = 0; n < fleaf.numValues(); n += 10) {
400+
Coord xyz = fleaf.offsetToGlobalCoord(n);
401+
fleaf.setValueOn(xyz, float(n));
402+
coords.insert(xyz);
403+
}
404+
405+
OPENVDB_NO_DEPRECATION_WARNING_BEGIN
406+
LeafType leaf(fleaf, openvdb::TopologyCopy());
407+
OPENVDB_NO_DEPRECATION_WARNING_END
327408

328-
EXPECT_TRUE(leaf.hasSameTopology(&fleaf));
409+
EXPECT_EQ(fleaf.onVoxelCount(), leaf.onVoxelCount());
410+
EXPECT_TRUE(leaf.hasSameTopology(&fleaf));
329411

330-
for (LeafType::ValueOnIter iter = leaf.beginValueOn(); iter; ++iter) {
331-
coords.erase(iter.getCoord());
412+
for (LeafType::ValueOnIter iter = leaf.beginValueOn(); iter; ++iter) {
413+
coords.erase(iter.getCoord());
414+
}
415+
EXPECT_TRUE(coords.empty());
332416
}
333-
EXPECT_TRUE(coords.empty());
417+
418+
334419
}
335420

336421

‎pendingchanges/leafnodebool.txt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
OpenVDB:
2+
- Bug Fixes:
3+
- Fixed a bug with LeafNodeBool Topology constructor with designated
4+
on/off values which wouldn't apply them correctly.
5+
[Reported by @hozhaoea]

0 commit comments

Comments
 (0)