[Impeller] cache even more text frame data to skip lookups.

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 =
-      (matrix_ * Matrix::MakeTranslation(Point(x, y))).GetMaxBasisLengthXY();
+  auto scale = TextFrame::RoundScaledFontSize(
+      (matrix_ * Matrix::MakeTranslation(Point(x, y))).GetMaxBasisLengthXY());
+
   renderer_.GetLazyGlyphAtlas()->AddTextFrame(
       text_frame,                                       //
       scale,                                            //

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(
-              scale_, font.GetMetrics().point_size);
+          Scalar rounded_scale = TextFrame::RoundScaledFontSize(scale_);
           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{

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.5321111f, 12), 0.53f);
-  ASSERT_EQ(TextFrame::RoundScaledFontSize(2.1f, 12), 2.1f);
-  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.0f, 12), 0.0f);
-  ASSERT_EQ(TextFrame::RoundScaledFontSize(100000000.0f, 12), 48.0f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.4321111f), 0.43f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.5321111f), 0.53f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(2.1f), 2.1f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.0f), 0.0f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(100000000.0f), 48.0f);
 }
 
 TEST_P(EntityTest, SpecializationConstantsAreAppliedToVariants) {

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),
-    // we could probably guarantee that a text frame that is complete does not
-    // need to be processed unless the scale or properties changed. I'm leaving
-    // this as a future optimization.
+    if (frame->IsFrameComplete() &&
+        frame->GetAtlasGeneration() == generation_id &&
+        !frame->GetFrameBounds(0).is_placeholder) {
+      continue;
+    }
     frame->ClearFrameBounds();
+    frame->SetAtlasGeneration(generation_id);
 
     for (const auto& run : frame->GetRuns()) {
       auto metrics = run.GetFont().GetMetrics();
 
-      auto rounded_scale =
-          TextFrame::RoundScaledFontSize(frame->GetScale(), metrics.point_size);
+      auto rounded_scale = TextFrame::RoundScaledFontSize(frame->GetScale());
       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);

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() &&
-             lhs.properties->stroke == rhs.properties->stroke &&
-             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;
+             GlyphProperties::Equal{}(lhs.properties.value(),
+                                      rhs.properties.value());
     }
   };
 };

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) {

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,

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

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_;
 };