Repository navigation
NanoVDB: pad Point grid blind data to the data alignment (CUDA) - #2329
Merged
swahtz merged 1 commit intoOct 20, 2026
Merged
Conversation
PointsToGrid sized a Point grid as the tree plus the raw point payload, with no padding. GridBlindMetaData::blindDataSize and every other builder round blind data up to NANOVDB_DATA_ALIGNMENT, so a payload that was not a multiple of 32 bytes (PointType::Voxel8 or Voxel16, or most point counts with World32) left mGridSize misaligned. The grid was fine on its own, but the next grid in any multi-grid buffer built from it (mergeGrids, cuda::mergeGridHandles) started at a misaligned address, and device code reading that header faulted. The blind data is now padded like blindDataSize, and the ThreePointsToGrid expectation follows. Point grids with such payloads grow by the padding, so their serialized bytes differ from files written before this change. New test mergePointGridsAligned builds two Voxel8 point grids, checks their sizes are aligned, merges them on the device and reads back the second grid. Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
swahtz
force-pushed
the
nanovdb-pointstogrid-fixes
branch
from
October 17, 2026 00:37
bfb8068 to
040bd7c
Compare
swahtz
changed the title
harrism
approved these changes
Oct 19, 2026
harrism
left a comment
harrism
left a comment
Contributor
There was a problem hiding this comment.
Simple fix, nice new test.
5 tasks done
swahtz
added a commit
to swahtz/openvdb
that referenced
this pull request
Oct 20, 2026
mergeGrids and splitGrids walk grids by the handle's metadata and throw when a grid header's size disagrees with it. Before, they sized from the metadata but advanced by mGridSize, so an adopted layout smaller than its headers overran the output buffer. splitGrids also builds its output with push_back, so a buffer over a non-default-constructible resource can use the pool path. cuda::copyTo reuses the source's layout through detail::LayoutCopy instead of adopt. The constructors accept grids at unaligned offsets (e.g. Point grids written before AcademySoftwareFoundation#2329), and copyTo no longer rejects them after the copy. The device GridHandle::copy() does the same. Public adopt keeps its alignment check and now also rejects GridType::End. Comments that overstated how much an adopted layout was validated are corrected, and merge/split document that they do not synchronize stream-ordered buffers. Tests: mergeSplitRejectMismatchedMetaData (heap corruption before this fix), CopyToKeepsUnalignedGrids, an End-type adopt case, and a ResourceRef pinned merge/split with a pool. Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
swahtz
added a commit
to swahtz/openvdb
that referenced
this pull request
Oct 20, 2026
mergeGrids and splitGrids walk grids by the handle's metadata and throw when a grid header's size disagrees with it. Before, they sized from the metadata but advanced by mGridSize, so an adopted layout smaller than its headers overran the output buffer. splitGrids also builds its output with push_back, so a buffer over a non-default-constructible resource can use the pool path. cuda::copyTo reuses the source's layout through detail::LayoutCopy instead of adopt. The constructors accept grids at unaligned offsets (e.g. Point grids written before AcademySoftwareFoundation#2329), and copyTo no longer rejects them after the copy. The device GridHandle::copy() does the same. Public adopt keeps its alignment check and now also rejects GridType::End. Comments that overstated how much an adopted layout was validated are corrected, and merge/split document that they do not synchronize stream-ordered buffers. Tests: mergeSplitRejectMismatchedMetaData (heap corruption before this fix), CopyToKeepsUnalignedGrids, an End-type adopt case, and a ResourceRef pinned merge/split with a pool. Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
harrism
added a commit
to harrism/openvdb
that referenced
this pull request
Oct 20, 2026
…eFoundation#2370) into nanovdb-resource-tool-api Both sides defined pointsToGrid(points, count, voxelSize, type, buffer, stream); keep this branch's definition, which adds the prototype-form constraint and the argument checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com>
swahtz
added a commit
to swahtz/openvdb
that referenced
this pull request
Oct 21, 2026
mergeGrids and splitGrids walk grids by the handle's metadata and throw when a grid header's size disagrees with it. Before, they sized from the metadata but advanced by mGridSize, so an adopted layout smaller than its headers overran the output buffer. splitGrids also builds its output with push_back, so a buffer over a non-default-constructible resource can use the pool path. cuda::copyTo reuses the source's layout through detail::LayoutCopy instead of adopt. The constructors accept grids at unaligned offsets (e.g. Point grids written before AcademySoftwareFoundation#2329), and copyTo no longer rejects them after the copy. The device GridHandle::copy() does the same. Public adopt keeps its alignment check and now also rejects GridType::End. Comments that overstated how much an adopted layout was validated are corrected, and merge/split document that they do not synchronize stream-ordered buffers. Tests: mergeSplitRejectMismatchedMetaData (heap corruption before this fix), CopyToKeepsUnalignedGrids, an End-type adopt case, and a ResourceRef pinned merge/split with a pool. Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
swahtz
added a commit
that referenced
this pull request
Oct 22, 2026
…2371) * NanoVDB: add GridHandle::adopt and metaData, deprecate HandleFactory GridHandle::adopt(buffer, meta) constructs a handle over bytes whose layout the caller already knows: a copy of another handle's grids, or grids the caller laid out itself. No grid header is read, so a device buffer is wrapped without the parse kernel, readback and synchronization the public constructor pays, and buffers over non-default-constructible resources need no default construction. adopt checks only the layout arithmetic: grids contiguous from offset zero, each starting at a multiple of NANOVDB_DATA_ALIGNMENT, each at least a header, all within the buffer, and an empty layout only over an empty buffer. GridHandle::metaData() exposes the layout to pass back in. cuda::copyTo and GridHandle::copy now use adopt. cuda::detail::HandleFactory, which downstream code (fvdb) has been reaching into for the same purpose, becomes a deprecated forwarder to adopt and metaData(). mergeGrids and splitGrids also accept host-accessible single-space handles such as GridHandle<cuda::Buffer<std::byte, PinnedResource>>, allocating through the pool's resource or a default-constructed one. New tests: GridHandleAdoptRejectsBadMeta covers each layout rejection and a byte-for-byte adopt round trip; mergeSplitPinnedGrids checks pinned merge and split against the HostBuffer result byte for byte. Signed-off-by: Jonathan Swartz <jonathan@jswartz.info> * NanoVDB: address review of GridHandle::adopt mergeGrids and splitGrids walk grids by the handle's metadata and throw when a grid header's size disagrees with it. Before, they sized from the metadata but advanced by mGridSize, so an adopted layout smaller than its headers overran the output buffer. splitGrids also builds its output with push_back, so a buffer over a non-default-constructible resource can use the pool path. cuda::copyTo reuses the source's layout through detail::LayoutCopy instead of adopt. The constructors accept grids at unaligned offsets (e.g. Point grids written before #2329), and copyTo no longer rejects them after the copy. The device GridHandle::copy() does the same. Public adopt keeps its alignment check and now also rejects GridType::End. Comments that overstated how much an adopted layout was validated are corrected, and merge/split document that they do not synchronize stream-ordered buffers. Tests: mergeSplitRejectMismatchedMetaData (heap corruption before this fix), CopyToKeepsUnalignedGrids, an End-type adopt case, and a ResourceRef pinned merge/split with a pool. Signed-off-by: Jonathan Swartz <jonathan@jswartz.info> * NanoVDB: note HandleFactory::make's new layout checks in pending changes Signed-off-by: Jonathan Swartz <jonathan@jswartz.info> * NanoVDB: switch cuda::mergeGridHandles/splitGridHandles to GridHandle::adopt The single-space merge and split from #2370 read layouts through GridHandle::metaData() and build their results with GridHandle::adopt instead of the deprecated HandleFactory. mergeGridHandles rejects a grid that would start at an unaligned offset before it allocates, since adopt would otherwise reject the merged layout only after the copies. Signed-off-by: Jonathan Swartz <jonathan@jswartz.info> * NanoVDB: close CopyToKeepsUnalignedGrids after the merge with master The merge resolution dropped the test's closing brace, so the next test was parsed inside its body. Signed-off-by: Jonathan Swartz <jonathan@jswartz.info> * NanoVDB: build cuda::copyTo's empty prototype handle through GridHandle::adopt The empty-source path that came in from master called the deprecated HandleFactory, which fails builds with warnings as errors. Signed-off-by: Jonathan Swartz <jonathan@jswartz.info> --------- Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tools::cuda::PointsToGridnow pads the point blind data toNANOVDB_DATA_ALIGNMENTwhen it sizes aPointgrid, asGridBlindMetaData::blindDataSizeand the other builders already do.Before this fix, a payload that was not a multiple of 32 bytes left
mGridSizemisaligned. That coversPointType::Voxel8andVoxel16, and most point counts withWorld32. The grid worked on its own. But the next grid in any multi-grid buffer built from it (mergeGrids,cuda::mergeGridHandles) started at a misaligned address, and device code that read its header faulted. TheThreePointsToGridsize expectation is updated to match.Behavior change: Point grids with unaligned payloads are now larger by the padding. Their serialized
.nvdbbytes differ from files written before this fix.File compatibility: Compatibility holds in both directions.
mGridSize, locates blind data throughmDataOffset(which does not move), and takes the point count frommValueCount. The padding is trailing bytes after the blind data, and the CPU builders andGridBlindMetaData::blindDataSizealready produce this padded layout.This PR originally also repaired the
std::vector<std::tuple>overloads ofvoxelsToGrid/pointsToGridand added the missingpointsToGrid(voxelSize)definition. #2370 now fixes both for single-space and dual-space buffers, so those changes were dropped from this PR, and it is rebased on current master.Test plan
TestNanoVDBCUDA.mergePointGridsAlignedbuilds twoVoxel8point grids, asserts that their sizes are aligned, merges them on the device, and reads back the second grid.nanovdb_test_cuda66/66 andnanovdb_test_nanovdb156/156 pass locally (CUDA 13.2, sm_120,NANOVDB_CUDA_WERRORon).