From fcaa8067b6a6d2bbcff8fe1305562e7f71c2c7cf Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 29 Aug 2023 16:29:53 -0700 Subject: [PATCH 1/3] [Impeller] transform text path offsets so color sources match expected positions. --- impeller/aiks/color_source.cc | 4 ++++ impeller/aiks/color_source.h | 2 ++ impeller/display_list/dl_dispatcher.cc | 9 +++++---- impeller/geometry/path.cc | 28 ++++++++++++++++++++++++++ impeller/geometry/path.h | 2 ++ 5 files changed, 41 insertions(+), 4 deletions(-) diff --git a/impeller/aiks/color_source.cc b/impeller/aiks/color_source.cc index 3329bcb4b832e..f04e8019194e0 100644 --- a/impeller/aiks/color_source.cc +++ b/impeller/aiks/color_source.cc @@ -230,6 +230,10 @@ ColorSource ColorSource::MakeScene(std::shared_ptr scene_node, } #endif // IMPELLER_ENABLE_3D +void ColorSource::ConcatEffectTransform(const Matrix& effect_transform) { + +} + ColorSource::Type ColorSource::GetType() const { return type_; } diff --git a/impeller/aiks/color_source.h b/impeller/aiks/color_source.h index 7849b841ec784..6a4a9ba737d96 100644 --- a/impeller/aiks/color_source.h +++ b/impeller/aiks/color_source.h @@ -97,6 +97,8 @@ class ColorSource { std::shared_ptr GetContents(const Paint& paint) const; + void ConcatEffectTransform(const Matrix& effect_transform); + private: Type type_ = Type::kColor; ColorSourceProc proc_; diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index 01f9b2766c1f0..68b9279e54324 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/impeller/display_list/dl_dispatcher.cc @@ -1118,12 +1118,13 @@ void DlDispatcher::drawTextBlob(const sk_sp& 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.ShiftPath(Point(x + bounds.left(), y + bounds.top())); + // canvas_.Save(); + // canvas_.Translate({x + bounds.left(), y + bounds.top(), 0.0}); canvas_.DrawPath(path, paint_); - canvas_.Restore(); + // canvas_.Restore(); return; } diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc index 1f677294f89df..f68a1012a7138 100644 --- a/impeller/geometry/path.cc +++ b/impeller/geometry/path.cc @@ -61,6 +61,34 @@ void Path::SetConvexity(Convexity value) { convexity_ = value; } +void Path::ShiftPath(Point shift) { + size_t currentIndex = 0; + for (const auto& component : components_) { + switch (component.type) { + case ComponentType::kLinear: + linears_[component.index].p1 = linears_[component.index].p1 + shift; + linears_[component.index].p2 = linears_[component.index].p2 + shift; + break; + case ComponentType::kQuadratic: + quads_[component.index].cp = quads_[component.index].cp + shift; + quads_[component.index].p1 = quads_[component.index].p1 + shift; + quads_[component.index].p2 = quads_[component.index].p2 + shift; + break; + case ComponentType::kCubic: + cubics_[component.index].cp1 = cubics_[component.index].cp1 + shift; + cubics_[component.index].cp2 = cubics_[component.index].cp2 + shift; + cubics_[component.index].p1 = cubics_[component.index].p1 + shift; + cubics_[component.index].p2 = cubics_[component.index].p2 + shift; + break; + case ComponentType::kContour: + contours_[component.index].destination = + 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..408a597a27163 100644 --- a/impeller/geometry/path.h +++ b/impeller/geometry/path.h @@ -111,6 +111,8 @@ class Path { void SetContourClosed(bool is_closed); + void ShiftPath(Point shift); + template using Applier = std::function; void EnumerateComponents( From 859cbfde2701c2e24a2c0be6b7fcc4242cea26b1 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 29 Aug 2023 17:31:45 -0700 Subject: [PATCH 2/3] add test and comment. --- impeller/aiks/color_source.cc | 4 --- impeller/aiks/color_source.h | 2 -- impeller/display_list/dl_dispatcher.cc | 5 +--- impeller/geometry/geometry_unittests.cc | 36 +++++++++++++++++++++++++ impeller/geometry/path.cc | 2 +- impeller/geometry/path.h | 3 ++- 6 files changed, 40 insertions(+), 12 deletions(-) diff --git a/impeller/aiks/color_source.cc b/impeller/aiks/color_source.cc index f04e8019194e0..3329bcb4b832e 100644 --- a/impeller/aiks/color_source.cc +++ b/impeller/aiks/color_source.cc @@ -230,10 +230,6 @@ ColorSource ColorSource::MakeScene(std::shared_ptr scene_node, } #endif // IMPELLER_ENABLE_3D -void ColorSource::ConcatEffectTransform(const Matrix& effect_transform) { - -} - ColorSource::Type ColorSource::GetType() const { return type_; } diff --git a/impeller/aiks/color_source.h b/impeller/aiks/color_source.h index 6a4a9ba737d96..7849b841ec784 100644 --- a/impeller/aiks/color_source.h +++ b/impeller/aiks/color_source.h @@ -97,8 +97,6 @@ class ColorSource { std::shared_ptr GetContents(const Paint& paint) const; - void ConcatEffectTransform(const Matrix& effect_transform); - private: Type type_ = Type::kColor; ColorSourceProc proc_; diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index 68b9279e54324..545d26c69dc7d 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/impeller/display_list/dl_dispatcher.cc @@ -1120,11 +1120,8 @@ void DlDispatcher::drawTextBlob(const sk_sp& blob, paint_.color_source.GetType() != ColorSource::Type::kColor) { auto bounds = blob->bounds(); auto path = skia_conversions::PathDataFromTextBlob(blob); - path.ShiftPath(Point(x + bounds.left(), y + bounds.top())); - // canvas_.Save(); - // canvas_.Translate({x + bounds.left(), y + bounds.top(), 0.0}); + 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 f68a1012a7138..591241e8b99f8 100644 --- a/impeller/geometry/path.cc +++ b/impeller/geometry/path.cc @@ -61,7 +61,7 @@ void Path::SetConvexity(Convexity value) { convexity_ = value; } -void Path::ShiftPath(Point shift) { +void Path::Shift(Point shift) { size_t currentIndex = 0; for (const auto& component : components_) { switch (component.type) { diff --git a/impeller/geometry/path.h b/impeller/geometry/path.h index 408a597a27163..dbed30bfae7d1 100644 --- a/impeller/geometry/path.h +++ b/impeller/geometry/path.h @@ -111,7 +111,8 @@ class Path { void SetContourClosed(bool is_closed); - void ShiftPath(Point shift); + /// @brief Transform the path by the given offset in-place. + void Shift(Point shift); template using Applier = std::function; From 484bf1f7c3a69a4f1ed2f618621da4a05a8c74d0 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 29 Aug 2023 17:57:58 -0700 Subject: [PATCH 3/3] ++ --- impeller/geometry/path.cc | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc index 591241e8b99f8..2901c53c1c9e2 100644 --- a/impeller/geometry/path.cc +++ b/impeller/geometry/path.cc @@ -66,23 +66,22 @@ void Path::Shift(Point shift) { for (const auto& component : components_) { switch (component.type) { case ComponentType::kLinear: - linears_[component.index].p1 = linears_[component.index].p1 + shift; - linears_[component.index].p2 = linears_[component.index].p2 + shift; + linears_[component.index].p1 += shift; + linears_[component.index].p2 += shift; break; case ComponentType::kQuadratic: - quads_[component.index].cp = quads_[component.index].cp + shift; - quads_[component.index].p1 = quads_[component.index].p1 + shift; - quads_[component.index].p2 = quads_[component.index].p2 + shift; + quads_[component.index].cp += shift; + quads_[component.index].p1 += shift; + quads_[component.index].p2 += shift; break; case ComponentType::kCubic: - cubics_[component.index].cp1 = cubics_[component.index].cp1 + shift; - cubics_[component.index].cp2 = cubics_[component.index].cp2 + shift; - cubics_[component.index].p1 = cubics_[component.index].p1 + shift; - cubics_[component.index].p2 = cubics_[component.index].p2 + shift; + 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 = - contours_[component.index].destination + shift; + contours_[component.index].destination += shift; break; } currentIndex++;