Rewrite the GPU Lightmapper's indirect logic to match Godot 3.5's CPU Lightmapper.

[DarioSamo]

Problem

The current lightmapper suffers from some implementation flaws that are very tough to fix without rewriting the logic.

One issue that has been repeatedly brought up is #73295, which clearly shows the wrong behavior of the indirect lighting logic as bounces increases in the scene. My conclusions of why the wrong behavior was caused were pretty apparent when digging into the implementation.

For anyone unfamiliar with the topic, the expectation when increasing bounces is not for the overall ambience of the scene to increase beyond the first few iterations. It is very rare for any lighting solutions out there to achieve anything significant past six bounces or so due to how the light transfer works on each step.

Godot 3.x's CPU lightmapper has a much more correct implementation of how this should behave. It's worth noting that while it's not exactly physically correct, it has the nice benefit of providing a user-tweakable attribute to get as much indirect energy in the bounces as the user wants, which this PR also adds as a configurable option (thanks to @Calinou for the reference PR at #51705).

As I did not feel it'd be right to rewrite the current implementation with one of my own in this particular case, it seemed to be a better fit to just port the behavior from Godot 3.x so it works pretty much the same way in 4.x.

Solution

This PR reimplements the logic used by both the indirect bounces step and the light probes step to match the behavior of Godot 3.x, but does it in the existing compute shader. There's a noticeable reduction in baking times, as extra bounces are not calculated by just iterating on top of the starting position and can be terminated early if it hits the environment or a back-face. It also implements Godot 3.5's Russian Roulette early termination for an additional optimization based on the probability of the current color being carried through the bounce.

Comparisons

All shots are taken with the same settings (high quality) except for the bounce. I would've gone higher but current master branch was taking far too long to bake it due to each bounce counting as a full iteration. Denoiser is off.

Given the nature of the algorithm has changed, the noise will be decreased further and further in master compared to this PR because it just traces more rays (as evident by the longer bake times). Just because master looks less noisy under the same quality setting doesn't imply the result is better, as it lacks the ability to converge to the correct result.

This behavior is expected and can be easily compensated by just increasing the ray count in the PR instead, as each bounce no longer implies another set of rays being dispatched but just an increase of cost per ray instead.

PR - 1 Bounce

Time to bake: 01:22

PR - 3 Bounces

The biggest benefits to indirect lighting are indeed achieved with at least one more additional bounce, as evident here.

Time to bake: 01:40

PR - 6 Bounces

As expected, it hardly looks any different from the 3 bounces version.

Time to bake: 01:51

As a bonus to showcase the noise difference I mentioned, here's an Ultra quality shot of the PR, showing the convergence.

Bonus Shot - PR - 6 Bounces - Ultra Quality

Just to showcase the difference that can be achieved with increasing the ray count and how it converges to a much better result.

Master - 1 Bounce

Expected to be pretty much identical to this PR.

Time to bake: 01:22

Master - 3 Bounces

Lighting starts to get significantly washed out.

Time to bake: 02:48

Master - 6 Bounces

We're going full speed ahead towards full bright at this point.

Time to bake: 04:31

Feedback

We should make sure this doesn't break any of the existing bake options, as there's different configurations (static, dynamic, directional, etc) to try out.

Sadly, it is expected the visual result will change when using existing settings and that artists will need to tweak them to get a similar result. There's no realistic way to automatically convert this and it's probably best to just address this as soon as possible given how many other issues this can fix.

However, it's worth noting Lightmap GI will have the new "Bounce Indirect Energy" property, as well as all types of light will actually use the "Indirect Energy" property when baking, which should allow much better control over the intensity of the indirect lighting from the baked result.

TODO

• Using physical light units seem to be broken.
• Review if directional baking works properly.
• Change ray counts of the quality presets to resemble the approach master takes for the default bounces.
• Change max property hint for bounces to 6.

Bugsquad edit: Fixes #77356; Fixes #73295; Fixes #77169

[jcostello]

