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] Allow non-power-of-two directional shadow size in 3D #54042

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Oct 20, 2021

3.x version of #54041.

This allows for more finegrained performance/quality tuning, especially at higher sizes. Any number can be used. For directional light shadows, 3072 and 6144 are sensible shadow sizes that can be used if the default 4096 is too slow or too low-resolution for a large scene.

This change only applies to GLES3, as NPOT texture support is not present on all devices that support GLES2. When using GLES2, shadow sizes are still rounded up to the nearest power of 2. It might be possible to support it regardless, but I'd rather not risk it, especially on mobile platforms.
Edit: Temporarily allowed in GLES2 to check if it works on mobile/web platforms.

This also updates the point light shadow atlas size property hint to reflect that it can be changed without restarting the engine.

Preview

Point light

point-shadow-size-npot-3.x.mp4

DirectionalLight

4096 (default)

2021-10-20_20 48 20

6144 (new possible value)

2021-10-20_20 48 56

8192

2021-10-20_20 48 38

@Calinou Calinou requested review from a team as code owners October 20, 2021 18:54
@Calinou Calinou added this to the 3.5 milestone Oct 20, 2021
@Calinou Calinou changed the title Allow non-power-of-two directional and point shadow atlas size in 3D Allow non-power-of-two directional and point shadow atlas size in 3D (3.x) Oct 20, 2021
@clayjohn
Copy link
Member

Does this work well when using many lights in the shadow atlas? AFAIK the reason it was limited to power of 2 was because the shadow atlas is recursively subdivided to provide space for all the shadow maps. I am concerned that odd numbers may lead to odd rendering artifacts when many lights are used.

@Calinou Calinou force-pushed the shadow-atlas-size-allow-npot-3.x branch from 8d0c46f to 1655067 Compare October 20, 2021 21:19
@Calinou
Copy link
Member Author

Calinou commented Oct 20, 2021

Does this work well when using many lights in the shadow atlas? AFAIK the reason it was limited to power of 2 was because the shadow atlas is recursively subdivided to provide space for all the shadow maps. I am concerned that odd numbers may lead to odd rendering artifacts when many lights are used.

I've tested it again with a larger scene and can confirm that point light shadows break with NPOT sizes 🙁

I reverted the change for the point light shadow atlas size, but I kept it for the directional shadow size.

The behavior in master is strange: NPOT sizes for the point light atlas look fine with no visible rendering glitches regardless of the size chosen. However, I occasionally get error spam:

ERROR: Condition "!li->shadow_atlases.has(p_shadow_atlas)" is true. Returning: 0 at: light_instance_get_shadow_texel_size (servers/rendering/renderer_rd/renderer_scene_render_rd.h:1009)

Testing projects:

@Calinou Calinou changed the title Allow non-power-of-two directional and point shadow atlas size in 3D (3.x) Allow non-power-of-two directional shadow atlas size in 3D (3.x) Oct 23, 2021
@Calinou Calinou force-pushed the shadow-atlas-size-allow-npot-3.x branch from 1655067 to 8fcc597 Compare October 26, 2021 18:33
@Calinou
Copy link
Member Author

Calinou commented Oct 26, 2021

I've amended this PR to allow setting the directional shadow size to NPOT values in GLES2 as well. Based on my understanding, NPOT textures in GLES2 work as long as they aren't set to repeat and have no mipmaps. So far, it works on my desktop PC, but it needs to be tested on Android and HTML5 at least.

2048

2021-10-26_20 32 51

3072 (new possible value)

2021-10-26_20 32 35

4096

2021-10-26_20 33 02

@Calinou Calinou force-pushed the shadow-atlas-size-allow-npot-3.x branch from 8fcc597 to dd86d50 Compare November 3, 2021 21:41
@Calinou Calinou changed the title Allow non-power-of-two directional shadow atlas size in 3D (3.x) Allow non-power-of-two directional shadow size in 3D (3.x) Nov 3, 2021
@Calinou Calinou force-pushed the shadow-atlas-size-allow-npot-3.x branch from dd86d50 to 5015047 Compare May 11, 2022 06:39
@akien-mga akien-mga modified the milestones: 3.5, 3.x Jul 2, 2022
This allows for more finegrained performance/quality tuning, especially
at higher sizes. Any number can be used. 3072 and 6144 are sensible
shadow sizes that can be used if the default 4096 is too slow or
too low-resolution for a large scene.

This also updates the point light shadow atlas size property hint
to reflect that it can be changed without restarting the engine.
Nonetheless, it's still rounded to the next power of 2 since the
atlas relies on dimensions being powers of 2.
@Calinou Calinou force-pushed the shadow-atlas-size-allow-npot-3.x branch from 5015047 to 132b8a9 Compare June 27, 2023 07:45
@Calinou Calinou changed the title Allow non-power-of-two directional shadow size in 3D (3.x) [3.x] Allow non-power-of-two directional shadow size in 3D Jun 27, 2023
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.

3 participants