Skip to content

Commit cca82ed

Browse files
auto-submit[bot]auto-submit[bot]
auto-submit[bot]
and
auto-submit[bot]
authored
Reverts "[Impeller] move AHB check to Vulkan, use Vulkan surface on 29. (#164109)" (#164166)
<!-- start_original_pr_link --> Reverts: #164109 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: jonahwilliams <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: brain not work too good. <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: jonahwilliams <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {matanlurey} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: Rather than conditionally disabling AHBs, just disable Vulkan on devices where AHB imports don't work. #163473 #160854 <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
1 parent 092be76 commit cca82ed

10 files changed

Lines changed: 114 additions & 10 deletions

File tree

‎engine/src/flutter/ci/licenses_golden/licenses_flutter‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42254,6 +42254,8 @@ ORIGIN: ../../../flutter/impeller/toolkit/android/native_window.cc + ../../../fl
4225442254
ORIGIN: ../../../flutter/impeller/toolkit/android/native_window.h + ../../../flutter/LICENSE
4225542255
ORIGIN: ../../../flutter/impeller/toolkit/android/proc_table.cc + ../../../flutter/LICENSE
4225642256
ORIGIN: ../../../flutter/impeller/toolkit/android/proc_table.h + ../../../flutter/LICENSE
42257+
ORIGIN: ../../../flutter/impeller/toolkit/android/shadow_realm.cc + ../../../flutter/LICENSE
42258+
ORIGIN: ../../../flutter/impeller/toolkit/android/shadow_realm.h + ../../../flutter/LICENSE
4225742259
ORIGIN: ../../../flutter/impeller/toolkit/android/surface_control.cc + ../../../flutter/LICENSE
4225842260
ORIGIN: ../../../flutter/impeller/toolkit/android/surface_control.h + ../../../flutter/LICENSE
4225942261
ORIGIN: ../../../flutter/impeller/toolkit/android/surface_transaction.cc + ../../../flutter/LICENSE
@@ -45167,6 +45169,8 @@ FILE: ../../../flutter/impeller/toolkit/android/native_window.cc
4516745169
FILE: ../../../flutter/impeller/toolkit/android/native_window.h
4516845170
FILE: ../../../flutter/impeller/toolkit/android/proc_table.cc
4516945171
FILE: ../../../flutter/impeller/toolkit/android/proc_table.h
45172+
FILE: ../../../flutter/impeller/toolkit/android/shadow_realm.cc
45173+
FILE: ../../../flutter/impeller/toolkit/android/shadow_realm.h
4517045174
FILE: ../../../flutter/impeller/toolkit/android/surface_control.cc
4517145175
FILE: ../../../flutter/impeller/toolkit/android/surface_control.h
4517245176
FILE: ../../../flutter/impeller/toolkit/android/surface_transaction.cc

‎engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/swapchain_vk.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#if FML_OS_ANDROID
1313
#include "impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_vk.h"
14+
#include "impeller/toolkit/android/shadow_realm.h"
1415
#endif // FML_OS_ANDROID
1516

1617
namespace impeller {

‎engine/src/flutter/impeller/toolkit/android/BUILD.gn‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ impeller_component("android") {
1919
"native_window.h",
2020
"proc_table.cc",
2121
"proc_table.h",
22+
"shadow_realm.cc",
23+
"shadow_realm.h",
2224
"surface_control.cc",
2325
"surface_control.h",
2426
"surface_transaction.cc",

‎engine/src/flutter/impeller/toolkit/android/shadow_realm.cc‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "impeller/toolkit/android/shadow_realm.h"
6+
7+
#include <sys/system_properties.h>
8+
9+
namespace impeller::android {
10+
11+
constexpr std::string_view kAndroidHuawei = "android-huawei";
12+
13+
bool ShadowRealm::ShouldDisableAHB() {
14+
char clientidbase[PROP_VALUE_MAX];
15+
__system_property_get("ro.com.google.clientidbase", clientidbase);
16+
17+
auto api_level = android_get_device_api_level();
18+
char first_api_level[PROP_VALUE_MAX];
19+
__system_property_get("ro.product.first_api_level", first_api_level);
20+
21+
return ShouldDisableAHBInternal(clientidbase, first_api_level, api_level);
22+
}
23+
24+
// static
25+
bool ShadowRealm::ShouldDisableAHBInternal(std::string_view clientidbase,
26+
std::string_view first_api_level,
27+
uint32_t api_level) {
28+
// Most devices that have updated to API 29 don't seem to correctly
29+
// support AHBs: https://github.com/flutter/flutter/issues/157113
30+
if (first_api_level.compare("28") == 0 ||
31+
first_api_level.compare("27") == 0 ||
32+
first_api_level.compare("26") == 0 ||
33+
first_api_level.compare("25") == 0 ||
34+
first_api_level.compare("24") == 0) {
35+
return true;
36+
}
37+
// From local testing, neither the swapchain nor AHB import works, see also:
38+
// https://github.com/flutter/flutter/issues/154068
39+
if (clientidbase == kAndroidHuawei && api_level <= 29) {
40+
return true;
41+
}
42+
return false;
43+
}
44+
45+
} // namespace impeller::android

‎engine/src/flutter/impeller/toolkit/android/shadow_realm.h‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#ifndef FLUTTER_IMPELLER_TOOLKIT_ANDROID_SHADOW_REALM_H_
6+
#define FLUTTER_IMPELLER_TOOLKIT_ANDROID_SHADOW_REALM_H_
7+
8+
#include <string_view>
9+
10+
namespace impeller::android {
11+
12+
// Looks like you're going to the Shadow Realm, Jimbo.
13+
class ShadowRealm {
14+
public:
15+
/// @brief Whether the device should disable any usage of Android Hardware
16+
/// Buffers regardless of stated support.
17+
static bool ShouldDisableAHB();
18+
19+
// For testing.
20+
static bool ShouldDisableAHBInternal(std::string_view clientidbase,
21+
std::string_view first_api_level,
22+
uint32_t api_level);
23+
};
24+
25+
} // namespace impeller::android
26+
27+
#endif // FLUTTER_IMPELLER_TOOLKIT_ANDROID_SHADOW_REALM_H_

‎engine/src/flutter/impeller/toolkit/android/toolkit_android_unittests.cc‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include "impeller/toolkit/android/choreographer.h"
88
#include "impeller/toolkit/android/hardware_buffer.h"
99
#include "impeller/toolkit/android/proc_table.h"
10+
#include "impeller/toolkit/android/shadow_realm.h"
1011
#include "impeller/toolkit/android/surface_control.h"
1112
#include "impeller/toolkit/android/surface_transaction.h"
1213

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

138+
TEST(ToolkitAndroidTest, ShouldDisableAHB) {
139+
EXPECT_FALSE(
140+
ShadowRealm::ShouldDisableAHBInternal("android-huawei", "30", 30));
141+
EXPECT_FALSE(
142+
ShadowRealm::ShouldDisableAHBInternal("something made up", "29", 29));
143+
144+
EXPECT_TRUE(
145+
ShadowRealm::ShouldDisableAHBInternal("android-huawei", "29", 29));
146+
EXPECT_TRUE(
147+
ShadowRealm::ShouldDisableAHBInternal("something made up", "27", 29));
148+
EXPECT_TRUE(
149+
ShadowRealm::ShouldDisableAHBInternal("android-huawei", "garbage", 29));
150+
}
151+
137152
} // namespace impeller::android::testing

‎engine/src/flutter/shell/platform/android/flutter_main.cc‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,6 @@ namespace {
4545

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

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

315-
__system_property_get("ro.com.google.clientidbase", product_model);
316-
if (strcmp(product_model, kAndroidHuawei)) {
317-
// Avoid using Vulkan on Huawei as AHB imports do not
318-
// consistently work.
319-
return kVulkanUnsupportedFallback;
320-
}
321-
322313
__system_property_get("ro.product.board", product_model);
323314
if (IsKnownBadSOC(product_model)) {
324315
// Avoid using Vulkan on known bad SoCs.

‎engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1637,6 +1637,16 @@ public interface AsyncWaitForVsyncDelegate {
16371637
void asyncWaitForVsync(final long cookie);
16381638
}
16391639

1640+
/**
1641+
* Whether Android Hardware Buffer import is known to not work on this particular vendor + API
1642+
* level and should be disabled.
1643+
*/
1644+
public boolean ShouldDisableAHB() {
1645+
return nativeShouldDisableAHB();
1646+
}
1647+
1648+
private native boolean nativeShouldDisableAHB();
1649+
16401650
/** Whether the SurfaceControl swapchain required for hcpp is enabled and active. */
16411651
public boolean IsSurfaceControlEnabled() {
16421652
return nativeIsSurfaceControlEnabled(nativeShellHolderId);

‎engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,9 @@ public SurfaceProducer createSurfaceProducer() {
212212
// version that is
213213
// running Vulkan, so we don't have to worry about it not being supported.
214214
final SurfaceProducer entry;
215-
if (!debugForceSurfaceProducerGlTextures && Build.VERSION.SDK_INT >= API_LEVELS.API_29) {
215+
if (!debugForceSurfaceProducerGlTextures
216+
&& Build.VERSION.SDK_INT >= API_LEVELS.API_29
217+
&& !flutterJNI.ShouldDisableAHB()) {
216218
final long id = nextTextureId.getAndIncrement();
217219
final ImageReaderSurfaceProducer producer = new ImageReaderSurfaceProducer(id);
218220
registerImageTexture(id, producer);

‎engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <memory>
1212
#include <utility>
1313

14+
#include "impeller/toolkit/android/shadow_realm.h"
1415
#include "unicode/uchar.h"
1516

1617
#include "flutter/common/constants.h"
@@ -870,6 +871,12 @@ bool RegisterApi(JNIEnv* env) {
870871
.signature = "(J)V",
871872
.fnPtr = reinterpret_cast<void*>(&UpdateDisplayMetrics),
872873
},
874+
{
875+
.name = "nativeShouldDisableAHB",
876+
.signature = "()Z",
877+
.fnPtr = reinterpret_cast<void*>(
878+
&impeller::android::ShadowRealm::ShouldDisableAHB),
879+
},
873880
{
874881
.name = "nativeIsSurfaceControlEnabled",
875882
.signature = "(J)Z",

0 commit comments

Comments
 (0)