Skip to content

Commit 77ae0ef

Browse files
auto-submit[bot]auto-submit[bot]
authored and
IvoneDjaja
committed
Reverts "Update all uses of mutable SkPath methods to use SkPathBuilder (flutter#177738)" (flutter#178125)
<!-- start_original_pr_link --> Reverts: flutter#177738 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: chingjun <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: Broke internal tests. Some shapes are not drawn. <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: kjlubick <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {jason-simmons} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: 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 <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
1 parent e319e2d commit 77ae0ef

File tree

18 files changed

+269
-305
lines changed

18 files changed

+269
-305
lines changed

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

Lines changed: 9 additions & 7 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::Rect(SkRect::MakeLTRB(left, top, right, bottom)));
47+
return DlPath(SkPath().addRect(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::Rect(SkRect::MakeXYWH(x, y, width, height)));
54+
return DlPath(SkPath().addRect(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-
SkPathBuilder path;
105+
SkPath 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.detach());
113+
return DlPath(path);
114114
}
115115

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

194196
[[nodiscard]] DlPath DlPath::WithFillType(DlPathFillType type) const {
@@ -257,9 +259,9 @@ bool DlPath::IsConvex() const {
257259
}
258260

259261
DlPath DlPath::operator+(const DlPath& other) const {
260-
SkPathBuilder path = SkPathBuilder(GetSkPath());
262+
SkPath path = GetSkPath();
261263
path.addPath(other.GetSkPath());
262-
return DlPath(path.detach());
264+
return DlPath(path);
263265
}
264266

265267
void DlPath::ReduceConic(DlPathReceiver& receiver,

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

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,38 +57,31 @@ 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-
// 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);
77+
path_.conicTo(cp.x, cp.y, p2.x, p2.y, weight);
8578
return *this;
8679
}
8780

8881
DlPathBuilder& DlPathBuilder::CubicCurveTo(DlPoint cp1,
8982
DlPoint cp2,
9083
DlPoint p2) {
91-
path_.cubicTo({cp1.x, cp1.y}, {cp2.x, cp2.y}, {p2.x, p2.y});
84+
path_.cubicTo(cp1.x, cp1.y, cp2.x, cp2.y, p2.x, p2.y);
9285
return *this;
9386
}
9487

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

148141
const DlPath DlPathBuilder::CopyPath() {
149-
return DlPath(path_.snapshot());
142+
return DlPath(path_);
150143
}
151144

152145
const DlPath DlPathBuilder::TakePath() {
153-
return DlPath(path_.detach());
146+
DlPath path = DlPath(path_);
147+
path_.reset();
148+
return path;
154149
}
155150

156151
} // 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/SkPathBuilder.h"
10+
#include "flutter/third_party/skia/include/core/SkPath.h"
1111

1212
namespace flutter {
1313

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

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

138138
} // namespace flutter

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

Lines changed: 69 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
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"
1312
#include "flutter/third_party/skia/include/core/SkRRect.h"
1413

1514
namespace flutter {
@@ -393,18 +392,14 @@ TEST(DisplayListPath, ConstructFromDlPathBuilderRoundRect) {
393392
}
394393

395394
TEST(DisplayListPath, ConstructFromPath) {
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-
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);
408403
DlPath path(sk_path1);
409404

410405
ASSERT_EQ(sk_path1, sk_path2);
@@ -433,21 +428,22 @@ TEST(DisplayListPath, ConstructFromDlPathBuilderEqualsConstructFromSkia) {
433428
path_builder.LineTo({0, 100});
434429
path_builder.Close();
435430

436-
SkPathBuilder sk_path(SkPathFillType::kWinding);
437-
sk_path.moveTo({0, 0});
438-
sk_path.lineTo({100, 0});
439-
sk_path.lineTo({0, 100});
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);
440436
sk_path.close();
441437

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

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

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

452448
DlPoint start;
453449
DlPoint end;
@@ -511,16 +507,16 @@ static void TestPathDispatchOneOfEachVerb(const DlPath& path) {
511507
}
512508

513509
TEST(DisplayListPath, DispatchSkiaPathOneOfEachVerb) {
514-
SkPathBuilder path;
510+
SkPath path;
515511

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});
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);
521517
path.close();
522518

