Add transparency support for LightmapGI

[Geometror]

Salvages #90109 (co-authored by @Guerro323)

Fixes https://github.com/godotengine/godot/issues/77590

I added the required changes for compatibility and adjusted the algorithm to the antialiasing of direct light samples that was introduced awhile ago.

Although unrelated to this PR (observable in master), there seems to be a regression/change of behavior that shadows of lights with size 0 appear much softer, depending on the texel scale. I know, a higher texel scale increases the resolution of the light map, but compared to the screenshots of the original PR it's definitely different. Maybe this new behavior is desired, because hard shadows can look bad on low-res light maps.

Note

(Copied from the original PR) The culling logic was updated in the Lightmapper to correctly support Back and Disabled cull modes. To preserve the current behavior and to not have any breaking change, the first direct light rays will retain the old behavior where light is blocked even if it should be culled. This behavior is still needed otherwise there would be holes in the shadows of penetrated meshes (which can be quite common) and would need the material to be updated to Disabled cull mode, which may not be intuitive in most cases.

TL;DR: There should be no breaking changes on existing scenes. This do need some more testing but all my scenes looked correct.

[clayjohn]

Although unrelated to this PR (observable in master), there seems to be a regression/change of behavior that shadows of lights with size 0 appear much softer, depending on the texel scale. I know, a higher texel scale increases the resolution of the light map, but compared to the screenshots of the original PR it's definitely different. Maybe this new behavior is desired, because hard shadows can look bad on low-res light maps.

That is from using bicubic sampling and shadow antialiasing https://godotengine.org/article/dev-snapshot-godot-4-4-dev-1/#lightmap-bicubic-sampling

[Geometror]

The issue that @Calinou found is tricky. So far I found out that the artifact only appears when the light direction and the surface are orthogonal to eachother. Rotating the directional light slightly removes it.

[Geometror]

Update: I could confirm that the original PR also had these issues. Furthermore, I found out the artifacts are gone when this part is commented out (trace_ray(...) in lm_compute.glsl):

if (icell != ivec3(hit_cell)) {
	// It's possible for the ray to hit a triangle in a position outside the bounds of the cell
	// if it's large enough to cover multiple ones. The hit must be ignored if this is the case.
	continue;
}

Doing that shouldn't be the solution, so maybe @DarioSamo could give some insight here, as I'm not that familiar with the lightmapper.

[DarioSamo]

Update: I could confirm that the original PR also had these issues. Furthermore, I found out the artifacts are gone when this part is commented out (trace_ray(...) in lm_compute.glsl):

if (icell != ivec3(hit_cell)) {
	// It's possible for the ray to hit a triangle in a position outside the bounds of the cell
	// if it's large enough to cover multiple ones. The hit must be ignored if this is the case.
	continue;
}

Doing that shouldn't be the solution, so maybe @DarioSamo could give some insight here, as I'm not that familiar with the lightmapper.

That code seems correct to me, I feel that'd probably indicate the DDA traversal is not reaching the cell it should. Large triangles can cover multiple cells, but the hit must be inside the cell picked by the DDA for it to be valid. It's not an easy area to debug I'm afraid.

I wonder if it could be falling on the edge case of a DDA cell? You mentioned it only happens if it's orthogonal, so perhaps it's only an issue because the DDA traversal is strictly following along the edge of one of the cells and never visiting the neighbours. You could try giving the if some tolerance as a hack to see if it's something along those lines.

[spookto]

Had a somewhat related issue which was a weird pattern being present when a directional light is only rotated on one axis and has shadow blur set to a value greater than 0 (the default is 1.0) The light in this setup has shadow blur set to 1.0, and rotated at (-45.0, 0.0, 0.0) Setting the blur to 0.0 removes the pattern

Another issue was light leaking when a point light had an integer position on the x-axis or z-axis, below is a picture where the light has a position of (0.0, 0.5, 2.0) Moving the light, with or without the mesh, to (0.015, 0.5, 2.015) seemed to remove the lines, any value below that exhibited some leaks but they got thinner as the value increased. Unlike the first issue, this one was not caused by shadow blur.

Tried giving it tolerances of 1.1, 1.414, 1.5, and 2.0, and found 2.0 to fix both issues faced. Not sure if 2.0 is too large to the point that it is equivalent to commenting out the code entirely. The code used was length(vec3(icell) - hit_cell) > 1.5 instead of icell != ivec3(hit_cell) Length > 1.5

Length > 2.0

Tested with Calinou's orthogonal MRP and a tolerance value of 1.4 and beyond seemed to fix the issue. Length > 1.3

Length > 1.5

[Geometror]

