[Impeller] More efficient usage of transient onscreen attachments. (flutter/engine#51206)

Work towards https://github.com/flutter/flutter/issues/144617

> The primary trick here is making sure that the transient attachments for the onscreen (MSAA and Depth/Stencil) are effectively recycled. In fact, I think we can actually make a single Onscreen MSAA and Onscreen Depth/Stencil and share them across the swapchain images - as they should be effectively synchronized already.

Fixes https://github.com/flutter/flutter/issues/141750

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

@@ -27,14 +27,14 @@ TEST_P(RendererTest, CachesRenderPassAndFramebuffer) {
       render_target.GetColorAttachments().find(0u)->second.resolve_texture;
   auto& texture_vk = TextureVK::Cast(*resolve_texture);
 
-  EXPECT_EQ(texture_vk.GetFramebuffer(), nullptr);
-  EXPECT_EQ(texture_vk.GetRenderPass(), nullptr);
+  EXPECT_EQ(texture_vk.GetCachedFramebuffer(), nullptr);
+  EXPECT_EQ(texture_vk.GetCachedRenderPass(), nullptr);
 
   auto buffer = GetContext()->CreateCommandBuffer();
   auto render_pass = buffer->CreateRenderPass(render_target);
 
-  EXPECT_NE(texture_vk.GetFramebuffer(), nullptr);
-  EXPECT_NE(texture_vk.GetRenderPass(), nullptr);
+  EXPECT_NE(texture_vk.GetCachedFramebuffer(), nullptr);
+  EXPECT_NE(texture_vk.GetCachedRenderPass(), nullptr);
 
   render_pass->EncodeCommands();
   GetContext()->GetCommandQueue()->Submit({buffer});

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

@@ -164,8 +164,10 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
   SharedHandleVK<vk::RenderPass> recycled_render_pass;
   SharedHandleVK<vk::Framebuffer> recycled_framebuffer;
   if (resolve_image_vk_) {
-    recycled_render_pass = TextureVK::Cast(*resolve_image_vk_).GetRenderPass();
-    recycled_framebuffer = TextureVK::Cast(*resolve_image_vk_).GetFramebuffer();
+    recycled_render_pass =
+        TextureVK::Cast(*resolve_image_vk_).GetCachedRenderPass();
+    recycled_framebuffer =
+        TextureVK::Cast(*resolve_image_vk_).GetCachedFramebuffer();
   }
 
   const auto& target_size = render_target_.GetRenderTargetSize();
@@ -192,8 +194,8 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
     return;
   }
   if (resolve_image_vk_) {
-    TextureVK::Cast(*resolve_image_vk_).SetFramebuffer(framebuffer);
-    TextureVK::Cast(*resolve_image_vk_).SetRenderPass(render_pass_);
+    TextureVK::Cast(*resolve_image_vk_).SetCachedFramebuffer(framebuffer);
+    TextureVK::Cast(*resolve_image_vk_).SetCachedRenderPass(render_pass_);
   }
 
   auto clear_values = GetVKClearValues(render_target_);

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

@@ -30,14 +30,13 @@ std::unique_ptr<SurfaceVK> SurfaceVK::WrapSwapchainImage(
     msaa_tex_desc.size = swapchain_image->GetSize();
     msaa_tex_desc.usage = static_cast<uint64_t>(TextureUsage::kRenderTarget);
 
-    if (!swapchain_image->HasMSAATexture()) {
+    if (!swapchain_image->GetMSAATexture()) {
       msaa_tex = context->GetResourceAllocator()->CreateTexture(msaa_tex_desc);
       msaa_tex->SetLabel("ImpellerOnscreenColorMSAA");
       if (!msaa_tex) {
         VALIDATION_LOG << "Could not allocate MSAA color texture.";
         return nullptr;
       }
-      swapchain_image->SetMSAATexture(msaa_tex);
     } else {
       msaa_tex = swapchain_image->GetMSAATexture();
     }
@@ -77,6 +76,16 @@ std::unique_ptr<SurfaceVK> SurfaceVK::WrapSwapchainImage(
 
   RenderTarget render_target_desc;
   render_target_desc.SetColorAttachment(color0, 0u);
+  render_target_desc.SetupDepthStencilAttachments(
+      /*context=*/*context,                            //
+      /*allocator=*/*context->GetResourceAllocator(),  //
+      /*size=*/swapchain_image->GetSize(),             //
+      /*msaa=*/enable_msaa,                            //
+      /*label=*/"Onscreen",                            //
+      /*stencil_attachment_config=*/
+      RenderTarget::kDefaultStencilAttachmentConfig,                       //
+      /*depth_stencil_texture=*/swapchain_image->GetDepthStencilTexture()  //
+  );
 
   // The constructor is private. So make_unique may not be used.
   return std::unique_ptr<SurfaceVK>(

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

@@ -7,7 +7,6 @@
 
 #include <memory>
 
-#include "flutter/fml/macros.h"
 #include "impeller/renderer/backend/vulkan/context_vk.h"
 #include "impeller/renderer/backend/vulkan/swapchain_image_vk.h"
 #include "impeller/renderer/surface.h"
@@ -18,6 +17,11 @@ class SurfaceVK final : public Surface {
  public:
   using SwapCallback = std::function<bool(void)>;
 
+  /// @brief Wrap the swapchain image in a Surface, which provides the
+  ///        additional configuration required for usage as on onscreen render
+  ///        target by Impeller.
+  ///
+  ///        This creates the associated MSAA and depth+stencil texture.
   static std::unique_ptr<SurfaceVK> WrapSwapchainImage(
       const std::shared_ptr<Context>& context,
       std::shared_ptr<SwapchainImageVK>& swapchain_image,

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

@@ -36,15 +36,20 @@ bool SwapchainImageVK::IsValid() const {
 }
 
 std::shared_ptr<Texture> SwapchainImageVK::GetMSAATexture() const {
-  return msaa_tex_;
+  return msaa_texture_;
 }
 
-bool SwapchainImageVK::HasMSAATexture() const {
-  return msaa_tex_ != nullptr;
+std::shared_ptr<Texture> SwapchainImageVK::GetDepthStencilTexture() const {
+  return depth_stencil_texture_;
 }
 
-void SwapchainImageVK::SetMSAATexture(std::shared_ptr<Texture> msaa_tex) {
-  msaa_tex_ = std::move(msaa_tex);
+void SwapchainImageVK::SetMSAATexture(std::shared_ptr<Texture> texture) {
+  msaa_texture_ = std::move(texture);
+}
+
+void SwapchainImageVK::SetDepthStencilTexture(
+    std::shared_ptr<Texture> texture) {
+  depth_stencil_texture_ = std::move(texture);
 }
 
 PixelFormat SwapchainImageVK::GetPixelFormat() const {

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

@@ -33,21 +33,24 @@ class SwapchainImageVK final : public TextureSourceVK {
 
   std::shared_ptr<Texture> GetMSAATexture() const;
 
-  bool HasMSAATexture() const;
+  std::shared_ptr<Texture> GetDepthStencilTexture() const;
 
   // |TextureSourceVK|
   vk::ImageView GetImageView() const override;
 
   vk::ImageView GetRenderTargetView() const override;
 
-  void SetMSAATexture(std::shared_ptr<Texture> msaa_tex);
+  void SetMSAATexture(std::shared_ptr<Texture> texture);
+
+  void SetDepthStencilTexture(std::shared_ptr<Texture> texture);
 
   bool IsSwapchainImage() const override { return true; }
 
  private:
   vk::Image image_ = VK_NULL_HANDLE;
   vk::UniqueImageView image_view_ = {};
-  std::shared_ptr<Texture> msaa_tex_;
+  std::shared_ptr<Texture> msaa_texture_;
+  std::shared_ptr<Texture> depth_stencil_texture_;
   bool is_valid_ = false;
 
   SwapchainImageVK(const SwapchainImageVK&) = delete;

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

@@ -6,6 +6,7 @@
 
 #include "fml/synchronization/semaphore.h"
 #include "impeller/base/validation.h"
+#include "impeller/core/formats.h"
 #include "impeller/renderer/backend/vulkan/command_buffer_vk.h"
 #include "impeller/renderer/backend/vulkan/command_encoder_vk.h"
 #include "impeller/renderer/backend/vulkan/context_vk.h"
@@ -228,6 +229,40 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr<Context>& context,
   texture_desc.size = ISize::MakeWH(swapchain_info.imageExtent.width,
                                     swapchain_info.imageExtent.height);
 
+  // Allocate a single onscreen MSAA texture and Depth+Stencil Texture to
+  // be shared by all swapchain images.
+  TextureDescriptor msaa_desc;
+  msaa_desc.storage_mode = StorageMode::kDeviceTransient;
+  msaa_desc.type = TextureType::kTexture2DMultisample;
+  msaa_desc.sample_count = SampleCount::kCount4;
+  msaa_desc.format = texture_desc.format;
+  msaa_desc.size = texture_desc.size;
+  msaa_desc.usage = static_cast<uint64_t>(TextureUsage::kRenderTarget);
+
+  // The depth+stencil configuration matches the configuration used by
+  // RenderTarget::SetupDepthStencilAttachments and matching the swapchain
+  // image dimensions and sample count.
+  TextureDescriptor depth_stencil_desc;
+  depth_stencil_desc.storage_mode = StorageMode::kDeviceTransient;
+  if (enable_msaa) {
+    depth_stencil_desc.type = TextureType::kTexture2DMultisample;
+    depth_stencil_desc.sample_count = SampleCount::kCount4;
+  } else {
+    depth_stencil_desc.type = TextureType::kTexture2D;
+    depth_stencil_desc.sample_count = SampleCount::kCount1;
+  }
+  depth_stencil_desc.format =
+      context->GetCapabilities()->GetDefaultDepthStencilFormat();
+  depth_stencil_desc.size = texture_desc.size;
+  depth_stencil_desc.usage = static_cast<uint64_t>(TextureUsage::kRenderTarget);
+
+  std::shared_ptr<Texture> msaa_texture;
+  if (enable_msaa) {
+    msaa_texture = context->GetResourceAllocator()->CreateTexture(msaa_desc);
+  }
+  std::shared_ptr<Texture> depth_stencil_texture =
+      context->GetResourceAllocator()->CreateTexture(depth_stencil_desc);
+
   std::vector<std::shared_ptr<SwapchainImageVK>> swapchain_images;
   for (const auto& image : images) {
     auto swapchain_image =
@@ -239,6 +274,8 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr<Context>& context,
       VALIDATION_LOG << "Could not create swapchain image.";
       return;
     }
+    swapchain_image->SetMSAATexture(msaa_texture);
+    swapchain_image->SetDepthStencilTexture(depth_stencil_texture);
 
     ContextVK::SetDebugName(
         vk_context.GetDevice(), swapchain_image->GetImage(),

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

@@ -9,7 +9,6 @@
 #include <utility>
 #include <vector>
 
-#include "fml/macros.h"
 #include "impeller/base/thread_safety.h"
 #include "impeller/renderer/backend/vulkan/vk.h"  // IWYU pragma: keep.
 #include "third_party/swiftshader/include/vulkan/vulkan_core.h"
@@ -40,10 +39,13 @@ struct MockImage {};
 
 struct MockSwapchainKHR {
   std::array<MockImage, 3> images;
+  size_t current_image = 0;
 };
 
 struct MockSemaphore {};
 
+struct MockFramebuffer {};
+
 static ISize currentImageSize = ISize{1, 1};
 
 class MockDevice final {
@@ -721,10 +723,26 @@ VkResult vkAcquireNextImageKHR(VkDevice device,
                                VkSemaphore semaphore,
                                VkFence fence,
                                uint32_t* pImageIndex) {
-  *pImageIndex = 0;
+  auto current_index =
+      reinterpret_cast<MockSwapchainKHR*>(swapchain)->current_image++;
+  *pImageIndex = (current_index + 1) % 3u;
   return VK_SUCCESS;
 }
 
+VkResult vkCreateFramebuffer(VkDevice device,
+                             const VkFramebufferCreateInfo* pCreateInfo,
+                             const VkAllocationCallbacks* pAllocator,
+                             VkFramebuffer* pFramebuffer) {
+  *pFramebuffer = reinterpret_cast<VkFramebuffer>(new MockFramebuffer());
+  return VK_SUCCESS;
+}
+
+void vkDestroyFramebuffer(VkDevice device,
+                          VkFramebuffer framebuffer,
+                          const VkAllocationCallbacks* pAllocator) {
+  delete reinterpret_cast<MockFramebuffer*>(framebuffer);
+}
+
 PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance,
                                             const char* pName) {
   if (strcmp("vkEnumerateInstanceExtensionProperties", pName) == 0) {
@@ -861,6 +879,10 @@ PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance,
     return (PFN_vkVoidFunction)vkDestroySurfaceKHR;
   } else if (strcmp("vkAcquireNextImageKHR", pName) == 0) {
     return (PFN_vkVoidFunction)vkAcquireNextImageKHR;
+  } else if (strcmp("vkCreateFramebuffer", pName) == 0) {
+    return (PFN_vkVoidFunction)vkCreateFramebuffer;
+  } else if (strcmp("vkDestroyFramebuffer", pName) == 0) {
+    return (PFN_vkVoidFunction)vkDestroyFramebuffer;
   }
   return noop;
 }

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

@@ -6,7 +6,9 @@
 #include "gtest/gtest.h"
 #include "impeller/renderer/backend/vulkan/swapchain_vk.h"
 #include "impeller/renderer/backend/vulkan/test/mock_vulkan.h"
+#include "impeller/renderer/backend/vulkan/texture_vk.h"
 #include "vulkan/vulkan_enums.hpp"
+#include "vulkan/vulkan_handles.hpp"
 
 namespace impeller {
 namespace testing {
@@ -52,5 +54,76 @@ TEST(SwapchainTest, RecreateSwapchainWhenSizeChanges) {
   EXPECT_EQ(image_b->GetSize(), expected_size);
 }
 
+TEST(SwapchainTest, CachesRenderPassOnSwapchainImage) {
+  auto const context = MockVulkanContextBuilder().Build();
+
+  auto surface = CreateSurface(*context);
+  auto swapchain =
+      SwapchainVK::Create(context, std::move(surface), ISize{1, 1});
+
+  EXPECT_TRUE(swapchain->IsValid());
+
+  // The mock swapchain will always create 3 images, verify each one starts
+  // out with the same MSAA and depth+stencil texture, and no cached
+  // framebuffer.
+  std::vector<std::shared_ptr<Texture>> msaa_textures;
+  std::vector<std::shared_ptr<Texture>> depth_stencil_textures;
+  for (auto i = 0u; i < 3u; i++) {
+    auto drawable = swapchain->AcquireNextDrawable();
+    RenderTarget render_target = drawable->GetTargetRenderPassDescriptor();
+
+    auto texture = render_target.GetRenderTargetTexture();
+    auto& texture_vk = TextureVK::Cast(*texture);
+    EXPECT_EQ(texture_vk.GetCachedFramebuffer(), nullptr);
+    EXPECT_EQ(texture_vk.GetCachedRenderPass(), nullptr);
+
+    auto command_buffer = context->CreateCommandBuffer();
+    auto render_pass = command_buffer->CreateRenderPass(render_target);
+    render_pass->EncodeCommands();
+
+    auto& depth = render_target.GetDepthAttachment();
+    depth_stencil_textures.push_back(depth.has_value() ? depth->texture
+                                                       : nullptr);
+    msaa_textures.push_back(
+        render_target.GetColorAttachments().find(0u)->second.texture);
+  }
+
+  for (auto i = 1; i < 3; i++) {
+    EXPECT_EQ(msaa_textures[i - 1], msaa_textures[i]);
+    EXPECT_EQ(depth_stencil_textures[i - 1], depth_stencil_textures[i]);
+  }
+
+  // After each images has been acquired once and the render pass presented,
+  // each should have a cached framebuffer and render pass.
+
+  std::vector<SharedHandleVK<vk::Framebuffer>> framebuffers;
+  std::vector<SharedHandleVK<vk::RenderPass>> render_passes;
+  for (auto i = 0u; i < 3u; i++) {
+    auto drawable = swapchain->AcquireNextDrawable();
+    RenderTarget render_target = drawable->GetTargetRenderPassDescriptor();
+
+    auto texture = render_target.GetRenderTargetTexture();
+    auto& texture_vk = TextureVK::Cast(*texture);
+
+    EXPECT_NE(texture_vk.GetCachedFramebuffer(), nullptr);
+    EXPECT_NE(texture_vk.GetCachedRenderPass(), nullptr);
+    framebuffers.push_back(texture_vk.GetCachedFramebuffer());
+    render_passes.push_back(texture_vk.GetCachedRenderPass());
+  }
+
+  // Iterate through once more to verify render passes and framebuffers are
+  // unchanged.
+  for (auto i = 0u; i < 3u; i++) {
+    auto drawable = swapchain->AcquireNextDrawable();
+    RenderTarget render_target = drawable->GetTargetRenderPassDescriptor();
+
+    auto texture = render_target.GetRenderTargetTexture();
+    auto& texture_vk = TextureVK::Cast(*texture);
+
+    EXPECT_EQ(texture_vk.GetCachedFramebuffer(), framebuffers[i]);
+    EXPECT_EQ(texture_vk.GetCachedRenderPass(), render_passes[i]);
+  }
+}
+
 }  // namespace testing
 }  // namespace impeller

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

@@ -62,4 +62,22 @@ fml::Status TextureSourceVK::SetLayout(const BarrierVK& barrier) const {
   return {};
 }
 
+void TextureSourceVK::SetCachedFramebuffer(
+    const SharedHandleVK<vk::Framebuffer>& framebuffer) {
+  framebuffer_ = framebuffer;
+}
+
+void TextureSourceVK::SetCachedRenderPass(
+    const SharedHandleVK<vk::RenderPass>& render_pass) {
+  render_pass_ = render_pass;
+}
+
+SharedHandleVK<vk::Framebuffer> TextureSourceVK::GetCachedFramebuffer() const {
+  return framebuffer_;
+}
+
+SharedHandleVK<vk::RenderPass> TextureSourceVK::GetCachedRenderPass() const {
+  return render_pass_;
+}
+
 }  // namespace impeller

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

@@ -125,12 +125,40 @@ class TextureSourceVK {
   ///
   virtual bool IsSwapchainImage() const = 0;
 
+  // These methods should only be used by render_pass_vk.h
+
+  /// Store the last framebuffer object used with this texture.
+  ///
+  /// This field is only set if this texture is used as the resolve texture
+  /// of a render pass. By construction, this framebuffer should be compatible
+  /// with any future render passes.
+  void SetCachedFramebuffer(const SharedHandleVK<vk::Framebuffer>& framebuffer);
+
+  /// Store the last render pass object used with this texture.
+  ///
+  /// This field is only set if this texture is used as the resolve texture
+  /// of a render pass. By construction, this framebuffer should be compatible
+  /// with any future render passes.
+  void SetCachedRenderPass(const SharedHandleVK<vk::RenderPass>& render_pass);
+
+  /// Retrieve the last framebuffer object used with this texture.
+  ///
+  /// May be nullptr if no previous framebuffer existed.
+  SharedHandleVK<vk::Framebuffer> GetCachedFramebuffer() const;
+
+  /// Retrieve the last render pass object used with this texture.
+  ///
+  /// May be nullptr if no previous render pass existed.
+  SharedHandleVK<vk::RenderPass> GetCachedRenderPass() const;
+
  protected:
   const TextureDescriptor desc_;
 
   explicit TextureSourceVK(TextureDescriptor desc);
 
  private:
+  SharedHandleVK<vk::Framebuffer> framebuffer_;
+  SharedHandleVK<vk::RenderPass> render_pass_;
   mutable RWMutex layout_mutex_;
   mutable vk::ImageLayout layout_ IPLR_GUARDED_BY(layout_mutex_) =
       vk::ImageLayout::eUndefined;

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

@@ -174,22 +174,22 @@ vk::ImageView TextureVK::GetRenderTargetView() const {
   return source_->GetRenderTargetView();
 }
 
-void TextureVK::SetFramebuffer(
+void TextureVK::SetCachedFramebuffer(
     const SharedHandleVK<vk::Framebuffer>& framebuffer) {
-  framebuffer_ = framebuffer;
+  source_->SetCachedFramebuffer(framebuffer);
 }
 
-void TextureVK::SetRenderPass(
+void TextureVK::SetCachedRenderPass(
     const SharedHandleVK<vk::RenderPass>& render_pass) {
-  render_pass_ = render_pass;
+  source_->SetCachedRenderPass(render_pass);
 }
 
-SharedHandleVK<vk::Framebuffer> TextureVK::GetFramebuffer() const {
-  return framebuffer_;
+SharedHandleVK<vk::Framebuffer> TextureVK::GetCachedFramebuffer() const {
+  return source_->GetCachedFramebuffer();
 }
 
-SharedHandleVK<vk::RenderPass> TextureVK::GetRenderPass() const {
-  return render_pass_;
+SharedHandleVK<vk::RenderPass> TextureVK::GetCachedRenderPass() const {
+  return source_->GetCachedRenderPass();
 }
 
 void TextureVK::SetMipMapGenerated() {

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

@@ -55,30 +55,28 @@ class TextureVK final : public Texture, public BackendCast<TextureVK, Texture> {
   /// This field is only set if this texture is used as the resolve texture
   /// of a render pass. By construction, this framebuffer should be compatible
   /// with any future render passes.
-  void SetFramebuffer(const SharedHandleVK<vk::Framebuffer>& framebuffer);
+  void SetCachedFramebuffer(const SharedHandleVK<vk::Framebuffer>& framebuffer);
 
   /// Store the last render pass object used with this texture.
   ///
   /// This field is only set if this texture is used as the resolve texture
   /// of a render pass. By construction, this framebuffer should be compatible
   /// with any future render passes.
-  void SetRenderPass(const SharedHandleVK<vk::RenderPass>& render_pass);
+  void SetCachedRenderPass(const SharedHandleVK<vk::RenderPass>& render_pass);
 
   /// Retrieve the last framebuffer object used with this texture.
   ///
   /// May be nullptr if no previous framebuffer existed.
-  SharedHandleVK<vk::Framebuffer> GetFramebuffer() const;
+  SharedHandleVK<vk::Framebuffer> GetCachedFramebuffer() const;
 
   /// Retrieve the last render pass object used with this texture.
   ///
   /// May be nullptr if no previous render pass existed.
-  SharedHandleVK<vk::RenderPass> GetRenderPass() const;
+  SharedHandleVK<vk::RenderPass> GetCachedRenderPass() const;
 
  private:
   std::weak_ptr<Context> context_;
   std::shared_ptr<TextureSourceVK> source_;
-  SharedHandleVK<vk::Framebuffer> framebuffer_ = nullptr;
-  SharedHandleVK<vk::RenderPass> render_pass_ = nullptr;
 
   // |Texture|
   void SetLabel(std::string_view label) override;

engine/src/flutter/impeller/renderer/render_target.cc

@@ -11,6 +11,7 @@
 #include "impeller/core/allocator.h"
 #include "impeller/core/formats.h"
 #include "impeller/core/texture.h"
+#include "impeller/core/texture_descriptor.h"
 #include "impeller/renderer/context.h"
 
 namespace impeller {