Skip to content

Codec Framework and Delayed Loading Removal - #2281

Open
danrbailey wants to merge 96 commits into
masterfrom
feature/io
Open

Codec Framework and Delayed Loading Removal#2281
danrbailey wants to merge 96 commits into
masterfrom
feature/io

Conversation

@danrbailey

Copy link
Copy Markdown
Contributor

This introduces the new codec framework and completely removes delayed-loading / out-of-core into the master branch.

All of the components included in this PR have been reviewed independently:

#2155
#2164
#2166
#2179
#2180
#2184
#2185
#2197
#2196
#2230

These changes have been tested extensively at ILM.

For completeness, here are the new additions to the changelog on this branch from aggregating all the pending changes:

OpenVDB:
    Highlights:
    - Completely removed support for out-of-core / delayed loading across OpenVDB
      core, including removal of the OPENVDB_USE_DELAYED_LOADING build option,
      DelayedLoadMetadata, and TempFile. This was done to improve performance,
      reduce codebase complexity and to entirely eliminate the optional Boost
      dependency.
    New features:
    - Introduced new openvdb::codecs subsystem and CodecRegistry for stream
      encoding and decoding.
    - Added support for implicit codec conversion fallback (e.g. float to half,
      scalar to mask) and ReadDiagnostics.
    API changes:
    - Extracted Point Data I/O function overloads from PointDataGrid.h into new
      openvdb/points/PointDataIO.h header.
    - GridDescriptor::read() has been deprecated, use GridDescriptor::readHeader()
      followed by GridDescriptor::readStreamPos() instead.
    - Added optional trailing WriteOptions parameter to io::File::write() and
      io::Stream::write(). No changes in behavior.
    - Added optional trailing ReadOptions parameter to io::File::getGrids() and
      io::File::readGrid(). No changes in behavior.
    - TreeBase::readTopology() and TreeBase::writeTopology() are now pure virtual.
      Derived classes of TreeBase now need to read and write int32_t(1) to remain
      backwards-compatible.
    Improvements:
    - Large refactor of I/O classes, many private and protected member functions have
      been modified. No change in behavior for io::File and io::Stream.
    - Remove direct testing of Tree I/O methods in favor of indirect testing using
      the Grid I/O.
    Houdini:
    - Add new Vulkan viewport support for VDB Points primitives and remove support
      for delayed loading.

danrbailey and others added 30 commits January 30, 2026 23:42
…by it

Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Remove unused class members and methods in future ABI=14
Signed-off-by: Dan Bailey <danbailey@ilm.com>
…f I/O methods

Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
danrbailey and others added 16 commits June 12, 2026 10:51
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
…nstead of StorageValueT causing inactive values to be read incorrectly

Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
…mance

Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
// Cleanup
std::remove(floatPath.c_str());
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex found an edge case of failing to read float as half when a file doesn't have a grid offsets

TEST_F(TestCodec, testReadModeHalfAppliesToFileWithoutGridOffsets)
{
    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>());
        f.close();
    }

    {
        io::File f(path);
        f.open();
        GridBase::Ptr grid = f.readGrid(srcGrid->getName(), readOptions);
        ASSERT_TRUE(grid);
        EXPECT_TRUE(grid->isType<HalfGrid>());
        f.close();
    }

    std::remove(path.c_str());
}

The problem is io::Stream(os).write(...) creates a VDB file without grid offsets. For that kind of file, File::open() reads the grids using default options before the caller passes ReadOptions. So later calls to getGrids(readOptions) or readGrid(name, readOptions) return the cached original FloatGrid, not a converted HalfGrid.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Andre, thanks for testing. This is true and I think it's possibly also an issue for instance VDBs too. We currently only offer on-the-fly conversion, as in the data is converted between types while it is being read. The caching approach used for non grid offset VDBs doesn't align with that.

I don't think it's a good idea to add the ReadOptions to the File::open() method, but there are a few of other options to discuss:

  • We just add a ReadDiagnostics warning that documents that on-the-fly grid conversion is not supported for non grid offset VDBs. I don't think it deserves to be an exception, but maybe we should consider that too.
  • We do in-memory grid-to-grid conversion - have you explored that for float->half grids and is it a feature that is currently supported?
  • We remove on-the-fly grid conversion altogether and support it properly for the new file format only. I don't intend to support writing non grid offset VDBs in the new file layouts (except for writing legacy files).

