NanoVDB: fix Point grid size alignment and vector entry points (CUDA) - #2329
Open
swahtz wants to merge 1 commit into
Open
NanoVDB: fix Point grid size alignment and vector entry points (CUDA)#2329swahtz wants to merge 1 commit into
swahtz wants to merge 1 commit into
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, the vector entry points) started at a misaligned address, and device code reading that header faulted with a misaligned-address error. The blind data is now padded like blindDataSize, and the ThreePointsToGrid expectation follows. The std::vector<std::tuple> overloads of tools::cuda::voxelsToGrid and pointsToGrid, which build one grid per tuple and merge them, could not compile when instantiated: they called the deprecated mergeDeviceGrids with the stream in the pool-buffer slot, and the point form named one template argument too many. They now merge through cuda::mergeGridHandles with the pool buffer and stream in place; a static_assert names the dual-space-only limitation instead of an opaque overload failure. The pointsToGrid(points, count, voxelSize, type, ...) overload, declared and called by the deprecated cudaPointsToGrid but never defined, gets its definition. New test pointsToGridVectorOverloads merges two Voxel8 point grids through the vector entry point (the case that faulted), asserts the grid sizes are aligned, and exercises the voxel-set overload and the new single-set definition. 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
1 participant
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 sizing aPointgrid, asGridBlindMetaData::blindDataSizeand the other builders do. A payload that was not a multiple of 32 bytes (PointType::Voxel8/Voxel16, or most point counts withWorld32) leftmGridSizemisaligned, so the next grid in any multi-grid buffer built from it (mergeGrids,cuda::mergeGridHandles, the vector entry points) started at a misaligned address and device code reading its header faulted. TheThreePointsToGridsize expectation is updated accordingly.std::vector<std::tuple>overloads oftools::cuda::voxelsToGrid/pointsToGridcould not compile when instantiated (they called the deprecatedmergeDeviceGridswith the stream in the pool-buffer slot, and the point form passed one template argument too many). They now merge throughcuda::mergeGridHandles; astatic_assertnames the dual-space-only limitation. The declared-but-undefinedpointsToGrid(points, count, voxelSize, type, buffer, stream)overload, which the deprecatedcudaPointsToGridcalls, gets its definition.pendingchanges/nanovdb.txtrecords both fixes.Found while adding single-space
mergeGridHandles/splitGridHandlesoverloads (follow-up to #2232, requested in openvdb/fvdb-core#770); those land separately.Test plan
TestNanoVDBCUDA.pointsToGridVectorOverloads: merges twoVoxel8point grids through the vector entry point (the case that faulted), assertsgridSize % NANOVDB_DATA_ALIGNMENT == 0, and exercises the voxel-set overload and the new single-set definition.nanovdb_test_cuda66/66 andnanovdb_test_nanovdb156/156 pass locally (CUDA 13.2, sm_120,NANOVDB_CUDA_WERRORon).