viser icon indicating copy to clipboard operation
viser copied to clipboard

[Feature] Spherical Harmonics Support (Viser 0.2.23)

Open Crezle opened this issue 10 months ago β€’ 9 comments

Inspired by PR #286, this feature aims to add further functionality for "add_gaussian_splats()" where view-dependent effects can be represented. This is often useful due to exported splat models often including Spherical Harmonics variables in addition to base color.

This PR also contains some traces of attempted surface normal implementation, but without any .PLY files including non-zero normals, this will be covered in a separate PR.

Crezle avatar Feb 18 '25 12:02 Crezle

This should be backward compatible with former use of "gaussian_splats.py" (e.g. if SH-coeffs are not provided). Only concern is that performance might be a tiny bit reduced due to larger packets having to be sent due to now 45 additional coefficients being sent (each at 16 bits).

Crezle avatar Feb 18 '25 13:02 Crezle

Hi @Crezle, sorry for the late response here!

This looks great to me, would love to merge support for SH in. πŸ™‚

I know @AntonioMacaronio and @beckyfeng08 also spent a fair amount of time on SH support in the shader; I'm interested in their thoughts.

For getting this merged:

  • The main thing I can think of is to eliminate the performance penalty when SH coeffs aren't specified. An extra 45 coefficients per Gaussian is a lot. In the shader itself, we could also do some preprocessor conditionals (#ifdef etc) to prevent the SH code from running when there are no coefficients.
  • From a glance it looks like the rgbs arg is unused when sh_coeffs is not None, if so this feels a bit weird?

brentyi avatar Mar 23 '25 23:03 brentyi

Hi @Crezle, sorry for the late response here!

This looks great to me, would love to merge support for SH in. πŸ™‚

I know @AntonioMacaronio and @beckyfeng08 also spent a fair amount of time on SH support in the shader; I'm interested in their thoughts.

For getting this merged:

* The main thing I can think of is to eliminate the performance penalty when SH coeffs aren't specified. An extra 45 coefficients per Gaussian is a lot. In the shader itself, we could also do some preprocessor conditionals (`#ifdef` etc) to prevent the SH code from running when there are no coefficients.

* From a glance it looks like the `rgbs` arg is unused when `sh_coeffs is not None`, if so this feels a bit weird?

Thank you for the response @brentyi! As I see it is that general 3DGS exports always contain 48 coefficients, where the first 3 are RGB. If we consider that we always have all the coefficients supplied, we can just ignore the RGB argument, but I assume we still want to keep the flexibility of using constant colors.

So, somehow we want to require at least 3 out of the 48 coefficients. I guess we can set a size requirement for the input SH coefficents, where we expect it to either have 3 or 48 coefficients.

To also respect the desire to not send a bunch of zeros if we only have 3 coefficients, we could try to handle this at client-end, where it can dynamically handle a message consisting of either 3 or 48 coefficients.

What do you think?

Crezle avatar Mar 24 '25 11:03 Crezle

In that case: to simplify the API, maybe we just add a separate Python function for adding the splats with SH support? Maybe like add_gaussian_splats_sh().

To also respect the desire to not send a bunch of zeros if we only have 3 coefficients, we could try to handle this at client-end, where it can dynamically handle a message consisting of either 3 or 48 coefficients.

Yeah, this sounds good. I think the networking overhead and shader overhead here are ~equally important. Since most applications are fine without SH it would be great if there was no additional overhead associated with it!

brentyi avatar Mar 28 '25 05:03 brentyi

Not an issue with this PR but I'm getting a weird error where I can't load a splat anymore on a dev version of viser (viser installed with pip install -e .) image

However, whenever viser is installed normally with pip install viser this issue does not arise πŸ‘€

AntonioMacaronio avatar Apr 16 '25 20:04 AntonioMacaronio

Yeah, I discovered something similar myself amongst with other fellow students. Seems to be an issue since I merged changes from main into this branch.

Seems like it was fixed with the last issue, but me and the others will take a look at it soonπŸ‘

Crezle avatar Apr 17 '25 09:04 Crezle

Can confirm it's fixed!! thank you brent

AntonioMacaronio avatar Apr 17 '25 09:04 AntonioMacaronio

Hi, I just wanted to update that I am going to look at this later on and have not "abandoned" this PR. Current focus is just to complete my thesis πŸ‘

Crezle avatar May 24 '25 09:05 Crezle

Best of luck @Crezle, and thanks for the update!!

brentyi avatar May 27 '25 01:05 brentyi