[Impeller] Move Gradient tests to display list. #53345

jonahwilliams · 2024-06-11T21:59:42Z

Migrate almost all of the gradient unittests to display list. Removes tests for disabled dithering and the dithering flag, as it has not been possible to disable dithering on gradients for months and months.

Part of flutter/flutter#142054

flutter-dashboard · 2024-06-12T02:09:39Z

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 #53345 at sha 5b29716

chinmaygarde

Are the "AiksTests" that test with DL so that the names are consistent for goldens? Presumably, they can be renamed later.

gaaclarke

LGTM, there are a few minor critiques, cleanups and nitpicks.

gaaclarke · 2024-06-12T15:24:49Z

impeller/display_list/aiks_dl_gradient_unittests.cc

+namespace testing {
+
+namespace {
+void CanRenderLinearGradient(AiksTest* aiks_test, DlTileMode tile_mode) {


Suggested change

void CanRenderLinearGradient(AiksTest* aiks_test, DlTileMode tile_mode) {

/// Test body for linear gradient tile mode tests (ex. CanRenderLinearGradientClamp).

void CanRenderLinearGradient(AiksTest* aiks_test, DlTileMode tile_mode) {

Or maybe just move it next to the tests.

gaaclarke · 2024-06-12T15:27:38Z

impeller/display_list/aiks_dl_gradient_unittests.cc

+
+TEST_P(AiksTest, CanRenderLinearGradientWithDitheringEnabled) {
+  CanRenderLinearGradientWithDithering(this);
+}  // namespace


Suggested change

} // namespace

}

gaaclarke · 2024-06-12T15:28:55Z

impeller/display_list/aiks_dl_gradient_unittests.cc

+  builder.Translate(100, 100);
+
+  std::vector<DlColor> colors = {
+      DlColor(Color{0x1f / 255.0, 0.0, 0x5c / 255.0, 1.0}.ToARGB()),


nit: This pattern is repeated many times in this file. It is worth factoring it out.

as a follow up I want to add a new constructor to DlColor so we can just replace the impeller colors here.

gaaclarke · 2024-06-12T15:30:27Z

impeller/display_list/aiks_dl_gradient_unittests.cc

+    static Matrix matrix = {
+        1, 0, 0, 0,  //
+        0, 1, 0, 0,  //
+        0, 0, 1, 0,  //
+        0, 0, 0, 1   //
+    };


Suggested change

static Matrix matrix = {

1, 0, 0, 0, //

0, 1, 0, 0, //

0, 0, 1, 0, //

0, 0, 0, 1 //

};

static Matrix matrix;

gaaclarke · 2024-06-12T15:31:04Z

impeller/display_list/aiks_dl_gradient_unittests.cc

+    static Matrix matrix = {
+        1, 0, 0, 0,  //
+        0, 1, 0, 0,  //
+        0, 0, 1, 0,  //
+        0, 0, 0, 1   //
+    };


Suggested change

static Matrix matrix = {

1, 0, 0, 0, //

0, 1, 0, 0, //

0, 0, 1, 0, //

0, 0, 0, 1 //

};

static Matrix matrix;

gaaclarke · 2024-06-12T15:33:34Z

impeller/golden_tests/golden_playground_test_mac.cc


+bool GoldenPlaygroundTest::OpenPlaygroundHere(
+    AiksDlPlaygroundCallback
+        callback) {  // NOLINT(performance-unnecessary-value-param)


Why NOLINT? Just make this a const reference.

gaaclarke · 2024-06-12T15:33:51Z

impeller/golden_tests/golden_playground_test_stub.cc


+bool GoldenPlaygroundTest::OpenPlaygroundHere(
+    AiksDlPlaygroundCallback
+        callback) {  // NOLINT(performance-unnecessary-value-param)


gaaclarke · 2024-06-12T15:35:34Z

impeller/display_list/aiks_dl_gradient_unittests.cc

+  ASSERT_TRUE(aiks_test->OpenPlaygroundHere(builder.Build()));
+}
+
+static Matrix ToMatrix(const SkMatrix& m) {


Suggested change

static Matrix ToMatrix(const SkMatrix& m) {

Matrix ToMatrix(const SkMatrix& m) {

gaaclarke · 2024-06-12T15:43:26Z

Are the "AiksTests" that test with DL so that the names are consistent for goldens? Presumably, they can be renamed later.

Yep.

gaaclarke

lgtm!

flutter-dashboard · 2024-06-12T16:49:26Z

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

Changes reported for pull request #53345 at sha aeb24f3

…ne#53345)

flutter/engine@c7fcbfc...4cb3025 2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from 0fe7f7ac43c5 to ecc64fb82374 (1 revision) (flutter/engine#53374) 2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from 9ea9f37e8e1d to 0fe7f7ac43c5 (1 revision) (flutter/engine#53373) 2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from b12af56eb100 to 9ea9f37e8e1d (1 revision) (flutter/engine#53372) 2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from 704f70b7d756 to b12af56eb100 (1 revision) (flutter/engine#53371) 2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from 69a53db434a6 to 704f70b7d756 (2 revisions) (flutter/engine#53370) 2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from b2f163ae11bf to 69a53db434a6 (8 revisions) (flutter/engine#53365) 2024-06-13 jonahwilliams@google.com [Impeller] Move Gradient tests to display list. (flutter/engine#53345) 2024-06-13 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 5VG7IYkpANkfxMKYx... to pGxbL7JoNb3yAYFw4... (flutter/engine#53363) 2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from be6a57569a92 to b2f163ae11bf (5 revisions) (flutter/engine#53357) 2024-06-13 15619084+vashworth@users.noreply.github.com Give mac_host_engine tests a longer timeout and allow to run on Mac-14 (flutter/engine#53355) 2024-06-13 codefu@google.com 'Starter Project': port planet fragment shader to impeller tests (flutter/engine#53362) 2024-06-12 skia-flutter-autoroll@skia.org Roll Skia from 1ba0c3ebc7a5 to be6a57569a92 (1 revision) (flutter/engine#53353) 2024-06-12 skia-flutter-autoroll@skia.org Roll Dart SDK from 24fbead7105d to aa2265e5a192 (1 revision) (flutter/engine#53352) 2024-06-12 skia-flutter-autoroll@skia.org Roll Skia from 52d829ac6a0b to 1ba0c3ebc7a5 (1 revision) (flutter/engine#53351) 2024-06-12 skia-flutter-autoroll@skia.org Roll Skia from 5bfbacc89543 to 52d829ac6a0b (1 revision) (flutter/engine#53350) 2024-06-12 skia-flutter-autoroll@skia.org Roll Skia from 51eabd0d1e44 to 5bfbacc89543 (1 revision) (flutter/engine#53349) 2024-06-12 skia-flutter-autoroll@skia.org Roll Dart SDK from b61536f05f99 to 24fbead7105d (1 revision) (flutter/engine#53348) 2024-06-12 skia-flutter-autoroll@skia.org Roll Dart SDK from 269125c33a35 to b61536f05f99 (1 revision) (flutter/engine#53347) 2024-06-12 jonahwilliams@google.com [Impeller] disabling the color write mask seems to improve performance on iOS compared to just the blend options. (flutter/engine#53322) 2024-06-11 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from LS0Ff8PwQEGXzO0iA... to 5VG7IYkpANkfxMKYx... (flutter/engine#53346) 2024-06-11 skia-flutter-autoroll@skia.org Roll Dart SDK from ab4d5a0b789d to 269125c33a35 (1 revision) (flutter/engine#53344) 2024-06-11 30870216+gaaclarke@users.noreply.github.com [Impeller] makes bgra10xr test more comprehensive (flutter/engine#53320) 2024-06-11 skia-flutter-autoroll@skia.org Roll Skia from af907b24d873 to 51eabd0d1e44 (1 revision) (flutter/engine#53343) 2024-06-11 jonahwilliams@google.com [Impeller] add missing position to exp canvas text. (flutter/engine#53339) 2024-06-11 skia-flutter-autoroll@skia.org Roll Skia from 4cbc1129e587 to af907b24d873 (1 revision) (flutter/engine#53341) 2024-06-11 yjbanov@google.com Revert "[web] switch from .didGain/LoseAccessibilityFocus to .focus (â�¦ (flutter/engine#53342) 2024-06-11 jonahwilliams@google.com [Impeller] move some tests from aiks to display list. (flutter/engine#53336) 2024-06-11 skia-flutter-autoroll@skia.org Roll Skia from b5f974ec2dcd to 4cbc1129e587 (1 revision) (flutter/engine#53340) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from LS0Ff8PwQEGX to pGxbL7JoNb3y 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 chinmaygarde@google.com,rmistry@google.com,zra@google.com 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

…50199) flutter/engine@c7fcbfc...4cb3025 2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from 0fe7f7ac43c5 to ecc64fb82374 (1 revision) (flutter/engine#53374) 2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from 9ea9f37e8e1d to 0fe7f7ac43c5 (1 revision) (flutter/engine#53373) 2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from b12af56eb100 to 9ea9f37e8e1d (1 revision) (flutter/engine#53372) 2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from 704f70b7d756 to b12af56eb100 (1 revision) (flutter/engine#53371) 2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from 69a53db434a6 to 704f70b7d756 (2 revisions) (flutter/engine#53370) 2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from b2f163ae11bf to 69a53db434a6 (8 revisions) (flutter/engine#53365) 2024-06-13 jonahwilliams@google.com [Impeller] Move Gradient tests to display list. (flutter/engine#53345) 2024-06-13 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 5VG7IYkpANkfxMKYx... to pGxbL7JoNb3yAYFw4... (flutter/engine#53363) 2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from be6a57569a92 to b2f163ae11bf (5 revisions) (flutter/engine#53357) 2024-06-13 15619084+vashworth@users.noreply.github.com Give mac_host_engine tests a longer timeout and allow to run on Mac-14 (flutter/engine#53355) 2024-06-13 codefu@google.com 'Starter Project': port planet fragment shader to impeller tests (flutter/engine#53362) 2024-06-12 skia-flutter-autoroll@skia.org Roll Skia from 1ba0c3ebc7a5 to be6a57569a92 (1 revision) (flutter/engine#53353) 2024-06-12 skia-flutter-autoroll@skia.org Roll Dart SDK from 24fbead7105d to aa2265e5a192 (1 revision) (flutter/engine#53352) 2024-06-12 skia-flutter-autoroll@skia.org Roll Skia from 52d829ac6a0b to 1ba0c3ebc7a5 (1 revision) (flutter/engine#53351) 2024-06-12 skia-flutter-autoroll@skia.org Roll Skia from 5bfbacc89543 to 52d829ac6a0b (1 revision) (flutter/engine#53350) 2024-06-12 skia-flutter-autoroll@skia.org Roll Skia from 51eabd0d1e44 to 5bfbacc89543 (1 revision) (flutter/engine#53349) 2024-06-12 skia-flutter-autoroll@skia.org Roll Dart SDK from b61536f05f99 to 24fbead7105d (1 revision) (flutter/engine#53348) 2024-06-12 skia-flutter-autoroll@skia.org Roll Dart SDK from 269125c33a35 to b61536f05f99 (1 revision) (flutter/engine#53347) 2024-06-12 jonahwilliams@google.com [Impeller] disabling the color write mask seems to improve performance on iOS compared to just the blend options. (flutter/engine#53322) 2024-06-11 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from LS0Ff8PwQEGXzO0iA... to 5VG7IYkpANkfxMKYx... (flutter/engine#53346) 2024-06-11 skia-flutter-autoroll@skia.org Roll Dart SDK from ab4d5a0b789d to 269125c33a35 (1 revision) (flutter/engine#53344) 2024-06-11 30870216+gaaclarke@users.noreply.github.com [Impeller] makes bgra10xr test more comprehensive (flutter/engine#53320) 2024-06-11 skia-flutter-autoroll@skia.org Roll Skia from af907b24d873 to 51eabd0d1e44 (1 revision) (flutter/engine#53343) 2024-06-11 jonahwilliams@google.com [Impeller] add missing position to exp canvas text. (flutter/engine#53339) 2024-06-11 skia-flutter-autoroll@skia.org Roll Skia from 4cbc1129e587 to af907b24d873 (1 revision) (flutter/engine#53341) 2024-06-11 yjbanov@google.com Revert "[web] switch from .didGain/LoseAccessibilityFocus to .focus (â�¦ (flutter/engine#53342) 2024-06-11 jonahwilliams@google.com [Impeller] move some tests from aiks to display list. (flutter/engine#53336) 2024-06-11 skia-flutter-autoroll@skia.org Roll Skia from b5f974ec2dcd to 4cbc1129e587 (1 revision) (flutter/engine#53340) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from LS0Ff8PwQEGX to pGxbL7JoNb3y 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 chinmaygarde@google.com,rmistry@google.com,zra@google.com 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 2 commits June 11, 2024 14:51

[Impeller] move gradient tests to display list.

65f73eb

++

022f0d2

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

jonahwilliams added 6 commits June 11, 2024 15:04

fix matrix conversion.

f239a6d

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

e2a5048

cleanups.

6b2ed8d

reverse start and focus.

de4afa4

turn off disabled dithering tests.

47ba0ca

++

25cdf13

jonahwilliams requested a review from gaaclarke June 12, 2024 00:54

jonahwilliams added 2 commits June 11, 2024 18:06

fix reversed gradient.

b39d862

actually fix order

5b29716

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

chinmaygarde approved these changes Jun 12, 2024

View reviewed changes

gaaclarke reviewed Jun 12, 2024

View reviewed changes

jonahwilliams added 2 commits June 12, 2024 08:55

aaron review.

981ccda

++

aeb24f3

gaaclarke approved these changes Jun 12, 2024

View reviewed changes

Merge branch 'main' into delete_dead_code

41c7d8f

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

auto-submit bot merged commit bb231b4 into flutter:main Jun 13, 2024

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

Roll Flutter Engine from c7fcbfce608f to bb231b4dac3d (22 revisions) flutter/flutter#150159

Closed

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

bb231b4 [Impeller] Move Gradient tests to display list. (flutter/engi…

2aaf0db

…ne#53345)

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

Roll Flutter Engine from c7fcbfce608f to fd693d94bcee (23 revisions) flutter/flutter#150161

Closed

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

bb231b4 [Impeller] Move Gradient tests to display list. (flutter/engi…

dd2a6d3

…ne#53345)

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

Roll Flutter Engine from c7fcbfce608f to fd693d94bcee (23 revisions) flutter/flutter#150166

Closed

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

bb231b4 [Impeller] Move Gradient tests to display list. (flutter/engi…

d9ce3dd

…ne#53345)

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

Roll Flutter Engine from c7fcbfce608f to eeb939e65202 (24 revisions) flutter/flutter#150170

Closed

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

bb231b4 [Impeller] Move Gradient tests to display list. (flutter/engi…

af05db5

…ne#53345)

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

Roll Flutter Engine from c7fcbfce608f to 9ac423829e22 (25 revisions) flutter/flutter#150178

Closed

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

bb231b4 [Impeller] Move Gradient tests to display list. (flutter/engi…

ce82a95

…ne#53345)

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

Roll Flutter Engine from c7fcbfce608f to 29fb5e3667f0 (26 revisions) flutter/flutter#150188

Closed

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

bb231b4 [Impeller] Move Gradient tests to display list. (flutter/engi…

52ee0fc

…ne#53345)

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

Roll Flutter Engine from c7fcbfce608f to 4a5d4f8aa6d2 (27 revisions) flutter/flutter#150190

Closed

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

bb231b4 [Impeller] Move Gradient tests to display list. (flutter/engi…

0678db0

…ne#53345)

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

Roll Flutter Engine from c7fcbfce608f to 4a5d4f8aa6d2 (27 revisions) flutter/flutter#150195

Closed

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

bb231b4 [Impeller] Move Gradient tests to display list. (flutter/engi…

2b48b2c

…ne#53345)

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

Roll Flutter Engine from c7fcbfce608f to 4cb3025d3abf (28 revisions) flutter/flutter#150199

Merged

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

bb231b4 [Impeller] Move Gradient tests to display list. (flutter/engi…

2d17f66

…ne#53345)

	void CanRenderLinearGradient(AiksTest* aiks_test, DlTileMode tile_mode) {
	/// Test body for linear gradient tile mode tests (ex. CanRenderLinearGradientClamp).
	void CanRenderLinearGradient(AiksTest* aiks_test, DlTileMode tile_mode) {

	static Matrix ToMatrix(const SkMatrix& m) {
	Matrix ToMatrix(const SkMatrix& m) {

[Impeller] Move Gradient tests to display list. #53345

[Impeller] Move Gradient tests to display list. #53345

Uh oh!

Conversation

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

Uh oh!

Uh oh!

flutter-dashboard bot commented Jun 12, 2024

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

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

gaaclarke commented Jun 12, 2024

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard bot commented Jun 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jonahwilliams commented Jun 11, 2024 •

edited

Loading