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

[3.x] Shadows for multiple directional lights are rendered despite being unnecessary #74299

Closed
Ansraer opened this issue Mar 3, 2023 · 5 comments

Comments

@Ansraer
Copy link
Contributor

Ansraer commented Mar 3, 2023

Godot version

3.5.1

System information

Win 10, gles3, 32gb ram, 5800x, rx6900xt, latest gpu driver

Issue description

I have been using Renderdoc to investigate the gles3 renderer recently and noticed that when you have multiple DirLights with shadows enabled Godot performs shadow passes for all of them.
This is a problem because during the fragment shader only the shadows of the first DirLight are used, so we are basically running unnecessary depth passes and are wasting memory to store the results despite never using it.

Below is a screenshot of what this looks like in Renderdoc:
image

According to the docs and various comments 3.x should only support one directional light with shadows. If someone could officially confirm that fact here I will take a stab at removing the unnecessary shadow pass.

Steps to reproduce

Create a basic scene, add some geometry, and finally add two different dir lights, both with shadows enabled.
Shadows are only rendered for the first light, but if you look at a Renderdoc capture you can see that the shadows were also calculated for the second light.

Minimal reproduction project

Small3DScene.zip

@clayjohn
Copy link
Member

clayjohn commented Mar 3, 2023

Testing locally it seems like only one shadow is supported. But looking at the code, it is clearly designed to support multiple directional light shadows (up to four). The fact that only one shadow works is a bug.

In the GLES2 renderer multiple shadows appear, but there are heavy artifacts that make it essentially broken as well

@Calinou
Copy link
Member

Calinou commented Mar 3, 2023

By the way, could multiple shadow passes be skipped in 4.x if two lights have the exact same angle, blur and angular distance? (This is probably uncommon enough not to bother though, as long as it's documented.)

@Ansraer
Copy link
Contributor Author

Ansraer commented Mar 4, 2023

Yes, but I really don't think such a scenario is common enough to justify the additional code. Maybe we should add a simple warning instead? Cause I reckon 90% of the time this happens people accidentally duplicated one of the lights.

But another optimization that would make sense is to not render every cascade every frame. Especially the more distant ones don't need to be updated that often. I will bring this up next week with hp and see if Ramatak would want me to work on that for 3.x (and maybe 4.x too).

@Ansraer
Copy link
Contributor Author

Ansraer commented Mar 4, 2023

Testing locally it seems like only one shadow is supported. But looking at the code, it is clearly designed to support multiple directional light shadows (up to four). The fact that only one shadow works is a bug.

Yeah, that's what had me confused as well. All the online documentation I could find indicated that only one light source should be supported, so I wasn't sure if the redundant passes were just redundant code that hadn't been removed yet.

That's why I asked on RC as well. Will look into this next week.

@akien-mga
Copy link
Member

Fixed by #74539.

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

4 participants