[Impeller] Improve resolution of text scaling. (#43533)

This patch does a few things:

- Changes ownership of the LazyGlyphAtlas to ContentContext from Canvas. This means that drawings with multiple canvases (e.g. calls to DrawPicture) share the same lazy atlas.
- Moves the scale property from Font::Metrics to FontGlyphPair. Font::Metrics contains properties related to the font, whereas scale has to do with the properties of the drawing at render time and is independent of the font.
- Makes the lazy atlas manage FontGlyphPair::Set rather than vectors of TextFrames.
- Makes the determination of font scaling at EntityPass::Render rather than Canvas::DrawTextFrame. The scaling may be altered by calls to Canvas::DrawPicture by render time and otherwise get missed. This is done via a new method: Contents::PopulateGlyphAtlas, which is only implemented by TextContents in this patch.
- Fixes up some miscelleaneous bugs in Font::Metrics hashing and unused/dead code.

This improves over prior attempts: LazyGlyphAtlas ownership is now unambiguous, and glyph scaling happens correctly for all rendered glyphs regardless of the order of canvas operations in Aiks.

Fixes flutter/flutter#130142

Author: dnfield

impeller/aiks/aiks_unittests.cc

@@ -2851,6 +2851,35 @@ TEST_P(AiksTest, CanCanvasDrawPicture) {
   ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
 }
 
+TEST_P(AiksTest, DrawPictureWithText) {
+  Canvas subcanvas;
+  ASSERT_TRUE(RenderTextInCanvas(
+      GetContext(), subcanvas,
+      "the quick brown fox jumped over the lazy dog!.?", "Roboto-Regular.ttf"));
+  subcanvas.Translate({0, 10});
+  subcanvas.Scale(Vector2(3, 3));
+  ASSERT_TRUE(RenderTextInCanvas(
+      GetContext(), subcanvas,
+      "the quick brown fox jumped over the very big lazy dog!.?",
+      "Roboto-Regular.ttf"));
+  auto picture = subcanvas.EndRecordingAsPicture();
+
+  Canvas canvas;
+  canvas.Scale(Vector2(.2, .2));
+  canvas.Save();
+  canvas.Translate({200, 200});
+  canvas.Scale(Vector2(3.5, 3.5));  // The text must not be blurry after this.
+  canvas.DrawPicture(picture);
+  canvas.Restore();
+
+  canvas.Scale(Vector2(1.5, 1.5));
+  ASSERT_TRUE(RenderTextInCanvas(
+      GetContext(), canvas,
+      "the quick brown fox jumped over the smaller lazy dog!.?",
+      "Roboto-Regular.ttf"));
+  ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
+}
+
 TEST_P(AiksTest, MatrixBackdropFilter) {
   Canvas canvas;
   canvas.SaveLayer({}, std::nullopt,

impeller/aiks/canvas.cc

@@ -42,7 +42,6 @@ void Canvas::Initialize(std::optional<Rect> cull_rect) {
   base_pass_ = std::make_unique<EntityPass>();
   current_pass_ = base_pass_.get();
   xformation_stack_.emplace_back(CanvasStackEntry{.cull_rect = cull_rect});
-  lazy_glyph_atlas_ = std::make_shared<LazyGlyphAtlas>();
   FML_DCHECK(GetSaveCount() == 1u);
   FML_DCHECK(base_pass_->GetSubpassesDepth() == 1u);
 }
@@ -51,7 +50,6 @@ void Canvas::Reset() {
   base_pass_ = nullptr;
   current_pass_ = nullptr;
   xformation_stack_ = {};
-  lazy_glyph_atlas_ = nullptr;
 }
 
 void Canvas::Save() {
@@ -516,15 +514,12 @@ void Canvas::SaveLayer(const Paint& paint,
 void Canvas::DrawTextFrame(const TextFrame& text_frame,
                            Point position,
                            const Paint& paint) {
-  lazy_glyph_atlas_->AddTextFrame(text_frame);
-
   Entity entity;
   entity.SetStencilDepth(GetStencilDepth());
   entity.SetBlendMode(paint.blend_mode);
 
   auto text_contents = std::make_shared<TextContents>();
   text_contents->SetTextFrame(text_frame);
-  text_contents->SetGlyphAtlas(lazy_glyph_atlas_);
 
   if (paint.color_source.GetType() != ColorSource::Type::kColor) {
     auto color_text_contents = std::make_shared<ColorSourceTextContents>();

impeller/aiks/canvas.h

@@ -162,7 +162,6 @@ class Canvas {
   std::unique_ptr<EntityPass> base_pass_;
   EntityPass* current_pass_ = nullptr;
   std::deque<CanvasStackEntry> xformation_stack_;
-  std::shared_ptr<LazyGlyphAtlas> lazy_glyph_atlas_;
   std::optional<Rect> initial_cull_rect_;
 
   void Initialize(std::optional<Rect> cull_rect);

impeller/display_list/dl_dispatcher.cc

@@ -1104,10 +1104,9 @@ void DlDispatcher::drawDisplayList(
 void DlDispatcher::drawTextBlob(const sk_sp<SkTextBlob> blob,
                                 SkScalar x,
                                 SkScalar y) {
-  Scalar scale = canvas_.GetCurrentTransformation().GetMaxBasisLengthXY();
-  canvas_.DrawTextFrame(TextFrameFromTextBlob(blob, scale),  //
-                        impeller::Point{x, y},               //
-                        paint_                               //
+  canvas_.DrawTextFrame(TextFrameFromTextBlob(blob),  //
+                        impeller::Point{x, y},        //
+                        paint_                        //
   );
 }
 

impeller/entity/contents/color_source_text_contents.cc

@@ -4,6 +4,7 @@
 
 #include "impeller/entity/contents/color_source_text_contents.h"
 
+#include "color_source_text_contents.h"
 #include "impeller/entity/contents/content_context.h"
 #include "impeller/entity/contents/texture_contents.h"
 #include "impeller/renderer/render_pass.h"
@@ -33,6 +34,12 @@ void ColorSourceTextContents::SetTextPosition(Point position) {
   position_ = position;
 }
 
+void ColorSourceTextContents::PopulateGlyphAtlas(
+    const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas,
+    Scalar scale) const {
+  text_contents_->PopulateGlyphAtlas(lazy_glyph_atlas, scale);
+}
+
 bool ColorSourceTextContents::Render(const ContentContext& renderer,
                                      const Entity& entity,
                                      RenderPass& pass) const {

impeller/entity/contents/color_source_text_contents.h

@@ -32,6 +32,11 @@ class ColorSourceTextContents final : public Contents {
   // |Contents|
   std::optional<Rect> GetCoverage(const Entity& entity) const override;
 
+  // |Contents|
+  void PopulateGlyphAtlas(
+      const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas,
+      Scalar scale) const override;
+
   // |Contents|
   bool Render(const ContentContext& renderer,
               const Entity& entity,

impeller/entity/contents/content_context.h

@@ -696,8 +696,18 @@ class ContentContext {
                                        const SubpassCallback& subpass_callback,
                                        bool msaa_enabled = true) const;
 
+  void SetLazyGlyphAtlas(
+      const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas) {
+    lazy_glyph_atlas_ = lazy_glyph_atlas;
+  }
+
+  std::shared_ptr<LazyGlyphAtlas> GetLazyGlyphAtlas() const {
+    return lazy_glyph_atlas_;
+  }
+
  private:
   std::shared_ptr<Context> context_;
+  std::shared_ptr<LazyGlyphAtlas> lazy_glyph_atlas_;
 
   template <class T>
   using Variants = std::unordered_map<ContentContextOptions,

impeller/entity/contents/contents.h

@@ -14,6 +14,7 @@
 #include "impeller/geometry/color.h"
 #include "impeller/geometry/rect.h"
 #include "impeller/renderer/snapshot.h"
+#include "impeller/typographer/lazy_glyph_atlas.h"
 
 namespace impeller {
 
@@ -49,6 +50,11 @@ class Contents {
 
   virtual ~Contents();
 
+  /// @brief  Add any text data to the specified lazy atlas.
+  virtual void PopulateGlyphAtlas(
+      const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas,
+      Scalar scale) const {}
+
   virtual bool Render(const ContentContext& renderer,
                       const Entity& entity,
                       RenderPass& pass) const = 0;

impeller/entity/contents/text_contents.cc

@@ -29,18 +29,15 @@ void TextContents::SetTextFrame(const TextFrame& frame) {
   frame_ = frame;
 }
 
-void TextContents::SetGlyphAtlas(std::shared_ptr<LazyGlyphAtlas> atlas) {
-  lazy_atlas_ = std::move(atlas);
-}
-
 std::shared_ptr<GlyphAtlas> TextContents::ResolveAtlas(
     GlyphAtlas::Type type,
+    const std::shared_ptr<LazyGlyphAtlas>& lazy_atlas,
     std::shared_ptr<GlyphAtlasContext> atlas_context,
     std::shared_ptr<Context> context) const {
-  FML_DCHECK(lazy_atlas_);
-  if (lazy_atlas_) {
-    return lazy_atlas_->CreateOrGetGlyphAtlas(type, std::move(atlas_context),
-                                              std::move(context));
+  FML_DCHECK(lazy_atlas);
+  if (lazy_atlas) {
+    return lazy_atlas->CreateOrGetGlyphAtlas(type, std::move(atlas_context),
+                                             std::move(context));
   }
 
   return nullptr;
@@ -78,14 +75,43 @@ std::optional<Rect> TextContents::GetCoverage(const Entity& entity) const {
   return bounds->TransformBounds(entity.GetTransformation());
 }
 
-static bool CommonRender(const ContentContext& renderer,
-                         const Entity& entity,
-                         RenderPass& pass,
-                         const Color& color,
-                         const TextFrame& frame,
-                         Vector2 offset,
-                         const std::shared_ptr<GlyphAtlas>& atlas,
-                         Command& cmd) {
+void TextContents::PopulateGlyphAtlas(
+    const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas,
+    Scalar scale) const {
+  lazy_glyph_atlas->AddTextFrame(frame_, scale);
+}
+
+bool TextContents::Render(const ContentContext& renderer,
+                          const Entity& entity,
+                          RenderPass& pass) const {
+  auto color = GetColor();
+  if (color.IsTransparent()) {
+    return true;
+  }
+
+  auto type = frame_.GetAtlasType();
+  auto scale = entity.DeriveTextScale();
+  auto atlas =
+      ResolveAtlas(type, renderer.GetLazyGlyphAtlas(),
+                   renderer.GetGlyphAtlasContext(type), renderer.GetContext());
+
+  if (!atlas || !atlas->IsValid()) {
+    VALIDATION_LOG << "Cannot render glyphs without prepared atlas.";
+    return false;
+  }
+
+  // Information shared by all glyph draw calls.
+  Command cmd;
+  cmd.label = "TextFrame";
+  auto opts = OptionsFromPassAndEntity(pass, entity);
+  opts.primitive_type = PrimitiveType::kTriangle;
+  if (type == GlyphAtlas::Type::kAlphaBitmap) {
+    cmd.pipeline = renderer.GetGlyphAtlasPipeline(opts);
+  } else {
+    cmd.pipeline = renderer.GetGlyphAtlasColorPipeline(opts);
+  }
+  cmd.stencil_reference = entity.GetStencilDepth();
+
   using VS = GlyphAtlasPipeline::VertexShader;
   using FS = GlyphAtlasPipeline::FragmentShader;
 
@@ -95,7 +121,7 @@ static bool CommonRender(const ContentContext& renderer,
   frame_info.atlas_size =
       Vector2{static_cast<Scalar>(atlas->GetTexture()->GetSize().width),
               static_cast<Scalar>(atlas->GetTexture()->GetSize().height)};
-  frame_info.offset = offset;
+  frame_info.offset = offset_;
   frame_info.is_translation_scale =
       entity.GetTransformation().IsTranslationScaleOnly();
   frame_info.entity_transform = entity.GetTransformation();
@@ -141,7 +167,7 @@ static bool CommonRender(const ContentContext& renderer,
 
   auto& host_buffer = pass.GetTransientsBuffer();
   size_t vertex_count = 0;
-  for (const auto& run : frame.GetRuns()) {
+  for (const auto& run : frame_.GetRuns()) {
     vertex_count += run.GetGlyphPositions().size();
   }
   vertex_count *= 6;
@@ -151,10 +177,10 @@ static bool CommonRender(const ContentContext& renderer,
       [&](uint8_t* contents) {
         VS::PerVertexData vtx;
         size_t vertex_offset = 0;
-        for (const auto& run : frame.GetRuns()) {
+        for (const auto& run : frame_.GetRuns()) {
           const Font& font = run.GetFont();
           for (const auto& glyph_position : run.GetGlyphPositions()) {
-            FontGlyphPair font_glyph_pair{font, glyph_position.glyph};
+            FontGlyphPair font_glyph_pair{font, glyph_position.glyph, scale};
             auto maybe_atlas_glyph_bounds =
                 atlas->FindFontGlyphBounds(font_glyph_pair);
             if (!maybe_atlas_glyph_bounds.has_value()) {
@@ -191,37 +217,4 @@ static bool CommonRender(const ContentContext& renderer,
   return pass.AddCommand(cmd);
 }
 
-bool TextContents::Render(const ContentContext& renderer,
-                          const Entity& entity,
-                          RenderPass& pass) const {
-  auto color = GetColor();
-  if (color.IsTransparent()) {
-    return true;
-  }
-
-  auto type = frame_.GetAtlasType();
-  auto atlas = ResolveAtlas(type, renderer.GetGlyphAtlasContext(type),
-                            renderer.GetContext());
-
-  if (!atlas || !atlas->IsValid()) {
-    VALIDATION_LOG << "Cannot render glyphs without prepared atlas.";
-    return false;
-  }
-
-  // Information shared by all glyph draw calls.
-  Command cmd;
-  cmd.label = "TextFrame";
-  auto opts = OptionsFromPassAndEntity(pass, entity);
-  opts.primitive_type = PrimitiveType::kTriangle;
-  if (type == GlyphAtlas::Type::kAlphaBitmap) {
-    cmd.pipeline = renderer.GetGlyphAtlasPipeline(opts);
-  } else {
-    cmd.pipeline = renderer.GetGlyphAtlasColorPipeline(opts);
-  }
-  cmd.stencil_reference = entity.GetStencilDepth();
-
-  return CommonRender(renderer, entity, pass, color, frame_, offset_, atlas,
-                      cmd);
-}
-
 }  // namespace impeller

impeller/entity/contents/text_contents.h

@@ -28,14 +28,14 @@ class TextContents final : public Contents {
 
   void SetTextFrame(const TextFrame& frame);
 
-  void SetGlyphAtlas(std::shared_ptr<LazyGlyphAtlas> atlas);
-
   void SetColor(Color color);
 
   Color GetColor() const;
 
+  // |Contents|
   bool CanInheritOpacity(const Entity& entity) const override;
 
+  // |Contents|
   void SetInheritedOpacity(Scalar opacity) override;
 
   void SetOffset(Vector2 offset);
@@ -45,6 +45,11 @@ class TextContents final : public Contents {
   // |Contents|
   std::optional<Rect> GetCoverage(const Entity& entity) const override;
 
+  // |Contents|
+  void PopulateGlyphAtlas(
+      const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas,
+      Scalar scale) const override;
+
   // |Contents|
   bool Render(const ContentContext& renderer,
               const Entity& entity,
@@ -54,11 +59,11 @@ class TextContents final : public Contents {
   TextFrame frame_;
   Color color_;
   Scalar inherited_opacity_ = 1.0;
-  mutable std::shared_ptr<LazyGlyphAtlas> lazy_atlas_;
   Vector2 offset_;
 
   std::shared_ptr<GlyphAtlas> ResolveAtlas(
       GlyphAtlas::Type type,
+      const std::shared_ptr<LazyGlyphAtlas>& lazy_atlas,
       std::shared_ptr<GlyphAtlasContext> atlas_context,
       std::shared_ptr<Context> context) const;
 

impeller/entity/entity.cc

@@ -166,4 +166,8 @@ bool Entity::Render(const ContentContext& renderer,
   return contents_->Render(renderer, *this, parent_pass);
 }
 
+Scalar Entity::DeriveTextScale() const {
+  return GetTransformation().GetMaxBasisLengthXY();
+}
+
 }  // namespace impeller

impeller/entity/entity.h

@@ -94,6 +94,8 @@ class Entity {
 
   std::optional<Color> AsBackgroundColor(ISize target_size) const;
 
+  Scalar DeriveTextScale() const;
+
  private:
   Matrix transformation_;
   std::shared_ptr<Contents> contents_;