From 88cfeedf51bf33ef0398d0af08f666828baaa792 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Thu, 27 Jul 2023 22:48:22 -0700
Subject: [PATCH] [Impeller] Set 'enable_vulkan_validation_layers' if --unopt
 and Vulkan is enabled. (flutter/engine#44055)

Context:

> @matanlurey:
> Would it make sense for `--unopt` to imply
`--enable-vulkan-validation-layers` if `--enable-vulkan` is set?
> The reason I ask is because it does seem to imply `glGetError` checks,
which (I could be wrong) is roughly similar?

> @chinmaygarde:
> Makes sense.

... so uh, here it is (let me know if we prefer anything different).
---
 .../renderer/backend/vulkan/context_vk.cc     | 22 ++++++++++++++-----
 engine/src/flutter/tools/gn                   |  4 ++++
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc
index 02f898eeba9..8b311333cbf 100644
--- a/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc
+++ b/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc
@@ -144,8 +144,18 @@ void ContextVK::Setup(Settings settings) {
   auto& dispatcher = VULKAN_HPP_DEFAULT_DISPATCHER;
   dispatcher.init(settings.proc_address_callback);
 
-  auto caps = std::shared_ptr<CapabilitiesVK>(
-      new CapabilitiesVK(settings.enable_validation));
+  // Enable Vulkan validation if either:
+  // 1. The user has explicitly enabled it.
+  // 2. We are in a combination of debug mode, and running on Android.
+  // (It's possible 2 is overly conservative and we can simplify this)
+  auto enable_validation = settings.enable_validation;
+
+#if defined(FML_OS_ANDROID) && !defined(NDEBUG)
+  enable_validation = true;
+#endif
+
+  auto caps =
+      std::shared_ptr<CapabilitiesVK>(new CapabilitiesVK(enable_validation));
 
   if (!caps->IsValid()) {
     VALIDATION_LOG << "Could not determine device capabilities.";
@@ -277,8 +287,8 @@ void ContextVK::Setup(Settings settings) {
   auto enabled_device_extensions =
       caps->GetEnabledDeviceExtensions(device_holder->physical_device);
   if (!enabled_device_extensions.has_value()) {
-    // This shouldn't happen since we already did device selection. But doesn't
-    // hurt to check again.
+    // This shouldn't happen since we already did device selection. But
+    // doesn't hurt to check again.
     return;
   }
 
@@ -420,8 +430,8 @@ void ContextVK::Setup(Settings settings) {
   is_valid_ = true;
 
   //----------------------------------------------------------------------------
-  /// Label all the relevant objects. This happens after setup so that the debug
-  /// messengers have had a chance to be set up.
+  /// Label all the relevant objects. This happens after setup so that the
+  /// debug messengers have had a chance to be set up.
   ///
   SetDebugName(GetDevice(), device_holder_->device.get(), "ImpellerDevice");
 }
diff --git a/engine/src/flutter/tools/gn b/engine/src/flutter/tools/gn
index eb77255ddbd..2ff92e8ea66 100755
--- a/engine/src/flutter/tools/gn
+++ b/engine/src/flutter/tools/gn
@@ -568,6 +568,10 @@ def to_gn_args(args):
       gn_args['android_api_level'] = 26
     gn_args['enable_vulkan_validation_layers'] = True
 
+  # Enable Vulkan validation layer automatically on debug builds for arm64.
+  if args.unoptimized and args.target_os == 'android' and args.android_cpu == 'arm64':
+    gn_args['enable_vulkan_validation_layers'] = True
+
   # Enable pointer compression on 64-bit mobile targets. iOS is excluded due to
   # its inability to allocate address space without allocating memory.
   if args.target_os in ['android'] and gn_args['target_cpu'] in ['x64', 'arm64'