[Android] update fallback and rendering state to combine impeller + android backend. #51008

jonahwilliams · 2024-02-27T18:14:40Z

Makes it possible to implement Impeller Vulkan -> Skia fallback.
Fixes [Impeller] Android needs a similar check to iOS to handle unsupported flag combinations of software rendering + Impeller flutter#143871
Fixes [Impeller] Read enable_impeller from surface or engine property instead of settings object. flutter#143873
Work towards [Proposal] Refactor AndroidRenderingAPI to encapsulate Impeller/Skia config flutter#137798

Combines AndroidRenderingAPI + enable_impeller into a 4 values enum that describes all combinations of rendering behavior: ImpellerVulkan, ImpellerOpenGLES, SkiaOpenGLES, Software.

Updates the fallback behavior to happen in flutter_main. This allows us to change the value of Settings.enable_impeller to support an Impeller -> Skia fallback.

…droid backend.

jonahwilliams · 2024-02-27T18:18:15Z

shell/platform/android/flutter_main.cc

+        /*enable_vulkan_gpu_tracing=*/false,
+        /*quiet=*/true);
+    if (!vulkan_backend->IsValid()) {
+      return AndroidRenderingAPI::kImpellerOpenGLES;


We can implement ImpellerVulkan -> Skia fallback by changing this value (or adding another configuration setting).

matanlurey · 2024-02-27T18:23:19Z

shell/platform/android/flutter_main.cc

+           "software rendering or disable impeller.";
+    return AndroidRenderingAPI::kSoftware;
+  }
+  // Debug/Profile only functionality for testing a specific


Maybe this should fault instead of falling back on releases? Let's ask the team.

matanlurey · 2024-02-27T18:24:18Z

shell/platform/android/platform_view_android.cc

This change is awesome

matanlurey · 2024-02-27T18:24:57Z

@chinmaygarde Can you review this as well? Thanks!

jonahwilliams · 2024-02-27T18:25:23Z

fml/message_loop.h

