A novice question about the math in Urho3D’s PBR shader

I was/am struggling to understand the math, and I just read this PI or not to PI in game lighting equation | Sébastien Lagarde to get some idea of whether or not there should be a PI in the calculation.

Now, from what I understand, generally the lightColor’s unit in a game is defined in the way so that outColor = PI * BRDF * materialDiffuse * lightColor * NdotL, which in turn makes the Lambertian diffuse as simple as outColor = materialDiffuse * NdotL * lightColor. Or if the lightColor represents the actual intensity, the equation would be materialDiffuse * NdotL * lightColor / PI.

But while reading Urho3D’s shader, I found that its Lambertian term seemingly strangely is divided by PI twice!

See in PBRLitSolid.hlsl: void PS(…)
finalColor.rgb = BRDF * lightColor * (atten * shadow) / M_PI;

and in BRDF.hlsl: LambertianDiffuse(…)
return diffuseColor * (1.0 / M_PI) ;

also in BRDF.hlsl: CustomLambertianDiffuse(…)
return diffuseColor * (1.0 / M_PI) * pow(NdotV, 0.5 + 0.3 * roughness)

How come “/ M_PI” is done twice?
Let’s forget the Lambertian term, why is the finalColor BRDF * lightColor / M_PI in the first place?
Should it be “* M_PI” instead?

Excuse me for being a total amateur in CG, but please somebody tell me where did I miss. Or if Urho3D is using some other rendering scheme, could you explain, or is it documented somewhere?

Thanks in advance!

As a person who recently worked with PBR in Urho…
I don’t trust anything in Urho PBR shaders and I rewrote them from scratch. There are many weird or wrong places, and I don’t feel like trying to find reason behind it.

If you want to understand maths of PBR, read Filament docs, not Urho shaders.

The PBR shaders are a kitchen sink hell problem. You can toggle this or that and there’s then this way or that way things play out (pretty sure Lambert divide is independent of specular and final contribution though). That leads to a lot of redundant math and as in this case, repeated math in the shaders where a more forceful “you will do it this way” approach would do things more simply (or at least clearly).

TL;DR the PBR shaders suck and it’s partially my fault. Don’t trust the shaders.

That’s a bit disappointing.
I’ll try Filament next. Thanks!

GetBRDF does call Diffuse:
float3 Diffuse(float3 diffuseColor, float roughness, float NdotV, float NdotL, float VdotH)
{
//return LambertianDiffuse(diffuseColor);
return CustomLambertianDiffuse(diffuseColor, NdotV, roughness);
//return BurleyDiffuse(diffuseColor, roughness, NdotV, NdotL, VdotH);
}.
Then it returns diffuseFactor + specularFactor.
Then the result BRDF gets divided by M_PI again, and as I have said, this line alone is difficult for me to understand:
finalColor.rgb = BRDF * lightColor * (atten * shadow) / M_PI;

In case you are curious, this is as close to “Filament shaders in Urho” as we will ever get:

It looks great!
I wasn’t sure if I understood the equations correctly. Your version seems to have nothing against my understanding.
I guess the devs didn’t agree on where to do the division, thus resulted in an extra one.

Correctness comes first. It seems to me PBR should be reworked for Urho.

That one isn’t me. I’m not related to the CustomLambertian, I know nothing about it or the reasoning behind it so I have no comment on what it should or shouldn’t be doing.

Presently I do Destiny 1 style LUT shading and only target VR so much of this PBR nonsense has exited my brain as pointless, I now do Doom 3 style low-tech tricks and care more about translucent shadowmaps than anyone else.

I had no clue at first, but now I believe somebody tried to extract the common divisor M_PI, from diffuse and specular, but it ended up repeated in multiple places somehow.

PBR.hlsl: GetBRDF(…)
specularFactor = distTerm * visTerm * fresnelTerm * ndl/ M_PI;
See, this “M_PI” is extracted from distTerm.

Hopefully someone can check and fix it if it’s indeed wrong.