Fix directional LightmapGI being too dark with static lights #61910

Calinou · 2022-06-10T17:42:44Z

The brightness now matches dynamic lights (indirect light baked only) when Directional is enabled. Thanks @techiepriyansh for the suggestion 🙂

The brightness of lightmapping for lights marked as dynamic is unchanged by this PR.

I ended up using an even stronger multiplier ((24 / 3.0) = 8.0). Static directional lightmapping now matches dynamic directional lightmapping exactly.

This closes #49936.

Preview

Dynamic

No Directional	With Directional

Static

No Directional	With Directional (before PR)	With Directional (after PR)

clayjohn · 2022-06-12T20:01:30Z

I think we need to dig a little deeper into the meaning behind those coefficients. The lighting still doesn't match. In the image you posted you can see that with this PR the faces facing left are significantly brighter than when using dynamic lights. To me this indicates that the problem lies somewhere else and we need to do the work to uncover what the problem is a fix it at source.

CC @JFonS

jcostello · 2022-06-16T04:43:52Z

Are the normals backed ni the right direction?

No lightmap

With Lightmap

Calinou · 2022-06-16T15:08:17Z

@jcostello This is an unrelated issue. Please open a new issue with a minimal reproduction project attached.

atirut-w · 2022-12-26T09:48:35Z

Static Directional seems to be just slightly brighter than Dynamic Directional.

clayjohn · 2023-02-13T20:03:49Z

Bumping to 4.x. I don't think this is the right fix and I don't want to introduce arbitrary scaling factors into the codebase without first investigating why they may be needed

jcostello · 2023-02-14T14:53:21Z

Without this we get this from

RealTime

To static bake (non-directional. Directional is still broken after the camera attributes changes)

atirut-w · 2023-09-19T01:24:07Z

Is this PR dead? Or is it superseeded?

jcostello · 2023-09-19T01:32:42Z

I belive directional light have to be fixed since beside it bakes darker, it has artifacts

DarioSamo · 2023-09-19T17:30:51Z

I'd like to clarify at first I haven't exactly gotten the entire grasp of how the spherical harmonics encoding works, but I believe I have enough to justify this PR's choice of the 8.0 value enough so the fix is merged.

The two chunks of relevant code to evaluate here are:

godot/modules/lightmapper_rd/lm_compute.glsl

Lines 408 to 420 in 571cd0e

    
           #ifdef USE_SH_LIGHTMAPS 
        
           			float c[4] = float[]( 
        
           					0.282095, //l0 
        
           					0.488603 * light_dir.y, //l1n1 
        
           					0.488603 * light_dir.z, //l1n0 
        
           					0.488603 * light_dir.x //l1p1 
        
           			); 
        
           			for (uint j = 0; j < 4; j++) { 
        
           				sh_accum[j].rgb += light * c[j] * (1.0 / 3.0); 
        
           			} 
        
           #endif

godot/modules/lightmapper_rd/lm_compute.glsl

Lines 516 to 528 in 571cd0e

    
           #ifdef USE_SH_LIGHTMAPS 
        
           		float c[4] = float[]( 
        
           				0.282095, //l0 
        
           				0.488603 * ray_dir.y, //l1n1 
        
           				0.488603 * ray_dir.z, //l1n0 
        
           				0.488603 * ray_dir.x //l1p1 
        
           		); 
        
           		for (uint j = 0; j < 4; j++) { 
        
           			sh_accum[j].rgb += light * c[j] * (8.0 / float(params.ray_count)); 
        
           		} 
        
           #endif

The first chunk belongs to the direct light computation step of the lightmapper. This is fairly straightforward: rays are not casted at random but rather traced directly to the light sources. The result is added with this coefficient of (1.0 / 3.0) along with the multipliers used for each component of the spherical harmonics.