Here is my test in my lightmap scene

https://imgsli.com/MjA3ODY1
https://imgsli.com/MjA3ODY2
https://imgsli.com/MjA3ODY3
https://imgsli.com/MjA3ODY4
https://imgsli.com/MjA3ODY5
https://imgsli.com/MjA3ODcw

This last 3 are energy comparison in the same PR
Fe de erratas: this one is 3 bounces vs 3 bounces
https://imgsli.com/MjA3ODcy
https://imgsli.com/MjA3ODcz
https://imgsli.com/MjA3ODc1

Seems that the bounces works better but still introduces a lot of light after each bounce

in the description of the PR 3 vs 6 still introduce a lot of sky light and ambient light but IDK the internals of the algoritm. In blender seems that after 4 bounces, only small details get more light.
This is only my opition but in general seems that the lightmap indirect lighting works much much better

Also, probes seems to work better with static lighting. May be fix this issue #77356

[DarioSamo]

@jcostello The thing I notice in these scenes is that the surfaces pretty much look to be white, so I imagine the albedo is basically full white. In that case, the attenuation from the bounces, at least according to 3.x's logic, will pretty much be non-existent, as it's the only factor used to decrease the intensity of the light transfer.

I think we can review that behavior to be more accurate, but it'll require changing the algorithm to decrease the throughput based on more than just the albedo. We could consider that for this PR or a future one as far as I see, but it's pretty clear at least the core issue is fixed for more common scenarios where the textures aren't just full white.

[jcostello]

@DarioSamo yeah, surface is pure white. I imagine that the length of the ray has to be taken into consideration for the atenuation after the bounce, even if its pure white.

Never the less the PR fixes most of the cases and probably #77356 (I have to test it further)

There is this case we have to test and figure it out what is going on before merge

[jcostello]

@DarioSamo with non pure white values atenuation works well. Never the less distance could be a factor to take into consideration because event pure white wont bounce the exact same energy at long distance (because light tends to spread)

Edit
3 Bounces

10 Bounces

Looking pretty good

[jcostello]

Static

Dynamic

Seems to fix #77356 for probes generation on static vs dynamic. Still need to fix invalid probes

[Calinou]

Testing project: test_dm3_open_sky.zip

Bake times comparison

OS: Fedora 38
CPU: Intel Core i9-13900K
GPU: GeForce RTX 4090

Using an optimized editor build (optimize=speed lto=full). Project uses 3 bounces in LightmapGI, and has Generate Probes subdiv set to 8 (the default). Denoiser is disabled. Project settings were left at their defaults.

I can confirm light probes now have the correct brightness level with this PR.

Unfortunately, I can't replicate any bake performance improvements with this PR:

Quality	Before	After (this PR)	After (this PR, tripled ray counts)
High	9 seconds	9 seconds	19 seconds
Ultra	25 seconds	25 seconds	68 seconds

Increasing number of bounces to 10 then baking lightmaps also makes the result much brighter (I double-checked my build is on commit 413bb09b5 of your branch):

The Bounce Indirect Energy property works correctly. Here it is set to 0.3 with 10 bounces:

Visual comparison

After screenshots were taken with the default ray count, which is currently the same as in master.

High

Before	After (this PR)

Before	After (this PR)

Before	After (this PR)

Before	After (this PR)

Ultra

Before	After (this PR)

Before	After (this PR)

Before	After (this PR)

Before	After (this PR)

[DarioSamo]

@Calinou I think the conclusion is pretty much the same as with @jcostello, if the surfaces are all white then I don't think the behavior will change, as 3.x only reduces the throughput of the ray by the albedo. That's pretty much why you're getting an increase in energy with each bounce, as it can't bounce against anything that isn't a white surface.

Godot 3.x's CPU Lightmapper:

godot/modules/lightmapper_cpu/lightmapper_cpu.cpp

Lines 923 to 925 in fc32e06

	color += throughput * sample.emission;
	throughput *= sample.albedo;
	color += throughput * sample.direct_light * parameters.bounce_indirect_energy;

