HDRenderPipeline: Fix issue with GetShiftedNormal (cause artifact in diffuse lighting)

Assets/ScriptableRenderPipeline/HDRenderPipeline/Material/Lit/Lit.hlsl

 // Precomputed lighting data to send to the various lighting functions
 struct PreLightData
 {
+    // General
+
+    // GGX iso
-    // Aniso
+    // GGX Aniso
-
+    // IBL
     float3 iblDirWS;    // Dominant specular direction, used for IBL in EvaluateBSDF_Env()
     float  iblMipLevel;
 
 {
     PreLightData preLightData;
 
-    // We have handle the case of NdotV being negative in GetData() function with GetShiftedNdotV.
-    // So we don't need to saturate or take the abs here.
-    // In case a material use negative normal for double sided lighting like speedtree this will be handle in the GetData() code too.
-    preLightData.NdotV = dot(bsdfData.normalWS, V);
+    // General
+    float3 iblNormalWS = bsdfData.normalWS;
+    // GetShiftedNdotV return a positive NdotV
+    // In case a material use negative normal for double sided lighting like Speedtree  they need to do a new calculation
+    preLightData.NdotV = GetShiftedNdotV(iblNormalWS, V); // Handle artificat for specular lighting
+    // GGX iso
-    float  iblNdotV    = preLightData.NdotV;
-    float3 iblNormalWS = bsdfData.normalWS;
-
-    // Check if we precompute anisotropy too
+    // GGX aniso
     if (bsdfData.materialId == MATERIALID_LIT_ANISO)
     {
         preLightData.TdotV = dot(bsdfData.tangentWS, V);
-        iblNormalWS = GetAnisotropicModifiedNormal(bsdfData.bitangentWS, bsdfData.normalWS, V, bsdfData.anisotropy);
+        iblNormalWS = GetAnisotropicModifiedNormal(bsdfData.bitangentWS, iblNormalWS, V, bsdfData.anisotropy);
-        // NOTE: If we follow the theory we should use the modified normal for the different calculation implying a normal (like NDotV) and use iblNormalWS
+        // NOTE: If we follow the theory we should use the modified normal for the different calculation implying a normal (like NdotV) and use iblNormalWS
-    GetPreIntegratedFGD(iblNdotV, bsdfData.perceptualRoughness, bsdfData.fresnel0, preLightData.specularFGD, preLightData.diffuseFGD);
+    // IBL
+    GetPreIntegratedFGD(preLightData.NdotV, bsdfData.perceptualRoughness, bsdfData.fresnel0, preLightData.specularFGD, preLightData.diffuseFGD);
-
-    // Area light specific
+    // Area light
     // UVs for sampling the LUTs
     float theta = FastACos(preLightData.NdotV);
     float2 uv = LTC_LUT_OFFSET + LTC_LUT_SCALE * float2(bsdfData.perceptualRoughness, theta * INV_HALF_PI);
     float NdotL    = saturate(dot(bsdfData.normalWS, L));
     float NdotV    = preLightData.NdotV;
     float LdotV    = dot(L, V);
-    float invLenLV = rsqrt(abs(2 + 2 * LdotV));    // invLenLV = rcp(length(L + V))
+    // GCN Optimization: reference PBR Diffuse Lighting for GGX + Smith Microsurfaces
+    float invLenLV = rsqrt(abs(2.0 * LdotV + 2.0));    // invLenLV = rcp(length(L + V))
-    float LdotH    = saturate(invLenLV + invLenLV * LdotV);
+    float LdotH    = saturate(invLenLV * LdotV + invLenLV);
 
     float3 F = F_Schlick(bsdfData.fresnel0, LdotH);
 

Assets/ScriptableRenderPipeline/HDRenderPipeline/Material/MaterialUtilities.hlsl

 }
 
 // This function convert the tangent space normal/tangent to world space and orthonormalize it + apply a correction of the normal if it is not pointing towards the near plane
-void GetNormalAndTangentWS(FragInputs input, float3 V, float3 normalTS, inout float3 normalWS, inout float3 tangentWS, bool wantNegativeNormal = false)
+void GetNormalAndTangentWS(FragInputs input, float3 V, float3 normalTS, inout float3 normalWS, inout float3 tangentWS)
 {
     #ifdef SURFACE_GRADIENT
     normalWS = SurfaceGradientResolveNormal(input.worldToTangent[2], normalTS);
-
-    GetShiftedNdotV(normalWS, V, wantNegativeNormal);
 
     // Orthonormalize the basis vectors using the Gram-Schmidt process.
     // We assume that the length of the surface normal is sufficiently close to 1.

Assets/ScriptableRenderPipeline/ShaderLibrary/AreaLighting.hlsl

 
     float3 F = float3(0, 0, 0);
 
-    for (int edge = 0; edge < 4; edge++)
+    for (uint edge = 0; edge < 4; edge++)
     {
         float3 V1 = L[edge];
         float3 V2 = L[(edge + 1) % 4];

Assets/ScriptableRenderPipeline/ShaderLibrary/CommonLighting.hlsl

 // Helper functions
 //-----------------------------------------------------------------------------
 
-// NdotV should not be negative for visible pixels, but it can happen due to the
-// perspective projection and the normal mapping + decals. In that case, the normal
-// should be modified to become valid (i.e facing the camera) to avoid weird artifacts.
-// Note: certain applications (e.g. SpeedTree) require to still have negative normal to perform their own two sided lighting, they can use wantNegativeNormal
-// This will potentially reduce the length of the normal at edges of geometry.
-float GetShiftedNdotV(inout float3 N, float3 V, bool wantNegativeNormal)
+// NdotV can be negative for visible pixels due to the perspective projection, the normal mapping and decals.
+// This can produce visible artifacts with direct specular lighting (white point, black point) and indirect specular (artifact with cubemap fetch)
+// A way to reduce artifact is to limit NdotV value to not be negative and calculate reflection vector for cubemap with a shifted normal (i.e what depends on the view)
+// This is what provide this function
+// Note: NdotV return by this function is always positive, no need for saturate
+float GetShiftedNdotV(inout float3 N, float3 V)
-    float limit = rcp(256.0); // Determined mostly by the quality of the G-buffer normal encoding
+    const float limit = 0.0001; // Epsilon value that avoid divide by 0 (several BSDF divide by NdotV)
-    if (!wantNegativeNormal && NdotV < limit)
+    if (NdotV < limit)
-        // We do not renormalize the normal because { abs(length(N) - 1.0) < limit }.
+        // We do not renormalize the normal because { abs(length(N) - 1.0) < limit } + It is use for cubemap
         N    += (-NdotV + limit) * V;
         NdotV = limit;
     }