From b5e86684b08b20acf96d0ce7b9bb64ee804c040c Mon Sep 17 00:00:00 2001
From: Jonah Williams <jonahwilliams@google.com>
Date: Wed, 13 Mar 2024 12:39:01 -0700
Subject: [PATCH] [Impeller] cleanup and test vk image usage flags.
 (flutter/engine#51301)

Cleanups to the vk::ImageUsageFlagBits

* add unit tests
* remove input attachment from depth stencil
* remove defensive checks
---
 .../renderer/backend/vulkan/allocator_vk.cc   | 24 ++++---------------
 .../renderer/backend/vulkan/allocator_vk.h    |  7 ++++++
 .../backend/vulkan/allocator_vk_unittests.cc  | 19 +++++++++++++++
 3 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.cc
index 332e234531..364b9aa49a 100644
--- a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.cc
+++ b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.cc
@@ -196,7 +196,7 @@ int32_t AllocatorVK::FindMemoryTypeIndex(
   return type_index;
 }
 
-static constexpr vk::ImageUsageFlags ToVKImageUsageFlags(
+vk::ImageUsageFlags AllocatorVK::ToVKImageUsageFlags(
     PixelFormat format,
     TextureUsageMask usage,
     StorageMode mode,
@@ -219,30 +219,16 @@ static constexpr vk::ImageUsageFlags ToVKImageUsageFlags(
       vk_usage |= vk::ImageUsageFlagBits::eDepthStencilAttachment;
     } else {
       vk_usage |= vk::ImageUsageFlagBits::eColorAttachment;
+      vk_usage |= vk::ImageUsageFlagBits::eInputAttachment;
     }
-    vk_usage |= vk::ImageUsageFlagBits::eInputAttachment;
   }
 
   if (usage & TextureUsage::kShaderRead) {
     vk_usage |= vk::ImageUsageFlagBits::eSampled;
-    // Device transient images can only be used as attachments. The caller
-    // specified incorrect usage flags and is attempting to read a device
-    // transient image in a shader. Unset the transient attachment flag. See:
-    // https://github.com/flutter/flutter/issues/121633
-    if (mode == StorageMode::kDeviceTransient) {
-      vk_usage &= ~vk::ImageUsageFlagBits::eTransientAttachment;
-    }
   }
 
   if (usage & TextureUsage::kShaderWrite) {
     vk_usage |= vk::ImageUsageFlagBits::eStorage;
-    // Device transient images can only be used as attachments. The caller
-    // specified incorrect usage flags and is attempting to read a device
-    // transient image in a shader. Unset the transient attachment flag. See:
-    // https://github.com/flutter/flutter/issues/121633
-    if (mode == StorageMode::kDeviceTransient) {
-      vk_usage &= ~vk::ImageUsageFlagBits::eTransientAttachment;
-    }
   }
 
   if (mode != StorageMode::kDeviceTransient) {
@@ -314,9 +300,9 @@ class AllocatedTextureSourceVK final : public TextureSourceVK {
     image_info.arrayLayers = ToArrayLayerCount(desc.type);
     image_info.tiling = vk::ImageTiling::eOptimal;
     image_info.initialLayout = vk::ImageLayout::eUndefined;
-    image_info.usage =
-        ToVKImageUsageFlags(desc.format, desc.usage, desc.storage_mode,
-                            supports_memoryless_textures);
+    image_info.usage = AllocatorVK::ToVKImageUsageFlags(
+        desc.format, desc.usage, desc.storage_mode,
+        supports_memoryless_textures);
     image_info.sharingMode = vk::SharingMode::eExclusive;
 
     VmaAllocationCreateInfo alloc_nfo = {};
diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.h b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.h
index 92b488c4b8..a664c84f4a 100644
--- a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.h
+++ b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.h
@@ -32,6 +32,13 @@ class AllocatorVK final : public Allocator {
       uint32_t memory_type_bits_requirement,
       vk::PhysicalDeviceMemoryProperties& memory_properties);
 
+  // Visible for testing.
+  static vk::ImageUsageFlags ToVKImageUsageFlags(
+      PixelFormat format,
+      TextureUsageMask usage,
+      StorageMode mode,
+      bool supports_memoryless_textures);
+
  private:
   friend class ContextVK;
 
diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk_unittests.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk_unittests.cc
index 1b0b858712..f8326c9833 100644
--- a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk_unittests.cc
+++ b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk_unittests.cc
@@ -13,6 +13,25 @@
 namespace impeller {
 namespace testing {
 
+TEST(AllocatorVKTest, ToVKImageUsageFlags) {
+  EXPECT_EQ(AllocatorVK::ToVKImageUsageFlags(
+                PixelFormat::kR8G8B8A8UNormInt,
+                static_cast<TextureUsageMask>(TextureUsage::kRenderTarget),
+                StorageMode::kDeviceTransient,
+                /*supports_memoryless_textures=*/true),
+            vk::ImageUsageFlagBits::eInputAttachment |
+                vk::ImageUsageFlagBits::eColorAttachment |
+                vk::ImageUsageFlagBits::eTransientAttachment);
+
+  EXPECT_EQ(AllocatorVK::ToVKImageUsageFlags(
+                PixelFormat::kD24UnormS8Uint,
+                static_cast<TextureUsageMask>(TextureUsage::kRenderTarget),
+                StorageMode::kDeviceTransient,
+                /*supports_memoryless_textures=*/true),
+            vk::ImageUsageFlagBits::eDepthStencilAttachment |
+                vk::ImageUsageFlagBits::eTransientAttachment);
+}
+
 TEST(AllocatorVKTest, MemoryTypeSelectionSingleHeap) {
   vk::PhysicalDeviceMemoryProperties properties;
   properties.memoryTypeCount = 1;