[Impeller] Reland: move AHB check into Flutter main, don't disable ImageReader on 29. (#164201)

Rather than conditionally disabling AHBs, just disable Vulkan on devices where AHB imports don't work. FIx from last time is in the last commit, I reversed the string check accidentally and disabled vulkan everywhere 🥺 https://github.com/flutter/flutter/issues/163473 https://github.com/flutter/flutter/issues/160854
2025-02-26 12:53:57 -08:00 · 2025-02-26 12:53:57 -08:00 · 04cc4b1f11
commit 04cc4b1f11
parent 05eb42f32d
10 changed files with 9 additions and 114 deletions
--- a/engine/src/flutter/ci/licenses_golden/licenses_flutter
+++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter
@ -42255,8 +42255,6 @@ ORIGIN: ../../../flutter/impeller/toolkit/android/native_window.cc + ../../../fl
 ORIGIN: ../../../flutter/impeller/toolkit/android/native_window.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/toolkit/android/proc_table.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/toolkit/android/proc_table.h + ../../../flutter/LICENSE
-ORIGIN: ../../../flutter/impeller/toolkit/android/shadow_realm.cc + ../../../flutter/LICENSE
-ORIGIN: ../../../flutter/impeller/toolkit/android/shadow_realm.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/toolkit/android/surface_control.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/toolkit/android/surface_control.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/toolkit/android/surface_transaction.cc + ../../../flutter/LICENSE
@ -45171,8 +45169,6 @@ FILE: ../../../flutter/impeller/toolkit/android/native_window.cc
 FILE: ../../../flutter/impeller/toolkit/android/native_window.h
 FILE: ../../../flutter/impeller/toolkit/android/proc_table.cc
 FILE: ../../../flutter/impeller/toolkit/android/proc_table.h
-FILE: ../../../flutter/impeller/toolkit/android/shadow_realm.cc
-FILE: ../../../flutter/impeller/toolkit/android/shadow_realm.h
 FILE: ../../../flutter/impeller/toolkit/android/surface_control.cc
 FILE: ../../../flutter/impeller/toolkit/android/surface_control.h
 FILE: ../../../flutter/impeller/toolkit/android/surface_transaction.cc
--- a/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/swapchain_vk.cc
+++ b/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/swapchain_vk.cc
@ -11,7 +11,6 @@

 #if FML_OS_ANDROID
 #include "impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_vk.h"
