From 50d3e42a35b2b65936d52aa0be9d3e2f8ac79f74 Mon Sep 17 00:00:00 2001 From: Dan Bailey Date: Tue, 18 Aug 2026 15:47:03 -0700 Subject: [PATCH 1/9] Add in-memory conversion for VDB grids with no file offsets Signed-off-by: Dan Bailey --- openvdb/openvdb/io/File.cc | 173 +++++++++++- openvdb/openvdb/unittest/TestCodec.cc | 372 ++++++++++++++++++++++++++ 2 files changed, 541 insertions(+), 4 deletions(-) diff --git a/openvdb/openvdb/io/File.cc b/openvdb/openvdb/io/File.cc index b783008986..6c0952ff0a 100644 --- a/openvdb/openvdb/io/File.cc +++ b/openvdb/openvdb/io/File.cc @@ -6,6 +6,7 @@ #include "File.h" #include +#include // for GridTypes #include #include #include @@ -18,6 +19,7 @@ #include #include #include +#include namespace openvdb { @@ -25,6 +27,18 @@ OPENVDB_USE_VERSION_NAMESPACE namespace OPENVDB_VERSION_NAME { namespace io { +namespace { + +/// @brief Convert @a source to the grid type that @a readOptions would have +/// produced had it been read through @a codec (looked up by the caller via +/// the protected @c Archive::findCodec(), since this is a free function). +/// Returns null if no conversion is needed or possible, in which case the +/// caller keeps the original grid and @a diagnostics is set accordingly. +GridBase::Ptr convertGridForReadMode(const GridBase& source, const ReadOptions& readOptions, + Codec* codec, ReadDiagnostics& diagnostics); + +} // anonymous namespace + File::File(const std::string& filename) : Archive() @@ -308,7 +322,40 @@ File::getGrids(const io::ReadOptions& readOptions) const if (!inputHasGridOffsets()) { // If the input file doesn't have grid offsets, then all of the grids // have already been streamed in and stored in mGrids. - ret = mGrids; + const auto& bbox = readOptions.clipBBox; + const bool clip = bbox.isSorted(); + + if (readOptions.readMode == io::ReadMode::Original && !clip) { + // Nothing to convert or clip: preserve pointer identity with mGrids. + ret = mGrids; + } else { + ret.reset(new GridPtrVec); + for (const auto& cachedGrid : *mGrids) { + io::Codec* codec = Archive::findCodec(cachedGrid->type(), readOptions); + GridBase::Ptr grid = + convertGridForReadMode(*cachedGrid, readOptions, codec, mReadDiagnostics); + if (!grid) { + grid = cachedGrid; + if (readOptions.readMode == io::ReadMode::Half || + readOptions.readMode == io::ReadMode::Bool || + readOptions.readMode == io::ReadMode::Mask) + { + OPENVDB_LOG_WARN(mFilename << ": grid \"" << cachedGrid->getName() + << "\" requested a read mode conversion, but no conversion is " + "available for grid type \"" << cachedGrid->type() + << "\"; returning the original type"); + } + } + if (clip) { + if (grid == cachedGrid) { + // Never mutate the cached grid in place; it stays owned by mGrids. + grid = grid->deepCopyGrid(); + } + grid->clipGrid(bbox); + } + ret->push_back(grid); + } + } } else { ret.reset(new GridPtrVec); @@ -455,12 +502,42 @@ File::readGrid(const Name& name, const io::ReadOptions& readOptions) // If a grid with the given name was already read and cached // (along with the entire contents of the file, because the file // doesn't support random access), retrieve and return it. - GridBase::Ptr grid = retrieveCachedGrid(name); - if (grid) { + GridBase::Ptr cachedGrid = retrieveCachedGrid(name); + GridBase::Ptr grid; + if (cachedGrid) { + grid = cachedGrid; + + if (readOptions.readMode == io::ReadMode::TopologyOnly) { + mReadDiagnostics.addWarning(grid->getName(), + "ReadMode::TopologyOnly is not supported for cached grids; " + "reading as original type"); + OPENVDB_LOG_WARN(mFilename << ": grid \"" << grid->getName() + << "\" requested ReadMode::TopologyOnly, but this file has no grid offsets " + "and the grid is already fully cached; returning the original type"); + } else { + io::Codec* codec = Archive::findCodec(grid->type(), readOptions); + GridBase::Ptr converted = + convertGridForReadMode(*grid, readOptions, codec, mReadDiagnostics); + if (converted) { + grid = converted; + } else if (readOptions.readMode == io::ReadMode::Half || + readOptions.readMode == io::ReadMode::Bool || + readOptions.readMode == io::ReadMode::Mask) + { + OPENVDB_LOG_WARN(mFilename << ": grid \"" << grid->getName() + << "\" requested a read mode conversion, but no conversion is " + "available for grid type \"" << grid->type() + << "\"; returning the original type"); + } + } + const auto& bbox = readOptions.clipBBox; const bool clip = bbox.isSorted(); if (clip) { - grid = grid->deepCopyGrid(); + if (grid == cachedGrid) { + // Never mutate the cached grid in place; it stays owned by mNamedGrids. + grid = grid->deepCopyGrid(); + } grid->clipGrid(bbox); } return grid; @@ -608,6 +685,94 @@ File::endName() const } +//////////////////////////////////////// + + +namespace { + +namespace convert_grid_internal { + +/// @brief Convert @a source to the grid type @c ValueConverter::Type, +/// provided the registry-reported @a targetType agrees and the value +/// conversion is legal. Returns null otherwise. +template +inline GridBase::Ptr +convertToBuildType(const GridBase& source, const std::string& targetType) +{ + GridBase::Ptr result; + source.apply([&](const auto& typedSource) { + using SourceGridT = std::decay_t; + using TargetGridT = + typename SourceGridT::template ValueConverter::Type; + // No LeafNode conversion constructor exists from ValueMask to any + // other build type, so exclude it here; nested if constexpr avoids + // instantiating the ambiguous CanConvertType. + if constexpr (!std::is_same_v) { + if constexpr (CanConvertType::value) { + if (TargetGridT::gridType() == targetType) { + result = typename TargetGridT::Ptr(new TargetGridT(typedSource)); + } + } + } + }); + return result; +} + +} // namespace convert_grid_internal + +GridBase::Ptr +convertGridForReadMode(const GridBase& source, const ReadOptions& readOptions, + Codec* codec, ReadDiagnostics& diagnostics) +{ + if (readOptions.readMode != ReadMode::Half && + readOptions.readMode != ReadMode::Bool && + readOptions.readMode != ReadMode::Mask) + { + return GridBase::Ptr(); + } + + const std::string modeStr = + readOptions.readMode == ReadMode::Half ? "Half" : + readOptions.readMode == ReadMode::Bool ? "Bool" : "Mask"; + + CodecData::Ptr codecData = codec ? codec->createData() : CodecData::Ptr(); + const std::string targetType = + (codecData && codecData->grid) ? codecData->grid->type() : std::string(); + + // No conversion codec registered for this grid type (falls through to the + // plain gridType codec), or the registry agrees the type is unchanged. + if (targetType.empty() || targetType == source.type()) { + diagnostics.addWarning(source.getName(), + "ReadMode::" + modeStr + " conversion is not supported for grid type '" + + source.type() + "'; reading as original type"); + return GridBase::Ptr(); + } + + GridBase::Ptr result; + if (readOptions.readMode == ReadMode::Half) { + result = convert_grid_internal::convertToBuildType(source, targetType); + } else if (readOptions.readMode == ReadMode::Bool) { + result = convert_grid_internal::convertToBuildType(source, targetType); + } else { + result = convert_grid_internal::convertToBuildType(source, targetType); + } + + // The registry named a target type that this dispatch could not produce + // (e.g. a custom-registered grid type outside GridTypes, or the + // CanConvertType guard rejected the pair): fall through with a warning + // rather than silently diverging from what the registry reported. + if (!result) { + diagnostics.addWarning(source.getName(), + "ReadMode::" + modeStr + " conversion is not supported for grid type '" + + source.type() + "'; reading as original type"); + } + + return result; +} + +} // anonymous namespace + + } // namespace io } // namespace OPENVDB_VERSION_NAME } // namespace openvdb diff --git a/openvdb/openvdb/unittest/TestCodec.cc b/openvdb/openvdb/unittest/TestCodec.cc index cfa14de91e..d1727543be 100644 --- a/openvdb/openvdb/unittest/TestCodec.cc +++ b/openvdb/openvdb/unittest/TestCodec.cc @@ -3,11 +3,14 @@ #include #include +#include #include #include #include #include #include +#include // for remove() +#include class TestCodec: public ::testing::Test { @@ -348,6 +351,375 @@ TEST_F(TestCodec, testFloatToHalfCodecConversion) std::remove(floatPath.c_str()); } +TEST_F(TestCodec, testFloatToHalfCodecConversionNoGridOffsets) +{ + using namespace openvdb; + using namespace openvdb::io; + + CodecRegistry::clear(); + io::internal::initialize(); + + FloatGrid::Ptr srcGrid = FloatGrid::create(3.25f); + srcGrid->setName("float_to_half"); + srcGrid->tree().setValue(Coord(0, 0, 0), 1.0f / 3.0f); + + const std::string path = "test_float_to_half_no_offsets.vdb"; + + { + std::ofstream os(path, std::ios_base::out | std::ios_base::binary); + io::Stream(os).write(GridPtrVec{srcGrid}); + } + + ReadOptions readOptions; + readOptions.readMode = ReadMode::Half; + + { + io::File f(path); + f.open(); + GridPtrVecPtr grids = f.getGrids(readOptions); + ASSERT_TRUE(grids); + ASSERT_EQ(size_t(1), grids->size()); + EXPECT_TRUE((*grids)[0]->isType()); + HalfGrid::Ptr halfGrid = gridPtrCast((*grids)[0]); + ASSERT_TRUE(halfGrid); + EXPECT_EQ(halfGrid->tree().getValue(Coord(0, 0, 0)), Half(1.0f / 3.0f)); + EXPECT_EQ(halfGrid->background(), Half(3.25f)); + f.close(); + } + + { + io::File f(path); + f.open(); + GridBase::Ptr grid = f.readGrid(srcGrid->getName(), readOptions); + ASSERT_TRUE(grid); + EXPECT_TRUE(grid->isType()); + HalfGrid::Ptr halfGrid = gridPtrCast(grid); + ASSERT_TRUE(halfGrid); + EXPECT_EQ(halfGrid->tree().getValue(Coord(0, 0, 0)), Half(1.0f / 3.0f)); + EXPECT_EQ(halfGrid->background(), Half(3.25f)); + f.close(); + } + + std::remove(path.c_str()); +} + +TEST_F(TestCodec, testBoolAndMaskConversionNoGridOffsets) +{ + using namespace openvdb; + using namespace openvdb::io; + + CodecRegistry::clear(); + io::internal::initialize(); + + auto runCase = [](ReadMode mode) { + FloatGrid::Ptr srcGrid = FloatGrid::create(0.0f); + srcGrid->setName("float_grid"); + srcGrid->fill(CoordBBox(Coord(-5), Coord(5)), 1.5f, true); + + const std::string path = + "test_bool_mask_no_offsets_" + std::to_string(int(mode)) + ".vdb"; + { + std::ofstream os(path, std::ios_base::out | std::ios_base::binary); + io::Stream(os).write(GridPtrVec{srcGrid}); + } + + ReadOptions readOptions; + readOptions.readMode = mode; + + auto checkGrid = [&](const GridBase::Ptr& grid) { + ASSERT_TRUE(grid); + if (mode == ReadMode::Bool) { + EXPECT_TRUE(grid->isType()); + BoolGrid::Ptr dstGrid = gridPtrCast(grid); + ASSERT_TRUE(dstGrid); + EXPECT_TRUE(srcGrid->tree().hasSameTopology(dstGrid->tree())); + for (BoolGrid::ValueOnCIter it = dstGrid->cbeginValueOn(); it; ++it) { + EXPECT_EQ(*it, true); + } + } else { + EXPECT_TRUE(grid->isType()); + MaskGrid::Ptr dstGrid = gridPtrCast(grid); + ASSERT_TRUE(dstGrid); + EXPECT_TRUE(srcGrid->tree().hasSameTopology(dstGrid->tree())); + for (MaskGrid::ValueOnCIter it = dstGrid->cbeginValueOn(); it; ++it) { + EXPECT_EQ(*it, true); + } + } + }; + + { + io::File f(path); + f.open(); + GridPtrVecPtr grids = f.getGrids(readOptions); + ASSERT_TRUE(grids); + ASSERT_EQ(size_t(1), grids->size()); + checkGrid((*grids)[0]); + f.close(); + } + + { + io::File f(path); + f.open(); + checkGrid(f.readGrid(srcGrid->getName(), readOptions)); + f.close(); + } + + std::remove(path.c_str()); + }; + + runCase(ReadMode::Bool); + runCase(ReadMode::Mask); +} + +TEST_F(TestCodec, testVec3FallsBackWithWarningNoGridOffsets) +{ + using namespace openvdb; + using namespace openvdb::io; + + CodecRegistry::clear(); + io::internal::initialize(); + + Vec3SGrid::Ptr srcGrid = Vec3SGrid::create(Vec3s(0.0f)); + srcGrid->setName("vec3_grid"); + srcGrid->fill(CoordBBox(Coord(-5), Coord(5)), Vec3s(1.0f), true); + + const std::string path = "test_vec3_fallback_no_offsets.vdb"; + { + std::ofstream os(path, std::ios_base::out | std::ios_base::binary); + io::Stream(os).write(GridPtrVec{srcGrid}); + } + + ReadOptions readOptions; + readOptions.readMode = ReadMode::Bool; + + // Determine the warning the offsets path produces for the same request, + // so the two paths can be compared for parity. + std::string offsetsWarning; + { + const std::string offsetsPath = "test_vec3_fallback_offsets.vdb"; + { + io::File f(offsetsPath); + f.write(GridPtrVec{srcGrid}); + } + io::File f(offsetsPath); + f.open(); + f.enableReadDiagnostics(); + GridBase::Ptr grid = f.readGrid(srcGrid->getName(), readOptions); + ASSERT_TRUE(grid); + EXPECT_TRUE(grid->isType()); + ASSERT_EQ(size_t(1), f.readDiagnostics().diagnostics().size()); + offsetsWarning = f.readDiagnostics().diagnostics()[0].message; + f.close(); + std::remove(offsetsPath.c_str()); + } + + { + io::File f(path); + f.open(); + f.enableReadDiagnostics(); + GridBase::Ptr grid; + EXPECT_NO_THROW(grid = f.readGrid(srcGrid->getName(), readOptions)); + ASSERT_TRUE(grid); + EXPECT_TRUE(grid->isType()); + ASSERT_EQ(size_t(1), f.readDiagnostics().diagnostics().size()); + EXPECT_EQ(f.readDiagnostics().diagnostics()[0].message, offsetsWarning); + f.close(); + } + + std::remove(path.c_str()); +} + +TEST_F(TestCodec, testOffsetsAndNoOffsetsParity) +{ + using namespace openvdb; + using namespace openvdb::io; + + CodecRegistry::clear(); + io::internal::initialize(); + + FloatGrid::Ptr srcGrid = FloatGrid::create(2.0f); + srcGrid->setName("parity_grid"); + srcGrid->fill(CoordBBox(Coord(-5), Coord(5)), 0.25f, true); + + const std::string offsetsPath = "test_parity_offsets.vdb"; + const std::string noOffsetsPath = "test_parity_no_offsets.vdb"; + { + io::File f(offsetsPath); + f.write(GridPtrVec{srcGrid}); + } + { + std::ofstream os(noOffsetsPath, std::ios_base::out | std::ios_base::binary); + io::Stream(os).write(GridPtrVec{srcGrid}); + } + + ReadOptions readOptions; + readOptions.readMode = ReadMode::Half; + + HalfGrid::Ptr offsetsGrid; + { + io::File f(offsetsPath); + f.open(); + offsetsGrid = gridPtrCast(f.readGrid(srcGrid->getName(), readOptions)); + f.close(); + } + HalfGrid::Ptr noOffsetsGrid; + { + io::File f(noOffsetsPath); + f.open(); + noOffsetsGrid = gridPtrCast(f.readGrid(srcGrid->getName(), readOptions)); + f.close(); + } + + ASSERT_TRUE(offsetsGrid); + ASSERT_TRUE(noOffsetsGrid); + EXPECT_EQ(offsetsGrid->type(), noOffsetsGrid->type()); + EXPECT_TRUE(offsetsGrid->tree().hasSameTopology(noOffsetsGrid->tree())); + for (HalfGrid::ValueOnCIter it = offsetsGrid->cbeginValueOn(); it; ++it) { + EXPECT_EQ(*it, noOffsetsGrid->tree().getValue(it.getCoord())); + } + + std::remove(offsetsPath.c_str()); + std::remove(noOffsetsPath.c_str()); +} + +TEST_F(TestCodec, testClipBBoxInGetGridsNoGridOffsets) +{ + using namespace openvdb; + using namespace openvdb::io; + + CodecRegistry::clear(); + io::internal::initialize(); + + FloatGrid::Ptr srcGrid = FloatGrid::create(0.0f); + srcGrid->setName("clip_grid"); + srcGrid->fill(CoordBBox(Coord(-5), Coord(5)), 1.5f, true); + + const std::string path = "test_clip_getgrids_no_offsets.vdb"; + { + std::ofstream os(path, std::ios_base::out | std::ios_base::binary); + io::Stream(os).write(GridPtrVec{srcGrid}); + } + + const BBoxd clipBBox(Vec3d(0.0), Vec3d(3.5)); + auto srcClipped = tools::clip(*srcGrid, clipBBox); + + io::File f(path); + f.open(); + + ReadOptions clipOptions; + clipOptions.clipBBox = clipBBox; + + { + GridPtrVecPtr grids = f.getGrids(clipOptions); + ASSERT_TRUE(grids); + ASSERT_EQ(size_t(1), grids->size()); + FloatGrid::Ptr clipped = gridPtrCast((*grids)[0]); + ASSERT_TRUE(clipped); + EXPECT_TRUE(srcClipped->tree().hasSameTopology(clipped->tree())); + } + + // mGrids must be unmutated: a default-options call still sees the full topology. + { + GridPtrVecPtr grids = f.getGrids(); + ASSERT_TRUE(grids); + ASSERT_EQ(size_t(1), grids->size()); + FloatGrid::Ptr full = gridPtrCast((*grids)[0]); + ASSERT_TRUE(full); + EXPECT_TRUE(srcGrid->tree().hasSameTopology(full->tree())); + } + + f.close(); + std::remove(path.c_str()); +} + +TEST_F(TestCodec, testClipAndConversionTogetherNoGridOffsets) +{ + using namespace openvdb; + using namespace openvdb::io; + + CodecRegistry::clear(); + io::internal::initialize(); + + FloatGrid::Ptr srcGrid = FloatGrid::create(0.0f); + srcGrid->setName("clip_convert_grid"); + srcGrid->fill(CoordBBox(Coord(-5), Coord(5)), 1.5f, true); + + const std::string path = "test_clip_and_convert_no_offsets.vdb"; + { + std::ofstream os(path, std::ios_base::out | std::ios_base::binary); + io::Stream(os).write(GridPtrVec{srcGrid}); + } + + const BBoxd clipBBox(Vec3d(0.0), Vec3d(3.5)); + auto srcClipped = tools::clip(*srcGrid, clipBBox); + + ReadOptions readOptions; + readOptions.readMode = ReadMode::Half; + readOptions.clipBBox = clipBBox; + + { + io::File f(path); + f.open(); + GridPtrVecPtr grids = f.getGrids(readOptions); + ASSERT_TRUE(grids); + ASSERT_EQ(size_t(1), grids->size()); + EXPECT_TRUE((*grids)[0]->isType()); + HalfGrid::Ptr clipped = gridPtrCast((*grids)[0]); + ASSERT_TRUE(clipped); + EXPECT_TRUE(srcClipped->tree().hasSameTopology(clipped->tree())); + f.close(); + } + + { + io::File f(path); + f.open(); + GridBase::Ptr grid = f.readGrid(srcGrid->getName(), readOptions); + ASSERT_TRUE(grid); + EXPECT_TRUE(grid->isType()); + HalfGrid::Ptr clipped = gridPtrCast(grid); + ASSERT_TRUE(clipped); + EXPECT_TRUE(srcClipped->tree().hasSameTopology(clipped->tree())); + f.close(); + } + + std::remove(path.c_str()); +} + +TEST_F(TestCodec, testTopologyOnlyWarnsAndIgnoresNoGridOffsets) +{ + using namespace openvdb; + using namespace openvdb::io; + + CodecRegistry::clear(); + io::internal::initialize(); + + FloatGrid::Ptr srcGrid = FloatGrid::create(0.0f); + srcGrid->setName("topology_only_grid"); + srcGrid->fill(CoordBBox(Coord(-5), Coord(5)), 1.5f, true); + + const std::string path = "test_topology_only_no_offsets.vdb"; + { + std::ofstream os(path, std::ios_base::out | std::ios_base::binary); + io::Stream(os).write(GridPtrVec{srcGrid}); + } + + ReadOptions readOptions; + readOptions.readMode = ReadMode::TopologyOnly; + + io::File f(path); + f.open(); + f.enableReadDiagnostics(); + + GridBase::Ptr grid; + EXPECT_NO_THROW(grid = f.readGrid(srcGrid->getName(), readOptions)); + ASSERT_TRUE(grid); + EXPECT_TRUE(grid->isType()); + EXPECT_EQ(size_t(1), f.readDiagnostics().diagnostics().size()); + + f.close(); + std::remove(path.c_str()); +} + template void testConvertCodecImpl() { From e4dbaaf905085c41685c72dcb30da873fb0c1cc4 Mon Sep 17 00:00:00 2001 From: Dan Bailey Date: Mon, 24 Aug 2026 21:46:27 -0700 Subject: [PATCH 2/9] Fix clipping and in-memory conversion for instanced grids Signed-off-by: Dan Bailey --- openvdb/openvdb/io/File.cc | 161 ++++++++----- openvdb/openvdb/io/File.h | 6 + openvdb/openvdb/unittest/TestCodec.cc | 312 ++++++++++++++++++++++++++ 3 files changed, 422 insertions(+), 57 deletions(-) diff --git a/openvdb/openvdb/io/File.cc b/openvdb/openvdb/io/File.cc index 6c0952ff0a..2f6cea1f14 100644 --- a/openvdb/openvdb/io/File.cc +++ b/openvdb/openvdb/io/File.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -37,6 +38,9 @@ namespace { GridBase::Ptr convertGridForReadMode(const GridBase& source, const ReadOptions& readOptions, Codec* codec, ReadDiagnostics& diagnostics); +/// @brief Return the name used in diagnostics and log messages for @a mode. +std::string readModeName(ReadMode mode); + } // anonymous namespace @@ -330,28 +334,43 @@ File::getGrids(const io::ReadOptions& readOptions) const ret = mGrids; } else { ret.reset(new GridPtrVec); + + // Instances (grids sharing a source tree) share the converted tree + // too, unless instancing is disabled. Under a clip, share only + // when transforms agree, since a clip depends on each grid's own + // transform. + const bool shareConvertedTrees = isInstancingEnabled() && + readOptions.readMode != io::ReadMode::MetadataOnly; + struct Resolved { GridBase::Ptr grid; math::Transform::ConstPtr transform; }; + std::map resolvedBySourceTree; + for (const auto& cachedGrid : *mGrids) { - io::Codec* codec = Archive::findCodec(cachedGrid->type(), readOptions); - GridBase::Ptr grid = - convertGridForReadMode(*cachedGrid, readOptions, codec, mReadDiagnostics); - if (!grid) { - grid = cachedGrid; - if (readOptions.readMode == io::ReadMode::Half || - readOptions.readMode == io::ReadMode::Bool || - readOptions.readMode == io::ReadMode::Mask) + const TreeBase* sourceTree = &cachedGrid->constBaseTree(); + GridBase::Ptr grid; + + if (shareConvertedTrees) { + auto it = resolvedBySourceTree.find(sourceTree); + if (it != resolvedBySourceTree.end() && + (!clip || *it->second.transform == cachedGrid->transform())) { - OPENVDB_LOG_WARN(mFilename << ": grid \"" << cachedGrid->getName() - << "\" requested a read mode conversion, but no conversion is " - "available for grid type \"" << cachedGrid->type() - << "\"; returning the original type"); + const GridBase::Ptr& resolved = it->second.grid; + grid = resolved->copyGridWithNewTree(); + grid->clearMetadata(); + grid->insertMeta(*cachedGrid); + grid->setTransform(cachedGrid->transformPtr()); + grid->setTree(resolved->baseTreePtr()); + ret->push_back(grid); + continue; } } - if (clip) { - if (grid == cachedGrid) { - // Never mutate the cached grid in place; it stays owned by mGrids. - grid = grid->deepCopyGrid(); - } - grid->clipGrid(bbox); + + grid = resolveCachedGrid(cachedGrid, readOptions, mReadDiagnostics); + + if (shareConvertedTrees) { + // Keep the first-seen entry as canonical, so a later + // mismatched transform under clip doesn't overwrite it. + resolvedBySourceTree.try_emplace( + sourceTree, Resolved{grid, cachedGrid->transformPtr()}); } ret->push_back(grid); } @@ -505,42 +524,7 @@ File::readGrid(const Name& name, const io::ReadOptions& readOptions) GridBase::Ptr cachedGrid = retrieveCachedGrid(name); GridBase::Ptr grid; if (cachedGrid) { - grid = cachedGrid; - - if (readOptions.readMode == io::ReadMode::TopologyOnly) { - mReadDiagnostics.addWarning(grid->getName(), - "ReadMode::TopologyOnly is not supported for cached grids; " - "reading as original type"); - OPENVDB_LOG_WARN(mFilename << ": grid \"" << grid->getName() - << "\" requested ReadMode::TopologyOnly, but this file has no grid offsets " - "and the grid is already fully cached; returning the original type"); - } else { - io::Codec* codec = Archive::findCodec(grid->type(), readOptions); - GridBase::Ptr converted = - convertGridForReadMode(*grid, readOptions, codec, mReadDiagnostics); - if (converted) { - grid = converted; - } else if (readOptions.readMode == io::ReadMode::Half || - readOptions.readMode == io::ReadMode::Bool || - readOptions.readMode == io::ReadMode::Mask) - { - OPENVDB_LOG_WARN(mFilename << ": grid \"" << grid->getName() - << "\" requested a read mode conversion, but no conversion is " - "available for grid type \"" << grid->type() - << "\"; returning the original type"); - } - } - - const auto& bbox = readOptions.clipBBox; - const bool clip = bbox.isSorted(); - if (clip) { - if (grid == cachedGrid) { - // Never mutate the cached grid in place; it stays owned by mNamedGrids. - grid = grid->deepCopyGrid(); - } - grid->clipGrid(bbox); - } - return grid; + return resolveCachedGrid(cachedGrid, readOptions, mReadDiagnostics); } NameMapCIter it = findDescriptor(name); @@ -720,6 +704,20 @@ convertToBuildType(const GridBase& source, const std::string& targetType) } // namespace convert_grid_internal +/// @brief Return the name used in diagnostics and log messages for @a mode. +std::string +readModeName(ReadMode mode) +{ + switch (mode) { + case ReadMode::Half: return "Half"; + case ReadMode::Bool: return "Bool"; + case ReadMode::Mask: return "Mask"; + case ReadMode::TopologyOnly: return "TopologyOnly"; + case ReadMode::MetadataOnly: return "MetadataOnly"; + default: return "Original"; + } +} + GridBase::Ptr convertGridForReadMode(const GridBase& source, const ReadOptions& readOptions, Codec* codec, ReadDiagnostics& diagnostics) @@ -731,9 +729,7 @@ convertGridForReadMode(const GridBase& source, const ReadOptions& readOptions, return GridBase::Ptr(); } - const std::string modeStr = - readOptions.readMode == ReadMode::Half ? "Half" : - readOptions.readMode == ReadMode::Bool ? "Bool" : "Mask"; + const std::string modeStr = readModeName(readOptions.readMode); CodecData::Ptr codecData = codec ? codec->createData() : CodecData::Ptr(); const std::string targetType = @@ -773,6 +769,57 @@ convertGridForReadMode(const GridBase& source, const ReadOptions& readOptions, } // anonymous namespace +GridBase::Ptr +File::resolveCachedGrid(const GridBase::Ptr& cachedGrid, const io::ReadOptions& readOptions, + ReadDiagnostics& diagnostics) const +{ + if (readOptions.readMode == ReadMode::MetadataOnly) { + return cachedGrid->copyGridWithNewTree(); + } + + GridBase::Ptr grid = cachedGrid; + + if (readOptions.readMode == ReadMode::TopologyOnly) { + diagnostics.addWarning(grid->getName(), + "ReadMode::TopologyOnly is not supported for grids cached from a file " + "without grid offsets; returning the original grid with values intact"); + OPENVDB_LOG_WARN(mFilename << ": grid \"" << grid->getName() + << "\" requested ReadMode::TopologyOnly, but this file has no grid offsets " + "and the grid is already fully cached; returning the original grid " + "with values intact"); + } else { + io::Codec* codec = Archive::findCodec(grid->type(), readOptions); + GridBase::Ptr converted = convertGridForReadMode(*grid, readOptions, codec, diagnostics); + if (converted) { + grid = converted; + } else if (readOptions.readMode == ReadMode::Half || + readOptions.readMode == ReadMode::Bool || + readOptions.readMode == ReadMode::Mask) + { + const std::string modeStr = readModeName(readOptions.readMode); + OPENVDB_LOG_WARN(mFilename << ": grid \"" << grid->getName() + << "\" requested ReadMode::" << modeStr << ", but no conversion is " + "available for grid type \"" << grid->type() + << "\"; returning the original type"); + } + } + + const auto& bbox = readOptions.clipBBox; + if (bbox.isSorted()) { + if (grid == cachedGrid) { + // Don't mutate the cached grid in place, it stays owned by the caller. + grid = grid->deepCopyGrid(); + } + grid->clipGrid(bbox); + diagnostics.addWarning(cachedGrid->getName(), + "bounding box clipping was applied as a post-process because the grid " + "was cached from a file without grid offsets"); + } + + return grid; +} + + } // namespace io } // namespace OPENVDB_VERSION_NAME } // namespace openvdb diff --git a/openvdb/openvdb/io/File.h b/openvdb/openvdb/io/File.h index 920b86a50f..b8934b4463 100644 --- a/openvdb/openvdb/io/File.h +++ b/openvdb/openvdb/io/File.h @@ -157,6 +157,12 @@ class OPENVDB_API File: public Archive /// @throw KeyError if no grid with the given name exists in this file. GridBase::Ptr retrieveCachedGrid(const Name&) const; + /// @brief Return the grid that @a readOptions asks for, given the already + /// cached @a cachedGrid. Doesn't mutate @a cachedGrid, and returns it + /// unchanged when no option applies. + GridBase::Ptr resolveCachedGrid(const GridBase::Ptr& cachedGrid, + const io::ReadOptions& readOptions, ReadDiagnostics& diagnostics) const; + void writeGrids(const GridCPtrVec&, const MetaMap&, const io::WriteOptions&) const; MetaMap::Ptr fileMetadata(); diff --git a/openvdb/openvdb/unittest/TestCodec.cc b/openvdb/openvdb/unittest/TestCodec.cc index d1727543be..7522e13075 100644 --- a/openvdb/openvdb/unittest/TestCodec.cc +++ b/openvdb/openvdb/unittest/TestCodec.cc @@ -605,6 +605,7 @@ TEST_F(TestCodec, testClipBBoxInGetGridsNoGridOffsets) io::File f(path); f.open(); + f.enableReadDiagnostics(); ReadOptions clipOptions; clipOptions.clipBBox = clipBBox; @@ -616,6 +617,7 @@ TEST_F(TestCodec, testClipBBoxInGetGridsNoGridOffsets) FloatGrid::Ptr clipped = gridPtrCast((*grids)[0]); ASSERT_TRUE(clipped); EXPECT_TRUE(srcClipped->tree().hasSameTopology(clipped->tree())); + EXPECT_EQ(size_t(1), f.readDiagnostics().diagnostics().size()); } // mGrids must be unmutated: a default-options call still sees the full topology. @@ -632,6 +634,46 @@ TEST_F(TestCodec, testClipBBoxInGetGridsNoGridOffsets) std::remove(path.c_str()); } +TEST_F(TestCodec, testClipOnOffsetsFileRecordsNoDiagnostic) +{ + using namespace openvdb; + using namespace openvdb::io; + + CodecRegistry::clear(); + io::internal::initialize(); + + FloatGrid::Ptr srcGrid = FloatGrid::create(0.0f); + srcGrid->setName("clip_grid_offsets"); + srcGrid->fill(CoordBBox(Coord(-5), Coord(5)), 1.5f, true); + + const std::string path = "test_clip_getgrids_offsets.vdb"; + { + io::File f(path); + f.write(GridPtrVec{srcGrid}); + } + + const BBoxd clipBBox(Vec3d(0.0), Vec3d(3.5)); + + ReadOptions clipOptions; + clipOptions.clipBBox = clipBBox; + + io::File f(path); + f.open(); + f.enableReadDiagnostics(); + + GridPtrVecPtr grids = f.getGrids(clipOptions); + ASSERT_TRUE(grids); + ASSERT_EQ(size_t(1), grids->size()); + EXPECT_EQ(size_t(0), f.readDiagnostics().diagnostics().size()); + + GridBase::Ptr grid = f.readGrid(srcGrid->getName(), clipOptions); + ASSERT_TRUE(grid); + EXPECT_EQ(size_t(0), f.readDiagnostics().diagnostics().size()); + + f.close(); + std::remove(path.c_str()); +} + TEST_F(TestCodec, testClipAndConversionTogetherNoGridOffsets) { using namespace openvdb; @@ -660,6 +702,7 @@ TEST_F(TestCodec, testClipAndConversionTogetherNoGridOffsets) { io::File f(path); f.open(); + f.enableReadDiagnostics(); GridPtrVecPtr grids = f.getGrids(readOptions); ASSERT_TRUE(grids); ASSERT_EQ(size_t(1), grids->size()); @@ -667,6 +710,8 @@ TEST_F(TestCodec, testClipAndConversionTogetherNoGridOffsets) HalfGrid::Ptr clipped = gridPtrCast((*grids)[0]); ASSERT_TRUE(clipped); EXPECT_TRUE(srcClipped->tree().hasSameTopology(clipped->tree())); + // Only the clip diagnostic is expected, the Half conversion succeeded. + EXPECT_EQ(size_t(1), f.readDiagnostics().diagnostics().size()); f.close(); } @@ -720,6 +765,273 @@ TEST_F(TestCodec, testTopologyOnlyWarnsAndIgnoresNoGridOffsets) std::remove(path.c_str()); } +TEST_F(TestCodec, testGetGridsTopologyOnlyWarnsNoGridOffsets) +{ + using namespace openvdb; + using namespace openvdb::io; + + CodecRegistry::clear(); + io::internal::initialize(); + + FloatGrid::Ptr srcGrid = FloatGrid::create(0.0f); + srcGrid->setName("topology_only_grid"); + srcGrid->fill(CoordBBox(Coord(-5), Coord(5)), 1.5f, true); + + const std::string path = "test_getgrids_topology_only_no_offsets.vdb"; + { + std::ofstream os(path, std::ios_base::out | std::ios_base::binary); + io::Stream(os).write(GridPtrVec{srcGrid}); + } + + ReadOptions readOptions; + readOptions.readMode = ReadMode::TopologyOnly; + + // Pin getGrids() and readGrid() to the same diagnostic message. + std::string readGridMessage; + { + io::File f(path); + f.open(); + f.enableReadDiagnostics(); + GridBase::Ptr grid; + EXPECT_NO_THROW(grid = f.readGrid(srcGrid->getName(), readOptions)); + ASSERT_TRUE(grid); + EXPECT_TRUE(grid->isType()); + ASSERT_EQ(size_t(1), f.readDiagnostics().diagnostics().size()); + readGridMessage = f.readDiagnostics().diagnostics()[0].message; + f.close(); + } + + { + io::File f(path); + f.open(); + f.enableReadDiagnostics(); + GridPtrVecPtr grids; + EXPECT_NO_THROW(grids = f.getGrids(readOptions)); + ASSERT_TRUE(grids); + ASSERT_EQ(size_t(1), grids->size()); + EXPECT_TRUE((*grids)[0]->isType()); + ASSERT_EQ(size_t(1), f.readDiagnostics().diagnostics().size()); + EXPECT_EQ(f.readDiagnostics().diagnostics()[0].message, readGridMessage); + f.close(); + } + + std::remove(path.c_str()); +} + +TEST_F(TestCodec, testMetadataOnlyNoGridOffsets) +{ + using namespace openvdb; + using namespace openvdb::io; + + CodecRegistry::clear(); + io::internal::initialize(); + + FloatGrid::Ptr srcGrid = FloatGrid::create(0.0f); + srcGrid->setName("metadata_only_grid"); + srcGrid->insertMeta("author", StringMetadata("Einstein")); + srcGrid->fill(CoordBBox(Coord(-5), Coord(5)), 1.5f, true); + + const std::string path = "test_metadata_only_no_offsets.vdb"; + { + std::ofstream os(path, std::ios_base::out | std::ios_base::binary); + io::Stream(os).write(GridPtrVec{srcGrid}); + } + + ReadOptions readOptions; + readOptions.readMode = ReadMode::MetadataOnly; + + GridBase::Ptr expected; + { + io::File f(path); + f.open(); + expected = f.readGridMetadata(srcGrid->getName()); + f.close(); + } + + { + io::File f(path); + f.open(); + f.enableReadDiagnostics(); + GridPtrVecPtr grids = f.getGrids(readOptions); + ASSERT_TRUE(grids); + ASSERT_EQ(size_t(1), grids->size()); + GridBase::Ptr grid = (*grids)[0]; + EXPECT_TRUE(grid->isType()); + EXPECT_TRUE(gridPtrCast(grid)->tree().empty()); + EXPECT_EQ(std::string("Einstein"), grid->metaValue("author")); + EXPECT_EQ(expected->transform(), grid->transform()); + EXPECT_EQ(size_t(0), f.readDiagnostics().diagnostics().size()); + f.close(); + } + + { + io::File f(path); + f.open(); + f.enableReadDiagnostics(); + GridBase::Ptr grid = f.readGrid(srcGrid->getName(), readOptions); + ASSERT_TRUE(grid); + EXPECT_TRUE(grid->isType()); + EXPECT_TRUE(gridPtrCast(grid)->tree().empty()); + EXPECT_EQ(std::string("Einstein"), grid->metaValue("author")); + EXPECT_EQ(expected->transform(), grid->transform()); + EXPECT_EQ(size_t(0), f.readDiagnostics().diagnostics().size()); + f.close(); + } + + std::remove(path.c_str()); +} + +TEST_F(TestCodec, testClipInstancingNoGridOffsets) +{ + using namespace openvdb; + using namespace openvdb::io; + + CodecRegistry::clear(); + io::internal::initialize(); + + FloatTree::Ptr tree(new FloatTree(0.0f)); + tree->fill(CoordBBox(Coord(-5), Coord(5)), 1.5f, true); + + GridBase::Ptr grid1 = createGrid(tree); + grid1->setName("parent"); + GridBase::Ptr grid2 = createGrid(tree); // instance of grid1 + grid2->setName("instance"); + + const std::string path = "test_clip_instancing_no_offsets.vdb"; + { + std::ofstream os(path, std::ios_base::out | std::ios_base::binary); + io::Stream(os).write(GridPtrVec{grid1, grid2}); + } + + ReadOptions readOptions; + readOptions.clipBBox = BBoxd(Vec3d(0.0), Vec3d(3.5)); + + io::File f(path); + f.open(); + GridPtrVecPtr grids = f.getGrids(readOptions); + ASSERT_TRUE(grids); + ASSERT_EQ(size_t(2), grids->size()); + + FloatGrid::Ptr resultParent = gridPtrCast(findGridByName(*grids, "parent")); + FloatGrid::Ptr resultInstance = gridPtrCast(findGridByName(*grids, "instance")); + ASSERT_TRUE(resultParent); + ASSERT_TRUE(resultInstance); + + // Same transform, so the clipped tree is shared. + EXPECT_EQ(resultParent->treePtr(), resultInstance->treePtr()); + + f.close(); + std::remove(path.c_str()); +} + +TEST_F(TestCodec, testConversionInstancingNoGridOffsets) +{ + using namespace openvdb; + using namespace openvdb::io; + + CodecRegistry::clear(); + io::internal::initialize(); + + FloatTree::Ptr tree(new FloatTree(0.0f)); + tree->fill(CoordBBox(Coord(-5), Coord(5)), 1.5f, true); + + GridBase::Ptr grid1 = createGrid(tree); + grid1->setName("parent"); + GridBase::Ptr grid2 = createGrid(tree); // instance of grid1 + grid2->setName("instance"); + + const std::string path = "test_conversion_instancing_no_offsets.vdb"; + { + std::ofstream os(path, std::ios_base::out | std::ios_base::binary); + io::Stream(os).write(GridPtrVec{grid1, grid2}); + } + + ReadOptions readOptions; + readOptions.readMode = ReadMode::Half; + + { + io::File f(path); + f.open(); + GridPtrVecPtr grids = f.getGrids(readOptions); + ASSERT_TRUE(grids); + ASSERT_EQ(size_t(2), grids->size()); + + HalfGrid::Ptr resultParent = gridPtrCast(findGridByName(*grids, "parent")); + HalfGrid::Ptr resultInstance = gridPtrCast(findGridByName(*grids, "instance")); + ASSERT_TRUE(resultParent); + ASSERT_TRUE(resultInstance); + + // Conversion doesn't break instancing, both results share the tree. + EXPECT_EQ(resultParent->treePtr(), resultInstance->treePtr()); + f.close(); + } + + // Instancing disabled, so the two results don't share a tree. + { + io::File f(path); + f.setInstancingEnabled(false); + f.open(); + GridPtrVecPtr grids = f.getGrids(readOptions); + ASSERT_TRUE(grids); + ASSERT_EQ(size_t(2), grids->size()); + + HalfGrid::Ptr resultParent = gridPtrCast(findGridByName(*grids, "parent")); + HalfGrid::Ptr resultInstance = gridPtrCast(findGridByName(*grids, "instance")); + ASSERT_TRUE(resultParent); + ASSERT_TRUE(resultInstance); + EXPECT_NE(resultParent->treePtr(), resultInstance->treePtr()); + f.close(); + } + + std::remove(path.c_str()); +} + +TEST_F(TestCodec, testInstanceKeepsOwnTransformNoGridOffsets) +{ + using namespace openvdb; + using namespace openvdb::io; + + CodecRegistry::clear(); + io::internal::initialize(); + + FloatTree::Ptr tree(new FloatTree(0.0f)); + tree->fill(CoordBBox(Coord(-5), Coord(5)), 1.5f, true); + + GridBase::Ptr grid1 = createGrid(tree); + grid1->setName("parent"); + grid1->setTransform(math::Transform::createLinearTransform(1.0)); + GridBase::Ptr grid2 = createGrid(tree); // instance of grid1 + grid2->setName("instance"); + grid2->setTransform(math::Transform::createLinearTransform(2.0)); + + const std::string path = "test_instance_own_transform_no_offsets.vdb"; + { + std::ofstream os(path, std::ios_base::out | std::ios_base::binary); + io::Stream(os).write(GridPtrVec{grid1, grid2}); + } + + ReadOptions readOptions; + readOptions.readMode = ReadMode::Half; + + io::File f(path); + f.open(); + GridPtrVecPtr grids = f.getGrids(readOptions); + ASSERT_TRUE(grids); + ASSERT_EQ(size_t(2), grids->size()); + + HalfGrid::Ptr resultParent = gridPtrCast(findGridByName(*grids, "parent")); + HalfGrid::Ptr resultInstance = gridPtrCast(findGridByName(*grids, "instance")); + ASSERT_TRUE(resultParent); + ASSERT_TRUE(resultInstance); + + EXPECT_EQ(resultParent->treePtr(), resultInstance->treePtr()); + EXPECT_EQ(1.0, resultParent->voxelSize()[0]); + EXPECT_EQ(2.0, resultInstance->voxelSize()[0]); + + f.close(); + std::remove(path.c_str()); +} + template void testConvertCodecImpl() { From af6988fcea22a0e1be3da9b454ec1de210887ac0 Mon Sep 17 00:00:00 2001 From: Dan Bailey Date: Wed, 26 Aug 2026 21:30:16 -0700 Subject: [PATCH 3/9] Fix TopologyOnly mode for BoolGrids and MaskGrids Signed-off-by: Dan Bailey --- openvdb/openvdb/codecs/TopologyCodec.h | 15 ++-- openvdb/openvdb/unittest/TestCodec.cc | 115 +++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 7 deletions(-) diff --git a/openvdb/openvdb/codecs/TopologyCodec.h b/openvdb/openvdb/codecs/TopologyCodec.h index 47f07b7ba0..10816609ab 100644 --- a/openvdb/openvdb/codecs/TopologyCodec.h +++ b/openvdb/openvdb/codecs/TopologyCodec.h @@ -312,17 +312,18 @@ void topologyCodecReadTopology(GridBase& gridBase, std::istream& is, const io::R internal::setTilesToBackground(grid.tree()); // allocate leaf buffers in parallel and fill with the background value; // ReadTopologyOp uses PartialCreate which leaves buffers unallocated. - const auto background = grid.tree().root().background(); - tree::LeafManager leafManager(grid.tree()); - leafManager.foreach([&background](auto& leaf, size_t) { - using LeafType = std::decay_t; - if constexpr (!std::is_same_v) { + // (skip BoolGrids/MaskGrids whose buffers are bit masks that are always + // allocated and so provide no empty()/allocate()) + if constexpr (!std::is_same_v) { + const auto background = grid.tree().root().background(); + tree::LeafManager leafManager(grid.tree()); + leafManager.foreach([&background](auto& leaf, size_t) { if (leaf.buffer().empty()) { leaf.buffer().allocate(); leaf.buffer().fill(background); } - } - }); + }); + } return; } } diff --git a/openvdb/openvdb/unittest/TestCodec.cc b/openvdb/openvdb/unittest/TestCodec.cc index 7522e13075..260cbb14e7 100644 --- a/openvdb/openvdb/unittest/TestCodec.cc +++ b/openvdb/openvdb/unittest/TestCodec.cc @@ -1196,3 +1196,118 @@ TEST_F(TestCodec, testInactiveValuesAfterReadBuffers) std::remove(path.c_str()); } } + +// A MaskGrid has no ValueMask specialization of InternalNode or RootNode, so its +// tiles carry a value separate from the active state. TopologyOnly resets those +// values to the background, which must leave the active topology untouched. +TEST_F(TestCodec, testTopologyOnlyPreservesMaskTiles) +{ + using namespace openvdb; + using namespace openvdb::io; + + const std::string gridName = "mask_tiles"; + const std::string path = "testTopologyOnlyMaskTiles.vdb"; + + // Active tiles at both internal levels and at the root, plus a region of + // individual voxels so that leaf nodes are exercised too. + MaskGrid::Ptr src = MaskGrid::create(); + src->setName(gridName); + src->tree().addTile(/*level=*/1, Coord(0), true, /*active=*/true); + src->tree().addTile(/*level=*/2, Coord(4096), true, /*active=*/true); + src->tree().addTile(/*level=*/3, Coord(-8192), true, /*active=*/true); + src->fill(CoordBBox(Coord(1024), Coord(1030)), true, /*active=*/true); + + const Index64 srcActiveVoxels = src->activeVoxelCount(); + const Index64 srcActiveTiles = src->tree().activeTileCount(); + ASSERT_TRUE(srcActiveTiles > 0); + + { + io::File f(path); + f.write(GridPtrVec{src}); + } + + ReadOptions topoOpts; + topoOpts.readMode = ReadMode::TopologyOnly; + + MaskGrid::Ptr readTopo, readOriginal; + { + io::File f(path); + f.open(); + readTopo = gridPtrCast(f.readGrid(gridName, topoOpts)); + readOriginal = gridPtrCast(f.readGrid(gridName, ReadOptions{})); + f.close(); + } + ASSERT_TRUE(readTopo); + ASSERT_TRUE(readOriginal); + + // TopologyOnly must not lose the active tiles. + EXPECT_EQ(readTopo->tree().activeTileCount(), srcActiveTiles); + EXPECT_EQ(readTopo->activeVoxelCount(), srcActiveVoxels); + EXPECT_TRUE(src->tree().hasSameTopology(readTopo->tree())); + EXPECT_TRUE(readOriginal->tree().hasSameTopology(readTopo->tree())); + + // Leaf voxels keep their values because value and active state share one bit. + auto topoAcc = readTopo->getConstAccessor(); + for (MaskGrid::ValueOnCIter it = readOriginal->cbeginValueOn(); it; ++it) { + if (it.isVoxelValue()) EXPECT_TRUE(topoAcc.isValueOn(it.getCoord())); + } + + // Tile values are reset to the background, matching a TopologyCopy, while the + // tiles stay active. + for (MaskGrid::ValueOnCIter it = readTopo->cbeginValueOn(); it; ++it) { + if (!it.isVoxelValue()) EXPECT_FALSE(*it); + } + + std::remove(path.c_str()); +} + +// A BoolGrid stores the value and the active state separately, so TopologyOnly is +// expected to discard values while keeping topology. +TEST_F(TestCodec, testTopologyOnlyClearsBoolTiles) +{ + using namespace openvdb; + using namespace openvdb::io; + + const std::string gridName = "bool_tiles"; + const std::string path = "testTopologyOnlyBoolTiles.vdb"; + + // Active tiles at both internal levels, plus a region of individual voxels. + BoolGrid::Ptr src = BoolGrid::create(/*background=*/false); + src->setName(gridName); + src->tree().addTile(/*level=*/1, Coord(0), true, /*active=*/true); + src->tree().addTile(/*level=*/2, Coord(4096), true, /*active=*/true); + src->fill(CoordBBox(Coord(1024), Coord(1030)), true, /*active=*/true); + + const Index64 srcActiveVoxels = src->activeVoxelCount(); + const Index64 srcActiveTiles = src->tree().activeTileCount(); + ASSERT_TRUE(srcActiveTiles > 0); + + { + io::File f(path); + f.write(GridPtrVec{src}); + } + + ReadOptions topoOpts; + topoOpts.readMode = ReadMode::TopologyOnly; + + BoolGrid::Ptr readTopo; + { + io::File f(path); + f.open(); + readTopo = gridPtrCast(f.readGrid(gridName, topoOpts)); + f.close(); + } + ASSERT_TRUE(readTopo); + + // Topology is preserved, including active tiles. + EXPECT_EQ(readTopo->tree().activeTileCount(), srcActiveTiles); + EXPECT_EQ(readTopo->activeVoxelCount(), srcActiveVoxels); + EXPECT_TRUE(src->tree().hasSameTopology(readTopo->tree())); + + // Values are discarded: tiles are reset to the background even where active. + for (BoolGrid::ValueAllCIter it = readTopo->cbeginValueAll(); it; ++it) { + if (!it.isVoxelValue()) EXPECT_FALSE(*it); + } + + std::remove(path.c_str()); +} From 3a86689535259919e93be9a7499f2f3585bf9eca Mon Sep 17 00:00:00 2001 From: Dan Bailey Date: Wed, 26 Aug 2026 22:29:57 -0700 Subject: [PATCH 4/9] Fix MaskGrid conversion for files with no offsets Signed-off-by: Dan Bailey --- openvdb/openvdb/io/File.cc | 100 ++++++++++++++------------ openvdb/openvdb/unittest/TestCodec.cc | 54 ++++++++++++++ 2 files changed, 109 insertions(+), 45 deletions(-) diff --git a/openvdb/openvdb/io/File.cc b/openvdb/openvdb/io/File.cc index 2f6cea1f14..6d543b9821 100644 --- a/openvdb/openvdb/io/File.cc +++ b/openvdb/openvdb/io/File.cc @@ -34,9 +34,10 @@ namespace { /// produced had it been read through @a codec (looked up by the caller via /// the protected @c Archive::findCodec(), since this is a free function). /// Returns null if no conversion is needed or possible, in which case the -/// caller keeps the original grid and @a diagnostics is set accordingly. +/// caller keeps the original grid. A conversion that was requested but cannot +/// be done is reported to @a diagnostics and logged against @a filename. GridBase::Ptr convertGridForReadMode(const GridBase& source, const ReadOptions& readOptions, - Codec* codec, ReadDiagnostics& diagnostics); + Codec* codec, ReadDiagnostics& diagnostics, const std::string& filename); /// @brief Return the name used in diagnostics and log messages for @a mode. std::string readModeName(ReadMode mode); @@ -677,25 +678,42 @@ namespace { namespace convert_grid_internal { /// @brief Convert @a source to the grid type @c ValueConverter::Type, -/// provided the registry-reported @a targetType agrees and the value -/// conversion is legal. Returns null otherwise. +/// provided the registry-reported @a targetType agrees and the value conversion +/// is legal. Sets @a alreadyTargetType when @a source has that build type +/// already. Returns null in both cases, so the caller keeps the original grid. template inline GridBase::Ptr -convertToBuildType(const GridBase& source, const std::string& targetType) +convertToTargetType(const GridBase& source, const std::string& targetType, + bool& alreadyTargetType) { + // A MaskGrid source is only visited when the target is itself a mask, where + // the build types match and the branch below attempts no conversion. No + // LeafNode conversion constructor exists from ValueMask to another build type. + using SourceGridTypes = std::conditional_t, + GridTypes, GridTypes::Remove>; + GridBase::Ptr result; - source.apply([&](const auto& typedSource) { + source.apply([&](const auto& typedSource) { using SourceGridT = std::decay_t; using TargetGridT = typename SourceGridT::template ValueConverter::Type; - // No LeafNode conversion constructor exists from ValueMask to any - // other build type, so exclude it here; nested if constexpr avoids - // instantiating the ambiguous CanConvertType. - if constexpr (!std::is_same_v) { - if constexpr (CanConvertType::value) { - if (TargetGridT::gridType() == targetType) { - result = typename TargetGridT::Ptr(new TargetGridT(typedSource)); - } + if constexpr (std::is_same_v) { + alreadyTargetType = true; + } else if constexpr (CanConvertType::value) { + if (TargetGridT::gridType() != targetType) return; + if constexpr (std::is_same_v) { + // A mask records active state, not values, so copy the topology + // instead of casting values. Casting would let a non-zero + // background or an inactive non-zero tile become true, which the + // codec path does not do. create() takes the GridBase overload to + // copy the metadata and transform without converting the values. + auto target = TargetGridT::create(static_cast(typedSource)); + target->setTree(typename TargetGridT::TreeType::Ptr( + new typename TargetGridT::TreeType(typedSource.constTree(), + /*inactiveValue=*/false, /*activeValue=*/true, TopologyCopy()))); + result = target; + } else { + result = typename TargetGridT::Ptr(new TargetGridT(typedSource)); } } }); @@ -720,7 +738,7 @@ readModeName(ReadMode mode) GridBase::Ptr convertGridForReadMode(const GridBase& source, const ReadOptions& readOptions, - Codec* codec, ReadDiagnostics& diagnostics) + Codec* codec, ReadDiagnostics& diagnostics, const std::string& filename) { if (readOptions.readMode != ReadMode::Half && readOptions.readMode != ReadMode::Bool && @@ -729,38 +747,38 @@ convertGridForReadMode(const GridBase& source, const ReadOptions& readOptions, return GridBase::Ptr(); } - const std::string modeStr = readModeName(readOptions.readMode); - + // Ask the codec that would have read this grid which type it produces. CodecData::Ptr codecData = codec ? codec->createData() : CodecData::Ptr(); const std::string targetType = (codecData && codecData->grid) ? codecData->grid->type() : std::string(); - // No conversion codec registered for this grid type (falls through to the - // plain gridType codec), or the registry agrees the type is unchanged. - if (targetType.empty() || targetType == source.type()) { - diagnostics.addWarning(source.getName(), - "ReadMode::" + modeStr + " conversion is not supported for grid type '" - + source.type() + "'; reading as original type"); - return GridBase::Ptr(); - } - GridBase::Ptr result; + bool alreadyTargetType = false; if (readOptions.readMode == ReadMode::Half) { - result = convert_grid_internal::convertToBuildType(source, targetType); + result = convert_grid_internal::convertToTargetType( + source, targetType, alreadyTargetType); } else if (readOptions.readMode == ReadMode::Bool) { - result = convert_grid_internal::convertToBuildType(source, targetType); + result = convert_grid_internal::convertToTargetType( + source, targetType, alreadyTargetType); } else { - result = convert_grid_internal::convertToBuildType(source, targetType); + result = convert_grid_internal::convertToTargetType( + source, targetType, alreadyTargetType); } - // The registry named a target type that this dispatch could not produce - // (e.g. a custom-registered grid type outside GridTypes, or the - // CanConvertType guard rejected the pair): fall through with a warning - // rather than silently diverging from what the registry reported. - if (!result) { + // Either no conversion codec is registered for this grid type (targetType is + // empty, or names the plain gridType codec), or the registry named a target + // type this dispatch cannot produce, such as a grid type outside GridTypes or + // a pair that CanConvertType rejects. A grid that already has the requested + // build type is not a failure, so it is not reported. + if (!result && !alreadyTargetType) { + const std::string modeStr = readModeName(readOptions.readMode); diagnostics.addWarning(source.getName(), "ReadMode::" + modeStr + " conversion is not supported for grid type '" + source.type() + "'; reading as original type"); + OPENVDB_LOG_WARN(filename << ": grid \"" << source.getName() + << "\" requested ReadMode::" << modeStr << ", but no conversion is " + "available for grid type \"" << source.type() + << "\"; returning the original type"); } return result; @@ -789,18 +807,10 @@ File::resolveCachedGrid(const GridBase::Ptr& cachedGrid, const io::ReadOptions& "with values intact"); } else { io::Codec* codec = Archive::findCodec(grid->type(), readOptions); - GridBase::Ptr converted = convertGridForReadMode(*grid, readOptions, codec, diagnostics); - if (converted) { - grid = converted; - } else if (readOptions.readMode == ReadMode::Half || - readOptions.readMode == ReadMode::Bool || - readOptions.readMode == ReadMode::Mask) + if (GridBase::Ptr converted = + convertGridForReadMode(*grid, readOptions, codec, diagnostics, mFilename)) { - const std::string modeStr = readModeName(readOptions.readMode); - OPENVDB_LOG_WARN(mFilename << ": grid \"" << grid->getName() - << "\" requested ReadMode::" << modeStr << ", but no conversion is " - "available for grid type \"" << grid->type() - << "\"; returning the original type"); + grid = converted; } } diff --git a/openvdb/openvdb/unittest/TestCodec.cc b/openvdb/openvdb/unittest/TestCodec.cc index 260cbb14e7..a952e21c2d 100644 --- a/openvdb/openvdb/unittest/TestCodec.cc +++ b/openvdb/openvdb/unittest/TestCodec.cc @@ -1311,3 +1311,57 @@ TEST_F(TestCodec, testTopologyOnlyClearsBoolTiles) std::remove(path.c_str()); } + +// ReadMode::Mask must record which voxels are active, not cast the source values. +// A non-zero background and an inactive non-zero tile would both become true under +// a value cast. Streamed files have no grid offsets, so this exercises the +// in-memory conversion in File::resolveCachedGrid rather than the codec path. +TEST_F(TestCodec, testCachedMaskConversionIgnoresValues) +{ + using namespace openvdb; + using namespace openvdb::io; + + io::internal::initialize(); + + const std::string gridName = "nonzero_background"; + const std::string path = "testCachedMaskConversion.vdb"; + + // A non-zero background, an inactive non-zero tile and an active region. + FloatGrid::Ptr src = FloatGrid::create(/*background=*/1.0f); + src->setName(gridName); + src->tree().addTile(/*level=*/2, Coord(4096), 0.1f, /*active=*/false); + src->fill(CoordBBox(Coord(0), Coord(6)), 5.0f, /*active=*/true); + + const Index64 srcActiveVoxels = src->activeVoxelCount(); + ASSERT_TRUE(srcActiveVoxels > 0); + + // Write via io::Stream so the file has no grid offsets and every grid is + // cached up front on open(). + { + std::ofstream os(path, std::ios_base::out | std::ios_base::binary); + io::Stream(os).write(GridPtrVec{src}); + } + + ReadOptions maskOpts; + maskOpts.readMode = ReadMode::Mask; + + MaskGrid::Ptr readMask; + { + io::File f(path); + f.open(); + readMask = gridPtrCast(f.readGrid(gridName, maskOpts)); + f.close(); + } + ASSERT_TRUE(readMask); + + // Only the active voxels are on. A value cast would have activated nothing + // extra, but it would have set the background and the inactive tile to true. + EXPECT_EQ(readMask->activeVoxelCount(), srcActiveVoxels); + EXPECT_TRUE(src->tree().hasSameTopology(readMask->tree())); + EXPECT_FALSE(readMask->background()); + + // The inactive 0.1f tile must not have become an active or true tile. + EXPECT_FALSE(readMask->tree().getValue(Coord(4096))); + + std::remove(path.c_str()); +} From 863b4c6246e2e672212e310cd90913b929371382 Mon Sep 17 00:00:00 2001 From: Dan Bailey Date: Mon, 31 Aug 2026 23:40:58 -0700 Subject: [PATCH 5/9] Add clang conversion warning pragmas Signed-off-by: Dan Bailey --- openvdb/openvdb/Platform.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/openvdb/openvdb/Platform.h b/openvdb/openvdb/Platform.h index 322a7cae46..f947efc2b6 100644 --- a/openvdb/openvdb/Platform.h +++ b/openvdb/openvdb/Platform.h @@ -219,6 +219,14 @@ #if defined __INTEL_COMPILER #define OPENVDB_NO_TYPE_CONVERSION_WARNING_BEGIN #define OPENVDB_NO_TYPE_CONVERSION_WARNING_END +#elif defined __clang__ + #define OPENVDB_NO_TYPE_CONVERSION_WARNING_BEGIN \ + _Pragma("clang diagnostic push") \ + _Pragma("clang diagnostic ignored \"-Wconversion\"") \ + _Pragma("clang diagnostic ignored \"-Wfloat-conversion\"") \ + _Pragma("clang diagnostic ignored \"-Wimplicit-float-conversion\"") + #define OPENVDB_NO_TYPE_CONVERSION_WARNING_END \ + _Pragma("clang diagnostic pop") #elif defined __GNUC__ // -Wfloat-conversion was only introduced in GCC 4.9 #define OPENVDB_NO_TYPE_CONVERSION_WARNING_BEGIN \ From 9acb15143bf4f7c159266e16a3439b6b5171e06f Mon Sep 17 00:00:00 2001 From: Dan Bailey Date: Mon, 31 Aug 2026 23:42:20 -0700 Subject: [PATCH 6/9] Add no type conversion warnings around Local::convertValue() and InternalNode::DeepCopy Signed-off-by: Dan Bailey --- openvdb/openvdb/tree/InternalNode.h | 2 ++ openvdb/openvdb/tree/LeafNode.h | 6 +++++- openvdb/openvdb/tree/RootNode.h | 6 +++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/openvdb/openvdb/tree/InternalNode.h b/openvdb/openvdb/tree/InternalNode.h index c61deedbfe..42558c4fcd 100644 --- a/openvdb/openvdb/tree/InternalNode.h +++ b/openvdb/openvdb/tree/InternalNode.h @@ -988,7 +988,9 @@ struct InternalNode::DeepCopy void operator()(const tbb::blocked_range &r) const { for (Index i = r.begin(), end=r.end(); i!=end; ++i) { if (s->mChildMask.isOff(i)) { + OPENVDB_NO_TYPE_CONVERSION_WARNING_BEGIN t->mNodes[i].setValue(ValueType(s->mNodes[i].getValue())); + OPENVDB_NO_TYPE_CONVERSION_WARNING_END } else { t->mNodes[i].setChild(new ChildNodeType(*(s->mNodes[i].getChild()))); } diff --git a/openvdb/openvdb/tree/LeafNode.h b/openvdb/openvdb/tree/LeafNode.h index 21cae3d7a2..79c8b3d9a7 100644 --- a/openvdb/openvdb/tree/LeafNode.h +++ b/openvdb/openvdb/tree/LeafNode.h @@ -1009,7 +1009,11 @@ LeafNode::LeafNode(const LeafNode& other) { struct Local { /// @todo Consider using a value conversion functor passed as an argument instead. - static inline ValueType convertValue(const OtherValueType& val) { return ValueType(val); } + static inline ValueType convertValue(const OtherValueType& val) { + OPENVDB_NO_TYPE_CONVERSION_WARNING_BEGIN + return ValueType(val); + OPENVDB_NO_TYPE_CONVERSION_WARNING_END + } }; for (Index i = 0; i < SIZE; ++i) { diff --git a/openvdb/openvdb/tree/RootNode.h b/openvdb/openvdb/tree/RootNode.h index ccf9bc6a24..3eec43a7f9 100644 --- a/openvdb/openvdb/tree/RootNode.h +++ b/openvdb/openvdb/tree/RootNode.h @@ -1178,7 +1178,11 @@ struct RootNodeCopyHelper struct Local { /// @todo Consider using a value conversion functor passed as an argument instead. - static inline ValueT convertValue(const OtherValueT& val) { return ValueT(val); } + static inline ValueT convertValue(const OtherValueT& val) { + OPENVDB_NO_TYPE_CONVERSION_WARNING_BEGIN + return ValueT(val); + OPENVDB_NO_TYPE_CONVERSION_WARNING_END + } }; self.mBackground = Local::convertValue(other.mBackground); From 829f6b75e71504773bdca897b593fbf321f8097b Mon Sep 17 00:00:00 2001 From: Dan Bailey Date: Tue, 1 Sep 2026 10:39:35 -0700 Subject: [PATCH 7/9] Fix a minor dangling else issue Signed-off-by: Dan Bailey --- openvdb/openvdb/unittest/TestCodec.cc | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/openvdb/openvdb/unittest/TestCodec.cc b/openvdb/openvdb/unittest/TestCodec.cc index a952e21c2d..7f02d14d72 100644 --- a/openvdb/openvdb/unittest/TestCodec.cc +++ b/openvdb/openvdb/unittest/TestCodec.cc @@ -1249,13 +1249,17 @@ TEST_F(TestCodec, testTopologyOnlyPreservesMaskTiles) // Leaf voxels keep their values because value and active state share one bit. auto topoAcc = readTopo->getConstAccessor(); for (MaskGrid::ValueOnCIter it = readOriginal->cbeginValueOn(); it; ++it) { - if (it.isVoxelValue()) EXPECT_TRUE(topoAcc.isValueOn(it.getCoord())); + if (it.isVoxelValue()) { + EXPECT_TRUE(topoAcc.isValueOn(it.getCoord())); + } } // Tile values are reset to the background, matching a TopologyCopy, while the // tiles stay active. for (MaskGrid::ValueOnCIter it = readTopo->cbeginValueOn(); it; ++it) { - if (!it.isVoxelValue()) EXPECT_FALSE(*it); + if (!it.isVoxelValue()) { + EXPECT_FALSE(*it); + } } std::remove(path.c_str()); @@ -1306,7 +1310,9 @@ TEST_F(TestCodec, testTopologyOnlyClearsBoolTiles) // Values are discarded: tiles are reset to the background even where active. for (BoolGrid::ValueAllCIter it = readTopo->cbeginValueAll(); it; ++it) { - if (!it.isVoxelValue()) EXPECT_FALSE(*it); + if (!it.isVoxelValue()) { + EXPECT_FALSE(*it); + } } std::remove(path.c_str()); From f5d807d33bfc52434ed628612785c327f6499526 Mon Sep 17 00:00:00 2001 From: Dan Bailey Date: Fri, 4 Sep 2026 11:04:11 -0700 Subject: [PATCH 8/9] Address feedback Signed-off-by: Dan Bailey --- openvdb/openvdb/io/Codec.h | 18 ++++++++++++------ openvdb/openvdb/unittest/TestCodec.cc | 12 ++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/openvdb/openvdb/io/Codec.h b/openvdb/openvdb/io/Codec.h index 473102b57a..95210bbf5c 100644 --- a/openvdb/openvdb/io/Codec.h +++ b/openvdb/openvdb/io/Codec.h @@ -77,12 +77,18 @@ enum class ReadMode { /// then constructing a @c MaskGrid from the active-voxel set, but may /// be performed more efficiently inside the codec. Mask, - /// Deserialize topology only; value buffers are skipped. The resulting - /// grid has a valid tree structure (active/inactive state, node - /// hierarchy) and all leaf buffers are allocated and filled with the - /// grid's background value. Useful when only the active-voxel mask is - /// needed and avoiding the cost of reading large value buffers is - /// desirable. + /// Deserialize topology only; reading of value buffers may be skipped. + /// Useful when only the active-voxel mask is needed and avoiding the cost + /// of reading large value buffers is desirable. + /// + /// @warning Only the topology is guaranteed. The resulting grid has a + /// valid tree structure (node hierarchy, tiles and active/inactive state), + /// but the voxel and tile values are unspecified. They are typically the + /// grid's background value, however a grid read from a file without grid + /// offsets is fully cached up-front, so the returned grid keeps the values + /// that were read. Deep-copying such a grid to reset its values would cost + /// more than the read this mode is meant to avoid. Callers must not rely + /// on the values being the background value. TopologyOnly, /// Deserialize grid metadata and transform only; no topology, no value /// buffers. The codec is still used to construct the correct grid type, diff --git a/openvdb/openvdb/unittest/TestCodec.cc b/openvdb/openvdb/unittest/TestCodec.cc index 7f02d14d72..765e7c1ac4 100644 --- a/openvdb/openvdb/unittest/TestCodec.cc +++ b/openvdb/openvdb/unittest/TestCodec.cc @@ -245,6 +245,7 @@ void testIOImpl( EXPECT_EQ(readTopo->tree().leafCount(), srcGrid->tree().leafCount()); EXPECT_TRUE(readTopo->tree().leafCount() > 0); EXPECT_EQ(readTopo->activeVoxelCount(), srcGrid->activeVoxelCount()); + EXPECT_TRUE(srcGrid->tree().hasSameTopology(readTopo->tree())); // verify leaf buffers are allocated (bool/mask buffers are always present, skip empty() check) if constexpr (!std::is_same_v) { for (auto leafIter = readTopo->tree().cbeginLeaf(); leafIter; ++leafIter) { @@ -761,6 +762,11 @@ TEST_F(TestCodec, testTopologyOnlyWarnsAndIgnoresNoGridOffsets) EXPECT_TRUE(grid->isType()); EXPECT_EQ(size_t(1), f.readDiagnostics().diagnostics().size()); + // Whatever happens to the values, the topology must match the source grid. + FloatGrid::Ptr floatGrid = gridPtrCast(grid); + ASSERT_TRUE(floatGrid); + EXPECT_TRUE(srcGrid->tree().hasSameTopology(floatGrid->tree())); + f.close(); std::remove(path.c_str()); } @@ -798,6 +804,9 @@ TEST_F(TestCodec, testGetGridsTopologyOnlyWarnsNoGridOffsets) EXPECT_TRUE(grid->isType()); ASSERT_EQ(size_t(1), f.readDiagnostics().diagnostics().size()); readGridMessage = f.readDiagnostics().diagnostics()[0].message; + FloatGrid::Ptr floatGrid = gridPtrCast(grid); + ASSERT_TRUE(floatGrid); + EXPECT_TRUE(srcGrid->tree().hasSameTopology(floatGrid->tree())); f.close(); } @@ -812,6 +821,9 @@ TEST_F(TestCodec, testGetGridsTopologyOnlyWarnsNoGridOffsets) EXPECT_TRUE((*grids)[0]->isType()); ASSERT_EQ(size_t(1), f.readDiagnostics().diagnostics().size()); EXPECT_EQ(f.readDiagnostics().diagnostics()[0].message, readGridMessage); + FloatGrid::Ptr floatGrid = gridPtrCast((*grids)[0]); + ASSERT_TRUE(floatGrid); + EXPECT_TRUE(srcGrid->tree().hasSameTopology(floatGrid->tree())); f.close(); } From ab58a58dd142adfb8f830e3b90adf3b298b7ffa0 Mon Sep 17 00:00:00 2001 From: Dan Bailey Date: Sat, 5 Sep 2026 18:31:37 -0700 Subject: [PATCH 9/9] Fix ReadTopology for MaskGrids to always use active state Signed-off-by: Dan Bailey --- openvdb/openvdb/codecs/TopologyCodec.h | 33 +++- openvdb/openvdb/unittest/TestCodec.cc | 211 +++++++++++++++++++++---- 2 files changed, 209 insertions(+), 35 deletions(-) diff --git a/openvdb/openvdb/codecs/TopologyCodec.h b/openvdb/openvdb/codecs/TopologyCodec.h index 10816609ab..a7855fe37b 100644 --- a/openvdb/openvdb/codecs/TopologyCodec.h +++ b/openvdb/openvdb/codecs/TopologyCodec.h @@ -117,6 +117,11 @@ struct ReadTopologyOp using LeafT = typename TreeT::LeafNodeType; using StorageValueT = typename StorageTreeT::ValueType; + // A ValueMask target records active state, not cast storage values: value + // must equal active state at every level, so the background and tile/node + // values are forced from activity instead of being converted from storage. + static constexpr bool isMaskTarget = std::is_same_v; + ReadTopologyOp(std::istream& _is, bool _saveFloatAsHalf, io::ReadDiagnostics& _diagnostics, const std::string& _gridName) : is(_is) @@ -140,7 +145,11 @@ struct ReadTopologyOp // Read a RootNode that was stored in the current format. is.read(reinterpret_cast(&storageBackground), sizeof(StorageValueT)); - background = static_cast(storageBackground); + if constexpr (isMaskTarget) { + background = false; + } else { + background = static_cast(storageBackground); + } Index numTiles = 0, numChildren = 0; is.read(reinterpret_cast(&numTiles), sizeof(Index)); @@ -156,7 +165,9 @@ struct ReadTopologyOp is.read(reinterpret_cast(&value), sizeof(StorageValueT)); is.read(reinterpret_cast(&active), sizeof(bool)); Coord origin(vec); - if constexpr (std::is_same_v) { + if constexpr (isMaskTarget) { + root.addTile(origin, active, active); + } else if constexpr (std::is_same_v) { root.addTile(origin, value, active); } else { root.addTile(origin, static_cast(value), active); @@ -194,7 +205,10 @@ struct ReadTopologyOp StorageValueT* values = valuePtr.get(); io::readCompressedValues(is, values, numValues, valueMask, saveFloatAsHalf, &storageBackground); - // Copy values from the array into this node's table. + // Copy values from the array into this node's table. For a + // ValueMask target the decoded values array is only read to keep + // the stream position correct; the value comes from the value + // mask instead, so that value equals active state. if (oldVersion) { // The node's member child mask is still empty at this point // (PartialCreate; setChildUnsafe runs below), so iterate the @@ -202,12 +216,21 @@ struct ReadTopologyOp // avoid over-reading the countOff-sized values array. Index n = 0; for (auto iter = childMask.beginOff(); iter; ++iter) { - node.setValueOnlyUnsafe(iter.pos(), static_cast(values[n++])); + if constexpr (isMaskTarget) { + node.setValueOnlyUnsafe(iter.pos(), valueMask.isOn(iter.pos())); + } else { + node.setValueOnlyUnsafe(iter.pos(), static_cast(values[n])); + } + ++n; } OPENVDB_ASSERT(n == numValues); } else { for (auto iter = node.beginValueAll(); iter; ++iter) { - node.setValueOnlyUnsafe(iter.pos(), static_cast(values[iter.pos()])); + if constexpr (isMaskTarget) { + node.setValueOnlyUnsafe(iter.pos(), valueMask.isOn(iter.pos())); + } else { + node.setValueOnlyUnsafe(iter.pos(), static_cast(values[iter.pos()])); + } } } } diff --git a/openvdb/openvdb/unittest/TestCodec.cc b/openvdb/openvdb/unittest/TestCodec.cc index 765e7c1ac4..ce163f0920 100644 --- a/openvdb/openvdb/unittest/TestCodec.cc +++ b/openvdb/openvdb/unittest/TestCodec.cc @@ -1330,56 +1330,207 @@ TEST_F(TestCodec, testTopologyOnlyClearsBoolTiles) std::remove(path.c_str()); } -// ReadMode::Mask must record which voxels are active, not cast the source values. -// A non-zero background and an inactive non-zero tile would both become true under -// a value cast. Streamed files have no grid offsets, so this exercises the -// in-memory conversion in File::resolveCachedGrid rather than the codec path. -TEST_F(TestCodec, testCachedMaskConversionIgnoresValues) +// Compare two grids of the same type for background, topology, values and active +// states, so that a conversion done on read can be checked against a reference. +template +void expectGridsMatch(const GridT& reference, const GridT& other, const std::string& context) +{ + using namespace openvdb; + + SCOPED_TRACE(context); + + EXPECT_EQ(reference.background(), other.background()); + EXPECT_TRUE(reference.constTree().hasSameTopology(other.constTree())); + EXPECT_EQ(reference.activeVoxelCount(), other.activeVoxelCount()); + EXPECT_EQ(reference.constTree().activeTileCount(), other.constTree().activeTileCount()); + + // Walk each grid against an accessor on the other so that a tile or voxel + // present in only one of them is still compared. + auto otherAccessor = other.getConstAccessor(); + for (typename GridT::ValueAllCIter it = reference.cbeginValueAll(); it; ++it) { + const Coord ijk = it.getCoord(); + EXPECT_EQ(*it, otherAccessor.getValue(ijk)); + EXPECT_EQ(it.isValueOn(), otherAccessor.isValueOn(ijk)); + } + auto referenceAccessor = reference.getConstAccessor(); + for (typename GridT::ValueAllCIter it = other.cbeginValueAll(); it; ++it) { + const Coord ijk = it.getCoord(); + EXPECT_EQ(*it, referenceAccessor.getValue(ijk)); + EXPECT_EQ(it.isValueOn(), referenceAccessor.isValueOn(ijk)); + } +} + +// A conversion readMode must produce the same grid as reading the grid in its +// original type and converting it in memory. That in-memory conversion is the +// reference for the conversion codec (files with grid offsets) and for the cached +// conversion in File::resolveCachedGrid (files without). This holds for Bool and +// Half, whose targets have genuine values distinct from their active state; Mask +// is the exception and is tested separately in testMaskConversionMatchesInMemory. +template +void testConversionMatchesInMemoryImpl(const std::string& label) { using namespace openvdb; using namespace openvdb::io; + CodecRegistry::clear(); io::internal::initialize(); - const std::string gridName = "nonzero_background"; - const std::string path = "testCachedMaskConversion.vdb"; + const std::string gridName = "conversion_parity"; - // A non-zero background, an inactive non-zero tile and an active region. + // A non-zero background, an inactive non-zero tile, an active zero tile and a + // region of active voxels. Casting values and copying activity disagree on + // the background and on both tiles, which is what makes this a useful case. + // The two tiles sit in different internal nodes so that neither node holds + // more than two distinct inactive values, keeping them exact under the + // default active mask compression. FloatGrid::Ptr src = FloatGrid::create(/*background=*/1.0f); src->setName(gridName); src->tree().addTile(/*level=*/2, Coord(4096), 0.1f, /*active=*/false); + src->tree().addTile(/*level=*/2, Coord(8192), 0.0f, /*active=*/true); src->fill(CoordBBox(Coord(0), Coord(6)), 5.0f, /*active=*/true); - const Index64 srcActiveVoxels = src->activeVoxelCount(); - ASSERT_TRUE(srcActiveVoxels > 0); - - // Write via io::Stream so the file has no grid offsets and every grid is - // cached up front on open(). + // io::File writes grid offsets, so a conversion readMode goes through the + // conversion codec. io::Stream writes none, so every grid is cached on open() + // and converted in memory instead. + const std::string offsetsPath = "test_conversion_parity_offsets_" + label + ".vdb"; + const std::string noOffsetsPath = "test_conversion_parity_no_offsets_" + label + ".vdb"; { - std::ofstream os(path, std::ios_base::out | std::ios_base::binary); + io::File f(offsetsPath); + f.write(GridPtrVec{src}); + } + { + std::ofstream os(noOffsetsPath, std::ios_base::out | std::ios_base::binary); io::Stream(os).write(GridPtrVec{src}); } - ReadOptions maskOpts; - maskOpts.readMode = ReadMode::Mask; + ReadOptions convertOptions; + convertOptions.readMode = mode; + + for (const std::string& path : {offsetsPath, noOffsetsPath}) { + FloatGrid::Ptr readOriginal; + typename DstGridT::Ptr readConverted; + { + io::File f(path); + f.open(); + readOriginal = gridPtrCast(f.readGrid(gridName, ReadOptions{})); + readConverted = gridPtrCast(f.readGrid(gridName, convertOptions)); + f.close(); + } + ASSERT_TRUE(readOriginal); + ASSERT_TRUE(readConverted); + + // Read as the original type, then convert in memory: the reference. + const DstGridT reference(*readOriginal); + + // Pin the reference semantics for the bool-valued targets, so that a change + // to the core grid conversion is caught here rather than silently redefining + // what the read paths are compared against. Values come from a cast, active + // state from the source topology, and the two disagree on both tiles. + if constexpr (std::is_same_v) { + auto accessor = reference.getConstAccessor(); + EXPECT_TRUE(reference.background()); // bool(1.0f) + EXPECT_TRUE(accessor.getValue(Coord(4096))); // bool(0.1f) + EXPECT_FALSE(accessor.isValueOn(Coord(4096))); + EXPECT_FALSE(accessor.getValue(Coord(8192))); // bool(0.0f) + EXPECT_TRUE(accessor.isValueOn(Coord(8192))); + EXPECT_TRUE(accessor.getValue(Coord(0))); // bool(5.0f) + EXPECT_TRUE(accessor.isValueOn(Coord(0))); + } + + expectGridsMatch(reference, *readConverted, path); + } + + std::remove(offsetsPath.c_str()); + std::remove(noOffsetsPath.c_str()); +} + +// ReadMode::Mask does not match MaskGrid(readOriginal): a mask records active +// state rather than a cast value, so the reference here is a topology copy of +// the source rather than the in-memory conversion used for Bool and Half. +TEST_F(TestCodec, testMaskConversionMatchesInMemory) +{ + using namespace openvdb; + using namespace openvdb::io; - MaskGrid::Ptr readMask; + CodecRegistry::clear(); + io::internal::initialize(); + + const std::string gridName = "conversion_parity"; + + // Same source grid as the generic parity test: a non-zero background, an + // inactive non-zero tile and an active zero tile disagree between a value + // cast and a topology copy, which is the case worth pinning here. + FloatGrid::Ptr src = FloatGrid::create(/*background=*/1.0f); + src->setName(gridName); + src->tree().addTile(/*level=*/2, Coord(4096), 0.1f, /*active=*/false); + src->tree().addTile(/*level=*/2, Coord(8192), 0.0f, /*active=*/true); + src->fill(CoordBBox(Coord(0), Coord(6)), 5.0f, /*active=*/true); + + const std::string offsetsPath = "test_conversion_parity_offsets_mask.vdb"; + const std::string noOffsetsPath = "test_conversion_parity_no_offsets_mask.vdb"; { - io::File f(path); - f.open(); - readMask = gridPtrCast(f.readGrid(gridName, maskOpts)); - f.close(); + io::File f(offsetsPath); + f.write(GridPtrVec{src}); + } + { + std::ofstream os(noOffsetsPath, std::ios_base::out | std::ios_base::binary); + io::Stream(os).write(GridPtrVec{src}); } - ASSERT_TRUE(readMask); - // Only the active voxels are on. A value cast would have activated nothing - // extra, but it would have set the background and the inactive tile to true. - EXPECT_EQ(readMask->activeVoxelCount(), srcActiveVoxels); - EXPECT_TRUE(src->tree().hasSameTopology(readMask->tree())); - EXPECT_FALSE(readMask->background()); + ReadOptions maskOptions; + maskOptions.readMode = ReadMode::Mask; - // The inactive 0.1f tile must not have become an active or true tile. - EXPECT_FALSE(readMask->tree().getValue(Coord(4096))); + for (const std::string& path : {offsetsPath, noOffsetsPath}) { + FloatGrid::Ptr readOriginal; + MaskGrid::Ptr readConverted; + { + io::File f(path); + f.open(); + readOriginal = gridPtrCast(f.readGrid(gridName, ReadOptions{})); + readConverted = gridPtrCast(f.readGrid(gridName, maskOptions)); + f.close(); + } + ASSERT_TRUE(readOriginal); + ASSERT_TRUE(readConverted); + + // The reference is a topology copy of the source, not MaskGrid(readOriginal): + // value equals active state everywhere, background is false, and inactive + // tiles are preserved rather than dropped. + MaskGrid::Ptr reference = MaskGrid::create(static_cast(*readOriginal)); + reference->setTree(MaskGrid::TreeType::Ptr( + new MaskGrid::TreeType(readOriginal->constTree(), + /*inactiveValue=*/false, /*activeValue=*/true, TopologyCopy()))); + + auto accessor = reference->getConstAccessor(); + EXPECT_FALSE(reference->background()); + EXPECT_FALSE(accessor.getValue(Coord(4096))); + EXPECT_FALSE(accessor.isValueOn(Coord(4096))); + EXPECT_TRUE(accessor.getValue(Coord(8192))); + EXPECT_TRUE(accessor.isValueOn(Coord(8192))); + EXPECT_TRUE(accessor.getValue(Coord(0))); + EXPECT_TRUE(accessor.isValueOn(Coord(0))); + EXPECT_EQ(reference->activeVoxelCount(), readOriginal->activeVoxelCount()); + EXPECT_EQ(reference->constTree().activeTileCount(), readOriginal->constTree().activeTileCount()); + + // Value equals active state at every position for a mask, which is what + // the topology-copy reference buys over a plain value cast. + for (auto it = reference->cbeginValueAll(); it; ++it) { + EXPECT_EQ(*it, it.isValueOn()); + } - std::remove(path.c_str()); + expectGridsMatch(*reference, *readConverted, path); + } + + std::remove(offsetsPath.c_str()); + std::remove(noOffsetsPath.c_str()); +} + +TEST_F(TestCodec, testBoolConversionMatchesInMemory) +{ + testConversionMatchesInMemoryImpl("bool"); +} + +TEST_F(TestCodec, testHalfConversionMatchesInMemory) +{ + testConversionMatchesInMemoryImpl("half"); }