Visual artifacts in specular lighting when dotNL < 0

[MoritzBrueckner]

When looking at a face whose normals point away from a sun lamp (i.e, dotNL < 0), there are some visible artifacts depending on the material's roughness value (example file attached below):

This issue was introduced in https://github.com/armory3d/armory/pull/2653, I could pinpoint it to g2_approx() in brdf.glsl. In the above picture on the right you can see an example material with a texture, on the left there is a color ramp driving the roughness from -0.5 to 0.5 (the blue bar on top marks the range from 0 to 1). As you can see, at a roughness of ~0.8 there are some visible artifacts and roughness values > 0.8 and < 1 seem to output 0.

When reversing the changes from https://github.com/armory3d/armory/pull/2653, the example looks correct again (apart from roughness values > 1, which probably shouldn't happen in practice anyways):

When clamping nl in specularBRDF() to [0, 1] (actually, using max(0, nl) should be sufficient), the issue is gone. However, I'm not sure whether this is the correct approach to solve this, and whether nh, nv and vh should be clamped as well (and if so, whether this should be done only in specularBRDF() or everywhere/globally in the lighting shader code). This blog post as well as this one clamp all the dot values, this post uses a not further explained BsdfNDot function to calculate the angle dot products which might also contain some clamping.

@LVutner Sorry to ping you, do you know an answer to this by chance? Also, out of curiousity, what did you mean with NdotL cancels out later here?

Test File Issue_BRDF.zip

[LVutner]

Hello

I thought the dot products are already clamped before specularBRDF() call. My bad. Your approach is correct, dot products passed to specularBRDF() should be clamped (0.0-1.0).

Why we are using *4.0 NdotV as denominator, instead of 4.0 NdotL NdotV? That's because we don't multiply sdirect by NdotL (in fact, that dot product is in lambertDiffuseBRDF()**), thus we can ignore NdotL from Cook-Torrance BRDF and use shorter denom.