Do we think it's right to introduce an attenuation factor on this PR to account for that? If we do we might want to make it user controllable for artistic reasons.

No change in performance is a bit odd, as there's definitely triple the set of compute dispatches in master although I guess you're getting three times as much rays traced from each one considering it can't attenuate over the distance nor can it hit the environment.

[clayjohn]

Only attenuating by albedo is definitely wrong, IMO we should use physically correct attenuation and rely on users setting indirect strength if they want to boost the impact of bounces.

[DarioSamo]

Only attenuating by albedo is definitely wrong, IMO we should use physically correct attenuation and rely on users setting indirect strength if they want to boost the impact of bounces.

I can't find much information that the attenuation itself is wrong in this case, as the materials being perfectly white means they're reflecting all light. A material that can do that and has no absorption whatsoever can't really exist.

The famous attenuation by a factor of PI applies if the distribution is uniform, but here it's cosine weighted so it cancels out (as the 3.x lightmapper points out).

I'm starting to think this is mostly an edge case but I'll see what I can find from other tracers on the topic.

[DarioSamo]

I tested out @Calinou's scene and I can confirm the performance improvements are definitely gone if the albedo is pure white and the light can't escape towards the sky.

Master: 00:53.
PR: 00:53.
PR albedo clamped to 90%: 00:52.
PR albedo clamped to 50%: 00:42.

Clamping the albedo also fixes the behavior of the bounces adding too much energy to the scene, although understandably even at 90% it still goes brighter until the bounce count is much higher. The fact the albedo represents a perfectly reflective material at full white is definitely a problem.

I've verified the implementation we have to match the behavior of other path tracers such as this one.

https://github.com/Esgrove/pathtracer/blob/431122fe3e06cc224257c40ed34f83582eec5d61/src/PathTraceRenderer.cpp#L393-L398

// p *= dot(N, V) * invPi; // cos(a) / pi

throughput *= diffuse;  // * invPi * dot(N, V); // BRDF * cos(a) = kd / pi * cos(a)

// trace ray
result = rt->raycast(O + 0.0001f * N, cameraCtrl.getFar() * V, false);

I don't think this testing methodology is representative of what most users will do and it's probably the only case where the previous incorrect algorithm will get an edge.

We could probably evaluate clamping the transfer from the albedo to a certain percentage to avoid falling in the case of a perfectly reflective material if deemed necessary.

[DarioSamo]

I ran another comparison to see if we can get a similar level of noise by just doubling the ray count without.

PR - 3 Bounces - Double Ray Count - Time to bake: 02:39

Master - 3 Bounces - Default Ray Count - Time to bake: 02:48

