Wrong math in LightProbeIntegration

[philcn]

I found this bug in the integrateDFG function that caused the DFG texture to be incorrect:

https://github.com/NVIDIAGameWorks/Falcor/blob/dev-4.0/Source/Falcor/Data/Framework/Shaders/LightProbeIntegration.ps.slang#L136

It should be L = reflect(-V, H).

[guoxx]

I think another issue regarding light probe pre-filtering is how to map roughness to mimap. Current implementation is mapping alpha to mipmap linearly during pre-filtering, however function linearRoughnessToLod is taking alpha and wrap squared root of alpha to mipmap, which doesn't match with pre-filtering configuration.

IMO, the better solution for that is map linear roughess to mipmap or squared root of linear roughness to mimap, as suggested by Frostbite in "moving frostbite to pbr".

[philcn]

@guoxx It wasn't obvious for me whether the integration code is treating the 0-1 range as GGX alpha or linear roughness, so I checked the shader and found a problem similar to https://github.com/NVIDIAGameWorks/Falcor/issues/212.

The importanceSampleGGX() function is taking the gRoughness and calculating a variable called "a", which I assume means "alpha", to be roughness^2. This looks like it's mapping the 0-1 range to linear roughness. However, Karis' original importance sample GGX function calculates alpha^2 (roughness^4). So I think the code is actually mapping GGX alpha to mip level, despite that the variable should be called "a2".

They're in https://github.com/NVIDIAGameWorks/Falcor/blob/master/Source/Falcor/Data/Framework/Shaders/LightProbeIntegration.ps.slang#L68. Please correct me if I'm wrong.

[guoxx]

@philcn in LightProbeIntegration.ps.slang, variable roughness is used as parameter for several functions such as evalGGX, evalSmithGGX and importanceSampleGGX. I assume it's the GGX alpha (mapping from roughness^2) in PBR context. So the implementation of importanceSampleGGX is wrong as you mentioned in https://github.com/NVIDIAGameWorks/Falcor/issues/212.

I had integrate the fix proposed by you, so that the mipmap of pre-filtered IBL is linearly mapped from gRoughness which is GGX alpha in my case. However in https://github.com/NVIDIAGameWorks/Falcor/blob/master/Source/Falcor/ShadingUtils/Lights.slang#192, the mip level of IBL is calculated by sqrt(ggxAlpha)*(mipCount-1) which doesn't match with pre-filtering code. It's trivial to fix by change it to ggxAlpha*(mipCount-1)

With all of these bug fixed. I got a image in below

As we can see that the the one with low roughness is too sharp compare to ground truth. With linear roughness mapping we are able to get something better

[philcn]

Thanks for testing it out. To summarize, there are two paths:

1. Map GGX alpha to mip level (treating gRoughness as GGX alpha). In this case we need to fix linearRoughnessToLod() as you proposed. I don't think we need to change importanceSampleGGX() despite making the variable name more accurate. Can you confirm that you didn't have to change importanceSampleGGX()?

2. Map linear roughness to mip level (treating gRoughness as linearRoughness). In this case we need to fix importanceSampleGGX() to calculate alpha^2, or linearRoughness^4, but we don't need to change linearRoughnessToLod().

The two approaches are theoretically identical but can distribute the perceptual roughness more evenly in the 0-1 range, so it works better with low roughness.