The second chunk belongs to the indirect light (bounces) step of the lightmapper. Here an the average result is computed from rays traced at random, which is why it opts to multiply the additional light divided by params.ray_count. At first I thought it was a bit odd how it didn't match the behavior of the accumulated bounce light, but as it turns out there's an error in how the behavior for that works (which I'll leave for a different issue). The short version is, the division on this one is the intended result at least to average out all the light computed from all the rays.

That indicates however the resulting indirect light is added with a coefficient of 8.0, which doesn't match the (1.0 / 3.0) that was found before for the direct light step. In fact, the intensity is far stronger for the indirect component than the direct one. This PR is justified to use the 8.0 value as it comes directly from the other step, which brings them to the same range in intensity as they should be (and it's how the result from both direct and bounce light are added together when not using spherical harmonics).

However, there's likely some underlying meaning to these coefficients chosen so I'll be taking a look at the resources @reduz used for implementing this to try to understand their origin better. However, as far as fixing things in the short term, I think we're good to merge this PR for now. As it stands there's some other fundamental core issues with the lightmapper that will take a longer time to fix and it'd be preferrable to at least make this feature not broken for now.

DarioSamo · 2023-09-19T17:37:12Z

As an addendum, I'd probably take out the comment entirely as there's no need to do this empirically, as the same constant can be found elsewhere in the code or link back to this PR instead.

modules/lightmapper_rd/lm_compute.glsl

The brightness now matches dynamic lights (indirect light baked only) when Directional is enabled. Co-authored-by: Priyansh Rathi <techiepriyansh@gmail.com>

Calinou · 2023-09-21T16:09:04Z

Rebased and tested again, it works as expected:

Directional OFF	Directional ON (`master`)	Directional ON (this PR)

Directional OFF	Directional ON (this PR)

Directional OFF	Directional ON (this PR)

Testing project: test_lightmap_preview_bake_4.x_2.zip

jcostello · 2023-09-21T18:38:36Z

Seems that the directional bake introduces artifacts because fo the normal calculations. I can't remember if there is an issue for that

clayjohn · 2023-09-21T21:33:39Z

Seems that the directional bake introduces artifacts because fo the normal calculations. I can't remember if there is an issue for that

The picture you posted above does indeed look pretty buggy. If there isn't already a bug report, can you open one?

clayjohn

Looks good to me given the discussion in the PR!

I didn't flag for cherrypicking as it will result in a very obvious physical change

Calinou · 2023-09-21T21:56:24Z

The picture you posted above does indeed look pretty buggy. If there isn't already a bug report, can you open one?

To be fair, the imported scene's normals themselves are weird (likely due to Godot's OBJ importer - I think the source OBJ file itself is fine). Directional lightmaps just make that issue more noticeable.

jcostello · 2023-09-21T23:31:51Z

Seems that the directional bake introduces artifacts because fo the normal calculations. I can't remember if there is an issue for that

The picture you posted above does indeed look pretty buggy. If there isn't already a bug report, can you open one?

I will open a Issue. Directional generate this weird alien like artifacts

atirut-w · 2023-09-22T14:21:24Z

Testing project: test_lightmap_preview_bake_4.x_2.zip

If you don't mind, where did you get the original model?

Calinou · 2023-09-22T17:01:37Z

If you don't mind, where did you get the original model?

https://github.com/Calinou/game-maps-obj/blob/master/redeclipse/octavus.obj

akien-mga · 2023-09-22T20:08:19Z

Thanks!

atirut-w · 2023-09-25T04:48:12Z

Directional generate this weird alien like artifacts

Have you opened the issue yet? If not, I'll open it, as the issue seem to have also cropped up in the Abandoned Spaceship demo.

jcostello · 2023-09-25T13:30:24Z

@atirut-w I didnt yet. If you can open up an issue for me. I tried to replicate it in a small MRP but I couldn't. It only happen to me in bigger scenes like sponza

Calinou · 2023-09-25T15:45:20Z

Please continue the discussion about the bug in #82307.

Calinou requested a review from a team as a code owner June 10, 2022 17:42

Calinou added bug topic:rendering topic:3d labels Jun 10, 2022

Calinou added this to the 4.0 milestone Jun 10, 2022

clayjohn requested a review from JFonS June 12, 2022 19:57

Calinou mentioned this pull request Jul 15, 2022

Use directional LightmapGI information to display specular lobes for fully baked (static) lights #63016

Open

