Allow non-power-of-two directional shadow size in 3D

[Calinou]

master version of #54042.

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.

Preview

[mrjustaguy]

Nice change, I can totally see this being useful, makes me wonder what would happen if they were uncapped (so going past 16k) when going past the current limit.. 🤔

[Calinou]

Nice change, I can totally see this being useful, makes me wonder what would happen if they were uncapped (so going past 16k) when going past the current limit.. thinking

GPUs don't support textures larger than 16384×16384, so it won't work. Not to mention a 16384×16384 shadowmap is pretty slow already.

[Calinou]

I amended this PR to only allow NPOT sizes for directional shadows, not point light shadows.

[reduz]

@Calinou I think using custom sizes is still not a great idea because for users its difficult to change this value. IMO I would simply do an enum with more intermediate values (2^n + 2^(n-1) ) such as:

• 256
• 384
• 512
• 768
• 1024
• 1536
• 2048
• 3072
• 4096
• 6144
• 8192

This is considerably easier for users to edit and it still provides relatively good divisors.

[Calinou]

This is considerably easier for users to edit and it still provides relatively good divisors.

See also #57610, where a counterargument to using enums was made. That said, I think your set of proposed values is good, so it probably won't cause problems in the future. The only question is whether we should still expose sizes above 8192 (which are very slow, usually for no real benefit).

[mrjustaguy]

16k Shadow Maps are very much useful for large open world games, that simply cannot use lower resolution Directional Shadow maps without massive issues, in cases where you CANNOT use Baked Lightmaps (because of things like Day Night Cycle)

The Benefit of higher Resolution Shadow Maps might not exist for small range Directional Shadows, but it's a night and day difference for larger ones with massive benefit, and also in those scenarios, you don't even get that much of a benefit from the resolution change aside from lower memory usage, because the triangle processing is what's eating significantly more performance overhead caused by Directional Shadows.

This is especially true when considering #48776 as with that distant shadows get extremely pixelated at lower resolutions, and blending no longer gives any benefits (whereas before it'd just shrink the shadows due to how the blur worked)

[YuriSizov]

Needs to address @reduz's review, and needs a rebase, then it should be good to go.

[Calinou]

Since #57610 was closed as it broke compatibility, this means we can't go with reduz's suggestion unless we break compatibility or implement a compatibility handler. This would mean creating a new project setting with a different name, hiding the old project setting and reading the old project setting's name on startup to set the new value.

[clayjohn]

I think going with reduz's suggestion along with a compatibility handler would be fine. IMO having Po2 or near-Po2 textures is ideal as it makes having multiple shadows much simpler. If we allow non-Po2 we will run into aliasing issues when the atlas doesn't neatly subdivide by the number of DirectionalLights that we have

[Calinou]

I think going with reduz's suggestion along with a compatibility handler would be fine. IMO having Po2 or near-Po2 textures is ideal as it makes having multiple shadows much simpler. If we allow non-Po2 we will run into aliasing issues when the atlas doesn't neatly subdivide by the number of DirectionalLights that we have

I assume a compatibility handler requires renaming the project setting, so that we can use a magic getter to read the old property name?

What about the RenderingServer setter method?

[Calinou]

Rebased and tested again, it works as expected.

This now keeps the integer property and uses an enum hint with numbered values (as per reduz's suggestion), so that compatibility is preserved. See OP for an updated screenshot.