Forward+: Replace the current BRDF approximation with a DFG LUT and add multiscattering energy compensation

[LiveTrower]

DFG LUT

This PR replaces the approximation described here with an analytical approximation described in real shading, but only for Forward+ rendering.
This implementation takes the approach of generating a 16-bit 128x128 DFG LUT with 1024 samples using a compute shader, and it also includes a modification described by google filament.

Original DFG	Multiscattering DFG

Finally, this DFG LUT also has the DG term of the cloth BRDF included in the blue channel for future implementation of cloth model.

Here are some images to compare before and after:

Before	After

Note: This PR only replaces the approximation in Forward+ rendering, as the current approximation is more suitable for renders like Mobile and Compatibility.

MULTISCATTERING

This PR also implements multiscattering by integrating Filament’s Energy preservation.

Here is a comparison between single scattering and multiscattering:

Single Scattering	Multiscattering

closes: godotengine/godot-proposals#4508

[Calinou]

Tested locally, it works as expected.

Testing project (from godotengine/godot-proposals#4508): test_multiscatter.zip

I'm not sure if the final visual appearance is intended though. I'd have expected the spheres in the bottom-left corner (which are rough and metallic) to have the same color as the background.

Edit: Reading the PR description again, it seems multiscattering isn't implemented yet in the engine by this PR. This PR only does the groundwork that would make it possible, so I guess the visual appearance is intended.

Spheres at the top are smooth, while spheres at the bottom are rough. Spheres at the right are dielectric, while spheres at the left are metallic.

With DirectionalLight3D

Before	After

Without DirectionalLight3D

Before	After

[LiveTrower]

Edit: Reading the PR description again, it seems multiscattering isn't implemented yet in the engine by this PR. This PR only does the groundwork that would make it possible, so I guess the visual appearance is intended.

@Calinou Yes, multiscattering will come in a separate PR to keep this PR simple.

[clayjohn]

How long does the LUT generation take?

[Calinou]

How long does the LUT generation take?

Startup/shutdown time on the MRP I linked seems to be largely unaffected (it's within margin of error on an average of 100 runs for each test).

PC specifications

• CPU: Intel Core i9-13900K
• GPU: NVIDIA GeForce RTX 4090
• RAM: 64 GB (2×32 GB DDR5-5800 C30)
• SSD: Solidigm P44 Pro 2 TB
• OS: Linux (Fedora 41)

Using the GPU

❯ hyperfine -m100 -iw1 "bin/godot.linuxbsd.template_release.x86_64 --path /tmp/test_multiscatter --resolution 64x64 --quit" "bin/godot.linuxbsd.template_release.x86_64.dfg-lut --path /tmp/test_multiscatter --resolution 64x64 --quit"              
Benchmark 1: bin/godot.linuxbsd.template_release.x86_64 --path /tmp/test_multiscatter --resolution 64x64 --quit
  Time (mean ± σ):      2.203 s ±  0.462 s    [User: 0.244 s, System: 0.363 s]
  Range (min … max):    1.609 s …  2.630 s    100 runs
 
Benchmark 2: bin/godot.linuxbsd.template_release.x86_64.dfg-lut --path /tmp/test_multiscatter --resolution 64x64 --quit
  Time (mean ± σ):      2.239 s ±  0.454 s    [User: 0.245 s, System: 0.363 s]
  Range (min … max):    1.565 s …  2.640 s    100 runs
 
Summary
  bin/godot.linuxbsd.template_release.x86_64 --path /tmp/test_multiscatter --resolution 64x64 --quit ran
    1.02 ± 0.30 times faster than bin/godot.linuxbsd.template_release.x86_64.dfg-lut --path /tmp/test_multiscatter --resolution 64x64 --quit

Using lavapipe (CPU rendering)

❯ hyperfine -m100 -iw1 "bin/godot.linuxbsd.template_release.x86_64 --path /tmp/test_multiscatter --resolution 64x64 --quit --gpu-index 1" "bin/godot.linuxbsd.template_release.x86_64.dfg-lut --path /tmp/test_multiscatter --resolution 64x64 --quit --gpu-index 1"
Benchmark 1: bin/godot.linuxbsd.template_release.x86_64 --path /tmp/test_multiscatter --resolution 64x64 --quit --gpu-index 1
  Time (mean ± σ):      2.555 s ±  0.343 s    [User: 3.639 s, System: 0.506 s]
  Range (min … max):    1.771 s …  2.893 s    100 runs
 
Benchmark 2: bin/godot.linuxbsd.template_release.x86_64.dfg-lut --path /tmp/test_multiscatter --resolution 64x64 --quit --gpu-index 1
  Time (mean ± σ):      2.566 s ±  0.318 s    [User: 3.822 s, System: 0.508 s]
  Range (min … max):    1.824 s …  2.887 s    100 runs
 
Summary
  bin/godot.linuxbsd.template_release.x86_64 --path /tmp/test_multiscatter --resolution 64x64 --quit --gpu-index 1 ran
    1.00 ± 0.18 times faster than bin/godot.linuxbsd.template_release.x86_64.dfg-lut --path /tmp/test_multiscatter --resolution 64x64 --quit --gpu-index 1

[BlueCube3310]

Some test results:

4.4.1-rc1	This PR	diff

[LiveTrower]

@clayjohn If the time it takes Godot to generate this DFG seems like a problem to you...
Do you think it's a good idea to generate a cache file of the DFG so that it only gets generated once?
And now, regarding how long it takes to generate this—honestly, I don’t know what optimizations could be made to speed up this process.

[clayjohn]

@LiveTrower how long does the LUT take to generate? Having hard numbers would be helpful. A few ms would be totally fine

[LiveTrower]

@clayjohn Didn’t @Calinou’s test already show how long it takes?

[BlueCube3310]

Uncached, it takes 10-35ms.
Cached, both on an RTX 4060TI and GTX1660TI it takes 2ms

[clayjohn]

What is the issue with implementing multiscattering?

We discussed this in the rendering meeting this week and agreed that we should just go straight for the multiscattering LUT unless there is a technical reason not to

[LiveTrower]

What is the issue with implementing multiscattering?

I don't understand your question. Could you explain more?

[mrjustaguy]

His question is, why not implement multiscattering with this all in one go instead of after this gets merged.

[LiveTrower]

His question is, why not implement multiscattering with this all in one go instead of after this gets merged.

Okay, that's clearer. The reason was that I wanted to keep the PR simple since it's my first PR, and I wasn't sure if this approach was appropriate because I didn’t know if you'd agree with the time it takes for Godot to generate the DFG LUT, and thus make more changes.

My plan was as follows:

• Create this first PR, where the current approximation is replaced by the LUT, and get it approved and merged.

• Continue with the second PR, where multiscattering would be applied with a new specular mode called MultiscatteringGGX. This is because multiscattering is only noticeable on metallic materials with high roughness, while there’s no visual difference in non-metallic materials.

• The final PR would be sheen shading (but that will be more complicated to implement in Godot’s current state due to multiple issues in the GLSL code for applying lighting). Someone has already been making fixes in #100681, #100348, #100503, and #100383, but unfortunately, there’s no further news on that.

These are the main issues:

1. Like clear coat, sheen shading has to scale diffuse and specular by (1.0 - max3(sheen_color) * dfg_sheen * sheen).
2. Clear coat doesn’t take reflection probe reflections into account, so sheen shading would suffer from the same problem.
3. The IBL of the clear coat is not correct since a BRDF approximation is not applied and in the case of sheen shading its own BRDF must also be applied.

Fixing these issues would also involve solving the clear coat problems, so I’d rather wait for proper solutions to make it easier to fully implement sheen shading later.

So, if you prefer, I can combine the first two PRs into one, but the third one will remain in limbo.

[mrjustaguy]

I'd say merging the first two PRs into one makes sense.

As far as the blocker PRs mentioned go for sheen shading, I don't know what their statuses are... If they need more work or are in the massive PR review backlog (seems like the latter at a glance)

[clayjohn]

I'd say merging the first two PRs into one makes sense.

Agreed. For context, we discussed this in the rendering meeting today and the consensus was that it looks slightly better (presumably more accurate which is good), but was really very noticeable. Given that we should move to a multiscattering model anyway, we figured it would make more sense to do both together.

As far as the blocker PRs mentioned go for sheen shading, I don't know what their statuses are... If they need more work or are in the massive PR review backlog (seems like the latter at a glance)

They need more work. What they need is someone willing to champion the PRs and help the contributor get them into a mergable state (i.e. improve code quality, test, ensure compatibility is maintained). I helped on the other big overhaul PRs, but I don't have the bandwidth right now to provide that much assistance.

[Calinou]

Tested locally, it works as expected.

However, the SPECULAR_GGX constant should be renamed back to SPECULAR_SCHLICK_GGX as this breaks compatibility otherwise. While the Schlick GGX name will be technically incorrect when using Forward+, we favor keeping compatibility with existing projects as much as possible.

Forward+

master	This PR

Mobile

master	This PR

Compatibility

master	This PR

[Ansraer]

However, the SPECULAR_GGX constant should be renamed back to SPECULAR_SCHLICK_GGX as this breaks compatibility otherwise. While the Schlick GGX name will be technically incorrect when using Forward+, we favor keeping compatibility with existing projects as much as possible.

@Calinou Why? Isn't the numerical value used for serialization? If so, how would changing the string name change anything?

https://chat.godotengine.org/channel/general/thread/29LjmAoEafWQ8Xwri

[clayjohn]

However, the SPECULAR_GGX constant should be renamed back to SPECULAR_SCHLICK_GGX as this breaks compatibility otherwise. While the Schlick GGX name will be technically incorrect when using Forward+, we favor keeping compatibility with existing projects as much as possible.

@Calinou Why? Isn't the numerical value used for serialization? If so, how would changing the string name change anything?

https://chat.godotengine.org/channel/general/thread/29LjmAoEafWQ8Xwri

In GDScript if I have a line of code that uses SPECULAR_SCHLICK_GGX I would get a parse error with this PR. Using the GDExtension system we can indeed handle compatibility for extensions and TSCN's will also work fine. But there is no reason to break user scripts for an arbitrary name change. API changes should only happen when they are needed

[Calinou]

Code looks good to me.

Please squash the commits together into a single commit. See PR workflow for instructions 🙂

[LiveTrower]

Code looks good to me.

Please squash the commits together into a single commit. See PR workflow for instructions 🙂

Done.

[clayjohn]

Looks great! Thank you for being so responsive to all the requested changes

Requested changes were made

[Repiteo]

Thanks! Congratulations on your first merged contribution! 🎉