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

Add Environment Min Light property to LightmapGI #50572

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Jul 17, 2021

This property works the same as BakedLightmap's Environment Min Light property in 3.x. It can be used to avoid overly dark areas in the lightmap, or to tweak the shadow color for artistic reasons.

This closes #50568.

Testing project: test-lightmapgi-environment-min-light.zip

Preview

The color is exaggerated for previewing purposes. Typically, you'll use much darker colors like #050505.

2021-07-18_00 50 11

TODO

  • Make lightmap probes affected by Environment Min Light so that dynamic objects are lit correctly.
  • Fix Environment Min Light being too bright when Directional is enabled.

@reduz
Copy link
Member

reduz commented Jul 30, 2022

I do not see the point of this given you have the ambient constant color, its quite redundant.

@Calinou
Copy link
Member Author

Calinou commented Jul 30, 2022

I do not see the point of this given you have the ambient constant color, its quite redundant.

The constant ambient color does not spread to shaded areas when using LightmapGI. While this is physically accurate, it can result in scenes looking too dark unless you manually add a bunch of OmniLight3Ds in dark spots (also known as "unmotivated light sources").

This PR is also in the interest of avoiding a feature regression for users moving from 3.x to 4.0. Several users seem to be relying on this feature in 3.x already, and we should ensure there are as few roadblocks as possible for upgrading to 4.0.

@ScarfKat
Copy link

ScarfKat commented Nov 6, 2024

This would be super helpful. I just ran into a relevant issue today, with lightmapping being darker than I anticipated. Fingers crossed this makes it into 4.4.

@Calinou Calinou force-pushed the lightmapgi-add-environment-min-light branch from c589144 to ac347fc Compare November 14, 2024 23:29
This property works the same as BakedLightmap's Environment Min Light
property in `3.x`. It can be used to avoid overly dark areas in the
lightmap, or to tweak the shadow color for artistic reasons.
@Calinou Calinou force-pushed the lightmapgi-add-environment-min-light branch from ac347fc to bd4de6d Compare November 14, 2024 23:45
@Calinou
Copy link
Member Author

Calinou commented Nov 14, 2024

Rebased and tested again, it works as expected.

This would be super helpful. I just ran into a relevant issue today, with lightmapping being darker than I anticipated. Fingers crossed this makes it into 4.4.

I'm afraid there are several outstanding issues I need to address before this is in a mergeable state (see TODO in the pull request description). These are nontrivial to fix as I'll need to modify the lightmap baking shader to bake the correct data on the GPU, instead of modifying the final image data on the CPU (which can't affect lightmap probes, or deal with directional lightmaps correctly). This is more involved since I'm not familiar with the lightmap baking shader's code.

In the meantime, there are a few things you can do to address lightmaps being too dark:

  • Increase the Bounce Indirect Energy property in your LightmapGI node.
  • Increase Indirect Energy on specific Light3D nodes.
  • Add some unmotivated light sources (i.e. lights with Static bake mode, relatively high range and a low energy) in specific dark areas.
  • Change the tonemapping method used in the Environment node. ACES has a tendency to overdarken scenes, so try using Filmic instead. AgX can be used as an alternative that displays bright colors in a more accurate fashion (similar to ACES) but without the overdarkening issues.

@filipworksdev
Copy link

This is very helpful! 👍

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 Environment Min Light property not implemented (unlike BakedLightmap in 3.x)
6 participants