From 6bbae13b93ca2626c9d189e0790a2f9cf6e0898a Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Mon, 29 Jan 2024 09:43:25 -0800
Subject: [PATCH 1/9] [Impeller] hacking caching of onscreen render pass.

---
 .../renderer/backend/vulkan/render_pass_vk.cc | 46 ++++++++++++++-----
 .../renderer/backend/vulkan/render_pass_vk.h  |  7 ++-
 .../backend/vulkan/swapchain_image_vk.h       | 21 +++++++++
 .../backend/vulkan/texture_source_vk.h        | 16 +++++++
 4 files changed, 76 insertions(+), 14 deletions(-)

diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc
index 8e45b70d7bd01..f8a1b7b4cf66a 100644
--- a/impeller/renderer/backend/vulkan/render_pass_vk.cc
+++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc
@@ -105,10 +105,8 @@ static vk::AttachmentDescription CreateAttachmentDescription(
   }
 
   // Always insert a barrier to transition to color attachment optimal.
-  if (current_layout != vk::ImageLayout::ePresentSrcKHR) {
-    // Note: This should incur a barrier.
-    current_layout = vk::ImageLayout::eGeneral;
-  }
+  // Note: This should incur a barrier.
+  current_layout = vk::ImageLayout::eGeneral;
 
   return CreateAttachmentDescription(desc.format,        //
                                      desc.sample_count,  //
@@ -152,7 +150,8 @@ static void SetTextureLayout(
 SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
     const ContextVK& context,
     const std::shared_ptr<CommandBufferVK>& command_buffer,
-    bool supports_framebuffer_fetch) const {
+    bool supports_framebuffer_fetch,
+    const SharedHandleVK<vk::RenderPass>& old_pass) const {
   std::vector<vk::AttachmentDescription> attachments;
 
   std::vector<vk::AttachmentReference> color_refs;
@@ -243,6 +242,10 @@ SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
   render_pass_desc.setPSubpasses(&subpass_desc);
   render_pass_desc.setSubpassCount(1u);
 
+  if (old_pass != nullptr) {
+    return old_pass;
+  }
+
   auto [result, pass] =
       context.GetDevice().createRenderPassUnique(render_pass_desc);
   if (result != vk::Result::eSuccess) {
@@ -256,6 +259,11 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
                            const RenderTarget& target,
                            std::shared_ptr<CommandBufferVK> command_buffer)
     : RenderPass(context, target), command_buffer_(std::move(command_buffer)) {
+  color_image_vk_ =
+      render_target_.GetColorAttachments().find(0u)->second.texture;
+  resolve_image_vk_ =
+      render_target_.GetColorAttachments().find(0u)->second.resolve_texture;
+
   const auto& vk_context = ContextVK::Cast(*context);
   const std::shared_ptr<CommandEncoderVK>& encoder =
       command_buffer_->GetEncoder();
@@ -269,16 +277,23 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
 
   const auto& target_size = render_target_.GetRenderTargetSize();
 
+  auto maybe_render_pass =
+      TextureVK::Cast(*resolve_image_vk_).GetTextureSource()->GetRenderPass();
+  auto maybe_framebuffer =
+      TextureVK::Cast(*resolve_image_vk_).GetTextureSource()->GetFramebuffer();
+
   render_pass_ = CreateVKRenderPass(
       vk_context, command_buffer_,
-      vk_context.GetCapabilities()->SupportsFramebufferFetch());
+      vk_context.GetCapabilities()->SupportsFramebufferFetch(),
+      maybe_render_pass);
   if (!render_pass_) {
     VALIDATION_LOG << "Could not create renderpass.";
     is_valid_ = false;
     return;
   }
 
-  auto framebuffer = CreateVKFramebuffer(vk_context, *render_pass_);
+  auto framebuffer =
+      CreateVKFramebuffer(vk_context, *render_pass_, maybe_framebuffer);
   if (!framebuffer) {
     VALIDATION_LOG << "Could not create framebuffer.";
     is_valid_ = false;
@@ -289,6 +304,12 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
     is_valid_ = false;
     return;
   }
+  TextureVK::Cast(*resolve_image_vk_)
+      .GetTextureSource()
+      ->SetFramebuffer(framebuffer);
+  TextureVK::Cast(*resolve_image_vk_)
+      .GetTextureSource()
+      ->SetRenderPass(render_pass_);
 
   auto clear_values = GetVKClearValues(render_target_);
 
@@ -320,10 +341,6 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
           .setExtent(vk::Extent2D(sc.GetWidth(), sc.GetHeight()));
   command_buffer_vk_.setScissor(0, 1, &scissor);
 
-  color_image_vk_ =
-      render_target_.GetColorAttachments().find(0u)->second.texture;
-  resolve_image_vk_ =
-      render_target_.GetColorAttachments().find(0u)->second.resolve_texture;
   is_valid_ = true;
 }
 
@@ -342,7 +359,8 @@ void RenderPassVK::OnSetLabel(std::string label) {
 
 SharedHandleVK<vk::Framebuffer> RenderPassVK::CreateVKFramebuffer(
     const ContextVK& context,
-    const vk::RenderPass& pass) const {
+    const vk::RenderPass& pass,
+    const SharedHandleVK<vk::Framebuffer>& old_framebuffer) const {
   vk::FramebufferCreateInfo fb_info;
 
   fb_info.renderPass = pass;
@@ -378,6 +396,10 @@ SharedHandleVK<vk::Framebuffer> RenderPassVK::CreateVKFramebuffer(
 
   fb_info.setAttachments(attachments);
 
+  if (old_framebuffer != nullptr) {
+    return old_framebuffer;
+  }
+
   auto [result, framebuffer] =
       context.GetDevice().createFramebufferUnique(fb_info);
 
diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h
index 04739363d31a9..05a227eb3f9b8 100644
--- a/impeller/renderer/backend/vulkan/render_pass_vk.h
+++ b/impeller/renderer/backend/vulkan/render_pass_vk.h
@@ -11,6 +11,7 @@
 #include "impeller/renderer/backend/vulkan/shared_object_vk.h"
 #include "impeller/renderer/render_pass.h"
 #include "impeller/renderer/render_target.h"
+#include "vulkan/vulkan_handles.hpp"
 
 namespace impeller {
 
@@ -124,11 +125,13 @@ class RenderPassVK final : public RenderPass {
   SharedHandleVK<vk::RenderPass> CreateVKRenderPass(
       const ContextVK& context,
       const std::shared_ptr<CommandBufferVK>& command_buffer,
-      bool has_subpass_dependency) const;
+      bool has_subpass_dependency,
+      const SharedHandleVK<vk::RenderPass>& old_pass) const;
 
   SharedHandleVK<vk::Framebuffer> CreateVKFramebuffer(
       const ContextVK& context,
-      const vk::RenderPass& pass) const;
+      const vk::RenderPass& pass,
+      const SharedHandleVK<vk::Framebuffer>& old_framebuffer) const;
 
   RenderPassVK(const RenderPassVK&) = delete;
 
diff --git a/impeller/renderer/backend/vulkan/swapchain_image_vk.h b/impeller/renderer/backend/vulkan/swapchain_image_vk.h
index ec426883d2975..ee2108e8dbee1 100644
--- a/impeller/renderer/backend/vulkan/swapchain_image_vk.h
+++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.h
@@ -10,6 +10,7 @@
 #include "impeller/renderer/backend/vulkan/formats_vk.h"
 #include "impeller/renderer/backend/vulkan/texture_source_vk.h"
 #include "impeller/renderer/backend/vulkan/vk.h"
+#include "vulkan/vulkan_handles.hpp"
 
 namespace impeller {
 
@@ -44,9 +45,29 @@ class SwapchainImageVK final : public TextureSourceVK {
 
   bool IsSwapchainImage() const override { return true; }
 
+  void SetFramebuffer(
+      const SharedHandleVK<vk::Framebuffer>& framebuffer) const override {
+    framebuffer_ = framebuffer;
+  }
+
+  void SetRenderPass(
+      const SharedHandleVK<vk::RenderPass>& renderpass) const override {
+    renderpass_ = renderpass;
+  }
+
+  SharedHandleVK<vk::Framebuffer> GetFramebuffer() const override {
+    return framebuffer_;
+  }
+
+  SharedHandleVK<vk::RenderPass> GetRenderPass() const override {
+    return renderpass_;
+  }
+
  private:
   vk::Image image_ = VK_NULL_HANDLE;
   vk::UniqueImageView image_view_ = {};
+  mutable SharedHandleVK<vk::Framebuffer> framebuffer_ = nullptr;
+  mutable SharedHandleVK<vk::RenderPass> renderpass_ = nullptr;
   std::shared_ptr<Texture> msaa_tex_;
   bool is_valid_ = false;
 
diff --git a/impeller/renderer/backend/vulkan/texture_source_vk.h b/impeller/renderer/backend/vulkan/texture_source_vk.h
index 0983bf74535d8..c466e36670771 100644
--- a/impeller/renderer/backend/vulkan/texture_source_vk.h
+++ b/impeller/renderer/backend/vulkan/texture_source_vk.h
@@ -10,7 +10,9 @@
 #include "impeller/core/texture_descriptor.h"
 #include "impeller/renderer/backend/vulkan/barrier_vk.h"
 #include "impeller/renderer/backend/vulkan/formats_vk.h"
+#include "impeller/renderer/backend/vulkan/shared_object_vk.h"
 #include "impeller/renderer/backend/vulkan/vk.h"
+#include "vulkan/vulkan_handles.hpp"
 
 namespace impeller {
 
@@ -63,6 +65,20 @@ class TextureSourceVK {
   /// Whether or not this is a swapchain image.
   virtual bool IsSwapchainImage() const = 0;
 
+  virtual void SetFramebuffer(
+      const SharedHandleVK<vk::Framebuffer>& framebuffer) const {}
+
+  virtual void SetRenderPass(
+      const SharedHandleVK<vk::RenderPass>& renderpass) const {}
+
+  virtual SharedHandleVK<vk::Framebuffer> GetFramebuffer() const {
+    return nullptr;
+  }
+
+  virtual SharedHandleVK<vk::RenderPass> GetRenderPass() const {
+    return nullptr;
+  }
+
  protected:
   const TextureDescriptor desc_;
 

From a292db791b6d401057c49b2608938faaf3de6a3e Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 31 Jan 2024 23:21:53 -0800
Subject: [PATCH 2/9] heck this works.

---
 .../renderer/backend/vulkan/context_vk.cc     |  2 +-
 .../backend/vulkan/swapchain_image_vk.h       | 20 ------------------
 .../backend/vulkan/texture_source_vk.h        | 21 +++++++++++--------
 3 files changed, 13 insertions(+), 30 deletions(-)

diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc
index 003d14b8b4e70..af58ede5d826f 100644
--- a/impeller/renderer/backend/vulkan/context_vk.cc
+++ b/impeller/renderer/backend/vulkan/context_vk.cc
@@ -158,7 +158,7 @@ void ContextVK::Setup(Settings settings) {
   // 1. The user has explicitly enabled it.
   // 2. We are in a combination of debug mode, and running on Android.
   // (It's possible 2 is overly conservative and we can simplify this)
-  auto enable_validation = settings.enable_validation;
+  auto enable_validation = false; //settings.enable_validation;
 
 #if defined(FML_OS_ANDROID) && !defined(NDEBUG)
   enable_validation = true;
diff --git a/impeller/renderer/backend/vulkan/swapchain_image_vk.h b/impeller/renderer/backend/vulkan/swapchain_image_vk.h
index 9847e06313f70..48673ebe4845d 100644
--- a/impeller/renderer/backend/vulkan/swapchain_image_vk.h
+++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.h
@@ -44,29 +44,9 @@ class SwapchainImageVK final : public TextureSourceVK {
 
   bool IsSwapchainImage() const override { return true; }
 
-  void SetFramebuffer(
-      const SharedHandleVK<vk::Framebuffer>& framebuffer) const override {
-    framebuffer_ = framebuffer;
-  }
-
-  void SetRenderPass(
-      const SharedHandleVK<vk::RenderPass>& renderpass) const override {
-    renderpass_ = renderpass;
-  }
-
-  SharedHandleVK<vk::Framebuffer> GetFramebuffer() const override {
-    return framebuffer_;
-  }
-
-  SharedHandleVK<vk::RenderPass> GetRenderPass() const override {
-    return renderpass_;
-  }
-
  private:
   vk::Image image_ = VK_NULL_HANDLE;
   vk::UniqueImageView image_view_ = {};
-  mutable SharedHandleVK<vk::Framebuffer> framebuffer_ = nullptr;
-  mutable SharedHandleVK<vk::RenderPass> renderpass_ = nullptr;
   std::shared_ptr<Texture> msaa_tex_;
   bool is_valid_ = false;
 
diff --git a/impeller/renderer/backend/vulkan/texture_source_vk.h b/impeller/renderer/backend/vulkan/texture_source_vk.h
index c466e36670771..5758dc148ff35 100644
--- a/impeller/renderer/backend/vulkan/texture_source_vk.h
+++ b/impeller/renderer/backend/vulkan/texture_source_vk.h
@@ -65,20 +65,21 @@ class TextureSourceVK {
   /// Whether or not this is a swapchain image.
   virtual bool IsSwapchainImage() const = 0;
 
-  virtual void SetFramebuffer(
-      const SharedHandleVK<vk::Framebuffer>& framebuffer) const {}
-
-  virtual void SetRenderPass(
-      const SharedHandleVK<vk::RenderPass>& renderpass) const {}
+  void SetFramebuffer(
+      const SharedHandleVK<vk::Framebuffer>& framebuffer) const {
+    framebuffer_ = framebuffer;
+  }
 
-  virtual SharedHandleVK<vk::Framebuffer> GetFramebuffer() const {
-    return nullptr;
+  void SetRenderPass(const SharedHandleVK<vk::RenderPass>& renderpass) const {
+    renderpass_ = renderpass;
   }
 
-  virtual SharedHandleVK<vk::RenderPass> GetRenderPass() const {
-    return nullptr;
+  SharedHandleVK<vk::Framebuffer> GetFramebuffer() const {
+    return framebuffer_;
   }
 
+  SharedHandleVK<vk::RenderPass> GetRenderPass() const { return renderpass_; }
+
  protected:
   const TextureDescriptor desc_;
 
@@ -88,6 +89,8 @@ class TextureSourceVK {
   mutable RWMutex layout_mutex_;
   mutable vk::ImageLayout layout_ IPLR_GUARDED_BY(layout_mutex_) =
       vk::ImageLayout::eUndefined;
+  mutable SharedHandleVK<vk::Framebuffer> framebuffer_ = nullptr;
+  mutable SharedHandleVK<vk::RenderPass> renderpass_ = nullptr;
 };
 
 }  // namespace impeller

From ffcefd120a11bdf68967348e2dd3c0891b3a6023 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Thu, 1 Feb 2024 08:18:24 -0800
Subject: [PATCH 3/9] ++

---
 .../backend/vulkan/texture_source_vk.cc        | 18 ++++++++++++++++++
 .../backend/vulkan/texture_source_vk.h         | 16 +++++-----------
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/impeller/renderer/backend/vulkan/texture_source_vk.cc b/impeller/renderer/backend/vulkan/texture_source_vk.cc
index 4a39bce151bd8..0d7440d45bf09 100644
--- a/impeller/renderer/backend/vulkan/texture_source_vk.cc
+++ b/impeller/renderer/backend/vulkan/texture_source_vk.cc
@@ -14,6 +14,24 @@ const TextureDescriptor& TextureSourceVK::GetTextureDescriptor() const {
   return desc_;
 }
 
+void TextureSourceVK::SetFramebuffer(
+    const SharedHandleVK<vk::Framebuffer>& framebuffer) const {
+  framebuffer_ = framebuffer;
+}
+
+void TextureSourceVK::SetRenderPass(
+    const SharedHandleVK<vk::RenderPass>& renderpass) const {
+  renderpass_ = renderpass;
+}
+
+SharedHandleVK<vk::Framebuffer> TextureSourceVK::GetFramebuffer() const {
+  return framebuffer_;
+}
+
+SharedHandleVK<vk::RenderPass> TextureSourceVK::GetRenderPass() const {
+  return renderpass_;
+}
+
 vk::ImageLayout TextureSourceVK::GetLayout() const {
   ReaderLock lock(layout_mutex_);
   return layout_;
diff --git a/impeller/renderer/backend/vulkan/texture_source_vk.h b/impeller/renderer/backend/vulkan/texture_source_vk.h
index 5758dc148ff35..00bf34de2ac02 100644
--- a/impeller/renderer/backend/vulkan/texture_source_vk.h
+++ b/impeller/renderer/backend/vulkan/texture_source_vk.h
@@ -65,20 +65,14 @@ class TextureSourceVK {
   /// Whether or not this is a swapchain image.
   virtual bool IsSwapchainImage() const = 0;
 
-  void SetFramebuffer(
-      const SharedHandleVK<vk::Framebuffer>& framebuffer) const {
-    framebuffer_ = framebuffer;
-  }
+  // These methods should only be used by render_pass_vk.h
+  void SetFramebuffer(const SharedHandleVK<vk::Framebuffer>& framebuffer) const;
 
-  void SetRenderPass(const SharedHandleVK<vk::RenderPass>& renderpass) const {
-    renderpass_ = renderpass;
-  }
+  void SetRenderPass(const SharedHandleVK<vk::RenderPass>& renderpass) const;
 
-  SharedHandleVK<vk::Framebuffer> GetFramebuffer() const {
-    return framebuffer_;
-  }
+  SharedHandleVK<vk::Framebuffer> GetFramebuffer() const;
 
-  SharedHandleVK<vk::RenderPass> GetRenderPass() const { return renderpass_; }
+  SharedHandleVK<vk::RenderPass> GetRenderPass() const;
 
  protected:
   const TextureDescriptor desc_;

From 4463ab82c60a903c8d1a4f32b91ba59d49ce1d37 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Thu, 1 Feb 2024 08:20:05 -0800
Subject: [PATCH 4/9] enable validation.

---
 impeller/renderer/backend/vulkan/context_vk.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc
index af58ede5d826f..003d14b8b4e70 100644
--- a/impeller/renderer/backend/vulkan/context_vk.cc
+++ b/impeller/renderer/backend/vulkan/context_vk.cc
@@ -158,7 +158,7 @@ void ContextVK::Setup(Settings settings) {
   // 1. The user has explicitly enabled it.
   // 2. We are in a combination of debug mode, and running on Android.
   // (It's possible 2 is overly conservative and we can simplify this)
-  auto enable_validation = false; //settings.enable_validation;
+  auto enable_validation = settings.enable_validation;
 
 #if defined(FML_OS_ANDROID) && !defined(NDEBUG)
   enable_validation = true;

From 43dd21c1b6be6f4dfe7264aa8444285da47d9b90 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Thu, 1 Feb 2024 10:23:03 -0800
Subject: [PATCH 5/9] move cache to texture itself.

---
 .../renderer/backend/vulkan/render_pass_vk.cc  | 14 ++++----------
 .../backend/vulkan/texture_source_vk.cc        | 18 ------------------
 .../backend/vulkan/texture_source_vk.h         | 11 -----------
 impeller/renderer/backend/vulkan/texture_vk.cc | 18 ++++++++++++++++++
 impeller/renderer/backend/vulkan/texture_vk.h  | 11 +++++++++++
 5 files changed, 33 insertions(+), 39 deletions(-)

diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc
index ca92760cf82bf..09bc6d10ff6f0 100644
--- a/impeller/renderer/backend/vulkan/render_pass_vk.cc
+++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc
@@ -162,10 +162,8 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
         encoder->Track(attachment.resolve_texture);
         return true;
       });
-  auto maybe_render_pass =
-      TextureVK::Cast(*resolve_image_vk_).GetTextureSource()->GetRenderPass();
-  auto maybe_framebuffer =
-      TextureVK::Cast(*resolve_image_vk_).GetTextureSource()->GetFramebuffer();
+  auto maybe_render_pass = TextureVK::Cast(*resolve_image_vk_).GetRenderPass();
+  auto maybe_framebuffer = TextureVK::Cast(*resolve_image_vk_).GetFramebuffer();
 
   const auto& target_size = render_target_.GetRenderTargetSize();
 
@@ -189,12 +187,8 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
     is_valid_ = false;
     return;
   }
-  TextureVK::Cast(*resolve_image_vk_)
-      .GetTextureSource()
-      ->SetFramebuffer(framebuffer);
-  TextureVK::Cast(*resolve_image_vk_)
-      .GetTextureSource()
-      ->SetRenderPass(render_pass_);
+  TextureVK::Cast(*resolve_image_vk_).SetFramebuffer(framebuffer);
+  TextureVK::Cast(*resolve_image_vk_).SetRenderPass(render_pass_);
 
   auto clear_values = GetVKClearValues(render_target_);
 
diff --git a/impeller/renderer/backend/vulkan/texture_source_vk.cc b/impeller/renderer/backend/vulkan/texture_source_vk.cc
index 0d7440d45bf09..4a39bce151bd8 100644
--- a/impeller/renderer/backend/vulkan/texture_source_vk.cc
+++ b/impeller/renderer/backend/vulkan/texture_source_vk.cc
@@ -14,24 +14,6 @@ const TextureDescriptor& TextureSourceVK::GetTextureDescriptor() const {
   return desc_;
 }
 
-void TextureSourceVK::SetFramebuffer(
-    const SharedHandleVK<vk::Framebuffer>& framebuffer) const {
-  framebuffer_ = framebuffer;
-}
-
-void TextureSourceVK::SetRenderPass(
-    const SharedHandleVK<vk::RenderPass>& renderpass) const {
-  renderpass_ = renderpass;
-}
-
-SharedHandleVK<vk::Framebuffer> TextureSourceVK::GetFramebuffer() const {
-  return framebuffer_;
-}
-
-SharedHandleVK<vk::RenderPass> TextureSourceVK::GetRenderPass() const {
-  return renderpass_;
-}
-
 vk::ImageLayout TextureSourceVK::GetLayout() const {
   ReaderLock lock(layout_mutex_);
   return layout_;
diff --git a/impeller/renderer/backend/vulkan/texture_source_vk.h b/impeller/renderer/backend/vulkan/texture_source_vk.h
index 00bf34de2ac02..4f76a8067cf4f 100644
--- a/impeller/renderer/backend/vulkan/texture_source_vk.h
+++ b/impeller/renderer/backend/vulkan/texture_source_vk.h
@@ -65,15 +65,6 @@ class TextureSourceVK {
   /// Whether or not this is a swapchain image.
   virtual bool IsSwapchainImage() const = 0;
 
-  // These methods should only be used by render_pass_vk.h
-  void SetFramebuffer(const SharedHandleVK<vk::Framebuffer>& framebuffer) const;
-
-  void SetRenderPass(const SharedHandleVK<vk::RenderPass>& renderpass) const;
-
-  SharedHandleVK<vk::Framebuffer> GetFramebuffer() const;
-
-  SharedHandleVK<vk::RenderPass> GetRenderPass() const;
-
  protected:
   const TextureDescriptor desc_;
 
@@ -83,8 +74,6 @@ class TextureSourceVK {
   mutable RWMutex layout_mutex_;
   mutable vk::ImageLayout layout_ IPLR_GUARDED_BY(layout_mutex_) =
       vk::ImageLayout::eUndefined;
-  mutable SharedHandleVK<vk::Framebuffer> framebuffer_ = nullptr;
-  mutable SharedHandleVK<vk::RenderPass> renderpass_ = nullptr;
 };
 
 }  // namespace impeller
diff --git a/impeller/renderer/backend/vulkan/texture_vk.cc b/impeller/renderer/backend/vulkan/texture_vk.cc
index f96b4940a4d98..9cc6526c25859 100644
--- a/impeller/renderer/backend/vulkan/texture_vk.cc
+++ b/impeller/renderer/backend/vulkan/texture_vk.cc
@@ -173,4 +173,22 @@ vk::ImageView TextureVK::GetRenderTargetView() const {
   return source_->GetRenderTargetView();
 }
 
+void TextureVK::SetFramebuffer(
+    const SharedHandleVK<vk::Framebuffer>& framebuffer) {
+  framebuffer_ = framebuffer;
+}
+
+void TextureVK::SetRenderPass(
+    const SharedHandleVK<vk::RenderPass>& renderpass) {
+  renderpass_ = renderpass;
+}
+
+SharedHandleVK<vk::Framebuffer> TextureVK::GetFramebuffer() const {
+  return framebuffer_;
+}
+
+SharedHandleVK<vk::RenderPass> TextureVK::GetRenderPass() const {
+  return renderpass_;
+}
+
 }  // namespace impeller
diff --git a/impeller/renderer/backend/vulkan/texture_vk.h b/impeller/renderer/backend/vulkan/texture_vk.h
index fd36213988bca..2b3a5383a17dc 100644
--- a/impeller/renderer/backend/vulkan/texture_vk.h
+++ b/impeller/renderer/backend/vulkan/texture_vk.h
@@ -44,9 +44,20 @@ class TextureVK final : public Texture, public BackendCast<TextureVK, Texture> {
 
   bool IsSwapchainImage() const { return source_->IsSwapchainImage(); }
 
+  // These methods should only be used by render_pass_vk.h
+  void SetFramebuffer(const SharedHandleVK<vk::Framebuffer>& framebuffer);
+
+  void SetRenderPass(const SharedHandleVK<vk::RenderPass>& renderpass);
+
+  SharedHandleVK<vk::Framebuffer> GetFramebuffer() const;
+
+  SharedHandleVK<vk::RenderPass> GetRenderPass() const;
+
  private:
   std::weak_ptr<Context> context_;
   std::shared_ptr<TextureSourceVK> source_;
+  SharedHandleVK<vk::Framebuffer> framebuffer_ = nullptr;
+  SharedHandleVK<vk::RenderPass> renderpass_ = nullptr;
 
   // |Texture|
   void SetLabel(std::string_view label) override;

From 216e185e0c23f71cd9579764a20fa7782d08bcaf Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Thu, 1 Feb 2024 14:05:11 -0800
Subject: [PATCH 6/9] NPE.

---
 .../renderer/backend/vulkan/render_pass_vk.cc     | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc
index 09bc6d10ff6f0..fe7cc7403c16f 100644
--- a/impeller/renderer/backend/vulkan/render_pass_vk.cc
+++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc
@@ -25,6 +25,7 @@
 #include "impeller/renderer/backend/vulkan/shared_object_vk.h"
 #include "impeller/renderer/backend/vulkan/texture_vk.h"
 #include "impeller/renderer/backend/vulkan/vk.h"
+#include "vulkan/vulkan_handles.hpp"
 
 namespace impeller {
 
@@ -162,8 +163,12 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
         encoder->Track(attachment.resolve_texture);
         return true;
       });
-  auto maybe_render_pass = TextureVK::Cast(*resolve_image_vk_).GetRenderPass();
-  auto maybe_framebuffer = TextureVK::Cast(*resolve_image_vk_).GetFramebuffer();
+  SharedHandleVK<vk::RenderPass> maybe_render_pass;
+  SharedHandleVK<vk::Framebuffer> maybe_framebuffer;
+  if (resolve_image_vk_) {
+    maybe_render_pass = TextureVK::Cast(*resolve_image_vk_).GetRenderPass();
+    maybe_framebuffer = TextureVK::Cast(*resolve_image_vk_).GetFramebuffer();
+  }
 
   const auto& target_size = render_target_.GetRenderTargetSize();
 
@@ -187,8 +192,10 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
     is_valid_ = false;
     return;
   }
-  TextureVK::Cast(*resolve_image_vk_).SetFramebuffer(framebuffer);
-  TextureVK::Cast(*resolve_image_vk_).SetRenderPass(render_pass_);
+  if (resolve_image_vk_) {
+    TextureVK::Cast(*resolve_image_vk_).SetFramebuffer(framebuffer);
+    TextureVK::Cast(*resolve_image_vk_).SetRenderPass(render_pass_);
+  }
 
   auto clear_values = GetVKClearValues(render_target_);
 

From 6873355e15d1ec9af98b4c55ebc5d0808ae930f9 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Fri, 2 Feb 2024 11:21:05 -0800
Subject: [PATCH 7/9] aaron review.

---
 .../renderer/backend/vulkan/render_pass_vk.cc | 35 ++++++++++---------
 .../renderer/backend/vulkan/render_pass_vk.h  |  5 ++-
 impeller/renderer/backend/vulkan/texture_vk.h | 17 +++++++++
 3 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc
index fe7cc7403c16f..f942d99ea64e2 100644
--- a/impeller/renderer/backend/vulkan/render_pass_vk.cc
+++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc
@@ -80,7 +80,7 @@ static std::vector<vk::ClearValue> GetVKClearValues(
 
 SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
     const ContextVK& context,
-    const SharedHandleVK<vk::RenderPass>& old_renderpass,
+    const SharedHandleVK<vk::RenderPass>& recycled_renderpass,
     const std::shared_ptr<CommandBufferVK>& command_buffer) const {
   BarrierVK barrier;
   barrier.new_layout = vk::ImageLayout::eGeneral;
@@ -128,8 +128,14 @@ SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
     );
     TextureVK::Cast(*stencil->texture).SetLayout(barrier);
   }
-  if (old_renderpass != nullptr) {
-    return old_renderpass;
+
+  // There may exist a previous recycled render pass that we can continue using.
+  // This is probably compatible with the render pass we are about to construct,
+  // but I have not conclusively proven this. If there are scenarios that
+  // produce validation warnings, we could use them to determine if we need
+  // additional checks at this point to determine reusability.
+  if (recycled_renderpass != nullptr) {
+    return recycled_renderpass;
   }
 
   auto pass = builder.Build(context.GetDevice());
@@ -163,25 +169,27 @@ RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,
         encoder->Track(attachment.resolve_texture);
         return true;
       });
-  SharedHandleVK<vk::RenderPass> maybe_render_pass;
-  SharedHandleVK<vk::Framebuffer> maybe_framebuffer;
+
+  SharedHandleVK<vk::RenderPass> recycled_render_pass;
+  SharedHandleVK<vk::Framebuffer> recycled_framebuffer;
   if (resolve_image_vk_) {
-    maybe_render_pass = TextureVK::Cast(*resolve_image_vk_).GetRenderPass();
-    maybe_framebuffer = TextureVK::Cast(*resolve_image_vk_).GetFramebuffer();
+    recycled_render_pass = TextureVK::Cast(*resolve_image_vk_).GetRenderPass();
+    recycled_framebuffer = TextureVK::Cast(*resolve_image_vk_).GetFramebuffer();
   }
 
   const auto& target_size = render_target_.GetRenderTargetSize();
 
   render_pass_ =
-      CreateVKRenderPass(vk_context, maybe_render_pass, command_buffer_);
+      CreateVKRenderPass(vk_context, recycled_render_pass, command_buffer_);
   if (!render_pass_) {
     VALIDATION_LOG << "Could not create renderpass.";
     is_valid_ = false;
     return;
   }
 
-  auto framebuffer =
-      CreateVKFramebuffer(vk_context, *render_pass_, maybe_framebuffer);
+  auto framebuffer = (recycled_framebuffer == nullptr)
+                         ? CreateVKFramebuffer(vk_context, *render_pass_)
+                         : recycled_framebuffer;
   if (!framebuffer) {
     VALIDATION_LOG << "Could not create framebuffer.";
     is_valid_ = false;
@@ -245,8 +253,7 @@ void RenderPassVK::OnSetLabel(std::string label) {
 
 SharedHandleVK<vk::Framebuffer> RenderPassVK::CreateVKFramebuffer(
     const ContextVK& context,
-    const vk::RenderPass& pass,
-    const SharedHandleVK<vk::Framebuffer>& old_framebuffer) const {
+    const vk::RenderPass& pass) const {
   vk::FramebufferCreateInfo fb_info;
 
   fb_info.renderPass = pass;
@@ -282,10 +289,6 @@ SharedHandleVK<vk::Framebuffer> RenderPassVK::CreateVKFramebuffer(
 
   fb_info.setAttachments(attachments);
 
-  if (old_framebuffer != nullptr) {
-    return old_framebuffer;
-  }
-
   auto [result, framebuffer] =
       context.GetDevice().createFramebufferUnique(fb_info);
 
diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h
index a9fd33411c001..bf05e523d7ced 100644
--- a/impeller/renderer/backend/vulkan/render_pass_vk.h
+++ b/impeller/renderer/backend/vulkan/render_pass_vk.h
@@ -124,13 +124,12 @@ class RenderPassVK final : public RenderPass {
 
   SharedHandleVK<vk::RenderPass> CreateVKRenderPass(
       const ContextVK& context,
-      const SharedHandleVK<vk::RenderPass>& old_renderpass,
+      const SharedHandleVK<vk::RenderPass>& recycled_renderpass,
       const std::shared_ptr<CommandBufferVK>& command_buffer) const;
 
   SharedHandleVK<vk::Framebuffer> CreateVKFramebuffer(
       const ContextVK& context,
-      const vk::RenderPass& pass,
-      const SharedHandleVK<vk::Framebuffer>& old_framebuffer) const;
+      const vk::RenderPass& pass) const;
 
   RenderPassVK(const RenderPassVK&) = delete;
 
diff --git a/impeller/renderer/backend/vulkan/texture_vk.h b/impeller/renderer/backend/vulkan/texture_vk.h
index 2b3a5383a17dc..1f10e15e18670 100644
--- a/impeller/renderer/backend/vulkan/texture_vk.h
+++ b/impeller/renderer/backend/vulkan/texture_vk.h
@@ -45,12 +45,29 @@ class TextureVK final : public Texture, public BackendCast<TextureVK, Texture> {
   bool IsSwapchainImage() const { return source_->IsSwapchainImage(); }
 
   // These methods should only be used by render_pass_vk.h
+
+  /// @brief 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 SetFramebuffer(const SharedHandleVK<vk::Framebuffer>& framebuffer);
 
+  /// @brief 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>& renderpass);
 
+  /// @brief Retrieve the last framebuffer object used with this texture.
+  ///
+  /// May be nullptr if no previous framebuffer existed.
   SharedHandleVK<vk::Framebuffer> GetFramebuffer() const;
 
+  /// @brief Retrieve the last render pass object used with this texture.
+  ///
+  /// May be nullptr if no previous render pass existed.
   SharedHandleVK<vk::RenderPass> GetRenderPass() const;
 
  private:

From 2b7e44faac39499a2a7ba1f1d09250060cde1fa9 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Fri, 2 Feb 2024 13:25:15 -0800
Subject: [PATCH 8/9] ++

---
 impeller/renderer/backend/vulkan/BUILD.gn     |  2 +
 .../vulkan/render_pass_cache_unittests.cc     | 51 +++++++++++++++++++
 .../renderer/backend/vulkan/texture_vk.cc     |  6 +--
 impeller/renderer/backend/vulkan/texture_vk.h | 12 ++---
 4 files changed, 62 insertions(+), 9 deletions(-)
 create mode 100644 impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc

diff --git a/impeller/renderer/backend/vulkan/BUILD.gn b/impeller/renderer/backend/vulkan/BUILD.gn
index b80b9abde342c..372dd3dcea6b9 100644
--- a/impeller/renderer/backend/vulkan/BUILD.gn
+++ b/impeller/renderer/backend/vulkan/BUILD.gn
@@ -14,6 +14,7 @@ impeller_component("vulkan_unittests") {
     "context_vk_unittests.cc",
     "descriptor_pool_vk_unittests.cc",
     "fence_waiter_vk_unittests.cc",
+    "render_pass_cache_unittests.cc",
     "resource_manager_vk_unittests.cc",
     "test/gpu_tracer_unittests.cc",
     "test/mock_vulkan.cc",
@@ -23,6 +24,7 @@ impeller_component("vulkan_unittests") {
   ]
   deps = [
     ":vulkan",
+    "../../../playground:playground_test",
     "//flutter/testing:testing_lib",
   ]
 }
diff --git a/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc b/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc
new file mode 100644
index 0000000000000..e2fc63ea5796a
--- /dev/null
+++ b/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc
@@ -0,0 +1,51 @@
+// 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 "gtest/gtest.h"
+#include "impeller/playground/playground_test.h"
+#include "impeller/renderer/backend/vulkan/texture_vk.h"
+#include "impeller/renderer/render_target.h"
+
+namespace impeller {
+namespace testing {
+
+using RendererTest = PlaygroundTest;
+
+TEST_P(RendererTest, CachesRenderPassAndFramebuffer) {
+  if (GetBackend() != PlaygroundBackend::kVulkan) {
+    GTEST_SKIP() << "Test only applies to Vulkan";
+  }
+
+  auto allocator = std::make_shared<RenderTargetAllocator>(
+      GetContext()->GetResourceAllocator());
+
+  auto render_target = RenderTarget::CreateOffscreenMSAA(
+      *GetContext(), *allocator, {100, 100}, 1);
+  auto resolve_texture =
+      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);
+
+  auto buffer = GetContext()->CreateCommandBuffer();
+  auto render_pass = buffer->CreateRenderPass(render_target);
+
+  EXPECT_NE(texture_vk.GetFramebuffer(), nullptr);
+  EXPECT_NE(texture_vk.GetRenderPass(), nullptr);
+
+  render_pass->EncodeCommands();
+  GetContext()->GetCommandQueue()->Submit({buffer});
+
+  // Can be reused without error.
+  auto buffer_2 = GetContext()->CreateCommandBuffer();
+  auto render_pass_2 = buffer_2->CreateRenderPass(render_target);
+
+  EXPECT_TRUE(render_pass_2->EncodeCommands());
+  EXPECT_TRUE(GetContext()->GetCommandQueue()->Submit({buffer_2}).ok());
+}
+
+}  // namespace testing
+}  // namespace impeller
diff --git a/impeller/renderer/backend/vulkan/texture_vk.cc b/impeller/renderer/backend/vulkan/texture_vk.cc
index 9cc6526c25859..b105d31b96e85 100644
--- a/impeller/renderer/backend/vulkan/texture_vk.cc
+++ b/impeller/renderer/backend/vulkan/texture_vk.cc
@@ -179,8 +179,8 @@ void TextureVK::SetFramebuffer(
 }
 
 void TextureVK::SetRenderPass(
-    const SharedHandleVK<vk::RenderPass>& renderpass) {
-  renderpass_ = renderpass;
+    const SharedHandleVK<vk::RenderPass>& render_pass) {
+  render_pass_ = render_pass;
 }
 
 SharedHandleVK<vk::Framebuffer> TextureVK::GetFramebuffer() const {
@@ -188,7 +188,7 @@ SharedHandleVK<vk::Framebuffer> TextureVK::GetFramebuffer() const {
 }
 
 SharedHandleVK<vk::RenderPass> TextureVK::GetRenderPass() const {
-  return renderpass_;
+  return render_pass_;
 }
 
 }  // namespace impeller
diff --git a/impeller/renderer/backend/vulkan/texture_vk.h b/impeller/renderer/backend/vulkan/texture_vk.h
index 1f10e15e18670..5826e3df78174 100644
--- a/impeller/renderer/backend/vulkan/texture_vk.h
+++ b/impeller/renderer/backend/vulkan/texture_vk.h
@@ -46,26 +46,26 @@ class TextureVK final : public Texture, public BackendCast<TextureVK, Texture> {
 
   // These methods should only be used by render_pass_vk.h
 
-  /// @brief Store the last framebuffer object used with this texture.
+  /// 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 SetFramebuffer(const SharedHandleVK<vk::Framebuffer>& framebuffer);
 
-  /// @brief Store the last render pass object used with this texture.
+  /// 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>& renderpass);
+  void SetRenderPass(const SharedHandleVK<vk::RenderPass>& render_pass);
 
-  /// @brief Retrieve the last framebuffer object used with this texture.
+  /// Retrieve the last framebuffer object used with this texture.
   ///
   /// May be nullptr if no previous framebuffer existed.
   SharedHandleVK<vk::Framebuffer> GetFramebuffer() const;
 
-  /// @brief Retrieve the last render pass object used with this texture.
+  /// Retrieve the last render pass object used with this texture.
   ///
   /// May be nullptr if no previous render pass existed.
   SharedHandleVK<vk::RenderPass> GetRenderPass() const;
@@ -74,7 +74,7 @@ class TextureVK final : public Texture, public BackendCast<TextureVK, Texture> {
   std::weak_ptr<Context> context_;
   std::shared_ptr<TextureSourceVK> source_;
   SharedHandleVK<vk::Framebuffer> framebuffer_ = nullptr;
-  SharedHandleVK<vk::RenderPass> renderpass_ = nullptr;
+  SharedHandleVK<vk::RenderPass> render_pass_ = nullptr;
 
   // |Texture|
   void SetLabel(std::string_view label) override;

From c11a0e243282a3de7d98b1cd7a9e4e9dbeeeb8ca Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Fri, 2 Feb 2024 14:43:47 -0800
Subject: [PATCH 9/9] add exclusion.

---
 ci/licenses_golden/excluded_files | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files
index f7f8bc69f2a99..b8a39ae1b825d 100644
--- a/ci/licenses_golden/excluded_files
+++ b/ci/licenses_golden/excluded_files
@@ -183,6 +183,7 @@
 ../../../flutter/impeller/renderer/backend/vulkan/context_vk_unittests.cc
 ../../../flutter/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc
 ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk_unittests.cc
+../../../flutter/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc
 ../../../flutter/impeller/renderer/backend/vulkan/resource_manager_vk_unittests.cc
 ../../../flutter/impeller/renderer/backend/vulkan/test
 ../../../flutter/impeller/renderer/blit_pass_unittests.cc