[Impeller] remove shared_ptr copies during text contents rendering. (#49837)

We copied the shared ptr to make it easy to return nullptr, but instead we could use a static const assigned to nullptr

Author: Jonah Williams

impeller/entity/contents/content_context.h

@@ -731,11 +731,11 @@ class ContentContext {
       const RenderTarget& subpass_target,
       const SubpassCallback& subpass_callback) const;
 
-  std::shared_ptr<LazyGlyphAtlas> GetLazyGlyphAtlas() const {
+  const std::shared_ptr<LazyGlyphAtlas>& GetLazyGlyphAtlas() const {
     return lazy_glyph_atlas_;
   }
 
-  std::shared_ptr<RenderTargetAllocator> GetRenderTargetCache() const {
+  const std::shared_ptr<RenderTargetAllocator>& GetRenderTargetCache() const {
     return render_target_cache_;
   }
 

impeller/entity/contents/text_contents.cc

@@ -26,18 +26,6 @@ void TextContents::SetTextFrame(const std::shared_ptr<TextFrame>& frame) {
   frame_ = frame;
 }
 
-std::shared_ptr<GlyphAtlas> TextContents::ResolveAtlas(
-    Context& context,
-    GlyphAtlas::Type type,
-    const std::shared_ptr<LazyGlyphAtlas>& lazy_atlas) const {
-  FML_DCHECK(lazy_atlas);
-  if (lazy_atlas) {
-    return lazy_atlas->CreateOrGetGlyphAtlas(context, type);
-  }
-
-  return nullptr;
-}
-
 void TextContents::SetColor(Color color) {
   color_ = color;
 }
@@ -82,8 +70,9 @@ bool TextContents::Render(const ContentContext& renderer,
   }
 
   auto type = frame_->GetAtlasType();
-  auto atlas =
-      ResolveAtlas(*renderer.GetContext(), type, renderer.GetLazyGlyphAtlas());
+  const std::shared_ptr<GlyphAtlas>& atlas =
+      renderer.GetLazyGlyphAtlas()->CreateOrGetGlyphAtlas(
+          *renderer.GetContext(), type);
 
   if (!atlas || !atlas->IsValid()) {
     VALIDATION_LOG << "Cannot render glyphs without prepared atlas.";

impeller/entity/contents/text_contents.h

@@ -5,12 +5,8 @@
 #ifndef FLUTTER_IMPELLER_ENTITY_CONTENTS_TEXT_CONTENTS_H_
 #define FLUTTER_IMPELLER_ENTITY_CONTENTS_TEXT_CONTENTS_H_
 
-#include <functional>
 #include <memory>
-#include <variant>
-#include <vector>
 
-#include "flutter/fml/macros.h"
 #include "impeller/entity/contents/contents.h"
 #include "impeller/geometry/color.h"
 #include "impeller/typographer/glyph_atlas.h"
@@ -70,11 +66,6 @@ class TextContents final : public Contents {
   Vector2 offset_;
   bool force_text_color_ = false;
 
-  std::shared_ptr<GlyphAtlas> ResolveAtlas(
-      Context& context,
-      GlyphAtlas::Type type,
-      const std::shared_ptr<LazyGlyphAtlas>& lazy_atlas) const;
-
   TextContents(const TextContents&) = delete;
 
   TextContents& operator=(const TextContents&) = delete;

impeller/typographer/backends/skia/typographer_context_skia.cc

@@ -313,7 +313,7 @@ static std::shared_ptr<Texture> UploadGlyphTextureAtlas(
 std::shared_ptr<GlyphAtlas> TypographerContextSkia::CreateGlyphAtlas(
     Context& context,
     GlyphAtlas::Type type,
-    std::shared_ptr<GlyphAtlasContext> atlas_context,
+    const std::shared_ptr<GlyphAtlasContext>& atlas_context,
     const FontGlyphMap& font_glyph_map) const {
   TRACE_EVENT0("impeller", __FUNCTION__);
   if (!IsValid()) {

impeller/typographer/backends/skia/typographer_context_skia.h

@@ -5,7 +5,6 @@
 #ifndef FLUTTER_IMPELLER_TYPOGRAPHER_BACKENDS_SKIA_TYPOGRAPHER_CONTEXT_SKIA_H_
 #define FLUTTER_IMPELLER_TYPOGRAPHER_BACKENDS_SKIA_TYPOGRAPHER_CONTEXT_SKIA_H_
 
-#include "flutter/fml/macros.h"
 #include "impeller/typographer/typographer_context.h"
 
 namespace impeller {
@@ -25,7 +24,7 @@ class TypographerContextSkia : public TypographerContext {
   std::shared_ptr<GlyphAtlas> CreateGlyphAtlas(
       Context& context,
       GlyphAtlas::Type type,
-      std::shared_ptr<GlyphAtlasContext> atlas_context,
+      const std::shared_ptr<GlyphAtlasContext>& atlas_context,
       const FontGlyphMap& font_glyph_map) const override;
 
  private:

impeller/typographer/backends/stb/typographer_context_stb.cc

@@ -375,7 +375,7 @@ static std::shared_ptr<Texture> UploadGlyphTextureAtlas(
 std::shared_ptr<GlyphAtlas> TypographerContextSTB::CreateGlyphAtlas(
     Context& context,
     GlyphAtlas::Type type,
-    std::shared_ptr<GlyphAtlasContext> atlas_context,
+    const std::shared_ptr<GlyphAtlasContext>& atlas_context,
     const FontGlyphMap& font_glyph_map) const {
   TRACE_EVENT0("impeller", __FUNCTION__);
   if (!IsValid()) {

impeller/typographer/backends/stb/typographer_context_stb.h

@@ -27,7 +27,7 @@ class TypographerContextSTB : public TypographerContext {
   std::shared_ptr<GlyphAtlas> CreateGlyphAtlas(
       Context& context,
       GlyphAtlas::Type type,
-      std::shared_ptr<GlyphAtlasContext> atlas_context,
+      const std::shared_ptr<GlyphAtlasContext>& atlas_context,
       const FontGlyphMap& font_glyph_map) const override;
 
  private:

impeller/typographer/lazy_glyph_atlas.cc

@@ -4,13 +4,17 @@
 
 #include "impeller/typographer/lazy_glyph_atlas.h"
 
+#include "fml/logging.h"
 #include "impeller/base/validation.h"
+#include "impeller/typographer/glyph_atlas.h"
 #include "impeller/typographer/typographer_context.h"
 
 #include <utility>
 
 namespace impeller {
 
+static const std::shared_ptr<GlyphAtlas> kNullGlyphAtlas = nullptr;
+
 LazyGlyphAtlas::LazyGlyphAtlas(
     std::shared_ptr<TypographerContext> typographer_context)
     : typographer_context_(std::move(typographer_context)),
@@ -24,7 +28,7 @@ LazyGlyphAtlas::LazyGlyphAtlas(
 LazyGlyphAtlas::~LazyGlyphAtlas() = default;
 
 void LazyGlyphAtlas::AddTextFrame(const TextFrame& frame, Scalar scale) {
-  FML_DCHECK(atlas_map_.empty());
+  FML_DCHECK(alpha_atlas_ == nullptr && color_atlas_ == nullptr);
   if (frame.GetAtlasType() == GlyphAtlas::Type::kAlphaBitmap) {
     frame.CollectUniqueFontGlyphPairs(alpha_glyph_map_, scale);
   } else {
@@ -35,42 +39,52 @@ void LazyGlyphAtlas::AddTextFrame(const TextFrame& frame, Scalar scale) {
 void LazyGlyphAtlas::ResetTextFrames() {
   alpha_glyph_map_.clear();
   color_glyph_map_.clear();
-  atlas_map_.clear();
+  alpha_atlas_.reset();
+  color_atlas_.reset();
 }
 
-std::shared_ptr<GlyphAtlas> LazyGlyphAtlas::CreateOrGetGlyphAtlas(
+const std::shared_ptr<GlyphAtlas>& LazyGlyphAtlas::CreateOrGetGlyphAtlas(
     Context& context,
     GlyphAtlas::Type type) const {
   {
-    auto atlas_it = atlas_map_.find(type);
-    if (atlas_it != atlas_map_.end()) {
-      return atlas_it->second;
+    if (type == GlyphAtlas::Type::kAlphaBitmap && alpha_atlas_) {
+      return alpha_atlas_;
+    }
+    if (type == GlyphAtlas::Type::kColorBitmap && color_atlas_) {
+      return color_atlas_;
     }
   }
 
   if (!typographer_context_) {
     VALIDATION_LOG << "Unable to render text because a TypographerContext has "
                       "not been set.";
-    return nullptr;
+    return kNullGlyphAtlas;
   }
   if (!typographer_context_->IsValid()) {
     VALIDATION_LOG
         << "Unable to render text because the TypographerContext is invalid.";
-    return nullptr;
+    return kNullGlyphAtlas;
   }
 
   auto& glyph_map = type == GlyphAtlas::Type::kAlphaBitmap ? alpha_glyph_map_
                                                            : color_glyph_map_;
-  auto atlas_context =
+  const std::shared_ptr<GlyphAtlasContext>& atlas_context =
       type == GlyphAtlas::Type::kAlphaBitmap ? alpha_context_ : color_context_;
-  auto atlas = typographer_context_->CreateGlyphAtlas(context, type,
-                                                      atlas_context, glyph_map);
+  std::shared_ptr<GlyphAtlas> atlas = typographer_context_->CreateGlyphAtlas(
+      context, type, atlas_context, glyph_map);
   if (!atlas || !atlas->IsValid()) {
     VALIDATION_LOG << "Could not create valid atlas.";
-    return nullptr;
+    return kNullGlyphAtlas;
+  }
+  if (type == GlyphAtlas::Type::kAlphaBitmap) {
+    alpha_atlas_ = std::move(atlas);
+    return alpha_atlas_;
+  }
+  if (type == GlyphAtlas::Type::kColorBitmap) {
+    color_atlas_ = std::move(atlas);
+    return color_atlas_;
   }
-  atlas_map_[type] = atlas;
-  return atlas;
+  FML_UNREACHABLE();
 }
 
 }  // namespace impeller

impeller/typographer/lazy_glyph_atlas.h

@@ -26,7 +26,7 @@ class LazyGlyphAtlas {
 
   void ResetTextFrames();
 
-  std::shared_ptr<GlyphAtlas> CreateOrGetGlyphAtlas(
+  const std::shared_ptr<GlyphAtlas>& CreateOrGetGlyphAtlas(
       Context& context,
       GlyphAtlas::Type type) const;
 
@@ -37,8 +37,8 @@ class LazyGlyphAtlas {
   FontGlyphMap color_glyph_map_;
   std::shared_ptr<GlyphAtlasContext> alpha_context_;
   std::shared_ptr<GlyphAtlasContext> color_context_;
-  mutable std::unordered_map<GlyphAtlas::Type, std::shared_ptr<GlyphAtlas>>
-      atlas_map_;
+  mutable std::shared_ptr<GlyphAtlas> alpha_atlas_;
+  mutable std::shared_ptr<GlyphAtlas> color_atlas_;
 
   LazyGlyphAtlas(const LazyGlyphAtlas&) = delete;
 

impeller/typographer/typographer_context.h

@@ -35,7 +35,7 @@ class TypographerContext {
   virtual std::shared_ptr<GlyphAtlas> CreateGlyphAtlas(
       Context& context,
       GlyphAtlas::Type type,
-      std::shared_ptr<GlyphAtlasContext> atlas_context,
+      const std::shared_ptr<GlyphAtlasContext>& atlas_context,
       const FontGlyphMap& font_glyph_map) const = 0;
 
  protected: