From c35d8ffbd6664fa9fba9ad6d9910ac47312ad34c Mon Sep 17 00:00:00 2001
From: Jonah Williams <jonahwilliams@google.com>
Date: Tue, 29 Aug 2023 18:30:59 -0700
Subject: [PATCH] [Impeller] transform text path offsets so color sources match
 expected offsets for gradients. (#45255)

Gradient offsets are computed based on path offsets + gradient positions, so we need to shift the points instead of just adding a concat to the canvas. Fixes https://github.com/flutter/flutter/issues/132972

### Skia

![flutter_03](https://github.com/flutter/engine/assets/8975114/182dff72-cd48-4f97-a1e3-ea924f4a0eee)

### Impeller (No Patch)

![flutter_04](https://github.com/flutter/engine/assets/8975114/bb421d86-a588-49b9-879f-209738821898)

### Impeller (W/ Patch)

![flutter_05](https://github.com/flutter/engine/assets/8975114/f2d601b4-d15f-487b-aa65-a61bfc98bde9)
---
 impeller/display_list/dl_dispatcher.cc  |  6 ++---
 impeller/geometry/geometry_unittests.cc | 36 +++++++++++++++++++++++++
 impeller/geometry/path.cc               | 27 +++++++++++++++++++
 impeller/geometry/path.h                |  3 +++
 4 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc
index 01f9b2766c1f0..545d26c69dc7d 100644
--- a/impeller/display_list/dl_dispatcher.cc
+++ b/impeller/display_list/dl_dispatcher.cc
@@ -1118,12 +1118,10 @@ void DlDispatcher::drawTextBlob(const sk_sp<SkTextBlob>& blob,
   const auto text_frame = maybe_text_frame.value();
   if (paint_.style == Paint::Style::kStroke ||
       paint_.color_source.GetType() != ColorSource::Type::kColor) {
-    auto path = skia_conversions::PathDataFromTextBlob(blob);
     auto bounds = blob->bounds();
-    canvas_.Save();
-    canvas_.Translate({x + bounds.left(), y + bounds.top(), 0.0});
+    auto path = skia_conversions::PathDataFromTextBlob(blob);
+    path.Shift(Point(x + bounds.left(), y + bounds.top()));
     canvas_.DrawPath(path, paint_);
-    canvas_.Restore();
     return;
   }
 
diff --git a/impeller/geometry/geometry_unittests.cc b/impeller/geometry/geometry_unittests.cc
index 2b9875bf67bd0..1a43f65cba1e5 100644
--- a/impeller/geometry/geometry_unittests.cc
+++ b/impeller/geometry/geometry_unittests.cc
@@ -2373,6 +2373,42 @@ TEST(GeometryTest, HalfConversions) {
 #endif  // FML_OS_WIN
 }
 
+TEST(GeometryTest, PathShifting) {
+  PathBuilder builder{};
+  auto path =
+      builder.AddLine(Point(0, 0), Point(10, 10))
+          .AddQuadraticCurve(Point(10, 10), Point(15, 15), Point(20, 20))
+          .AddCubicCurve(Point(20, 20), Point(25, 25), Point(-5, -5),
+                         Point(30, 30))
+          .Close()
+          .TakePath();
+  path.Shift(Point(1, 1));
+
+  ContourComponent contour;
+  LinearPathComponent linear;
+  QuadraticPathComponent quad;
+  CubicPathComponent cubic;
+
+  ASSERT_TRUE(path.GetContourComponentAtIndex(0, contour));
+  ASSERT_TRUE(path.GetLinearComponentAtIndex(1, linear));
+  ASSERT_TRUE(path.GetQuadraticComponentAtIndex(3, quad));
+  ASSERT_TRUE(path.GetCubicComponentAtIndex(5, cubic));
+
+  ASSERT_EQ(contour.destination, Point(1, 1));
+
+  ASSERT_EQ(linear.p1, Point(1, 1));
+  ASSERT_EQ(linear.p2, Point(11, 11));
+
+  ASSERT_EQ(quad.cp, Point(16, 16));
+  ASSERT_EQ(quad.p1, Point(11, 11));
+  ASSERT_EQ(quad.p2, Point(21, 21));
+
+  ASSERT_EQ(cubic.cp1, Point(26, 26));
+  ASSERT_EQ(cubic.cp2, Point(-4, -4));
+  ASSERT_EQ(cubic.p1, Point(21, 21));
+  ASSERT_EQ(cubic.p2, Point(31, 31));
+}
+
 }  // namespace testing
 }  // namespace impeller
 
diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc
index 1f677294f89df..2901c53c1c9e2 100644
--- a/impeller/geometry/path.cc
+++ b/impeller/geometry/path.cc
@@ -61,6 +61,33 @@ void Path::SetConvexity(Convexity value) {
   convexity_ = value;
 }
 
+void Path::Shift(Point shift) {
+  size_t currentIndex = 0;
+  for (const auto& component : components_) {
+    switch (component.type) {
+      case ComponentType::kLinear:
+        linears_[component.index].p1 += shift;
+        linears_[component.index].p2 += shift;
+        break;
+      case ComponentType::kQuadratic:
+        quads_[component.index].cp += shift;
+        quads_[component.index].p1 += shift;
+        quads_[component.index].p2 += shift;
+        break;
+      case ComponentType::kCubic:
+        cubics_[component.index].cp1 += shift;
+        cubics_[component.index].cp2 += shift;
+        cubics_[component.index].p1 += shift;
+        cubics_[component.index].p2 += shift;
+        break;
+      case ComponentType::kContour:
+        contours_[component.index].destination += shift;
+        break;
+    }
+    currentIndex++;
+  }
+}
+
 Path& Path::AddLinearComponent(Point p1, Point p2) {
   linears_.emplace_back(p1, p2);
   components_.emplace_back(ComponentType::kLinear, linears_.size() - 1);
diff --git a/impeller/geometry/path.h b/impeller/geometry/path.h
index 78a6e5f6647e2..dbed30bfae7d1 100644
--- a/impeller/geometry/path.h
+++ b/impeller/geometry/path.h
@@ -111,6 +111,9 @@ class Path {
 
   void SetContourClosed(bool is_closed);
 
+  /// @brief Transform the path by the given offset in-place.
+  void Shift(Point shift);
+
   template <class T>
   using Applier = std::function<void(size_t index, const T& component)>;
   void EnumerateComponents(