I do agree that quietly ignoring the hint and just returning the float grid is unexpected and should be resolved, so I'll have a think about which of these options might be most viable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opted to do the in-memory grid conversion to resolve this issue. See #2298

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing this issue this way.

bool mDelayedLoadMeta = false;
uint64_t mLeaf = 0;
uint32_t mTest = 0; // for testing only
bool mAllocateLeafBuffers = false;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be blocking because of ABI incompatibilities between 13.0 and 13. 1.

StreamMetadata is shared between coexisting OpenVDB versions and is part of the protected Grid ABI. A 13.1 reader can access or copy this field from an Impl created by 13.0, where this bool does not exist or was never initialized. Tail padding does not make that access safe. Could this flag use existing compatible storage such as AuxDataMap, or be restricted to ABI 14?

For example, setAllocateLeafBuffers(true) could store true under a private key in auxData(). allocateLeafBuffers() could look up that key with std::any_cast<bool>, and clearing the flag could erase it. This uses storage that already exists and is initialized in ABI 13, so no new Impl member is needed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From TSC meeting 8/26/2026 this is okay, because ABI changes is applied only for Tree and Grid contracts say that we want to be able to reinterpret_cast the pointer. In the discussion, it was mentioned that it's for the old grid reading workflow. It is used to be read during a topology-only read. So it seems okay.

For completeness, I'll also include a discussion all the way back from 2019: https://github.com/AcademySoftwareFoundation/openvdb/blob/master/tsc/meetings/2019-09-26.md?plain=1#L27-L45. So there may be a compatibility issue with Houdini.

@apradhana

Copy link
Copy Markdown
Contributor

This is mainly a comment regarding the removal of PImpl and not a blocking issue. I just want us all to be aware that removing PImpl reduces ABI flexibility. With PImpl, private implementation members can be changed without changing the public layout of File or Stream. For example, a new version could add more fields to the hidden implementation while the public object remains just a pointer.

Without PImpl, those private members become part of the public object’s binary layout. Future internal changes may therefore require applications and plugins to be rebuilt. Mixing versions also becomes unsafe: old headers might allocate 72 bytes for File, while the new library expects 296 bytes, potentially causing crashes or memory corruption.

Again, this is non-blocking.

Comment thread openvdb/openvdb/io/File.h
Comment thread openvdb/openvdb/io/Stream.h
@danrbailey

Copy link
Copy Markdown
Contributor Author

This is mainly a comment regarding the removal of PImpl and not a blocking issue. I just want us all to be aware that removing PImpl reduces ABI flexibility. With PImpl, private implementation members can be changed without changing the public layout of File or Stream. For example, a new version could add more fields to the hidden implementation while the public object remains just a pointer.

Without PImpl, those private members become part of the public object’s binary layout. Future internal changes may therefore require applications and plugins to be rebuilt. Mixing versions also becomes unsafe: old headers might allocate 72 bytes for File, while the new library expects 296 bytes, potentially causing crashes or memory corruption.

Again, this is non-blocking.

This is sort of a general comment about why pimpl might be useful. If we remove delayed loading, then the StreamMetadata is no longer part of the Grid ABI because you cannot pass around a partially created grid. Using the codec framework improves flexibility and does away with all the complexity of managing and maintaining StreamMetadata objects. That is the overall intention here.

@apradhana

Copy link
Copy Markdown
Contributor

This is mainly a comment regarding the removal of PImpl and not a blocking issue. I just want us all to be aware that removing PImpl reduces ABI flexibility. With PImpl, private implementation members can be changed without changing the public layout of File or Stream. For example, a new version could add more fields to the hidden implementation while the public object remains just a pointer.
Without PImpl, those private members become part of the public object’s binary layout. Future internal changes may therefore require applications and plugins to be rebuilt. Mixing versions also becomes unsafe: old headers might allocate 72 bytes for File, while the new library expects 296 bytes, potentially causing crashes or memory corruption.
Again, this is non-blocking.

This is sort of a general comment about why pimpl might be useful. If we remove delayed loading, then the StreamMetadata is no longer part of the Grid ABI because you cannot pass around a partially created grid. Using the codec framework improves flexibility and does away with all the complexity of managing and maintaining StreamMetadata objects. That is the overall intention here.

Thanks for clarifying this and for the discussion during TSC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants