From 0b15945604b96b92d74a93308136f0b54bd7c940 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 3 Sep 2023 14:42:55 -0700 Subject: [PATCH 1/2] [Impeller] cache Skia path bounds. --- impeller/display_list/skia_conversions.cc | 3 +++ impeller/geometry/path.cc | 9 ++++++++- impeller/geometry/path.h | 4 ++++ impeller/geometry/path_builder.cc | 5 +++++ impeller/geometry/path_builder.h | 8 ++++++++ 5 files changed, 28 insertions(+), 1 deletion(-) diff --git a/impeller/display_list/skia_conversions.cc b/impeller/display_list/skia_conversions.cc index 92db8d8034ed4..8b17ad0acf315 100644 --- a/impeller/display_list/skia_conversions.cc +++ b/impeller/display_list/skia_conversions.cc @@ -122,6 +122,8 @@ Path ToPath(const SkPath& path, Point shift) { builder.SetConvexity(path.isConvex() ? Convexity::kConvex : Convexity::kUnknown); builder.Shift(shift); + auto sk_bounds = path.getBounds().makeOutset(shift.x, shift.y); + builder.SetBounds(ToRect(sk_bounds)); return builder.TakePath(fill_type); } @@ -129,6 +131,7 @@ Path ToPath(const SkRRect& rrect) { return PathBuilder{} .AddRoundedRect(ToRect(rrect.getBounds()), ToRoundingRadii(rrect)) .SetConvexity(Convexity::kConvex) + .SetBounds(ToRect(rrect.getBounds())) .TakePath(); } diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc index 2901c53c1c9e2..49518587dc594 100644 --- a/impeller/geometry/path.cc +++ b/impeller/geometry/path.cc @@ -397,6 +397,9 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const { } std::optional Path::GetBoundingBox() const { + if (computed_bounds_.has_value()) { + return computed_bounds_; + } auto min_max = GetMinMaxCoveragePoints(); if (!min_max.has_value()) { return std::nullopt; @@ -404,7 +407,7 @@ std::optional Path::GetBoundingBox() const { auto min = min_max->first; auto max = min_max->second; const auto difference = max - min; - return Rect{min.x, min.y, difference.x, difference.y}; + return computed_bounds_ = Rect{min.x, min.y, difference.x, difference.y}; } std::optional Path::GetTransformedBoundingBox( @@ -461,4 +464,8 @@ std::optional> Path::GetMinMaxCoveragePoints() const { return std::make_pair(min.value(), max.value()); } +void Path::SetBounds(Rect rect) { + computed_bounds_ = rect; +} + } // namespace impeller diff --git a/impeller/geometry/path.h b/impeller/geometry/path.h index be3c9cce4e50d..e558b2cb8a80a 100644 --- a/impeller/geometry/path.h +++ b/impeller/geometry/path.h @@ -138,6 +138,8 @@ class Path { void SetFillType(FillType fill); + void SetBounds(Rect rect); + Path& AddLinearComponent(Point p1, Point p2); Path& AddQuadraticComponent(Point p1, Point cp, Point p2); @@ -178,6 +180,8 @@ class Path { std::vector quads_; std::vector cubics_; std::vector contours_; + + mutable std::optional computed_bounds_; }; } // namespace impeller diff --git a/impeller/geometry/path_builder.cc b/impeller/geometry/path_builder.cc index f7876a9f50416..bef630a1a0fab 100644 --- a/impeller/geometry/path_builder.cc +++ b/impeller/geometry/path_builder.cc @@ -458,4 +458,9 @@ PathBuilder& PathBuilder::Shift(Point offset) { return *this; } +PathBuilder& PathBuilder::SetBounds(Rect bounds) { + prototype_.SetBounds(bounds); + return *this; +} + } // namespace impeller diff --git a/impeller/geometry/path_builder.h b/impeller/geometry/path_builder.h index 2361da56a55f4..92918a03eeed7 100644 --- a/impeller/geometry/path_builder.h +++ b/impeller/geometry/path_builder.h @@ -97,6 +97,14 @@ class PathBuilder { /// `offset`. PathBuilder& Shift(Point offset); + /// @brief Set the bounding box that will be used by `Path.GetBoundingBox` in + /// place of performing the computation. + /// + /// When Impeller recieves Skia Path objects, many of these already + /// have computed bounds. This method is used to avoid needlessly + /// recomputing these bounds. + PathBuilder& SetBounds(Rect bounds); + struct RoundingRadii { Point top_left; Point bottom_left; From cce9481dc075a8237513bcbd3d752815635a9fd7 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 5 Sep 2023 09:49:16 -0700 Subject: [PATCH 2/2] cache bounds on construction. --- impeller/geometry/geometry_unittests.cc | 19 +++++++++++++++++++ impeller/geometry/path.cc | 12 +++++++----- impeller/geometry/path.h | 8 +++++++- impeller/geometry/path_builder.cc | 5 +++++ impeller/geometry/path_builder.h | 1 + 5 files changed, 39 insertions(+), 6 deletions(-) diff --git a/impeller/geometry/geometry_unittests.cc b/impeller/geometry/geometry_unittests.cc index d5780fb70bb77..9c4d7a77f05ec 100644 --- a/impeller/geometry/geometry_unittests.cc +++ b/impeller/geometry/geometry_unittests.cc @@ -2422,6 +2422,25 @@ TEST(GeometryTest, PathShifting) { ASSERT_EQ(cubic.p2, Point(31, 31)); } +TEST(GeometryTest, PathBuilderWillComputeBounds) { + PathBuilder builder; + auto path_1 = builder.AddLine({0, 0}, {1, 1}).TakePath(); + + ASSERT_EQ(path_1.GetBoundingBox().value(), Rect::MakeLTRB(0, 0, 1, 1)); + + auto path_2 = builder.AddLine({-1, -1}, {1, 1}).TakePath(); + + // Verify that PathBuilder recomputes the bounds. + ASSERT_EQ(path_2.GetBoundingBox().value(), Rect::MakeLTRB(-1, -1, 1, 1)); + + // PathBuilder can set the bounds to whatever it wants + auto path_3 = builder.AddLine({0, 0}, {1, 1}) + .SetBounds(Rect::MakeLTRB(0, 0, 100, 100)) + .TakePath(); + + ASSERT_EQ(path_3.GetBoundingBox().value(), Rect::MakeLTRB(0, 0, 100, 100)); +} + } // namespace testing } // namespace impeller diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc index 49518587dc594..ff2c39d6ecaf0 100644 --- a/impeller/geometry/path.cc +++ b/impeller/geometry/path.cc @@ -397,17 +397,19 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const { } std::optional Path::GetBoundingBox() const { - if (computed_bounds_.has_value()) { - return computed_bounds_; - } + return computed_bounds_; +} + +void Path::ComputeBounds() { auto min_max = GetMinMaxCoveragePoints(); if (!min_max.has_value()) { - return std::nullopt; + computed_bounds_ = std::nullopt; + return; } auto min = min_max->first; auto max = min_max->second; const auto difference = max - min; - return computed_bounds_ = Rect{min.x, min.y, difference.x, difference.y}; + computed_bounds_ = Rect{min.x, min.y, difference.x, difference.y}; } std::optional Path::GetTransformedBoundingBox( diff --git a/impeller/geometry/path.h b/impeller/geometry/path.h index e558b2cb8a80a..757c5c03d9548 100644 --- a/impeller/geometry/path.h +++ b/impeller/geometry/path.h @@ -148,6 +148,12 @@ class Path { Path& AddContourComponent(Point destination, bool is_closed = false); + /// @brief Called by `PathBuilder` to compute the bounds for certain paths. + /// + /// `PathBuilder` may set the bounds directly, in case they come from a source + /// with already computed bounds, such as an SkPath. + void ComputeBounds(); + void SetContourClosed(bool is_closed); void Shift(Point shift); @@ -181,7 +187,7 @@ class Path { std::vector cubics_; std::vector contours_; - mutable std::optional computed_bounds_; + std::optional computed_bounds_; }; } // namespace impeller diff --git a/impeller/geometry/path_builder.cc b/impeller/geometry/path_builder.cc index bef630a1a0fab..83359cc6b4452 100644 --- a/impeller/geometry/path_builder.cc +++ b/impeller/geometry/path_builder.cc @@ -22,6 +22,10 @@ Path PathBuilder::TakePath(FillType fill) { auto path = prototype_; path.SetFillType(fill); path.SetConvexity(convexity_); + if (!did_compute_bounds_) { + path.ComputeBounds(); + } + did_compute_bounds_ = false; return path; } @@ -460,6 +464,7 @@ PathBuilder& PathBuilder::Shift(Point offset) { PathBuilder& PathBuilder::SetBounds(Rect bounds) { prototype_.SetBounds(bounds); + did_compute_bounds_ = true; return *this; } diff --git a/impeller/geometry/path_builder.h b/impeller/geometry/path_builder.h index 92918a03eeed7..342f0912aca46 100644 --- a/impeller/geometry/path_builder.h +++ b/impeller/geometry/path_builder.h @@ -141,6 +141,7 @@ class PathBuilder { Point current_; Path prototype_; Convexity convexity_; + bool did_compute_bounds_ = false; PathBuilder& AddRoundedRectTopLeft(Rect rect, RoundingRadii radii);