TheRealMJP / BakingLab

A D3D11 application for experimenting with Spherical Gaussian lightmaps
MIT License
700 stars 91 forks source link

SH projection issue #2

Closed benoitjacquier closed 6 years ago

benoitjacquier commented 6 years ago

Hi! first of all thanks for sharing this amazing work.

I was playing with the SH functions and i think there is a mistake: In the ProjectOntoSH4 and ProjectOntoSH9 the negative sign is missing in the 1 / 3 / 5 / 7 coefficient. I think it should be:

SH9 ProjectOntoSH9( const float3& dir ) {
    SH9 sh;

    // Band 0
    sh.Coefficients[0] = 0.282095f;

    // Band 1
    sh.Coefficients[1] = -0.488603f * dir.y;
    sh.Coefficients[2] = 0.488603f * dir.z;
    sh.Coefficients[3] = -0.488603f * dir.x;

    // Band 2
    sh.Coefficients[4] = 1.092548f * dir.x * dir.y;
    sh.Coefficients[5] = -1.092548f * dir.y * dir.z;
    sh.Coefficients[6] = 0.315392f * (3.0f * dir.z * dir.z-1.0f);
    sh.Coefficients[7] = -1.092548f * dir.x * dir.z;
    sh.Coefficients[8] = 0.546274f * (dir.x * dir.x-dir.y * dir.y);

    return sh;
}

( see "Stupid Spherical Harmonics (SH) Tricks / Appendix A1 Polynomial Forms of SH Basis " )

Regards

TheRealMJP commented 6 years ago

Hello!

The SH code in this project originally came from many years ago when we were first doing experiments at work. I can't quite remember the circumstances, but we ended up performing simple transformation on the XYZ components to account for differences in our coordinate system compared to what was used in some of the papers we were reading through. Either way we ended up negating X and Y, and when you propagate that through the polynomials you get the code that's in this repository, I believe that this is okay as long as you're careful to be consistent and do the same transformation everywhere, which is why there's no visual errors in the sample. However I think for publicly-released code it would probably be best to fix the polynomials so that they match the literature, which would make it less confusing. It would also be good for anyone that wanted to extend the code further, since they would probably be thrown off by the coordinate transformation.

Either way, I really appreciate you pointing this out. It's all too easy for things like this to unnoticed, and end up forgotten over the years. When I get some time I'll go through the code and fix the SH polynomials, but until then I'll leave this open.