[Impeller] cache even more text frame data to skip lookups. (flutter/engine#56798)

Every text frame must inform the glyph atlas about the glyphs + scale it contains. When this happens, the glyph atlas will populate the glyph and then tell teh text frame about the location in the atlas, so that the text contents shader can sample it correctly.

Once this has been done once for a given text frame + scale + offset, we can actually just keep reusing the same data provided 1) the atlas itself hasn't changed and 2) the scale/offset/properties are the same.

This constitutes a nice CPU usage reduction in places where there is a lot of text that isn't being invalidated, like scrolling on the flutter gallery.

engine/src/flutter/impeller/display_list/dl_dispatcher.cc

@@ -1109,8 +1109,9 @@ void FirstPassDispatcher::drawTextFrame(
     // we do not double-apply the alpha.
     properties.color = paint_.color.WithAlpha(1.0);
   }
-  auto scale =
+  auto scale = TextFrame::RoundScaledFontSize(
-      (matrix_ * Matrix::MakeTranslation(Point(x, y))).GetMaxBasisLengthXY();
+      (matrix_ * Matrix::MakeTranslation(Point(x, y))).GetMaxBasisLengthXY());
+
   renderer_.GetLazyGlyphAtlas()->AddTextFrame(
       text_frame,                                       //
       scale,                                            //

engine/src/flutter/impeller/entity/contents/text_contents.cc

@@ -175,8 +175,7 @@ bool TextContents::Render(const ContentContext& renderer,
         size_t bounds_offset = 0u;
         for (const TextRun& run : frame_->GetRuns()) {
           const Font& font = run.GetFont();
-          Scalar rounded_scale = TextFrame::RoundScaledFontSize(
+          Scalar rounded_scale = TextFrame::RoundScaledFontSize(scale_);
-              scale_, font.GetMetrics().point_size);
           FontGlyphAtlas* font_atlas = nullptr;
 
           // Adjust glyph position based on the subpixel rounding
@@ -220,9 +219,9 @@ bool TextContents::Render(const ContentContext& renderer,
                 VALIDATION_LOG << "Could not find font in the atlas.";
                 continue;
               }
-              // Note: uses unrounded scale for more accurate subpixel position.
               Point subpixel = TextFrame::ComputeSubpixelPosition(
-                  glyph_position, font.GetAxisAlignment(), offset_, scale_);
+                  glyph_position, font.GetAxisAlignment(), offset_,
+                  rounded_scale);
 
               std::optional<FrameBounds> maybe_atlas_glyph_bounds =
                   font_atlas->FindGlyphBounds(SubpixelGlyph{

engine/src/flutter/impeller/entity/entity_unittests.cc

@@ -2120,11 +2120,11 @@ TEST_P(EntityTest, ColorFilterContentsWithLargeGeometry) {
 }
 
 TEST_P(EntityTest, TextContentsCeilsGlyphScaleToDecimal) {
-  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.4321111f, 12), 0.43f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.4321111f), 0.43f);
-  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.5321111f, 12), 0.53f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.5321111f), 0.53f);
-  ASSERT_EQ(TextFrame::RoundScaledFontSize(2.1f, 12), 2.1f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(2.1f), 2.1f);
-  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.0f, 12), 0.0f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.0f), 0.0f);
-  ASSERT_EQ(TextFrame::RoundScaledFontSize(100000000.0f, 12), 48.0f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(100000000.0f), 48.0f);
 }
 
 TEST_P(EntityTest, SpecializationConstantsAreAppliedToVariants) {

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

@@ -413,18 +413,20 @@ TypographerContextSkia::CollectNewGlyphs(
     const std::vector<std::shared_ptr<TextFrame>>& text_frames) {
   std::vector<FontGlyphPair> new_glyphs;
   std::vector<Rect> glyph_sizes;
+  size_t generation_id = atlas->GetAtlasGeneration();
   for (const auto& frame : text_frames) {
-    // TODO(jonahwilliams): unless we destroy the atlas (which we know about),
+    if (frame->IsFrameComplete() &&
-    // we could probably guarantee that a text frame that is complete does not
+        frame->GetAtlasGeneration() == generation_id &&
-    // need to be processed unless the scale or properties changed. I'm leaving
+        !frame->GetFrameBounds(0).is_placeholder) {
-    // this as a future optimization.
+      continue;
+    }
     frame->ClearFrameBounds();
+    frame->SetAtlasGeneration(generation_id);
 
     for (const auto& run : frame->GetRuns()) {
       auto metrics = run.GetFont().GetMetrics();
 
-      auto rounded_scale =
+      auto rounded_scale = TextFrame::RoundScaledFontSize(frame->GetScale());
-          TextFrame::RoundScaledFontSize(frame->GetScale(), metrics.point_size);
       ScaledFont scaled_font{.font = run.GetFont(), .scale = rounded_scale};
 
       FontGlyphAtlas* font_glyph_atlas =
@@ -566,7 +568,8 @@ std::shared_ptr<GlyphAtlas> TypographerContextSkia::CreateGlyphAtlas(
   if (atlas_context->GetAtlasSize().height >= max_texture_height ||
       context.GetBackendType() == Context::BackendType::kOpenGLES) {
     blit_old_atlas = false;
-    new_atlas = std::make_shared<GlyphAtlas>(type);
+    new_atlas = std::make_shared<GlyphAtlas>(
+        type, /*initial_generation=*/last_atlas->GetAtlasGeneration() + 1);
 
     auto [update_glyphs, update_sizes] =
         CollectNewGlyphs(new_atlas, text_frames);

engine/src/flutter/impeller/typographer/font_glyph_pair.h

@@ -20,6 +20,17 @@ struct GlyphProperties {
   Join stroke_join = Join::kMiter;
   Scalar stroke_miter = 4.0;
   bool stroke = false;
+
+  struct Equal {
+    constexpr bool operator()(const impeller::GlyphProperties& lhs,
+                              const impeller::GlyphProperties& rhs) const {
+      return lhs.color.ToARGB() == rhs.color.ToARGB() &&
+             lhs.stroke == rhs.stroke && lhs.stroke_cap == rhs.stroke_cap &&
+             lhs.stroke_join == rhs.stroke_join &&
+             lhs.stroke_miter == rhs.stroke_miter &&
+             lhs.stroke_width == rhs.stroke_width;
+    }
+  };
 };
 
 //------------------------------------------------------------------------------
@@ -85,12 +96,8 @@ struct SubpixelGlyph {
              lhs.glyph.type == rhs.glyph.type &&
              lhs.subpixel_offset == rhs.subpixel_offset &&
              lhs.properties.has_value() && rhs.properties.has_value() &&
-             lhs.properties->color.ToARGB() == rhs.properties->color.ToARGB() &&
+             GlyphProperties::Equal{}(lhs.properties.value(),
-             lhs.properties->stroke == rhs.properties->stroke &&
+                                      rhs.properties.value());
-             lhs.properties->stroke_cap == rhs.properties->stroke_cap &&
-             lhs.properties->stroke_join == rhs.properties->stroke_join &&
-             lhs.properties->stroke_miter == rhs.properties->stroke_miter &&
-             lhs.properties->stroke_width == rhs.properties->stroke_width;
     }
   };
 };

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

@@ -12,7 +12,8 @@
 namespace impeller {
 
 GlyphAtlasContext::GlyphAtlasContext(GlyphAtlas::Type type)
-    : atlas_(std::make_shared<GlyphAtlas>(type)), atlas_size_(ISize(0, 0)) {}
+    : atlas_(std::make_shared<GlyphAtlas>(type, /*initial_generation=*/0)),
+      atlas_size_(ISize(0, 0)) {}
 
 GlyphAtlasContext::~GlyphAtlasContext() {}
 
@@ -45,7 +46,8 @@ void GlyphAtlasContext::UpdateRectPacker(
   rect_packer_ = std::move(rect_packer);
 }
 
-GlyphAtlas::GlyphAtlas(Type type) : type_(type) {}
+GlyphAtlas::GlyphAtlas(Type type, size_t initial_generation)
+    : type_(type), generation_(initial_generation) {}
 
 GlyphAtlas::~GlyphAtlas() = default;
 
@@ -65,6 +67,14 @@ void GlyphAtlas::SetTexture(std::shared_ptr<Texture> texture) {
   texture_ = std::move(texture);
 }
 
+size_t GlyphAtlas::GetAtlasGeneration() const {
+  return generation_;
+}
+
+void GlyphAtlas::SetAtlasGeneration(size_t generation) {
+  generation_ = generation;
+}
+
 void GlyphAtlas::AddTypefaceGlyphPositionAndBounds(const FontGlyphPair& pair,
                                                    Rect position,
                                                    Rect bounds) {

engine/src/flutter/impeller/typographer/glyph_atlas.h

@@ -58,8 +58,9 @@ class GlyphAtlas {
   /// @brief      Create an empty glyph atlas.
   ///
   /// @param[in]  type  How the glyphs are represented in the texture.
+  /// @param[in]  initial_generation the atlas generation.
   ///
-  explicit GlyphAtlas(Type type);
+  GlyphAtlas(Type type, size_t initial_generation);
 
   ~GlyphAtlas();
 
@@ -142,9 +143,22 @@ class GlyphAtlas {
   ///
   FontGlyphAtlas* GetOrCreateFontGlyphAtlas(const ScaledFont& scaled_font);
 
+  //----------------------------------------------------------------------------
+  /// @brief      Retrieve the generation id for this glyph atlas.
+  ///
+  ///             The generation id is used to match with a TextFrame to
+  ///             determine if the frame is guaranteed to already be populated
+  ///             in the atlas.
+  size_t GetAtlasGeneration() const;
+
+  //----------------------------------------------------------------------------
+  /// @brief      Update the atlas generation.
+  void SetAtlasGeneration(size_t value);
+
  private:
   const Type type_;
   std::shared_ptr<Texture> texture_;
+  size_t generation_ = 0;
 
   std::unordered_map<ScaledFont,
                      FontGlyphAtlas,

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

@@ -3,11 +3,25 @@
 // found in the LICENSE file.
 
 #include "impeller/typographer/text_frame.h"
+#include "impeller/geometry/scalar.h"
 #include "impeller/typographer/font.h"
 #include "impeller/typographer/font_glyph_pair.h"
 
 namespace impeller {
 
+namespace {
+static bool TextPropertiesEquals(const std::optional<GlyphProperties>& a,
+                                 const std::optional<GlyphProperties>& b) {
+  if (!a.has_value() && !b.has_value()) {
+    return true;
+  }
+  if (a.has_value() && b.has_value()) {
+    return GlyphProperties::Equal{}(a.value(), b.value());
+  }
+  return false;
+}
+}  // namespace
+
 TextFrame::TextFrame() = default;
 
 TextFrame::TextFrame(std::vector<TextRun>& runs, Rect bounds, bool has_color)
@@ -37,7 +51,7 @@ bool TextFrame::HasColor() const {
 }
 
 // static
-Scalar TextFrame::RoundScaledFontSize(Scalar scale, Scalar point_size) {
+Scalar TextFrame::RoundScaledFontSize(Scalar scale) {
   // An arbitrarily chosen maximum text scale to ensure that regardless of the
   // CTM, a glyph will fit in the atlas. If we clamp significantly, this may
   // reduce fidelity but is preferable to the alternative of failing to render.
@@ -87,6 +101,12 @@ Point TextFrame::ComputeSubpixelPosition(
 void TextFrame::SetPerFrameData(Scalar scale,
                                 Point offset,
                                 std::optional<GlyphProperties> properties) {
+  if (!ScalarNearlyEqual(scale_, scale) ||
+      !ScalarNearlyEqual(offset_.x, offset.x) ||
+      !ScalarNearlyEqual(offset_.y, offset.y) ||
+      !TextPropertiesEquals(properties_, properties)) {
+    bound_values_.clear();
+  }
   scale_ = scale;
   offset_ = offset;
   properties_ = properties;
@@ -121,7 +141,15 @@ bool TextFrame::IsFrameComplete() const {
 }
 
 const FrameBounds& TextFrame::GetFrameBounds(size_t index) const {
-  return bound_values_.at(index);
+  return bound_values_[index];
+}
+
+size_t TextFrame::GetAtlasGeneration() const {
+  return generation_;
+}
+
+void TextFrame::SetAtlasGeneration(size_t value) {
+  generation_ = value;
 }
 
 }  // namespace impeller

engine/src/flutter/impeller/typographer/text_frame.h

@@ -32,7 +32,7 @@ class TextFrame {
       Point offset,
       Scalar scale);
 
-  static Scalar RoundScaledFontSize(Scalar scale, Scalar point_size);
+  static Scalar RoundScaledFontSize(Scalar scale);
 
   //----------------------------------------------------------------------------
   /// @brief      The conservative bounding box for this text frame.
@@ -84,13 +84,18 @@ class TextFrame {
                        Point offset,
                        std::optional<GlyphProperties> properties);
 
+  // A generation id for the glyph atlas this text run was associated
+  // with. As long as the frame generation matches the atlas generation,
+  // the contents are guaranteed to be populated and do not need to be
+  // processed.
+  size_t GetAtlasGeneration() const;
+
   TextFrame& operator=(TextFrame&& other) = default;
 
   TextFrame(const TextFrame& other) = default;
 
  private:
   friend class TypographerContextSkia;
-  friend class TypographerContextSTB;
   friend class LazyGlyphAtlas;
 
   Scalar GetScale() const;
@@ -103,14 +108,17 @@ class TextFrame {
 
   void ClearFrameBounds();
 
+  void SetAtlasGeneration(size_t value);
+
   std::vector<TextRun> runs_;
   Rect bounds_;
   bool has_color_;
 
   // Data that is cached when rendering the text frame and is only
-  // valid for a single frame.
+  // valid for the current atlas generation.
   std::vector<FrameBounds> bound_values_;
   Scalar scale_ = 0;
+  size_t generation_ = 0;
   Point offset_;
   std::optional<GlyphProperties> properties_;
 };

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

@@ -508,7 +508,7 @@ TEST_P(TypographerTest, TextFrameInitialBoundsArePlaceholder) {
                                         GetContext()->GetIdleWaiter());
 
   auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                                GlyphAtlas::Type::kAlphaBitmap, 1.0f,
+                                GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
                                 atlas_context, frame);
 
   // The glyph position in the atlas was not known when this value
@@ -517,14 +517,101 @@ TEST_P(TypographerTest, TextFrameInitialBoundsArePlaceholder) {
   EXPECT_TRUE(frame->GetFrameBounds(0).is_placeholder);
 
   atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                           GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context,
+                           GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
-                           frame);
+                           atlas_context, frame);
 
   // The second time the glyph is rendered, the bounds are correcly known.
   EXPECT_TRUE(frame->IsFrameComplete());
   EXPECT_FALSE(frame->GetFrameBounds(0).is_placeholder);
 }
 
+TEST_P(TypographerTest, TextFrameInvalidationWithScale) {
+  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);
+
+  // Change the scale and the glyph data will still be a placeholder, as the
+  // old data is no longer valid.
+  atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
+                           GlyphAtlas::Type::kAlphaBitmap, /*scale=*/2.0f,
+                           atlas_context, frame);
+
+  // The second time the glyph is rendered, the bounds are correcly known.
+  EXPECT_TRUE(frame->IsFrameComplete());
+  EXPECT_TRUE(frame->GetFrameBounds(0).is_placeholder);
+}
+
+TEST_P(TypographerTest, TextFrameAtlasGenerationTracksState) {
+  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);
+  }
+
+  atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
+                           GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                           atlas_context, frame);
+
+  // The second time the glyph is rendered, the bounds are correcly known.
+  EXPECT_TRUE(frame->IsFrameComplete());
+  EXPECT_FALSE(frame->GetFrameBounds(0).is_placeholder);
+  if (GetBackend() == PlaygroundBackend::kOpenGLES) {
+    EXPECT_EQ(frame->GetAtlasGeneration(), 1u);
+  } else {
+    EXPECT_EQ(frame->GetAtlasGeneration(), 0u);
+  }
+
+  // Force increase the generation.
+  atlas_context->GetGlyphAtlas()->SetAtlasGeneration(2u);
+  atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
+                           GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                           atlas_context, frame);
+
+  EXPECT_EQ(frame->GetAtlasGeneration(), 2u);
+}
+
 }  // namespace testing
 }  // namespace impeller