Adds wide gamut saveLayer integration test #120131

gaaclarke · 2023-02-06T21:48:58Z

integration test for: flutter/engine#39482

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

bdero · 2023-02-14T00:54:03Z

dev/integration_tests/wide_gamut_test/lib/main.dart

+      canvas.drawImage(_image!,
+          Offset(-_image!.width / 2.0, -_image!.height / 2.0), Paint());
+      canvas.restore();
+      canvas.restore();


Impeller doesn't do so today, but I think it would be possible to optimize the second savelayer away by just reversing the draw order and swapping the blend mode.

A reasonably future proof way to force a savelayer would be to draw 4 overlapping objects:

Draw two blended things in the background,

append a savelayer that applies using a second blend mode,

and draw two blended things within the savelayer that use a third blend mode.

For example:

Draw an image in the background

Cover the whole image with a partially translucent rectangle (with just kSrcOver, or any non-destructive blend mode)

Push a savelayer with a non-separable blend mode such as kHue or kLuminosity

Draw two partially overlapping circles within the image area using a simple separable blend mode, such as kPlus

Impeller basically has no choice but to render something offscreen for this, regardless of what nutty optimizations we manage to come up with.

Are you sure the offscreen buffer could be optimized away? dstOver is a raster operation. It can't be calculated without having raster images which require offscreen buffers to create. I know we can look at the results and say there is an alternative way to get the same result. I'm not sure it's feasible to create such an optimization at runtime though.

Are you sure the offscreen buffer could be optimized away?

The first savelayer is almost a no-op because the background is empty and the blend mode is srcOver. It does have a bounds hint, but that can be handled by inserting a clip rect.

The second savelayer only contains one srcOver draw within it. In this situation we can always remove the savelayer and replace the one draw call's blend mode with the savelayer's blend mode.

dstOver is a raster operation. It can't be calculated without having raster images which require offscreen buffers to create.

dstOver (and all of the other "Porter-Duff" composites) are implemented using the pipeline blend config in Impeller, and so the blend source doesn't actually need to be rendered to a separate texture first (see this doc for more deets about the blending situation).

Thanks, reading through the docs it looks like multiply is not able to be optimized similarly since it is operating on the colors, not just the alpha values. So I've switched the test to use multiply. I think that solves the problem, let me know if I'm missing something.

The second savelayer can be optimized away regardless of the blend mode.
And for some targets (Vulkan and OpenGL with common extensions), well be adding a path to implement all of the blend modes as pipeline ops.

If you don't care about testing the savelayer path specifically and just want to force a scenario where something draws to an offscreen texture, a safer option would be to blur an image with a reasonably large sigma.

bdero

LGTM -- up to you with how deep you want to go with future proofing.

…20131)

…#120131)

…20131)

