From effea219cbcd4e7cde5a8b46bcfeef7c050d13f5 Mon Sep 17 00:00:00 2001
From: Jonah Williams <jonahwilliams@google.com>
Date: Mon, 2 Dec 2024 09:39:09 -0800
Subject: [PATCH] [Impeller] dont cache glyph layout if atlas is invalid.
 (flutter/engine#56879)

Fixes https://github.com/flutter/flutter/issues/159578

When the screen is shut off, the atlas context is destroyed but the text frames are not. This can lead to a state where we expect the glyph atlas to be populated but it is not. make sure to check if the atlas itself is valid.
---
 .../backends/skia/typographer_context_skia.cc |  2 +-
 .../typographer/typographer_unittests.cc      | 43 +++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc b/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc
index 13dc254e67..6ef7c10abb 100644
--- a/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc
+++ b/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc
@@ -415,7 +415,7 @@ TypographerContextSkia::CollectNewGlyphs(
   std::vector<Rect> glyph_sizes;
   size_t generation_id = atlas->GetAtlasGeneration();
   for (const auto& frame : text_frames) {
-    if (frame->IsFrameComplete() &&
+    if (atlas->IsValid() && frame->IsFrameComplete() &&
         frame->GetAtlasGeneration() == generation_id &&
         !frame->GetFrameBounds(0).is_placeholder) {
       continue;
diff --git a/engine/src/flutter/impeller/typographer/typographer_unittests.cc b/engine/src/flutter/impeller/typographer/typographer_unittests.cc
index 4b8cc1cbc9..d067c6032f 100644
--- a/engine/src/flutter/impeller/typographer/typographer_unittests.cc
+++ b/engine/src/flutter/impeller/typographer/typographer_unittests.cc
@@ -612,6 +612,49 @@ TEST_P(TypographerTest, TextFrameAtlasGenerationTracksState) {
   EXPECT_EQ(frame->GetAtlasGeneration(), 2u);
 }
 
+TEST_P(TypographerTest, InvalidAtlasForcesRepopulation) {
+  SkFont font = flutter::testing::CreateTestFontOfSize(12);
+  auto blob = SkTextBlob::MakeFromString(
+      "the quick brown fox jumped over the lazy dog.", font);
+  ASSERT_TRUE(blob);
+  auto frame = MakeTextFrameFromTextBlobSkia(blob);
+
+  EXPECT_FALSE(frame->IsFrameComplete());
+
+  auto context = TypographerContextSkia::Make();
+  auto atlas_context =
+      context->CreateGlyphAtlasContext(GlyphAtlas::Type::kAlphaBitmap);
+  auto host_buffer = HostBuffer::Create(GetContext()->GetResourceAllocator(),
+                                        GetContext()->GetIdleWaiter());
+
+  auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
+                                GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                                atlas_context, frame);
+
+  // The glyph position in the atlas was not known when this value
+  // was recorded. It is marked as a placeholder.
+  EXPECT_TRUE(frame->IsFrameComplete());
+  EXPECT_TRUE(frame->GetFrameBounds(0).is_placeholder);
+  if (GetBackend() == PlaygroundBackend::kOpenGLES) {
+    // OpenGLES must always increase the atlas backend if the texture grows.
+    EXPECT_EQ(frame->GetAtlasGeneration(), 1u);
+  } else {
+    EXPECT_EQ(frame->GetAtlasGeneration(), 0u);
+  }
+
+  auto second_context = TypographerContextSkia::Make();
+  auto second_atlas_context =
+      second_context->CreateGlyphAtlasContext(GlyphAtlas::Type::kAlphaBitmap);
+
+  EXPECT_FALSE(second_atlas_context->GetGlyphAtlas()->IsValid());
+
+  atlas = CreateGlyphAtlas(*GetContext(), second_context.get(), *host_buffer,
+                           GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                           second_atlas_context, frame);
+
+  EXPECT_TRUE(second_atlas_context->GetGlyphAtlas()->IsValid());
+}
+
 }  // namespace testing
 }  // namespace impeller