[Impeller] Cache entire render target and not just allocations. (#50990)

This may be required to fix flutter/flutter#144116 , and in general the safety of the render pass caches.

Because the RenderPass / Framebuffer objects refer to specific attachments, the caches are placed on the resolve texture, which is "real". The stencil/depth and or MSAA textures may be different but that didn't seem to matter on many Android devices. It doesn't seem like its actually valid though, whether or not there is a validation error to catch it.

I can confirm this fixes flutter/flutter#144116  locally.

This change moves the cache one level up to cover the properties of the RenderTarget we care about, and ensures that we always use the same attachment combinations.

Fixes flutter/flutter#141750
Maybe flutter/flutter#144255 ?

Author: Jonah Williams

impeller/aiks/aiks_unittests.cc

@@ -3348,10 +3348,9 @@ TEST_P(AiksTest, GaussianBlurAllocatesCorrectMipCountRenderTarget) {
   picture.ToImage(aiks_context, {100, 100});
 
   size_t max_mip_count = 0;
-  for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd();
-       ++it) {
-    max_mip_count =
-        std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count);
+  for (auto it = cache->GetRenderTargetDataBegin();
+       it != cache->GetRenderTargetDataEnd(); ++it) {
+    max_mip_count = std::max(it->config.mip_count, max_mip_count);
   }
   EXPECT_EQ(max_mip_count, blur_required_mip_count);
 }
@@ -3378,10 +3377,9 @@ TEST_P(AiksTest, GaussianBlurMipMapNestedLayer) {
   picture.ToImage(aiks_context, {100, 100});
 
   size_t max_mip_count = 0;
-  for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd();
-       ++it) {
-    max_mip_count =
-        std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count);
+  for (auto it = cache->GetRenderTargetDataBegin();
+       it != cache->GetRenderTargetDataEnd(); ++it) {
+    max_mip_count = std::max(it->config.mip_count, max_mip_count);
   }
   EXPECT_EQ(max_mip_count, blur_required_mip_count);
   // The log is FML_DLOG, so only check in debug builds.
@@ -3414,10 +3412,9 @@ TEST_P(AiksTest, GaussianBlurMipMapImageFilter) {
   picture.ToImage(aiks_context, {1024, 768});
 
   size_t max_mip_count = 0;
-  for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd();
-       ++it) {
-    max_mip_count =
-        std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count);
+  for (auto it = cache->GetRenderTargetDataBegin();
+       it != cache->GetRenderTargetDataEnd(); ++it) {
+    max_mip_count = std::max(it->config.mip_count, max_mip_count);
   }
   EXPECT_EQ(max_mip_count, blur_required_mip_count);
   // The log is FML_DLOG, so only check in debug builds.
@@ -3456,10 +3453,9 @@ TEST_P(AiksTest, GaussianBlurMipMapSolidColor) {
   picture.ToImage(aiks_context, {1024, 768});
 
   size_t max_mip_count = 0;
-  for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd();
-       ++it) {
-    max_mip_count =
-        std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count);
+  for (auto it = cache->GetRenderTargetDataBegin();
+       it != cache->GetRenderTargetDataEnd(); ++it) {
+    max_mip_count = std::max(it->config.mip_count, max_mip_count);
   }
   EXPECT_EQ(max_mip_count, blur_required_mip_count);
   // The log is FML_DLOG, so only check in debug builds.

impeller/aiks/picture.cc

