Reverts "[Impeller] cache onscreen render targets. (#50751)" (flutter/engine#50871)

Reverts flutter/engine#50751

Initiated by: jonahwilliams

Reason for reverting: breaking flutter tester --enable-impeller workflow

Original PR Author: jonahwilliams

Reviewed By: {matanlurey}

This change reverts the following previous change:
Original Description:
Currently we cache render pass objects to offscreen render targets only. Caching onscreen doesn't work as 1) the render targets are stored on the texture_vk object and 2) the texture_vk object is recreated for each frame (see https://github.com/flutter/engine/blob/main/impeller/renderer/backend/vulkan/surface_vk.cc#L14 ).

To make sure the render pass is cached, move the storage to the texture source.

part of https://github.com/flutter/flutter/issues/141750

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

@@ -42,8 +42,6 @@ struct MockImage {};
 
 struct MockSemaphore {};
 
-struct MockFramebuffer {};
-
 static ISize currentImageSize = ISize{1, 1};
 
 class MockDevice final {
@@ -688,14 +686,6 @@ VkResult vkAcquireNextImageKHR(VkDevice device,
   return VK_SUCCESS;
 }
 
-VkResult vkCreateFramebuffer(VkDevice device,
-                             const VkFramebufferCreateInfo* pCreateInfo,
-                             const VkAllocationCallbacks* pAllocator,
-                             VkFramebuffer* pFramebuffer) {
-  *pFramebuffer = reinterpret_cast<VkFramebuffer>(new MockFramebuffer());
-  return VK_SUCCESS;
-}
-
 PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance,
                                             const char* pName) {
   if (strcmp("vkEnumerateInstanceExtensionProperties", pName) == 0) {
@@ -824,8 +814,6 @@ 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;
   }
   return noop;
 }

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

@@ -6,7 +6,6 @@
 #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"
 
 namespace impeller {
@@ -53,43 +52,5 @@ 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());
-
-  // We should create 3 swapchain images with the current mock setup. However,
-  // we will only ever return the first image, so the render pass and
-  // framebuffer will be cached after one call to AcquireNextDrawable.
-  auto drawable = swapchain->AcquireNextDrawable();
-  RenderTarget render_target = drawable->GetTargetRenderPassDescriptor();
-
-  auto texture = render_target.GetRenderTargetTexture();
-  auto& texture_vk = TextureVK::Cast(*texture);
-  EXPECT_EQ(texture_vk.GetFramebuffer(), nullptr);
-  EXPECT_EQ(texture_vk.GetRenderPass(), nullptr);
-
-  auto command_buffer = context->CreateCommandBuffer();
-  auto render_pass = command_buffer->CreateRenderPass(render_target);
-
-  render_pass->EncodeCommands();
-
-  EXPECT_NE(texture_vk.GetFramebuffer(), nullptr);
-  EXPECT_NE(texture_vk.GetRenderPass(), nullptr);
-
-  {
-    auto drawable = swapchain->AcquireNextDrawable();
-    auto texture = render_target.GetRenderTargetTexture();
-    auto& texture_vk = TextureVK::Cast(*texture);
-
-    EXPECT_NE(texture_vk.GetFramebuffer(), nullptr);
-    EXPECT_NE(texture_vk.GetRenderPass(), nullptr);
-  }
-}
-
 }  // namespace testing
 }  // namespace impeller

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

@@ -58,22 +58,4 @@ fml::Status TextureSourceVK::SetLayout(const BarrierVK& barrier) const {
   return {};
 }
 
-void TextureSourceVK::SetFramebuffer(
-    const SharedHandleVK<vk::Framebuffer>& framebuffer) {
-  framebuffer_ = framebuffer;
-}
-
-void TextureSourceVK::SetRenderPass(
-    const SharedHandleVK<vk::RenderPass>& render_pass) {
-  render_pass_ = render_pass;
-}
-
-SharedHandleVK<vk::Framebuffer> TextureSourceVK::GetFramebuffer() const {
-  return framebuffer_;
-}
-
-SharedHandleVK<vk::RenderPass> TextureSourceVK::GetRenderPass() const {
-  return render_pass_;
-}
-
 }  // namespace impeller

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

@@ -65,38 +65,12 @@ class TextureSourceVK {
   /// Whether or not this is a swapchain image.
   virtual bool IsSwapchainImage() const = 0;
 
-  /// 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);
-
-  /// 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);
-
-  /// Retrieve the last framebuffer object used with this texture.
-  ///
-  /// May be nullptr if no previous framebuffer existed.
-  SharedHandleVK<vk::Framebuffer> GetFramebuffer() 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;
-
  protected:
   const TextureDescriptor desc_;
 
   explicit TextureSourceVK(TextureDescriptor desc);
 
  private:
-  SharedHandleVK<vk::Framebuffer> framebuffer_ = nullptr;
-  SharedHandleVK<vk::RenderPass> render_pass_ = nullptr;
   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

@@ -175,20 +175,20 @@ vk::ImageView TextureVK::GetRenderTargetView() const {
 
 void TextureVK::SetFramebuffer(
     const SharedHandleVK<vk::Framebuffer>& framebuffer) {
-  source_->SetFramebuffer(framebuffer);
+  framebuffer_ = framebuffer;
 }
 
 void TextureVK::SetRenderPass(
     const SharedHandleVK<vk::RenderPass>& render_pass) {
-  source_->SetRenderPass(render_pass);
+  render_pass_ = render_pass;
 }
 
 SharedHandleVK<vk::Framebuffer> TextureVK::GetFramebuffer() const {
-  return source_->GetFramebuffer();
+  return framebuffer_;
 }
 
 SharedHandleVK<vk::RenderPass> TextureVK::GetRenderPass() const {
-  return source_->GetRenderPass();
+  return render_pass_;
 }
 
 }  // namespace impeller

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

@@ -73,6 +73,8 @@ class TextureVK final : public Texture, public BackendCast<TextureVK, Texture> {
  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;