* 3ad7ea3 Roll Plugins from 9c312d4d2f5f to 2ce625f1a87e (5 revisions) (flutter/flutter#120793) * 7865713 Roll Flutter Engine from 1328c4bc6299 to 4db9673d48d6 (2 revisions) (flutter/flutter#120796) * 541a8bf Fix switching from scrollable and non-scrollable tab bars throws (flutter/flutter#120771) * ab1390e Use black30 for CupertinoTabBar's border (flutter/flutter#119509) * a513d4e Fix `flutter_localizations` README references (flutter/flutter#120800) * a664f08 In test of --(no-)fatal-infos analyzer flags, pin missing_return to info (flutter/flutter#120797) * ef49f56 Add Android unit tests to plugin template (flutter/flutter#120720) * a12e242 Improve CupertinoContextMenu to match native more (flutter/flutter#117698) * a9f4366 Fix the `flutter run -d linux` tests (flutter/flutter#120721) * dff0955 09da59a5a Roll Dart SDK from c022d475e9d8 to 5d17a336bdfe (1 revision) (flutter/engine#39649) (flutter/flutter#120816) * f35de0c Adds wide gamut saveLayer integration test (flutter/flutter#120131) * 99dcaa2 Roll Flutter Engine from 09da59a5adcf to a8b3d1af55b6 (3 revisions) (flutter/flutter#120821) * 8d15083 Use the impellerc GLES output flag when compiling shaders for Android (flutter/flutter#120647) * c6b636f [flutter_tools] Replace Future.catchError() with Future.then(onError: ...) (flutter/flutter#120637) * 2b7d709 Add `@widgetFactory` annotation (flutter/flutter#117455) * e65dfba Add Linux unit tests to plugin template (flutter/flutter#120814) * dccec41 5de007b90 Remove "bringup: true" from "Linux Fuchsia FEMU" (flutter/engine#39651) (flutter/flutter#120826) * d6de6bc 9f3b061b7 Roll buildroot to 64b0c3deecaff8e66c2deb74e2171e8297b2bfcd (flutter/engine#39653) (flutter/flutter#120830) * da2508c bb1ff84b6 Add a white background to app anatomy diagram (flutter/engine#39638) (flutter/flutter#120832) * 1f85497 [flutter_tools] Add the NoProfile parameter to the PowerShell execution statement (flutter/flutter#120786) * 4ad47fb Fix `StretchingOverscrollIndicator` not handling directional changes correctly (flutter/flutter#116548) * 9a721c4 Update AndroidManifest.xml.tmpl (flutter/flutter#120527) * c0b7d2d Roll Flutter Engine from bb1ff84b6c4f to 02a379db1d38 (4 revisions) (flutter/flutter#120845) * a10e295 Added identical(a,b) short circuit to Material Library lerp methods (flutter/flutter#120829) * efde350 Roll Flutter Engine from 02a379db1d38 to a966cf878ffd (2 revisions) (flutter/flutter#120846) * cc473e4 Roll Flutter Engine from a966cf878ffd to 3fc40ca5beb9 (3 revisions) (flutter/flutter#120850) * d125242 Roll Flutter Engine from 3fc40ca5beb9 to 9fa2a5c3cfbd (2 revisions) (flutter/flutter#120856) * 22e17bb ea1d087c4 Roll Skia from b8b36146c7a0 to 7b3fb04bc3d4 (3 revisions) (flutter/engine#39673) (flutter/flutter#120860) * f85438b c8b1d2ffa Roll Fuchsia Mac SDK from YpQKlqmyn8r_snx06... to xl9Y8o-9FDyvPogki... (flutter/engine#39675) (flutter/flutter#120887) * 174a562 d699b4a91 Roll Flutter from e3471f0 to df41e58 (83 revisions) (flutter/plugins#7184) (flutter/flutter#120888) * 170539f Roll Flutter Engine from c8b1d2ffaec8 to 0d8d93306822 (2 revisions) (flutter/flutter#120891)

* 3ad7ea3c9 Roll Plugins from 9c312d4 to 2ce625f (5 revisions) (flutter/flutter#120793) * 786571368 Roll Flutter Engine from 1328c4bc6299 to 4db9673d48d6 (2 revisions) (flutter/flutter#120796) * 541a8bfd9 Fix switching from scrollable and non-scrollable tab bars throws (flutter/flutter#120771) * ab1390e0a Use black30 for CupertinoTabBar's border (flutter/flutter#119509) * a513d4e7b Fix `flutter_localizations` README references (flutter/flutter#120800) * a664f08a5 In test of --(no-)fatal-infos analyzer flags, pin missing_return to info (flutter/flutter#120797) * ef49f5661 Add Android unit tests to plugin template (flutter/flutter#120720) * a12e242c0 Improve CupertinoContextMenu to match native more (flutter/flutter#117698) * a9f43665c Fix the `flutter run -d linux` tests (flutter/flutter#120721) * dff09558d 09da59a5a Roll Dart SDK from c022d475e9d8 to 5d17a336bdfe (1 revision) (flutter/engine#39649) (flutter/flutter#120816) * f35de0c80 Adds wide gamut saveLayer integration test (flutter/flutter#120131) * 99dcaa2d9 Roll Flutter Engine from 09da59a5adcf to a8b3d1af55b6 (3 revisions) (flutter/flutter#120821) * 8d150833b Use the impellerc GLES output flag when compiling shaders for Android (flutter/flutter#120647) * c6b636fa5 [flutter_tools] Replace Future.catchError() with Future.then(onError: ...) (flutter/flutter#120637) * 2b7d709fd Add `@widgetFactory` annotation (flutter/flutter#117455) * e65dfba8e Add Linux unit tests to plugin template (flutter/flutter#120814) * dccec41d5 5de007b90 Remove "bringup: true" from "Linux Fuchsia FEMU" (flutter/engine#39651) (flutter/flutter#120826) * d6de6bc68 9f3b061b7 Roll buildroot to 64b0c3deecaff8e66c2deb74e2171e8297b2bfcd (flutter/engine#39653) (flutter/flutter#120830) * da2508c9f bb1ff84b6 Add a white background to app anatomy diagram (flutter/engine#39638) (flutter/flutter#120832) * 1f85497ef [flutter_tools] Add the NoProfile parameter to the PowerShell execution statement (flutter/flutter#120786) * 4ad47fb47 Fix `StretchingOverscrollIndicator` not handling directional changes correctly (flutter/flutter#116548) * 9a721c456 Update AndroidManifest.xml.tmpl (flutter/flutter#120527) * c0b7d2ddd Roll Flutter Engine from bb1ff84b6c4f to 02a379db1d38 (4 revisions) (flutter/flutter#120845) * a10e295a0 Added identical(a,b) short circuit to Material Library lerp methods (flutter/flutter#120829) * efde35081 Roll Flutter Engine from 02a379db1d38 to a966cf878ffd (2 revisions) (flutter/flutter#120846) * cc473e4f1 Roll Flutter Engine from a966cf878ffd to 3fc40ca5beb9 (3 revisions) (flutter/flutter#120850) * d1252428c Roll Flutter Engine from 3fc40ca5beb9 to 9fa2a5c3cfbd (2 revisions) (flutter/flutter#120856) * 22e17bb71 ea1d087c4 Roll Skia from b8b36146c7a0 to 7b3fb04bc3d4 (3 revisions) (flutter/engine#39673) (flutter/flutter#120860)

…20131)

…r#7186) * 3ad7ea3c9 Roll Plugins from 9c312d4 to 2ce625f (5 revisions) (flutter/flutter#120793) * 786571368 Roll Flutter Engine from 1328c4bc6299 to 4db9673d48d6 (2 revisions) (flutter/flutter#120796) * 541a8bfd9 Fix switching from scrollable and non-scrollable tab bars throws (flutter/flutter#120771) * ab1390e0a Use black30 for CupertinoTabBar's border (flutter/flutter#119509) * a513d4e7b Fix `flutter_localizations` README references (flutter/flutter#120800) * a664f08a5 In test of --(no-)fatal-infos analyzer flags, pin missing_return to info (flutter/flutter#120797) * ef49f5661 Add Android unit tests to plugin template (flutter/flutter#120720) * a12e242c0 Improve CupertinoContextMenu to match native more (flutter/flutter#117698) * a9f43665c Fix the `flutter run -d linux` tests (flutter/flutter#120721) * dff09558d 09da59a5a Roll Dart SDK from c022d475e9d8 to 5d17a336bdfe (1 revision) (flutter/engine#39649) (flutter/flutter#120816) * f35de0c80 Adds wide gamut saveLayer integration test (flutter/flutter#120131) * 99dcaa2d9 Roll Flutter Engine from 09da59a5adcf to a8b3d1af55b6 (3 revisions) (flutter/flutter#120821) * 8d150833b Use the impellerc GLES output flag when compiling shaders for Android (flutter/flutter#120647) * c6b636fa5 [flutter_tools] Replace Future.catchError() with Future.then(onError: ...) (flutter/flutter#120637) * 2b7d709fd Add `@widgetFactory` annotation (flutter/flutter#117455) * e65dfba8e Add Linux unit tests to plugin template (flutter/flutter#120814) * dccec41d5 5de007b90 Remove "bringup: true" from "Linux Fuchsia FEMU" (flutter/engine#39651) (flutter/flutter#120826) * d6de6bc68 9f3b061b7 Roll buildroot to 64b0c3deecaff8e66c2deb74e2171e8297b2bfcd (flutter/engine#39653) (flutter/flutter#120830) * da2508c9f bb1ff84b6 Add a white background to app anatomy diagram (flutter/engine#39638) (flutter/flutter#120832) * 1f85497ef [flutter_tools] Add the NoProfile parameter to the PowerShell execution statement (flutter/flutter#120786) * 4ad47fb47 Fix `StretchingOverscrollIndicator` not handling directional changes correctly (flutter/flutter#116548) * 9a721c456 Update AndroidManifest.xml.tmpl (flutter/flutter#120527) * c0b7d2ddd Roll Flutter Engine from bb1ff84b6c4f to 02a379db1d38 (4 revisions) (flutter/flutter#120845) * a10e295a0 Added identical(a,b) short circuit to Material Library lerp methods (flutter/flutter#120829) * efde35081 Roll Flutter Engine from 02a379db1d38 to a966cf878ffd (2 revisions) (flutter/flutter#120846) * cc473e4f1 Roll Flutter Engine from a966cf878ffd to 3fc40ca5beb9 (3 revisions) (flutter/flutter#120850) * d1252428c Roll Flutter Engine from 3fc40ca5beb9 to 9fa2a5c3cfbd (2 revisions) (flutter/flutter#120856) * 22e17bb71 ea1d087c4 Roll Skia from b8b36146c7a0 to 7b3fb04bc3d4 (3 revisions) (flutter/engine#39673) (flutter/flutter#120860)

flutter-dashboard bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Feb 6, 2023

gaaclarke force-pushed the wide-gamut-save-layer-integration-test branch from 2d0965a to ffceaee Compare February 6, 2023 23:02

gaaclarke mentioned this pull request Feb 8, 2023

Added wide gamut colors to offscreen buffers flutter/engine#39482

Merged

8 tasks

gaaclarke added 2 commits February 13, 2023 14:50

Added wide gamut integration test that uses save layers.

2127d8b

updated the test to support bgra too

c561624

gaaclarke force-pushed the wide-gamut-save-layer-integration-test branch from 0b2da13 to c561624 Compare February 13, 2023 22:51

gaaclarke requested a review from bdero February 13, 2023 22:52

analysis errors

56b520f

bdero reviewed Feb 14, 2023

View reviewed changes

gaaclarke marked this pull request as ready for review February 14, 2023 01:06

bdero approved these changes Feb 14, 2023

View reviewed changes

switched blend mode to multiply to avoid future optimizations

b67e0e5

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 15, 2023

auto-submit bot merged commit f35de0c into flutter:master Feb 15, 2023

This was referenced Feb 16, 2023

Roll Flutter from df41e58f6f4e to 22e17bb71050 (28 revisions) flutter/packages#3211

Closed

Roll Flutter from df41e58f6f4e to 22e17bb71050 (28 revisions) flutter/plugins#7186

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2023

f35de0c Adds wide gamut saveLayer integration test (flutter/flutter#1…

90f7b6b

…20131)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 16, 2023

f35de0c80 Adds wide gamut saveLayer integration test (flutter/flutter…

d972a54

…#120131)

engine-flutter-autoroll mentioned this pull request Feb 16, 2023

Roll Flutter from df41e58f6f4e to 174a562a665d (30 revisions) flutter/packages#3212

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2023

f35de0c Adds wide gamut saveLayer integration test (flutter/flutter#1…

97f89a8

…20131)

engine-flutter-autoroll mentioned this pull request Feb 16, 2023

Roll Flutter from df41e58f6f4e to 170539f83dc8 (31 revisions) flutter/packages#3214

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2023

f35de0c Adds wide gamut saveLayer integration test (flutter/flutter#1…

cb6e5b1

…20131)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

f35de0c Adds wide gamut saveLayer integration test (flutter/flutter#1…

f89e9f7

…20131)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

f35de0c Adds wide gamut saveLayer integration test (flutter/flutter#1…

9a5e579

…20131)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Adds wide gamut saveLayer integration test #120131

Adds wide gamut saveLayer integration test #120131

Uh oh!

gaaclarke commented Feb 6, 2023 •

edited

Loading

Uh oh!

bdero Feb 14, 2023 •

edited

Loading

Uh oh!

gaaclarke Feb 14, 2023

Uh oh!

bdero Feb 14, 2023

Uh oh!

gaaclarke Feb 15, 2023

Uh oh!

bdero Feb 15, 2023

Uh oh!

bdero left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Adds wide gamut saveLayer integration test #120131

Adds wide gamut saveLayer integration test #120131

Uh oh!

Conversation

gaaclarke commented Feb 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

bdero Feb 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaaclarke Feb 14, 2023

Choose a reason for hiding this comment

Uh oh!

bdero Feb 14, 2023

Choose a reason for hiding this comment

Uh oh!

gaaclarke Feb 15, 2023

Choose a reason for hiding this comment

Uh oh!

bdero Feb 15, 2023

Choose a reason for hiding this comment

Uh oh!

bdero left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gaaclarke commented Feb 6, 2023 •

edited

Loading

bdero Feb 14, 2023 •

edited

Loading