fixes the vulkan image layout transitions for mipmap generation (flutter#173884)

Speculative fix for flutter#171252

The image layout transitions were wrong when generating mipmaps where
we'd run into `width <= 1 || height <= 1`. Previously the logic would
short circuit the process of converting everything from Dst->Src,
potentially leaving the mips in a series like [Read, Read, Src, Dst].
Then only the last one would get set to read, so [Read, Read, Src,
Read].

Now instead we group transition them all to Dst, then one by one we
switch them to src, make sure the last one gets set to src, then group
transition them to read. We make sure the loop doesn't break to make
sure everything transitions to src.

While this fixes a hypothetical bug, I couldn't replicate the bug inside
of unit tests because of validations. I couldn't find a mip count /
texture size that would allow me to hit the `break` statement that jonah
added previously. That doesn't mean it couldn't happen on release builds
though. I'm less confident that this fixes the issue after having
written the tests.

This does reduce the number of barriers though, so that's a win.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

Author: gaaclarke

engine/src/flutter/impeller/renderer/backend/vulkan/BUILD.gn

@@ -10,6 +10,7 @@ impeller_component("vulkan_unittests") {
   testonly = true
   sources = [
     "allocator_vk_unittests.cc",
+    "blit_pass_vk_unittests.cc",
     "command_encoder_vk_unittests.cc",
     "command_pool_vk_unittests.cc",
     "context_vk_unittests.cc",

engine/src/flutter/impeller/renderer/backend/vulkan/blit_pass_vk.cc

@@ -479,7 +479,8 @@ bool BlitPassVK::OnGenerateMipmapCommand(std::shared_ptr<Texture> texture,
     width = width / 2;
     height = height / 2;
     if (width <= 1 || height <= 1) {
-      break;
+      // Continue to make sure everything is placed into eTransferSrcOptimal.
+      continue;
     }
 
     // offsets[0] is origin.
@@ -495,24 +496,27 @@ bool BlitPassVK::OnGenerateMipmapCommand(std::shared_ptr<Texture> texture,
                   &blit,                                 // regions
                   vk::Filter::eLinear                    // filter
     );
-
-    barrier.oldLayout = vk::ImageLayout::eTransferSrcOptimal;
-    barrier.newLayout = vk::ImageLayout::eShaderReadOnlyOptimal;
-    barrier.srcAccessMask = vk::AccessFlagBits::eTransferRead;
-    barrier.dstAccessMask = vk::AccessFlagBits::eShaderRead;
-
-    // Now that the blit is done, the image at the previous level (N-1)
-    // is done reading from (TransferSrc)/ Now we must prepare it to be read
-    // from a shader (ShaderReadOnly).
-    cmd.pipelineBarrier(vk::PipelineStageFlagBits::eTransfer,
-                        vk::PipelineStageFlagBits::eFragmentShader, {}, {}, {},
-                        {barrier});
   }
 
+  // Switch the last one to eTransferSrcOptimal.
   barrier.subresourceRange.baseMipLevel = mip_count - 1;
+  barrier.subresourceRange.levelCount = 1;
   barrier.oldLayout = vk::ImageLayout::eTransferDstOptimal;
-  barrier.newLayout = vk::ImageLayout::eShaderReadOnlyOptimal;
+  barrier.newLayout = vk::ImageLayout::eTransferSrcOptimal;
   barrier.srcAccessMask = vk::AccessFlagBits::eTransferWrite;
+  barrier.dstAccessMask = vk::AccessFlagBits::eTransferRead;
+
+  cmd.pipelineBarrier(vk::PipelineStageFlagBits::eTransfer,
+                      vk::PipelineStageFlagBits::eTransfer, {}, {}, {},
+                      {barrier});
+
+  // Now everything is in eTransferSrcOptimal, switch everything to
+  // eShaderReadOnlyOptimal.
+  barrier.subresourceRange.baseMipLevel = 0;
+  barrier.subresourceRange.levelCount = mip_count;
+  barrier.oldLayout = vk::ImageLayout::eTransferSrcOptimal;
+  barrier.newLayout = vk::ImageLayout::eShaderReadOnlyOptimal;
+  barrier.srcAccessMask = vk::AccessFlagBits::eTransferRead;
   barrier.dstAccessMask = vk::AccessFlagBits::eShaderRead;
 
   cmd.pipelineBarrier(vk::PipelineStageFlagBits::eTransfer,

engine/src/flutter/impeller/renderer/backend/vulkan/blit_pass_vk.h

@@ -5,7 +5,8 @@
 #ifndef FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_BLIT_PASS_VK_H_
 #define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_BLIT_PASS_VK_H_
 
-#include "flutter/impeller/base/config.h"
+#include "flutter/fml/macros.h"
+#include "impeller/base/config.h"
 #include "impeller/geometry/rect.h"
 #include "impeller/renderer/backend/vulkan/workarounds_vk.h"
 #include "impeller/renderer/blit_pass.h"
@@ -22,6 +23,8 @@ class BlitPassVK final : public BlitPass {
 
  private:
   friend class CommandBufferVK;
+  FML_FRIEND_TEST(BlitPassVKTest,
+                  MipmapGenerationTransitionsAllLevelsCorrectly);
 
   std::shared_ptr<CommandBufferVK> command_buffer_;
   const WorkaroundsVK workarounds_;

engine/src/flutter/impeller/renderer/backend/vulkan/blit_pass_vk_unittests.cc

@@ -0,0 +1,57 @@
+// 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 "flutter/testing/testing.h"
+#include "fml/macros.h"
+#include "impeller/renderer/backend/vulkan/blit_pass_vk.h"
+#include "impeller/renderer/backend/vulkan/command_buffer_vk.h"
+#include "impeller/renderer/backend/vulkan/test/mock_vulkan.h"
+
+namespace impeller {
+
+TEST(BlitPassVKTest, MipmapGenerationTransitionsAllLevelsCorrectly) {
+  auto context = testing::MockVulkanContextBuilder().Build();
+  ASSERT_TRUE(context->IsValid());
+
+  auto cmd_buffer = context->CreateCommandBuffer();
+  ASSERT_TRUE(cmd_buffer);
+  auto blit_pass = cmd_buffer->CreateBlitPass();
+  ASSERT_TRUE(blit_pass);
+
+  auto vk_blit_pass = reinterpret_cast<BlitPassVK*>(blit_pass.get());
+  auto vk_cmd_buffer = reinterpret_cast<CommandBufferVK*>(cmd_buffer.get());
+
+  TextureDescriptor desc;
+  desc.size = ISize(100, 65);
+  desc.format = PixelFormat::kR8G8B8A8UNormInt;
+  desc.mip_count = 6;
+  auto texture = context->GetResourceAllocator()->CreateTexture(desc);
+  ASSERT_TRUE(texture);
+
+  ASSERT_TRUE(vk_blit_pass->OnGenerateMipmapCommand(texture, "TestMipmap"));
+
+  auto& barriers =
+      testing::GetImageMemoryBarriers(vk_cmd_buffer->GetCommandBuffer());
+
+  ASSERT_EQ(barriers.size(), 8u);
+
+  EXPECT_EQ(barriers[0].oldLayout, VK_IMAGE_LAYOUT_UNDEFINED);
+  EXPECT_EQ(barriers[0].newLayout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL);
+  EXPECT_EQ(barriers[0].subresourceRange.baseMipLevel, 0u);
+  EXPECT_EQ(barriers[0].subresourceRange.levelCount, 6u);
+
+  for (uint32_t i = 1; i < 7; ++i) {
+    EXPECT_EQ(barriers[i].oldLayout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL) << i;
+    EXPECT_EQ(barriers[i].newLayout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL) << i;
+    EXPECT_EQ(barriers[i].subresourceRange.baseMipLevel, i - 1) << i;
+    EXPECT_EQ(barriers[i].subresourceRange.levelCount, 1u) << i;
+  }
+
+  EXPECT_EQ(barriers[7].oldLayout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL);
+  EXPECT_EQ(barriers[7].newLayout, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL);
+  EXPECT_EQ(barriers[7].subresourceRange.baseMipLevel, 0u);
+  EXPECT_EQ(barriers[7].subresourceRange.levelCount, 6u);
+}
+
+}  // namespace impeller

engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.cc

@@ -25,6 +25,7 @@ struct MockCommandBuffer {
       std::shared_ptr<std::vector<std::string>> called_functions)
       : called_functions_(std::move(called_functions)) {}
   std::shared_ptr<std::vector<std::string>> called_functions_;
+  std::vector<VkImageMemoryBarrier> image_memory_barriers_;
 };
 
 struct MockQueryPool {};
@@ -448,6 +449,27 @@ void vkCmdBindPipeline(VkCommandBuffer commandBuffer,
   mock_command_buffer->called_functions_->push_back("vkCmdBindPipeline");
 }
 
+void vkCmdPipelineBarrier(VkCommandBuffer commandBuffer,
+                          VkPipelineStageFlags srcStageMask,
+                          VkPipelineStageFlags dstStageMask,
+                          VkDependencyFlags dependencyFlags,
+                          uint32_t memoryBarrierCount,
+                          const VkMemoryBarrier* pMemoryBarriers,
+                          uint32_t bufferMemoryBarrierCount,
+                          const VkBufferMemoryBarrier* pBufferMemoryBarriers,
+                          uint32_t imageMemoryBarrierCount,
+                          const VkImageMemoryBarrier* pImageMemoryBarriers) {
+  MockCommandBuffer* mock_command_buffer =
+      reinterpret_cast<MockCommandBuffer*>(commandBuffer);
+  mock_command_buffer->called_functions_->push_back("vkCmdPipelineBarrier");
+  if (pImageMemoryBarriers) {
+    for (uint32_t i = 0; i < imageMemoryBarrierCount; ++i) {
+      mock_command_buffer->image_memory_barriers_.push_back(
+          pImageMemoryBarriers[i]);
+    }
+  }
+}
+
 void vkCmdSetStencilReference(VkCommandBuffer commandBuffer,
                               VkStencilFaceFlags faceMask,
                               uint32_t reference) {
@@ -863,6 +885,8 @@ PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance,
     return reinterpret_cast<PFN_vkVoidFunction>(vkDestroyPipelineCache);
   } else if (strcmp("vkCmdBindPipeline", pName) == 0) {
     return reinterpret_cast<PFN_vkVoidFunction>(vkCmdBindPipeline);
+  } else if (strcmp("vkCmdPipelineBarrier", pName) == 0) {
+    return reinterpret_cast<PFN_vkVoidFunction>(vkCmdPipelineBarrier);
   } else if (strcmp("vkCmdSetStencilReference", pName) == 0) {
     return reinterpret_cast<PFN_vkVoidFunction>(vkCmdSetStencilReference);
   } else if (strcmp("vkCmdSetScissor", pName) == 0) {
@@ -988,5 +1012,12 @@ void SetSwapchainImageSize(ISize size) {
   currentImageSize = size;
 }
 
+std::vector<VkImageMemoryBarrier>& GetImageMemoryBarriers(
+    VkCommandBuffer buffer) {
+  MockCommandBuffer* mock_command_buffer =
+      reinterpret_cast<MockCommandBuffer*>(buffer);
+  return mock_command_buffer->image_memory_barriers_;
+}
+
 }  // namespace testing
 }  // namespace impeller

engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.h

@@ -131,6 +131,9 @@ class MockVulkanContextBuilder {
 /// @brief Override the image size returned by all swapchain images.
 void SetSwapchainImageSize(ISize size);
 
+std::vector<VkImageMemoryBarrier>& GetImageMemoryBarriers(
+    VkCommandBuffer buffer);
+
 }  // namespace testing
 }  // namespace impeller