-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Gsplat fix #7696
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
Gsplat fix #7696
Conversation
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 PR updates the Gaussian clamp in both WGSL and GLSL vertex shaders to use the smaller viewport dimension instead of a fixed value (1024), preventing oversized gaussians at low resolutions and improving performance.
- Replace hardcoded clamp max (1024) with viewport-relative clamp
- Apply changes in both
chunks-wgsl
andchunks-glsl
shader modules
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/scene/shader-lib/chunks-wgsl/gsplat/vert/gsplatCorner.js | Use uniform.viewport to derive max Gaussian clamp dynamically |
src/scene/shader-lib/chunks-glsl/gsplat/vert/gsplatCorner.js | Use viewport uniform to derive max Gaussian clamp dynamically |
Comments suppressed due to low confidence (4)
src/scene/shader-lib/chunks-wgsl/gsplat/vert/gsplatCorner.js:49
- [nitpick] The variable name
vmin
is ambiguous. Consider renaming it toviewportMin
orminViewportDimension
to clarify its purpose.
let vmin = min(uniform.viewport.x, uniform.viewport.y);
src/scene/shader-lib/chunks-wgsl/gsplat/vert/gsplatCorner.js:51
- Consider adding test cases or render tests to verify that the Gaussian radius scales correctly with different viewport sizes, ensuring this relative clamp behavior remains valid.
let l1 = 2.0 * min(sqrt(2.0 * lambda1), vmin);
src/scene/shader-lib/chunks-glsl/gsplat/vert/gsplatCorner.js:48
- [nitpick] The variable name
vmin
is not immediately clear. Renaming it toviewportMin
orminViewportDimension
would improve readability.
float vmin = min(viewport.x, viewport.y);
src/scene/shader-lib/chunks-glsl/gsplat/vert/gsplatCorner.js:50
- Add automated tests or render validations to ensure the Gaussian kernel correctly adapts when the viewport size changes, preventing regressions.
float l1 = 2.0 * min(sqrt(2.0 * lambda1), vmin);
This PR updates the gaussian clamp to be relative to the viewport size, instead of fixed 1024.
Using a fixed value resulted in too-large gaussians being rendered at lower resolution and actually flooding the viewport with noise.
This update might improve performance at lower resolutions.