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 maximum roughness cutoff to SSR to improve performance #56804

Merged
merged 1 commit into from
Dec 15, 2022

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Jan 15, 2022

master version of #56815.

In a test scene with mixed rough and non-rough materials, this saves upwards of 0.15 ms of GPU time with very little visual artifacting (GTX 1080, 2560×1440). This improves performance the most in scenes that have mostly rough materials. The optimization works regardless of the SSR roughness quality selected in the Project Settings.

The roughness cutoff was empirically chosen – it seems to strike a good balance between improving performance and avoiding obvious visual artifacts. The roughness cutoff could be exposed to a project setting if needed, but the Params push constant is already full (128 bytes).

Testing project: test_ssr_performance.zip

Preview

Some more comparisons: Trying to find a good roughness cutoff, High-quality settings

SSR Roughness Low quality (default)

Before

2022-01-15_01 28 23_no_cutoff

After

2022-01-15_01 27 40_cutoff_above_075

SSR Roughness Disabled

Before

2022-01-15_01 29 26_no_cutoff

After

2022-01-15_01 30 13_cutoff_above_075

@fire
Copy link
Member

fire commented Jan 15, 2022

I made a pr in the same area of roughness #56822.

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.

With the changes listed here this could be merged after #69828

@Calinou Calinou force-pushed the ssr-add-max-roughness-cutoff branch from 4826d79 to 1886bcd Compare December 10, 2022 16:34
In a test scene with mixed rough and non-rough materials, this saves
upwards of 0.15 ms of GPU time with very little visual artifacting
(GTX 1080, 2560×1440).
@Calinou Calinou force-pushed the ssr-add-max-roughness-cutoff branch from 1886bcd to 7745bd4 Compare December 10, 2022 16:35
@Calinou
Copy link
Member Author

Calinou commented Dec 10, 2022

Benchmark with the new roughness cutoff of 0.6 in 3840×2160 on a AMD Radeon RX 6900 XT (Fedora 36, RADV), using the same test scene as before:

Before After (this PR)
479 FPS (2.08 mspf) 551 FPS (1.81 mspf)

@akien-mga akien-mga merged commit e1bcadd into godotengine:master Dec 15, 2022
@akien-mga
Copy link
Member

Thanks!

@Calinou
Copy link
Member Author

Calinou commented Jun 27, 2023

Despite the comment in the shader, I noticed the roughness cutoff in master doesn't actually match #69828, since it's 0.6 instead of 0.7:

// The roughness cutoff of 0.6 is chosen to match the roughness fadeout from GH-69828.
if (roughness > 0.6) {

// This is an ad-hoc term to fade out the SSR as roughness increases. Values used
// are meant to match the visual appearance of a ReflectionProbe.
float roughness_fade = smoothstep(0.4, 0.7, 1.0 - normal_roughness.w);

In practice, this appears to have no visual impact, but restoring the "correct" cutoff value of 0.7 makes performance slightly worse (+0.03 mspf in 3840×2160 on a GeForce RTX 4090):

0.6 (master)

Screenshot

0.7 (proposed change)

Screenshot

What should we do? For reference, I've made #56815 use the "correct" cutoff value of 0.7, but maybe we should make it use 0.6.

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