Skip to content

Commit 4f91425

Browse files
authored
Revert "Set SkPath::setIsVolatile based on whether the path survives at least two frames (flutter#22620)" (flutter#23044)
This reverts commit 2efc7c1.
1 parent 21691f1 commit 4f91425

27 files changed

+121
-593
lines changed

ci/licenses_golden/licenses_flutter

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,6 @@ FILE: ../../../flutter/lib/ui/painting/path.cc
361361
FILE: ../../../flutter/lib/ui/painting/path.h
362362
FILE: ../../../flutter/lib/ui/painting/path_measure.cc
363363
FILE: ../../../flutter/lib/ui/painting/path_measure.h
364-
FILE: ../../../flutter/lib/ui/painting/path_unittests.cc
365364
FILE: ../../../flutter/lib/ui/painting/picture.cc
366365
FILE: ../../../flutter/lib/ui/painting/picture.h
367366
FILE: ../../../flutter/lib/ui/painting/picture_recorder.cc
@@ -405,8 +404,6 @@ FILE: ../../../flutter/lib/ui/ui.dart
405404
FILE: ../../../flutter/lib/ui/ui_benchmarks.cc
406405
FILE: ../../../flutter/lib/ui/ui_dart_state.cc
407406
FILE: ../../../flutter/lib/ui/ui_dart_state.h
408-
FILE: ../../../flutter/lib/ui/volatile_path_tracker.cc
409-
FILE: ../../../flutter/lib/ui/volatile_path_tracker.h
410407
FILE: ../../../flutter/lib/ui/window.dart
411408
FILE: ../../../flutter/lib/ui/window/platform_configuration.cc
412409
FILE: ../../../flutter/lib/ui/window/platform_configuration.h

lib/ui/BUILD.gn

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,6 @@ source_set("ui") {
9191
"text/text_box.h",
9292
"ui_dart_state.cc",
9393
"ui_dart_state.h",
94-
"volatile_path_tracker.cc",
95-
"volatile_path_tracker.h",
9694
"window/platform_configuration.cc",
9795
"window/platform_configuration.h",
9896
"window/platform_message.cc",
@@ -190,7 +188,6 @@ if (enable_unittests) {
190188
sources = [
191189
"painting/image_dispose_unittests.cc",
192190
"painting/image_encoding_unittests.cc",
193-
"painting/path_unittests.cc",
194191
"painting/vertices_unittests.cc",
195192
"window/platform_configuration_unittests.cc",
196193
"window/pointer_data_packet_converter_unittests.cc",

lib/ui/fixtures/ui_test.dart

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,8 @@ void createVertices() {
3535
);
3636
_validateVertices(vertices);
3737
}
38-
void _validateVertices(Vertices vertices) native 'ValidateVertices';
3938

40-
@pragma('vm:entry-point')
41-
void createPath() {
42-
final Path path = Path()..lineTo(10, 10);
43-
_validatePath(path);
44-
// Arbitrarily hold a reference to the path to make sure it does not get
45-
// garbage collected.
46-
Future<void>.delayed(const Duration(days: 100)).then((_) {
47-
path.lineTo(100, 100);
48-
});
49-
}
50-
void _validatePath(Path path) native 'ValidatePath';
39+
void _validateVertices(Vertices vertices) native 'ValidateVertices';
5140

5241
@pragma('vm:entry-point')
5342
void frameCallback(FrameInfo info) {

lib/ui/painting/path.cc

Lines changed: 41 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -67,69 +67,43 @@ void CanvasPath::RegisterNatives(tonic::DartLibraryNatives* natives) {
6767
FOR_EACH_BINDING(DART_REGISTER_NATIVE)});
6868
}
6969

70-
CanvasPath::CanvasPath()
71-
: path_tracker_(UIDartState::Current()->GetVolatilePathTracker()),
72-
tracked_path_(std::make_shared<VolatilePathTracker::TrackedPath>()) {
73-
FML_DCHECK(path_tracker_);
74-
resetVolatility();
75-
}
76-
77-
CanvasPath::~CanvasPath() = default;
78-
79-
void CanvasPath::resetVolatility() {
80-
if (!tracked_path_->tracking_volatility) {
81-
mutable_path().setIsVolatile(true);
82-
tracked_path_->frame_count = 0;
83-
tracked_path_->tracking_volatility = true;
84-
path_tracker_->Insert(tracked_path_);
85-
}
86-
}
70+
CanvasPath::CanvasPath() {}
8771

88-
void CanvasPath::ReleaseDartWrappableReference() const {
89-
FML_DCHECK(path_tracker_);
90-
path_tracker_->Erase(tracked_path_);
91-
}
72+
CanvasPath::~CanvasPath() {}
9273

9374
int CanvasPath::getFillType() {
94-
return static_cast<int>(path().getFillType());
75+
return static_cast<int>(path_.getFillType());
9576
}
9677

9778
void CanvasPath::setFillType(int fill_type) {
98-
mutable_path().setFillType(static_cast<SkPathFillType>(fill_type));
99-
resetVolatility();
79+
path_.setFillType(static_cast<SkPathFillType>(fill_type));
10080
}
10181

10282
void CanvasPath::moveTo(float x, float y) {
103-
mutable_path().moveTo(x, y);
104-
resetVolatility();
83+
path_.moveTo(x, y);
10584
}
10685

10786
void CanvasPath::relativeMoveTo(float x, float y) {
108-
mutable_path().rMoveTo(x, y);
109-
resetVolatility();
87+
path_.rMoveTo(x, y);
11088
}
11189

11290
void CanvasPath::lineTo(float x, float y) {
113-
mutable_path().lineTo(x, y);
114-
resetVolatility();
91+
path_.lineTo(x, y);
11592
}
11693

11794
void CanvasPath::relativeLineTo(float x, float y) {
118-
mutable_path().rLineTo(x, y);
119-
resetVolatility();
95+
path_.rLineTo(x, y);
12096
}
12197

12298
void CanvasPath::quadraticBezierTo(float x1, float y1, float x2, float y2) {
123-
mutable_path().quadTo(x1, y1, x2, y2);
124-
resetVolatility();
99+
path_.quadTo(x1, y1, x2, y2);
125100
}
126101

127102
void CanvasPath::relativeQuadraticBezierTo(float x1,
128103
float y1,
129104
float x2,
130105
float y2) {
131-
mutable_path().rQuadTo(x1, y1, x2, y2);
132-
resetVolatility();
106+
path_.rQuadTo(x1, y1, x2, y2);
133107
}
134108

135109
void CanvasPath::cubicTo(float x1,
@@ -138,8 +112,7 @@ void CanvasPath::cubicTo(float x1,
138112
float y2,
139113
float x3,
140114
float y3) {
141-
mutable_path().cubicTo(x1, y1, x2, y2, x3, y3);
142-
resetVolatility();
115+
path_.cubicTo(x1, y1, x2, y2, x3, y3);
143116
}
144117

145118
void CanvasPath::relativeCubicTo(float x1,
@@ -148,22 +121,19 @@ void CanvasPath::relativeCubicTo(float x1,
148121
float y2,
149122
float x3,
150123
float y3) {
151-
mutable_path().rCubicTo(x1, y1, x2, y2, x3, y3);
152-
resetVolatility();
124+
path_.rCubicTo(x1, y1, x2, y2, x3, y3);
153125
}
154126

155127
void CanvasPath::conicTo(float x1, float y1, float x2, float y2, float w) {
156-
mutable_path().conicTo(x1, y1, x2, y2, w);
157-
resetVolatility();
128+
path_.conicTo(x1, y1, x2, y2, w);
158129
}
159130

160131
void CanvasPath::relativeConicTo(float x1,
161132
float y1,
162133
float x2,
163134
float y2,
164135
float w) {
165-
mutable_path().rConicTo(x1, y1, x2, y2, w);
166-
resetVolatility();
136+
path_.rConicTo(x1, y1, x2, y2, w);
167137
}
168138

169139
void CanvasPath::arcTo(float left,
@@ -173,10 +143,9 @@ void CanvasPath::arcTo(float left,
173143
float startAngle,
174144
float sweepAngle,
175145
bool forceMoveTo) {
176-
mutable_path().arcTo(SkRect::MakeLTRB(left, top, right, bottom),
177-
startAngle * 180.0 / M_PI, sweepAngle * 180.0 / M_PI,
178-
forceMoveTo);
179-
resetVolatility();
146+
path_.arcTo(SkRect::MakeLTRB(left, top, right, bottom),
147+
startAngle * 180.0 / M_PI, sweepAngle * 180.0 / M_PI,
148+
forceMoveTo);
180149
}
181150

182151
void CanvasPath::arcToPoint(float arcEndX,
@@ -191,9 +160,8 @@ void CanvasPath::arcToPoint(float arcEndX,
191160
const auto direction =
192161
isClockwiseDirection ? SkPathDirection::kCW : SkPathDirection::kCCW;
193162

194-
mutable_path().arcTo(radiusX, radiusY, xAxisRotation, arcSize, direction,
195-
arcEndX, arcEndY);
196-
resetVolatility();
163+
path_.arcTo(radiusX, radiusY, xAxisRotation, arcSize, direction, arcEndX,
164+
arcEndY);
197165
}
198166

199167
void CanvasPath::relativeArcToPoint(float arcEndDeltaX,
@@ -207,19 +175,16 @@ void CanvasPath::relativeArcToPoint(float arcEndDeltaX,
207175
: SkPath::ArcSize::kSmall_ArcSize;
208176
const auto direction =
209177
isClockwiseDirection ? SkPathDirection::kCW : SkPathDirection::kCCW;
210-
mutable_path().rArcTo(radiusX, radiusY, xAxisRotation, arcSize, direction,
211-
arcEndDeltaX, arcEndDeltaY);
212-
resetVolatility();
178+
path_.rArcTo(radiusX, radiusY, xAxisRotation, arcSize, direction,
179+
arcEndDeltaX, arcEndDeltaY);
213180
}
214181

215182
void CanvasPath::addRect(float left, float top, float right, float bottom) {
216-
mutable_path().addRect(SkRect::MakeLTRB(left, top, right, bottom));
217-
resetVolatility();
183+
path_.addRect(SkRect::MakeLTRB(left, top, right, bottom));
218184
}
219185

220186
void CanvasPath::addOval(float left, float top, float right, float bottom) {
221-
mutable_path().addOval(SkRect::MakeLTRB(left, top, right, bottom));
222-
resetVolatility();
187+
path_.addOval(SkRect::MakeLTRB(left, top, right, bottom));
223188
}
224189

225190
void CanvasPath::addArc(float left,
@@ -228,29 +193,25 @@ void CanvasPath::addArc(float left,
228193
float bottom,
229194
float startAngle,
230195
float sweepAngle) {
231-
mutable_path().addArc(SkRect::MakeLTRB(left, top, right, bottom),
232-
startAngle * 180.0 / M_PI, sweepAngle * 180.0 / M_PI);
233-
resetVolatility();
196+
path_.addArc(SkRect::MakeLTRB(left, top, right, bottom),
197+
startAngle * 180.0 / M_PI, sweepAngle * 180.0 / M_PI);
234198
}
235199

236200
void CanvasPath::addPolygon(const tonic::Float32List& points, bool close) {
237-
mutable_path().addPoly(reinterpret_cast<const SkPoint*>(points.data()),
238-
points.num_elements() / 2, close);
239-
resetVolatility();
201+
path_.addPoly(reinterpret_cast<const SkPoint*>(points.data()),
202+
points.num_elements() / 2, close);
240203
}
241204

242205
void CanvasPath::addRRect(const RRect& rrect) {
243-
mutable_path().addRRect(rrect.sk_rrect);
244-
resetVolatility();
206+
path_.addRRect(rrect.sk_rrect);
245207
}
246208

247209
void CanvasPath::addPath(CanvasPath* path, double dx, double dy) {
248210
if (!path) {
249211
Dart_ThrowException(ToDart("Path.addPath called with non-genuine Path."));
250212
return;
251213
}
252-
mutable_path().addPath(path->path(), dx, dy, SkPath::kAppend_AddPathMode);
253-
resetVolatility();
214+
path_.addPath(path->path(), dx, dy, SkPath::kAppend_AddPathMode);
254215
}
255216

256217
void CanvasPath::addPathWithMatrix(CanvasPath* path,
@@ -266,9 +227,8 @@ void CanvasPath::addPathWithMatrix(CanvasPath* path,
266227
SkMatrix matrix = ToSkMatrix(matrix4);
267228
matrix.setTranslateX(matrix.getTranslateX() + dx);
268229
matrix.setTranslateY(matrix.getTranslateY() + dy);
269-
mutable_path().addPath(path->path(), matrix, SkPath::kAppend_AddPathMode);
230+
path_.addPath(path->path(), matrix, SkPath::kAppend_AddPathMode);
270231
matrix4.Release();
271-
resetVolatility();
272232
}
273233

274234
void CanvasPath::extendWithPath(CanvasPath* path, double dx, double dy) {
@@ -277,8 +237,7 @@ void CanvasPath::extendWithPath(CanvasPath* path, double dx, double dy) {
277237
ToDart("Path.extendWithPath called with non-genuine Path."));
278238
return;
279239
}
280-
mutable_path().addPath(path->path(), dx, dy, SkPath::kExtend_AddPathMode);
281-
resetVolatility();
240+
path_.addPath(path->path(), dx, dy, SkPath::kExtend_AddPathMode);
282241
}
283242

284243
void CanvasPath::extendWithPathAndMatrix(CanvasPath* path,
@@ -294,43 +253,37 @@ void CanvasPath::extendWithPathAndMatrix(CanvasPath* path,
294253
SkMatrix matrix = ToSkMatrix(matrix4);
295254
matrix.setTranslateX(matrix.getTranslateX() + dx);
296255
matrix.setTranslateY(matrix.getTranslateY() + dy);
297-
mutable_path().addPath(path->path(), matrix, SkPath::kExtend_AddPathMode);
256+
path_.addPath(path->path(), matrix, SkPath::kExtend_AddPathMode);
298257
matrix4.Release();
299-
resetVolatility();
300258
}
301259

302260
void CanvasPath::close() {
303-
mutable_path().close();
304-
resetVolatility();
261+
path_.close();
305262
}
306263

307264
void CanvasPath::reset() {
308-
mutable_path().reset();
309-
resetVolatility();
265+
path_.reset();
310266
}
311267

312268
bool CanvasPath::contains(double x, double y) {
313-
return path().contains(x, y);
269+
return path_.contains(x, y);
314270
}
315271

316272
void CanvasPath::shift(Dart_Handle path_handle, double dx, double dy) {
317273
fml::RefPtr<CanvasPath> path = CanvasPath::Create(path_handle);
318-
auto& other_mutable_path = path->mutable_path();
319-
mutable_path().offset(dx, dy, &other_mutable_path);
320-
resetVolatility();
274+
path_.offset(dx, dy, &path->path_);
321275
}
322276

323277
void CanvasPath::transform(Dart_Handle path_handle,
324278
tonic::Float64List& matrix4) {
325279
fml::RefPtr<CanvasPath> path = CanvasPath::Create(path_handle);
326-
auto& other_mutable_path = path->mutable_path();
327-
mutable_path().transform(ToSkMatrix(matrix4), &other_mutable_path);
280+
path_.transform(ToSkMatrix(matrix4), &path->path_);
328281
matrix4.Release();
329282
}
330283

331284
tonic::Float32List CanvasPath::getBounds() {
332285
tonic::Float32List rect(Dart_NewTypedData(Dart_TypedData_kFloat32, 4));
333-
const SkRect& bounds = path().getBounds();
286+
const SkRect& bounds = path_.getBounds();
334287
rect[0] = bounds.left();
335288
rect[1] = bounds.top();
336289
rect[2] = bounds.right();
@@ -340,22 +293,21 @@ tonic::Float32List CanvasPath::getBounds() {
340293

341294
bool CanvasPath::op(CanvasPath* path1, CanvasPath* path2, int operation) {
342295
return Op(path1->path(), path2->path(), static_cast<SkPathOp>(operation),
343-
&tracked_path_->path);
344-
resetVolatility();
296+
&path_);
345297
}
346298

347299
void CanvasPath::clone(Dart_Handle path_handle) {
348300
fml::RefPtr<CanvasPath> path = CanvasPath::Create(path_handle);
349301
// per Skia docs, this will create a fast copy
350302
// data is shared until the source path or dest path are mutated
351-
path->mutable_path() = this->path();
303+
path->path_ = path_;
352304
}
353305

354306
// This is doomed to be called too early, since Paths are mutable.
355307
// However, it can help for some of the clone/shift/transform type methods
356308
// where the resultant path will initially have a meaningful size.
357309
size_t CanvasPath::GetAllocationSize() const {
358-
return sizeof(CanvasPath) + path().approximateBytesUsed();
310+
return sizeof(CanvasPath) + path_.approximateBytesUsed();
359311
}
360312

361313
} // namespace flutter

lib/ui/painting/path.h

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

88
#include "flutter/lib/ui/dart_wrapper.h"
99
#include "flutter/lib/ui/painting/rrect.h"
10-
#include "flutter/lib/ui/volatile_path_tracker.h"
1110
#include "third_party/skia/include/core/SkPath.h"
1211
#include "third_party/skia/include/pathops/SkPathOps.h"
1312
#include "third_party/tonic/typed_data/typed_list.h"
@@ -37,7 +36,7 @@ class CanvasPath : public RefCountedDartWrappable<CanvasPath> {
3736
static fml::RefPtr<CanvasPath> CreateFrom(Dart_Handle path_handle,
3837
const SkPath& src) {
3938
fml::RefPtr<CanvasPath> path = CanvasPath::Create(path_handle);
40-
path->tracked_path_->path = src;
39+
path->path_ = src;
4140
return path;
4241
}
4342

@@ -109,24 +108,16 @@ class CanvasPath : public RefCountedDartWrappable<CanvasPath> {
109108
bool op(CanvasPath* path1, CanvasPath* path2, int operation);
110109
void clone(Dart_Handle path_handle);
111110

112-
const SkPath& path() const { return tracked_path_->path; }
111+
const SkPath& path() const { return path_; }
113112

114113
size_t GetAllocationSize() const override;
115114

116115
static void RegisterNatives(tonic::DartLibraryNatives* natives);
117116

118-
virtual void ReleaseDartWrappableReference() const override;
119-
120117
private:
121118
CanvasPath();
122119

123-
std::shared_ptr<VolatilePathTracker> path_tracker_;
124-
std::shared_ptr<VolatilePathTracker::TrackedPath> tracked_path_;
125-
126-
// Must be called whenever the path is created or mutated.
127-
void resetVolatility();
128-
129-
SkPath& mutable_path() { return tracked_path_->path; }
120+
SkPath path_;
130121
};
131122

132123
} // namespace flutter

0 commit comments

Comments
 (0)