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

[Impeller] support static shader variants using specialization constants #119357

Closed
jonahwilliams opened this issue Jan 27, 2023 · 3 comments · Fixed by flutter/engine#47432, flutter/engine#47678, flutter/engine#47762 or flutter/engine#47765
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) e: impeller Impeller rendering backend issues and features requests engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list team-engine Owned by Engine team triaged-engine Triaged by Engine team

Comments

@jonahwilliams
Copy link
Member

Originally in go/impeller-shader-variants, related to #116103

Problems

Specialization constants

spriv-cross seems to mostly do reasonable things with specialization constants declared in a shader.

For example, with the morphology filter:

uniform FragInfo {
  float texture_sampler_y_coord_scale;
  vec2 texture_size;
  vec2 direction;
  float radius;
}
frag_info;

layout(constant_id = 0) const float morph_type = 0;

For GLES, the following code is generated:

#ifndef SPIRV_CROSS_CONSTANT_ID_0
#define SPIRV_CROSS_CONSTANT_ID_0 0.0
#endif
const float morph_type = SPIRV_CROSS_CONSTANT_ID_0;

For Metal, the following code is generated:

constant float morph_type_tmp [[function_constant(0)]];
constant float morph_type = is_function_constant_defined(morph_type_tmp) ? morph_type_tmp : 0.0;

We'll probably need to do some plumbing to set it but it might do what we need.

We'll still need to declare all of the possible values ahead of time. This will allow us to generate specialized versions for GLES and have correct bindings for metal, which seems to need all possible values during pipeline creation.

From Metal documentation:

This sample uses three different MTLRenderPipelineState objects, each representing a different LOD. Specializing functions and building pipelines is expensive, so the sample performs these tasks asynchronously before starting the render loop. When the AAPLRenderer object is initialized, each LOD pipeline is created asynchronously by using dispatch groups, completion handlers, and notification blocks.

AIs

  • Add PSO benchmarks and compare specialization constants to N shaders
  • Add support for function constant registration in metal backend
  • Ensure that opengl vs vulkan on android can avoid loading opengl shaders if we end up generating a large amount of them.
  • Verify Android on Vulkan sees similar performance improvements from specialization constants compared to shader variants.
@jonahwilliams jonahwilliams added engine flutter/engine repository. See also e: labels. c: performance Relates to speed or footprint issues (see "perf:" labels) P2 Important issues not at the top of the work list e: impeller Impeller rendering backend issues and features requests labels Jan 27, 2023
@jonahwilliams
Copy link
Member Author

Rather than performance, we may consider this as a means to drive down bundle size on Android.

Right now we have a decent amount of shader duplication between the advanced blends, and additionally a bit with blurs. We could probably reduce our bundle size by at least a few Kbs by instead expressing the shaders as a big switch over specialization constants.

We'd need to build support in the tool for "unrolling" these for GLES though.

@jonahwilliams
Copy link
Member Author

The current mystery here is how to use specialization constants with GLES. The SPIRV output contains #define based on the constant value, but the way we load the shaders in a binary format doesn't seem to have any room for adding additional define values, so by default they would be false.

Additionally, impellerc cannot handle multiple output files.

@jonahwilliams jonahwilliams reopened this Nov 3, 2023
auto-submit bot pushed a commit to flutter/engine that referenced this issue Nov 7, 2023
Reland of #47432

Also includes:
  * #47617
  * #47637
  
Fixes the performance on iOS by removing blocking on compilation of shaders. From local testing this has identical before/after numbers. Additional, ensures that we don't unecessarily specialize vertex shaders and notes this restriction in the documentation.

----

Adds support for Specialization constants to Impeller for our usage in the engine. A motivating example has been added in the impeller markdown docs.

Fixes flutter/flutter#136210
Fixes flutter/flutter#119357
auto-submit bot added a commit to flutter/engine that referenced this issue Nov 7, 2023
…47762)

Reverts #47678
Initiated by: jonahwilliams
This change reverts the following previous change:
Original Description:
Reland of #47432

Also includes:
  * #47617
  * #47637
  
Fixes the performance on iOS by removing blocking on compilation of shaders. From local testing this has identical before/after numbers. Additional, ensures that we don't unecessarily specialize vertex shaders and notes this restriction in the documentation.

----

Adds support for Specialization constants to Impeller for our usage in the engine. A motivating example has been added in the impeller markdown docs.

Fixes flutter/flutter#136210
Fixes flutter/flutter#119357
auto-submit bot pushed a commit to flutter/engine that referenced this issue Nov 8, 2023
Reland of #47432

Also includes:

#47617
#47637

Fixes the performance on iOS by removing blocking on compilation of shaders. From local testing this has identical before/after numbers. Additional, ensures that we don't unecessarily specialize vertex shaders and notes this restriction in the documentation.

Adds support for Specialization constants to Impeller for our usage in the engine. A motivating example has been added in the impeller markdown docs.

Fixes flutter/flutter#136210
Fixes flutter/flutter#119357

Investigating: flutter/flutter#138028
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) e: impeller Impeller rendering backend issues and features requests engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list team-engine Owned by Engine team triaged-engine Triaged by Engine team
Projects
No open projects
Archived in project
1 participant