[Impeller] correct multisample resolve/store configuration for Vulkan.

impeller/renderer/backend/vulkan/formats_vk.h

@@ -311,18 +311,39 @@ constexpr vk::AttachmentLoadOp ToVKAttachmentLoadOp(LoadAction load_action) {
   FML_UNREACHABLE();
 }
 
-constexpr vk::AttachmentStoreOp ToVKAttachmentStoreOp(
-    StoreAction store_action) {
+constexpr vk::AttachmentStoreOp ToVKAttachmentStoreOp(StoreAction store_action,
+                                                      bool is_resolve_texture) {
   switch (store_action) {
     case StoreAction::kStore:
+      // Both MSAA and resolve textures need to be stored. A resolve is NOT
+      // performed.
       return vk::AttachmentStoreOp::eStore;
     case StoreAction::kDontCare:
+      // Both MSAA and resolve textures can be discarded. A resolve is NOT
+      // performed.
       return vk::AttachmentStoreOp::eDontCare;
     case StoreAction::kMultisampleResolve:
+      // The resolve texture is stored but the MSAA texture can be discarded. A
+      // resolve IS performed.
+      return is_resolve_texture ? vk::AttachmentStoreOp::eStore
+                                : vk::AttachmentStoreOp::eDontCare;
     case StoreAction::kStoreAndMultisampleResolve:
-      return vk::AttachmentStoreOp::eDontCare;
+      // Both MSAA and resolve textures need to be stored. A resolve IS
+      // performed.
+      return vk::AttachmentStoreOp::eStore;
   }
+  FML_UNREACHABLE();
+}
 
+constexpr bool StoreActionPerformsResolve(StoreAction store_action) {
+  switch (store_action) {
+    case StoreAction::kDontCare:
+    case StoreAction::kStore:
+      return false;
+    case StoreAction::kMultisampleResolve:
+    case StoreAction::kStoreAndMultisampleResolve:
+      return true;
+  }
   FML_UNREACHABLE();
 }
 

impeller/renderer/backend/vulkan/render_pass_builder_vk.cc

@@ -4,7 +4,6 @@
 
 #include "impeller/renderer/backend/vulkan/render_pass_builder_vk.h"
 
-#include <algorithm>
 #include <vector>
 
 #include "impeller/renderer/backend/vulkan/formats_vk.h"
@@ -37,16 +36,20 @@ RenderPassBuilderVK& RenderPassBuilderVK::SetColorAttachment(
   desc.format = ToVKImageFormat(format);
   desc.samples = ToVKSampleCount(sample_count);
   desc.loadOp = ToVKAttachmentLoadOp(load_action);
-  desc.storeOp = ToVKAttachmentStoreOp(store_action);
+  desc.storeOp = ToVKAttachmentStoreOp(store_action, false);
   desc.stencilLoadOp = vk::AttachmentLoadOp::eDontCare;
   desc.stencilStoreOp = vk::AttachmentStoreOp::eDontCare;
   desc.initialLayout = vk::ImageLayout::eGeneral;
   desc.finalLayout = vk::ImageLayout::eGeneral;
   colors_[index] = desc;
 
-  desc.samples = vk::SampleCountFlagBits::e1;
-  resolves_[index] = desc;
-
+  if (StoreActionPerformsResolve(store_action)) {
+    desc.storeOp = ToVKAttachmentStoreOp(store_action, true);
+    desc.samples = vk::SampleCountFlagBits::e1;
+    resolves_[index] = desc;
+  } else {
+    resolves_.erase(index);
+  }
   return *this;
 }
 
@@ -59,7 +62,7 @@ RenderPassBuilderVK& RenderPassBuilderVK::SetDepthStencilAttachment(
   desc.format = ToVKImageFormat(format);
   desc.samples = ToVKSampleCount(sample_count);
   desc.loadOp = ToVKAttachmentLoadOp(load_action);
-  desc.storeOp = ToVKAttachmentStoreOp(store_action);
+  desc.storeOp = ToVKAttachmentStoreOp(store_action, false);
   desc.stencilLoadOp = desc.loadOp;    // Not separable in Impeller.
   desc.stencilStoreOp = desc.storeOp;  // Not separable in Impeller.
   desc.initialLayout = vk::ImageLayout::eUndefined;
@@ -79,7 +82,7 @@ RenderPassBuilderVK& RenderPassBuilderVK::SetStencilAttachment(
   desc.loadOp = vk::AttachmentLoadOp::eDontCare;
   desc.storeOp = vk::AttachmentStoreOp::eDontCare;
   desc.stencilLoadOp = ToVKAttachmentLoadOp(load_action);
-  desc.stencilStoreOp = ToVKAttachmentStoreOp(store_action);
+  desc.stencilStoreOp = ToVKAttachmentStoreOp(store_action, false);
   desc.initialLayout = vk::ImageLayout::eUndefined;
   desc.finalLayout = vk::ImageLayout::eDepthStencilAttachmentOptimal;
   depth_stencil_ = desc;
@@ -88,8 +91,6 @@ RenderPassBuilderVK& RenderPassBuilderVK::SetStencilAttachment(
 
 vk::UniqueRenderPass RenderPassBuilderVK::Build(
     const vk::Device& device) const {
-  FML_DCHECK(colors_.size() == resolves_.size());
-
   // This must be less than `VkPhysicalDeviceLimits::maxColorAttachments` but we
   // are not checking.
   const auto color_attachments_count =
@@ -110,12 +111,12 @@ vk::UniqueRenderPass RenderPassBuilderVK::Build(
     color_refs[color.first] = color_ref;
     attachments.push_back(color.second);
 
-    if (color.second.samples != vk::SampleCountFlagBits::e1) {
+    if (auto found = resolves_.find(color.first); found != resolves_.end()) {
       vk::AttachmentReference resolve_ref;
       resolve_ref.attachment = attachments.size();
       resolve_ref.layout = vk::ImageLayout::eGeneral;
       resolve_refs[color.first] = resolve_ref;
-      attachments.push_back(resolves_.at(color.first));
+      attachments.push_back(found->second);
     }
   }
 

impeller/renderer/backend/vulkan/render_pass_builder_vk_unittests.cc

@@ -99,5 +99,39 @@ TEST(RenderPassBuilder, CreatesRenderPassWithOnlyStencil) {
   EXPECT_EQ(depth_stencil.stencilStoreOp, vk::AttachmentStoreOp::eDontCare);
 }
 
+TEST(RenderPassBuilder, CreatesMSAAResolveWithCorrectStore) {
+  RenderPassBuilderVK builder = RenderPassBuilderVK();
+  auto const context = MockVulkanContextBuilder().Build();
+
+  // Create an MSAA color attachment.
+  builder.SetColorAttachment(0, PixelFormat::kR8G8B8A8UNormInt,
+                             SampleCount::kCount4, LoadAction::kClear,
+                             StoreAction::kMultisampleResolve);
+
+  auto render_pass = builder.Build(context->GetDevice());
+
+  EXPECT_TRUE(!!render_pass);
+
+  auto maybe_color = builder.GetColorAttachments().find(0u);
+  ASSERT_NE(maybe_color, builder.GetColorAttachments().end());
+  auto color = maybe_color->second;
+
+  // MSAA Texture.
+  EXPECT_EQ(color.initialLayout, vk::ImageLayout::eGeneral);
+  EXPECT_EQ(color.finalLayout, vk::ImageLayout::eGeneral);
+  EXPECT_EQ(color.loadOp, vk::AttachmentLoadOp::eClear);
+  EXPECT_EQ(color.storeOp, vk::AttachmentStoreOp::eDontCare);
+
+  auto maybe_resolve = builder.GetResolves().find(0u);
+  ASSERT_NE(maybe_resolve, builder.GetResolves().end());
+  auto resolve = maybe_resolve->second;
+
+  // MSAA Resolve Texture.
+  EXPECT_EQ(resolve.initialLayout, vk::ImageLayout::eGeneral);
+  EXPECT_EQ(resolve.finalLayout, vk::ImageLayout::eGeneral);
+  EXPECT_EQ(resolve.loadOp, vk::AttachmentLoadOp::eClear);
+  EXPECT_EQ(resolve.storeOp, vk::AttachmentStoreOp::eStore);
+}
+
 }  // namespace testing
 }  // namespace impeller