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

Use the Light3D Indirect Energy property in LightmapGI #52256

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Aug 30, 2021

Follow-up to #52252. See also #51705.

This implementation currently only works if the light's bake mode is Dynamic, not Static. I'm not sure how to make it work for static as well, but I documented this limitation nonetheless.

This closes #52257.

Testing project: test_lightmap_normalmap2.zip

@jcostello
Copy link
Contributor

@Calinou the plan is to make it work for static as well, right? BTW static baking is not entirely working right now

@Calinou
Copy link
Member Author

Calinou commented Aug 30, 2021

the plan is to make it work for static as well, right?

If someone manages to find a way, yes 🙂

BTW static baking is not entirely working right now

Could you describe what's not working? Here, it works as expected as long as Directional is not enabled.

@jcostello
Copy link
Contributor

Could you describe what's not working? Here, it works as expected as long as Directional is not enabled.

Baking spot lights doesn't work even if directional is not enabled.

@Calinou
Copy link
Member Author

Calinou commented Aug 30, 2021

Baking spot lights doesn't work even if directional is not enabled.

Please open a new issue with a minimal reproduction project attached.

It currently only works if the light's bake mode is Dynamic, not Static.
@Calinou Calinou force-pushed the lightmapgi-use-light-indirect-energy branch from ce787cd to b611e8b Compare February 3, 2022 16:51
@akien-mga akien-mga requested a review from JFonS February 8, 2022 13:20
@JFonS
Copy link
Contributor

JFonS commented Feb 8, 2022

The current changes look good.

I haven't tested it, but adding support for static indirect light may just be a matter of adding the factor multiplication here:

static_light += light;

Or better yet, multiply directly in the light variable initialization.

@Calinou
Copy link
Member Author

Calinou commented Feb 8, 2022

I haven't tested it, but adding support for static indirect light may just be a matter of adding the factor multiplication here:

I've tried this, but unfortunately, it multiplies both the direct and indirect light:

DirectionalLight and OmniLight Indirect Energy set to 2

2022-02-08_21 28 27

DirectionalLight and OmniLight Indirect Energy set to 1

2022-02-08_21 28 36

DirectionalLight and OmniLight Indirect Energy set to 0

2022-02-08_21 28 48

(The red light source is an emissive cube, not a light node.)

@JFonS
Copy link
Contributor

JFonS commented Feb 9, 2022

@Calinou Right, that makes sense. In this case, I think we need to have two separate buffers for the direct light pass. The currently used one should stay the same, as it's the final light accumulation buffer, and we should add a new one that stores the "scaled" direct light depending on the light's indirect energy value. This new buffer can then be used as the source buffer for the first bounce and discarded.

@YuriSizov
Copy link
Contributor

If this is a relevant fix for a regression in 4.0, it needs a rebase. cc @clayjohn

@Calinou
Copy link
Member Author

Calinou commented Feb 12, 2023

If this is a relevant fix for a regression in 4.0, it needs a rebase. cc @clayjohn

The way this is implemented is incomplete, and it's too difficult for me to do it in a complete way. This can be done for a future 4.0.x release anyway, as this property isn't relied upon very often (at least with lightmaps).

@jcostello
Copy link
Contributor

If this is a relevant fix for a regression in 4.0, it needs a rebase. cc @clayjohn

The way this is implemented is incomplete, and it's too difficult for me to do it in a complete way. This can be done for a future 4.0.x release anyway, as this property isn't relied upon very often (at least with lightmaps).

It is use a lot by artist. Most of the UE4 lighting tutorials I've seen use indirect energy when baking lightmaps

@clayjohn clayjohn modified the milestones: 4.0, 4.x Feb 12, 2023
@Calinou
Copy link
Member Author

Calinou commented Jan 17, 2024

This was resolved for all light types and bake modes by #82068, so this PR is no longer necessary. Both per-light and global indirect energy controls work as of 4.2, closing.

@Calinou Calinou closed this Jan 17, 2024
@Calinou Calinou removed this from the 4.x milestone Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vulkan: LightmapGI does not take light nodes' indirect energy into account (unlike BakedLightmap in 3.x)
5 participants