From 8b3f1a4c0f9795db84bf1b3ea37741d1c1d10e02 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 25 Feb 2025 18:50:12 -0800 Subject: [PATCH] [Impeller] move AHB check to Vulkan, use Vulkan surface on 29. (#164109) Rather than conditionally disabling AHBs, just disable Vulkan on devices where AHB imports don't work. https://github.com/flutter/flutter/issues/163473 https://github.com/flutter/flutter/issues/160854 --- .../ci/licenses_golden/licenses_flutter | 4 -- .../backend/vulkan/swapchain/swapchain_vk.cc | 1 - .../flutter/impeller/toolkit/android/BUILD.gn | 2 - .../impeller/toolkit/android/shadow_realm.cc | 45 ------------------- .../impeller/toolkit/android/shadow_realm.h | 27 ----------- .../android/toolkit_android_unittests.cc | 15 ------- .../shell/platform/android/flutter_main.cc | 9 ++++ .../flutter/embedding/engine/FlutterJNI.java | 10 ----- .../engine/renderer/FlutterRenderer.java | 4 +- .../android/platform_view_android_jni_impl.cc | 7 --- 10 files changed, 10 insertions(+), 114 deletions(-) delete mode 100644 engine/src/flutter/impeller/toolkit/android/shadow_realm.cc delete mode 100644 engine/src/flutter/impeller/toolkit/android/shadow_realm.h diff --git a/engine/src/flutter/ci/licenses_golden/licenses_flutter b/engine/src/flutter/ci/licenses_golden/licenses_flutter index 7c45a24340..348bd12248 100644 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter @@ -42254,8 +42254,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 @@ -45169,8 +45167,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 diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/swapchain_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/swapchain_vk.cc index 64a85733ca..07e94042de 100644 --- 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 { diff --git a/engine/src/flutter/impeller/toolkit/android/BUILD.gn b/engine/src/flutter/impeller/toolkit/android/BUILD.gn index c4a95f7681..0e0b30337f 100644 --- 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", diff --git a/engine/src/flutter/impeller/toolkit/android/shadow_realm.cc b/engine/src/flutter/impeller/toolkit/android/shadow_realm.cc deleted file mode 100644 index 7aeb4ac074..0000000000 --- a/engine/src/flutter/impeller/toolkit/android/shadow_realm.cc +++ /dev/null @@ -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 - -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 diff --git a/engine/src/flutter/impeller/toolkit/android/shadow_realm.h b/engine/src/flutter/impeller/toolkit/android/shadow_realm.h deleted file mode 100644 index 90f265ec61..0000000000 --- a/engine/src/flutter/impeller/toolkit/android/shadow_realm.h +++ /dev/null @@ -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 - -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_ diff --git a/engine/src/flutter/impeller/toolkit/android/toolkit_android_unittests.cc b/engine/src/flutter/impeller/toolkit/android/toolkit_android_unittests.cc index ca8837d732..9c6218fa87 100644 --- 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 diff --git a/engine/src/flutter/shell/platform/android/flutter_main.cc b/engine/src/flutter/shell/platform/android/flutter_main.cc index d255f4a766..b0147f9ccf 100644 --- 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* 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,13 @@ AndroidRenderingAPI FlutterMain::SelectedRenderingAPI( return kVulkanUnsupportedFallback; } + __system_property_get("ro.com.google.clientidbase", product_model); + if (strcmp(product_model, kAndroidHuawei)) { + // Avoid using Vulkan on Huawei as AHB imports do not + // consistently work. + return kVulkanUnsupportedFallback; + } + __system_property_get("ro.product.board", product_model); if (IsKnownBadSOC(product_model)) { // Avoid using Vulkan on known bad SoCs. diff --git 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 index dac6d25149..d67f5daa65 100644 --- 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); diff --git 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 index e3095ff7bb..33c8a81954 100644 --- 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); diff --git 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 index 06d2b3467b..b8e806fef1 100644 --- 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 #include -#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(&UpdateDisplayMetrics), }, - { - .name = "nativeShouldDisableAHB", - .signature = "()Z", - .fnPtr = reinterpret_cast( - &impeller::android::ShadowRealm::ShouldDisableAHB), - }, { .name = "nativeIsSurfaceControlEnabled", .signature = "(J)Z",