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

Allow changing directional shadow size at run-time #54165

Merged

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Oct 23, 2021

Factoring out the directional shadow creation code allows calling it at any time. This works in both GLES3 and GLES2.

This makes no performance difference when the feature isn't used. In the test scene, I get 540 FPS with and without this PR.

With this change, we can look into exposing support for 16-bit shadow buffers (in a future PR) including run-time changes of the precision setting. This can improve performance without impacting visual quality much, and is already the default in the master branch.
Edit: Done in #57430.

This fixes #18621, #30087 and #30241 in 3.x (these were already fixed in master).

Preview

GLES3

gles3-runtime-directional-shadow-size-change.mp4

GLES2

gles2-runtime-directional-shadow-size-change.mp4

Factoring out the directional shadow creation code allows calling it
at any time. This works in both GLES3 and GLES2.
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I think if we do this then we need to expose an enum for size, otherwise the buffer will be reallocated every time the slider updates. On good hardware, that works fine, but on lower end hardware it will make the slider impossible to use.

@Calinou
Copy link
Member Author

Calinou commented Oct 23, 2021

Looks great! I think if we do this then we need to expose an enum for size, otherwise the buffer will be reallocated every time the slider updates. On good hardware, that works fine, but on lower end hardware it will make the slider impossible to use.

The code that compares the "old" and "new" shadow size already uses next_power_of_2() to avoid this, so you can drag the slider smoothly on any kind of hardware. Also, if #54042 is merged, we won't need to call next_power_of_2() anymore 🙂

@lawnjelly
Copy link
Member

This looks very useful!

Another option, worth discussing at least (and potentially more for 4.x), is changing the matrices / glViewports and change the shadow map size per frame, without requiring re-allocation. This would allow dynamically scaling performance as you moved around a map. This is similar to the often used technique of scaling the main viewport for rendering (then upscale to final result).

I've never used this but it may not be that complex to implement, and would be an interesting experiment.

Copy link
Member

@lawnjelly lawnjelly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry didn't go through this properly at the time. Given the comments about the check for power of 2 it sounds like it doesn't cause constant reallocation.

This is also something that can use #53296 when we finalize that to prevent checking the setting unless there have been changes.

@akien-mga akien-mga merged commit 6c0c0c4 into godotengine:3.x Jan 22, 2022
@akien-mga
Copy link
Member

Thanks!

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.

4 participants