[Impeller] Fix text scaling issues again, this time with perspective when a save layer is involved (#43695)

Alternative to #43662

Records the basis vector of the current CTM into text contents and reuses that rather than trying to get it again at render time. That method breaks if perspective is involved in the CTM and a subpass gets translated, which can modify the scale (and rotation) of the matrix.

We're definitely not doing things quite right with perspective here, but the real fix to that is to either record the fully transformed glyph into the atlas or to use SDF/path based rendering.

Fixes flutter/flutter#130476

I still have some concerns about how `EntityPass::Render` is a mix of mutations and constness but we can try to address that independently. 

I expect this to re-improve the regression noted in flutter/flutter#130594

Author: dnfield

impeller/aiks/aiks_unittests.cc

@@ -2877,6 +2877,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,
@@ -2923,5 +2952,41 @@ APPLY_COLOR_FILTER_GRADIENT_TEST(Radial);
 APPLY_COLOR_FILTER_GRADIENT_TEST(Conical);
 APPLY_COLOR_FILTER_GRADIENT_TEST(Sweep);
 
+TEST_P(AiksTest, DrawScaledTextWithPerspectiveNoSaveLayer) {
+  Canvas canvas;
+  // clang-format off
+  canvas.Transform(Matrix(
+       2.000000,       0.000000,   0.000000,  0.000000,
+       1.445767,       2.637070,  -0.507928,  0.001524,
+      -2.451887,      -0.534662,   0.861399, -0.002584,
+    1063.481934,    1025.951416, -48.300270,  1.144901
+  ));
+  // clang-format on
+
+  ASSERT_TRUE(RenderTextInCanvas(GetContext(), canvas, "Hello world",
+                                 "Roboto-Regular.ttf"));
+
+  ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
+}
+
+TEST_P(AiksTest, DrawScaledTextWithPerspectiveSaveLayer) {
+  Canvas canvas;
+  Paint save_paint;
+  canvas.SaveLayer(save_paint);
+  // clang-format off
+  canvas.Transform(Matrix(
+       2.000000,       0.000000,   0.000000,  0.000000,
+       1.445767,       2.637070,  -0.507928,  0.001524,
+      -2.451887,      -0.534662,   0.861399, -0.002584,
+    1063.481934,    1025.951416, -48.300270,  1.144901
+  ));
+  // clang-format on
+
+  ASSERT_TRUE(RenderTextInCanvas(GetContext(), canvas, "Hello world",
+                                 "Roboto-Regular.ttf"));
+
+  ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
+}
+
 }  // namespace testing
 }  // namespace impeller

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,8 +1104,7 @@ void DlDispatcher::drawDisplayList(
 void DlDispatcher::drawTextBlob(const sk_sp<SkTextBlob> blob,
                                 SkScalar x,
                                 SkScalar y) {
-  Scalar scale = canvas_.GetCurrentTransformation().GetMaxBasisLengthXY();
-  const auto text_frame = TextFrameFromTextBlob(blob, scale);
+  const auto text_frame = TextFrameFromTextBlob(blob);
   if (paint_.style == Paint::Style::kStroke) {
     auto path = skia_conversions::PathDataFromTextBlob(blob);
     auto bounds = text_frame.GetBounds();

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) {
+  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) 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 {
 
@@ -53,6 +54,12 @@ class Contents {
 
   virtual ~Contents();
 
+  /// @brief  Add any text data to the specified lazy atlas. The scale parameter
+  ///         must be used again later when drawing the text.
+  virtual void PopulateGlyphAtlas(
+      const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas,
+      Scalar scale) {}
+
   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) {
+  lazy_glyph_atlas->AddTextFrame(frame_, scale);
+  scale_ = 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 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,20 +45,26 @@ 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) override;
+
   // |Contents|
   bool Render(const ContentContext& renderer,
               const Entity& entity,
               RenderPass& pass) const override;
 
  private:
   TextFrame frame_;
+  Scalar scale_ = 1.0;
   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;