-
-
Notifications
You must be signed in to change notification settings - Fork 22.5k
Fix LightmapGI shadow leaks #107254
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
base: master
Are you sure you want to change the base?
Fix LightmapGI shadow leaks #107254
Conversation
Side-by-side comparison to see the difference better: |
I'm not sure why that CI check failed when it ran fine here: https://github.com/passivestar/godot/actions/runs/15506456707/job/43661953613 edit: all good 👍 |
I thought that was intentional baked ambient occlusion haha |
It's a bit hard to make out what's going on from those screenshots, any chance you could share a small reproduction project for that problem so I could take a look? |
Sure, here's a reproduction project: |
@H3XAGON3ST if I rebake the lightmap in your project the artifacts go away, so this appears to be a hardware-related issue. I'll see what I can find, thanks for testing ![]() |
See also #107179 (comment) which likewise has NaN only on the reporter's system. |
That's weird. I did some more testing to understand what this error might be and noticed that the artifacts are affected by the type of compression of the EXR file. Choosing "Basic universal" completely fixes the artifacts. 4.5 dev 5 works fine without artifacts. VRAM Compressed (Default)VRAM UncompressedBasis Universal |
@passivestar can you check the different texture compression modes? I noticed that in your test project, by default, the lightmap texture uses the BPTC_RGBFU format. This is the only format where I don't have artifacts. When the compression mode is changed to VRAM Uncompressed, the texture starts using the RGBHalf format, and artifacts appear even without rebaking the lightmap. |
Can reproduce the problem with VRAM Uncompressed |
The lightmap textures now have transparent spots with the color values set to NAN, which causes these artifacts. |
dfbd146
to
219035c
Compare
Lightmap before/after division by zero fix, transparent pixels are gone: @H3XAGON3ST Let me know if it works for you now |
Yes, there are no more artifacts after the last changes👍 |
Fixes shadow leaks
Test project: shadowleaks.zip
Lightmapper already does unocclusion pass to hide dark semi-occluded texels (inspired by bakery approach), but all of that work becomes undone when you enable denoiser. Denoiser is unaware of what's occluded and what isn't so it samples from occluded black texels which results in shadow leaks. This PR solves the issue by using unocclusion pass as a mask to prevent denoiser from sampling occluded texels.
Note
Under some conditions it's easy to mistake shadowleaks for AO. However unlike AO this doesn't happen in all crevices, only in places where geometry intersects. It's also resolution-dependent, angle-independent, and can leak asymmetrically on 2 adjacent surfaces. If you're interested in additional baked AO for creative purposes (and which you have control over) I opened a proposal for that last year
Note
OIDN has this issue too, though to a lesser extent (leaks are much thinner). It's third-party though so not much can be done. I recommend using JNLM, it gives great results
Keep in mind that your geometry needs to have backface culling enabled for godot lightmapper to work properly. While this is important for the fix in this PR to work properly, this isn't specific only to this PR. If you have double-sided materials godot won't be able to do even the basic 1-texel unocclusion that it already does.
Here's comparison of one-sided vs double-sided material in the current master branch with denoiser disabled. Notice that there are both light leaks and shadow leaks here caused by light reaching surfaces that it shouldn't and unocclusion algorithm not being able to do its job properly:
If you're importing textured meshes from Blender you can make sure backface culling is enabled in the material properties: