Implement opacity `FlutterMutator` for hcpp #164147

gmackall · 2025-02-25T22:10:17Z

This is essentially a reland of flutter/engine#30264, except that it only applies the opacity if the new alpha (alpha = 255*opacity, where opacity∈[0,1]) is different from the current alpha.

Also adds a new opacity screenshot test. Need to figure out how to bring up a new screenshot test... does the screenshot get uploaded automatically? Do I need to manually upload somewhere?

Fixes #164212

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.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

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

jonahwilliams · 2025-02-25T22:32:53Z

...ter/shell/platform/android/io/flutter/embedding/engine/mutatorsstack/FlutterMutatorView.java

    }
+    if (mutatorsStack.getFinalOpacity() != 1.f) {
+      paint.setAlpha((int) (255 * mutatorsStack.getFinalOpacity()));
+      this.setLayerType(View.LAYER_TYPE_HARDWARE, paint);


I think this isn't really necessary. according to https://developer.android.com/develop/ui/views/graphics/hardware-accel LAYER_TYPE_HARDWARE is the default (I think?)

Hmm, I tried without it (instead just using setLayerPaint, and it didn't work.

From what I can tell the default is LAYER_TYPE_NONE:
https://developer.android.com/develop/ui/views/graphics/hardware-accel#layers

LAYER_TYPE_NONE: The view is rendered normally and is not backed by an off-screen buffer. This is the default behavior.

And that's also the type I see when I log it out with this.getLayerType(). When the type is none, it looks like setLayerPaint does nothing:

Updates the Paint object used with the current layer (used only if the current layer type is not set to LAYER_TYPE_NONE).

So I think it either needs to be this, or we could also try using View.setAlpha, which would probably do a similar thing?

Also, it's occurred to me that this if statement will prevent us ever getting back to 1 opacity, so I'll need to fix that

ahh good research :)

That was probably part of the regression. If we toggle to offscreen layer for opacity, we need to toggle back if there is no alpha.

I think View.setAlpha might not impact child views? No idea. If this approach works I'd stick with it.

Ok, from the docs of View.setAlpha()

For a view with a frequently changing alpha, such as during a fading animation, it is strongly recommended for performance reasons to either override hasOverlappingRendering() to return false if appropriate, or setting a layer type on the view for the duration of the animation. On versions Build. VERSION_CODES. M and below, the default path for rendering an unlayered View with alpha could add multiple milliseconds of rendering cost, even for simple or small views. Starting with Build. VERSION_CODES. M, LAYER_TYPE_HARDWARE is automatically applied to the view at the rendering level.

It looks like this approach is recommended for < API 23, and equivalent for others? So I'm happy to continue using setLayerType as written

I would only worry about more modern devices. But yeah, I'm not enough of an Android expert to say which is the best practice, so I'd defer to the Android team's judgement :)

jonahwilliams

Not related to ...but it looks like the transform stuff is only partially hooked up too 🙈

jonahwilliams · 2025-02-26T03:07:16Z

dev/integration_tests/android_engine_test/test_driver/hcpp/platform_view_opacity_main_test.dart

+    expect(response['supported'], true);
+  }, timeout: Timeout.none);
+
+  test('should screenshot a rectangle with specified opacity', () async {


Lets add one that adds an opacity and shows it can be toggled off

jonahwilliams · 2025-02-26T18:34:14Z

Oh to answer the question, I believe now that this is non-draft skia gold will ping with the updated goldens. If not we can figure out if we're missing some wiring.

reidbaker · 2025-02-26T18:52:11Z

...r/shell/platform/android/io/flutter/embedding/engine/mutatorsstack/FlutterMutatorsStack.java

+    /**
+     * Get the opacity of the mutator if the {@link #getType()} returns FlutterMutatorType.OPACITY.
+     *
+     * @return the opacity if the type is FlutterMutatorType.OPACITY; otherwise null.


I dont think you can return null from this method.

If this is a mutator not of type FlutterMutatorType.OPACITY, this will return null, no?

since float is a java primitive type I believe it will actually return 0, right?

In any case, 1.0 is a safer default.

It would need to return Float to have null as an option. I agree with @jonahwilliams though that 1.0f is a safer default.

reidbaker · 2025-02-26T18:57:20Z

...r/shell/platform/android/io/flutter/embedding/engine/mutatorsstack/FlutterMutatorsStack.java

    @Nullable private Rect rect;
    @Nullable private Path path;
    @Nullable private float[] radiis;
+    @Nullable private float opacity;


Somewhere in this pr can you javadoc what the expected range of values of opacity can be and what you do if the values are invalid (like negative)

jonahwilliams · 2025-02-26T19:38:15Z

You can click the gold check to see if the images are ready before it pings the PR btw: https://flutter-gold.skia.org/search?issue=164147&crs=github&patchsets=7&corpus=flutter

Seems to be working?

jonahwilliams

LGTM

auto-submit · 2025-02-26T21:21:04Z

autosubmit label was removed for flutter/flutter/164147, because - The status or check suite Linux linux_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

reidbaker

I think you need a ci.yaml entry and as checked in you will have a test that isnt run by ci.

jonahwilliams · 2025-02-26T21:25:14Z

You don't need a CI yaml entry, there is already a task written that runs all the driver tests in that application directory (since this doesn't run in the devicelab).

Hence the golden file update triggering.

reidbaker · 2025-02-26T21:33:52Z

You don't need a CI yaml entry, there is already a task written that runs all the driver tests in that application directory (since this doesn't run in the devicelab).

Hence the golden file update triggering.

Sorry you are correct I was trying to catch an error that bit me a few time.

@gmackall consider updating the readme like the other tests.
https://github.com/flutter/flutter/blob/eb3f171e136b88e77320778a56e40ecac8166954/dev/integration_tests/android_engine_test/README.md

…164147)

Roll Flutter from 1659206 to 2e570ca (75 revisions) flutter/flutter@1659206...2e570ca 2025-02-27 [email protected] Roll Skia from ac08df9c8d82 to e5a33102401c (1 revision) (flutter/flutter#164288) 2025-02-27 [email protected] Roll Skia from 6018dff0233a to ac08df9c8d82 (1 revision) (flutter/flutter#164275) 2025-02-27 [email protected] Roll Dart SDK from 4a218fbffc80 to fcda71ce147b (1 revision) (flutter/flutter#164276) 2025-02-27 [email protected] Roll Skia from 0f2e106c9abc to 6018dff0233a (1 revision) (flutter/flutter#164266) 2025-02-27 [email protected] Add windowing channel support to Linux embedder (flutter/flutter#163180) 2025-02-27 [email protected] Roll Dart SDK from 7fa5901bd8a3 to 4a218fbffc80 (2 revisions) (flutter/flutter#164260) 2025-02-27 [email protected] Roll Skia from fdd97386193e to 0f2e106c9abc (4 revisions) (flutter/flutter#164259) 2025-02-27 [email protected] Run more builds faster (flutter/flutter#164125) 2025-02-27 [email protected] Increase customer test timeout to 60 minutes (flutter/flutter#164239) 2025-02-27 [email protected] Reland "Add a buildtools directory and move third_party/ninja to the project root in order to match the expectations of depot_tools" (flutter/flutter#164240) 2025-02-27 [email protected] Use the Dart isolate ownership API on the root isolate (flutter/flutter#163703) 2025-02-26 [email protected] Roll Fuchsia Linux SDK from g-2SJtblPjjaH7Egy... to 1elkOxihZuTEiTXzY... (flutter/flutter#164243) 2025-02-26 [email protected] Show Linux driver information in flutter doctor (flutter/flutter#163980) 2025-02-26 [email protected] Implement opacity `FlutterMutator` for hcpp (flutter/flutter#164147) 2025-02-26 [email protected] Roll Fuchsia Linux SDK from g-2SJtblPjjaH7Egy... to 1elkOxihZuTEiTXzY... (flutter/flutter#164232) 2025-02-26 [email protected] Roll Skia from 47b84f354604 to fdd97386193e (8 revisions) (flutter/flutter#164221) 2025-02-26 [email protected] [Impeller] Reland: move AHB check into Flutter main, don't disable ImageReader on 29. (flutter/flutter#164201) 2025-02-26 [email protected] Roll Dart SDK from 80865748abe0 to 7fa5901bd8a3 (3 revisions) (flutter/flutter#164226) 2025-02-26 [email protected] Update fuchsia_test_scripts_version to the latest version (flutter/flutter#164123) 2025-02-26 [email protected] Guard against zero item extent for carousel (flutter/flutter#163310) 2025-02-26 [email protected] [Impeller] work around for crashy Nexus 5 Driver. (flutter/flutter#164040) 2025-02-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add a buildtools directory and move third_party/ninja to the project root in order to match the expectations of depot_tools (#163890)" (flutter/flutter#164209) 2025-02-26 [email protected] Add localization for `Back` and `Cancel` buttons in CupertinoNavigationBar (flutter/flutter#162581) 2025-02-26 [email protected] Added compilation failure if a max ubo size is exceeded (flutter/flutter#164038) 2025-02-26 [email protected] Roll Skia from 5336e20498d9 to 47b84f354604 (2 revisions) (flutter/flutter#164188) 2025-02-26 [email protected] Add a buildtools directory and move third_party/ninja to the project root in order to match the expectations of depot_tools (flutter/flutter#163890) 2025-02-26 [email protected] Roll Skia from 3ce6f25dc13e to 5336e20498d9 (9 revisions) (flutter/flutter#164174) 2025-02-26 [email protected] Roll Dart SDK from b8292dfeaa67 to 80865748abe0 (9 revisions) (flutter/flutter#164168) 2025-02-26 [email protected] [Engine] Remove dead code for RoundedSuperellipse (flutter/flutter#164163) 2025-02-26 [email protected] [Impeller] detect mediatek soc and fall back to GLES. (flutter/flutter#164126) 2025-02-26 [email protected] [Impeller] make DLOG into LOG for startup errors. (flutter/flutter#164110) 2025-02-26 [email protected] Intercept error when iOS 18.4 crashes with JIT mode and give guided error (flutter/flutter#164072) 2025-02-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] move AHB check to Vulkan, use Vulkan surface on 29. (#164109)" (flutter/flutter#164166) 2025-02-26 [email protected] Run new gallery transition perf benchmark on Galaxy S24 (flutter/flutter#163665) 2025-02-26 [email protected] [Impeller] move AHB check to Vulkan, use Vulkan surface on 29. (flutter/flutter#164109) 2025-02-26 [email protected] [iOS] Add platform view to integration_test example (flutter/flutter#164144) 2025-02-26 [email protected] Fix minor issues in documentation of WidgetsApp (flutter/flutter#163942) 2025-02-26 [email protected] Fix extra numbers showing up when enabling VoiceControl (flutter/flutter#163593) 2025-02-26 [email protected] Set SliverResizingHeader's maxScrollObstructionExtent to minExtent (flutter/flutter#162955) 2025-02-26 [email protected] Roll Dart SDK from aea6fff33f06 to b8292dfeaa67 (1 revision) (flutter/flutter#163973) 2025-02-26 [email protected] [DisplayList] Delete all legacy Skia-oriented method overloads in DlCanvas (flutter/flutter#164054) 2025-02-25 [email protected] Clean up leak tracker instrumentation tech debt. (flutter/flutter#164070) 2025-02-25 [email protected] Revert "Marks Linux_pixel_7pro service_extensions_test to be flaky" (flutter/flutter#163882) 2025-02-25 [email protected] Check if simctl is installed before trying to list devices or runtimes (flutter/flutter#163895) 2025-02-25 [email protected] Update dragDevices doc to include default PointerDeviceKind.trackpad (flutter/flutter#163898) 2025-02-25 [email protected] Update multiple flutters benchmark test to latest gradle and agp and gradle defined dependencies (flutter/flutter#164029) ...

…164147)

Roll Flutter from 1659206 to 2e570ca (75 revisions) flutter/flutter@1659206...2e570ca 2025-02-27 [email protected] Roll Skia from ac08df9c8d82 to e5a33102401c (1 revision) (flutter/flutter#164288) 2025-02-27 [email protected] Roll Skia from 6018dff0233a to ac08df9c8d82 (1 revision) (flutter/flutter#164275) 2025-02-27 [email protected] Roll Dart SDK from 4a218fbffc80 to fcda71ce147b (1 revision) (flutter/flutter#164276) 2025-02-27 [email protected] Roll Skia from 0f2e106c9abc to 6018dff0233a (1 revision) (flutter/flutter#164266) 2025-02-27 [email protected] Add windowing channel support to Linux embedder (flutter/flutter#163180) 2025-02-27 [email protected] Roll Dart SDK from 7fa5901bd8a3 to 4a218fbffc80 (2 revisions) (flutter/flutter#164260) 2025-02-27 [email protected] Roll Skia from fdd97386193e to 0f2e106c9abc (4 revisions) (flutter/flutter#164259) 2025-02-27 [email protected] Run more builds faster (flutter/flutter#164125) 2025-02-27 [email protected] Increase customer test timeout to 60 minutes (flutter/flutter#164239) 2025-02-27 [email protected] Reland "Add a buildtools directory and move third_party/ninja to the project root in order to match the expectations of depot_tools" (flutter/flutter#164240) 2025-02-27 [email protected] Use the Dart isolate ownership API on the root isolate (flutter/flutter#163703) 2025-02-26 [email protected] Roll Fuchsia Linux SDK from g-2SJtblPjjaH7Egy... to 1elkOxihZuTEiTXzY... (flutter/flutter#164243) 2025-02-26 [email protected] Show Linux driver information in flutter doctor (flutter/flutter#163980) 2025-02-26 [email protected] Implement opacity `FlutterMutator` for hcpp (flutter/flutter#164147) 2025-02-26 [email protected] Roll Fuchsia Linux SDK from g-2SJtblPjjaH7Egy... to 1elkOxihZuTEiTXzY... (flutter/flutter#164232) 2025-02-26 [email protected] Roll Skia from 47b84f354604 to fdd97386193e (8 revisions) (flutter/flutter#164221) 2025-02-26 [email protected] [Impeller] Reland: move AHB check into Flutter main, don't disable ImageReader on 29. (flutter/flutter#164201) 2025-02-26 [email protected] Roll Dart SDK from 80865748abe0 to 7fa5901bd8a3 (3 revisions) (flutter/flutter#164226) 2025-02-26 [email protected] Update fuchsia_test_scripts_version to the latest version (flutter/flutter#164123) 2025-02-26 [email protected] Guard against zero item extent for carousel (flutter/flutter#163310) 2025-02-26 [email protected] [Impeller] work around for crashy Nexus 5 Driver. (flutter/flutter#164040) 2025-02-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add a buildtools directory and move third_party/ninja to the project root in order to match the expectations of depot_tools (#163890)" (flutter/flutter#164209) 2025-02-26 [email protected] Add localization for `Back` and `Cancel` buttons in CupertinoNavigationBar (flutter/flutter#162581) 2025-02-26 [email protected] Added compilation failure if a max ubo size is exceeded (flutter/flutter#164038) 2025-02-26 [email protected] Roll Skia from 5336e20498d9 to 47b84f354604 (2 revisions) (flutter/flutter#164188) 2025-02-26 [email protected] Add a buildtools directory and move third_party/ninja to the project root in order to match the expectations of depot_tools (flutter/flutter#163890) 2025-02-26 [email protected] Roll Skia from 3ce6f25dc13e to 5336e20498d9 (9 revisions) (flutter/flutter#164174) 2025-02-26 [email protected] Roll Dart SDK from b8292dfeaa67 to 80865748abe0 (9 revisions) (flutter/flutter#164168) 2025-02-26 [email protected] [Engine] Remove dead code for RoundedSuperellipse (flutter/flutter#164163) 2025-02-26 [email protected] [Impeller] detect mediatek soc and fall back to GLES. (flutter/flutter#164126) 2025-02-26 [email protected] [Impeller] make DLOG into LOG for startup errors. (flutter/flutter#164110) 2025-02-26 [email protected] Intercept error when iOS 18.4 crashes with JIT mode and give guided error (flutter/flutter#164072) 2025-02-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] move AHB check to Vulkan, use Vulkan surface on 29. (#164109)" (flutter/flutter#164166) 2025-02-26 [email protected] Run new gallery transition perf benchmark on Galaxy S24 (flutter/flutter#163665) 2025-02-26 [email protected] [Impeller] move AHB check to Vulkan, use Vulkan surface on 29. (flutter/flutter#164109) 2025-02-26 [email protected] [iOS] Add platform view to integration_test example (flutter/flutter#164144) 2025-02-26 [email protected] Fix minor issues in documentation of WidgetsApp (flutter/flutter#163942) 2025-02-26 [email protected] Fix extra numbers showing up when enabling VoiceControl (flutter/flutter#163593) 2025-02-26 [email protected] Set SliverResizingHeader's maxScrollObstructionExtent to minExtent (flutter/flutter#162955) 2025-02-26 [email protected] Roll Dart SDK from aea6fff33f06 to b8292dfeaa67 (1 revision) (flutter/flutter#163973) 2025-02-26 [email protected] [DisplayList] Delete all legacy Skia-oriented method overloads in DlCanvas (flutter/flutter#164054) 2025-02-25 [email protected] Clean up leak tracker instrumentation tech debt. (flutter/flutter#164070) 2025-02-25 [email protected] Revert "Marks Linux_pixel_7pro service_extensions_test to be flaky" (flutter/flutter#163882) 2025-02-25 [email protected] Check if simctl is installed before trying to list devices or runtimes (flutter/flutter#163895) 2025-02-25 [email protected] Update dragDevices doc to include default PointerDeviceKind.trackpad (flutter/flutter#163898) 2025-02-25 [email protected] Update multiple flutters benchmark test to latest gradle and agp and gradle defined dependencies (flutter/flutter#164029) ...

Gray Mackall added 2 commits February 25, 2025 14:08

initial impl

6bf5681

format

9e68674

github-actions bot added platform-android Android applications specifically engine flutter/engine related. See also e: labels. labels Feb 25, 2025

jonahwilliams reviewed Feb 25, 2025

View reviewed changes

Gray Mackall and others added 3 commits February 25, 2025 17:50

change logic so that we only apply opacity if it is a new opacity

823ae59

add new opacity test

f9942a7

Merge branch 'master' into opacity

5edfa39

gmackall marked this pull request as ready for review February 26, 2025 02:28

gmackall requested a review from matanlurey as a code owner February 26, 2025 02:28

gmackall changed the title ~~[wip] Implement opacity FlutterMutator for hcpp~~ Implement opacity FlutterMutator for hcpp Feb 26, 2025

jonahwilliams reviewed Feb 26, 2025

View reviewed changes

Gray Mackall added 2 commits February 26, 2025 10:22

make a test that toggle opacity, refactor the test app a bit

d99e655

Merge remote-tracking branch 'refs/remotes/origin/opacity' into opacity

f3d626b

reidbaker self-requested a review February 26, 2025 18:49

reidbaker reviewed Feb 26, 2025

View reviewed changes

Gray Mackall added 2 commits February 26, 2025 11:07

analyze

b69ebcb

some javadoc

3f04dcf

Gray Mackall added 2 commits February 26, 2025 11:59

primitives can't be null, you've spent to long using boxed types

8cb1d8f

format

4abc2ff

jonahwilliams approved these changes Feb 26, 2025

View reviewed changes

gmackall added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 26, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 26, 2025

reidbaker reviewed Feb 26, 2025

View reviewed changes

gmackall added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 26, 2025

auto-submit bot added this pull request to the merge queue Feb 26, 2025

Merged via the queue into flutter:master with commit b287365 Feb 26, 2025
178 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 26, 2025

engine-flutter-autoroll mentioned this pull request Feb 27, 2025

Roll Flutter from 16592066d70b to 2e570ca393c8 (75 revisions) flutter/packages#8731

Merged

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

b287365 Implement opacity FlutterMutator for hcpp (flutter/flutter#…

ad8a6d7

…164147)

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

b287365 Implement opacity FlutterMutator for hcpp (flutter/flutter#…

14744b1

…164147)

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

b287365 Implement opacity FlutterMutator for hcpp (flutter/flutter#…

2789c64

…164147)

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

b287365 Implement opacity FlutterMutator for hcpp (flutter/flutter#…

6073696

…164147)

Implement opacity FlutterMutator for hcpp #164147

Implement opacity FlutterMutator for hcpp #164147

Uh oh!

Conversation

gmackall commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

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!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Feb 26, 2025

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!

jonahwilliams commented Feb 26, 2025

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Feb 26, 2025

Uh oh!

reidbaker left a comment

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Feb 26, 2025

Uh oh!

reidbaker commented Feb 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Implement opacity `FlutterMutator` for hcpp #164147

Implement opacity `FlutterMutator` for hcpp #164147

gmackall commented Feb 25, 2025 •

edited

Loading