Disabled Lights Are Included in Lightmap

[sandsalamand]

Godot version

4.1.2 stable mono

System information

Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2070 SUPER (NVIDIA; 30.0.15.1123) - Intel(R) Core(TM) i7-9700K CPU @ 3.60GHz (8 Threads)

Issue description

Invisible lights (tested with OmniLight3D and SpotLight3D) are included in the baked lightmap.

Expected: disabled/invisible lights are not included in the lightmap.

Steps to reproduce

1. Place an OmniLight3D or SpotLight3D in the scene.
2. Nest it below a LightMapGI
3. Disable the Light
4. Place a MeshInstance3D in the scene (in my case, saved as a scene, but not sure if that matters).
5. Bake. Lightmap settings don't seem to change anything, but my last test used Low + 3 Bounces + Directional ON + Interior OFF + Use Denoiser ON.

Minimal reproduction project

N/A

[jsjtxietian]

Hi, by invisible lights do you mean toggle visibility at scene hierachy or change the bake mode to disabled? If the first situation, the doc said "Keep in mind hiding a light will have no effect for baking, so this must be used instead of hiding the Light node." https://docs.godotengine.org/en/latest/tutorials/3d/global_illumination/using_lightmap_gi.html

However, I do think invisible light in the scene hierarchy should not contribute to lightmap baking.

[Calinou]

• Related to https://github.com/godotengine/godot/issues/56611.

This may be done by design, so that you can maximize performance by having lights that only affect baked surfaces with no real-time computation. That said, https://github.com/godotengine/godot/issues/73289 is probably a better solution as it'll also affect direct light in LightmapProbes.

[jsjtxietian]

Well I would slightly argue that it's not intuitive that invisible light is also contributing to lightmap baking. Other than this it's fine.

At least support hidding light when baking is trivial. If @sandsalamand you really need this, I think just add a condition here for a quick fix: https://github.com/godotengine/godot/blob/d31794c4a26e5e10fc30c34a1ae9722fd9f50123/scene/3d/lightmap_gi.cpp#L400

[sandsalamand]

@Calinou So from reading that issue, are you saying that Static Baked lights would be completely invisible in the editor until baked? And thus, they would solve the use case of wanting lights that are invisible in the editor but included in the lightmap?

If so, I agree. If that option was available, then we could safely change it so that invisible dynamic lights are not included in the lightmap.

[sandsalamand]

@jsjtxietian It's not essential right now for my project, but it's definitely unintuitive behavior that should (IMO) be changed.

I read that documentation page, but I didn't notice that part. Maybe it's because of the use of the word "hidden" instead of "invisible". Also, if we're keeping the current behavior, it should probably be in a warning box to call more attention to it.

[Calinou]

@Calinou So from reading that issue, are you saying that Static Baked lights would be completely invisible in the editor until baked? And thus, they would solve the use case of wanting lights that are invisible in the editor but included in the lightmap?

This would be an additional bake mode for the reasons I described in https://github.com/godotengine/godot/issues/73289#issuecomment-1430259719. The existing Static bake mode would not be affected by this change, as it still has some use cases (such as more precise direct lighting for direct objects, and the ability for them to receive real-time shadows from baked objects).

[sandsalamand]

Yes, I understand. So, should we add the fix which @jsjtxietian mentioned to your fork?

[Calinou]

Yes, I understand. So, should we add the fix which @jsjtxietian mentioned to your fork?

I would prefer we make the changes separately, as it has important implications. My branch is still far from being finished (and I don't know if I'll be able to complete it) anyway.

[atirut-w]

This is very similar to a problem I encountered where editor-only lights are included in lightmaps. Could it be the same issue as this?

[Calinou]

This is very similar to a problem I encountered where editor-only lights are included in lightmaps. Could it be the same issue as this?

This can likely be the fixed the same way in the lightmapper's light gathering code. However, unlike hidden lights, I consider including editor-only lights in the bake a bug for sure.

[reptofrog]

I have attached a reproduction project, I hope it could be useful.

Lightmap Bug.zip

[sandsalamand]

@reptofrog Are you a bot?

[Calinou]

@reptofrog Are you a bot?

No, I can attest they are a real person 🙂

[sandsalamand]

@reptofrog Are you a bot?

No, I can attest they are a real person 🙂

Ok, it just seemed weird because they randomly posted a picture and a .zip file without any context.

[reptofrog]

@sandsalamand It's a reproduction project that demonstrates how disabled lights are included in the lightmap 😅

[Zireael07]

@sandsalamand The post literally says "a reproduction project" above the picture and zip

[sandsalamand]

@Zireael07 Right, but Calinou expressed doubt that this behavior is actually a bug, stating that it might be intentional that invisible lights are included. So it seemed weird for reptofrog to upload a file called "Lightmap Bug".