Merge pull request #2230 from danrbailey/fix_io_bugs

Fix I/O bugs

Author: danrbailey

ci/install_windows.ps1

@@ -21,20 +21,52 @@ $vcpkgPackages = @(
     "nanobind"
 )
 
+$maxAttempts = 3
+
+# curl's schannel backend reports an unreachable CRL/OCSP responder as a
+# certificate verification failure (error 60), which vcpkg then treats as
+# permanent. Downgrade a missing revocation answer to a warning; the rest of
+# certificate validation still applies.
+$env:VCPKG_SSL_REVOKE_BEST_EFFORT = "1"
+
 # Update vcpkg
 vcpkg update
 
-# Allow the vcpkg command to fail once so we can retry with the latest
-try {
-    vcpkg install $vcpkgPackages
-} catch {
-    Write-Host "vcpkg install failed, retrying with latest ports..."
-    # Retry the installation with updated ports
-    Push-Location $env:VCPKG_INSTALLATION_ROOT
-    git pull
-    Pop-Location
-    vcpkg update
+$installed = $false
+
+for ($attempt = 1; $attempt -le $maxAttempts; $attempt++) {
     vcpkg install $vcpkgPackages
+
+    # A failing native command does not raise a terminating error, so the exit
+    # code has to be inspected explicitly rather than relying on try/catch.
+    if ($LASTEXITCODE -eq 0) {
+        $installed = $true
+        break
+    }
+
+    if ($attempt -eq $maxAttempts) {
+        break
+    }
+
+    # vcpkg fetches port sources directly from upstream hosts and won't retry
+    # downloads it classifies as permanent failures, so a single flaky TLS
+    # handshake aborts the whole install.
+    Write-Host "vcpkg install failed (attempt $attempt of $maxAttempts), retrying..."
+    Start-Sleep -Seconds 15
+
+    # Refresh the ports before the last attempt in case the failure is caused
+    # by a stale port rather than the network.
+    if ($attempt -eq ($maxAttempts - 1)) {
+        Write-Host "Retrying with latest ports..."
+        Push-Location $env:VCPKG_INSTALLATION_ROOT
+        git pull
+        Pop-Location
+        vcpkg update
+    }
+}
+
+if (-not $installed) {
+    throw "vcpkg install failed after $maxAttempts attempts"
 }
 
 Write-Host "vcpkg install completed successfully"

doc/changes.txt

@@ -2034,7 +2034,7 @@ Bug fixes:
 New features:
 - Added @vdblink{tools::FindActiveValues,FindActiveValues}, which counts
   the active values in a tree that intersect a given bounding box.
-- Added @vdblink{io::DelayedLoadMetadata,DelayedLoadMetadata}, which stores
+- Added @c io::DelayedLoadMetadata, which stores
   mask offsets and compression sizes on write to accelerate delayed load
   reading.
 
@@ -2933,7 +2933,7 @@ New features:
 - Added a toggle to the @vdblink::tools::clip() clip@endlink tool
   to invert the clipping mask.
 - Custom leaf node implementations may now optimize their file layout
-  by inheriting from @vdblink::io::MultiPass io::MultiPass@endlink.
+  by inheriting from @c io::MultiPass.
   Voxel data for grids with such leaf nodes will be written and read in
   multiple passes, allowing blocks of related data to be stored contiguously.
   <I>[Contributed&nbsp;by&nbsp;Double&nbsp;Negative]</I>

openvdb/openvdb/Grid.h

@@ -9,6 +9,7 @@
 #include "Types.h"
 #include "io/io.h"
 #include "math/Transform.h"
+#include "tree/LeafManager.h"
 #include "tree/Tree.h"
 #include "util/Assert.h"
 #include "util/logging.h"
@@ -1632,6 +1633,28 @@ inline void
 Grid<TreeT>::readTopology(std::istream& is)
 {
     tree().readTopology(is, saveFloatAsHalf());
+    // When called from the legacy (non-codec) TopologyOnly path, the stream
+    // metadata carries a flag requesting that leaf buffers be allocated and
+    // filled with the background value (PartialCreate leaves them
+    // unallocated after readTopology).
+    if (io::StreamMetadata::Ptr meta = io::getStreamMetadataPtr(is)) {
+        if (meta->allocateLeafBuffers()) {
+            meta->setAllocateLeafBuffers(false);
+            if constexpr (!std::is_void_v<typename TreeT::LeafNodeType>) {
+                const auto background = tree().root().background();
+                tree::LeafManager<TreeT> leafManager(tree());
+                leafManager.foreach([&background](auto& leaf, size_t) {
+                    using LeafType = std::decay_t<decltype(leaf)>;
+                    if constexpr (!std::is_same_v<typename LeafType::ValueType, bool>) {
+                        if (leaf.buffer().empty()) {
+                            leaf.buffer().allocate();
+                            leaf.buffer().fill(background);
+                        }
+                    }
+                });
+            }
+        }
+    }
 }
 
 

openvdb/openvdb/codecs/BoolCodec.h

@@ -110,7 +110,7 @@ struct BoolCodec final: public TopologyCodec<GridT>
 
     static inline std::string name() { return GridT::gridType(); }
 
-    void readBuffers(std::istream& is, io::CodecData& data, const io::ReadOptions& options, io::ReadDiagnostics&) final
+    void readBuffers(std::istream& is, Index64 /*size*/, io::CodecData& data, const io::ReadOptions& options, io::ReadDiagnostics&) final
     {
         GridT& grid = static_cast<GridT&>(*data.grid);
 

openvdb/openvdb/codecs/PointDataCodec.h

@@ -111,13 +111,14 @@ template <typename LeafT>
 inline void readPointDataVoxelData(const std::vector<LeafT*>& leaves,
     std::istream& is, bool saveFloatAsHalf,
     const typename LeafT::ValueType& background,
-    [[maybe_unused]] const std::unordered_map<Coord, uint16_t>& voxelBufferSizes)
+    [[maybe_unused]] const std::unordered_map<Coord, uint16_t>& voxelBufferSizes,
+    const typename LeafT::ValueType* storageBackground = nullptr)
 {
     using BaseLeaf = typename LeafT::BaseLeaf;
     for (auto* leaf : leaves) {
         OPENVDB_ASSERT(voxelBufferSizes.find(leaf->origin()) != voxelBufferSizes.end());
         BaseLeaf& baseLeaf = static_cast<BaseLeaf&>(*leaf);
-        readScalarLeafBuffers(baseLeaf, is, saveFloatAsHalf, background);
+        readScalarLeafBuffers(baseLeaf, is, saveFloatAsHalf, background, /*skip=*/false, /*clipBBox=*/nullptr, storageBackground);
     }
 }
 
@@ -310,7 +311,7 @@ struct PointDataCodec final: public TopologyCodec<GridT>
 
     static inline std::string name() { return GridT::gridType(); }
 
-    void readBuffers(std::istream& is, io::CodecData& data, const io::ReadOptions& options, io::ReadDiagnostics&) final
+    void readBuffers(std::istream& is, Index64 /*size*/, io::CodecData& data, const io::ReadOptions& options, io::ReadDiagnostics&) final
     {
         OPENVDB_ASSERT(dynamic_cast<GridT*>(data.grid.get()));
 
@@ -334,7 +335,11 @@ struct PointDataCodec final: public TopologyCodec<GridT>
 
         uint16_t numPasses = 1;
         is.read(reinterpret_cast<char*>(&numPasses), sizeof(uint16_t));
-        const Index attributes = (numPasses - 4) / 2;
+        // The pass layout is: voxel sizes (1) + descriptors (1) + attribute
+        // sizes (N) + voxel data (1) + attribute data (N) = 2N + 4 passes.
+        // A leafless grid stores numPasses == 0, and malformed files may store
+        // numPasses < 4; guard against unsigned underflow in either case.
+        const Index attributes = numPasses >= 4 ? Index(numPasses - 4) / 2 : 0;
 
         using LeafT = typename GridT::TreeType::LeafNodeType;
         std::vector<LeafT*> leaves;
@@ -351,7 +356,17 @@ struct PointDataCodec final: public TopologyCodec<GridT>
         // An empty pointAttributeNames means no filtering (read all attributes).
         std::set<Index> skipIndices;
         if (!pointAttributeNames.empty() && !leaves.empty()) {
-            const auto& nameMap = leaves[0]->attributeSet().descriptor().map();
+            // Attribute filtering requires homogeneous descriptors across all
+            // leaves because skip decisions are made per-index across all leaves.
+            const auto* firstDesc = &leaves[0]->attributeSet().descriptor();
+            for (size_t i = 1; i < leaves.size(); ++i) {
+                if (&leaves[i]->attributeSet().descriptor() != firstDesc) {
+                    OPENVDB_THROW(IoError,
+                        "Attribute filtering is not supported for PointDataGrids "
+                        "with heterogeneous descriptors");
+                }
+            }
+            const auto& nameMap = firstDesc->map();
             const std::set<std::string> wantedNames(
                 pointAttributeNames.begin(),
                 pointAttributeNames.end());
@@ -373,8 +388,10 @@ struct PointDataCodec final: public TopologyCodec<GridT>
         }
 
         // Pass N+2: read voxel data
+        using ValueT = typename GridT::TreeType::ValueType;
+        auto& topoData = static_cast<TopologyCodecData<ValueT>&>(data);
         internal::readPointDataVoxelData(leaves, is, saveFloatAsHalf,
-            tree.background(), voxelBufferSizes);
+            tree.background(), voxelBufferSizes, &topoData.storageBackground);
 
         // Passes N+3..2N+2: read attribute data buffers
         for (Index i = 0; i < attributes; ++i) {
@@ -419,7 +436,9 @@ struct PointDataCodec final: public TopologyCodec<GridT>
             static_cast<uint16_t>(internal::countPointDataPasses(leaves));
         os.write(reinterpret_cast<const char*>(&numPasses), sizeof(uint16_t));
 
-        const Index attributes = (numPasses - 4) / 2;
+        // See readBuffers(): a leafless grid yields numPasses == 0, so guard
+        // against unsigned underflow rather than computing (numPasses - 4) / 2.
+        const Index attributes = numPasses >= 4 ? Index(numPasses - 4) / 2 : 0;
 
         // Pass 0: write voxel data sizes + descriptor tracking
         bool matching = true;

openvdb/openvdb/codecs/PointIndexCodec.h

@@ -26,10 +26,11 @@ struct ReadPointIndexBuffersOp
     using ValueT = typename TreeT::ValueType;
 
     ReadPointIndexBuffersOp(std::istream& _is, bool _saveFloatAsHalf,
-        const ValueT& _background)
+        const ValueT& _background, const ValueT* _storageBackground = nullptr)
         : is(_is)
         , saveFloatAsHalf(_saveFloatAsHalf)
-        , background(_background) { }
+        , background(_background)
+        , storageBackground(_storageBackground) { }
 
     template <typename NodeT>
     void operator()(NodeT&, size_t) { }
@@ -40,7 +41,7 @@ struct ReadPointIndexBuffersOp
 
         // Read the value mask and voxel data via base class
         BaseLeaf& baseLeaf = static_cast<BaseLeaf&>(leaf);
-        readScalarLeafBuffers(baseLeaf, is, saveFloatAsHalf, background, /*skip=*/false, /*clipBBox=*/nullptr);
+        readScalarLeafBuffers(baseLeaf, is, saveFloatAsHalf, background, /*skip=*/false, /*clipBBox=*/nullptr, storageBackground);
 
         // Read the number of indices.
         Index64 numIndices = Index64(0);
@@ -63,6 +64,7 @@ struct ReadPointIndexBuffersOp
     std::istream& is;
     const bool saveFloatAsHalf;
     const ValueT& background;
+    const ValueT* storageBackground = nullptr;
 }; // struct ReadPointIndexBuffersOp
 
 template <typename GridT>
