search

sebh / HLSL-Spherical-Harmonics

A collection of HLSL functions one can include to use spherical harmonics in shaders. This repository can be simply be used as a submodule.
MIT License
249 stars 20 forks source link

Typo? #1

Closed nensanders closed 3 years ago

nensanders commented 3 years ago

Hi, thanks for the snippets.

I was wondering, if the plus sign in that line makes any sense? https://github.com/sebh/HLSL-Spherical-Harmonics/blob/7d6c4550b8f762c5372a1258b633f7a196d8bba3/SphericalHarmonics.hlsl#L176

Cheers, Sven

sebh commented 3 years ago

Hi @nensanders , Thanks for looking at the details, but why do you think so?

I have checked this paper again https://d3cw3dd2w32x2b.cloudfront.net/wp-content/uploads/2011/06/10-14.pdf and it looks correct. However maybe I have made a mistake relative to how the SH are encoded? (now that I think of it am not sure I have tested that function apart from that old demo that was not using the same code)

nensanders commented 3 years ago

Hi @sebh I mean the second plus (shL.w+shR.x). Great demo. I basically want to implement the same just on a planet scale. (Also using your SkyAtmosphere code as an inspiration) I am using the SHs for ambient occlusion (That works). I only have problems reconstructing a proper occlusion signal for an entire sphere using only 4 sh coeffiencts and only 1 channel. It appears to be non symmetrical. Maybe that is just a property of SHs or I have a error somewhere, so I looked deeper. For your demo: Did you generate the SHs axis aligned and in object space?

sebh commented 3 years ago

Aaah, Ooouuups! So now it is not "I think" but "I must" not have tested that code :) I really was blind to that operator issue here... Thanks a lot for noticing that issue: it will indeed break everything. I will fix that ASAP.

sebh commented 3 years ago

I also noticed that there must be an error because in the paper, w is the isopriopic SH band (1 non directional value) but in my code x is. So I think there is a remapping that must be happening (probably equation probably xyzw = wzyx as I see it but I will have to test)

sebh commented 3 years ago

And to answer your question: I always stored the SH in world space in order to avoid the SH rotation step.

sebh commented 3 years ago

So, I fixed the issue you mentioned in https://github.com/sebh/HLSL-Spherical-Harmonics/commit/8dc1ea1e15e8025d6f6e11e16513db1e9b56c2ef , thanks @nensanders !

I have also fixed another problem I have found: paper [4] does not store the indices in the same order as [2]. But this implementation follows [2]-Appendix2 order for now. So that should also be a good fix I believe (I will test that but later). See https://github.com/sebh/HLSL-Spherical-Harmonics/commit/4c27263805553bc003ef8c5013962e7fbcbed340

Cheers!