[Impeller] Tiny optimizations for text rendering (avoiding extra ops …

…and copies). (flutter#44822)

…and copies)

While investigating flutter/flutter#131620 I
tried many things to squeeze the logic to make text batching faster. It
didn't pan out but here are a handful of tweaks that make a tiny
difference that I don't want to lose.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or Hixie said the PR is test-exempt. See [testing the engine]
for instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat

impeller/aiks/canvas.cc

@@ -542,7 +542,7 @@ void Canvas::DrawTextFrame(const TextFrame& text_frame,
   entity.SetBlendMode(paint.blend_mode);
 
   auto text_contents = std::make_shared<TextContents>();
-  text_contents->SetTextFrame(text_frame);
+  text_contents->SetTextFrame(TextFrame(text_frame));
 
   if (paint.color_source.GetType() != ColorSource::Type::kColor) {
     auto color_text_contents = std::make_shared<ColorSourceTextContents>();

impeller/entity/contents/text_contents.cc

@@ -23,8 +23,8 @@ TextContents::TextContents() = default;
 
 TextContents::~TextContents() = default;
 
-void TextContents::SetTextFrame(const TextFrame& frame) {
-  frame_ = frame;
+void TextContents::SetTextFrame(TextFrame&& frame) {
+  frame_ = std::move(frame);
 }
 
 std::shared_ptr<GlyphAtlas> TextContents::ResolveAtlas(
@@ -168,22 +168,24 @@ bool TextContents::Render(const ContentContext& renderer,
       vertex_count * sizeof(VS::PerVertexData), alignof(VS::PerVertexData),
       [&](uint8_t* contents) {
         VS::PerVertexData vtx;
-        size_t vertex_offset = 0;
-        for (const auto& run : frame_.GetRuns()) {
+        VS::PerVertexData* vtx_contents =
+            reinterpret_cast<VS::PerVertexData*>(contents);
+        for (const TextRun& run : frame_.GetRuns()) {
           const Font& font = run.GetFont();
-          auto rounded_scale = TextFrame::RoundScaledFontSize(
+          Scalar rounded_scale = TextFrame::RoundScaledFontSize(
               scale_, font.GetMetrics().point_size);
 
-          for (const auto& glyph_position : run.GetGlyphPositions()) {
+          for (const TextRun::GlyphPosition& glyph_position :
+               run.GetGlyphPositions()) {
             FontGlyphPair font_glyph_pair{font, glyph_position.glyph,
                                           rounded_scale};
-            auto maybe_atlas_glyph_bounds =
+            std::optional<Rect> maybe_atlas_glyph_bounds =
                 atlas->FindFontGlyphBounds(font_glyph_pair);
             if (!maybe_atlas_glyph_bounds.has_value()) {
               VALIDATION_LOG << "Could not find glyph position in the atlas.";
               continue;
             }
-            auto atlas_glyph_bounds = maybe_atlas_glyph_bounds.value();
+            const Rect& atlas_glyph_bounds = maybe_atlas_glyph_bounds.value();
             vtx.atlas_glyph_bounds = Vector4(
                 atlas_glyph_bounds.origin.x, atlas_glyph_bounds.origin.y,
                 atlas_glyph_bounds.size.width, atlas_glyph_bounds.size.height);
@@ -193,11 +195,9 @@ bool TextContents::Render(const ContentContext& renderer,
                                        glyph_position.glyph.bounds.size.height);
             vtx.glyph_position = glyph_position.position;
 
-            for (const auto& point : unit_points) {
+            for (const Point& point : unit_points) {
               vtx.unit_position = point;
-              ::memcpy(contents + vertex_offset, &vtx,
-                       sizeof(VS::PerVertexData));
-              vertex_offset += sizeof(VS::PerVertexData);
+              ::memcpy(vtx_contents++, &vtx, sizeof(VS::PerVertexData));
             }
           }
         }

impeller/entity/contents/text_contents.h

@@ -26,7 +26,7 @@ class TextContents final : public Contents {
 
   ~TextContents();
 
-  void SetTextFrame(const TextFrame& frame);
+  void SetTextFrame(TextFrame&& frame);
 
   void SetColor(Color color);
 

impeller/entity/entity_unittests.cc

@@ -2180,7 +2180,7 @@ TEST_P(EntityTest, InheritOpacityTest) {
   lazy_glyph_atlas->AddTextFrame(frame, 1.0f);
 
   auto text_contents = std::make_shared<TextContents>();
-  text_contents->SetTextFrame(frame);
+  text_contents->SetTextFrame(std::move(frame));
   text_contents->SetColor(Color::Blue().WithAlpha(0.5));
 
   ASSERT_TRUE(text_contents->CanInheritOpacity(entity));

impeller/typographer/backends/skia/text_frame_skia.cc

@@ -97,7 +97,7 @@ TextFrame TextFrameFromTextBlob(const sk_sp<SkTextBlob>& blob) {
         FML_DLOG(ERROR) << "Unimplemented.";
         continue;
     }
-    frame.AddTextRun(text_run);
+    frame.AddTextRun(std::move(text_run));
   }
 
   return frame;

impeller/typographer/text_frame.cc

@@ -25,12 +25,12 @@ std::optional<Rect> TextFrame::GetBounds() const {
   return result;
 }
 
-bool TextFrame::AddTextRun(const TextRun& run) {
+bool TextFrame::AddTextRun(TextRun&& run) {
   if (!run.IsValid()) {
     return false;
   }
   has_color_ |= run.HasColor();
-  runs_.emplace_back(run);
+  runs_.emplace_back(std::move(run));
   return true;
 }
 

impeller/typographer/text_frame.h

@@ -48,7 +48,7 @@ class TextFrame {
   ///
   /// @return     If the text run could be added to this frame.
   ///
-  bool AddTextRun(const TextRun& run);
+  bool AddTextRun(TextRun&& run);
 
   //----------------------------------------------------------------------------
   /// @brief      Returns a reference to all the text runs in this frame.
@@ -71,6 +71,10 @@ class TextFrame {
   /// @brief      The type of atlas this run should be emplaced in.
   GlyphAtlas::Type GetAtlasType() const;
 
+  TextFrame& operator=(TextFrame&& other) = default;
+
+  TextFrame(const TextFrame& other) = default;
+
  private:
   std::vector<TextRun> runs_;
   bool has_color_ = false;