@@ -114,7 +116,7 @@ struct PointIndexCodec final: public TopologyCodec<GridT>
 
     static inline std::string name() { return GridT::gridType(); }
 
-    void readBuffers(std::istream& is, io::CodecData& data, const io::ReadOptions& options, io::ReadDiagnostics& diagnostics) final
+    void readBuffers(std::istream& is, Index64 /*size*/, io::CodecData& data, const io::ReadOptions& options, io::ReadDiagnostics& diagnostics) final
     {
         OPENVDB_ASSERT(dynamic_cast<GridT*>(data.grid.get()));
 
@@ -135,7 +137,9 @@ struct PointIndexCodec final: public TopologyCodec<GridT>
             diagnostics.addWarning(grid.getName(), "bounding box clipping is not supported for PointIndexGrids");
         }
 
-        internal::ReadPointIndexBuffersOp<GridT> readBuffersOp(is, saveFloatAsHalf, tree.background());
+        using ValueT = typename GridT::TreeType::ValueType;
+        auto& topoData = static_cast<TopologyCodecData<ValueT>&>(data);
+        internal::ReadPointIndexBuffersOp<GridT> readBuffersOp(is, saveFloatAsHalf, tree.background(), &topoData.storageBackground);
         tools::visitNodesDepthFirst(grid.tree(), readBuffersOp, /*idx=*/0, /*topDown=*/false);
     }
 

