Page 1 of 1

PBS Math question

Posted: Tue Dec 01, 2015 5:07 pm
by aymar
I was reviewing the PBS implementation and I stumbled upon this line in the getTSNormal function:

Code: Select all

tsNormal.z	= sqrt( 1.0 - max( 0, tsNormal.x * tsNormal.x - tsNormal.y * tsNormal.y ) );
Shouldn't this be:

Code: Select all

tsNormal.z	= sqrt( 1.0 - max( 0, tsNormal.x * tsNormal.x + tsNormal.y * tsNormal.y ) );
(Note the plus sign in the equation).

And in this case the max isn't necessary there, reducing it to:

Code: Select all

tsNormal.z	= sqrt( 1.0 - ( tsNormal.x * tsNormal.x + tsNormal.y * tsNormal.y ) );
Or:

Code: Select all

tsNormal.z	= sqrt( 1.0 - tsNormal.x * tsNormal.x - tsNormal.y * tsNormal.y );
Is it possible that this is an error due to the following:
Z² + X² + Y² = 1
Z² = 1 - X² - Y²
Z = sqrt(1 - X² - Y²)
Z = sqrt(1 - (X² + Y²))
Note that if you fail to switch the signal the equation gets invalidated, this isn't the same as:
Z = sqrt(1 - (X² - Y²))
And this last line doesn't equate to this: Z² + X² + Y² = 1, meaning that the vector isn't normalized anymore, instead, it seems the normal vectors from the normalmaps could be having lengths equal to (1 - 2Y²), and not 1.

I couldn't find a reason why the first version is there, for me it makes more sense doing the latter, am I missing something here?

Re: PBS Math question

Posted: Tue Dec 01, 2015 7:34 pm
by aymar
Well, I tried changing it, and it messed up my scene, the normals were all wrong, what am I not seeing here??

Re: PBS Math question

Posted: Wed Dec 02, 2015 8:18 am
by dark_sylinc
Damn! You're right.

This broke recently.

Should've been:

Code: Select all

sqrt( max( 0, 1.0 - tsNormal.x * tsNormal.x - tsNormal.y * tsNormal.y ) );
The max prevents the final value from going negative (sqrt of negative values is a NaN). An invalid normal map would produce NaNs. Since this is very common because normal maps often get postprocessed via Photoshop (or due to 8-bit rounding issues), the max must be there.

PS. Well spotted!

Re: PBS Math question

Posted: Wed Dec 02, 2015 12:49 pm
by aymar
Gotcha, I'll replace the current implementation with this last one with max and see the effects.