From 64effd6af15c795c645c388f3a151f480c7a62e1 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 11 Oct 2022 13:12:03 -0700 Subject: [PATCH 1/2] [Impeller] cache glyph atlas if contents are unchanged --- impeller/entity/contents/content_context.cc | 8 +++- impeller/entity/contents/content_context.h | 5 +++ impeller/entity/contents/text_contents.cc | 10 +++-- impeller/entity/contents/text_contents.h | 1 + .../backends/skia/text_render_context_skia.cc | 32 +++++++++++---- .../backends/skia/text_render_context_skia.h | 1 + impeller/typographer/glyph_atlas.cc | 24 +++++++++++ impeller/typographer/glyph_atlas.h | 34 +++++++++++++++ impeller/typographer/lazy_glyph_atlas.cc | 3 +- impeller/typographer/lazy_glyph_atlas.h | 1 + impeller/typographer/text_render_context.cc | 3 +- impeller/typographer/text_render_context.h | 7 +++- impeller/typographer/typographer_unittests.cc | 41 +++++++++++++++---- 13 files changed, 145 insertions(+), 25 deletions(-) diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index 0e514334f358d..f459a22e9acd8 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -143,7 +143,8 @@ static std::unique_ptr CreateDefaultPipeline( ContentContext::ContentContext(std::shared_ptr context) : context_(std::move(context)), - tessellator_(std::make_shared()) { + tessellator_(std::make_shared()), + glyph_atlas_context_(std::make_shared()) { if (!context_ || !context_->IsValid()) { return; } @@ -291,6 +292,11 @@ std::shared_ptr ContentContext::GetTessellator() const { return tessellator_; } +std::shared_ptr ContentContext::GetGlyphAtlasContext() + const { + return glyph_atlas_context_; +} + std::shared_ptr ContentContext::GetContext() const { return context_; } diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index 8601b4e80c92f..2fe847cd88e27 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -68,6 +68,8 @@ #include "impeller/entity/position_color.vert.h" #include "impeller/entity/position_uv.vert.h" +#include "impeller/typographer/glyph_atlas.h" + namespace impeller { using LinearGradientFillPipeline = @@ -376,6 +378,8 @@ class ContentContext { std::shared_ptr GetContext() const; + std::shared_ptr GetGlyphAtlasContext() const; + using SubpassCallback = std::function; @@ -465,6 +469,7 @@ class ContentContext { bool is_valid_ = false; std::shared_ptr tessellator_; + std::shared_ptr glyph_atlas_context_; FML_DISALLOW_COPY_AND_ASSIGN(ContentContext); }; diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc index 97b8aab0b3e68..7b24d8dadf559 100644 --- a/impeller/entity/contents/text_contents.cc +++ b/impeller/entity/contents/text_contents.cc @@ -34,10 +34,11 @@ void TextContents::SetGlyphAtlas(std::shared_ptr atlas) { std::shared_ptr TextContents::ResolveAtlas( GlyphAtlas::Type type, + std::shared_ptr atlas_context, std::shared_ptr context) const { FML_DCHECK(lazy_atlas_); if (lazy_atlas_) { - return lazy_atlas_->CreateOrGetGlyphAtlas(type, context); + return lazy_atlas_->CreateOrGetGlyphAtlas(type, atlas_context, context); } return nullptr; @@ -172,8 +173,9 @@ static bool CommonRender(const ContentContext& renderer, bool TextContents::RenderSdf(const ContentContext& renderer, const Entity& entity, RenderPass& pass) const { - auto atlas = ResolveAtlas(GlyphAtlas::Type::kSignedDistanceField, - renderer.GetContext()); + auto atlas = + ResolveAtlas(GlyphAtlas::Type::kSignedDistanceField, + renderer.GetGlyphAtlasContext(), renderer.GetContext()); if (!atlas || !atlas->IsValid()) { VALIDATION_LOG << "Cannot render glyphs without prepared atlas."; @@ -208,7 +210,7 @@ bool TextContents::Render(const ContentContext& renderer, auto atlas = ResolveAtlas(lazy_atlas_->HasColor() ? GlyphAtlas::Type::kColorBitmap : GlyphAtlas::Type::kAlphaBitmap, - renderer.GetContext()); + renderer.GetGlyphAtlasContext(), renderer.GetContext()); if (!atlas || !atlas->IsValid()) { VALIDATION_LOG << "Cannot render glyphs without prepared atlas."; diff --git a/impeller/entity/contents/text_contents.h b/impeller/entity/contents/text_contents.h index 015502ac5028f..40d96612fc54c 100644 --- a/impeller/entity/contents/text_contents.h +++ b/impeller/entity/contents/text_contents.h @@ -52,6 +52,7 @@ class TextContents final : public Contents { std::shared_ptr ResolveAtlas( GlyphAtlas::Type type, + std::shared_ptr atlas_context, std::shared_ptr context) const; FML_DISALLOW_COPY_AND_ASSIGN(TextContents); diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index 96ef3f6851839..41c17988d1a9f 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -340,13 +340,13 @@ static std::shared_ptr UploadGlyphTextureAtlas( std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( GlyphAtlas::Type type, + std::shared_ptr atlas_context, FrameIterator frame_iterator) const { TRACE_EVENT0("impeller", __FUNCTION__); if (!IsValid()) { return nullptr; } - - auto glyph_atlas = std::make_shared(type); + auto last_atlas = atlas_context->GetGlyphAtlas(); // --------------------------------------------------------------------------- // Step 1: Collect unique font-glyph pairs in the frame. @@ -354,11 +354,25 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( auto font_glyph_pairs = CollectUniqueFontGlyphPairs(type, frame_iterator); if (font_glyph_pairs.empty()) { - return glyph_atlas; + return last_atlas; } // --------------------------------------------------------------------------- - // Step 2: Get the optimum size of the texture atlas. + // Step 2: Determine if the atlas type and font glyph pairs are identical to + // the current atlas and reuse if possible. + // --------------------------------------------------------------------------- + if (last_atlas->GetType() == type) { + auto new_glyphs = last_atlas->FindNewGlyphs(font_glyph_pairs); + if (new_glyphs.empty()) { + return last_atlas; + } + } + + auto glyph_atlas = std::make_shared(type); + atlas_context->UpdateGlyphAtlas(glyph_atlas); + + // --------------------------------------------------------------------------- + // Step 3: Get the optimum size of the texture atlas. // --------------------------------------------------------------------------- std::vector glyph_positions; const auto atlas_size = @@ -368,7 +382,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( } // --------------------------------------------------------------------------- - // Step 3: Find location of font-glyph pairs in the atlas. We have this from + // Step 4: Find location of font-glyph pairs in the atlas. We have this from // the last step. So no need to do create another rect packer. But just do a // sanity check of counts. This could also be just an assertion as only a // construction issue would cause such a failure. @@ -378,7 +392,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( } // --------------------------------------------------------------------------- - // Step 4: Record the positions in the glyph atlas. + // Step 5: Record the positions in the glyph atlas. // --------------------------------------------------------------------------- for (size_t i = 0, count = glyph_positions.size(); i < count; i++) { glyph_atlas->AddTypefaceGlyphPosition(font_glyph_pairs[i], @@ -386,7 +400,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( } // --------------------------------------------------------------------------- - // Step 5: Draw font-glyph pairs in the correct spot in the atlas. + // Step 6: Draw font-glyph pairs in the correct spot in the atlas. // --------------------------------------------------------------------------- auto bitmap = CreateAtlasBitmap(*glyph_atlas, atlas_size); if (!bitmap) { @@ -394,7 +408,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( } // --------------------------------------------------------------------------- - // Step 6: Upload the atlas as a texture. + // Step 7: Upload the atlas as a texture. // --------------------------------------------------------------------------- PixelFormat format; switch (type) { @@ -416,7 +430,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( } // --------------------------------------------------------------------------- - // Step 7: Record the texture in the glyph atlas. + // Step 8: Record the texture in the glyph atlas. // --------------------------------------------------------------------------- glyph_atlas->SetTexture(std::move(texture)); diff --git a/impeller/typographer/backends/skia/text_render_context_skia.h b/impeller/typographer/backends/skia/text_render_context_skia.h index 1b11df9d79cb0..0c97a8716e7ad 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.h +++ b/impeller/typographer/backends/skia/text_render_context_skia.h @@ -18,6 +18,7 @@ class TextRenderContextSkia : public TextRenderContext { // |TextRenderContext| std::shared_ptr CreateGlyphAtlas( GlyphAtlas::Type type, + std::shared_ptr atlas_context, FrameIterator iterator) const override; private: diff --git a/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc index dc1d48ccbbf21..1cedc2d7fea58 100644 --- a/impeller/typographer/glyph_atlas.cc +++ b/impeller/typographer/glyph_atlas.cc @@ -6,6 +6,19 @@ namespace impeller { +GlyphAtlasContext::GlyphAtlasContext() + : atlas_(std::make_shared(GlyphAtlas::Type::kAlphaBitmap)) {} + +GlyphAtlasContext::~GlyphAtlasContext() {} + +std::shared_ptr GlyphAtlasContext::GetGlyphAtlas() const { + return atlas_; +} + +void GlyphAtlasContext::UpdateGlyphAtlas(std::shared_ptr atlas) { + atlas_ = atlas; +} + GlyphAtlas::GlyphAtlas(Type type) : type_(type) {} GlyphAtlas::~GlyphAtlas() = default; @@ -61,4 +74,15 @@ size_t GlyphAtlas::IterateGlyphs( return count; } +FontGlyphPair::Vector GlyphAtlas::FindNewGlyphs( + const FontGlyphPair::Vector& new_glyphs) { + FontGlyphPair::Vector results; + for (const auto& pair : new_glyphs) { + if (positions_.find(pair) == positions_.end()) { + results.push_back(pair); + } + } + return results; +} + } // namespace impeller diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index 3c278f64550e1..1d2d757f9bf68 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -116,6 +116,17 @@ class GlyphAtlas { /// std::optional FindFontGlyphPosition(const FontGlyphPair& pair) const; + //---------------------------------------------------------------------------- + /// @brief Find a list of all font-glyph pairs not currently in the + /// atlas. + /// + /// @param[in] new_glyphs The full set of new glyphs + /// + /// @return A vector of all glyphs in new_glyphs that are not in the + /// current atlas. + /// + FontGlyphPair::Vector FindNewGlyphs(const FontGlyphPair::Vector& new_glyphs); + private: const Type type_; std::shared_ptr texture_; @@ -129,4 +140,27 @@ class GlyphAtlas { FML_DISALLOW_COPY_AND_ASSIGN(GlyphAtlas); }; +//------------------------------------------------------------------------------ +/// @brief A container for caching a glyph atlas across frames. +/// +class GlyphAtlasContext { + public: + GlyphAtlasContext(); + + ~GlyphAtlasContext(); + + //---------------------------------------------------------------------------- + /// @brief Retrieve the current glyph atlas. + std::shared_ptr GetGlyphAtlas() const; + + //---------------------------------------------------------------------------- + /// @brief Update the context with a newly constructed glyph atlas. + void UpdateGlyphAtlas(std::shared_ptr atlas); + + private: + std::shared_ptr atlas_; + + FML_DISALLOW_COPY_AND_ASSIGN(GlyphAtlasContext); +}; + } // namespace impeller diff --git a/impeller/typographer/lazy_glyph_atlas.cc b/impeller/typographer/lazy_glyph_atlas.cc index c8e05330acf58..a6f15e558fa89 100644 --- a/impeller/typographer/lazy_glyph_atlas.cc +++ b/impeller/typographer/lazy_glyph_atlas.cc @@ -26,6 +26,7 @@ bool LazyGlyphAtlas::HasColor() const { std::shared_ptr LazyGlyphAtlas::CreateOrGetGlyphAtlas( GlyphAtlas::Type type, + std::shared_ptr atlas_context, std::shared_ptr context) const { { auto atlas_it = atlas_map_.find(type); @@ -47,7 +48,7 @@ std::shared_ptr LazyGlyphAtlas::CreateOrGetGlyphAtlas( i++; return &result; }; - auto atlas = text_context->CreateGlyphAtlas(type, iterator); + auto atlas = text_context->CreateGlyphAtlas(type, atlas_context, iterator); if (!atlas || !atlas->IsValid()) { VALIDATION_LOG << "Could not create valid atlas."; return nullptr; diff --git a/impeller/typographer/lazy_glyph_atlas.h b/impeller/typographer/lazy_glyph_atlas.h index 68869aba11fe5..841bb4f2e0e46 100644 --- a/impeller/typographer/lazy_glyph_atlas.h +++ b/impeller/typographer/lazy_glyph_atlas.h @@ -23,6 +23,7 @@ class LazyGlyphAtlas { std::shared_ptr CreateOrGetGlyphAtlas( GlyphAtlas::Type type, + std::shared_ptr atlas_context, std::shared_ptr context) const; bool HasColor() const; diff --git a/impeller/typographer/text_render_context.cc b/impeller/typographer/text_render_context.cc index c19e7ac01f1fb..eabb998e18078 100644 --- a/impeller/typographer/text_render_context.cc +++ b/impeller/typographer/text_render_context.cc @@ -34,6 +34,7 @@ const std::shared_ptr& TextRenderContext::GetContext() const { std::shared_ptr TextRenderContext::CreateGlyphAtlas( GlyphAtlas::Type type, + std::shared_ptr atlas_context, const TextFrame& frame) const { size_t count = 0; FrameIterator iterator = [&]() -> const TextFrame* { @@ -43,7 +44,7 @@ std::shared_ptr TextRenderContext::CreateGlyphAtlas( } return nullptr; }; - return CreateGlyphAtlas(type, iterator); + return CreateGlyphAtlas(type, atlas_context, iterator); } } // namespace impeller diff --git a/impeller/typographer/text_render_context.h b/impeller/typographer/text_render_context.h index 7c2277070c856..c63ac912a1fa8 100644 --- a/impeller/typographer/text_render_context.h +++ b/impeller/typographer/text_render_context.h @@ -44,10 +44,13 @@ class TextRenderContext { virtual std::shared_ptr CreateGlyphAtlas( GlyphAtlas::Type type, + std::shared_ptr atlas_context, FrameIterator iterator) const = 0; - std::shared_ptr CreateGlyphAtlas(GlyphAtlas::Type type, - const TextFrame& frame) const; + std::shared_ptr CreateGlyphAtlas( + GlyphAtlas::Type type, + std::shared_ptr atlas_context, + const TextFrame& frame) const; protected: //---------------------------------------------------------------------------- diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index 3802043307901..2ed78e84927f1 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -36,12 +36,14 @@ TEST_P(TypographerTest, CanCreateRenderContext) { TEST_P(TypographerTest, CanCreateGlyphAtlas) { auto context = TextRenderContext::Create(GetContext()); + auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); SkFont sk_font; auto blob = SkTextBlob::MakeFromString("hello", sk_font); ASSERT_TRUE(blob); - auto atlas = context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, - TextFrameFromTextBlob(blob)); + auto atlas = + context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, + TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); OpenPlaygroundHere([](RenderTarget&) { return true; }); } @@ -95,12 +97,14 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { auto context = TextRenderContext::Create(GetContext()); + auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); SkFont sk_font; auto blob = SkTextBlob::MakeFromString("AGH", sk_font); ASSERT_TRUE(blob); - auto atlas = context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, - TextFrameFromTextBlob(blob)); + auto atlas = + context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, + TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -110,8 +114,32 @@ TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { atlas->GetTexture()->GetSize().height); } +TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { + auto context = TextRenderContext::Create(GetContext()); + auto atlas_context = std::make_shared(); + 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()); + + // now attempt to re-create an atlas with the same text blob. + + auto next_atlas = + context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, + TextFrameFromTextBlob(blob)); + ASSERT_EQ(atlas, next_atlas); + ASSERT_EQ(atlas_context->GetGlyphAtlas(), atlas); +} + TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { auto context = TextRenderContext::Create(GetContext()); + auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); SkFont sk_font; @@ -133,9 +161,8 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { } return nullptr; }; - - auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, iterator); + auto atlas = context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, + atlas_context, iterator); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); From e168ce280368f66863e4b3f7e52964955a479588 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 11 Oct 2022 14:22:42 -0700 Subject: [PATCH 2/2] dont create vector of pairs --- .../backends/skia/text_render_context_skia.cc | 12 +++++------- impeller/typographer/glyph_atlas.cc | 8 +++----- impeller/typographer/glyph_atlas.h | 9 ++++----- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index 41c17988d1a9f..e36a4e55e2eb0 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -358,14 +358,12 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( } // --------------------------------------------------------------------------- - // Step 2: Determine if the atlas type and font glyph pairs are identical to - // the current atlas and reuse if possible. + // Step 2: Determine if the atlas type and font glyph pairs are compatible + // with the current atlas and reuse if possible. // --------------------------------------------------------------------------- - if (last_atlas->GetType() == type) { - auto new_glyphs = last_atlas->FindNewGlyphs(font_glyph_pairs); - if (new_glyphs.empty()) { - return last_atlas; - } + if (last_atlas->GetType() == type && + last_atlas->HasSamePairs(font_glyph_pairs)) { + return last_atlas; } auto glyph_atlas = std::make_shared(type); diff --git a/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc index 1cedc2d7fea58..bbd7af8a1f1d5 100644 --- a/impeller/typographer/glyph_atlas.cc +++ b/impeller/typographer/glyph_atlas.cc @@ -74,15 +74,13 @@ size_t GlyphAtlas::IterateGlyphs( return count; } -FontGlyphPair::Vector GlyphAtlas::FindNewGlyphs( - const FontGlyphPair::Vector& new_glyphs) { - FontGlyphPair::Vector results; +bool GlyphAtlas::HasSamePairs(const FontGlyphPair::Vector& new_glyphs) { for (const auto& pair : new_glyphs) { if (positions_.find(pair) == positions_.end()) { - results.push_back(pair); + return false; } } - return results; + return true; } } // namespace impeller diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index 1d2d757f9bf68..0b533643b6437 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -117,15 +117,14 @@ class GlyphAtlas { std::optional FindFontGlyphPosition(const FontGlyphPair& pair) const; //---------------------------------------------------------------------------- - /// @brief Find a list of all font-glyph pairs not currently in the - /// atlas. + /// @brief whether this atlas contains all of the same font-glyph pairs + /// as the vector. /// /// @param[in] new_glyphs The full set of new glyphs /// - /// @return A vector of all glyphs in new_glyphs that are not in the - /// current atlas. + /// @return Whether this atlas contains all passed pairs. /// - FontGlyphPair::Vector FindNewGlyphs(const FontGlyphPair::Vector& new_glyphs); + bool HasSamePairs(const FontGlyphPair::Vector& new_glyphs); private: const Type type_;