Revert "[Impeller] reuse texture if size and type matches (flutter#37527

)" (flutter#37727) This reverts commit 9500d8c.
Nayuta403 · Nov 17, 2022 · fe6f22c · fe6f22c
1 parent 49b52db
commit fe6f22c
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 65 deletions.
diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc
@@ -297,40 +297,34 @@ 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__);
   if (!allocator) {
     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
@@ -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>();