Unwanted division by PI in reflection map contribution

[TaaTT4]

First things first, let me summarize how divisions by PI are handled in OGRE (to check that my assumptions are good):

• The PI at the denominator of the normal distribution function (Trowbridge-Reitz GGX) is stripped away from the R calculation and re-applied subsequently, in the G calculation (geometry function);
• The division by PI in diffuse BRDF calculation (Rd term in OGRE shader) is not present since it's already embedded in kD (which is then stored in pixelData.diffuse);

If everything is OK so far, I'll move on. When it comes time to add the reflection contribution to the final color, pixelData.diffuse is still affected by a PI division which, at this stage, shouldn't be present anymore (or at least counteracted).

[darksylinc]

• The PI at the denominator of the normal distribution function (Trowbridge-Reitz GGX) is stripped away from the R calculation and re-applied subsequently, in the G calculation (geometry function);

I believe that's correct. Some literature presents the formula like that btw.

• The division by PI in diffuse BRDF calculation (Rd term in OGRE shader) is not present since it's already embedded in kD (which is then stored in pixelData.diffuse);

That is correct. We avoid the operation in shader by baking it from CPU

If everything is OK so far, I'll move on. When it comes time to add the reflection contribution to the final color, pixelData.diffuse is still affected by a PI division which, at this stage, shouldn't be present anymore (or at least counteracted).

I don't know why you believe that. I don't know if our current formula is correct either. You're probably right I suspect. So... could you elaborate? :)

[TaaTT4]

I don't know why you believe that. I don't know if our current formula is correct either. You're probably right I suspect. So... could you elaborate? :)

This is how Filament applies the diffuse component of the reflection map: vec3 Fd = pixel.diffuseColor * diffuseIrradiance * (1.0 - E) * diffuseBRDF;

• pixel.diffuseColor is the material diffuse color;
• diffuseIrradiance is the diffuse component of the reflection map;
• (1.0 - E) should be the fresnel coefficient (which, tbh, is calculated with a quite different approach than OGRE);
• diffuseBRDF should be something related to ambient occlusion and/or cloth shading model (nothing of this exists in OGRE);

As you can see, there's no PI involved. And I'm 99% sure that pixel.diffuseColor doesn't embed a PI division in it either. Indeed, in the direct light calculations, pixel.diffuseColor is multiplied with the BRDF diffuse (Lambert or Burley) and the division by PI is present in both formulas as per literature.

[TaaTT4]

Here you can see a comparison without (first image) and with (second image) the division by PI embedded in kD counteracted:

	finalColour += pixelData.envColourD * pixelData.diffuse.xyz * 3.141592654 * fresnelD +
				   pixelData.envColourS * pixelData.specular.xyz * ( fresnelS * envBRDF.x + envBRDF.y );

I've used the Sample_Tutorial_DynamicCubemap sample because it relies on the ibl_specular compositor pass (and thus it generates a more accurate pre-filtered reflection map). I'm not an artist, but the second image, apart being more bright, shows a more pronounced green tint (which in my opinion should be present).

[darksylinc]

Going through this with a clear and open head it was obvious you were right.

It's quite simple once it's pointed out like this:

• The division by PI is needed because one incoming ray gets scattered across the hemisphere. We have ONE INCOMING RAY, and INFINITE OUTGOING RAYS.
• However with IBL we have INFINITE INCOMING RAYS, and INFINITE OUTGOING RAYS

This is a "explain it like I'm 5". A lot of integration math would need to be performed to see if there are no missing correction factors to apply. But if everyone else insists the division by PI should not happen, I won't argue against it :smile: