Skip to content

Shader optimizations to reduce VGPR usage and increase occupancy#45023

Merged
akien-mga merged 2 commits into
godotengine:masterfrom
reduz:optimize-shader-vgpr1
Jan 19, 2021
Merged

Shader optimizations to reduce VGPR usage and increase occupancy#45023
akien-mga merged 2 commits into
godotengine:masterfrom
reduz:optimize-shader-vgpr1

Conversation

@reduz

@reduz reduz commented Jan 8, 2021

Copy link
Copy Markdown
Member

Reduced from 116 to 80 VGPRs in normal usage, still more work is needed.

Some not often used features are gone for the sake of this and some are disabled, will re-enable later.
-Shadow color per light is gone
-Oren Nayar is gone (implement manually if you really want it, makes little difference)
-SSAO color is gone
-SSAO ao affect is gone because this confused users, min(ao,ssao) now used, should work in most cases.
Also

-softshadows (based on angular distance) are likely going to be removed from directional light, too expensive dont look great, and dont make much of a difference
-projectors are disabled.


light_data.attenuation_energy[0] = Math::make_half_float(storage->light_get_param(base, RS::LIGHT_PARAM_ATTENUATION));
light_data.attenuation_energy[1] = Math::make_half_float(sign * storage->light_get_param(base, RS::LIGHT_PARAM_ENERGY) * Math_PI);
light_data.attenuation = storage->light_get_param(base, RS::LIGHT_PARAM_ATTENUATION);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This just cleaned up how parameters are sent to the reflection probe struct

uint8_t color_specular[4]; //rgb color, a specular (8 bit unorm)
uint16_t cone_attenuation_angle[2]; // attenuation and angle, (16bit float)
uint8_t shadow_color_enabled[4]; //shadow rgb color, a>0.5 enabled (8bit unorm)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Likewise cleaned up lights

@clayjohn clayjohn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks mostly good!

Comment thread servers/rendering/renderer_rd/shaders/scene_forward.glsl
}
#endif //LOW_END_MODE
// multiply by albedo
diffuse_light *= albedo; // ambient must be multiplied by albedo at the end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

comment out of date

#ifndef LOW_END_MODE
if (scene_data.ssao_enabled) {
float ssao = texture(sampler2D(ao_buffer, material_samplers[SAMPLER_LINEAR_CLAMP]), screen_uv).r;
ao = min(ao, ssao);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need to delete SSAO_ao_affect from environment etc.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For reference, I opened an issue to track this: #54611

@Calinou Calinou added this to the 4.0 milestone Jan 8, 2021
@reduz reduz force-pushed the optimize-shader-vgpr1 branch 5 times, most recently from 1131b2c to 8b6c33c Compare January 17, 2021 20:55
@clayjohn

Copy link
Copy Markdown
Member

Im getting 2 types of artifacts with shaders. One is random blocks of shadow that flicker in and out (likely entire clusters missing a light). And the second is that the derivative shadow optimization doesnt scale well with distance from a shadow. So when you get far enough away from a shadow the edges take on a very aliased look

Screenshot (60)

Comment thread servers/rendering/renderer_rd/shaders/cluster_render.glsl Outdated
@reduz reduz force-pushed the optimize-shader-vgpr1 branch from 8b6c33c to dc2ec58 Compare January 18, 2021 00:51
@reduz

reduz commented Jan 18, 2021

Copy link
Copy Markdown
Member Author

@akien-mga Should hopefully be ok to merge but the static checks are not passing and I already have clang-format-11.

@Calinou

Calinou commented Jan 18, 2021

Copy link
Copy Markdown
Member

@reduz I would recommend avoiding nested ternary operators, it should fix the clang-format issue at the same time.

@reduz reduz force-pushed the optimize-shader-vgpr1 branch from dc2ec58 to 9683c70 Compare January 18, 2021 13:31
@clayjohn

clayjohn commented Jan 18, 2021

Copy link
Copy Markdown
Member

I haven't reviewed the code yet, but running this build there are some artifacts created on triangle edges. It must be due to the shadow derivative path.

Screenshot (72)

edit: I can confirm that commenting out the shadow derivative path fixes this issue

@reduz reduz force-pushed the optimize-shader-vgpr1 branch from 9683c70 to e71492a Compare January 18, 2021 22:21
@reduz reduz requested review from a team, akien-mga and neikeq as code owners January 18, 2021 22:21
@reduz

reduz commented Jan 18, 2021

Copy link
Copy Markdown
Member Author

@clayjohn disabled it for now

@reduz reduz force-pushed the optimize-shader-vgpr1 branch from e71492a to 1f56612 Compare January 18, 2021 22:35
@JFonS

JFonS commented Jan 19, 2021

Copy link
Copy Markdown
Contributor

I went over the files, it wasn't an in-depth review but I didn't spot anything wrong.

Other than that, Github lists some files as edited, but they don't contain any changes, not sure what's going on. Also, static checks have failed, I think they should be restarted.

@akien-mga

Copy link
Copy Markdown
Member

I restarted CI jobs for static checks and Android which failed for a seemingly unrelated reason, likely a CI oops.

@akien-mga

Copy link
Copy Markdown
Member

I restarted CI jobs for static checks and Android which failed for a seemingly unrelated reason, likely a CI oops.

Ah no it's not a CI issue, @reduz did remove executable permission on those files, so they can't run on CI:
Screenshot_20210119_231914

@akien-mga akien-mga force-pushed the optimize-shader-vgpr1 branch from 1f56612 to a5fbe24 Compare January 19, 2021 22:28
Clustering is now GPU based, uses an implementation based on the Activision algorithm.
@akien-mga akien-mga force-pushed the optimize-shader-vgpr1 branch from a5fbe24 to 099dee3 Compare January 19, 2021 22:31
@akien-mga akien-mga merged commit 8a6d4dd into godotengine:master Jan 19, 2021
@akien-mga

Copy link
Copy Markdown
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.

6 participants