-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Gaussian splat spherical harmonics support #12790
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
Thank you for the pull request, @keyboardspecialist! ✅ We can confirm we have a CLA on file for you. |
What is the SH degree of that example asset? In any case, I think that proper SH support can hardly be implemented (and certainly not tested properly) based on the current SPZ loader, because it does not properly decode the SH data - unless the CesiumJS code is anticipating that wrong encoding, and compensating manually, in which case we just have to hope that it will never be fixed in the SPZ loader. |
The underlying SPZ wasm module decodes it. The raw decoded SH buffer is all we need. Example is degree 3 |
Asked for clarification at drumath2237/spz-loader#36 (comment) There is an effect that is clearly visible: Differences to BabylonJS may be due to coordinate system differences (it's difficult) Test data: |
Babylon makes their lives simpler by rotating the world around the model. The model's inverse rotation matrix is applied to the view angle which should give us the correct sampling. One thing to keep in mind is we do apply a axis correction matrix which may contribute to what you are seeing. When I load our tiled SPZ into Babylon its sideways. That 90 degree rotation might be what you are seeing? |
I think that the different coordinate system conventions can likely explain some differences. Until now, I didn't dive into the maths behind the spherical harmonics. Yes, it's a bunch of coefficients that are mushed together with the view direction and eventually yield a "color". But I don't thoroughly understand the 'meaning of the values'. For example, I'd really like to create such a unit cube with spherical harmonics that looks
This could help to see whether the SHs are taken into account correctly, and whether there are any orientation issues. (If someone knows, from the tip of the head, what the SH values would have to be for that, I'd create an example - until then, this is scheduled in the 'When I have way too much time on my hands'-section of my TODO list). |
Creating predefined test data for this is a bit tricky, though. Deriving the right values from the shader code is close to impossible - there's a reason why there is some magic "iterative learner" blackbox generating these coefficients in "real" applications. I considered a lazy/sneaky approach, and just created a GLB of such a (solid) cube, recorded a video of that spinning it in all directions, and uploaded that to poly.cam, but ... that simply refuses to generate splats from that. (Even if it worked, it might not even possible to derive the right values from that). A bit of trial and error gave the "red-cyan" effect, but only that, and only along the diagonal... ... so I think that it might be possible, and maybe even "easy" for someone who knows what he's doing. Not me, at least, not at the moment. |
Hi @keyboardspecialist, what's the status here? Any input on the testing data discussion above? |
CHANGES.md
Outdated
@@ -23,6 +23,7 @@ | |||
- Expand the CustomShader Sample to support real-time modification of CustomShader. [#12702](https://github.com/CesiumGS/cesium/pull/12702) | |||
- Add wrapR property to Sampler and Texture3D, to support the newly added third dimension wrap.[#12701](https://github.com/CesiumGS/cesium/pull/12701) | |||
- Added the ability to load a specific changeset for iTwin Mesh Exports using `ITwinData.createTilesetFromIModelId` [#12778](https://github.com/CesiumGS/cesium/issues/12778) | |||
- Added spherical harmonics support for Gaussian Splats. Supports degrees 1, 2, and 3 in the SPZ format. |
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.
Where can we find a spec or description of the SPZ format? Is the format stable?
A link to the spec, perhaps in GltfVertexBufferLoader
, could go a long way to shortening the spin-up time for future maintainers.
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.
The thing that is closest to a spec is the README of https://github.com/nianticlabs/spz . The actual truth is in the code. Request for clarification are in nianticlabs/spz#42 . This does not yet cover the details of SHs and their layout, but maybe what's in the README is enough... (?)
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.
I should reword that... we load them from SPZ currently, but the rendering side is not tied to SPZ.
@jjhembd Here is a small ply data where the correctness of higher-order SH rendering can be clearly perceived. It would be great if it could help. Below is a comparison screenshot from Supersplat. ![]() ![]() |
I guess that it's hard to just "come up" with the proper SH coefficients for that. So I ported the GLSL code from the GaussianSplattingViewer to Java, took the view- and projection matrix and the camera positions for the top/bottom/left/right/front/back view configuration as the inputs, the desired color as the outputs, and interpreted the whole thing as an optimization problem that I fed into the Apache BOBYQAOptimizer. The result as ASCII PLY, SPZ, GLB, and a matching tileset JSON and Sandcastle: Splat SH orientation experiment 2025-08-13.zip This is what it looks like:
(This is rendered with the viewer from JSplat. BabylonJS shows the same for SPZ. For PLY, there seems to be an orientation issue, or I'm writing the coefficients in the wrong order... who knows what's "right" or "wrong" here...) I tried to check the different views in CesiumJS: Looks about right. The bottom is missing - let's just assume it's magenta. |
@keyboardspecialist @javagl @jjhembd Are we comfortable that the test case above proves out the CesiumJS implementation? If so, can we add it as a unit test? |
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.
Found some places where you need to update the SH prefixes to support the new and the old extensions.
I think this will work. I can put it into a test. |
Thanks @keyboardspecialist, I'm taking a review pass on the code now. For context, my understanding that this PR now also accounts for the new standard where glTF attribute names use extension-specific namespace prefixes. And this in additional to supporting the older style of prefixing attribute names with an underscore
@keyboardspecialist I have now seen some confusion about this from a few users. I assume this axis correction is accounting for the up-axis difference from glTF to 3D Tiles. I want to make sure I'm clear on this so I'm responding to users correctly.
|
@keyboardspecialist It looks like this PR contains some changes from #12843. Could you please merge main into this branch so we're reviewing code specific to SH? |
* @type {number} | ||
* @private | ||
*/ | ||
this.shDegree = 0; |
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.
Do we need both degree
and coefficient
as independent properties? It's possible to determine one based on the other, no?
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.
You are correct it can be derived, but it's nice to not have to and keeps things more explicit. They are used in different ways when making checks and indexing.
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.
Could we set one and use a getter function for the other? I'm mostly concerned about these values getting out of sync since they can both be set.
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.
Oh, I see now they are set privately and only exposed via the getter. Should all be fine then. 👍
I think that the context of the first quote was very specifically the different appearances of the SH colors. When something is supposed to be "red from the front and green from the top", everything depends on what "front" and "top" are. But more generally, there are two aspects of ~"coordinate systems" standing out:
Now one could argue that this is independent of this PR. After all, this PR is only about spherical harmonics. When there's an issue with the orientation, then you can just rotate the splats. Just use the right matrix at the right place. Well... it's not that easy. On top of the (always confusing and error prone) differences of up-axis-conventions, "rotating splats" also involves "rotating the spherical harmonics (~'coefficients')". Have a look at this |
Checks 3 rotations around the central splat in the cube.
Some of the axis issues were due to the old SPZ loader that always rotated the incoming data around X. Indeed, rotating spherical harmonics is prohibitively complex and expensive at runtime which is why we avoid it. When we sample we apply the model rotation to the view, so we sample like it's in its original orientation. |
|
switch (tile.content.sphericalHarmonicsDegree) { | ||
case 1: | ||
coefs = 9; | ||
break; | ||
case 2: | ||
coefs = 24; | ||
break; | ||
case 3: | ||
coefs = 45; | ||
} |
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.
Should we throw an error here in case the SH is not 1
, 2
, or 3
? Or is that unlikely to happen (both with our tiler or third-party ones)?
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.
I think it's ok not to check here since the data should be checked and ready for rendering. 3 is the max anyone has ever trained that I'm aware of. I could see adding a check in the loader and notifying the user.
With our data coming from SPZ it shouldn't be an issue.
const totalLength = tileContent.pointsLength * (coefs * (2 / 3)); //3 packs into 2 | ||
const packedData = new Uint32Array(totalLength); | ||
|
||
const shAttributes = tileContent.splatPrimitive.attributes.filter((attr) => |
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.
I don't think the attributes
property was properly defined in the GaussianSplatPrimitive
class. Looks like it was getting set as a side-effect of a helper function in ModelUtility
. Please add and document it.
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.
It's defined in the GS content class. Very ambiguous, but it's a reference to the glTF primitive. I renamed it for better clarity.
Description
Adds spherical harmonic support for Gaussian splats in the SPZ format. Supports degrees 1-3 and auto detects what is available. This brings full SPZ support to CesiumJS.
Issue number and link
Fixes #12756
Testing plan
View the same asset tiled with 3 degrees of SH in the current main build versus the CI brand build:
No SH Support
SH Support
Author checklist
CONTRIBUTORS.md
CHANGES.md
with a short summary of my change