microsoft / DirectXTK12

The DirectX Tool Kit (aka DirectXTK12) is a collection of helper classes for writing DirectX 12 code in C++
https://walbourn.github.io/directx-tool-kit-for-directx-12/
MIT License
1.45k stars 371 forks source link

PBREffect directional lights still impact metalness when they are supposed to be off #129

Closed walbourn closed 1 year ago

walbourn commented 2 years ago

The current implementation of directional lights in the PBR effect mean that even when they are 'off' by default, they still add to the specular metalness factors.

The helper LightSurface in PBRCommon.fxh needs to properly ignore a light that has it's color set to 0,0,0.

walbourn commented 2 years ago

Looking at the shader in more detail, it should end up contributing zero if the light is set to 0,0,0. If you don't call EnableDefaultLighting, it ends up looking a lot more 'metal' than it should.

walbourn commented 1 year ago

After reviewing the shader some more, I'm beginning to think it's not a bug after all. The 'specular radiance' is taken from the IBL and not based on the directional lights at all.

walbourn commented 1 year ago

To be clear, if I change the LightSurface function to this:

    // Accumulate light values
    for (int i = 0; i < numLights; i++)
    {
        if (step(dot(lightColor[i], lightColor[i]), EPSILON) > 0.f)
        {
            // light vector (to light)
            const float3 L = normalize(-lightDirection[i]);

            // Half vector
            const float3 H = normalize(L + V);

            // products
            const float NdotL = saturate(dot(N, L));
            const float LdotH = saturate(dot(L, H));
            const float NdotH = saturate(dot(N, H));

            // Diffuse & specular factors
            float diffuse_factor = Diffuse_Burley(NdotL, NdotV, LdotH, roughness);
            float3 specular = Specular_BRDF(alpha, c_spec, NdotV, NdotL, LdotH, NdotH);

            // Directional light
            acc_color += NdotL * lightColor[i] * (((c_diff * diffuse_factor) + specular));
        }
    }

It made no difference.