-#include "impeller/toolkit/android/shadow_realm.h"
 #endif  // FML_OS_ANDROID

 namespace impeller {
--- a/engine/src/flutter/impeller/toolkit/android/BUILD.gn
+++ b/engine/src/flutter/impeller/toolkit/android/BUILD.gn
@ -19,8 +19,6 @@ impeller_component("android") {
    "native_window.h",
    "proc_table.cc",
    "proc_table.h",
-    "shadow_realm.cc",
-    "shadow_realm.h",
    "surface_control.cc",
    "surface_control.h",
    "surface_transaction.cc",
--- a/engine/src/flutter/impeller/toolkit/android/shadow_realm.cc
+++ b/engine/src/flutter/impeller/toolkit/android/shadow_realm.cc
@ -1,45 +0,0 @@
-// Copyright 2013 The Flutter Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "impeller/toolkit/android/shadow_realm.h"
-
-#include <sys/system_properties.h>
-
-namespace impeller::android {
-
-constexpr std::string_view kAndroidHuawei = "android-huawei";
-
-bool ShadowRealm::ShouldDisableAHB() {
-  char clientidbase[PROP_VALUE_MAX];
-  __system_property_get("ro.com.google.clientidbase", clientidbase);
-
-  auto api_level = android_get_device_api_level();
-  char first_api_level[PROP_VALUE_MAX];
-  __system_property_get("ro.product.first_api_level", first_api_level);
-
-  return ShouldDisableAHBInternal(clientidbase, first_api_level, api_level);
-}
-
-// static
-bool ShadowRealm::ShouldDisableAHBInternal(std::string_view clientidbase,
-                                           std::string_view first_api_level,
-                                           uint32_t api_level) {
-  // Most devices that have updated to API 29 don't seem to correctly
-  // support AHBs: https://github.com/flutter/flutter/issues/157113
-  if (first_api_level.compare("28") == 0 ||
-      first_api_level.compare("27") == 0 ||
-      first_api_level.compare("26") == 0 ||
-      first_api_level.compare("25") == 0 ||
-      first_api_level.compare("24") == 0) {
-    return true;
-  }
-  // From local testing, neither the swapchain nor AHB import works, see also:
-  // https://github.com/flutter/flutter/issues/154068
-  if (clientidbase == kAndroidHuawei && api_level <= 29) {
-    return true;
-  }
-  return false;
-}
-
-}  // namespace impeller::android
--- a/engine/src/flutter/impeller/toolkit/android/shadow_realm.h
+++ b/engine/src/flutter/impeller/toolkit/android/shadow_realm.h
@ -1,27 +0,0 @@
-// Copyright 2013 The Flutter Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef FLUTTER_IMPELLER_TOOLKIT_ANDROID_SHADOW_REALM_H_
-#define FLUTTER_IMPELLER_TOOLKIT_ANDROID_SHADOW_REALM_H_
-
-#include <string_view>
-
-namespace impeller::android {
-
-// Looks like you're going to the Shadow Realm, Jimbo.
-class ShadowRealm {
- public:
-  /// @brief Whether the device should disable any usage of Android Hardware
-  ///        Buffers regardless of stated support.
-  static bool ShouldDisableAHB();
-
-  // For testing.
-  static bool ShouldDisableAHBInternal(std::string_view clientidbase,
-                                       std::string_view first_api_level,
-                                       uint32_t api_level);
-};
-
-}  // namespace impeller::android
-
-#endif  // FLUTTER_IMPELLER_TOOLKIT_ANDROID_SHADOW_REALM_H_
--- a/engine/src/flutter/impeller/toolkit/android/toolkit_android_unittests.cc
+++ b/engine/src/flutter/impeller/toolkit/android/toolkit_android_unittests.cc
@ -7,7 +7,6 @@
 #include "impeller/toolkit/android/choreographer.h"
 #include "impeller/toolkit/android/hardware_buffer.h"
 #include "impeller/toolkit/android/proc_table.h"
-#include "impeller/toolkit/android/shadow_realm.h"
 #include "impeller/toolkit/android/surface_control.h"
 #include "impeller/toolkit/android/surface_transaction.h"

@ -135,18 +134,4 @@ TEST(ToolkitAndroidTest, CanPostAndWaitForFrameCallbacks) {
  event.Wait();
 }

-TEST(ToolkitAndroidTest, ShouldDisableAHB) {
-  EXPECT_FALSE(
-      ShadowRealm::ShouldDisableAHBInternal("android-huawei", "30", 30));
-  EXPECT_FALSE(
-      ShadowRealm::ShouldDisableAHBInternal("something made up", "29", 29));
-
-  EXPECT_TRUE(
-      ShadowRealm::ShouldDisableAHBInternal("android-huawei", "29", 29));
-  EXPECT_TRUE(
-      ShadowRealm::ShouldDisableAHBInternal("something made up", "27", 29));
-  EXPECT_TRUE(
-      ShadowRealm::ShouldDisableAHBInternal("android-huawei", "garbage", 29));
-}
-
 }  // namespace impeller::android::testing
--- a/engine/src/flutter/shell/platform/android/flutter_main.cc
+++ b/engine/src/flutter/shell/platform/android/flutter_main.cc
@ -45,6 +45,8 @@ namespace {

 fml::jni::ScopedJavaGlobalRef<jclass>* g_flutter_jni_class = nullptr;

+static const constexpr char* kAndroidHuawei = "android-huawei";
+
 /// These are SoCs that crash when using AHB imports.
 static constexpr const char* kBLC[] = {
    // Most Exynos Series SoC
@ -310,6 +312,12 @@ AndroidRenderingAPI FlutterMain::SelectedRenderingAPI(
      return kVulkanUnsupportedFallback;
    }

+    __system_property_get("ro.com.google.clientidbase", product_model);
+    if (strcmp(product_model, kAndroidHuawei) == 0) {
+      // Avoid using Vulkan on Huawei as AHB imports do not
+      // consistently work.
+    }
+
    if (__system_property_find("ro.vendor.mediatek.platform") != nullptr) {
      // Probably MediaTek. Avoid Vulkan.
      return kVulkanUnsupportedFallback;
--- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java
+++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java
@ -1637,16 +1637,6 @@ public class FlutterJNI {
    void asyncWaitForVsync(final long cookie);
  }

-  /**
-   * Whether Android Hardware Buffer import is known to not work on this particular vendor + API
-   * level and should be disabled.
-   */
-  public boolean ShouldDisableAHB() {
-    return nativeShouldDisableAHB();
-  }
-
-  private native boolean nativeShouldDisableAHB();
-
  /** Whether the SurfaceControl swapchain required for hcpp is enabled and active. */
  public boolean IsSurfaceControlEnabled() {
    return nativeIsSurfaceControlEnabled(nativeShellHolderId);
--- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java
+++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java
@ -212,9 +212,7 @@ public class FlutterRenderer implements TextureRegistry {
    // version that is
    // running Vulkan, so we don't have to worry about it not being supported.
    final SurfaceProducer entry;
-    if (!debugForceSurfaceProducerGlTextures
-        && Build.VERSION.SDK_INT >= API_LEVELS.API_29
-        && !flutterJNI.ShouldDisableAHB()) {
+    if (!debugForceSurfaceProducerGlTextures && Build.VERSION.SDK_INT >= API_LEVELS.API_29) {
      final long id = nextTextureId.getAndIncrement();
      final ImageReaderSurfaceProducer producer = new ImageReaderSurfaceProducer(id);
      registerImageTexture(id, producer);
--- a/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc
+++ b/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc
@ -11,7 +11,6 @@
 #include <memory>
 #include <utility>

-#include "impeller/toolkit/android/shadow_realm.h"
 #include "unicode/uchar.h"

 #include "flutter/common/constants.h"
@ -871,12 +870,6 @@ bool RegisterApi(JNIEnv* env) {
          .signature = "(J)V",
          .fnPtr = reinterpret_cast<void*>(&UpdateDisplayMetrics),
      },
-      {
-          .name = "nativeShouldDisableAHB",
-          .signature = "()Z",
-          .fnPtr = reinterpret_cast<void*>(
-              &impeller::android::ShadowRealm::ShouldDisableAHB),
-      },
      {
          .name = "nativeIsSurfaceControlEnabled",
          .signature = "(J)Z",