[Impeller] Fixes to YUV imports on Android, Incomplete read of pipeline cache data, missing enabled extensions. (#164744)
- Handle textures that require a YUV import but aren't an undefined
format.
- INCOMPLETE is actually a success case for the pipeline cache. CERTAIN
drivers ALWAYS return incomplete, even when they wrote all the data.
Probably an off by one or something like that...
- Ensures Optional AndroidExtensions are enabled
- Only creates a YUV conversion if necessary
diff --git a/engine/src/flutter/.ci.yaml b/engine/src/flutter/.ci.yaml
index 732fe85..9de03af 100644
--- a/engine/src/flutter/.ci.yaml
+++ b/engine/src/flutter/.ci.yaml
@@ -67,13 +67,6 @@
{"dependency": "goldctl", "version": "git_revision:720a542f6fe4f92922c3b8f0fdcc4d2ac6bb83cd"}
]
timeout: 90
- runIf:
- - DEPS
- - engine/src/flutter/.ci.yaml
- - engine/src/flutter/ci/builders/linux_android_emulator.json
- - engine/src/flutter/lib/ui/**
- - engine/src/flutter/shell/platform/android/**
- - engine/src/flutter/testing/skia_gold_client/**
- name: Linux builder_cache
enabled_branches:
diff --git a/engine/src/flutter/BUILD.gn b/engine/src/flutter/BUILD.gn
index 8a854b2..12d69c6 100644
--- a/engine/src/flutter/BUILD.gn
+++ b/engine/src/flutter/BUILD.gn
@@ -170,6 +170,8 @@
public_deps = []
if (is_android) {
public_deps += [
+ "//flutter/impeller/renderer/backend/vulkan:vulkan_android_apk_unittests",
+ "//flutter/impeller/renderer/backend/vulkan:vulkan_android_unittests",
"//flutter/impeller/toolkit/android:apk_unittests",
"//flutter/impeller/toolkit/android:unittests",
"//flutter/shell/platform/android:flutter_shell_native_unittests",
diff --git a/engine/src/flutter/ci/builders/linux_android_emulator.json b/engine/src/flutter/ci/builders/linux_android_emulator.json
index 2ff2383..53a4c90 100644
--- a/engine/src/flutter/ci/builders/linux_android_emulator.json
+++ b/engine/src/flutter/ci/builders/linux_android_emulator.json
@@ -30,6 +30,8 @@
"ninja": {
"config": "ci/android_emulator_debug_x64",
"targets": [
+ "flutter/impeller/renderer/backend/vulkan:vulkan_android_unittests",
+ "flutter/impeller/renderer/backend/vulkan:vulkan_android_apk_unittests",
"flutter/impeller/toolkit/android:unittests",
"flutter/shell/platform/android:flutter_shell_native_unittests"
]
@@ -97,6 +99,8 @@
"ninja": {
"config": "ci/android_emulator_debug_x86",
"targets": [
+ "flutter/impeller/renderer/backend/vulkan:vulkan_android_unittests",
+ "flutter/impeller/renderer/backend/vulkan:vulkan_android_apk_unittests",
"flutter/impeller/toolkit/android:unittests",
"flutter/shell/platform/android:flutter_shell_native_unittests"
]
diff --git a/engine/src/flutter/ci/licenses_golden/excluded_files b/engine/src/flutter/ci/licenses_golden/excluded_files
index 6e43c10..3a2ce74 100644
--- a/engine/src/flutter/ci/licenses_golden/excluded_files
+++ b/engine/src/flutter/ci/licenses_golden/excluded_files
@@ -192,6 +192,7 @@
../../../flutter/impeller/renderer/backend/metal/swapchain_transients_mtl_unittests.mm
../../../flutter/impeller/renderer/backend/metal/texture_mtl_unittests.mm
../../../flutter/impeller/renderer/backend/vulkan/allocator_vk_unittests.cc
+../../../flutter/impeller/renderer/backend/vulkan/android/ahb_texture_source_vk_unittests.cc
../../../flutter/impeller/renderer/backend/vulkan/command_encoder_vk_unittests.cc
../../../flutter/impeller/renderer/backend/vulkan/command_pool_vk_unittests.cc
../../../flutter/impeller/renderer/backend/vulkan/context_vk_unittests.cc
diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/BUILD.gn b/engine/src/flutter/impeller/renderer/backend/vulkan/BUILD.gn
index 6e8b85e..cc800da 100644
--- a/engine/src/flutter/impeller/renderer/backend/vulkan/BUILD.gn
+++ b/engine/src/flutter/impeller/renderer/backend/vulkan/BUILD.gn
@@ -2,6 +2,7 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
+import("//flutter/testing/android/native_activity/native_activity.gni")
import("//flutter/vulkan/config.gni")
import("../../../tools/impeller.gni")
@@ -161,3 +162,50 @@
public_deps += [ "../../../toolkit/android" ]
}
}
+
+if (is_android) {
+ config("public_android_config") {
+ defines = [ "__ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__" ]
+ }
+
+ test_fixtures("unittests_fixtures") {
+ fixtures = []
+ }
+
+ source_set("unittests_lib") {
+ visibility = [ ":*" ]
+
+ testonly = true
+
+ sources = [ "android/ahb_texture_source_vk_unittests.cc" ]
+
+ deps = [
+ ":unittests_fixtures",
+ ":vulkan",
+ "//flutter/testing",
+ ]
+
+ defines = [ "TESTING" ]
+ }
+
+ executable("vulkan_android_unittests") {
+ assert(is_android)
+
+ testonly = true
+
+ output_name = "impeller_vulkan_android_unittests"
+
+ deps = [ ":unittests_lib" ]
+ }
+
+ native_activity_apk("vulkan_android_apk_unittests") {
+ apk_name = "impeller_vulkan_android_unittests"
+
+ testonly = true
+
+ deps = [
+ ":unittests_lib",
+ "//flutter/testing/android/native_activity:gtest_activity",
+ ]
+ }
+}
diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/android/ahb_texture_source_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/android/ahb_texture_source_vk.cc
index 9234eec..39663ca 100644
--- a/engine/src/flutter/impeller/renderer/backend/vulkan/android/ahb_texture_source_vk.cc
+++ b/engine/src/flutter/impeller/renderer/backend/vulkan/android/ahb_texture_source_vk.cc
@@ -11,13 +11,30 @@
namespace impeller {
+namespace {
+
+bool RequiresYCBCRConversion(vk::Format format) {
+ switch (format) {
+ case vk::Format::eG8B8R83Plane420Unorm:
+ case vk::Format::eG8B8R82Plane420Unorm:
+ case vk::Format::eG8B8R83Plane422Unorm:
+ case vk::Format::eG8B8R82Plane422Unorm:
+ case vk::Format::eG8B8R83Plane444Unorm:
+ return true;
+ default:
+ // NOTE: NOT EXHAUSTIVE.
+ break;
+ }
+ return false;
+}
+
using AHBProperties = vk::StructureChain<
// For VK_ANDROID_external_memory_android_hardware_buffer
vk::AndroidHardwareBufferPropertiesANDROID,
// For VK_ANDROID_external_memory_android_hardware_buffer
vk::AndroidHardwareBufferFormatPropertiesANDROID>;
-static vk::UniqueImage CreateVKImageWrapperForAndroidHarwareBuffer(
+vk::UniqueImage CreateVKImageWrapperForAndroidHarwareBuffer(
const vk::Device& device,
const AHBProperties& ahb_props,
const AHardwareBuffer_Desc& ahb_desc) {
@@ -94,7 +111,7 @@
return std::move(image.value);
}
-static vk::UniqueDeviceMemory ImportVKDeviceMemoryFromAndroidHarwareBuffer(
+vk::UniqueDeviceMemory ImportVKDeviceMemoryFromAndroidHarwareBuffer(
const vk::Device& device,
const vk::PhysicalDevice& physical_device,
const vk::Image& image,
@@ -138,7 +155,7 @@
return std::move(device_memory.value);
}
-static std::shared_ptr<YUVConversionVK> CreateYUVConversion(
+std::shared_ptr<YUVConversionVK> CreateYUVConversion(
const ContextVK& context,
const AHBProperties& ahb_props) {
YUVConversionDescriptorVK conversion_chain;
@@ -176,10 +193,10 @@
return context.GetYUVConversionLibrary()->GetConversion(conversion_chain);
}
-static vk::UniqueImageView CreateVKImageView(
+vk::UniqueImageView CreateVKImageView(
const vk::Device& device,
const vk::Image& image,
- const vk::SamplerYcbcrConversion& yuv_conversion,
+ const std::shared_ptr<YUVConversionVK>& yuv_conversion_wrapper,
const AHBProperties& ahb_props,
const AHardwareBuffer_Desc& ahb_desc) {
const auto& ahb_format =
@@ -205,9 +222,10 @@
view_info.subresourceRange.layerCount = ahb_desc.layers;
// We need a custom YUV conversion only if we don't recognize the format.
- if (view_info.format == vk::Format::eUndefined) {
+ if (view_info.format == vk::Format::eUndefined ||
+ RequiresYCBCRConversion(view_info.format)) {
view_chain.get<vk::SamplerYcbcrConversionInfo>().conversion =
- yuv_conversion;
+ yuv_conversion_wrapper->GetConversion();
} else {
view_chain.unlink<vk::SamplerYcbcrConversionInfo>();
}
@@ -222,7 +240,7 @@
return std::move(image_view.value);
}
-static PixelFormat ToPixelFormat(AHardwareBuffer_Format format) {
+PixelFormat ToPixelFormat(AHardwareBuffer_Format format) {
switch (format) {
case AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM:
return PixelFormat::kR8G8B8A8UNormInt;
@@ -256,7 +274,7 @@
return PixelFormat::kR8G8B8A8UNormInt;
}
-static TextureType ToTextureType(const AHardwareBuffer_Desc& ahb_desc) {
+TextureType ToTextureType(const AHardwareBuffer_Desc& ahb_desc) {
if (ahb_desc.layers == 1u) {
return TextureType::kTexture2D;
}
@@ -269,8 +287,7 @@
return TextureType::kTexture2D;
}
-static TextureDescriptor ToTextureDescriptor(
- const AHardwareBuffer_Desc& ahb_desc) {
+TextureDescriptor ToTextureDescriptor(const AHardwareBuffer_Desc& ahb_desc) {
const auto ahb_size = ISize{ahb_desc.width, ahb_desc.height};
TextureDescriptor desc;
// We are not going to touch hardware buffers on the CPU or use them as
@@ -290,6 +307,7 @@
}
return desc;
}
+} // namespace
AHBTextureSourceVK::AHBTextureSourceVK(
const std::shared_ptr<Context>& p_context,
@@ -339,23 +357,27 @@
}
// Figure out how to perform YUV conversions.
- auto yuv_conversion = CreateYUVConversion(context, ahb_props);
- if (!yuv_conversion || !yuv_conversion->IsValid()) {
- return;
+ needs_yuv_conversion_ = ahb_format.format == vk::Format::eUndefined ||
+ RequiresYCBCRConversion(ahb_format.format);
+ std::shared_ptr<YUVConversionVK> yuv_conversion;
+ if (needs_yuv_conversion_) {
+ yuv_conversion = CreateYUVConversion(context, ahb_props);
+ if (!yuv_conversion || !yuv_conversion->IsValid()) {
+ return;
+ }
}
// Create image view for the newly created image.
- auto image_view = CreateVKImageView(device, //
- image.get(), //
- yuv_conversion->GetConversion(), //
- ahb_props, //
- ahb_desc //
+ auto image_view = CreateVKImageView(device, //
+ image.get(), //
+ yuv_conversion, //
+ ahb_props, //
+ ahb_desc //
);
if (!image_view) {
return;
}
- needs_yuv_conversion_ = ahb_format.format == vk::Format::eUndefined;
device_memory_ = std::move(device_memory);
image_ = std::move(image);
yuv_conversion_ = std::move(yuv_conversion);
@@ -364,7 +386,10 @@
#ifdef IMPELLER_DEBUG
context.SetDebugName(device_memory_.get(), "AHB Device Memory");
context.SetDebugName(image_.get(), "AHB Image");
- context.SetDebugName(yuv_conversion_->GetConversion(), "AHB YUV Conversion");
+ if (yuv_conversion_) {
+ context.SetDebugName(yuv_conversion_->GetConversion(),
+ "AHB YUV Conversion");
+ }
context.SetDebugName(image_view_.get(), "AHB ImageView");
#endif // IMPELLER_DEBUG
diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/android/ahb_texture_source_vk_unittests.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/android/ahb_texture_source_vk_unittests.cc
new file mode 100644
index 0000000..4e40da0
--- /dev/null
+++ b/engine/src/flutter/impeller/renderer/backend/vulkan/android/ahb_texture_source_vk_unittests.cc
@@ -0,0 +1,97 @@
+// 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 <android/hardware_buffer.h>
+#include <memory>
+
+#include "flutter/testing/testing.h"
+#include "gtest/gtest.h"
+#include "impeller/renderer/backend/vulkan/android/ahb_texture_source_vk.h"
+#include "impeller/renderer/backend/vulkan/context_vk.h"
+#include "impeller/renderer/backend/vulkan/surface_context_vk.h"
+#include "impeller/toolkit/android/hardware_buffer.h"
+#include "impeller/toolkit/android/surface_transaction.h"
+
+namespace impeller::android::testing {
+
+// Set up context.
+std::shared_ptr<Context> CreateContext() {
+ auto vulkan_dylib = fml::NativeLibrary::Create("libvulkan.so");
+ auto instance_proc_addr =
+ vulkan_dylib->ResolveFunction<PFN_vkGetInstanceProcAddr>(
+ "vkGetInstanceProcAddr");
+
+ if (!instance_proc_addr.has_value()) {
+ VALIDATION_LOG << "Could not setup Vulkan proc table.";
+ return nullptr;
+ }
+
+ impeller::ContextVK::Settings settings;
+ settings.proc_address_callback = instance_proc_addr.value();
+ settings.shader_libraries_data = {};
+ settings.enable_validation = false;
+ settings.enable_gpu_tracing = false;
+ settings.enable_surface_control = false;
+
+ return ContextVK::Create(std::move(settings));
+}
+
+TEST(AndroidVulkanTest, CanImportRGBA) {
+ if (!HardwareBuffer::IsAvailableOnPlatform()) {
+ GTEST_SKIP() << "Hardware buffers are not supported on this platform.";
+ }
+
+ HardwareBufferDescriptor desc;
+ desc.size = ISize{1, 1};
+ desc.format = HardwareBufferFormat::kR8G8B8A8UNormInt;
+ desc.usage = HardwareBufferUsageFlags::kSampledImage;
+
+ auto ahb = std::make_unique<HardwareBuffer>(desc);
+ ASSERT_TRUE(ahb);
+ auto context_vk = CreateContext();
+ ASSERT_TRUE(context_vk);
+
+ AHBTextureSourceVK source(context_vk, std::move(ahb),
+ /*is_swapchain_image=*/false);
+
+ EXPECT_TRUE(source.IsValid());
+ EXPECT_EQ(source.GetYUVConversion(), nullptr);
+
+ context_vk->Shutdown();
+}
+
+TEST(AndroidVulkanTest, CanImportWithYUB) {
+ if (!HardwareBuffer::IsAvailableOnPlatform()) {
+ GTEST_SKIP() << "Hardware buffers are not supported on this platform.";
+ }
+
+ AHardwareBuffer_Desc desc;
+ desc.width = 16;
+ desc.height = 16;
+ desc.format = AHARDWAREBUFFER_FORMAT_Y8Cb8Cr8_420;
+ desc.stride = 0;
+ desc.layers = 1;
+ desc.rfu0 = 0;
+ desc.rfu1 = 0;
+ desc.usage = AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE |
+ AHARDWAREBUFFER_USAGE_CPU_WRITE_MASK |
+ AHARDWAREBUFFER_USAGE_CPU_READ_MASK;
+
+ EXPECT_EQ(AHardwareBuffer_isSupported(&desc), 1);
+
+ AHardwareBuffer* buffer = nullptr;
+ ASSERT_EQ(AHardwareBuffer_allocate(&desc, &buffer), 0);
+
+ auto context_vk = CreateContext();
+ ASSERT_TRUE(context_vk);
+
+ AHBTextureSourceVK source(context_vk, buffer, desc);
+
+ EXPECT_TRUE(source.IsValid());
+ EXPECT_NE(source.GetYUVConversion(), nullptr);
+
+ context_vk->Shutdown();
+}
+
+} // namespace impeller::android::testing
diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc
index 6e59d9f..7cd75c7 100644
--- a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc
+++ b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc
@@ -298,6 +298,17 @@
return true;
};
+ auto for_each_optional_android_extension =
+ [&](OptionalAndroidDeviceExtensionVK ext) {
+#ifdef FML_OS_ANDROID
+ auto name = GetExtensionName(ext);
+ if (exts.find(name) != exts.end()) {
+ enabled.push_back(name);
+ }
+#endif // FML_OS_ANDROID
+ return true;
+ };
+
auto for_each_optional_extension = [&](OptionalDeviceExtensionVK ext) {
auto name = GetExtensionName(ext);
if (exts.find(name) != exts.end()) {
@@ -311,7 +322,10 @@
for_each_common_extension) &&
IterateExtensions<RequiredAndroidDeviceExtensionVK>(
for_each_android_extension) &&
- IterateExtensions<OptionalDeviceExtensionVK>(for_each_optional_extension);
+ IterateExtensions<OptionalDeviceExtensionVK>(
+ for_each_optional_extension) &&
+ IterateExtensions<OptionalAndroidDeviceExtensionVK>(
+ for_each_optional_android_extension);
if (!iterate_extensions) {
VALIDATION_LOG << "Device not suitable since required extensions are not "
diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_cache_data_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_cache_data_vk.cc
index 9cdd199..ecb46b7 100644
--- a/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_cache_data_vk.cc
+++ b/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_cache_data_vk.cc
@@ -36,9 +36,13 @@
}
const auto header = PipelineCacheHeaderVK{props, data_size};
std::memcpy(allocation->GetBuffer(), &header, sizeof(header));
- if (cache.getOwner().getPipelineCacheData(
- *cache, &data_size, allocation->GetBuffer() + sizeof(header)) !=
- vk::Result::eSuccess) {
+ vk::Result lookup_result = cache.getOwner().getPipelineCacheData(
+ *cache, &data_size, allocation->GetBuffer() + sizeof(header));
+
+ // Some drivers may return incomplete erroneously, but this is not an
+ // error condition as some/all data was still written.
+ if (lookup_result != vk::Result::eSuccess &&
+ lookup_result != vk::Result::eIncomplete) {
VALIDATION_LOG << "Could not copy pipeline cache data.";
return false;
}
diff --git a/engine/src/flutter/testing/run_tests.py b/engine/src/flutter/testing/run_tests.py
index 062b4b0..8cd8776 100755
--- a/engine/src/flutter/testing/run_tests.py
+++ b/engine/src/flutter/testing/run_tests.py
@@ -765,6 +765,7 @@
run_android_unittest('flutter_shell_native_unittests', android_variant, adb_path)
run_android_unittest('impeller_toolkit_android_unittests', android_variant, adb_path)
+ run_android_unittest('impeller_vulkan_android_unittests', android_variant, adb_path)
def run_objc_tests(ios_variant='ios_debug_sim_unopt', test_filter=None):