Update: Just for reference: The reason why this fails now is that trace_ray_any_hit was used before, which calls trace_ray with p_any_hit = true, returning RAY_ANY before the discussed optimization check. @spookto Thanks for doing those experiments. I came to a similar conclusion with the method shown below: 0.5 - which is still unexpectedly large to my understanding (since I don't know where the error comes from, it's too large to be caused by numerical instability/floating point precision errors).

vec3 position = p_from + dir * distance;
if (any(lessThan(position, cell_min)) ||
		any(greaterThan(position, cell_max))) {
	// It's possible for the ray to hit a triangle in a position outside the bounds of the cell
	// if it's large enough to cover multiple ones. The hit can be ignored if this is the case.
	continue;
}

with

const float CELL_EPSILON = 0.5;
const vec3 cell_min = bake_params.to_cell_offset + vec3(icell) / bake_params.to_cell_size - CELL_EPSILON;
const vec3 cell_max = bake_params.to_cell_offset + vec3(icell + 1) / bake_params.to_cell_size + CELL_EPSILON;

However, this broke again when increasing the bounds of the lightmap domain (by placing objects further away).

Regardless of the actual cause: This is a pure optimization and not needed for the lightmapper to work properly. And the operations that are skipped aren't that costly, so we might as well remove the branch entirely. In fact, my gut feeling was right, without this check the lightmap of my test scene baked in 26 seconds vs. 28 seconds before. The test scene consisted of many large and simple surfaces. After that, I tested it with the more complex global illumination demo (https://github.com/godotengine/godot-demo-projects), which showed a very similar improvement (218s -> 201s, 7-8% faster, release build).

So, removing it fixes all weird artifacts and actually improves performance, although we might conceal the underlying issue. I'm not sure how we should proceed at this point.

[badsectoracula]

I was testing this and there are two related issues i noticed:

1. When multiple translucent surfaces overlap they get "cancelled" (blacked/shadowed) out very easy
2. Even if the translucent areas have color, the shadows are still treated as grayscale

Fortunately both are easy to fix with the following patch, which essentially "mixes" both the shadow contribution and surface color from all hits and applies the color to the final light contribution:

diff --git a/modules/lightmapper_rd/lm_compute.glsl b/modules/lightmapper_rd/lm_compute.glsl
index 12291de6ea..0b91cc5445 100644
--- a/modules/lightmapper_rd/lm_compute.glsl
+++ b/modules/lightmapper_rd/lm_compute.glsl
@@ -474,6 +474,7 @@ void trace_direct_light(vec3 p_position, vec3 p_normal, uint p_light_index, bool
 	}
 
 	float penumbra = 0.0;
+	vec3 colorization = vec3(1.0);
 	if (false) {
 		const bool use_soft_shadows = (light_data.size > 0.0);
 		const uint ray_count = AA_SAMPLES;
@@ -625,7 +626,8 @@ void trace_direct_light(vec3 p_position, vec3 p_normal, uint p_light_index, bool
 				}
 
 				if (contribute) {
-					penumbra = max(penumbra - hit_albedo.a - EPSILON, 0.0);
+					colorization = mix(colorization, hit_albedo.rgb, hit_albedo.a);
+					penumbra *= 1.0 - hit_albedo.a;
 				}
 
 				p_position = hit_position + r_light_dir * bake_params.bias;
@@ -639,7 +641,7 @@ void trace_direct_light(vec3 p_position, vec3 p_normal, uint p_light_index, bool
 		penumbra = clamp(penumbra, 0.0, 1.0);
 	}
 
-	r_light = light_data.color * light_data.energy * attenuation * penumbra;
+	r_light = light_data.color * light_data.energy * attenuation * colorization * penumbra;
 }
 
 #endif

Technically this isn't physically correct (that'd require collecting all hits, sorting by the distance from the ray origin and doing alpha blending for each one of them) but it is still better than not taking colors into account. It can easily be seen when bounces are at 0 as shown below.

Before the patch without bounces:

(notice the black blob where the two shadows from the planes at the bottom middle intersect)

After the patch without bounces:

(notice that instead of a black blob where the two shadows from the planes at the bottom middle intersect now there is some colorized light that comes from mixing the reddish translucent surface with the yellow translucent surface)

It is also visible with bounces, though less so as the indirect lighting give the impression that there is some colorization going on.

Before the patch with bounces:

After the patch with bounces:

I noticed that the soft shadow code is disabled so i didn't touch it to add the change.

[jcostello]

Have you tested it with directional and with physically based lights?

[badsectoracula]

Have you tested it with directional and with physically based lights?

I didn't try directional lights but they seem to work fine:

