-
-
Notifications
You must be signed in to change notification settings - Fork 22k
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
Spread direct lighting calculation for LightmapGI over several submissions #102257
Conversation
Does it fix it for linux? |
Possibly. I wasn't able to reproduce a crash on linux even on integrated graphics |
26d9cbc
to
c85b399
Compare
With so many crashes on the same day I started to suspect on my device, so tested the hundred trees MRP on the Shadowmap PR (189c8eb). It bakes without issues. Now I tested the windows artifact (c85b399) and it does not crash \o/. But unfortunately, Shadowmaks are not working anymore. Here is a comparison:
|
I've noticed the same behavior. It doesn't crash anymore, but the shadows are weird. Not sure if it is related to this fix or not. _shadow.mp4 |
@SpockBauru thank you for testing and confirming that it no longer crashes for you. I have a theory about why shadowmask broke. I'll take a look using your MRP |
Did also a test on relatively large scene. I did a bake with ultra quality, single directional light with angular distance = 0.5 Bake took almost 2 hours without issues. I can confirm no crashes on Windows on this PR for me! |
…sions to avoid TDR on Windows devices Also add percentage progress for direct lighting step
c85b399
to
49a004f
Compare
@SpockBauru @GustJc Shadows are back! Thanks for catching that. Please test again once the CI finishes building and let me know if you have any problems |
All working now! Great work! |
No crashes and now Shadowmask work! \o/ Tested the Forward+, Mobile and Compatibility renderers. No crashes and shadowmask now have a even higher quality thanks to the transparency, yay! Made the size of the terrain lightmap 1.8x higher, no crashes and now the smadowmask shows even the brunches of the trees! This was not possible before! \o/ Baking times doubled, but this is expected due the high amount of transparent materials in this scene.
Awesome work! No crashes at all! I believe that #101391 can be finally closed. Excelent! Now the only remaining issues with this MRP are the tunnel regression in #102164 and I just noted the wrong colors with static DynamicLight in #102203, but these belongs to other PRs. |
This seems like a great idea. not using soft-shadows when baking made lightmapping way less crash prone on previous versions of 4.x, probably because of the more memory and time consumed for the more rays, so this seems to be the correct approach in making the lightmapper more reliable. |
Crash at the begining of the bake is fixed. How ever in the backyard distro scene with 0.5 texel density or higher I get this error: VRAM before the crash:
Error:
|
That is a crash in indirect lighting. Can you try reducing |
That fix the issue and let me bake. Should I create a new issue to prevent this to happen on the default settings? |
As soon as this PR gets merged, I think it is better to just include in #102243 since these settings make baking times longer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on Windows with the TPS demo, baking lightmaps no longer crashes
Thanks! |
Fixes: #101391
Fixes: #98180
The root of the problem here is TDR on Windows which is an OS level interrupt that intentionally crashes GPU contexts that it suspects are frozen. The TDR limit by default is very low on Windows, it can be adjusted (and many users choose to do so), but we don't expect that all users will dig around in the Windows registry in order to do so.
Prior to this PR, we calculate all direct lighting in a single command buffer by doing one compute dispatch per atlas slice. Contrast that to indirect lighting where we do one command buffer for each ray, for each region, for each slice). In 4.3 and earlier we didn't have any issues with this simplistic approach because direct lighting was very simple, it either cast one ray for hard shadows or
rendering/lightmapping/bake_quality/medium_quality_ray_count
(depending on the quality level) for soft shadows. With the introduction of shadow map antialsing in #95828, we multiply the shadow rays by 16, and now with transparency support in #99538 we also multiply the number of rays byrendering/lightmapping/bake_performance/max_transparency_rays
.All that is to say, we do a lot more work in the direct light compute invocations, so we need to spread the work out over multiple command buffers.
This PR does that simply by splitting the work up over regions like we do for indirect lighting so they can take advantage of
rendering/lightmapping/bake_performance/region_size