Skip to content

Commit effea21

Browse files
author
Jonah Williams
authored
[Impeller] dont cache glyph layout if atlas is invalid. (flutter/engine#56879)
Fixes flutter#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.
1 parent d28811c commit effea21

File tree

2 files changed

+44
-1
lines changed

2 files changed

+44
-1
lines changed

engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ TypographerContextSkia::CollectNewGlyphs(
415415
std::vector<Rect> glyph_sizes;
416416
size_t generation_id = atlas->GetAtlasGeneration();
417417
for (const auto& frame : text_frames) {
418-
if (frame->IsFrameComplete() &&
418+
if (atlas->IsValid() && frame->IsFrameComplete() &&
419419
frame->GetAtlasGeneration() == generation_id &&
420420
!frame->GetFrameBounds(0).is_placeholder) {
421421
continue;

engine/src/flutter/impeller/typographer/typographer_unittests.cc

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,49 @@ TEST_P(TypographerTest, TextFrameAtlasGenerationTracksState) {
612612
EXPECT_EQ(frame->GetAtlasGeneration(), 2u);
613613
}
614614

615+
TEST_P(TypographerTest, InvalidAtlasForcesRepopulation) {
616+
SkFont font = flutter::testing::CreateTestFontOfSize(12);
617+
auto blob = SkTextBlob::MakeFromString(
618+
"the quick brown fox jumped over the lazy dog.", font);
619+
ASSERT_TRUE(blob);
620+
auto frame = MakeTextFrameFromTextBlobSkia(blob);
621+
622+
EXPECT_FALSE(frame->IsFrameComplete());
623+
624+
auto context = TypographerContextSkia::Make();
625+
auto atlas_context =
626+
context->CreateGlyphAtlasContext(GlyphAtlas::Type::kAlphaBitmap);
627+
auto host_buffer = HostBuffer::Create(GetContext()->GetResourceAllocator(),
628+
GetContext()->GetIdleWaiter());
629+
630+
auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
631+
GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
632+
atlas_context, frame);
633+
634+
// The glyph position in the atlas was not known when this value
635+
// was recorded. It is marked as a placeholder.
636+
EXPECT_TRUE(frame->IsFrameComplete());
637+
EXPECT_TRUE(frame->GetFrameBounds(0).is_placeholder);
638+
if (GetBackend() == PlaygroundBackend::kOpenGLES) {
639+
// OpenGLES must always increase the atlas backend if the texture grows.
640+
EXPECT_EQ(frame->GetAtlasGeneration(), 1u);
641+
} else {
642+
EXPECT_EQ(frame->GetAtlasGeneration(), 0u);
643+
}
644+
645+
auto second_context = TypographerContextSkia::Make();
646+
auto second_atlas_context =
647+
second_context->CreateGlyphAtlasContext(GlyphAtlas::Type::kAlphaBitmap);
648+
649+
EXPECT_FALSE(second_atlas_context->GetGlyphAtlas()->IsValid());
650+
651+
atlas = CreateGlyphAtlas(*GetContext(), second_context.get(), *host_buffer,
652+
GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
653+
second_atlas_context, frame);
654+
655+
EXPECT_TRUE(second_atlas_context->GetGlyphAtlas()->IsValid());
656+
}
657+
615658
} // namespace testing
616659
} // namespace impeller
617660

0 commit comments

Comments
 (0)