(It's worth considering noise is also less noticeable is the result is washed out)

Given the similar levels of time, I have to conclude whether the performance is an improvement or not depends on the following.

Master: Total Rays Traced = Ray Count X Bounces. The amount of time it'll take is determined exactly by this and the noise is averaged over Ray Count X Bounces.

PR: Total Rays Traced = Ray Count X Bounces Each Ray Can Reach. How far the bounces can go will depend significantly on the environment and the albedo properties of the surfaces, pure white being the worst case possible. The noise is averaged over Ray Count only.

Given the similar time each bake took, I think we can conclude "Bounces Each Ray Can Reach" averaged out to about 1.5x in this scenario (considering I used double the ray count).

It's probably fair to say Master's approach will result in less noisy scenes in faster times on edge cases where the Albedo is pretty much pure white, but I can't see a way of fixing its behavior as it is. On the contrary, I find largely improvements and the possibility of increasing bounce count even further much more achievable with the PR/3.X's approach.

[DarioSamo]

Is too simplistic to implement something like this for distance attenuation?

https://www.miniphysics.com/light-attenuation.html

I think it'd be too ineffective to model this based on the atmosphere being the transfer medium to matter.

A reasonable alternative might be to just clamp the max throughput based on the albedo to avoid modeling perfectly white materials. But I'm not sure how much it can matter as soon as you use a more realistic scenario than a stage with all white textures (not even Portal test chambers are that bright), the issue solves itself pretty quickly.

[DarioSamo]

I did everything in the TODO list so I think we're ready to take this out of drafts for further testing.

[jcostello]

Physical lights now works but I think its broken in master. Emission materials emite too much energy and omni and spot light dont event show (even without the lightmapper)

[jcostello]

@Calinou about your test, pure white materials wont decrease light energy on each bounce. We come up with a solution to cap the max energy per bounce to x 0.9 even to white materials. What do you think?

[Calinou]

I tested the latest revision from this PR (40bda87fd) and material albedo set to Color(0.9, 0.9, 0.9) on all materials in the scene:

All bakes are in High quality:

master (10 bounces, 20s bake)	This PR (10 bounces, 15s bake)

This PR (3 bounces, 11s bake)	This PR (10 bounces, 1.5× ray count, 21s bake)

@Calinou about your test, pure white materials wont decrease light energy on each bounce. We come up with a solution to cap the max energy per bounce to x 0.9 even to white materials. What do you think?

That makes sense. I think the difference in real world projects should be unnoticeable, but this situation is unlikely to be reached in the first place anyway.

Script to set material albedo on all materials in the scene (assign to the dm3_open_sky node):

@tool
extends Node3D

func _ready() -> void:
	var material := StandardMaterial3D.new()
	material.albedo_color = Color(0.9, 0.9, 0.9)
	# Match the fallback 3D material's parameters.
	material.roughness = 0.8
	material.metallic = 0.2
	for node in get_children():
		node.material_override = material

[JFonS]

Here are some notes regarding attenuation, mostly based on what I recall from working on the 3.x lightmapper:

• Distance attenuation (i.e. air scattering and/or absorbing light) should be pretty much negligible in normal conditions, but could be added for very large scenes or together with a "fogginess" property that can be tweaked.

• Typical "real-time" light attenuation (i.e. get_omni_attenuation) should only be applied in the direct lighting pass. This attenuation is modelling the fact that, as you get further away from a point light source, light "rays" coming out of it spread and you get less intensity at any given point. We can take this shortcut in the direct light pass because we know the exact shape of the light source and how the light's solid angle changes based on distance. We can't do the same for indirect bounces, but montecarlo integration should do its job there and achieve a similar effect.

• Pure white albedo is never good friends with realistic lighting. Adding a maximum value seems a bit arbitrary but could make sense if people keep stumbling on this.

[jcostello]

• Pure white albedo is never good friends with realistic lighting. Adding a maximum value seems a bit arbitrary but could make sense if people keep stumbling on this.

100% on this

[DarioSamo]

Here are some notes regarding attenuation, mostly based on what I recall from working on the 3.x lightmapper:

* Distance attenuation (i.e. air scattering and/or absorbing light) should be pretty much negligible in normal conditions, but could be added for very large scenes or together with a "fogginess" property that can be tweaked.

* Typical "real-time" light attenuation (i.e. `get_omni_attenuation`) should only be applied in the direct lighting pass. This attenuation is modelling the fact that, as you get further away from a point light source, light "rays" coming out of it spread and you get less intensity at any given point. We can take this shortcut in the direct light pass because we know the exact shape of the light source and how the light's solid angle changes based on distance. We can't do the same for indirect bounces, but montecarlo integration should do its job there and achieve a similar effect.

* Pure white albedo is never good friends with realistic lighting. Adding a maximum value seems a bit arbitrary but could make sense if people keep stumbling on this.

Thanks, I'd say these all match the current behavior.

[jcostello]

I assume those are physical omni lights?

First image is directional light, second image is a sphere with emissive material

[DarioSamo]

I assume those are physical omni lights?

First image is directional light, second image is a sphere with emissive material

Thanks, think you can upload the scene just in case I can't find the configuration that matches?

[jcostello]

Without physical lights

[jcostello]

MRP. Just bake the lights and the issue will be noticeable
Lightmap.zip

[clayjohn]

@jcostello That issue with physical lights, is it just in this PR, or is the issue present in master as well?

[jcostello]

@jcostello That issue with physical lights, is it just in this PR, or is the issue present in master as well?

In this PR. I just recheck with master. But it seems that master has problem with omni, spot light, and emission intensity

Emission, omni and spotlight almost not showing (probably by the lux diference?)

[DarioSamo]

I've added a new option to this PR that allows the indirect bounces to trace the lights individually. As expected, this increases the cost significantly and might even lead to TDRs if the region size is not lowered to compensate for it in a more expensive scene (due to the performance of the DDA scaling poorly).

What it does essentially is that during indirect bounces, instead of sampling the texture that was generated in the direct light pass, it'll choose to trace the lights in the scene again (if they're within range) to check if the hit point is lit or not. This does not provide any benefit if the texture was representative already of the result (e.g. not stretched a lot due to UVs), but it can potentially fix leaks as seen in this scenario.