523-
TestPathDispatchOneOfEachVerb(DlPath(path.detach()));
519+
TestPathDispatchOneOfEachVerb(DlPath(path));
524520
}
525521

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

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

572568
static void TestImpellerPathDispatchConicToQuads(
@@ -666,12 +662,12 @@ static void TestPathDispatchUnclosedTriangle(const DlPath& path) {
666662
}
667663

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

674-
TestPathDispatchUnclosedTriangle(DlPath(sk_path.detach()));
670+
TestPathDispatchUnclosedTriangle(DlPath(sk_path));
675671
}
676672

677673
TEST(DisplayListPath, DispatchUnclosedImpellerTriangle) {
@@ -700,13 +696,13 @@ static void TestPathDispatchClosedTriangle(const DlPath& path) {
700696
}
701697

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

709-
TestPathDispatchClosedTriangle(DlPath(sk_path.detach()));
705+
TestPathDispatchClosedTriangle(DlPath(sk_path));
710706
}
711707

712708
TEST(DisplayListPath, DispatchClosedPathBuilderTriangle) {
@@ -742,19 +738,19 @@ static void TestPathDispatchMixedCloseTriangles(const DlPath& path) {
742738
}
743739

744740
TEST(DisplayListPath, DispatchMixedCloseSkiaPath) {
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});
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);
752748
sk_path.close();
753-
sk_path.moveTo({210, 10});
754-
sk_path.lineTo({220, 10});
755-
sk_path.lineTo({210, 20});
749+
sk_path.moveTo(210, 10);
750+
sk_path.lineTo(220, 10);
751+
sk_path.lineTo(210, 20);
756752

757-
TestPathDispatchMixedCloseTriangles(DlPath(sk_path.detach()));
753+
TestPathDispatchMixedCloseTriangles(DlPath(sk_path));
758754
}
759755

760756
TEST(DisplayListPath, DispatchMixedCloseImpellerPath) {
@@ -793,15 +789,15 @@ static void TestPathDispatchImplicitMoveAfterClose(const DlPath& path) {
793789
}
794790

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

804-
TestPathDispatchImplicitMoveAfterClose(DlPath(sk_path.detach()));
800+
TestPathDispatchImplicitMoveAfterClose(DlPath(sk_path));
805801
}
806802

807803
TEST(DisplayListPath, DispatchImplicitMoveAfterClosePathBuilder) {
@@ -821,25 +817,25 @@ TEST(DisplayListPath, DispatchImplicitMoveAfterClosePathBuilder) {
821817
// supported by either Flutter public APIs or Impeller
822818

823819
TEST(DisplayListPath, CannotConstructFromSkiaInverseWinding) {
824-
SkPathBuilder sk_path(SkPathFillType::kInverseWinding);
825-
sk_path.moveTo({0, 0});
826-
sk_path.lineTo({100, 0});
827-
sk_path.lineTo({0, 100});
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);
828825
sk_path.close();
829826

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

834830
TEST(DisplayListPath, CannotConstructFromSkiaInverseEvenOdd) {
835-
SkPathBuilder sk_path(SkPathFillType::kInverseEvenOdd);
836-
sk_path.moveTo({0, 0});
837-
sk_path.lineTo({100, 0});
838-
sk_path.lineTo({0, 100});
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);
839836
sk_path.close();
840837

841-
EXPECT_DEATH_IF_SUPPORTED(new DlPath(sk_path.detach()),
842-
"SkPathFillType_IsInverse");
838+
EXPECT_DEATH_IF_SUPPORTED(new DlPath(sk_path), "SkPathFillType_IsInverse");
843839
}
844840
#endif
845841

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

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

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

7-
#include "third_party/skia/include/core/SkRect.h"
8-
97
namespace impeller::interop {
108

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

1311
Path::~Path() = default;
1412

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

1917
ImpellerRect Path::GetBounds() const {
20-
const auto bounds = path_.computeFiniteBounds().value_or(SkRect());
18+
const auto bounds = path_.getBounds();
2119
return ImpellerRect{
2220
.x = bounds.x(),
2321
.y = bounds.y(),

0 commit comments

Comments
 (0)