diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 2d143f4a23129..653377315c5c4 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -403,34 +403,14 @@ static Rect ComputeGlyphSize(const SkFont& font, scaled_bounds.fBottom); }; -std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( - Context& context, - GlyphAtlas::Type type, - HostBuffer& host_buffer, - const std::shared_ptr& atlas_context, - const FontGlyphMap& font_glyph_map) const { - TRACE_EVENT0("impeller", __FUNCTION__); - if (!IsValid()) { - return nullptr; - } - std::shared_ptr last_atlas = atlas_context->GetGlyphAtlas(); - FML_DCHECK(last_atlas->GetType() == type); - - if (font_glyph_map.empty()) { - return last_atlas; - } - - // --------------------------------------------------------------------------- - // Step 1: Determine if the atlas type and font glyph pairs are compatible - // with the current atlas and reuse if possible. For each new font and - // glyph pair, compute the glyph size at scale. - // --------------------------------------------------------------------------- - std::vector glyph_sizes; - std::vector new_glyphs; +static void CollectNewGlyphs(const std::shared_ptr& atlas, + const FontGlyphMap& font_glyph_map, + std::vector& new_glyphs, + std::vector& glyph_sizes) { for (const auto& font_value : font_glyph_map) { const ScaledFont& scaled_font = font_value.first; const FontGlyphAtlas* font_glyph_atlas = - last_atlas->GetFontGlyphAtlas(scaled_font.font, scaled_font.scale); + atlas->GetFontGlyphAtlas(scaled_font.font, scaled_font.scale); auto metrics = scaled_font.font.GetMetrics(); @@ -462,6 +442,33 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( } } } +} + +std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( + Context& context, + GlyphAtlas::Type type, + HostBuffer& host_buffer, + const std::shared_ptr& atlas_context, + const FontGlyphMap& font_glyph_map) const { + TRACE_EVENT0("impeller", __FUNCTION__); + if (!IsValid()) { + return nullptr; + } + std::shared_ptr last_atlas = atlas_context->GetGlyphAtlas(); + FML_DCHECK(last_atlas->GetType() == type); + + if (font_glyph_map.empty()) { + return last_atlas; + } + + // --------------------------------------------------------------------------- + // Step 1: Determine if the atlas type and font glyph pairs are compatible + // with the current atlas and reuse if possible. For each new font and + // glyph pair, compute the glyph size at scale. + // --------------------------------------------------------------------------- + std::vector new_glyphs; + std::vector glyph_sizes; + CollectNewGlyphs(last_atlas, font_glyph_map, new_glyphs, glyph_sizes); if (new_glyphs.size() == 0) { return last_atlas; } @@ -528,10 +535,16 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( if (atlas_context->GetAtlasSize().height >= max_texture_height || context.GetBackendType() == Context::BackendType::kOpenGLES) { blit_old_atlas = false; - first_missing_index = 0; + new_atlas = std::make_shared(type); + + new_glyphs.clear(); + glyph_sizes.clear(); + CollectNewGlyphs(new_atlas, font_glyph_map, new_glyphs, glyph_sizes); glyph_positions.clear(); + glyph_positions.reserve(new_glyphs.size()); + first_missing_index = 0; + height_adjustment = 0; - new_atlas = std::make_shared(type); atlas_context->UpdateRectPacker(nullptr); atlas_context->UpdateGlyphAtlas(new_atlas, {0, 0}, 0); } diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index 928cba18f0c7e..7c8e9cd08852c 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -453,9 +453,22 @@ TEST_P(TypographerTest, GlyphAtlasTextureWillGrowTilMaxTextureSize) { {4096, 4096} // Shrinks! }; + SkFont sk_font_small = flutter::testing::CreateTestFontOfSize(10); + for (int i = 0; i < 13; i++) { + SkTextBlobBuilder builder; + + auto add_char = [&](const SkFont& sk_font, char c) { + int count = sk_font.countText(&c, 1, SkTextEncoding::kUTF8); + auto buffer = builder.allocRunPos(sk_font, count); + sk_font.textToGlyphs(&c, 1, SkTextEncoding::kUTF8, buffer.glyphs, count); + sk_font.getPos(buffer.glyphs, count, buffer.points(), {0, 0}); + }; + SkFont sk_font = flutter::testing::CreateTestFontOfSize(50 + i); - auto blob = SkTextBlob::MakeFromString("A", sk_font); + add_char(sk_font, 'A'); + add_char(sk_font_small, 'B'); + auto blob = builder.make(); atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, @@ -465,6 +478,11 @@ TEST_P(TypographerTest, GlyphAtlasTextureWillGrowTilMaxTextureSize) { EXPECT_EQ(atlas->GetTexture()->GetTextureDescriptor().size, expected_sizes[i]); } + + // The final atlas should contain both the "A" glyph (which was not present + // in the previous atlas) and the "B" glyph (which existed in the previous + // atlas). + ASSERT_EQ(atlas->GetGlyphCount(), 2u); } } // namespace testing