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

Add setting for shadow cubemap max size #48059

Merged
merged 1 commit into from
Apr 21, 2021

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Apr 21, 2021

As discussed in #48036

This PR allows users to configure max cubemap shadow size. In 3.2 this value was fixed at 512 meaning that there was no improvement in shadow quality for shadow atlases of size 2048 or greater. #47160 set max cubemap shadow size based on atlas size. This is good for quality, but also meant that shadows are by default rendered with a higher resolution.

A cap at 512 artificially makes it so that shadows are always low res. For example, a 2048x2048 quadrant (default quadrant size for first light) would use a 512x512 cubemap while a 1024x1024 segment would still use a 512x512 cubemap.

Another thing that 3.2 did funny was request a cubemap that was much to large for a given section. For example, if rendering to an 128x128 section of the atlas, a 256x256 cubemap would be used. This is a huge waste of resolution accordingly, pre-#47160 was using too low of resolution on high-detail shadows and too much resolution on low-detail shadows.

This PR defaults to making the max cubemap shadow size 512. this matches the visual behaviour of 3.2 but allows users to increase to have higher resolution shadows if they need it at the cost of some performance.

With this change, high-detail shadows should run the same as in 3.2 while low-detail shadows should look the same and run faster (in theory, in practice the bottleneck is elsewhere)

CC @puchik

@clayjohn clayjohn added this to the 3.3 milestone Apr 21, 2021
@clayjohn clayjohn requested a review from a team as a code owner April 21, 2021 05:47
@clayjohn clayjohn requested a review from a team as a code owner April 21, 2021 06:17
@clayjohn
Copy link
Member Author

Also pro-tip for readers of this PR. Try setting shadow_atlas/size to 8192 and cubemap size to 2048. :)

@akien-mga
Copy link
Member

I'm wondering if we should set the default to 1024 to match the advice of using one quarter as max. Might be that the performance drop on affected hardware would not be big (I'll test on mine).

That can be done for 3.4 though, we don't need to rush things now. Adding the option and restoring defaults that prevent unexpected performance drops is good enough for 3.3 👍

@akien-mga
Copy link
Member

Redid yesterday's tests (#48036 (comment)) today as my laptop tends to perform differently after a reboot if not in the same mood (when I started testing this PR I had 108 FPS, then fixed somehow...):

3.3 RC 6, default settings (4096/512): 154 FPS
3.3 RC 7, default settings (4096/4096): 136 FPS
3.3.rc with this PR, default settings (4096/512): 154 FPS (as expected :))
3.3.rc with this PR, 4096/1024: 150 FPS (no significant visual difference in Cornell box MRP)
3.3.rc with this PR, 4096/256: 155 FPS (but looks bad)
3.3.rc with this PR, 8192/2048: 118 FPS (does look significantly better though might not be worth the frame time hit on a more complex scene for this hardware)

So all good, and 512 seems to be a good default for this scene.

@akien-mga akien-mga merged commit 64cf72a into godotengine:3.x Apr 21, 2021
@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.

None yet

2 participants