@@ -27,7 +27,6 @@ class MessageLoopImpl;
 /// \see fml::Wakeable
 class MessageLoop {
 public:
-  FML_EMBEDDER_ONLY


@chinmaygarde not sure what to do here. This macro is flagging usages of this API in flutter_main.cc, it looks like those hadn't been touched in a while. Is there an alternatie usage of this API I can switch to?

So the GetCurrent cannot be supported in all configurations because the thread or embedder may not have the ability to fetch the thread local task runner and instead must already have a handle to its task runner. So this is warning (via deprecation tags) that the API may only be used on embedder that have known safe access to the TLS message loop.

If the embedder knows that it is safe to access the TLS message loop slot, it can #define FML_USED_ON_EMBEDDER to make this available. flutter_main.cc should already have this and you shouldn't have to mess with this here.

Ideally, we should get rid of GetCurrent altogether but that is a migration.

chinmaygarde · 2024-02-27T18:29:26Z

common/settings.h

@@ -22,6 +22,14 @@

 namespace flutter {

+// The combination of targeted graphics API and Impeller support.
+enum class AndroidRenderingAPI {


"Android"RenderingAPI going in common is unfortunate. I suppose no more so that common itself. Couldn't this go in //flutter/shell/common instead?

Not unless common/settings.h can import flutter/common.

chinmaygarde · 2024-02-27T18:32:56Z

fml/message_loop.h

@@ -27,7 +27,6 @@ class MessageLoopImpl;
 /// \see fml::Wakeable
 class MessageLoop {
 public:
-  FML_EMBEDDER_ONLY


So the GetCurrent cannot be supported in all configurations because the thread or embedder may not have the ability to fetch the thread local task runner and instead must already have a handle to its task runner. So this is warning (via deprecation tags) that the API may only be used on embedder that have known safe access to the TLS message loop.

If the embedder knows that it is safe to access the TLS message loop slot, it can #define FML_USED_ON_EMBEDDER to make this available. flutter_main.cc should already have this and you shouldn't have to mess with this here.

Ideally, we should get rid of GetCurrent altogether but that is a migration.

chinmaygarde · 2024-02-27T18:33:44Z

shell/platform/android/flutter_main.cc

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.

+#include "common/settings.h"
+#include "impeller/base/validation.h"
 #define FML_USED_ON_EMBEDDER


@jonahwilliams You are running into the message loop error above because this define is not at the top.

🤦 Thanks!

While you are at it, https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes for the include order. These includes would go below the related header.

flutter-dashboard · 2024-02-27T19:39:17Z

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 #51008 at sha ed8e432

flutter-dashboard · 2024-02-27T20:48:41Z

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

Changes reported for pull request #51008 at sha 8547119

jonahwilliams · 2024-02-27T21:37:29Z

I wonder if this change is breaking how the scenario tests choose their backend. The golden diffs are mysterious to me, since its only off by a little bit

jonahwilliams · 2024-02-28T16:55:05Z

rerunning a few times to see if its just flakey

jonahwilliams · 2024-02-28T18:46:23Z

seems like it was just a flake, landing this.

…peller + android backend. (flutter/engine#51008)

…144338) flutter/engine@91898e3...c9381fb 2024-02-28 [email protected] Revert "Bump everything to Android 21" (flutter/engine#51056) 2024-02-28 [email protected] Roll Skia from 0c294ee206e8 to 93f245da0097 (2 revisions) (flutter/engine#51055) 2024-02-28 [email protected] [Android] update fallback and rendering state to combine impeller + android backend. (flutter/engine#51008) 2024-02-28 [email protected] [Impeller] Fix GL depth state. (flutter/engine#51040) 2024-02-28 [email protected] Roll buildroot to 7b537de78ac2239982ace130d1845374e5dcf113 (flutter/engine#51053) 2024-02-28 [email protected] Roll Skia from d935943bedcd to 0c294ee206e8 (3 revisions) (flutter/engine#51051) 2024-02-28 [email protected] When run_tests.py is in --quiet mode, write verbose logs to a file (flutter/engine#51029) 2024-02-28 [email protected] Roll Skia from fe34002ee826 to d935943bedcd (1 revision) (flutter/engine#51049) 2024-02-28 [email protected] Bump everything to Android 21 (flutter/engine#51032) 2024-02-28 [email protected] Add flutter prefix to import (flutter/engine#51042) 2024-02-28 [email protected] Roll Skia from d591703635b0 to fe34002ee826 (1 revision) (flutter/engine#51048) 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

[Android] update fallback and renderng state to combine impeller + an…

abdf41f

…droid backend.

github-actions bot added the platform-android label Feb 27, 2024

jonahwilliams requested a review from matanlurey February 27, 2024 18:15

add doc comment

b19e86c

jonahwilliams commented Feb 27, 2024

View reviewed changes

matanlurey approved these changes Feb 27, 2024

View reviewed changes

matanlurey requested a review from chinmaygarde February 27, 2024 18:24

jonahwilliams commented Feb 27, 2024

View reviewed changes

chinmaygarde reviewed Feb 27, 2024

View reviewed changes

jonahwilliams added 2 commits February 27, 2024 10:44

chinmay review and test cleanup.

c405e38

remove unused code.

ed8e432

jonahwilliams force-pushed the separate_blend branch from 78a4084 to ed8e432 Compare February 27, 2024 19:09

flutter-dashboard bot added the will affect goldens label Feb 27, 2024

Merge branch 'main' into separate_blend

8547119

jonahwilliams changed the title ~~[Android] update fallback and renderng state to combine impeller + android backend.~~ [Android] update fallback and rendering state to combine impeller + android backend. Feb 27, 2024

jonahwilliams added 2 commits February 27, 2024 20:24

Merge branch 'main' into separate_blend

37ae3b0

Merge branch 'main' into separate_blend

2de48e5

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

auto-submit bot merged commit 5b60aa5 into flutter:main Feb 28, 2024

engine-flutter-autoroll mentioned this pull request Feb 28, 2024

Roll Flutter Engine from 91898e397261 to 5b60aa5ae6b8 (9 revisions) flutter/flutter#144335

Closed

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

5b60aa5ae [Android] update fallback and rendering state to combine im…

2f8dd55

…peller + android backend. (flutter/engine#51008)

engine-flutter-autoroll mentioned this pull request Feb 28, 2024

Roll Flutter Engine from 91898e397261 to ce88fbc84e51 (10 revisions) flutter/flutter#144337

Closed

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

5b60aa5ae [Android] update fallback and rendering state to combine im…

8104097

…peller + android backend. (flutter/engine#51008)

engine-flutter-autoroll mentioned this pull request Feb 28, 2024

Roll Flutter Engine from 91898e397261 to c9381fb8ef4c (11 revisions) flutter/flutter#144338

Merged

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

5b60aa5ae [Android] update fallback and rendering state to combine im…

25fccbb

…peller + android backend. (flutter/engine#51008)

jonahwilliams mentioned this pull request Sep 8, 2024

[Proposal] Refactor AndroidRenderingAPI to encapsulate Impeller/Skia config flutter/flutter#137798

Closed

[Android] update fallback and rendering state to combine impeller + android backend. #51008

[Android] update fallback and rendering state to combine impeller + android backend. #51008

Uh oh!

Conversation

jonahwilliams commented Feb 27, 2024

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!

matanlurey commented Feb 27, 2024

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!

flutter-dashboard bot commented Feb 27, 2024

Uh oh!

flutter-dashboard bot commented Feb 27, 2024

Uh oh!

jonahwilliams commented Feb 27, 2024

Uh oh!

jonahwilliams commented Feb 28, 2024

Uh oh!

jonahwilliams commented Feb 28, 2024

Uh oh!

Uh oh!