Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vulkan Mobile: LightmapGI sampling is not disabled on lights with Static bake mode, causing double lighting to appear #85145

Open
Calinou opened this issue Nov 20, 2023 · 3 comments

Comments

@Calinou
Copy link
Member

Calinou commented Nov 20, 2023

Godot version

4.2.beta1 (first version to be able to sample lightmaps correctly with Mobile), 4.2.rc1

System information

Fedora 38, GeForce RTX 4090 (NVIDIA 535.129.03)

Issue description

LightmapGI sampling is not disabled on lightmapped meshes with Static bake mode, causing double lighting to appear:

Forward+ Mobile
Screenshot_20231120_175447 Screenshot_20231120_175831

The dynamic object rendering issue is tracked in #84846.

Steps to reproduce

  • Create meshes with Static bake mode that have UV2 (you can use Add UV2 in PrimitiveMesh for this).
  • Add some lights, enable shadows on them and set their bake mode to Static.
  • Bake lightmaps and look at the scene's appearance.
  • Switch to the Mobile rendering method and compare visuals.

Minimal reproduction project

test_lightmapgi_mobile.zip

@clayjohn
Copy link
Member

clayjohn commented Sep 6, 2024

To clarify, the problem here is that the light is still drawn dynamically despite using the static bake mode right? If a light is statically baked, my expectation is that only the lightmap will be used. I wouldn't expect the lightmap to be disabled.

I noticed while working on something else that the dynamic light is added to the lightmap even when the light is statically baked

@Calinou
Copy link
Member Author

Calinou commented Sep 6, 2024

To clarify, the problem here is that the light is still drawn dynamically despite using the static bake mode right? If a light is statically baked, my expectation is that only the lightmap will be used. I wouldn't expect the lightmap to be disabled.

Yes, I expect it to render like in Forward+ and Compatibility where the real-time light is skipped on the lightmapped surface (since it's already been baked).

This does not apply to surfaces that don't have a lightmap due to not having been baked (or not having UV2 generated), even if the light uses the Static bake mode.

@clayjohn
Copy link
Member

clayjohn commented Sep 6, 2024

To be clear, in the Forward+ backend, the dynamic light is still added on lightmapped surfaces. So this issue must exist in both backends.

Forward+ also has an unitialized memory error because the bake mode is only passed to the shader if the light is using shadows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants