godotengine / godot

Godot Engine ā€“ Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.79k stars 20.13k forks source link

Lightmap Probes applying to dynamic objects offset by LightmapGI node orientation. #94824

Open Braxtogoo opened 1 month ago

Braxtogoo commented 1 month ago

Tested versions

System information

Godot v4.3.rc1 - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3060 (NVIDIA; 31.0.15.3742) - AMD Ryzen 9 5900X 12-Core Processor (24 Threads)

Issue description

When using lightmap probes they apply based on the LightmapGI node's rotation and position. This leads to odd behavior if your node is rotated in odd directions. And can allow for baked lighting to be "moved" or "spun" in run time. The baking doesn't seem to be affected by this but how the effect is applied is. Below are some examples of the light probes applying their effect to dynamic objects while the LightmapGI node is rotated to be upside down and backwards.

Examples: Two emissive cubes, the sphere in the middle is dynamic: image

Note that the over kill amount of light probes show the light on the correct sides. image

Another example of a sphere with an emissive cube and a static spotlight, it happens when the spot light is baked dynamic as well. (Note that I turned off the spot light's direct light to make the effect more apparent.) image

Light probes once again, look correct. image

Finally, here's an example showing that it happens with regular lights as well, here's the sphere centered under the spot light with the direct light turned off, as you can see, based on the light probes it should be getting more indirect lighting from below, however even without the direct light it appears brighter on top. image

Probes: image

You can even spin the lighting in real time! Here I've centered the LightmapGI node on the sphere and I rotate it with the editor. Moving the position of the node will have similar results. output2

Steps to reproduce

Minimal reproduction project (MRP)

I did also add a camera to the scene to make sure it wasn't just an editor only bug. Hitting play also shows the bug, however I had to wiggle the sphere with the remote inspector panel to get the lightmap probe to apply to the sphere, this feels like a separate bug, I've reported it here #94825.

Backwards_Lightmap_Probes.zip

BlueCube3310 commented 1 month ago

https://github.com/godotengine/godot/blob/607b230ffe120b2757c56bd3d52a7a0d4e502cfe/servers/rendering/renderer_rd/shaders/forward_clustered/scene_forward_clustered.glsl#L1414

The issue lies probably in either the inverse matrix or normal, the formula for SH apppears to be correct. The probe gizmos apply the same formula directly to the vertex colors, which is why they look correct.

Braxtogoo commented 1 month ago

https://github.com/godotengine/godot/blob/607b230ffe120b2757c56bd3d52a7a0d4e502cfe/servers/rendering/renderer_rd/shaders/forward_clustered/scene_forward_clustered.glsl#L1414

The issue lies probably in either the inverse matrix or normal, the formula for SH apppears to be correct. The probe gizmos apply the same formula directly to the vertex colors, which is why they look correct.

You'll have to forgive me if this line of thinking is incorrect, shader programming is not my specialty.

I have tried flipping the normals of the model (turning it inside out), and this does invert the colors each face receives (as expected) however that is also incorrect behavior as now the faces are facing the opposite direction as well. Here's a gif showing that the light a flipped model receives is still backwards. output

Obviously if you flipped the normals just inside the code only for the lightmap probe calculation it would likely work correctly, but I would assume that your other guess involving the inverse matrix (which I don't know much about) is probably closer to the answer... unless the normals are being flipped erroneously inside the shader somewhere.

BlueCube3310 commented 1 month ago

I would assume that your other guess involving the inverse matrix (which I don't know much about) is probably closer to the answer... unless the normals are being flipped erroneously inside the shader somewhere.

The inverse matrix is used to transform the view matrix into the world matrix. The same logic is used elsewhere in the shader, so that appears to be correct.

SpockBauru commented 1 month ago

This issue is really interesting, it seems that the rotation on LightmapGI node have an influence on Dynamic objects.

Here is the LightmapGI transform on @Braxtogoo's MRP:

image

Now if I reset the rotation and bake again, everything is fixed:

image

(For some reason I'm chuckling on this weird issue šŸ˜„)

Calinou commented 1 month ago

It might be a local/global space issue in the shader then.

BlueCube3310 commented 1 month ago

The lightmapper appears to treat the LightmapGI's transform as the world basis for generating rays, which is mitigated by the directional lightmaps which multiply the normal by the lightmapGI's transform. The probes are never corrected in this way however, so it is always assumed that they were baked with a global transform.

Braxtogoo commented 1 month ago

That's wild, and now I'm trying to think of any useful applications for this behavior. šŸ˜… Can't think of any yet but regardless these are interesting findings. I don't even remember rotating the LightmapGI node, wonder why I did that, regardless I'll update the steps to reproduce and title. Awesome find @SpockBauru!

Braxtogoo commented 1 month ago

Interesting update, while updating the description to be more accurate and getting footage for the new gif, I found something else. Moving the position of the LightmapGI node doesn't affect the static lighting, but rotating it does. I'll check if there's already a bug report but this feels unrelated, so I may make a separate issue for it as this doesn't seem intended. output3

Braxtogoo commented 1 month ago

There is a tiny part of me that wants both of these behaviors preserved because if you had a spherical room full of dynamic objects you could spin the LightmapGI probe to make it look like the baked lights were moving around the room. In fact, if the static lighting also reacted to the position of the node like the dynamic probes do you could have moving objects with baked lighting, which could be useful for things like spaceship interiors... hmmmmm...

Edit: Static lighting actually should not be affected by the position and rotation of the LightmapGI probe because the geo can already be moved and it will preserve the lighting, so maybe if the weird rotation issue of the static lighting were fixed then the effect of the position and rotation of the probes would actually be a feature, in fact, now that I think about it, this probably was intentional, as you would want the position and rotation of the light probes to be taken into consideration when loading scenes into other scenes, maybe the only bug here is the static lighting being affected by the rotation. If this is made to be the way I've described it would actually give the lightmapping system in Godot a pretty major leg up on the default lightmapper in Unity which is much more rigid in how it can be used, not sure if there are performance losses though.

I'll hold off on making a new issue until others on this thread weigh in.

Edit2: Also, if this is intended (or becomes intended), it should probably be made more intuitive or documented clearly.