From 8116ea14c7dff310e45935d3f84fc6c444bb9d37 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Thu, 30 Mar 2023 13:07:51 -0700 Subject: [PATCH] [Impeller] Enable vulkan validations via a flag on Android. (flutter/engine#40792) [Impeller] Enable vulkan validations via a flag on Android. --- engine/src/flutter/common/settings.h | 4 ++++ engine/src/flutter/shell/common/switches.cc | 3 +++ engine/src/flutter/shell/common/switches.h | 5 +++++ .../android/android_surface_vulkan_impeller.cc | 13 ++++++++----- .../android/android_surface_vulkan_impeller.h | 3 ++- .../embedding/engine/FlutterShellArgs.java | 5 +++++ .../platform/android/platform_view_android.cc | 18 +++++++++++------- .../platform/android/platform_view_android.h | 4 +++- .../engine/loader/FlutterLoaderTest.java | 18 ++++++++++++++++++ 9 files changed, 59 insertions(+), 14 deletions(-) diff --git a/engine/src/flutter/common/settings.h b/engine/src/flutter/common/settings.h index d4722b1dbda..9e9fa7dea24 100644 --- a/engine/src/flutter/common/settings.h +++ b/engine/src/flutter/common/settings.h @@ -217,6 +217,10 @@ struct Settings { bool enable_impeller = false; #endif + // Enable Vulkan validation on backends that support it. The validation layers + // must be available to the application. + bool enable_vulkan_validation = false; + // Data set by platform-specific embedders for use in font initialization. uint32_t font_initialization_data = 0; diff --git a/engine/src/flutter/shell/common/switches.cc b/engine/src/flutter/shell/common/switches.cc index a94d1cbabf0..702cbd45ea3 100644 --- a/engine/src/flutter/shell/common/switches.cc +++ b/engine/src/flutter/shell/common/switches.cc @@ -450,6 +450,9 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) { enable_impeller_value.empty() || "true" == enable_impeller_value; } + settings.enable_vulkan_validation = + command_line.HasOption(FlagForSwitch(Switch::EnableVulkanValidation)); + settings.enable_embedder_api = command_line.HasOption(FlagForSwitch(Switch::EnableEmbedderAPI)); diff --git a/engine/src/flutter/shell/common/switches.h b/engine/src/flutter/shell/common/switches.h index f0cdd5e26c8..30d1f0307d5 100644 --- a/engine/src/flutter/shell/common/switches.h +++ b/engine/src/flutter/shell/common/switches.h @@ -261,6 +261,11 @@ DEF_SWITCH(EnableImpeller, "enable-impeller", "Enable the Impeller renderer on supported platforms. Ignored if " "Impeller is not supported on the platform.") +DEF_SWITCH(EnableVulkanValidation, + "enable-vulkan-validation", + "Enable loading Vulkan validation layers. The layers must be " + "available to the application and loadable. On non-Vulkan backends, " + "this flag does nothing.") DEF_SWITCH(LeakVM, "leak-vm", "When the last shell shuts down, the shared VM is leaked by default " diff --git a/engine/src/flutter/shell/platform/android/android_surface_vulkan_impeller.cc b/engine/src/flutter/shell/platform/android/android_surface_vulkan_impeller.cc index 2e09bee8105..0e5ac734e3b 100644 --- a/engine/src/flutter/shell/platform/android/android_surface_vulkan_impeller.cc +++ b/engine/src/flutter/shell/platform/android/android_surface_vulkan_impeller.cc @@ -20,9 +20,10 @@ namespace flutter { -std::shared_ptr CreateImpellerContext( +static std::shared_ptr CreateImpellerContext( const fml::RefPtr& proc_table, - const std::shared_ptr& concurrent_loop) { + const std::shared_ptr& concurrent_loop, + bool enable_vulkan_validation) { std::vector> shader_mappings = { std::make_shared(impeller_entity_shaders_vk_data, impeller_entity_shaders_vk_length), @@ -40,17 +41,19 @@ std::shared_ptr CreateImpellerContext( settings.shader_libraries_data = std::move(shader_mappings); settings.cache_directory = fml::paths::GetCachesDirectory(); settings.worker_task_runner = concurrent_loop->GetTaskRunner(); - + settings.enable_validation = enable_vulkan_validation; return impeller::ContextVK::Create(std::move(settings)); } AndroidSurfaceVulkanImpeller::AndroidSurfaceVulkanImpeller( const std::shared_ptr& android_context, - const std::shared_ptr& jni_facade) + const std::shared_ptr& jni_facade, + bool enable_vulkan_validation) : AndroidSurface(android_context), proc_table_(fml::MakeRefCounted()), workers_(fml::ConcurrentMessageLoop::Create()) { - impeller_context_ = CreateImpellerContext(proc_table_, workers_); + impeller_context_ = + CreateImpellerContext(proc_table_, workers_, enable_vulkan_validation); is_valid_ = proc_table_->HasAcquiredMandatoryProcAddresses() && impeller_context_; } diff --git a/engine/src/flutter/shell/platform/android/android_surface_vulkan_impeller.h b/engine/src/flutter/shell/platform/android/android_surface_vulkan_impeller.h index c96cbd21a8f..3fff43d9d97 100644 --- a/engine/src/flutter/shell/platform/android/android_surface_vulkan_impeller.h +++ b/engine/src/flutter/shell/platform/android/android_surface_vulkan_impeller.h @@ -17,7 +17,8 @@ class AndroidSurfaceVulkanImpeller : public AndroidSurface { public: AndroidSurfaceVulkanImpeller( const std::shared_ptr& android_context, - const std::shared_ptr& jni_facade); + const std::shared_ptr& jni_facade, + bool enable_vulkan_validation); ~AndroidSurfaceVulkanImpeller() override; diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterShellArgs.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterShellArgs.java index 49519f1f979..e97eb66bfce 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterShellArgs.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterShellArgs.java @@ -44,6 +44,8 @@ public class FlutterShellArgs { public static final String ARG_TRACE_SYSTRACE = "--trace-systrace"; public static final String ARG_KEY_ENABLE_IMPELLER = "enable-impeller"; public static final String ARG_ENABLE_IMPELLER = "--enable-impeller"; + public static final String ARG_KEY_ENABLE_VULKAN_VALIDATION = "enable-vulkan-validation"; + public static final String ARG_ENABLE_VULKAN_VALIDATION = "--enable-vulkan-validation"; public static final String ARG_KEY_DUMP_SHADER_SKP_ON_SHADER_COMPILATION = "dump-skp-on-shader-compilation"; public static final String ARG_DUMP_SHADER_SKP_ON_SHADER_COMPILATION = @@ -121,6 +123,9 @@ public class FlutterShellArgs { if (intent.getBooleanExtra(ARG_KEY_ENABLE_IMPELLER, false)) { args.add(ARG_ENABLE_IMPELLER); } + if (intent.getBooleanExtra(ARG_KEY_ENABLE_VULKAN_VALIDATION, false)) { + args.add(ARG_ENABLE_VULKAN_VALIDATION); + } if (intent.getBooleanExtra(ARG_KEY_DUMP_SHADER_SKP_ON_SHADER_COMPILATION, false)) { args.add(ARG_DUMP_SHADER_SKP_ON_SHADER_COMPILATION); } diff --git a/engine/src/flutter/shell/platform/android/platform_view_android.cc b/engine/src/flutter/shell/platform/android/platform_view_android.cc index 1b9d440a2d4..804b0f111de 100644 --- a/engine/src/flutter/shell/platform/android/platform_view_android.cc +++ b/engine/src/flutter/shell/platform/android/platform_view_android.cc @@ -32,10 +32,12 @@ namespace flutter { AndroidSurfaceFactoryImpl::AndroidSurfaceFactoryImpl( const std::shared_ptr& context, std::shared_ptr jni_facade, - bool enable_impeller) + bool enable_impeller, + bool enable_vulkan_validation) : android_context_(context), jni_facade_(std::move(jni_facade)), - enable_impeller_(enable_impeller) {} + enable_impeller_(enable_impeller), + enable_vulkan_validation_(enable_vulkan_validation) {} AndroidSurfaceFactoryImpl::~AndroidSurfaceFactoryImpl() = default; @@ -48,8 +50,8 @@ std::unique_ptr AndroidSurfaceFactoryImpl::CreateSurface() { if (enable_impeller_) { // TODO(kaushikiska@): Enable this after wiring a preference for Vulkan backend. #if false - return std::make_unique(android_context_, - jni_facade_); + return std::make_unique( + android_context_, jni_facade_, enable_vulkan_validation_); #else return std::make_unique(android_context_, @@ -114,10 +116,12 @@ PlatformViewAndroid::PlatformViewAndroid( FML_CHECK(android_context_->IsValid()) << "Could not create surface from invalid Android context."; surface_factory_ = std::make_shared( - android_context_, jni_facade_, - delegate.OnPlatformViewGetSettings().enable_impeller); + android_context_, // + jni_facade_, // + delegate.OnPlatformViewGetSettings().enable_impeller, // + delegate.OnPlatformViewGetSettings().enable_vulkan_validation // + ); android_surface_ = surface_factory_->CreateSurface(); - FML_CHECK(android_surface_ && android_surface_->IsValid()) << "Could not create an OpenGL, Vulkan or Software surface to set up " "rendering."; diff --git a/engine/src/flutter/shell/platform/android/platform_view_android.h b/engine/src/flutter/shell/platform/android/platform_view_android.h index 536cb1d20f7..d7a8e8ce657 100644 --- a/engine/src/flutter/shell/platform/android/platform_view_android.h +++ b/engine/src/flutter/shell/platform/android/platform_view_android.h @@ -28,7 +28,8 @@ class AndroidSurfaceFactoryImpl : public AndroidSurfaceFactory { public: AndroidSurfaceFactoryImpl(const std::shared_ptr& context, std::shared_ptr jni_facade, - bool enable_impeller); + bool enable_impeller, + bool enable_vulkan_validation); ~AndroidSurfaceFactoryImpl() override; @@ -38,6 +39,7 @@ class AndroidSurfaceFactoryImpl : public AndroidSurfaceFactory { const std::shared_ptr& android_context_; std::shared_ptr jni_facade_; const bool enable_impeller_; + const bool enable_vulkan_validation_; }; class PlatformViewAndroid final : public PlatformView { diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/loader/FlutterLoaderTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/loader/FlutterLoaderTest.java index 2e765dce069..1d6bf535737 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/loader/FlutterLoaderTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/loader/FlutterLoaderTest.java @@ -175,6 +175,24 @@ public class FlutterLoaderTest { assertFalse(arguments.contains(enableImpellerArg)); } + @Test + public void itDoesNotSetEnableVulkanValidationByDefault() { + FlutterJNI mockFlutterJNI = mock(FlutterJNI.class); + FlutterLoader flutterLoader = new FlutterLoader(mockFlutterJNI); + + assertFalse(flutterLoader.initialized()); + flutterLoader.startInitialization(ctx); + flutterLoader.ensureInitializationComplete(ctx, null); + shadowOf(getMainLooper()).idle(); + + final String enableVulkanValidationArg = "--enable-vulkan-validation"; + ArgumentCaptor shellArgsCaptor = ArgumentCaptor.forClass(String[].class); + verify(mockFlutterJNI, times(1)) + .init(eq(ctx), shellArgsCaptor.capture(), anyString(), anyString(), anyString(), anyLong()); + List arguments = Arrays.asList(shellArgsCaptor.getValue()); + assertFalse(arguments.contains(enableVulkanValidationArg)); + } + @Test public void itSetsEnableImpellerFromMetaData() { FlutterJNI mockFlutterJNI = mock(FlutterJNI.class);