-
Notifications
You must be signed in to change notification settings - Fork 17
Gaussian Projection with the Unscented Transform #420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This commit implements the projection of 3D Gaussians to 2D screen space using Unscented Transform. It includes the necessary header and source files, as well as a test suite to validate the functionality. The CMake configuration is updated to include the new test. - Added GaussianProjectionUT.h and GaussianProjectionUT.cu for the projection logic. - Implemented unit tests in GaussianProjectionUTTest.cpp to verify the projection accuracy. - Updated CMakeLists.txt to include the new test. Signed-off-by: Francis Williams <francis@fwilliams.info>
|
What issues does this close? |
This commit introduces a new function, `rotationMatrixToQuaternion`, which converts a 3x3 rotation matrix to its equivalent quaternion representation. The function ensures normalization and handles degenerate inputs by falling back to the identity quaternion. Additionally, the CMake configuration is updated to include a new test for this functionality. - Implemented `rotationMatrixToQuaternion` in GaussianUtils.cuh. - Updated CMakeLists.txt to include GaussianUtilsTest for testing the new function. Signed-off-by: [Your Name] <your.email@example.com> Signed-off-by: Francis Williams <francis@fwilliams.info>
This commit introduces a new `Pose` structure to represent a rigid pose with a rotation quaternion and translation vector. It also implements the `interpolatePose` function, which linearly interpolates between two camera poses, combining translation and quaternion rotation using normalized linear interpolation (NLERP). Additionally, new test cases are added to validate the pose interpolation functionality in `GaussianUtilsTest.cu`. - Added `Pose` structure in GaussianUtils.cuh. - Implemented `interpolatePose` function for pose interpolation. - Updated `GaussianUtilsTest.cu` with tests for pose interpolation. Signed-off-by: [Your Name] <your.email@example.com> Signed-off-by: Francis Williams <francis@fwilliams.info>
This commit cleans up the GaussianProjectionUTTest.cpp file by removing the unnecessary inclusion of the Tensor.h header file, streamlining the code and improving compilation efficiency. Signed-off-by: [Your Name] <your.email@example.com> Signed-off-by: Francis Williams <francis@fwilliams.info>
Signed-off-by: Francis Williams <francis@fwilliams.info>
Signed-off-by: Francis Williams <francis@fwilliams.info>
Signed-off-by: Francis Williams <francis@fwilliams.info>
Signed-off-by: Francis Williams <francis@fwilliams.info>
Signed-off-by: Francis Williams <francis@fwilliams.info>
Signed-off-by: Francis Williams <francis@fwilliams.info>
Signed-off-by: Francis Williams <francis@fwilliams.info>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Implements 3D Gaussian → 2D screen-space projection using an Unscented Transform (UT), including OpenCV-style distortion support and a new test suite.
Changes:
- Added a UT-based CUDA projection path (
GaussianProjectionUT.*) with OpenCV distortion variants and rolling-shutter handling. - Extended gsplat math utilities with rotation-matrix↔quaternion conversion and pose interpolation helpers.
- Added unit tests for the UT projection and new Gaussian math utilities; wired them into the test CMake.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/fvdb/detail/ops/gsplat/GaussianProjectionUT.h | Declares UT projection API, distortion models, and UT parameters. |
| src/fvdb/detail/ops/gsplat/GaussianProjectionUT.cu | Implements UT projection kernel, distortion application, sigma point generation, and CUDA dispatch. |
| src/fvdb/detail/ops/gsplat/GaussianUtils.cuh | Adds quaternion/pose utilities used by UT projection and tests. |
| src/fvdb/detail/utils/AccessorHelpers.cuh | Adjusts tensor accessor includes/types for C++/CUDA compatibility. |
| src/tests/GaussianProjectionUTTest.cpp | Adds UT projection correctness tests (pinhole + OpenCV distortion cases). |
| src/tests/GaussianUtilsTest.cu | Adds tests for new quaternion/pose math helpers. |
| src/tests/CMakeLists.txt | Registers the new UT and utils tests. |
| src/fvdb/detail/ops/gsplat/GaussianProjectionForward.cu | Minor constructor cleanup (init-list only). |
| src/fvdb/detail/GridBatchImpl.cu | Adds casts to avoid size/type mismatches in checks. |
| src/CMakeLists.txt | Adds the new UT CUDA source file to the build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Francis Williams <francis@fwilliams.info>
Signed-off-by: Francis Williams <francis@fwilliams.info>
Signed-off-by: Francis Williams <francis@fwilliams.info>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/fvdb/detail/GridBatchImpl.cu:1344
GridBatchImpl::deserializeV0trusts thenumGridsandtotalBytesfields from the serialized header without fully validating that the metadata region fits inside the provided tensor, which can lead to out-of-bounds reads on attacker-controlled input. IfnumGridsis large ortotalBytesis inconsistent with the implied metadata size, thegridMetadata[i]loop and subsequentmemcpy(buf.data(), gridBuffer, sizeofGrid)can read past the end of theserializedbuffer and copy adjacent process memory into the grid handle, potentially exposing sensitive data when the grid is later re-serialized or inspected. To fix this, add strict bounds checks thatserialized.numel()(andheader->totalBytes) are at leastsizeof(V01Header) + sizeof(GridBatchMetadata) + numGrids * sizeof(GridMetadata)before accessingbatchMetadata,gridMetadata[i], or computingsizeofGrid, and reject inputs where these invariants do not hold.
TORCH_CHECK(serialized.scalar_type() == torch::kInt8, "Serialized data must be of type int8");
TORCH_CHECK(serialized.numel() >= static_cast<int64_t>(sizeof(V01Header)),
"Serialized data is too small to be a valid grid handle");
const int8_t *serializedPtr = serialized.data_ptr<int8_t>();
const V01Header *header = reinterpret_cast<const V01Header *>(serializedPtr);
TORCH_CHECK(header->magic == 0x0F0F0F0F0F0F0F0F,
"Serialized data is not a valid grid handle. Bad magic.");
TORCH_CHECK(header->version == 0, "Serialized data is not a valid grid handle. Bad version.");
TORCH_CHECK(static_cast<uint64_t>(serialized.numel()) == header->totalBytes,
"Serialized data is not a valid grid handle. Bad total bytes.");
const uint64_t numGrids = header->numGrids;
const GridBatchMetadata *batchMetadata =
reinterpret_cast<const GridBatchMetadata *>(serializedPtr + sizeof(V01Header));
TORCH_CHECK(batchMetadata->version == 1,
"Serialized data is not a valid grid handle. Bad batch metadata version.");
const GridMetadata *gridMetadata = reinterpret_cast<const GridMetadata *>(
serializedPtr + sizeof(V01Header) + sizeof(GridBatchMetadata));
for (uint64_t i = 0; i < numGrids; i += 1) {
TORCH_CHECK(gridMetadata[i].version == 1,
"Serialized data is not a valid grid handle. Bad grid metadata version.");
}
const int8_t *gridBuffer = serializedPtr + sizeof(V01Header) + sizeof(GridBatchMetadata) +
numGrids * sizeof(GridMetadata);
const uint64_t sizeofMetadata =
sizeof(V01Header) + sizeof(GridBatchMetadata) + numGrids * sizeof(GridMetadata);
const uint64_t sizeofGrid = header->totalBytes - sizeofMetadata;
auto buf = TorchDeviceBuffer(sizeofGrid, torch::kCPU);
memcpy(buf.data(), gridBuffer, sizeofGrid);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…tailed explanations and improved clarity. Signed-off-by: Francis Williams <francis@fwilliams.info>
Signed-off-by: Francis Williams <francis@fwilliams.info>
harrism
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fwilliams This looks great the added documentation really helps me follow the code. I made a number of suggestions around the design regarding pointers vs. reference / array parameters. I think these could make the code more robust.
Signed-off-by: Francis Williams <francis@fwilliams.info>
Signed-off-by: Francis Williams <francis@fwilliams.info>
harrism
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for the cleanup!
|
BTW you don't need to commit in order to rerun CI. Go to the failed workflow and click "rerun failed jobs" (little circle arrows) in the top right. |
Summary
This PR implements and hardens the Unscented Transform (UT) variant of Gaussian projection, and adds comprehensive native unit tests.
Key highlights:
GaussianProjectionUT.{h,cu}) with shared-memory camera parameters.OpenCVCameraModel, including a constructor-based initialization path.CameraModel::ORTHOGRAPHIC(no perspective divide; distortion is not supported for orthographic).RollingShutterType::NONE, depth/visibility decisions use the start pose (u=0.0) instead of the center pose.mNumDistortionCoeffs == 0(e.g. coeff tensor shape[C,0]): passnullptrinstead of forming&nullptr[0].rotationMatrixToQuaternionfor degenerate inputs.Tests
Added/updated GTests covering:
[C,0]distortion coeff tensors for pinhole projection (null coeff pointer path)Test plan
conda run -n fvdb ./build.sh install verbose -C cmake.define.FVDB_BUILD_TESTS=ONconda run -n fvdb ./build.sh ctest