[Impeller] Workaround for mismatched transform in preroll vs paint for text frames. (#164931)

Fixes flutter/flutter#164606

When we preroll the text frame, we record the scale/transform used to
compute the size and subpixel position. Use this same size + transform
for the subsequent paint so that it is not possible for it to mismatch.

This is not really a fix for the underlying issue where the subpixel
position may be mismatched.

Author: Jonah Williams

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

@@ -13,9 +13,12 @@
 #include "flutter/fml/build_config.h"
 #include "flutter/impeller/display_list/aiks_unittests.h"
 #include "flutter/testing/testing.h"
+#include "impeller/entity/contents/text_contents.h"
+#include "impeller/entity/entity.h"
 #include "impeller/geometry/matrix.h"
 #include "impeller/typographer/backends/skia/text_frame_skia.h"
 
+#include "impeller/typographer/backends/skia/typographer_context_skia.h"
 #include "txt/platform.h"
 
 using namespace flutter;
@@ -595,5 +598,71 @@ TEST_P(AiksTest, DifferenceClipsMustRenderIdenticallyAcrossBackends) {
   ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
 }
 
+TEST_P(AiksTest, TextContentsMismatchedTransformTest) {
+  AiksContext aiks_context(GetContext(),
+                           std::make_shared<TypographerContextSkia>());
+
+  // Verifies that TextContents only use the scale/transform that is
+  // computed during preroll.
+  constexpr const char* font_fixture = "Roboto-Regular.ttf";
+
+  // Construct the text blob.
+  auto c_font_fixture = std::string(font_fixture);
+  auto mapping = flutter::testing::OpenFixtureAsSkData(c_font_fixture.c_str());
+  ASSERT_TRUE(mapping);
+
+  sk_sp<SkFontMgr> font_mgr = txt::GetDefaultFontManager();
+  SkFont sk_font(font_mgr->makeFromData(mapping), 16);
+
+  auto blob = SkTextBlob::MakeFromString("Hello World", sk_font);
+  ASSERT_TRUE(blob);
+
+  auto text_frame = MakeTextFrameFromTextBlobSkia(blob);
+
+  // Simulate recording the text frame during preroll.
+  Matrix preroll_matrix =
+      Matrix::MakeTranslateScale({1.5, 1.5, 1}, {100, 50, 0});
+  Point preroll_point = Point{23, 45};
+  {
+    auto scale = TextFrame::RoundScaledFontSize(
+        (preroll_matrix * Matrix::MakeTranslation(preroll_point))
+            .GetMaxBasisLengthXY());
+
+    aiks_context.GetContentContext().GetLazyGlyphAtlas()->AddTextFrame(
+        text_frame,     //
+        scale,          //
+        preroll_point,  //
+        preroll_matrix,
+        std::nullopt  //
+    );
+  }
+
+  // Now simulate rendering with a slightly different scale factor.
+  RenderTarget render_target =
+      aiks_context.GetContentContext()
+          .GetRenderTargetCache()
+          ->CreateOffscreenMSAA(*aiks_context.GetContext(), {100, 100}, 1);
+
+  TextContents text_contents;
+  text_contents.SetTextFrame(text_frame);
+  text_contents.SetOffset(preroll_point);
+  text_contents.SetScale(1.6);
+  text_contents.SetColor(Color::Aqua());
+
+  Matrix not_preroll_matrix =
+      Matrix::MakeTranslateScale({1.5, 1.5, 1}, {100, 50, 0});
+
+  Entity entity;
+  entity.SetTransform(not_preroll_matrix);
+
+  std::shared_ptr<CommandBuffer> command_buffer =
+      aiks_context.GetContext()->CreateCommandBuffer();
+  std::shared_ptr<RenderPass> render_pass =
+      command_buffer->CreateRenderPass(render_target);
+
+  EXPECT_TRUE(text_contents.Render(aiks_context.GetContentContext(), entity,
+                                   *render_pass));
+}
+
 }  // namespace testing
 }  // namespace impeller

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

