Fix LightmapGI dynamic object lighting

[DearthDev]

fixes #57890.
potentially fixes #57131 too.

[akien-mga]

The fix seems sensible looking at surrounding code, but please amend the commit message to be explicit. "fix" doesn't really cut it :)
https://github.com/godotengine/godot/blob/master/CONTRIBUTING.md#format-your-commit-messages-with-readability-in-mind

[DearthDev]

Thank you for reviewing the pull request. I appreciate your feedback and have amended the commit message to provide more clarity and explicitness regarding the changes made.

[akien-mga]

Looks good to me. I'll let @clayjohn double check as the rendering expert (especially in case this pattern of error might be found somewhere else).

[jcostello]

I tested in my project and is not quite working.

Sphere with UVs set to dynamic

Sphere with UVs set to static

[DearthDev]

Does it work if you move the object's position in the editor to force update it's lighting?

[clayjohn]

Change looks good to me. It seems the bug was introduced by #44623 Looking at other uses of lightmap_captures makes it clear that this particular usage is reversed.

Testing locally it seems that you need to toggle the global illumination mode to make sure that the dynamic object receives dynamic lighting. So there is still something missing before we can close the linked PRs

[jcostello]

Does it work if you move the object's position in the editor to force update it's lighting?

I move it several times and it didn't seems to be affected by the probes

[clayjohn]

Does it work if you move the object's position in the editor to force update it's lighting?

I move it several times and it didn't seems to be affected by the probes

See my comment above, I think something is wrong with the pairing code as well. I found that it worked after turning the GI mode to static and then back to dynamic. Also remember to bake with probes

[jcostello]

See my comment above, I think something is wrong with the pairing code as well. I found that it worked after turning the GI mode to static and then back to dynamic. Also remember to bake with probes

I already baked it with probes. I tried turning the Global Illumination mode from static to dynamic several times (dynamic still said only VoxelGI) and still I have the result of the pictures. Also I disabled and re enabled the LightmapGI node. Do I miss something?

[clayjohn]

Does it work if you move the object's position in the editor to force update it's lighting?

I move it several times and it didn't seems to be affected by the probes

See my comment above, I think something is wrong with the pairing code as well. I found that it worked after turning the GI mode to static and then back to dynamic. Also remember to bake with probes

I already baked it with probes. I tried turning the Global Illumination mode from static to dynamic several times (dynamic still said only VoxelGI) and still I have the result of the pictures. Also I disabled and re enabled the LightmapGI node. Do I miss something?

I found that hiding the LightmapGI node broke the pairing. For some reason the LightmapGI node isn't properly propagating the dirty flag to GeometryInstances.

[DearthDev]

I already baked it with probes. I tried turning the Global Illumination mode from static to dynamic several times (dynamic still said only VoxelGI) and still I have the result of the pictures. Also I disabled and re enabled the LightmapGI node. Do I miss something?

If you save the scene and reopen it will that get them to pair? I can't reproduce your issue. I'll look into fixing the pairing issue in a separate PR.

[jcostello]

If you save the scene and reopen it will that get them to pair? I can't reproduce your issue. I'll look into fixing the pairing issue in a separate PR.

I can push my project so you can check the issue

[jcostello]

@DearthDev https://github.com/jcostello/SponzaHD

Here you are

[Calinou]

PS: Please do not nest quotes in replies as it makes the thread difficult to read. Only quote the exact text you are replying to. (Unfortunately, GitHub won't do this for you, so you have to edit the quoted text manually.)

[DearthDev]

@DearthDev https://github.com/jcostello/SponzaHD

Here you are

Upon reviewing the code in the debugger, everything appeared to be functioning properly. To troubleshoot the issue, I attempted to rebuild the light map. Subsequently, my screen displayed unexpected behavior and the Godot became unresponsive.

After realizing that there were errors related to VoxelGI in the scene, I suspected that the scene might have been corrupted or damaged. Again, I think it's an issue with pairing. Then, I tried creating a new scene and baking a lightmap with all the same meshes. The dynamic lighting worked perfectly, However, I did observe some visual errors, which I attributed to the process of copying the directional light and world environment from the previous scene. It seemed that some settings in the world environment or camera attributes, such as exposure, were causing issues.

The visual errors are unrelated to lightmap captures and resetting all the world environment settings and camera attributes settings fix all the visual errors.

[jcostello]

@DearthDev have you been able to get lighting from the probes?

[DearthDev]

@DearthDev have you been able to get lighting from the probes?

Yes, after making the new scene it worked fine.

[jcostello]

@DearthDev I recreated the scene and I notice the problem might be with reflection probes. I can't figure it out but when reflection probes are enabled in order for a dynamic mesh to have the lightmap probes light I have to enable the mesh first before the reflection probes. Can you try that?

[jcostello]

When playing the scene, objects are not receiving dynamic light (No reflection probe in this case to avoid the issue above)

Edit: Moving the object after lauching the game applies the dynamic but if the object doesn't move at all it will receive no light until moves. It should be applied even if the object didn't move yet

Playing

Editor

[jcostello]

I tried it on the TPS demo and in some cases it samples from the invalid probes inside meshes. We should avoid those probes

Close invalid probe

Far from invalid probe

[jcostello]

scene/3d/lightmap_gi.cpp:558 - Inconsistency found in triangulation while building BSP, probe interpolation quality may degrade a bit.

Message while baking. IDK if its relevant

[DearthDev]

I've ran into that error in my personal projects.
Should these be opened as issues?

We know:

1. LightmapSH and geom isn't getting paired upon baking
2. Inconsistency found in triangulation while building BSP error
3. Invalid probes

[jcostello]

I've ran into that error in my personal projects. Should these be opened as issues?

We know:

1. LightmapSH and geom isn't getting paired upon baking
2. Inconsistency found in triangulation while building BSP error
3. Invalid probes

To me, those issues are related to this preventing dynamic light to work so could be fix on this PR. But lets hear @Calinou opinion

[clayjohn]

My preference is to create the 3 suggested issues then merge this as-is. We shouldn't block a perfectly good bug fix just because it doesn't fix all bugs with the feature it touches. That is too much to expect from a contributor

[DearthDev]

My preference is to create the 3 suggested issues then merge this as-is. We shouldn't block a perfectly good bug fix just because it doesn't fix all bugs with the feature it touches. That is too much to expect from a contributor

I agree and then we can have any discussion pertaining to each issue separately.

[akien-mga]

Thanks! And congrats for your first merged Godot contribution 🎉

[jcostello]

Congratz @DearthDev 😄

BTW can you create the issues for

1. LightmapSH and geom isn't getting paired upon baking
2. Inconsistency found in triangulation while building BSP error

I will create for the invalid probes that I have more information about it

[YuriSizov]

Cherry-picked for 4.0.4.