@@ -60,21 +60,19 @@ std::shared_ptr<Texture> Picture::RenderToTexture(
       RenderTargetAllocator(impeller_context->GetResourceAllocator());
   RenderTarget target;
   if (impeller_context->GetCapabilities()->SupportsOffscreenMSAA()) {
-    target = RenderTarget::CreateOffscreenMSAA(
-        *impeller_context,        // context
-        render_target_allocator,  // allocator
-        size,                     // size
+    target = render_target_allocator.CreateOffscreenMSAA(
+        *impeller_context,  // context
+        size,               // size
         /*mip_count=*/1,
         "Picture Snapshot MSAA",  // label
         RenderTarget::
             kDefaultColorAttachmentConfigMSAA,  // color_attachment_config
         std::nullopt                            // stencil_attachment_config
     );
   } else {
-    target = RenderTarget::CreateOffscreen(
-        *impeller_context,        // context
-        render_target_allocator,  // allocator
-        size,                     // size
+    target = render_target_allocator.CreateOffscreen(
+        *impeller_context,  // context
+        size,               // size
         /*mip_count=*/1,
         "Picture Snapshot",                           // label
         RenderTarget::kDefaultColorAttachmentConfig,  // color_attachment_config

impeller/entity/contents/checkerboard_contents_unittests.cc

@@ -34,10 +34,10 @@ TEST_P(EntityTest, RendersWithoutError) {
 
   auto content_context = GetContentContext();
   auto buffer = content_context->GetContext()->CreateCommandBuffer();
-  auto render_target = RenderTarget::CreateOffscreenMSAA(
-      *content_context->GetContext(),
-      *GetContentContext()->GetRenderTargetCache(), {100, 100},
-      /*mip_count=*/1);
+  auto render_target =
+      GetContentContext()->GetRenderTargetCache()->CreateOffscreenMSAA(
+          *content_context->GetContext(), {100, 100},
+          /*mip_count=*/1);
   auto render_pass = buffer->CreateRenderPass(render_target);
   auto recording_pass = std::make_shared<RecordingRenderPass>(
       render_pass, GetContext(), render_target);

impeller/entity/contents/content_context.cc

@@ -490,13 +490,13 @@ fml::StatusOr<RenderTarget> ContentContext::MakeSubpass(
                             : std::optional<RenderTarget::AttachmentConfig>();
 
   if (context->GetCapabilities()->SupportsOffscreenMSAA() && msaa_enabled) {
-    subpass_target = RenderTarget::CreateOffscreenMSAA(
-        *context, *GetRenderTargetCache(), texture_size,
+    subpass_target = GetRenderTargetCache()->CreateOffscreenMSAA(
+        *context, texture_size,
         /*mip_count=*/mip_count, SPrintF("%s Offscreen", label.c_str()),
         RenderTarget::kDefaultColorAttachmentConfigMSAA, depth_stencil_config);
   } else {
-    subpass_target = RenderTarget::CreateOffscreen(
-        *context, *GetRenderTargetCache(), texture_size,
+    subpass_target = GetRenderTargetCache()->CreateOffscreen(
+        *context, texture_size,
         /*mip_count=*/mip_count, SPrintF("%s Offscreen", label.c_str()),
         RenderTarget::kDefaultColorAttachmentConfig, depth_stencil_config);
   }

impeller/entity/contents/scene_contents.cc

@@ -46,9 +46,8 @@ bool SceneContents::Render(const ContentContext& renderer,
 
   RenderTarget subpass_target;
   if (renderer.GetContext()->GetCapabilities()->SupportsOffscreenMSAA()) {
-    subpass_target = RenderTarget::CreateOffscreenMSAA(
+    subpass_target = renderer.GetRenderTargetCache()->CreateOffscreenMSAA(
         *renderer.GetContext(),             // context
-        *renderer.GetRenderTargetCache(),   // allocator
         ISize(coverage.value().GetSize()),  // size
         /*mip_count=*/1,
         "SceneContents",  // label
@@ -65,9 +64,8 @@ bool SceneContents::Render(const ContentContext& renderer,
         }  // stencil_attachment_config
     );
   } else {
-    subpass_target = RenderTarget::CreateOffscreen(
+    subpass_target = renderer.GetRenderTargetCache()->CreateOffscreen(
         *renderer.GetContext(),             // context
-        *renderer.GetRenderTargetCache(),   // allocator
         ISize(coverage.value().GetSize()),  // size
         /*mip_count=*/1,
         "SceneContents",  // label

impeller/entity/contents/tiled_texture_contents_unittests.cc

@@ -32,10 +32,10 @@ TEST_P(EntityTest, TiledTextureContentsRendersWithCorrectPipeline) {
 
   auto content_context = GetContentContext();
   auto buffer = content_context->GetContext()->CreateCommandBuffer();
-  auto render_target = RenderTarget::CreateOffscreenMSAA(
-      *content_context->GetContext(),
-      *GetContentContext()->GetRenderTargetCache(), {100, 100},
-      /*mip_count=*/1);
+  auto render_target =
+      GetContentContext()->GetRenderTargetCache()->CreateOffscreenMSAA(
+          *content_context->GetContext(), {100, 100},
+          /*mip_count=*/1);
   auto render_pass = buffer->CreateRenderPass(render_target);
   auto recording_pass = std::make_shared<RecordingRenderPass>(
       render_pass, GetContext(), render_target);
@@ -74,10 +74,10 @@ TEST_P(EntityTest, TiledTextureContentsRendersWithCorrectPipelineExternalOES) {
 
   auto content_context = GetContentContext();
   auto buffer = content_context->GetContext()->CreateCommandBuffer();
-  auto render_target = RenderTarget::CreateOffscreenMSAA(
-      *content_context->GetContext(),
-      *GetContentContext()->GetRenderTargetCache(), {100, 100},
-      /*mip_count=*/1);
+  auto render_target =
+      GetContentContext()->GetRenderTargetCache()->CreateOffscreenMSAA(
+          *content_context->GetContext(), {100, 100},
+          /*mip_count=*/1);
   auto render_pass = buffer->CreateRenderPass(render_target);
 
   ASSERT_TRUE(contents.Render(*GetContentContext(), {}, *render_pass));

impeller/entity/contents/vertices_contents_unittests.cc

@@ -60,10 +60,10 @@ TEST_P(EntityTest, RendersDstPerColorWithAlpha) {
 
   auto content_context = GetContentContext();
   auto buffer = content_context->GetContext()->CreateCommandBuffer();
-  auto render_target = RenderTarget::CreateOffscreenMSAA(
-      *content_context->GetContext(),
-      *GetContentContext()->GetRenderTargetCache(), {100, 100},
-      /*mip_count=*/1);
+  auto render_target =
+      GetContentContext()->GetRenderTargetCache()->CreateOffscreenMSAA(
+          *content_context->GetContext(), {100, 100},
+          /*mip_count=*/1);
   auto render_pass = buffer->CreateRenderPass(render_target);
   auto recording_pass = std::make_shared<RecordingRenderPass>(
       render_pass, GetContext(), render_target);

impeller/entity/entity_pass.cc

@@ -292,9 +292,8 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer,
 
   RenderTarget target;
   if (context->GetCapabilities()->SupportsOffscreenMSAA()) {
-    target = RenderTarget::CreateOffscreenMSAA(
+    target = renderer.GetRenderTargetCache()->CreateOffscreenMSAA(
         /*context=*/*context,
-        /*allocator=*/*renderer.GetRenderTargetCache(),
         /*size=*/size,
         /*mip_count=*/mip_count,
         /*label=*/"EntityPass",
@@ -308,10 +307,9 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer,
         /*stencil_attachment_config=*/
         kDefaultStencilConfig);
   } else {
-    target = RenderTarget::CreateOffscreen(
-        *context,                          // context
-        *renderer.GetRenderTargetCache(),  // allocator
-        size,                              // size
+    target = renderer.GetRenderTargetCache()->CreateOffscreen(
+        *context,  // context
+        size,      // size
         /*mip_count=*/mip_count,
         "EntityPass",  // label
         RenderTarget::AttachmentConfig{
@@ -485,7 +483,7 @@ bool EntityPass::Render(ContentContext& renderer,
   // provided by the caller.
   else {
     root_render_target.SetupDepthStencilAttachments(
-        *renderer.GetContext(), *renderer.GetRenderTargetCache(),
+        *renderer.GetContext(), *renderer.GetContext()->GetResourceAllocator(),
         color0.texture->GetSize(),
         renderer.GetContext()->GetCapabilities()->SupportsOffscreenMSAA(),
         "ImpellerOnscreen", kDefaultStencilConfig);

impeller/entity/entity_pass_target_unittests.cc

@@ -24,10 +24,10 @@ TEST_P(EntityPassTargetTest, SwapWithMSAATexture) {
   }
   auto content_context = GetContentContext();
   auto buffer = content_context->GetContext()->CreateCommandBuffer();
-  auto render_target = RenderTarget::CreateOffscreenMSAA(
-      *content_context->GetContext(),
-      *GetContentContext()->GetRenderTargetCache(), {100, 100},
-      /*mip_count=*/1);
+  auto render_target =
+      GetContentContext()->GetRenderTargetCache()->CreateOffscreenMSAA(
+          *content_context->GetContext(), {100, 100},
+          /*mip_count=*/1);
 
   auto entity_pass_target = EntityPassTarget(render_target, false, false);
 

impeller/entity/entity_unittests.cc

@@ -38,6 +38,7 @@
 #include "impeller/entity/geometry/geometry.h"
 #include "impeller/entity/geometry/point_field_geometry.h"
 #include "impeller/entity/geometry/stroke_path_geometry.h"
+#include "impeller/entity/render_target_cache.h"
 #include "impeller/geometry/color.h"
 #include "impeller/geometry/geometry_asserts.h"
 #include "impeller/geometry/path_builder.h"
@@ -2248,10 +2249,10 @@ TEST_P(EntityTest, RuntimeEffectCanSuccessfullyRender) {
 
   // Create a render target with a depth-stencil, similar to how EntityPass
   // does.
-  RenderTarget target = RenderTarget::CreateOffscreenMSAA(
-      *GetContext(), *GetContentContext()->GetRenderTargetCache(),
-      {GetWindowSize().width, GetWindowSize().height}, 1,
-      "RuntimeEffect Texture");
+  RenderTarget target =
+      GetContentContext()->GetRenderTargetCache()->CreateOffscreenMSAA(
+          *GetContext(), {GetWindowSize().width, GetWindowSize().height}, 1,
+          "RuntimeEffect Texture");
   testing::MockRenderPass pass(GetContext(), target);
 
   ASSERT_TRUE(contents->Render(*GetContentContext(), entity, pass));
@@ -2590,33 +2591,6 @@ TEST_P(EntityTest, TextContentsCeilsGlyphScaleToDecimal) {
   ASSERT_EQ(TextFrame::RoundScaledFontSize(0.0f, 12), 0.0f);
 }
 
-class TestRenderTargetAllocator : public RenderTargetAllocator {
- public:
-  explicit TestRenderTargetAllocator(std::shared_ptr<Allocator> allocator)
-      : RenderTargetAllocator(std::move(allocator)) {}
-
-  ~TestRenderTargetAllocator() = default;
-
-  std::shared_ptr<Texture> CreateTexture(
-      const TextureDescriptor& desc) override {
-    allocated_.push_back(desc);
-    return RenderTargetAllocator::CreateTexture(desc);
-  }
-
-  void Start() override { RenderTargetAllocator::Start(); }
-
-  void End() override { RenderTargetAllocator::End(); }
-
-  std::vector<TextureDescriptor> GetAllocatedTextureDescriptors() const {
-    return allocated_;
-  }
-
-  void ResetDescriptors() { allocated_.clear(); }
-
- private:
-  std::vector<TextureDescriptor> allocated_;
-};
-
 TEST_P(EntityTest, AdvancedBlendCoverageHintIsNotResetByEntityPass) {
   if (GetContext()->GetCapabilities()->SupportsFramebufferFetch()) {
     GTEST_SKIP() << "Backends that support framebuffer fetch dont use coverage "
@@ -2636,31 +2610,29 @@ TEST_P(EntityTest, AdvancedBlendCoverageHintIsNotResetByEntityPass) {
   EXPECT_TRUE(coverage.has_value());
 
   auto pass = std::make_unique<EntityPass>();
-  auto test_allocator = std::make_shared<TestRenderTargetAllocator>(
-      GetContext()->GetResourceAllocator());
+  std::shared_ptr<RenderTargetCache> render_target_allocator =
+      std::make_shared<RenderTargetCache>(GetContext()->GetResourceAllocator());
   auto stencil_config = RenderTarget::AttachmentConfig{
       .storage_mode = StorageMode::kDevicePrivate,
       .load_action = LoadAction::kClear,
       .store_action = StoreAction::kDontCare,
       .clear_color = Color::BlackTransparent()};
-  auto rt = RenderTarget::CreateOffscreen(
-      *GetContext(), *test_allocator, ISize::MakeWH(1000, 1000),
+  auto rt = render_target_allocator->CreateOffscreen(
+      *GetContext(), ISize::MakeWH(1000, 1000),
       /*mip_count=*/1, "Offscreen", RenderTarget::kDefaultColorAttachmentConfig,
       stencil_config);
   auto content_context = ContentContext(
-      GetContext(), TypographerContextSkia::Make(), test_allocator);
+      GetContext(), TypographerContextSkia::Make(), render_target_allocator);
   pass->AddEntity(std::move(entity));
-  test_allocator->ResetDescriptors();
 
   EXPECT_TRUE(pass->Render(content_context, rt));
 
-  std::vector<TextureDescriptor> descriptors =
-      test_allocator->GetAllocatedTextureDescriptors();
-
-  auto contains_size = [&descriptors](ISize size) -> bool {
-    return std::find_if(descriptors.begin(), descriptors.end(),
-                        [&size](auto desc) { return desc.size == size; }) !=
-           descriptors.end();
+  auto contains_size = [&render_target_allocator](ISize size) -> bool {
+    return std::find_if(render_target_allocator->GetRenderTargetDataBegin(),
+                        render_target_allocator->GetRenderTargetDataEnd(),
+                        [&size](const auto& data) {
+                          return data.config.size == size;
+                        }) != render_target_allocator->GetRenderTargetDataEnd();
   };
 
   EXPECT_TRUE(contains_size(ISize(1000, 1000)))