From 9f1522f44a0f90ac208b73836a8b5aae8478ed63 Mon Sep 17 00:00:00 2001
From: Jonah Williams <jonahwilliams@google.com>
Date: Thu, 17 Nov 2022 13:53:44 -0800
Subject: [PATCH] Revert "[Impeller] reuse texture if size and type matches
 (#37527)"

This reverts commit 9500d8c4a4ce28bd024cc6e75be6289e71174b09.
---
 .../backends/skia/text_render_context_skia.cc | 55 ++++++-------------
 impeller/typographer/typographer_unittests.cc | 28 ----------
 2 files changed, 18 insertions(+), 65 deletions(-)

diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc
index b710768505a9c..c1b6405bc193b 100644
--- a/impeller/typographer/backends/skia/text_render_context_skia.cc
+++ b/impeller/typographer/backends/skia/text_render_context_skia.cc
@@ -297,8 +297,9 @@ static std::shared_ptr<SkBitmap> CreateAtlasBitmap(const GlyphAtlas& atlas,
   return bitmap;
 }
 
-static std::shared_ptr<Texture> CreateGlyphTextureAtlas(
+static std::shared_ptr<Texture> UploadGlyphTextureAtlas(
     const std::shared_ptr<Allocator>& allocator,
+    std::shared_ptr<SkBitmap> bitmap,
     const ISize& atlas_size,
     PixelFormat format) {
   TRACE_EVENT0("impeller", __FUNCTION__);
@@ -306,31 +307,24 @@ static std::shared_ptr<Texture> CreateGlyphTextureAtlas(
     return nullptr;
   }
 
+  FML_DCHECK(bitmap != nullptr);
+  const auto& pixmap = bitmap->pixmap();
+
   TextureDescriptor texture_descriptor;
   texture_descriptor.storage_mode = StorageMode::kHostVisible;
   texture_descriptor.format = format;
   texture_descriptor.size = atlas_size;
 
+  if (pixmap.rowBytes() * pixmap.height() !=
+      texture_descriptor.GetByteSizeOfBaseMipLevel()) {
+    return nullptr;
+  }
+
   auto texture = allocator->CreateTexture(texture_descriptor);
   if (!texture || !texture->IsValid()) {
     return nullptr;
   }
   texture->SetLabel("GlyphAtlas");
-  return texture;
-}
-
-bool UploadGlyphTextureAtlas(const std::shared_ptr<Texture>& texture,
-                             std::shared_ptr<SkBitmap> bitmap) {
-  TRACE_EVENT0("impeller", __FUNCTION__);
-
-  FML_DCHECK(bitmap != nullptr);
-  const auto& pixmap = bitmap->pixmap();
-
-  auto texture_descriptor = texture->GetTextureDescriptor();
-  if (pixmap.rowBytes() * pixmap.height() !=
-      texture_descriptor.GetByteSizeOfBaseMipLevel()) {
-    return false;
-  }
 
   auto mapping = std::make_shared<fml::NonOwnedMapping>(
       reinterpret_cast<const uint8_t*>(bitmap->getAddr(0, 0)),  // data
@@ -339,9 +333,9 @@ bool UploadGlyphTextureAtlas(const std::shared_ptr<Texture>& texture,
   );
 
   if (!texture->SetContents(mapping)) {
-    return false;
+    return nullptr;
   }
-  return true;
+  return texture;
 }
 
 std::shared_ptr<GlyphAtlas> TextRenderContextSkia::CreateGlyphAtlas(
@@ -427,29 +421,16 @@ std::shared_ptr<GlyphAtlas> TextRenderContextSkia::CreateGlyphAtlas(
       format = PixelFormat::kR8G8B8A8UNormInt;
       break;
   }
+  auto texture = UploadGlyphTextureAtlas(GetContext()->GetResourceAllocator(),
+                                         bitmap, atlas_size, format);
+  if (!texture) {
+    return nullptr;
+  }
 
   // ---------------------------------------------------------------------------
   // Step 8: Record the texture in the glyph atlas.
-  //
-  // If the last_texture is the same size and type, reuse this instead of
-  // creating a new texture.
   // ---------------------------------------------------------------------------
-  auto old_texture = last_atlas->GetTexture();
-  if (old_texture != nullptr &&
-      old_texture->GetTextureDescriptor().size == atlas_size &&
-      old_texture->GetTextureDescriptor().format == format) {
-    if (!UploadGlyphTextureAtlas(old_texture, bitmap)) {
-      return nullptr;
-    }
-    glyph_atlas->SetTexture(std::move(old_texture));
-  } else {
-    auto texture = CreateGlyphTextureAtlas(GetContext()->GetResourceAllocator(),
-                                           atlas_size, format);
-    if (!texture || !UploadGlyphTextureAtlas(texture, bitmap)) {
-      return nullptr;
-    }
-    glyph_atlas->SetTexture(std::move(texture));
-  }
+  glyph_atlas->SetTexture(std::move(texture));
 
   return glyph_atlas;
 }
diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc
index 4e9a79f3dd134..7bd81264c4df7 100644
--- a/impeller/typographer/typographer_unittests.cc
+++ b/impeller/typographer/typographer_unittests.cc
@@ -137,34 +137,6 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) {
   ASSERT_EQ(atlas_context->GetGlyphAtlas(), atlas);
 }
 
-TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) {
-  auto context = TextRenderContext::Create(GetContext());
-  auto atlas_context = std::make_shared<GlyphAtlasContext>();
-  ASSERT_TRUE(context && context->IsValid());
-  SkFont sk_font;
-  auto blob = SkTextBlob::MakeFromString("spooky skellingtons", sk_font);
-  ASSERT_TRUE(blob);
-  auto atlas =
-      context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context,
-                                TextFrameFromTextBlob(blob));
-  ASSERT_NE(atlas, nullptr);
-  ASSERT_NE(atlas->GetTexture(), nullptr);
-  ASSERT_EQ(atlas, atlas_context->GetGlyphAtlas());
-
-  auto* first_texture = atlas->GetTexture().get();
-
-  // now create a new glyph atlas with a nearly identical blob.
-
-  auto blob2 = SkTextBlob::MakeFromString("spooky skellington2", sk_font);
-  auto next_atlas =
-      context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context,
-                                TextFrameFromTextBlob(blob2));
-  ASSERT_NE(atlas, next_atlas);
-  auto* second_texture = next_atlas->GetTexture().get();
-
-  ASSERT_EQ(second_texture, first_texture);
-}
-
 TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) {
   auto context = TextRenderContext::Create(GetContext());
   auto atlas_context = std::make_shared<GlyphAtlasContext>();