I'm not entirely convinced about the naming of the option or how to explain it. The default behavior should be what we already have, and the option is mostly useful to fix light leaks in scenarios where the texture lightmaps aren't very accurate in giving a representation of what light reached the point being traced. They're ultimately a last resort for scenes like this.

I don't think most people would have a reason to use this path and it does lead to a significant increase in baking time even in my Sponza test with one light.

Use Texture for Bounces On (Default): Done baking lightmaps in 00:00:54.

Use Texture for Bounces Off: Done baking lightmaps in 00:01:20.

As expected in Sponza I see no visible difference, but it does fix some of the leaks in the other scenario I posted above due to the low resolution of the lightmap and the nature of the UVs.

[DarioSamo]

As a side note, this branch doesn't have the fix implemented by #75440 yet. Once that's done I'll rebase this one to have the two fixes I mentioned, as the leaks in the Unreal scene I posted above can't be fixed without both fixes applied.

[DarioSamo]

Physical lights still have problems.

Specially with emissive materials, they have way much power than without physical lights

This should be "fixed" now, as in, it does the same result that master has.

Normalization factor was not being taken into account for emissive light.

[jcostello]

Physical lights seems to be fixed for Directional, Omni and emissive materials but not for spotlights

[jcostello]

@DarioSamo I restest the last issue and seems it was something about the light energy. I amp the denoiser strenght and that fixed it

[jcostello]

With physical lights, dynamic objects dont receive light from the probes, maybe they dont capture enought light.

RGB

Physical

Also seems physical has much more energy in the indirect light (maybe its because de default values dont work the same)

[atirut-w]

Not sure if this is because my materials are fully white, but increasing bounces brighten up my scene a lot to the point of fullbright. The scene has a good amount of emissive materials acting as light sources.

Changing albedo to 0.9 0.9 0.9 still results in significant difference between 3 and 16 bounces.

3 bounces:

16 bounces:

MRP: https://github.com/godotengine/godot/files/12890164/UV2.Compression.zip

[DarioSamo]

Rebased to fix conflicts.

I asked @jcostello over DM and his latest problems seem to have been unrelated to the PR.

@atirut-w That is expected behavior from the 3.5 lightmapper as a white material represents something that is purely reflective of light. We discussed it a few posts above but I don't think we'll do anything in particular to address it as part of this PR yet.

I don't think it's worth addressing as no actual game scenario will rely on pure white albedo anyway when the behavior works as it should for the rest. That said you have the other indirect bounce energy controls to work around it if you'd want the bounces to be less intense.

[clayjohn]

Looks good to me! This has been tested pretty carefully and the changes have been discussed and fixed at length. I think it is time to merge!

For maintainers, this doesn't need to be cherrypicked as it is a pretty big change

[akien-mga]

Thanks!

[atirut-w]

For maintainers, this doesn't need to be cherrypicked as it is a pretty big change

Can the parts that fix #77169 and #77356 still be backported, though?