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

LightmapGI: Lightmap static lights are still counted in real-time light calculations #73289

Closed
Tracked by #56033
jcostello opened this issue Feb 14, 2023 · 6 comments
Closed
Tracked by #56033

Comments

@jcostello
Copy link
Contributor

Godot version

v4.0.beta17.official [c400205]

System information

Ubuntu with Nvidia 2060

Issue description

Lights statically baked are still count into light calculation. Shouldn't be disabled by default? Dinamic or not baked objects should be lighted by lightmaps probes (not working yet)

static light enabled
image

static light disabled (Notice the FPS increase when disable it)
image

Steps to reproduce

Bake a scene with static light enabled
Disabled light
See diference in FPS

Minimal reproduction project

No needed

@Calinou
Copy link
Member

Calinou commented Feb 14, 2023

Dynamic objects are still lit in real-time by lights marked as static, so a performance impact is expected for static lights.

What we should do is add an option to make lights marked as static be fully baked into probes for dynamic object lighting. Currently, only their indirect lighting is baked – this option would make it so their direct light is also baked. This may warrant adding a fourth GI mode for lights such as "Fully Static", as this option may also be relevant for VoxelGI to a lesser extent.

This needs to be an option that is disabled by default, as direct light using probes is far less accurate than real-time lighting. You can't have precise shadows cast onto dynamic objects this way (the fully baked static mesh keeps casting shadows for other dynamic lights by design).

@Calinou Calinou changed the title Lightmap static lights are still count into light calculation unnecesary LightmapGI: Lightmap static lights are still counted in real-time light calculations Feb 14, 2023
@Calinou
Copy link
Member

Calinou commented Sep 28, 2023

I have a proof of concept: https://github.com/Calinou/godot/tree/light3d-add-static-baked-mode

Testing project: test_light3d_baked_static_mode.zip

It has some issues:

  • It crashes when you use Scene > Reload Saved Scene due to the culling code having some "uneven" cases with lights sometimes being paired, sometimes not. The goal here is to have the Static Baked lights never added to the list of instances to render. Only the lightmapper (and perhaps the VoxelGI baker in the future) should bother about these nodes.
  • Toggling visibility of a Static Baked light breaks lightmap rendering, likely due to an error in my culling code changes.
  • Lightmap probes don't take direct light into account for these Static Baked lights yet.

However, I was able to get a successful bake with lightmaps having the Static Baked lights present, yet dynamic objects not being lit by these lights in real-time at all.

Light3D bake modes from left to right: Disabled, Static, Dynamic, Static Baked. The white box is a static (baked) object, while the cyan torus is a dynamic object.

image

I'd appreciate help on this area as I'm not sure how to modify the lightmap probe baking code to get it to bake direct light for lights that use the Static Baked bake mode (and only those lights).

@jcostello
Copy link
Contributor Author

In the future we have to come with a better name like: Dynamic, Mix / Movable (old static), Static

@jcostello
Copy link
Contributor Author

I'd appreciate help on this area as I'm not sure how to modify the lightmap probe baking code to get it to bake direct light for lights that use the Static Baked bake mode (and only those lights).

It will be better to wait for #82068 to be merged first. Then maybe @DarioSamo can help you

@Calinou
Copy link
Member

Calinou commented Sep 28, 2023

In the future we have to come with a better name like: Dynamic, Mix / Movable (old static), Static

Lights with Static bake mode that are moved or have their color/energy changed will not affect baked surfaces, so I think that bake mode should remain Static in the UI. In the UI, I would only add a distinction when it comes to lighting of dynamic objects by the static light.

Note that we can't rename the constant name in the enum either for compatibility reasons.

@Calinou
Copy link
Member

Calinou commented Oct 9, 2023

Closing in favor of godotengine/godot-proposals#8040, as feature proposals are now tracked on the Godot proposals repository.

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

5 participants