openvdb/openvdb/codecs/ScalarCodec.h

@@ -23,21 +23,26 @@ template <typename TreeT>
 struct WriteBuffersOp
 {
     using LeafT = typename TreeT::LeafNodeType;
+    using ValueT = typename TreeT::ValueType;
 
-    WriteBuffersOp(std::ostream& _os, bool _saveFloatAsHalf)
+    WriteBuffersOp(std::ostream& _os, bool _saveFloatAsHalf, const ValueT& _background)
         : os(_os)
-        , saveFloatAsHalf(_saveFloatAsHalf) { }
+        , saveFloatAsHalf(_saveFloatAsHalf)
+        , background(_background) { }
 
     template <typename NodeT>
     void operator()(const NodeT&, size_t) { }
 
     void operator()(const LeafT& leaf, size_t)
     {
-        writeScalarLeafBuffers(leaf, os, saveFloatAsHalf);
+        // Pass the background explicitly so leaf compression does not depend on
+        // the stream's background pointer (which the codec path no longer sets).
+        writeScalarLeafBuffers(leaf, os, saveFloatAsHalf, &background);
     }
 
     std::ostream& os;
     const bool saveFloatAsHalf;
+    const ValueT& background;
 }; // struct WriteBuffersOp
 
 
@@ -48,13 +53,15 @@ struct ReadBuffersOp
     using LeafT = typename TreeT::LeafNodeType;
     using ValueT = typename TreeT::ValueType;
     using StorageLeafT = typename StorageTreeT::LeafNodeType;
