From beb4e397b3da52e9cc7b60eb20c475a6a9072916 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Thu, 29 Aug 2024 17:09:24 -0700 Subject: [PATCH] [android] Disable AHB swapchain and import on huawei API 29 devices. (flutter/engine#54879) From local testing, it seems that AHB imports and swapchain usage of AHBs does not work on these Huawei devices. Vulkan does seem to work OK with these optional features disabled, but so far we've only tested on the P40 Pro. Fixes https://github.com/flutter/flutter/issues/154068 * https://github.com/flutter/flutter/issues/153228 * https://github.com/flutter/flutter/issues/153762 * https://github.com/flutter/flutter/issues/154068 --- .../ci/licenses_golden/licenses_flutter | 4 +++ .../renderer/backend/vulkan/driver_info_vk.cc | 4 +++ .../renderer/backend/vulkan/driver_info_vk.h | 1 + .../backend/vulkan/swapchain/swapchain_vk.cc | 4 ++- .../flutter/impeller/toolkit/android/BUILD.gn | 2 ++ .../impeller/toolkit/android/shadow_realm.cc | 33 +++++++++++++++++++ .../impeller/toolkit/android/shadow_realm.h | 26 +++++++++++++++ .../android/toolkit_android_unittests.cc | 9 +++++ .../flutter/embedding/engine/FlutterJNI.java | 10 ++++++ .../engine/renderer/FlutterRenderer.java | 4 ++- .../android/platform_view_android_jni_impl.cc | 8 ++++- 11 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 engine/src/flutter/impeller/toolkit/android/shadow_realm.cc create 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 653a2f5cc8..0a1c159518 100644 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter @@ -43298,6 +43298,8 @@ 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 @@ -46187,6 +46189,8 @@ 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/driver_info_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.cc index 3dbfaacab3..725acdad90 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.cc @@ -35,6 +35,8 @@ constexpr VendorVK IdentifyVendor(uint32_t vendor) { return VendorVK::kIntel; case 0x106B: return VendorVK::kApple; + case 0x19E5: + return VendorVK::kHuawei; } // Check if the ID is a known Khronos vendor. switch (vendor) { @@ -68,6 +70,8 @@ constexpr const char* VendorToString(VendorVK vendor) { return "Mesa"; case VendorVK::kApple: return "Apple"; + case VendorVK::kHuawei: + return "Huawei"; } FML_UNREACHABLE(); } diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.h b/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.h index 152b1ba3b3..e973985105 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.h +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.h @@ -23,6 +23,7 @@ enum class VendorVK { kAMD, kNvidia, kIntel, + kHuawei, //---------------------------------------------------------------------------- /// Includes the LLVM Pipe CPU implementation. /// 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 534c1f8b22..f6c6d440e4 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,6 +11,7 @@ #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 { @@ -59,7 +60,8 @@ std::shared_ptr SwapchainVK::Create( const auto emulator = ContextVK::Cast(*context).GetDriverInfo()->IsEmulator(); // Try AHB swapchains first. - if (!emulator && AHBSwapchainVK::IsAvailableOnPlatform()) { + if (!emulator && AHBSwapchainVK::IsAvailableOnPlatform() && + !android::ShadowRealm::ShouldDisableAHB()) { auto ahb_swapchain = std::shared_ptr(new AHBSwapchainVK( context, // window.GetHandle(), // diff --git a/engine/src/flutter/impeller/toolkit/android/BUILD.gn b/engine/src/flutter/impeller/toolkit/android/BUILD.gn index 0e0b30337f..c4a95f7681 100644 --- a/engine/src/flutter/impeller/toolkit/android/BUILD.gn +++ b/engine/src/flutter/impeller/toolkit/android/BUILD.gn @@ -19,6 +19,8 @@ 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 new file mode 100644 index 0000000000..6d19e6fe45 --- /dev/null +++ b/engine/src/flutter/impeller/toolkit/android/shadow_realm.cc @@ -0,0 +1,33 @@ +// 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(); + + return ShouldDisableAHBInternal(clientidbase, api_level); +} + +// static +bool ShadowRealm::ShouldDisableAHBInternal(std::string_view clientidbase, + uint32_t api_level) { + // 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 new file mode 100644 index 0000000000..90f913b5af --- /dev/null +++ b/engine/src/flutter/impeller/toolkit/android/shadow_realm.h @@ -0,0 +1,26 @@ +// 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, + 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 9c6218fa87..c96e04c2df 100644 --- a/engine/src/flutter/impeller/toolkit/android/toolkit_android_unittests.cc +++ b/engine/src/flutter/impeller/toolkit/android/toolkit_android_unittests.cc @@ -7,6 +7,7 @@ #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" @@ -134,4 +135,12 @@ TEST(ToolkitAndroidTest, CanPostAndWaitForFrameCallbacks) { event.Wait(); } +TEST(ToolkitAndroidTest, ShouldDisableAHB) { + EXPECT_FALSE(ShadowRealm::ShouldDisableAHB()); + + EXPECT_TRUE(ShadowRealm::ShouldDisableAHBInternal("android-huawei", 29)); + EXPECT_FALSE(ShadowRealm::ShouldDisableAHBInternal("android-huawei", 30)); + EXPECT_FALSE(ShadowRealm::ShouldDisableAHBInternal("something made up", 29)); +} + } // namespace impeller::android::testing 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 eaf2bd6058..3c966d2f4f 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 @@ -1536,4 +1536,14 @@ public class FlutterJNI { public interface AsyncWaitForVsyncDelegate { 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(); } 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 c4e3df02c1..532e692358 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 @@ -211,7 +211,9 @@ 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) { + if (!debugForceSurfaceProducerGlTextures + && Build.VERSION.SDK_INT >= API_LEVELS.API_29 + && !flutterJNI.ShouldDisableAHB()) { 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 17b9ecbebe..2b884fdab6 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 @@ -12,6 +12,7 @@ #include #include +#include "impeller/toolkit/android/shadow_realm.h" #include "include/android/SkImageAndroid.h" #include "unicode/uchar.h" @@ -853,7 +854,12 @@ bool RegisterApi(JNIEnv* env) { .signature = "(J)V", .fnPtr = reinterpret_cast(&UpdateDisplayMetrics), }, - }; + { + .name = "nativeShouldDisableAHB", + .signature = "()Z", + .fnPtr = reinterpret_cast( + &impeller::android::ShadowRealm::ShouldDisableAHB), + }}; if (env->RegisterNatives(g_flutter_jni_class->obj(), flutter_jni_methods, std::size(flutter_jni_methods)) != 0) {