@@ -105,7 +105,8 @@ void TextContents::ComputeVertexData(
   size_t bounds_offset = 0u;
   for (const TextRun& run : frame->GetRuns()) {
     const Font& font = run.GetFont();
-    Scalar rounded_scale = TextFrame::RoundScaledFontSize(scale);
+    Scalar rounded_scale = frame->GetScale();
+    const Matrix transform = frame->GetOffsetTransform();
     FontGlyphAtlas* font_atlas = nullptr;
 
     // Adjust glyph position based on the subpixel rounding
@@ -149,7 +150,7 @@ void TextContents::ComputeVertexData(
           continue;
         }
         Point subpixel = TextFrame::ComputeSubpixelPosition(
-            glyph_position, font.GetAxisAlignment(), entity_transform);
+            glyph_position, font.GetAxisAlignment(), transform);
 
         std::optional<FrameBounds> maybe_atlas_glyph_bounds =
             font_atlas->FindGlyphBounds(SubpixelGlyph{

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

@@ -58,8 +58,10 @@ std::shared_ptr<GlyphAtlas> CreateGlyphAtlas(
     const std::shared_ptr<GlyphAtlasContext>& atlas_context,
     const std::shared_ptr<TextFrame>& frame,
     Point offset) {
-  frame->SetPerFrameData(scale, /*offset=*/offset, /*transform=*/Matrix(),
-                         /*properties=*/std::nullopt);
+  frame->SetPerFrameData(
+      TextFrame::RoundScaledFontSize(scale), /*offset=*/offset,
+      /*transform=*/Matrix::MakeScale(Vector3{scale, scale, 1}),
+      /*properties=*/std::nullopt);
   return typographer_context->CreateGlyphAtlas(context, type, host_buffer,
                                                atlas_context, {frame});
 }
@@ -183,6 +185,7 @@ TEST_P(TextContentsTest, MaintainsShape) {
   std::shared_ptr<HostBuffer> host_buffer = HostBuffer::Create(
       GetContext()->GetResourceAllocator(), GetContext()->GetIdleWaiter());
   ASSERT_TRUE(context && context->IsValid());
+
   for (int i = 0; i <= 1000; ++i) {
     Scalar font_scale = 0.440 + (i / 1000.0);
     Rect position_rect[2];

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

@@ -17,7 +17,6 @@
 #include "impeller/core/host_buffer.h"
 #include "impeller/core/raw_ptr.h"
 #include "impeller/core/texture_descriptor.h"
-#include "impeller/entity/contents/clip_contents.h"
 #include "impeller/entity/contents/conical_gradient_contents.h"
 #include "impeller/entity/contents/content_context.h"
 #include "impeller/entity/contents/contents.h"
@@ -55,10 +54,7 @@
 #include "impeller/renderer/render_target.h"
 #include "impeller/renderer/testing/mocks.h"
 #include "impeller/renderer/vertex_buffer_builder.h"
-#include "impeller/typographer/backends/skia/text_frame_skia.h"
-#include "impeller/typographer/backends/skia/typographer_context_skia.h"
 #include "third_party/imgui/imgui.h"
-#include "third_party/skia/include/core/SkTextBlob.h"
 
 // TODO(zanderso): https://github.com/flutter/flutter/issues/127701
 // NOLINTBEGIN(bugprone-unchecked-optional-access)

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

@@ -459,8 +459,7 @@ TypographerContextSkia::CollectNewGlyphs(
       for (const auto& glyph_position : run.GetGlyphPositions()) {
         Point subpixel = TextFrame::ComputeSubpixelPosition(
             glyph_position, scaled_font.font.GetAxisAlignment(),
-            frame->GetTransform() *
-                Matrix::MakeTranslation(frame->GetOffset()));
+            frame->GetOffsetTransform());
         SubpixelGlyph subpixel_glyph(glyph_position.glyph, subpixel,
                                      frame->GetProperties());
         const auto& font_glyph_bounds =

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

@@ -9,19 +9,6 @@
 
 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)
@@ -97,16 +84,15 @@ Point TextFrame::ComputeSubpixelPosition(
   }
 }
 
+Matrix TextFrame::GetOffsetTransform() const {
+  return transform_ * Matrix::MakeTranslation(offset_);
+}
+
 void TextFrame::SetPerFrameData(Scalar scale,
                                 Point offset,
                                 const Matrix& transform,
                                 std::optional<GlyphProperties> properties) {
-  if (!transform_.Equals(transform) || !ScalarNearlyEqual(scale_, scale) ||
-      !ScalarNearlyEqual(offset_.x, offset.x) ||
-      !ScalarNearlyEqual(offset_.y, offset.y) ||
-      !TextPropertiesEquals(properties_, properties)) {
-    bound_values_.clear();
-  }
+  bound_values_.clear();
   scale_ = scale;
   offset_ = offset;
   properties_ = properties;

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

@@ -91,20 +91,22 @@ class TextFrame {
   // processed.
   std::pair<size_t, intptr_t> GetAtlasGenerationAndID() const;
 
+  Scalar GetScale() const;
+
   TextFrame& operator=(TextFrame&& other) = default;
 
   TextFrame(const TextFrame& other) = default;
 
   const Matrix& GetTransform() const { return transform_; }
 
+  Point GetOffset() const;
+
+  Matrix GetOffsetTransform() const;
+
  private:
   friend class TypographerContextSkia;
   friend class LazyGlyphAtlas;
 
-  Scalar GetScale() const;
-
-  Point GetOffset() const;
-
   std::optional<GlyphProperties> GetProperties() const;
 
   void AppendFrameBounds(const FrameBounds& frame_bounds);