+    using StorageValueT = typename StorageTreeT::ValueType;
 
     ReadBuffersOp(std::istream& _is, bool _saveFloatAsHalf, const ValueT& _background,
-        const CoordBBox* _clipBBox)
+        const CoordBBox* _clipBBox, const StorageValueT* _storageBackground = nullptr)
         : is(_is)
         , saveFloatAsHalf(_saveFloatAsHalf)
         , background(_background)
-        , clipBBox(_clipBBox) { }
+        , clipBBox(_clipBBox)
+        , storageBackground(_storageBackground) { }
 
     void operator()(RootT& root, size_t)
     {
@@ -73,20 +80,22 @@ struct ReadBuffersOp
 
     void operator()(LeafT& leaf, size_t)
     {
-        readScalarLeafBuffers<LeafT, StorageLeafT>(leaf, is, saveFloatAsHalf, background, /*skip=*/false, clipBBox);
+        readScalarLeafBuffers<LeafT, StorageLeafT>(leaf, is, saveFloatAsHalf, background, /*skip=*/false, clipBBox, storageBackground);
     }
 
     std::istream& is;
     const bool saveFloatAsHalf;
     const ValueT& background;
     const CoordBBox* clipBBox = nullptr;
+    const StorageValueT* storageBackground = nullptr;
 }; // struct ReadBuffersOp
 
 
 // Free-standing function for both standard and conversion codec cases
 // Uses StorageGridT = GridT by default, but allows different storage type for conversions
 template<typename GridT, typename StorageGridT = GridT>