clayjohn modified the milestones: 4.0, 4.x Feb 13, 2023

This comment was marked as off-topic.

Sign in to view

clayjohn modified the milestones: 4.x, 4.2 Sep 21, 2023

clayjohn reviewed Sep 21, 2023

View reviewed changes

modules/lightmapper_rd/lm_compute.glsl Outdated Show resolved Hide resolved

Fix directional LightmapGI being too dark with static lights

7831eed

The brightness now matches dynamic lights (indirect light baked only) when Directional is enabled. Co-authored-by: Priyansh Rathi <techiepriyansh@gmail.com>

Calinou force-pushed the lightmapgi-static-directional-fix-brightness branch from b543bba to 7831eed Compare September 21, 2023 16:04

clayjohn approved these changes Sep 21, 2023

View reviewed changes

akien-mga merged commit 6094738 into godotengine:master Sep 22, 2023
16 checks passed

Calinou deleted the lightmapgi-static-directional-fix-brightness branch September 22, 2023 20:26

Calinou mentioned this pull request Aug 20, 2024

LightmapGI Directional results in more aliased shadows for lights with Static bake mode #88470

Closed

clayjohn mentioned this pull request Aug 21, 2024

Use correct lightmap coefficients to ensure that the directional lightmap mode looks correct #95888

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix directional LightmapGI being too dark with static lights #61910

Fix directional LightmapGI being too dark with static lights #61910

Calinou commented Jun 10, 2022 •

edited

Loading

clayjohn commented Jun 12, 2022

jcostello commented Jun 16, 2022 •

edited

Loading

Calinou commented Jun 16, 2022

atirut-w commented Dec 26, 2022

clayjohn commented Feb 13, 2023

This comment was marked as off-topic.

This comment was marked as off-topic.

jcostello commented Feb 14, 2023

atirut-w commented Sep 19, 2023

jcostello commented Sep 19, 2023

DarioSamo commented Sep 19, 2023

DarioSamo commented Sep 19, 2023 •

edited

Loading

Calinou commented Sep 21, 2023 •

edited

Loading

jcostello commented Sep 21, 2023

clayjohn commented Sep 21, 2023

clayjohn left a comment •

edited

Loading

Calinou commented Sep 21, 2023

jcostello commented Sep 21, 2023

atirut-w commented Sep 22, 2023

Calinou commented Sep 22, 2023

akien-mga commented Sep 22, 2023

atirut-w commented Sep 25, 2023

jcostello commented Sep 25, 2023

Calinou commented Sep 25, 2023

Fix directional LightmapGI being too dark with static lights #61910

Fix directional LightmapGI being too dark with static lights #61910

Conversation

Calinou commented Jun 10, 2022 • edited Loading

Preview

Dynamic

Static

clayjohn commented Jun 12, 2022

jcostello commented Jun 16, 2022 • edited Loading

Calinou commented Jun 16, 2022

atirut-w commented Dec 26, 2022

clayjohn commented Feb 13, 2023

This comment was marked as off-topic.

This comment was marked as off-topic.

jcostello commented Feb 14, 2023

atirut-w commented Sep 19, 2023

jcostello commented Sep 19, 2023

DarioSamo commented Sep 19, 2023

DarioSamo commented Sep 19, 2023 • edited Loading

Calinou commented Sep 21, 2023 • edited Loading

jcostello commented Sep 21, 2023

clayjohn commented Sep 21, 2023

clayjohn left a comment • edited Loading

Choose a reason for hiding this comment

Calinou commented Sep 21, 2023

jcostello commented Sep 21, 2023

atirut-w commented Sep 22, 2023

Calinou commented Sep 22, 2023

akien-mga commented Sep 22, 2023

atirut-w commented Sep 25, 2023

jcostello commented Sep 25, 2023

Calinou commented Sep 25, 2023

Calinou commented Jun 10, 2022 •

edited

Loading

jcostello commented Jun 16, 2022 •

edited

Loading

DarioSamo commented Sep 19, 2023 •

edited

Loading

Calinou commented Sep 21, 2023 •

edited

Loading

clayjohn left a comment •

edited

Loading