As for physical lights, i never used those before so i just enabled them in the advanced settings. I may have done something wrong since they look more broken than nice but the colors do get taken into account during mixing:

For comparison this is how master (i.e. without this PR and the patch i wrote above) currently looks:

[guerro323]

@badsectoracula This is similar to how it was done in the original colored transparency shadow PR: https://github.com/godotengine/godot/pull/90132 They were separated at first since it was requested by the Godot devs (as the effects couldn't be replicated outside of the lightmapper unlike the grayscale shadows)

[atirut-w]

As for physical lights, i never used those before so i just enabled them in the advanced settings. I may have done something wrong since they look more broken than nice but the colors do get taken into account during mixing:

Adding a camera attribute to your LightmapGI node should fix it. It's an "expected behavior". See: #87531

[Geometror]

Originally this was supposed to be only a salvage of https://github.com/godotengine/godot/pull/90109, and I was planning to do the other one after that, but if we can agree on combining the functionality I'd be happy to do that. The main issue we need to decide on still remains: how we proceed with the DDA traversal fix.

[clayjohn]

Originally this was supposed to be only a salvage of #90109, and I was planning to do the other one after that, but if we can agree on combining the functionality I'd be happy to do that. The main issue we need to decide on still remains: how we proceed with the DDA traversal fix.

Let's keep this PR short and focused. We can add transparent color shortly after.

I would like to get @DarioSamo's input on the DDA traversal fix. If it is indeed just a small optimization, I have no problem removing it for now.

[DarioSamo]

@Geometror A lot of the performance optimizations I made were using the scene from here: https://github.com/godotengine/godot/issues/75440. If you find no regressions with that, then the optimization can go.

[DarioSamo]

Although we might conceal the underlying issue.

This also remains true, but I've reviewed the code that generates the triangle clusters and found no issues I could identify. I didn't code the DDA traversal myself either but it didn't look incorrect to me. Either of those elements could be wrong here and it's pretty tough to find out without a more exact testing setup on the CPU.

[spookto]

Trying the scene at #75440 seems impossible as the 'Plotting mesh into acceleration structure' step seems to freeze, however this freeze doesn't happen on v4.4dev6 and v4.3.

Also found that the console gets flooded with ERROR: servers/rendering/renderer_rd/storage_rd/material_storage.cpp:2040 - Parameter "shader" is null. in Forward+ and Mobile, and ERROR: drivers/gles3/storage/material_storage.cpp:2273 - Parameter "shader" is null. in Compatibility, when baking a scene with a MeshInstance3D that has surface materials, which is common on imported meshes. However the error messages don't seem to cause any visible issues in the baking result. Making the mesh unique and then clearing its surface_0 material seems to remove the error. Changing the mesh's override_material doesn't seem to change the error's behavior.

Issues I previously experienced have been fixed. Thanks a lot. :smile:

MRP for the error light_error_mrp.zip

[spookto]

Tried latest artifact. Errors don't print and it no longer freezes on the plotting step. Baking the sun temple looks lighter compared to 4.4dev6. There are also small light leaks in the corners seen in the global illumination demo.

[Geometror]

@spookto Thanks a lot for testing, but just to save you time, sometimes I do a push to sync the progress between my devices (which can be in a broken state). Usually I write a comment whenever meaningful progress was made.

Currently, I'm fighting with the issue you described in your last comment. The sun temple scene completely breaks when the cell check discussed above isn't in place (only after #85653 and so far I couldn't figure out why). There are also some additional light leaks along seams, so I'm going to mark this as draft since it clearly isn't ready and consumes a lot more time than originally anticipated.

With cell check/without before #85653 it looks like that (no lights enabled besides sun)

[Geometror]

I was finally able to fix all issues:

• The DDA algorithm had a small issue in the way the cells were traversed that caused rays parallel to and close to grid planes to miss. By fixing that, the discussed cell check could be kept without causing grid line artifacts.
• Fixed the light leaks along edges caused by incorrectly trying to get the shader data using the material RID (removed shader_get_cull_mode in favor of material_get_cull_mode)

As expected, baking of the sun temple scene is a bit slower (since the scene contains transparent vegetation), but not too bad. (90s -> 111s, default of max. 8 transparency rays) The original PR had a default of 64 transparency rays, but I think for most scenes that is way to high, so I changed it to 8.

Sun temple with texel scale set to 2

[atirut-w]

• Fixed the light leaks along edges caused by incorrectly trying to get the shader data using the material RID (removed shader_get_cull_mode in favor of material_get_cull_mode)

Is that an existing issue? I might be remembering wrong.

[Repiteo]

Thanks!