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

Shader optimizations to reduce VGPR usage and increase occupancy #45023

Merged
merged 2 commits into from
Jan 19, 2021

Conversation

reduz
Copy link
Member

@reduz reduz commented Jan 8, 2021

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.

@@ -6288,13 +6283,14 @@ void RendererSceneRenderRD::_setup_lights(const PagedArray<RID> &p_lights, const
float sign = storage->light_is_negative(base) ? -1 : 1;
Color linear_col = storage->light_get_color(base).to_linear();

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
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
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

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.

Looks mostly good!

}
#endif //LOW_END_MODE
// multiply by albedo
diffuse_light *= albedo; // ambient must be multiplied by albedo at the end
Copy link
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
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
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
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)

@reduz reduz force-pushed the optimize-shader-vgpr1 branch from 8b6c33c to dc2ec58 Compare January 18, 2021 00:51
@reduz
Copy link
Member Author

reduz commented Jan 18, 2021

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

@Calinou
Copy link
Member

Calinou commented Jan 18, 2021

@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
Copy link
Member

clayjohn commented Jan 18, 2021

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 akien-mga, neikeq and a team as code owners January 18, 2021 22:21
@reduz
Copy link
Member Author

reduz commented Jan 18, 2021

@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
Copy link
Contributor

JFonS commented Jan 19, 2021

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
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
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
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