From 36664266fbde4b353c5aa0c815e0537a27eef60b Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Fri, 8 Mar 2024 15:05:17 -0800
Subject: [PATCH] [Impeller] cleanup and test vk image usage flags.

---
 .../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/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc
index 851d78e48716b..12d42f510032f 100644
--- a/impeller/renderer/backend/vulkan/allocator_vk.cc
+++ b/impeller/renderer/backend/vulkan/allocator_vk.cc
@@ -170,7 +170,7 @@ ISize AllocatorVK::GetMaxTextureSizeSupported() const {
   return max_texture_size_;
 }
 
-static constexpr vk::ImageUsageFlags ToVKImageUsageFlags(
+vk::ImageUsageFlags AllocatorVK::ToVKImageUsageFlags(
     PixelFormat format,
     TextureUsageMask usage,
     StorageMode mode,
@@ -193,30 +193,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 & static_cast<TextureUsageMask>(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 & static_cast<TextureUsageMask>(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) {
@@ -288,9 +274,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/impeller/renderer/backend/vulkan/allocator_vk.h b/impeller/renderer/backend/vulkan/allocator_vk.h
index 339e496ffd7db..7910a52e23bce 100644
--- a/impeller/renderer/backend/vulkan/allocator_vk.h
+++ b/impeller/renderer/backend/vulkan/allocator_vk.h
@@ -27,6 +27,13 @@ class AllocatorVK final : public Allocator {
   // Visible for testing
   size_t DebugGetHeapUsage() const;
 
+  // Visible for testing.
+  static vk::ImageUsageFlags ToVKImageUsageFlags(
+      PixelFormat format,
+      TextureUsageMask usage,
+      StorageMode mode,
+      bool supports_memoryless_textures);
+
  private:
   friend class ContextVK;
 
diff --git a/impeller/renderer/backend/vulkan/allocator_vk_unittests.cc b/impeller/renderer/backend/vulkan/allocator_vk_unittests.cc
index 26856987e20de..7855ad27f85ee 100644
--- a/impeller/renderer/backend/vulkan/allocator_vk_unittests.cc
+++ b/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);
+}
+
 #ifdef IMPELLER_DEBUG
 
 TEST(AllocatorVKTest, RecreateSwapchainWhenSizeChanges) {