[Impeller] Use varying interpolation to compute some linear gradients. #53166

jonahwilliams · 2024-06-02T17:37:51Z

Create a fast gradient variant that assumes vertex interpolation is sufficient to compute the gradient color. This follows the approximate design from flutter/flutter#148651

This is only implemented for draws that are exactly horizontal or vertical gradients on a rectangle with an identity effect transform. This could be easily extended to all horizontal or vertical gradients by extending the drawGeometry call to make the cover rect customizable. Handling diagonal gradients and effect transforms is more work but feasible.

this is sufficient to make flutter/flutter#148496 about 2x as fast, at least for me. Applications with different gradients will not yet benefit.

…g_interpolation

flutter-dashboard · 2024-06-03T19:25:25Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #53166 at sha a14f666

flutter-dashboard · 2024-06-03T21:03:43Z

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #53166 at sha 59ba1c4

jonahwilliams · 2024-06-03T22:32:06Z

Checked on wonderous - this hits almost all gradient usage, except for the shimmer icons which have a transform.

chinmaygarde

Well worth the extra pipeline.

chinmaygarde · 2024-06-03T22:56:59Z

impeller/entity/shaders/gradients/fast_gradient.vert

+in vec2 position;
+in vec4 color;
+
+out vec4 v_color;


Maybe add a comment here saying, "We exploit the simplicity of this gradient and make the varying unit perform the interpolation instead of performing the calculations ourselves."

chinmaygarde · 2024-06-03T22:59:48Z

impeller/entity/contents/linear_gradient_contents.cc

+  VertexBufferBuilder<VS::PerVertexData> vtx_builder;
+  vtx_builder.Reserve(6 * (stops_.size() - 1));
+  Point prev = start_point_;
+  for (auto i = 1u; i < stops_.size(); i++) {


chinmaygarde · 2024-06-03T23:00:54Z

impeller/entity/contents/linear_gradient_contents.cc

+// the gradient is divided into regions based on the stop values.
+// Currently restricted to rect geometry where the start and end points are
+// perfectly horizontal/vertical, but could easily be expanded to StC cases
+// provided that the start/end are on our outside the coverage rect.


Typo towards the end of the sentence?

chinmaygarde · 2024-06-03T23:01:34Z

impeller/entity/contents/linear_gradient_contents.cc

+  Geometry& geometry = *GetGeometry();
+
+  // We already know this is an axis aligned rectangle, so the coverage will
+  // be approximately the same as the geometry. For non AARs, we can force


Just say "axis aligned rectangles" instead of AARs?

chinmaygarde · 2024-06-03T23:01:53Z

impeller/entity/contents/linear_gradient_contents.cc

+
+  // We already know this is an axis aligned rectangle, so the coverage will
+  // be approximately the same as the geometry. For non AARs, we can force
+  // stencil then cover (not done here). We give an identity transform to


A TODO with a followup for non-AARs?

Done (below)

chinmaygarde · 2024-06-03T23:02:51Z

impeller/entity/contents/content_context.h


 namespace impeller {

+using GradientPipeline =


FastGradientPipeline instead?

chinmaygarde · 2024-06-03T23:44:00Z

impeller/entity/shaders/gradients/fast_gradient.frag

+
+void main() {
+  frag_color = IPPremultiply(v_color) * frag_info.alpha;
+  // mod operator is not supported in GLES 2.0


Whats with this comment? Also, even though the operator is not supported, you can use the function.

This shader would fail to load on GLES because the ordered dither uses a bunch of bit manipulation logic?

Oh. Gotcha. That method has a mod in there somewhere.

We should be able to fix it using the mod() function though. It's just the operator that's bad in ES.

Nothing to do in this patch. Was just curious.

what an odd restriction

… gradients. (flutter/engine#53166)

…149658) flutter/engine@a6aa5d8...e211c43 2024-06-04 [email protected] Roll Fuchsia Linux SDK from lKBLxel8iBaHpT5q6... to pagJoGS4kQ8Efa_if... (flutter/engine#53192) 2024-06-04 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[display_list] allow applying opacity peephole to single glyph. (#53160)" (flutter/engine#53189) 2024-06-04 [email protected] Roll Dart SDK from 49b5590c8d80 to 9dce57c343ec (1 revision) (flutter/engine#53187) 2024-06-04 [email protected] [Impeller] Use varying interpolation to compute some linear gradients. (flutter/engine#53166) 2024-06-03 [email protected] [Impeller] match rrect_blur math to the gaussian math (flutter/engine#53130) 2024-06-03 [email protected] [web] Add Ethiopic font fallback. (flutter/engine#53180) 2024-06-03 [email protected] Fix rendering corruption by Flutter and GDK sharing the same OpenGL context (flutter/engine#53103) 2024-06-03 [email protected] [display_list] allow applying opacity peephole to single glyph. (flutter/engine#53160) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from lKBLxel8iBaH to pagJoGS4kQ8E If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

jonahwilliams added 3 commits May 20, 2024 09:31

[Impeller] use varying interpolation for linear gradients.

e9e2ef4

Merge branch 'main' of github.com:flutter/engine into gradient_varyin…

29b4597

…g_interpolation

++

af7c7ee

github-actions bot added the e: impeller label Jun 2, 2024

jonahwilliams added 7 commits June 2, 2024 13:18

++

b5f764a

++

9b35a37

test deleting

765e00f

lets try spinning thats a neat trick.

1aed825

adjust goldens.

7623919

simplify.

61ed98a

++

a14f666

flutter-dashboard bot added the will affect goldens label Jun 3, 2024

jonahwilliams added 2 commits June 3, 2024 13:01

fix hoz gradients and reversed.

4cfd1cf

++

59ba1c4

jonahwilliams changed the title ~~[WIP] Use varying interpolation to compute gradients.~~ [Impeller] Use varying interpolation to compute some linear gradients. Jun 3, 2024

jonahwilliams requested review from bdero and chinmaygarde and removed request for chinmaygarde June 3, 2024 22:33

chinmaygarde approved these changes Jun 3, 2024

View reviewed changes

jonahwilliams added 2 commits June 3, 2024 16:26

chinmay review.

f33c417

++

6c92f8e

jonahwilliams mentioned this pull request Jun 3, 2024

[Impeller] Impeller RRect blur is slower than Skia's cached blur shadows. flutter/flutter#148496

Closed

1 task

chinmaygarde reviewed Jun 3, 2024

View reviewed changes

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 4, 2024

auto-submit bot merged commit 75be94d into flutter:main Jun 4, 2024

engine-flutter-autoroll mentioned this pull request Jun 4, 2024

Roll Flutter Engine from a6aa5d826649 to e211c43f3dc1 (8 revisions) flutter/flutter#149658

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 4, 2024

75be94d54 [Impeller] Use varying interpolation to compute some linear…

da32c10

… gradients. (flutter/engine#53166)

mark8044 mentioned this pull request Jun 4, 2024

[Impeller] LinearGradient no longer fills in all the way flutter/flutter#149690

Closed

[Impeller] Use varying interpolation to compute some linear gradients. #53166

[Impeller] Use varying interpolation to compute some linear gradients. #53166

Uh oh!

Conversation

jonahwilliams commented Jun 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flutter-dashboard bot commented Jun 3, 2024

Uh oh!

flutter-dashboard bot commented Jun 3, 2024

Uh oh!

jonahwilliams commented Jun 3, 2024

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jonahwilliams commented Jun 2, 2024 •

edited

Loading