[Impeller] Make IPComputeConical not crash older GLES drivers.

The IPComputeConicalT is extremely complicated, and is crashing the shader compiler of older Adreno GPU. We need to simplify this function, and get it working. Some ideas:

Lift out special cases for radial + strip. We should be able to detect this in the contents and use the radial gradient shader, or a different kind of shader if need be.
Replace branching with mix. Yes, in theory we don't need to do this any more. In practice, some older shader compilers choke on too many nested if statements.
Compute more values in the contents. For example, I believe is_focal_on_circle is computed entirely from uniform values, et cetera. Maybe is_well_behaved ?
... And more

I think the check for is_swapped can also be lifted out, we can apply the swap ourselves...

Ignore all of this, and lets just update the shader: https://github.com/google/skia/blob/main/src/sksl/sksl_graphite_frag.sksl#L851-L895

@jason-simmons

Notes from @jason-simmons

The other shader that is failing on the Nexus 5 is ConicalGradientUniformFill. I've been unable to find a way to get that to work.

ConicalGradientUniformFill links if I comment out all of IPComputeConicalT. But if I leave in any of the branching based on radial and strip then the Adreno shader compiler crashes during glLinkProgram.

I suspect for ConicalGradientUniformFill its due to the uniform array being too large. We should see if we can get the limits stuff working and then turn the size down a bit.

I suspect for ConicalGradientUniformFill its due to the uniform array being too large. We should see if we can get the limits stuff working and then turn the size down a bit.

That tracks. I think the adreno 330 has a UBO size of 4k bytes. The vec4 colors[256] field is that big alone.

If the ubo size is a problem there are more shaders that break the 4k limit:

shader	ubo size
linear_gradient_uniform_fill.frag	6192
conical_gradient_uniform_fill.frag	6208
radial_gradient_uniform_fill.frag	6192
sweep_gradient_uniform_fill.frag	6192

Maybe 6192 is the limit?

IIRC each of the float uniform values (radius, tile_mode) is actually taking up a full vec4. You can sort of fix this by manually packing them:

  float focus_radius;
  float radius;
  float tile_mode;
  float alpha;

Becomes

vec4 data_stuff;


  float focus_radius = data_stuff.x;
  float radius = data_stuff.y;
  float tile_mode = ..
  float alpha = ..

That and we could always drop a few array values. 256 is already pretty massive and we still have teh texture fallback.

That and we could always drop a few array values. 256 is already pretty massive and we still have teh texture fallback.

We could conditionally make it smaller for opengles too. Shaderc allows you to define your own preprocessor macros which could be handy.

This is an opengl only shader. So no need to get fancy.

Jason has stated in chat that he tried to comment out the fields in FragInfo and it didn't seem to get rid of the linking crash on the nexus 5. So, this is unlikely related to the ubo size. I also found Qualcomm documentation that seems to suggest 7372 bytes is a good limit.

Jason stated that it's something about IPComputeConicalT in particular that is freaking out the compiler.

@gaaclarke

@gaaclarke yeah I think the fix is probably to replace the very complicated conical we use now with something closer to https://github.com/google/skia/blob/main/src/sksl/sksl_graphite_frag.sksl#L851-L895

Jason has provided further information that the compiler crashed when the function was reduced to just its branching:

vec2 IPComputeConicalT(vec2 c0, float r0, vec2 c1, float r1, vec2 pos) {
  bool radial = c1.x > c0.x;
  bool strip = r1 > r0;

  if (radial && strip) {
    return vec2(1);
  } else if (radial) {
    return vec2(2);
  } else if (strip) {
    return vec2(3);
  } else {
    return vec2(4);
  }
}

If that is the case then at a minimum we need to slip into two shaders (radial/strip) and (regular/strip) for GLES. The updated graphite shader has the same amount of branching.

Added compilation failure if a max ubo size is exceeded (#164038)

Split up the conical gradient fragment shader (#164058)

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.

Navigation Menu

Repository navigation

[Impeller] Make IPComputeConical not crash older GLES drivers. #163580

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[Impeller] Make IPComputeConical not crash older GLES drivers. #163580

Description

Activity

jonahwilliams commented on Feb 19, 2025

jonahwilliams commented on Feb 24, 2025

gaaclarke commented on Feb 24, 2025

jonahwilliams commented on Feb 24, 2025

gaaclarke commented on Feb 24, 2025

gaaclarke commented on Feb 24, 2025

jonahwilliams commented on Feb 24, 2025

gaaclarke commented on Feb 24, 2025

jonahwilliams commented on Feb 24, 2025

gaaclarke commented on Feb 24, 2025

jonahwilliams commented on Feb 24, 2025

gaaclarke commented on Feb 24, 2025

jonahwilliams commented on Feb 24, 2025

github-actions commented on Mar 14, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions