Skip to content

Commit 5dd51f3

Browse files
kjlubickIvoneDjaja
kjlubick
authored and
IvoneDjaja
committed
Update all uses of mutable SkPath methods to use SkPathBuilder (flutter#177738)
Skia is removing the APIs that allow changing an SkPath. This updates those callsites to use SkPathBuilder where appropriate. ## 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], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent 7ec2a7c commit 5dd51f3

File tree

18 files changed

+305
-269
lines changed

18 files changed

+305
-269
lines changed

‎engine/src/flutter/display_list/geometry/dl_path.cc‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,14 @@ DlPath DlPath::MakeRectLTRB(DlScalar left,
4444
DlScalar top,
4545
DlScalar right,
4646
DlScalar bottom) {
47-
return DlPath(SkPath().addRect(left, top, right, bottom));
47+
return DlPath(SkPath::Rect(SkRect::MakeLTRB(left, top, right, bottom)));
4848
}
4949

5050
DlPath DlPath::MakeRectXYWH(DlScalar x,
5151
DlScalar y,
5252
DlScalar width,
5353
DlScalar height) {
54-
return DlPath(SkPath().addRect(SkRect::MakeXYWH(x, y, width, height)));
54+
return DlPath(SkPath::Rect(SkRect::MakeXYWH(x, y, width, height)));
5555
}
5656

5757
DlPath DlPath::MakeOval(const DlRect& bounds) {
@@ -102,15 +102,15 @@ DlPath DlPath::MakeArc(const DlRect& bounds,
102102
DlDegrees start,
103103
DlDegrees sweep,
104104
bool use_center) {
105-
SkPath path;
105+
SkPathBuilder path;
106106
if (use_center) {
107107
path.moveTo(ToSkPoint(bounds.GetCenter()));
108108
}
109109
path.arcTo(ToSkRect(bounds), start.degrees, sweep.degrees, !use_center);
110110
if (use_center) {
111111
path.close();
112112
}
113-
return DlPath(path);
113+
return DlPath(path.detach());
114114
}
115115

116116
const SkPath& DlPath::GetSkPath() const {
@@ -188,9 +188,7 @@ void DlPath::WillRenderSkPath() const {
188188
if (!offset.IsFinite()) {
189189
return DlPath();
190190
}
191-
SkPath path = data_->sk_path;
192-
path = path.offset(offset.x, offset.y);
193-
return DlPath(path);
191+
return DlPath(data_->sk_path.makeOffset(offset.x, offset.y));
194192
}
195193

196194
[[nodiscard]] DlPath DlPath::WithFillType(DlPathFillType type) const {
@@ -259,9 +257,9 @@ bool DlPath::IsConvex() const {
259257
}
260258

261259
DlPath DlPath::operator+(const DlPath& other) const {
262-
SkPath path = GetSkPath();
260+
SkPathBuilder path = SkPathBuilder(GetSkPath());
263261
path.addPath(other.GetSkPath());
264-
return DlPath(path);
262+
return DlPath(path.detach());
265263
}
266264

267265
void DlPath::ReduceConic(DlPathReceiver& receiver,

‎engine/src/flutter/display_list/geometry/dl_path_builder.cc‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,31 +57,38 @@ DlPathBuilder& DlPathBuilder::SetFillType(DlPathFillType fill_type) {
5757
}
5858

5959
DlPathBuilder& DlPathBuilder::MoveTo(DlPoint p2) {
60-
path_.moveTo(p2.x, p2.y);
60+
path_.moveTo({p2.x, p2.y});
6161
return *this;
6262
}
6363

6464
DlPathBuilder& DlPathBuilder::LineTo(DlPoint p2) {
65-
path_.lineTo(p2.x, p2.y);
65+
path_.lineTo({p2.x, p2.y});
6666
return *this;
6767
}
6868

6969
DlPathBuilder& DlPathBuilder::QuadraticCurveTo(DlPoint cp, DlPoint p2) {
70-
path_.quadTo(cp.x, cp.y, p2.x, p2.y);
70+
path_.quadTo({cp.x, cp.y}, {p2.x, p2.y});
7171
return *this;
7272
}
7373

7474
DlPathBuilder& DlPathBuilder::ConicCurveTo(DlPoint cp,
7575
DlPoint p2,
7676
DlScalar weight) {
77-
path_.conicTo(cp.x, cp.y, p2.x, p2.y, weight);
77+
// Skia's SkPath object used to do these checks, but SkPathBuilder does not.
78+
if (!(weight > 0)) {
79+
return this->LineTo(p2);
80+
} else if (!std::isfinite(weight)) {
81+
this->LineTo(cp);
82+
return this->LineTo(p2);
83+
}
84+
path_.conicTo({cp.x, cp.y}, {p2.x, p2.y}, weight);
7885
return *this;
7986
}
8087

8188
DlPathBuilder& DlPathBuilder::CubicCurveTo(DlPoint cp1,
8289
DlPoint cp2,
8390
DlPoint p2) {
84-
path_.cubicTo(cp1.x, cp1.y, cp2.x, cp2.y, p2.x, p2.y);
91+
path_.cubicTo({cp1.x, cp1.y}, {cp2.x, cp2.y}, {p2.x, p2.y});
8592
return *this;
8693
}
8794

@@ -139,13 +146,11 @@ DlPathBuilder& DlPathBuilder::AddPath(const DlPath& path) {
139146
}
140147

141148
const DlPath DlPathBuilder::CopyPath() {
142-
return DlPath(path_);
149+
return DlPath(path_.snapshot());
143150
}
144151

145152
const DlPath DlPathBuilder::TakePath() {
146-
DlPath path = DlPath(path_);
147-
path_.reset();
148-
return path;
153+
return DlPath(path_.detach());
149154
}
150155

151156
} // namespace flutter

‎engine/src/flutter/display_list/geometry/dl_path_builder.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
#include "flutter/display_list/geometry/dl_geometry_types.h"
99
#include "flutter/display_list/geometry/dl_path.h"
10-
#include "flutter/third_party/skia/include/core/SkPath.h"
10+
#include "flutter/third_party/skia/include/core/SkPathBuilder.h"
1111

1212
namespace flutter {
1313

@@ -132,7 +132,7 @@ class DlPathBuilder {
132132
const DlPath TakePath();
133133

134134
private:
135-
SkPath path_;
135+
SkPathBuilder path_;
136136
};
137137

138138
} // namespace flutter

‎engine/src/flutter/display_list/geometry/dl_path_unittests.cc‎

Lines changed: 73 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "flutter/display_list/geometry/dl_path_builder.h"
1010
#include "flutter/display_list/testing/dl_test_mock_path_receiver.h"
1111
#include "flutter/third_party/skia/include/core/SkPath.h"
12+
#include "flutter/third_party/skia/include/core/SkPoint.h"
1213
#include "flutter/third_party/skia/include/core/SkRRect.h"
1314

1415
namespace flutter {
@@ -392,14 +393,18 @@ TEST(DisplayListPath, ConstructFromDlPathBuilderRoundRect) {
392393
}
393394

394395
TEST(DisplayListPath, ConstructFromPath) {
395-
SkPath sk_path1;
396-
sk_path1.moveTo(10, 10);
397-
sk_path1.lineTo(20, 20);
398-
sk_path1.lineTo(20, 10);
399-
SkPath sk_path2;
400-
sk_path2.moveTo(10, 10);
401-
sk_path2.lineTo(20, 20);
402-
sk_path2.lineTo(20, 10);
396+
SkPathBuilder pb1;
397+
pb1.moveTo({10, 10});
398+
pb1.lineTo({20, 20});
399+
pb1.lineTo({20, 10});
400+
SkPathBuilder pb2;
401+
pb2.moveTo({10, 10});
402+
pb2.lineTo({20, 20});
403+
pb2.lineTo({20, 10});
404+
405+
SkPath sk_path1 = pb1.detach();
406+
SkPath sk_path2 = pb2.detach();
407+
403408
DlPath path(sk_path1);
404409

405410
ASSERT_EQ(sk_path1, sk_path2);
@@ -428,22 +433,21 @@ TEST(DisplayListPath, ConstructFromDlPathBuilderEqualsConstructFromSkia) {
428433
path_builder.LineTo({0, 100});
429434
path_builder.Close();
430435

431-
SkPath sk_path;
432-
sk_path.setFillType(SkPathFillType::kWinding);
433-
sk_path.moveTo(0, 0);
434-
sk_path.lineTo(100, 0);
435-
sk_path.lineTo(0, 100);
436+
SkPathBuilder sk_path(SkPathFillType::kWinding);
437+
sk_path.moveTo({0, 0});
438+
sk_path.lineTo({100, 0});
439+
sk_path.lineTo({0, 100});
436440
sk_path.close();
437441

438-
EXPECT_EQ(path_builder.TakePath(), DlPath(sk_path));
442+
EXPECT_EQ(path_builder.TakePath(), DlPath(sk_path.detach()));
439443
}
440444

441445
TEST(DisplayListPath, IsLineFromSkPath) {
442-
SkPath sk_path;
443-
sk_path.moveTo(SkPoint::Make(0, 0));
444-
sk_path.lineTo(SkPoint::Make(100, 100));
446+
SkPathBuilder sk_path;
447+
sk_path.moveTo({0, 0});
448+
sk_path.lineTo({100, 100});
445449

446-
DlPath path = DlPath(sk_path);
450+
DlPath path = DlPath(sk_path.detach());
447451

448452
DlPoint start;
449453
DlPoint end;
@@ -507,16 +511,16 @@ static void TestPathDispatchOneOfEachVerb(const DlPath& path) {
507511
}
508512

509513
TEST(DisplayListPath, DispatchSkiaPathOneOfEachVerb) {
510-
SkPath path;
514+
SkPathBuilder path;
511515

512-
path.moveTo(100, 200);
513-
path.lineTo(101, 201);
514-
path.quadTo(110, 202, 102, 210);
515-
path.conicTo(150, 240, 250, 140, 0.5);
516-
path.cubicTo(300, 300, 350, 300, 300, 350);
516+
path.moveTo({100, 200});
517+
path.lineTo({101, 201});
518+
path.quadTo({110, 202}, {102, 210});
519+
path.conicTo({150, 240}, {250, 140}, 0.5);
520+
path.cubicTo({300, 300}, {350, 300}, {300, 350});
517521
path.close();
518522

519-
TestPathDispatchOneOfEachVerb(DlPath(path));
523+
TestPathDispatchOneOfEachVerb(DlPath(path.detach()));
520524
}
521525

522526
TEST(DisplayListPath, DispatchImpellerPathOneOfEachVerb) {
@@ -558,11 +562,11 @@ static void TestPathDispatchConicToQuads(
558562
static void TestSkiaPathDispatchConicToQuads(
559563
DlScalar weight,
560564
const std::array<DlPoint, 4>& quad_points) {
561-
SkPath sk_path;
562-
sk_path.moveTo(10, 10);
563-
sk_path.conicTo(20, 10, 20, 20, weight);
565+
SkPathBuilder sk_path;
566+
sk_path.moveTo({10, 10});
567+
sk_path.conicTo({20, 10}, {20, 20}, weight);
564568

565-
TestPathDispatchConicToQuads(DlPath(sk_path), weight, quad_points);
569+
TestPathDispatchConicToQuads(DlPath(sk_path.detach()), weight, quad_points);
566570
}
567571

568572
static void TestImpellerPathDispatchConicToQuads(
@@ -662,12 +666,12 @@ static void TestPathDispatchUnclosedTriangle(const DlPath& path) {
662666
}
663667

664668
TEST(DisplayListPath, DispatchUnclosedSkiaTriangle) {
665-
SkPath sk_path;
666-
sk_path.moveTo(10, 10);
667-
sk_path.lineTo(20, 10);
668-
sk_path.lineTo(10, 20);
669+
SkPathBuilder sk_path;
670+
sk_path.moveTo({10, 10});
671+
sk_path.lineTo({20, 10});
672+
sk_path.lineTo({10, 20});
669673

670-
TestPathDispatchUnclosedTriangle(DlPath(sk_path));
674+
TestPathDispatchUnclosedTriangle(DlPath(sk_path.detach()));
671675
}
672676

673677
TEST(DisplayListPath, DispatchUnclosedImpellerTriangle) {
@@ -696,13 +700,13 @@ static void TestPathDispatchClosedTriangle(const DlPath& path) {
696700
}
697701

698702
TEST(DisplayListPath, DispatchClosedSkiaTriangle) {
699-
SkPath sk_path;
700-
sk_path.moveTo(10, 10);
701-
sk_path.lineTo(20, 10);
702-
sk_path.lineTo(10, 20);
703+
SkPathBuilder sk_path;
704+
sk_path.moveTo({10, 10});
705+
sk_path.lineTo({20, 10});
706+
sk_path.lineTo({10, 20});
703707
sk_path.close();
704708

705-
TestPathDispatchClosedTriangle(DlPath(sk_path));
709+
TestPathDispatchClosedTriangle(DlPath(sk_path.detach()));
706710
}
707711

708712
TEST(DisplayListPath, DispatchClosedPathBuilderTriangle) {
@@ -738,19 +742,19 @@ static void TestPathDispatchMixedCloseTriangles(const DlPath& path) {
738742
}
739743

740744
TEST(DisplayListPath, DispatchMixedCloseSkiaPath) {
741-
SkPath sk_path;
742-
sk_path.moveTo(10, 10);
743-
sk_path.lineTo(20, 10);
744-
sk_path.lineTo(10, 20);
745-
sk_path.moveTo(110, 10);
746-
sk_path.lineTo(120, 10);
747-
sk_path.lineTo(110, 20);
745+
SkPathBuilder sk_path;
746+
sk_path.moveTo({10, 10});
747+
sk_path.lineTo({20, 10});
748+
sk_path.lineTo({10, 20});
749+
sk_path.moveTo({110, 10});
750+
sk_path.lineTo({120, 10});
751+
sk_path.lineTo({110, 20});
748752
sk_path.close();
749-
sk_path.moveTo(210, 10);
750-
sk_path.lineTo(220, 10);
751-
sk_path.lineTo(210, 20);
753+
sk_path.moveTo({210, 10});
754+
sk_path.lineTo({220, 10});
755+
sk_path.lineTo({210, 20});
752756

753-
TestPathDispatchMixedCloseTriangles(DlPath(sk_path));
757+
TestPathDispatchMixedCloseTriangles(DlPath(sk_path.detach()));
754758
}
755759

756760
TEST(DisplayListPath, DispatchMixedCloseImpellerPath) {
@@ -789,15 +793,15 @@ static void TestPathDispatchImplicitMoveAfterClose(const DlPath& path) {
789793
}
790794

791795
TEST(DisplayListPath, DispatchImplicitMoveAfterCloseSkiaPath) {
792-
SkPath sk_path;
793-
sk_path.moveTo(10, 10);
794-
sk_path.lineTo(20, 10);
795-
sk_path.lineTo(10, 20);
796+
SkPathBuilder sk_path;
797+
sk_path.moveTo({10, 10});
798+
sk_path.lineTo({20, 10});
799+
sk_path.lineTo({10, 20});
796800
sk_path.close();
797-
sk_path.lineTo(-20, 10);
798-
sk_path.lineTo(10, -20);
801+
sk_path.lineTo({-20, 10});
802+
sk_path.lineTo({10, -20});
799803

800-
TestPathDispatchImplicitMoveAfterClose(DlPath(sk_path));
804+
TestPathDispatchImplicitMoveAfterClose(DlPath(sk_path.detach()));
801805
}
802806

803807
TEST(DisplayListPath, DispatchImplicitMoveAfterClosePathBuilder) {
@@ -817,25 +821,25 @@ TEST(DisplayListPath, DispatchImplicitMoveAfterClosePathBuilder) {
817821
// supported by either Flutter public APIs or Impeller
818822

819823
TEST(DisplayListPath, CannotConstructFromSkiaInverseWinding) {
820-
SkPath sk_path;
821-
sk_path.setFillType(SkPathFillType::kInverseWinding);
822-
sk_path.moveTo(0, 0);
823-
sk_path.lineTo(100, 0);
824-
sk_path.lineTo(0, 100);
824+
SkPathBuilder sk_path(SkPathFillType::kInverseWinding);
825+
sk_path.moveTo({0, 0});
826+
sk_path.lineTo({100, 0});
827+
sk_path.lineTo({0, 100});
825828
sk_path.close();
826829

827-
EXPECT_DEATH_IF_SUPPORTED(new DlPath(sk_path), "SkPathFillType_IsInverse");
830+
EXPECT_DEATH_IF_SUPPORTED(new DlPath(sk_path.detach()),
831+
"SkPathFillType_IsInverse");
828832
}
829833

830834
TEST(DisplayListPath, CannotConstructFromSkiaInverseEvenOdd) {
831-
SkPath sk_path;
832-
sk_path.setFillType(SkPathFillType::kInverseEvenOdd);
833-
sk_path.moveTo(0, 0);
834-
sk_path.lineTo(100, 0);
835-
sk_path.lineTo(0, 100);
835+
SkPathBuilder sk_path(SkPathFillType::kInverseEvenOdd);
836+
sk_path.moveTo({0, 0});
837+
sk_path.lineTo({100, 0});
838+
sk_path.lineTo({0, 100});
836839
sk_path.close();
837840

838-
EXPECT_DEATH_IF_SUPPORTED(new DlPath(sk_path), "SkPathFillType_IsInverse");
841+
EXPECT_DEATH_IF_SUPPORTED(new DlPath(sk_path.detach()),
842+
"SkPathFillType_IsInverse");
839843
}
840844
#endif
841845

‎engine/src/flutter/impeller/toolkit/interop/path.cc‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,20 @@
44

55
#include "impeller/toolkit/interop/path.h"
66

7+
#include "third_party/skia/include/core/SkRect.h"
8+
79
namespace impeller::interop {
810

9-
Path::Path(const SkPath& path) : path_(path) {}
11+
Path::Path(const SkPath& path) : path_(SkPathBuilder(path)) {}
1012

1113
Path::~Path() = default;
1214

13-
const SkPath& Path::GetPath() const {
14-
return path_;
15+
SkPath Path::GetPath() const {
16+
return path_.snapshot();
1517
}
1618

1719
ImpellerRect Path::GetBounds() const {
18-
const auto bounds = path_.getBounds();
20+
const auto bounds = path_.computeFiniteBounds().value_or(SkRect());
1921
return ImpellerRect{
2022
.x = bounds.x(),
2123
.y = bounds.y(),

0 commit comments

Comments
 (0)