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] Switch to uniform arrays for gradient data #56294

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

flar
Copy link
Contributor

@flar flar commented Nov 1, 2024

Currently the most generalized form of the Impeller gradient shaders uses an SSBO to store the gradient information, but SSBO data is not supported on older platforms. To make the capability more general we introduce variants of the gradient shaders that uses uniform arrays which are more widely supported.

[Currently in draft form - the new shaders are written and example entity content code is written to demonstrate the approach for early reviews, but it is not yet plumbed into the system]

float tile_mode;
vec4 decal_border_color;
float alpha;
int colors_length;
Copy link
Member

Choose a reason for hiding this comment

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

GLES 2.0 requires that we use a float for this 😨

highp vec2 start_to_end;
float alpha;
float tile_mode;
int colors_length;
Copy link
Member

Choose a reason for hiding this comment

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

Same here

float tile_mode;
vec4 decal_border_color;
float alpha;
int colors_length;
Copy link
Member

Choose a reason for hiding this comment

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

Same here

@@ -53,6 +53,10 @@ class LinearGradientContents final : public ColorSourceContents {
const Entity& entity,
Copy link
Member

Choose a reason for hiding this comment

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

Can we also remove RenderTexture in this patch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eventually, I want to leave the old stuff around while I get the new stuff working and then I can run comparisons for testing/benchmarking.

Copy link
Member

Choose a reason for hiding this comment

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

SGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per other discussions I think we decided it would stay in case we get more than 256 colors. Eventually we hope to have a vertex tiling solution that works efficiently on all platforms, but until then we'll still have the issue with zero-length stops on older platforms, but only if there are more than 256 stops total...

/**
* @brief Populate 2 arrays with the colors and stop data for a gradient
*
* The color data is simple converted to a vec4 format, but the stop data
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

@flar
Copy link
Contributor Author

flar commented Nov 5, 2024

FYI - The latest push uses the Uniform versions of the shaders for all types as long as there are fewer than 256 colors. References to the old shaders are commented out for now just as a test run to test the new shaders as much as possible. If the test results are fine then I'll restore and finalize the code.

@@ -145,4 +145,30 @@ vec3 IPComputeFixedGradientValues(float t, float colors_length) {
return vec3(lower_index, upper_index, scale);
}

vec4 IPComputeColorFromTables(vec4 colors[256],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this is used any more as the call overhead swamped everything else. I'll delete when I finalize the code.

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.

2 participants