-void scalarCodecReadBuffers(GridT& grid, std::istream& is, const io::ReadOptions& options)
+void scalarCodecReadBuffers(GridT& grid, std::istream& is, const io::ReadOptions& options,
+    const typename StorageGridT::TreeType::ValueType* storageBackground)
 {
     if (grid.hasMultiPassIO()) {
         OPENVDB_THROW(IoError, "Multi-pass IO is not supported in ScalarCodec");
@@ -109,7 +118,7 @@ void scalarCodecReadBuffers(GridT& grid, std::istream& is, const io::ReadOptions
 
     // Works for both standard (TreeT == StorageTreeT) and conversion cases
     ReadBuffersOp<TreeT, StorageTreeT> readBuffersOp(is, saveFloatAsHalf, tree.background(),
-        clipIndexBBox.get());
+        clipIndexBBox.get(), storageBackground);
     tools::visitNodesDepthFirst(grid.tree(), readBuffersOp, /*idx=*/0, /*topDown=*/false);
 }
 
@@ -123,7 +132,7 @@ void scalarCodecWriteBuffers(const GridT& grid, std::ostream& os)
         OPENVDB_THROW(IoError, "Multi-pass IO is not supported in ScalarCodec");
     }
 
-    WriteBuffersOp<TreeType> writeBuffersOp(os, grid.saveFloatAsHalf());
+    WriteBuffersOp<TreeType> writeBuffersOp(os, grid.saveFloatAsHalf(), grid.tree().background());
     tools::visitNodesDepthFirst(grid.tree(), writeBuffersOp);
 }
 
@@ -150,18 +159,25 @@ struct ScalarCodec final: public TopologyCodec<GridT, StorageGridT, Mode>
         }
     }
 
-    void readBuffers(std::istream& is, io::CodecData& data, const io::ReadOptions& options, io::ReadDiagnostics&) final
+    void readBuffers(std::istream& is, Index64 /*size*/, io::CodecData& data, const io::ReadOptions& options, io::ReadDiagnostics&) final
     {
+        using StorageValueT = typename StorageGridT::TreeType::ValueType;
         GridT& grid = static_cast<GridT&>(*data.grid);
-        internal::scalarCodecReadBuffers<GridT, StorageGridT>(grid, is, options);
+        auto& topoData = static_cast<TopologyCodecData<StorageValueT>&>(data);
+        internal::scalarCodecReadBuffers<GridT, StorageGridT>(grid, is, options, &topoData.storageBackground);
     }
 
     void writeBuffers(std::ostream& os, const GridBase& gridBase, const io::WriteOptions&) final
     {
-        if constexpr (Mode == io::CodecMode::ReadOnly) return;
-
-        const GridT& grid = static_cast<const GridT&>(gridBase);
-        internal::scalarCodecWriteBuffers(grid, os);
+        // Note: the write body must live inside the negated if constexpr branch
+        // so it is not instantiated for read-only codecs. A bare
+        // `if constexpr (Mode == ReadOnly) return;` would still instantiate the
+        // code that follows, which fails to compile for the scalar-to-mask/bool
+        // convert codecs (their leaf buffers expose WordType*, not ValueType*).
+        if constexpr (Mode != io::CodecMode::ReadOnly) {
+            const GridT& grid = static_cast<const GridT&>(gridBase);
+            internal::scalarCodecWriteBuffers(grid, os);
+        }
     }
 }; // struct ScalarCodec