Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] compute path bounds once, use Skia computed bounds where possible. #45456

Merged
merged 2 commits into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions impeller/display_list/skia_conversions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,16 @@ 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);
}

Path ToPath(const SkRRect& rrect) {
return PathBuilder{}
.AddRoundedRect(ToRect(rrect.getBounds()), ToRoundingRadii(rrect))
.SetConvexity(Convexity::kConvex)
.SetBounds(ToRect(rrect.getBounds()))
.TakePath();
}

Expand Down
19 changes: 19 additions & 0 deletions impeller/geometry/geometry_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 11 additions & 2 deletions impeller/geometry/path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -397,14 +397,19 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const {
}

std::optional<Rect> Path::GetBoundingBox() const {
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 Rect{min.x, min.y, difference.x, difference.y};
computed_bounds_ = Rect{min.x, min.y, difference.x, difference.y};
}

std::optional<Rect> Path::GetTransformedBoundingBox(
Expand Down Expand Up @@ -461,4 +466,8 @@ std::optional<std::pair<Point, Point>> Path::GetMinMaxCoveragePoints() const {
return std::make_pair(min.value(), max.value());
}

void Path::SetBounds(Rect rect) {
computed_bounds_ = rect;
}

} // namespace impeller
10 changes: 10 additions & 0 deletions impeller/geometry/path.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -146,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);
Expand Down Expand Up @@ -178,6 +186,8 @@ class Path {
std::vector<QuadraticPathComponent> quads_;
std::vector<CubicPathComponent> cubics_;
std::vector<ContourComponent> contours_;

std::optional<Rect> computed_bounds_;
};

} // namespace impeller
10 changes: 10 additions & 0 deletions impeller/geometry/path_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -458,4 +462,10 @@ PathBuilder& PathBuilder::Shift(Point offset) {
return *this;
}

PathBuilder& PathBuilder::SetBounds(Rect bounds) {
prototype_.SetBounds(bounds);
did_compute_bounds_ = true;
return *this;
}

} // namespace impeller
9 changes: 9 additions & 0 deletions impeller/geometry/path_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -133,6 +141,7 @@ class PathBuilder {
Point current_;
Path prototype_;
Convexity convexity_;
bool did_compute_bounds_ = false;

PathBuilder& AddRoundedRectTopLeft(Rect rect, RoundingRadii radii);

Expand Down