-
Notifications
You must be signed in to change notification settings - Fork 17
Rasterization using 3d gaussians #444
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
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
This pull request adds 3D Gaussian Splatting (3DGS) rasterization capability that enables rendering directly from world-space Gaussians rather than from 2D projections. This allows geometry gradients to flow through the rasterization step, which is particularly useful for non-pinhole camera models with distortion.
Changes:
- Adds new
render_images_from_world_3dgsmethod that performs ray-ellipsoid rasterization directly in world space - Introduces
CameraModelandRollingShutterTypeenums for flexible camera projection support - Implements CUDA kernels for forward and backward passes of the 3DGS rasterizer with support for multiple camera models (pinhole, orthographic, and OpenCV distortion variants)
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/fvdb/detail/ops/gsplat/GaussianCameraModels.h | Defines camera model and rolling shutter enums (contains critical duplication bug) |
| src/fvdb/detail/ops/gsplat/GaussianRasterizeFromWorld3DGS.cuh | Shared utilities for ray generation, distortion, and coordinate transformations |
| src/fvdb/detail/ops/gsplat/GaussianRasterizeFromWorld3DGSForward.cu | CUDA kernel for forward rasterization pass |
| src/fvdb/detail/ops/gsplat/GaussianRasterizeFromWorld3DGSBackward.cu | CUDA kernel for backward gradient computation |
| src/fvdb/detail/autograd/GaussianRasterizeFromWorld3DGS.{h,cpp} | PyTorch autograd wrapper for the rasterization operation |
| src/fvdb/GaussianSplat3d.{h,cpp} | Main API implementation routing to appropriate projection and rasterization paths |
| src/python/GaussianSplatBinding.cpp | Python bindings for the new enums and method |
| fvdb/gaussian_splatting.py | Python wrapper for the new rendering method |
| fvdb/enums.py | Python enum definitions for camera models and rolling shutter types |
| fvdb/_fvdb_cpp.pyi, fvdb/init.pyi, fvdb/init.py | Type stubs and exports for the new functionality |
| tests/unit/test_rasterize_world3dgs.py | Unit tests for gradient correctness and finite difference validation |
| src/CMakeLists.txt | Build configuration for new source files |
| src/fvdb/detail/ops/gsplat/GaussianProjectionUT.h | Refactored to use shared enum definitions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/fvdb/detail/ops/gsplat/GaussianRasterizeFromWorld3DGSForward.cu
Outdated
Show resolved
Hide resolved
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
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 19 out of 19 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Francis Williams <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: Francis Williams <[email protected]>
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 19 out of 19 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/fvdb/detail/ops/gsplat/GaussianRasterizeFromWorldForward.cu
Outdated
Show resolved
Hide resolved
src/fvdb/detail/ops/gsplat/GaussianRasterizeFromWorldBackward.cu
Outdated
Show resolved
Hide resolved
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
Signed-off-by: Francis Williams <[email protected]>
|
Could we get a description of the PR please? |
|
@swahtz : updated:
Folding camera model + rolling-shutter policy into |
Use RenderSettings for image sizing/origin/tileSize in rasterize-from-world dispatch APIs, matching other rasterize utilities. Co-authored-by: Cursor <[email protected]> Signed-off-by: Francis Williams <[email protected]> Co-authored-by: Cursor <[email protected]>
7f39626 to
04b4862
Compare
Allow rasterize-from-world-3dgs to accept opacities as either [N] or [C,N] by broadcasting to [C,N] internally. Also fix autograd backward to return exactly one gradient per forward input. Signed-off-by: Francis Williams <[email protected]> Co-authored-by: Cursor <[email protected]>
src/fvdb/detail/ops/gsplat/GaussianRasterizeFromWorldForward.cu
Outdated
Show resolved
Hide resolved
src/fvdb/detail/ops/gsplat/GaussianRasterizeFromWorldForward.cu
Outdated
Show resolved
Hide resolved
src/fvdb/detail/ops/gsplat/GaussianRasterizeFromWorldForward.cu
Outdated
Show resolved
Hide resolved
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.
There are a lot of things that are inconsistent with current rasterization implementation. It would be good to harmonize that. My main concern is the deadlock test. Is it necessary? Was this discovered in code in the wild? Do we need to test for deadlock both in Python and C++? I don't think so.
src/fvdb/detail/ops/gsplat/GaussianRasterizeFromWorldForward.cu
Outdated
Show resolved
Hide resolved
src/fvdb/detail/ops/gsplat/GaussianRasterizeFromWorldForward.cu
Outdated
Show resolved
Hide resolved
swahtz
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.
There's quite a few inconsistencies in this rasterization kernel compared to the precedents set by the other rasterization kernels (building a RasterizeCommonArgs, variable naming and documentation of functions). I'd prefer to see those specific aspects addressed to be more maintainable and other comments could be left for later issues.
- Use more descriptive camera pose variable names in the forward kernel. - Rename transmittance accumulator (T) for clarity. - Add short comments/TODOs aligning follow-ups with existing rasterizer conventions. Signed-off-by: Francis Williams <[email protected]> Co-authored-by: Cursor <[email protected]>
Switch rasterize-from-world 3DGS forward/backward kernels from PackedTensorAccessor32 to PackedTensorAccessor64 for consistency with other rasterization kernels and to support larger tensors. Signed-off-by: Francis Williams <[email protected]> Co-authored-by: Cursor <[email protected]>
Address review feedback by clarifying Unscented Transform wording, improving gradient discontinuity notes, documenting rasterize-from-world helpers, and tightening minor forward-kernel readability/sync details. Signed-off-by: Francis Williams <[email protected]> Co-authored-by: Cursor <[email protected]>
swahtz
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.
Thank you for making some of the requested changes.
I think in the next steps, we should first refactor this to match the patterns of other rasterization functions (RasterizeCommonArgs, etc.). After that I think we should discuss with @harrism refactoring of common functionality. There's a few changes I've also wanted to be make to the general pipeline but have held off such as separating Camera definition settings; Rasterizer method-agnostic settings and Rasterizer-specific settings in a semantically useful way as well as build in the ability use other shading evaluation methods and not just force all rasterization to use the SH evaluation path.
Summary
This PR adds a dense, differentiable rasterize-from-world path for Gaussian splatting that renders images directly from world-space 3D Gaussians (means/quats/log-scales) instead of relying only on 2D projected conics. It supports UT-based projection for OpenCV camera models, integrates into the
GaussianSplat3dpublic API, and includes extensive regression/gradient tests.Key changes
New public API
GaussianSplat3d.render_images_from_world(...)(Python + C++ bindings)CameraModel+RollingShutterTypeand packed OpenCV distortion coefficients.New kernels + autograd (dense-only)
Camera model consolidation + reuse
CameraModel/RollingShutterTypeenums (C++) and PythonIntEnumwrappers infvdb.enums.Masks + backgrounds parity
Docs + naming cleanup
render_images_from_world.*_3dgssuffix in the public API).Test plan
./build.sh ctest/tmpso Python imports the installed extension:conda run -n fvdb python -m pytest -q --rootdir /path/to/fvdb-core/tests /path/to/fvdb-core/testsNotes / follow-ups
CameraModel/RollingShutterTypeare kept explicit in some dispatch APIs; folding them intoRenderSettingscould be a follow-up if we want a single transport object everywhere.