diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 2cf7f3024104034d93c4abc1b5a171662a597032..02fd4f5879677c09c44210ac0adbcecc73cb7643 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -359,6 +359,7 @@ FILE: ../../../flutter/lib/ui/painting/path.cc FILE: ../../../flutter/lib/ui/painting/path.h FILE: ../../../flutter/lib/ui/painting/path_measure.cc FILE: ../../../flutter/lib/ui/painting/path_measure.h +FILE: ../../../flutter/lib/ui/painting/path_unittests.cc FILE: ../../../flutter/lib/ui/painting/picture.cc FILE: ../../../flutter/lib/ui/painting/picture.h FILE: ../../../flutter/lib/ui/painting/picture_recorder.cc @@ -402,6 +403,8 @@ FILE: ../../../flutter/lib/ui/ui.dart FILE: ../../../flutter/lib/ui/ui_benchmarks.cc FILE: ../../../flutter/lib/ui/ui_dart_state.cc FILE: ../../../flutter/lib/ui/ui_dart_state.h +FILE: ../../../flutter/lib/ui/volatile_path_tracker.cc +FILE: ../../../flutter/lib/ui/volatile_path_tracker.h FILE: ../../../flutter/lib/ui/window.dart FILE: ../../../flutter/lib/ui/window/platform_configuration.cc FILE: ../../../flutter/lib/ui/window/platform_configuration.h diff --git a/fml/trace_event.h b/fml/trace_event.h index ae80298cfd69985d78339732386aa806a44fabf4..3928d68c7d24c7b089f8426db8d78cd211170a7b 100644 --- a/fml/trace_event.h +++ b/fml/trace_event.h @@ -68,6 +68,19 @@ ::fml::tracing::TraceCounter((category_group), (name), (counter_id), (arg1), \ __VA_ARGS__); +// Avoid using the same `name` and `argX_name` for nested traces, which can +// lead to double free errors. E.g. the following code should be avoided: +// +// ```cpp +// { +// TRACE_EVENT1("flutter", "Foo::Bar", "count", "initial_count_value"); +// ... +// TRACE_EVENT_INSTANT1("flutter", "Foo::Bar", +// "count", "updated_count_value"); +// } +// ``` +// +// Instead, either use different `name` or `arg1` parameter names. #define FML_TRACE_EVENT(category_group, name, ...) \ ::fml::tracing::TraceEvent((category_group), (name), __VA_ARGS__); \ __FML__AUTO_TRACE_END(name) diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index 81c115454ea2b48ba587eda8bb0ee630102fd121..2f9efe9e9acfec3bf43cb706e5a0538915dc138b 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -91,6 +91,8 @@ source_set("ui") { "text/text_box.h", "ui_dart_state.cc", "ui_dart_state.h", + "volatile_path_tracker.cc", + "volatile_path_tracker.h", "window/platform_configuration.cc", "window/platform_configuration.h", "window/platform_message.cc", @@ -191,6 +193,7 @@ if (enable_unittests) { sources = [ "painting/image_dispose_unittests.cc", "painting/image_encoding_unittests.cc", + "painting/path_unittests.cc", "painting/vertices_unittests.cc", "window/platform_configuration_unittests.cc", "window/pointer_data_packet_converter_unittests.cc", diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index 7fd54da91b9f236fb7b3ab560acb4d2c82d557dc..9e274e94eb70ea2ddfefe912fa9a35ab0f7f1eaa 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -37,6 +37,18 @@ void createVertices() { } void _validateVertices(Vertices vertices) native 'ValidateVertices'; +@pragma('vm:entry-point') +void createPath() { + final Path path = Path()..lineTo(10, 10); + _validatePath(path); + // Arbitrarily hold a reference to the path to make sure it does not get + // garbage collected. + Future.delayed(const Duration(days: 100)).then((_) { + path.lineTo(100, 100); + }); +} +void _validatePath(Path path) native 'ValidatePath'; + @pragma('vm:entry-point') void frameCallback(FrameInfo info) { print('called back'); diff --git a/lib/ui/painting/path.cc b/lib/ui/painting/path.cc index d0898a4ca283c7e9572e9a11d4d7c346a18f4aa0..650aa231f8addea7e423f91c1ce94976550f0d61 100644 --- a/lib/ui/painting/path.cc +++ b/lib/ui/painting/path.cc @@ -67,43 +67,70 @@ void CanvasPath::RegisterNatives(tonic::DartLibraryNatives* natives) { FOR_EACH_BINDING(DART_REGISTER_NATIVE)}); } -CanvasPath::CanvasPath() {} +CanvasPath::CanvasPath() + : path_tracker_(UIDartState::Current()->GetVolatilePathTracker()), + tracked_path_(std::make_shared()) { + FML_DCHECK(path_tracker_); + resetVolatility(); +} + +CanvasPath::~CanvasPath() = default; + +void CanvasPath::resetVolatility() { + if (!tracked_path_->tracking_volatility) { + mutable_path().setIsVolatile(true); + tracked_path_->frame_count = 0; + tracked_path_->tracking_volatility = true; + path_tracker_->Insert(tracked_path_); + } +} -CanvasPath::~CanvasPath() {} +void CanvasPath::ReleaseDartWrappableReference() const { + FML_DCHECK(path_tracker_); + path_tracker_->Erase(tracked_path_); + RefCountedDartWrappable::ReleaseDartWrappableReference(); +} int CanvasPath::getFillType() { - return static_cast(path_.getFillType()); + return static_cast(path().getFillType()); } void CanvasPath::setFillType(int fill_type) { - path_.setFillType(static_cast(fill_type)); + mutable_path().setFillType(static_cast(fill_type)); + resetVolatility(); } void CanvasPath::moveTo(float x, float y) { - path_.moveTo(x, y); + mutable_path().moveTo(x, y); + resetVolatility(); } void CanvasPath::relativeMoveTo(float x, float y) { - path_.rMoveTo(x, y); + mutable_path().rMoveTo(x, y); + resetVolatility(); } void CanvasPath::lineTo(float x, float y) { - path_.lineTo(x, y); + mutable_path().lineTo(x, y); + resetVolatility(); } void CanvasPath::relativeLineTo(float x, float y) { - path_.rLineTo(x, y); + mutable_path().rLineTo(x, y); + resetVolatility(); } void CanvasPath::quadraticBezierTo(float x1, float y1, float x2, float y2) { - path_.quadTo(x1, y1, x2, y2); + mutable_path().quadTo(x1, y1, x2, y2); + resetVolatility(); } void CanvasPath::relativeQuadraticBezierTo(float x1, float y1, float x2, float y2) { - path_.rQuadTo(x1, y1, x2, y2); + mutable_path().rQuadTo(x1, y1, x2, y2); + resetVolatility(); } void CanvasPath::cubicTo(float x1, @@ -112,7 +139,8 @@ void CanvasPath::cubicTo(float x1, float y2, float x3, float y3) { - path_.cubicTo(x1, y1, x2, y2, x3, y3); + mutable_path().cubicTo(x1, y1, x2, y2, x3, y3); + resetVolatility(); } void CanvasPath::relativeCubicTo(float x1, @@ -121,11 +149,13 @@ void CanvasPath::relativeCubicTo(float x1, float y2, float x3, float y3) { - path_.rCubicTo(x1, y1, x2, y2, x3, y3); + mutable_path().rCubicTo(x1, y1, x2, y2, x3, y3); + resetVolatility(); } void CanvasPath::conicTo(float x1, float y1, float x2, float y2, float w) { - path_.conicTo(x1, y1, x2, y2, w); + mutable_path().conicTo(x1, y1, x2, y2, w); + resetVolatility(); } void CanvasPath::relativeConicTo(float x1, @@ -133,7 +163,8 @@ void CanvasPath::relativeConicTo(float x1, float x2, float y2, float w) { - path_.rConicTo(x1, y1, x2, y2, w); + mutable_path().rConicTo(x1, y1, x2, y2, w); + resetVolatility(); } void CanvasPath::arcTo(float left, @@ -143,9 +174,10 @@ void CanvasPath::arcTo(float left, float startAngle, float sweepAngle, bool forceMoveTo) { - path_.arcTo(SkRect::MakeLTRB(left, top, right, bottom), - startAngle * 180.0 / M_PI, sweepAngle * 180.0 / M_PI, - forceMoveTo); + mutable_path().arcTo(SkRect::MakeLTRB(left, top, right, bottom), + startAngle * 180.0 / M_PI, sweepAngle * 180.0 / M_PI, + forceMoveTo); + resetVolatility(); } void CanvasPath::arcToPoint(float arcEndX, @@ -160,8 +192,9 @@ void CanvasPath::arcToPoint(float arcEndX, const auto direction = isClockwiseDirection ? SkPathDirection::kCW : SkPathDirection::kCCW; - path_.arcTo(radiusX, radiusY, xAxisRotation, arcSize, direction, arcEndX, - arcEndY); + mutable_path().arcTo(radiusX, radiusY, xAxisRotation, arcSize, direction, + arcEndX, arcEndY); + resetVolatility(); } void CanvasPath::relativeArcToPoint(float arcEndDeltaX, @@ -175,16 +208,19 @@ void CanvasPath::relativeArcToPoint(float arcEndDeltaX, : SkPath::ArcSize::kSmall_ArcSize; const auto direction = isClockwiseDirection ? SkPathDirection::kCW : SkPathDirection::kCCW; - path_.rArcTo(radiusX, radiusY, xAxisRotation, arcSize, direction, - arcEndDeltaX, arcEndDeltaY); + mutable_path().rArcTo(radiusX, radiusY, xAxisRotation, arcSize, direction, + arcEndDeltaX, arcEndDeltaY); + resetVolatility(); } void CanvasPath::addRect(float left, float top, float right, float bottom) { - path_.addRect(SkRect::MakeLTRB(left, top, right, bottom)); + mutable_path().addRect(SkRect::MakeLTRB(left, top, right, bottom)); + resetVolatility(); } void CanvasPath::addOval(float left, float top, float right, float bottom) { - path_.addOval(SkRect::MakeLTRB(left, top, right, bottom)); + mutable_path().addOval(SkRect::MakeLTRB(left, top, right, bottom)); + resetVolatility(); } void CanvasPath::addArc(float left, @@ -193,17 +229,20 @@ void CanvasPath::addArc(float left, float bottom, float startAngle, float sweepAngle) { - path_.addArc(SkRect::MakeLTRB(left, top, right, bottom), - startAngle * 180.0 / M_PI, sweepAngle * 180.0 / M_PI); + mutable_path().addArc(SkRect::MakeLTRB(left, top, right, bottom), + startAngle * 180.0 / M_PI, sweepAngle * 180.0 / M_PI); + resetVolatility(); } void CanvasPath::addPolygon(const tonic::Float32List& points, bool close) { - path_.addPoly(reinterpret_cast(points.data()), - points.num_elements() / 2, close); + mutable_path().addPoly(reinterpret_cast(points.data()), + points.num_elements() / 2, close); + resetVolatility(); } void CanvasPath::addRRect(const RRect& rrect) { - path_.addRRect(rrect.sk_rrect); + mutable_path().addRRect(rrect.sk_rrect); + resetVolatility(); } void CanvasPath::addPath(CanvasPath* path, double dx, double dy) { @@ -211,7 +250,8 @@ void CanvasPath::addPath(CanvasPath* path, double dx, double dy) { Dart_ThrowException(ToDart("Path.addPath called with non-genuine Path.")); return; } - path_.addPath(path->path(), dx, dy, SkPath::kAppend_AddPathMode); + mutable_path().addPath(path->path(), dx, dy, SkPath::kAppend_AddPathMode); + resetVolatility(); } void CanvasPath::addPathWithMatrix(CanvasPath* path, @@ -227,8 +267,9 @@ void CanvasPath::addPathWithMatrix(CanvasPath* path, SkMatrix matrix = ToSkMatrix(matrix4); matrix.setTranslateX(matrix.getTranslateX() + dx); matrix.setTranslateY(matrix.getTranslateY() + dy); - path_.addPath(path->path(), matrix, SkPath::kAppend_AddPathMode); + mutable_path().addPath(path->path(), matrix, SkPath::kAppend_AddPathMode); matrix4.Release(); + resetVolatility(); } void CanvasPath::extendWithPath(CanvasPath* path, double dx, double dy) { @@ -237,7 +278,8 @@ void CanvasPath::extendWithPath(CanvasPath* path, double dx, double dy) { ToDart("Path.extendWithPath called with non-genuine Path.")); return; } - path_.addPath(path->path(), dx, dy, SkPath::kExtend_AddPathMode); + mutable_path().addPath(path->path(), dx, dy, SkPath::kExtend_AddPathMode); + resetVolatility(); } void CanvasPath::extendWithPathAndMatrix(CanvasPath* path, @@ -253,37 +295,43 @@ void CanvasPath::extendWithPathAndMatrix(CanvasPath* path, SkMatrix matrix = ToSkMatrix(matrix4); matrix.setTranslateX(matrix.getTranslateX() + dx); matrix.setTranslateY(matrix.getTranslateY() + dy); - path_.addPath(path->path(), matrix, SkPath::kExtend_AddPathMode); + mutable_path().addPath(path->path(), matrix, SkPath::kExtend_AddPathMode); matrix4.Release(); + resetVolatility(); } void CanvasPath::close() { - path_.close(); + mutable_path().close(); + resetVolatility(); } void CanvasPath::reset() { - path_.reset(); + mutable_path().reset(); + resetVolatility(); } bool CanvasPath::contains(double x, double y) { - return path_.contains(x, y); + return path().contains(x, y); } void CanvasPath::shift(Dart_Handle path_handle, double dx, double dy) { fml::RefPtr path = CanvasPath::Create(path_handle); - path_.offset(dx, dy, &path->path_); + auto& other_mutable_path = path->mutable_path(); + mutable_path().offset(dx, dy, &other_mutable_path); + resetVolatility(); } void CanvasPath::transform(Dart_Handle path_handle, tonic::Float64List& matrix4) { fml::RefPtr path = CanvasPath::Create(path_handle); - path_.transform(ToSkMatrix(matrix4), &path->path_); + auto& other_mutable_path = path->mutable_path(); + mutable_path().transform(ToSkMatrix(matrix4), &other_mutable_path); matrix4.Release(); } tonic::Float32List CanvasPath::getBounds() { tonic::Float32List rect(Dart_NewTypedData(Dart_TypedData_kFloat32, 4)); - const SkRect& bounds = path_.getBounds(); + const SkRect& bounds = path().getBounds(); rect[0] = bounds.left(); rect[1] = bounds.top(); rect[2] = bounds.right(); @@ -293,21 +341,22 @@ tonic::Float32List CanvasPath::getBounds() { bool CanvasPath::op(CanvasPath* path1, CanvasPath* path2, int operation) { return Op(path1->path(), path2->path(), static_cast(operation), - &path_); + &tracked_path_->path); + resetVolatility(); } void CanvasPath::clone(Dart_Handle path_handle) { fml::RefPtr path = CanvasPath::Create(path_handle); // per Skia docs, this will create a fast copy // data is shared until the source path or dest path are mutated - path->path_ = path_; + path->mutable_path() = this->path(); } // This is doomed to be called too early, since Paths are mutable. // However, it can help for some of the clone/shift/transform type methods // where the resultant path will initially have a meaningful size. size_t CanvasPath::GetAllocationSize() const { - return sizeof(CanvasPath) + path_.approximateBytesUsed(); + return sizeof(CanvasPath) + path().approximateBytesUsed(); } } // namespace flutter diff --git a/lib/ui/painting/path.h b/lib/ui/painting/path.h index dbd6d7a0d1cc2c20e5e52fd502cde852255db2d6..3c05cdd140837e45026f87c499879819ab8cc4d7 100644 --- a/lib/ui/painting/path.h +++ b/lib/ui/painting/path.h @@ -7,6 +7,7 @@ #include "flutter/lib/ui/dart_wrapper.h" #include "flutter/lib/ui/painting/rrect.h" +#include "flutter/lib/ui/volatile_path_tracker.h" #include "third_party/skia/include/core/SkPath.h" #include "third_party/skia/include/pathops/SkPathOps.h" #include "third_party/tonic/typed_data/typed_list.h" @@ -36,7 +37,7 @@ class CanvasPath : public RefCountedDartWrappable { static fml::RefPtr CreateFrom(Dart_Handle path_handle, const SkPath& src) { fml::RefPtr path = CanvasPath::Create(path_handle); - path->path_ = src; + path->tracked_path_->path = src; return path; } @@ -108,16 +109,24 @@ class CanvasPath : public RefCountedDartWrappable { bool op(CanvasPath* path1, CanvasPath* path2, int operation); void clone(Dart_Handle path_handle); - const SkPath& path() const { return path_; } + const SkPath& path() const { return tracked_path_->path; } size_t GetAllocationSize() const override; static void RegisterNatives(tonic::DartLibraryNatives* natives); + virtual void ReleaseDartWrappableReference() const override; + private: CanvasPath(); - SkPath path_; + std::shared_ptr path_tracker_; + std::shared_ptr tracked_path_; + + // Must be called whenever the path is created or mutated. + void resetVolatility(); + + SkPath& mutable_path() { return tracked_path_->path; } }; } // namespace flutter diff --git a/lib/ui/painting/path_unittests.cc b/lib/ui/painting/path_unittests.cc new file mode 100644 index 0000000000000000000000000000000000000000..f8b2eabc9f30d12d766140439d3a6d62d945e7d2 --- /dev/null +++ b/lib/ui/painting/path_unittests.cc @@ -0,0 +1,188 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/lib/ui/painting/path.h" + +#include + +#include "flutter/common/task_runners.h" +#include "flutter/fml/synchronization/waitable_event.h" +#include "flutter/runtime/dart_vm.h" +#include "flutter/shell/common/shell_test.h" +#include "flutter/shell/common/thread_host.h" +#include "flutter/testing/testing.h" + +namespace flutter { +namespace testing { + +TEST_F(ShellTest, PathVolatilityOldPathsBecomeNonVolatile) { + auto message_latch = std::make_shared(); + + auto native_validate_path = [message_latch](Dart_NativeArguments args) { + auto handle = Dart_GetNativeArgument(args, 0); + intptr_t peer = 0; + Dart_Handle result = Dart_GetNativeInstanceField( + handle, tonic::DartWrappable::kPeerIndex, &peer); + EXPECT_FALSE(Dart_IsError(result)); + CanvasPath* path = reinterpret_cast(peer); + EXPECT_TRUE(path); + EXPECT_TRUE(path->path().isVolatile()); + std::shared_ptr tracker = + UIDartState::Current()->GetVolatilePathTracker(); + EXPECT_TRUE(tracker); + + for (int i = 0; i < VolatilePathTracker::kFramesOfVolatility; i++) { + EXPECT_TRUE(path->path().isVolatile()); + tracker->OnFrame(); + } + EXPECT_FALSE(path->path().isVolatile()); + message_latch->Signal(); + }; + + Settings settings = CreateSettingsForFixture(); + TaskRunners task_runners("test", // label + GetCurrentTaskRunner(), // platform + CreateNewThread(), // raster + CreateNewThread(), // ui + CreateNewThread() // io + ); + + AddNativeCallback("ValidatePath", CREATE_NATIVE_ENTRY(native_validate_path)); + + std::unique_ptr shell = + CreateShell(std::move(settings), std::move(task_runners)); + + ASSERT_TRUE(shell->IsSetup()); + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("createPath"); + + shell->RunEngine(std::move(configuration), [](auto result) { + ASSERT_EQ(result, Engine::RunStatus::Success); + }); + + message_latch->Wait(); + + DestroyShell(std::move(shell), std::move(task_runners)); +} + +TEST_F(ShellTest, PathVolatilityGCRemovesPathFromTracker) { + auto message_latch = std::make_shared(); + + auto native_validate_path = [message_latch](Dart_NativeArguments args) { + auto handle = Dart_GetNativeArgument(args, 0); + intptr_t peer = 0; + Dart_Handle result = Dart_GetNativeInstanceField( + handle, tonic::DartWrappable::kPeerIndex, &peer); + EXPECT_FALSE(Dart_IsError(result)); + CanvasPath* path = reinterpret_cast(peer); + EXPECT_TRUE(path); + path->AddRef(); + EXPECT_TRUE(path->path().isVolatile()); + std::shared_ptr tracker = + UIDartState::Current()->GetVolatilePathTracker(); + EXPECT_TRUE(tracker); + + static_assert(VolatilePathTracker::kFramesOfVolatility > 1); + EXPECT_TRUE(path->path().isVolatile()); + tracker->OnFrame(); + EXPECT_TRUE(path->path().isVolatile()); + + // simulate GC + path->ReleaseDartWrappableReference(); + + tracker->OnFrame(); + // Because the path got GC'd, it was removed from the cache and we're the + // only one holding it. + EXPECT_TRUE(path->path().isVolatile()); + + path->Release(); + + message_latch->Signal(); + }; + + Settings settings = CreateSettingsForFixture(); + TaskRunners task_runners("test", // label + GetCurrentTaskRunner(), // platform + CreateNewThread(), // raster + CreateNewThread(), // ui + CreateNewThread() // io + ); + + AddNativeCallback("ValidatePath", CREATE_NATIVE_ENTRY(native_validate_path)); + + std::unique_ptr shell = + CreateShell(std::move(settings), std::move(task_runners)); + + ASSERT_TRUE(shell->IsSetup()); + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("createPath"); + + shell->RunEngine(std::move(configuration), [](auto result) { + ASSERT_EQ(result, Engine::RunStatus::Success); + }); + + message_latch->Wait(); + + DestroyShell(std::move(shell), std::move(task_runners)); +} + +// Screen diffing tests use deterministic rendering. Allowing a path to be +// volatile or not for an individual frame can result in minor pixel differences +// that cause the test to fail. +// If deterministic rendering is enabled, the tracker should be disabled and +// paths should always be non-volatile. +TEST_F(ShellTest, DeterministicRenderingDisablesPathVolatility) { + auto message_latch = std::make_shared(); + + auto native_validate_path = [message_latch](Dart_NativeArguments args) { + auto handle = Dart_GetNativeArgument(args, 0); + intptr_t peer = 0; + Dart_Handle result = Dart_GetNativeInstanceField( + handle, tonic::DartWrappable::kPeerIndex, &peer); + EXPECT_FALSE(Dart_IsError(result)); + CanvasPath* path = reinterpret_cast(peer); + EXPECT_TRUE(path); + EXPECT_FALSE(path->path().isVolatile()); + std::shared_ptr tracker = + UIDartState::Current()->GetVolatilePathTracker(); + EXPECT_TRUE(tracker); + EXPECT_FALSE(tracker->enabled()); + + for (int i = 0; i < VolatilePathTracker::kFramesOfVolatility; i++) { + tracker->OnFrame(); + EXPECT_FALSE(path->path().isVolatile()); + } + EXPECT_FALSE(path->path().isVolatile()); + message_latch->Signal(); + }; + + Settings settings = CreateSettingsForFixture(); + settings.skia_deterministic_rendering_on_cpu = true; + TaskRunners task_runners("test", // label + GetCurrentTaskRunner(), // platform + CreateNewThread(), // raster + CreateNewThread(), // ui + CreateNewThread() // io + ); + + AddNativeCallback("ValidatePath", CREATE_NATIVE_ENTRY(native_validate_path)); + + std::unique_ptr shell = + CreateShell(std::move(settings), std::move(task_runners)); + + ASSERT_TRUE(shell->IsSetup()); + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("createPath"); + + shell->RunEngine(std::move(configuration), [](auto result) { + ASSERT_EQ(result, Engine::RunStatus::Success); + }); + + message_latch->Wait(); + + DestroyShell(std::move(shell), std::move(task_runners)); +} + +} // namespace testing +} // namespace flutter diff --git a/lib/ui/ui_benchmarks.cc b/lib/ui/ui_benchmarks.cc index b0f09e373607a486c9424f5e222299e39967842c..739885710d7a2bbdca0cecb7fea44638184722a4 100644 --- a/lib/ui/ui_benchmarks.cc +++ b/lib/ui/ui_benchmarks.cc @@ -4,6 +4,7 @@ #include "flutter/benchmarking/benchmarking.h" #include "flutter/common/settings.h" +#include "flutter/lib/ui/volatile_path_tracker.h" #include "flutter/lib/ui/window/platform_message_response_dart.h" #include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/shell/common/thread_host.h" @@ -66,7 +67,54 @@ static void BM_PlatformMessageResponseDartComplete(benchmark::State& state) { } } +static void BM_PathVolatilityTracker(benchmark::State& state) { + ThreadHost thread_host("test", + ThreadHost::Type::Platform | ThreadHost::Type::RASTER | + ThreadHost::Type::IO | ThreadHost::Type::UI); + TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(), + thread_host.raster_thread->GetTaskRunner(), + thread_host.ui_thread->GetTaskRunner(), + thread_host.io_thread->GetTaskRunner()); + + VolatilePathTracker tracker(task_runners.GetUITaskRunner(), true); + + while (state.KeepRunning()) { + std::vector> paths; + constexpr int path_count = 1000; + for (int i = 0; i < path_count; i++) { + auto path = std::make_shared(); + path->path = SkPath(); + path->path.setIsVolatile(true); + paths.push_back(std::move(path)); + } + + fml::AutoResetWaitableEvent latch; + task_runners.GetUITaskRunner()->PostTask([&]() { + for (auto path : paths) { + tracker.Insert(path); + } + latch.Signal(); + }); + + latch.Wait(); + + task_runners.GetUITaskRunner()->PostTask([&]() { tracker.OnFrame(); }); + + for (int i = 0; i < path_count - 10; ++i) { + tracker.Erase(paths[i]); + } + + task_runners.GetUITaskRunner()->PostTask([&]() { tracker.OnFrame(); }); + + latch.Reset(); + task_runners.GetUITaskRunner()->PostTask([&]() { latch.Signal(); }); + latch.Wait(); + } +} + BENCHMARK(BM_PlatformMessageResponseDartComplete) ->Unit(benchmark::kMicrosecond); +BENCHMARK(BM_PathVolatilityTracker)->Unit(benchmark::kMillisecond); + } // namespace flutter diff --git a/lib/ui/ui_dart_state.cc b/lib/ui/ui_dart_state.cc index e5514b21f956de7fd45b1f82b16a4cf94e1f755b..3def189206afc5dbbe40520b2af3d2dd5e821b19 100644 --- a/lib/ui/ui_dart_state.cc +++ b/lib/ui/ui_dart_state.cc @@ -28,6 +28,7 @@ UIDartState::UIDartState( UnhandledExceptionCallback unhandled_exception_callback, std::shared_ptr isolate_name_server, bool is_root_isolate, + std::shared_ptr volatile_path_tracker, bool enable_skparagraph) : task_runners_(std::move(task_runners)), add_callback_(std::move(add_callback)), @@ -37,6 +38,7 @@ UIDartState::UIDartState( io_manager_(std::move(io_manager)), skia_unref_queue_(std::move(skia_unref_queue)), image_decoder_(std::move(image_decoder)), + volatile_path_tracker_(std::move(volatile_path_tracker)), advisory_script_uri_(std::move(advisory_script_uri)), advisory_script_entrypoint_(std::move(advisory_script_entrypoint)), logger_prefix_(std::move(logger_prefix)), @@ -108,6 +110,11 @@ fml::RefPtr UIDartState::GetSkiaUnrefQueue() const { return skia_unref_queue_; } +std::shared_ptr UIDartState::GetVolatilePathTracker() + const { + return volatile_path_tracker_; +} + void UIDartState::ScheduleMicrotask(Dart_Handle closure) { if (tonic::LogIfError(closure) || !Dart_IsClosure(closure)) { return; diff --git a/lib/ui/ui_dart_state.h b/lib/ui/ui_dart_state.h index d05ad9f81167d45b6038583839802645bcad522b..0a3e44a97ca783803ce0e57b75907caf71f31516 100644 --- a/lib/ui/ui_dart_state.h +++ b/lib/ui/ui_dart_state.h @@ -20,6 +20,7 @@ #include "flutter/lib/ui/isolate_name_server/isolate_name_server.h" #include "flutter/lib/ui/painting/image_decoder.h" #include "flutter/lib/ui/snapshot_delegate.h" +#include "flutter/lib/ui/volatile_path_tracker.h" #include "third_party/dart/runtime/include/dart_api.h" #include "third_party/skia/include/gpu/GrDirectContext.h" #include "third_party/tonic/dart_microtask_queue.h" @@ -59,6 +60,8 @@ class UIDartState : public tonic::DartState { fml::RefPtr GetSkiaUnrefQueue() const; + std::shared_ptr GetVolatilePathTracker() const; + fml::WeakPtr GetSnapshotDelegate() const; fml::WeakPtr GetHintFreedDelegate() const; @@ -102,6 +105,7 @@ class UIDartState : public tonic::DartState { UnhandledExceptionCallback unhandled_exception_callback, std::shared_ptr isolate_name_server, bool is_root_isolate_, + std::shared_ptr volatile_path_tracker, bool enable_skparagraph); ~UIDartState() override; @@ -124,6 +128,7 @@ class UIDartState : public tonic::DartState { fml::WeakPtr io_manager_; fml::RefPtr skia_unref_queue_; fml::WeakPtr image_decoder_; + std::shared_ptr volatile_path_tracker_; const std::string advisory_script_uri_; const std::string advisory_script_entrypoint_; const std::string logger_prefix_; diff --git a/lib/ui/volatile_path_tracker.cc b/lib/ui/volatile_path_tracker.cc new file mode 100644 index 0000000000000000000000000000000000000000..b82012bfa30a8560e7af8ae79555ce2b7fe3ca0c --- /dev/null +++ b/lib/ui/volatile_path_tracker.cc @@ -0,0 +1,85 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/lib/ui/volatile_path_tracker.h" + +namespace flutter { + +VolatilePathTracker::VolatilePathTracker( + fml::RefPtr ui_task_runner, + bool enabled) + : ui_task_runner_(ui_task_runner), enabled_(enabled) {} + +void VolatilePathTracker::Insert(std::shared_ptr path) { + FML_DCHECK(ui_task_runner_->RunsTasksOnCurrentThread()); + FML_DCHECK(path); + FML_DCHECK(path->path.isVolatile()); + if (!enabled_) { + path->path.setIsVolatile(false); + return; + } + paths_.insert(path); +} + +void VolatilePathTracker::Erase(std::shared_ptr path) { + if (!enabled_) { + return; + } + FML_DCHECK(path); + if (ui_task_runner_->RunsTasksOnCurrentThread()) { + paths_.erase(path); + return; + } + + std::scoped_lock lock(paths_to_remove_mutex_); + needs_drain_ = true; + paths_to_remove_.push_back(path); +} + +void VolatilePathTracker::OnFrame() { + FML_DCHECK(ui_task_runner_->RunsTasksOnCurrentThread()); + if (!enabled_) { + return; + } + std::string total_count = std::to_string(paths_.size()); + TRACE_EVENT1("flutter", "VolatilePathTracker::OnFrame", "total_count", + total_count.c_str()); + + Drain(); + + std::set> surviving_paths_; + for (const std::shared_ptr& path : paths_) { + path->frame_count++; + if (path->frame_count >= kFramesOfVolatility) { + path->path.setIsVolatile(false); + path->tracking_volatility = false; + } else { + surviving_paths_.insert(path); + } + } + paths_.swap(surviving_paths_); + std::string post_removal_count = std::to_string(paths_.size()); + TRACE_EVENT_INSTANT1("flutter", "VolatilePathTracker::OnFrame", + "remaining_count", post_removal_count.c_str()); +} + +void VolatilePathTracker::Drain() { + if (needs_drain_) { + TRACE_EVENT0("flutter", "VolatilePathTracker::Drain"); + std::deque> paths_to_remove; + { + std::scoped_lock lock(paths_to_remove_mutex_); + paths_to_remove.swap(paths_to_remove_); + needs_drain_ = false; + } + std::string count = std::to_string(paths_to_remove.size()); + TRACE_EVENT_INSTANT1("flutter", "VolatilePathTracker::Drain", "count", + count.c_str()); + for (auto& path : paths_to_remove) { + paths_.erase(path); + } + } +} + +} // namespace flutter diff --git a/lib/ui/volatile_path_tracker.h b/lib/ui/volatile_path_tracker.h new file mode 100644 index 0000000000000000000000000000000000000000..40f28311a008abdbd957d80b5a9aa5845f0c6c0e --- /dev/null +++ b/lib/ui/volatile_path_tracker.h @@ -0,0 +1,82 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_LIB_VOLATILE_PATH_TRACKER_H_ +#define FLUTTER_LIB_VOLATILE_PATH_TRACKER_H_ + +#include +#include +#include + +#include "flutter/fml/macros.h" +#include "flutter/fml/task_runner.h" +#include "flutter/fml/trace_event.h" +#include "third_party/skia/include/core/SkPath.h" + +namespace flutter { + +/// A cache for paths drawn from dart:ui. +/// +/// Whenever a flutter::CanvasPath is created, it must Insert an entry into +/// this cache. Whenever a frame is drawn, the shell must call OnFrame. The +/// cache will flip the volatility bit on the SkPath and remove it from the +/// cache. If the Dart object is released, Erase must be called to avoid +/// tracking a path that is no longer referenced in Dart code. +/// +/// Enabling this cache may cause difficult to predict minor pixel differences +/// when paths are rendered. If deterministic rendering is needed, e.g. for a +/// screen diffing test, this class will not cache any paths and will +/// automatically set the volatility of the path to false. +class VolatilePathTracker { + public: + /// The fields of this struct must only accessed on the UI task runner. + struct TrackedPath { + bool tracking_volatility = false; + int frame_count = 0; + SkPath path; + }; + + VolatilePathTracker(fml::RefPtr ui_task_runner, + bool enabled); + + static constexpr int kFramesOfVolatility = 2; + + // Starts tracking a path. + // Must be called from the UI task runner. + // + // Callers should only insert paths that are currently volatile. + void Insert(std::shared_ptr path); + + // Removes a path from tracking. + // + // May be called from any thread. + void Erase(std::shared_ptr path); + + // Called by the shell at the end of a frame after notifying Dart about idle + // time. + // + // This method will flip the volatility bit to false for any paths that have + // survived the |kFramesOfVolatility|. + // + // Must be called from the UI task runner. + void OnFrame(); + + bool enabled() const { return enabled_; } + + private: + fml::RefPtr ui_task_runner_; + std::atomic_bool needs_drain_ = false; + std::mutex paths_to_remove_mutex_; + std::deque> paths_to_remove_; + std::set> paths_; + bool enabled_ = true; + + void Drain(); + + FML_DISALLOW_COPY_AND_ASSIGN(VolatilePathTracker); +}; + +} // namespace flutter + +#endif // FLUTTER_LIB_VOLATILE_PATH_TRACKER_H_ diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 6ea5a2256cf431273c6cca126b8718c3d7bd7370..9cb2fce5e91a6a60c36e853b251728f9c864154e 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -92,7 +92,8 @@ std::weak_ptr DartIsolate::SpawnIsolate( GetIOManager(), GetSkiaUnrefQueue(), GetImageDecoder(), advisory_script_uri, advisory_script_entrypoint, flags, isolate_create_callback, isolate_shutdown_callback, dart_entrypoint, - dart_entrypoint_library, std::move(isolate_configration), this); + dart_entrypoint_library, std::move(isolate_configration), + GetVolatilePathTracker(), this); } std::weak_ptr DartIsolate::CreateRunningRootIsolate( @@ -113,6 +114,7 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( std::optional dart_entrypoint, std::optional dart_entrypoint_library, std::unique_ptr isolate_configration, + std::shared_ptr volatile_path_tracker, const DartIsolate* spawning_isolate) { if (!isolate_snapshot) { FML_LOG(ERROR) << "Invalid isolate snapshot."; @@ -140,7 +142,8 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( advisory_script_entrypoint, // isolate_flags, // isolate_create_callback, // - isolate_shutdown_callback // + isolate_shutdown_callback, // + std::move(volatile_path_tracker) // ) .lock(); @@ -173,8 +176,8 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( } if (settings.root_isolate_create_callback) { - // Isolate callbacks always occur in isolate scope and before user code - // has had a chance to run. + // Isolate callbacks always occur in isolate scope and before user code has + // had a chance to run. tonic::DartState::Scope scope(isolate.get()); settings.root_isolate_create_callback(*isolate.get()); } @@ -212,6 +215,7 @@ std::weak_ptr DartIsolate::CreateRootIsolate( Flags flags, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback, + std::shared_ptr volatile_path_tracker, const DartIsolate* spawning_isolate) { TRACE_EVENT0("flutter", "DartIsolate::CreateRootIsolate"); @@ -231,16 +235,17 @@ std::weak_ptr DartIsolate::CreateRootIsolate( auto isolate_data = std::make_unique>( std::shared_ptr(new DartIsolate( - settings, // settings - task_runners, // task runners - std::move(snapshot_delegate), // snapshot delegate - std::move(hint_freed_delegate), // hint freed delegate - std::move(io_manager), // IO manager - std::move(unref_queue), // Skia unref queue - std::move(image_decoder), // Image Decoder - advisory_script_uri, // advisory URI - advisory_script_entrypoint, // advisory entrypoint - true // is_root_isolate + settings, // settings + task_runners, // task runners + std::move(snapshot_delegate), // snapshot delegate + std::move(hint_freed_delegate), // hint freed delegate + std::move(io_manager), // IO manager + std::move(unref_queue), // Skia unref queue + std::move(image_decoder), // Image Decoder + advisory_script_uri, // advisory URI + advisory_script_entrypoint, // advisory entrypoint + true, // is_root_isolate + std::move(volatile_path_tracker) // volatile path tracker ))); DartErrorString error; @@ -281,16 +286,18 @@ std::weak_ptr DartIsolate::CreateRootIsolate( return (*root_isolate_data)->GetWeakIsolatePtr(); } -DartIsolate::DartIsolate(const Settings& settings, - TaskRunners task_runners, - fml::WeakPtr snapshot_delegate, - fml::WeakPtr hint_freed_delegate, - fml::WeakPtr io_manager, - fml::RefPtr unref_queue, - fml::WeakPtr image_decoder, - std::string advisory_script_uri, - std::string advisory_script_entrypoint, - bool is_root_isolate) +DartIsolate::DartIsolate( + const Settings& settings, + TaskRunners task_runners, + fml::WeakPtr snapshot_delegate, + fml::WeakPtr hint_freed_delegate, + fml::WeakPtr io_manager, + fml::RefPtr unref_queue, + fml::WeakPtr image_decoder, + std::string advisory_script_uri, + std::string advisory_script_entrypoint, + bool is_root_isolate, + std::shared_ptr volatile_path_tracker) : UIDartState(std::move(task_runners), settings.task_observer_add, settings.task_observer_remove, @@ -305,6 +312,7 @@ DartIsolate::DartIsolate(const Settings& settings, settings.unhandled_exception_callback, DartVMRef::GetIsolateNameServer(), is_root_isolate, + std::move(volatile_path_tracker), settings.enable_skparagraph), may_insecurely_connect_to_all_domains_( settings.may_insecurely_connect_to_all_domains), @@ -725,8 +733,8 @@ bool DartIsolate::RunFromLibrary(std::optional library_name, bool DartIsolate::Shutdown() { TRACE_EVENT0("flutter", "DartIsolate::Shutdown"); // This call may be re-entrant since Dart_ShutdownIsolate can invoke the - // cleanup callback which deletes the embedder side object of the dart - // isolate (a.k.a. this). + // cleanup callback which deletes the embedder side object of the dart isolate + // (a.k.a. this). if (phase_ == Phase::Shutdown) { return false; } @@ -754,8 +762,7 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate( if (!vm_data) { *error = fml::strdup( - "Could not access VM data to initialize isolates. This may be " - "because " + "Could not access VM data to initialize isolates. This may be because " "the VM has initialized shutdown on another thread already."); return nullptr; } @@ -773,8 +780,8 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate( #if (FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_DEBUG) // TODO(68663): The service isolate in debug mode is always launched without - // sound null safety. Fix after the isolate snapshot data is created with - // the right flags. + // sound null safety. Fix after the isolate snapshot data is created with the + // right flags. flags->null_safety = vm_data->GetIsolateSnapshot()->IsNullSafetyEnabled(nullptr); #endif @@ -794,7 +801,8 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate( DART_VM_SERVICE_ISOLATE_NAME, // script entrypoint DartIsolate::Flags{flags}, // flags nullptr, // isolate create callback - nullptr // isolate shutdown callback + nullptr, // isolate shutdown callback + nullptr // volatile path tracker ); std::shared_ptr service_isolate = weak_service_isolate.lock(); @@ -862,9 +870,8 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback( // The VM attempts to start the VM service for us on |Dart_Initialize|. In // such a case, the callback data will be null and the script URI will be // DART_VM_SERVICE_ISOLATE_NAME. In such cases, we just create the service - // isolate like normal but dont hold a reference to it at all. We also - // start this isolate since we will never again reference it from the - // engine. + // isolate like normal but dont hold a reference to it at all. We also start + // this isolate since we will never again reference it from the engine. return DartCreateAndStartServiceIsolate(package_root, // package_config, // flags, // @@ -906,7 +913,8 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback( fml::WeakPtr{}, // image_decoder advisory_script_uri, // advisory_script_uri advisory_script_entrypoint, // advisory_script_entrypoint - false))); // is_root_isolate + false, // is_root_isolate + nullptr))); // volatile path tracker Dart_Isolate vm_isolate = CreateDartIsolateGroup( std::move(isolate_group_data), std::move(isolate_data), flags, error, @@ -963,7 +971,8 @@ bool DartIsolate::DartIsolateInitializeCallback(void** child_callback_data, (*isolate_group_data)->GetAdvisoryScriptURI(), // advisory_script_uri (*isolate_group_data) ->GetAdvisoryScriptEntrypoint(), // advisory_script_entrypoint - false))); // is_root_isolate + false, // is_root_isolate + nullptr))); // volatile path tracker // root isolate should have been created via CreateRootIsolate if (!InitializeIsolate(*embedder_isolate, isolate, error)) { @@ -997,8 +1006,7 @@ Dart_Isolate DartIsolate::CreateDartIsolateGroup( return nullptr; } - // Ownership of the isolate data objects has been transferred to the Dart - // VM. + // Ownership of the isolate data objects has been transferred to the Dart VM. std::shared_ptr embedder_isolate(*isolate_data); isolate_group_data.release(); isolate_data.release(); @@ -1028,9 +1036,9 @@ bool DartIsolate::InitializeIsolate( return false; } - // Root isolates will be setup by the engine and the service isolate (which - // is also a root isolate) by the utility routines in the VM. However, - // secondary isolates will be run by the VM if they are marked as runnable. + // Root isolates will be setup by the engine and the service isolate (which is + // also a root isolate) by the utility routines in the VM. However, secondary + // isolates will be run by the VM if they are marked as runnable. if (!embedder_isolate->IsRootIsolate()) { auto child_isolate_preparer = embedder_isolate->GetIsolateGroupData().GetChildIsolatePreparer(); diff --git a/runtime/dart_isolate.h b/runtime/dart_isolate.h index a42ec378c3ea0e3c36d42b0d7348b43c3190f6a6..b4705ad9c7b76a061e8cdc1f2ddeaeaf05ee659f 100644 --- a/runtime/dart_isolate.h +++ b/runtime/dart_isolate.h @@ -233,6 +233,7 @@ class DartIsolate : public UIDartState { std::optional dart_entrypoint, std::optional dart_entrypoint_library, std::unique_ptr isolate_configration, + std::shared_ptr volatile_path_tracker, const DartIsolate* spawning_isolate = nullptr); //---------------------------------------------------------------------------- @@ -463,6 +464,7 @@ class DartIsolate : public UIDartState { Flags flags, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback, + std::shared_ptr volatile_path_tracker, const DartIsolate* spawning_isolate = nullptr); DartIsolate(const Settings& settings, @@ -474,7 +476,8 @@ class DartIsolate : public UIDartState { fml::WeakPtr image_decoder, std::string advisory_script_uri, std::string advisory_script_entrypoint, - bool is_root_isolate); + bool is_root_isolate, + std::shared_ptr volatile_path_tracker); [[nodiscard]] bool Initialize(Dart_Isolate isolate); diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index 92589d8663ea7a857991abbebebd318d2660b50f..3b25c02dec0ae788e30475d669e0ea5030b41ff4 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -68,7 +68,8 @@ TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) { settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint std::nullopt, // dart entrypoint library - std::move(isolate_configuration) // isolate configuration + std::move(isolate_configuration), // isolate configuration + nullptr // Volatile path tracker ); auto root_isolate = weak_isolate.lock(); ASSERT_TRUE(root_isolate); @@ -171,7 +172,8 @@ TEST_F(DartIsolateTest, IsolateShutdownCallbackIsInIsolateScope) { settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint std::nullopt, // dart entrypoint library - std::move(isolate_configuration) // isolate configuration + std::move(isolate_configuration), // isolate configuration + nullptr // Volatile path tracker ); auto root_isolate = weak_isolate.lock(); ASSERT_TRUE(root_isolate); @@ -429,7 +431,8 @@ TEST_F(DartIsolateTest, CanCreateServiceIsolate) { settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint std::nullopt, // dart entrypoint library - std::move(isolate_configuration) // isolate configuration + std::move(isolate_configuration), // isolate configuration + nullptr // Volatile path tracker ); auto root_isolate = weak_isolate.lock(); ASSERT_TRUE(root_isolate); @@ -529,7 +532,8 @@ TEST_F(DartIsolateTest, InvalidLoadingUnitFails) { settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint std::nullopt, // dart entrypoint library - std::move(isolate_configuration) // isolate configuration + std::move(isolate_configuration), // isolate configuration + nullptr // volatile path tracker ); auto root_isolate = weak_isolate.lock(); ASSERT_TRUE(root_isolate); diff --git a/runtime/dart_lifecycle_unittests.cc b/runtime/dart_lifecycle_unittests.cc index 8576b3fb11292ef8a47a31d84d39f4844f45896a..09de19f6e1687bcda0ce92311f504a5033fc9e14 100644 --- a/runtime/dart_lifecycle_unittests.cc +++ b/runtime/dart_lifecycle_unittests.cc @@ -73,7 +73,8 @@ static std::shared_ptr CreateAndRunRootIsolate( settings.isolate_shutdown_callback, // isolate shutdown callback, entrypoint, // dart entrypoint std::nullopt, // dart entrypoint library - std::move(isolate_configuration) // isolate configuration + std::move(isolate_configuration), // isolate configuration + nullptr // Volatile path tracker ) .lock(); diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 55dd1e736315cc6b311b90f57b284f2a3bcbf6fe..42a6ea6bc9d67bdcd59ae323b2540305ef126529 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -37,7 +37,8 @@ RuntimeController::RuntimeController( const PlatformData& p_platform_data, const fml::closure& p_isolate_create_callback, const fml::closure& p_isolate_shutdown_callback, - std::shared_ptr p_persistent_isolate_data) + std::shared_ptr p_persistent_isolate_data, + std::shared_ptr p_volatile_path_tracker) : client_(p_client), vm_(p_vm), isolate_snapshot_(std::move(p_isolate_snapshot)), @@ -53,7 +54,8 @@ RuntimeController::RuntimeController( platform_data_(std::move(p_platform_data)), isolate_create_callback_(p_isolate_create_callback), isolate_shutdown_callback_(p_isolate_shutdown_callback), - persistent_isolate_data_(std::move(p_persistent_isolate_data)) {} + persistent_isolate_data_(std::move(p_persistent_isolate_data)), + volatile_path_tracker_(std::move(p_volatile_path_tracker)) {} std::unique_ptr RuntimeController::Spawn( RuntimeDelegate& client, @@ -68,7 +70,8 @@ std::unique_ptr RuntimeController::Spawn( hint_freed_delegate_, io_manager_, unref_queue_, image_decoder_, advisory_script_uri, advisory_script_entrypoint, idle_notification_callback, platform_data_, isolate_create_callback, - isolate_shutdown_callback, persistent_isolate_data); + isolate_shutdown_callback, persistent_isolate_data, + volatile_path_tracker_); result->spawning_isolate_ = root_isolate_; return result; } @@ -111,7 +114,8 @@ std::unique_ptr RuntimeController::Clone() const { platform_data_, // isolate_create_callback_, // isolate_shutdown_callback_, // - persistent_isolate_data_ // + persistent_isolate_data_, // + volatile_path_tracker_ // )); } @@ -387,7 +391,8 @@ bool RuntimeController::LaunchRootIsolate( isolate_shutdown_callback_, // dart_entrypoint, // dart_entrypoint_library, // - std::move(isolate_configuration) // + std::move(isolate_configuration), // + volatile_path_tracker_ // ) .lock(); diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 4a836902c296fb120509c713f7d9c206e819098d..09ff677814e1719f8e9f92bd9b4e79e91b62ac6f 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -15,6 +15,7 @@ #include "flutter/lib/ui/io_manager.h" #include "flutter/lib/ui/text/font_collection.h" #include "flutter/lib/ui/ui_dart_state.h" +#include "flutter/lib/ui/volatile_path_tracker.h" #include "flutter/lib/ui/window/platform_configuration.h" #include "flutter/lib/ui/window/pointer_data_packet.h" #include "flutter/runtime/dart_vm.h" @@ -110,6 +111,8 @@ class RuntimeController : public PlatformConfigurationClient { /// @param[in] persistent_isolate_data Unstructured persistent read-only /// data that the root isolate can /// access in a synchronous manner. + /// @param[in] volatile_path_tracker Cache for tracking path + /// volatility. /// RuntimeController( RuntimeDelegate& client, @@ -127,7 +130,8 @@ class RuntimeController : public PlatformConfigurationClient { const PlatformData& platform_data, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback, - std::shared_ptr persistent_isolate_data); + std::shared_ptr persistent_isolate_data, + std::shared_ptr volatile_path_tracker); //---------------------------------------------------------------------------- /// @brief Create a RuntimeController that shares as many resources as @@ -610,6 +614,7 @@ class RuntimeController : public PlatformConfigurationClient { const fml::closure isolate_create_callback_; const fml::closure isolate_shutdown_callback_; std::shared_ptr persistent_isolate_data_; + std::shared_ptr volatile_path_tracker_; PlatformConfiguration* GetPlatformConfigurationIfAvailable(); diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 416ce577a7861108e0f4d5890ac9187cff50ae42..c25511da130e0a64676027e12742114b3beb278b 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -68,7 +68,8 @@ Engine::Engine(Delegate& delegate, std::unique_ptr animator, fml::WeakPtr io_manager, fml::RefPtr unref_queue, - fml::WeakPtr snapshot_delegate) + fml::WeakPtr snapshot_delegate, + std::shared_ptr volatile_path_tracker) : Engine(delegate, dispatcher_maker, vm.GetConcurrentWorkerTaskRunner(), @@ -94,7 +95,8 @@ Engine::Engine(Delegate& delegate, platform_data, // platform data settings_.isolate_create_callback, // isolate create callback settings_.isolate_shutdown_callback, // isolate shutdown callback - settings_.persistent_isolate_data // persistent isolate data + settings_.persistent_isolate_data, // persistent isolate data + std::move(volatile_path_tracker) // volatile path tracker ); } diff --git a/shell/common/engine.h b/shell/common/engine.h index bbdd4d0b69bfc0070f45b28a3e189d7d480e505c..a5581611e5c31805de7448c0f0f5990cde193892 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -19,6 +19,7 @@ #include "flutter/lib/ui/semantics/semantics_node.h" #include "flutter/lib/ui/snapshot_delegate.h" #include "flutter/lib/ui/text/font_collection.h" +#include "flutter/lib/ui/volatile_path_tracker.h" #include "flutter/lib/ui/window/platform_message.h" #include "flutter/lib/ui/window/viewport_metrics.h" #include "flutter/runtime/dart_vm.h" @@ -351,7 +352,8 @@ class Engine final : public RuntimeDelegate, std::unique_ptr animator, fml::WeakPtr io_manager, fml::RefPtr unref_queue, - fml::WeakPtr snapshot_delegate); + fml::WeakPtr snapshot_delegate, + std::shared_ptr volatile_path_tracker); //---------------------------------------------------------------------------- /// @brief Create a Engine that shares as many resources as diff --git a/shell/common/shell.cc b/shell/common/shell.cc index cebcd4b8f8f1cb5dafe7b4c9adfa79ea692947c8..435138d7ce7aff508d524a27f02fd3e643857822 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -51,7 +51,8 @@ std::unique_ptr CreateEngine( std::unique_ptr animator, fml::WeakPtr io_manager, fml::RefPtr unref_queue, - fml::WeakPtr snapshot_delegate) { + fml::WeakPtr snapshot_delegate, + std::shared_ptr volatile_path_tracker) { return std::make_unique(delegate, // dispatcher_maker, // vm, // @@ -62,7 +63,8 @@ std::unique_ptr CreateEngine( std::move(animator), // io_manager, // unref_queue, // - snapshot_delegate); + snapshot_delegate, // + volatile_path_tracker); } } // namespace @@ -80,8 +82,11 @@ std::unique_ptr Shell::CreateShellOnPlatformThread( return nullptr; } - auto shell = - std::unique_ptr(new Shell(std::move(vm), task_runners, settings)); + auto shell = std::unique_ptr( + new Shell(std::move(vm), task_runners, settings, + std::make_shared( + task_runners.GetUITaskRunner(), + !settings.skia_deterministic_rendering_on_cpu))); // Create the rasterizer on the raster thread. std::promise> rasterizer_promise; @@ -177,17 +182,18 @@ std::unique_ptr Shell::CreateShellOnPlatformThread( std::move(vsync_waiter)); engine_promise.set_value( - on_create_engine(*shell, // - dispatcher_maker, // - *shell->GetDartVM(), // - std::move(isolate_snapshot), // - task_runners, // - platform_data, // - shell->GetSettings(), // - std::move(animator), // - weak_io_manager_future.get(), // - unref_queue_future.get(), // - snapshot_delegate_future.get())); + on_create_engine(*shell, // + dispatcher_maker, // + *shell->GetDartVM(), // + std::move(isolate_snapshot), // + task_runners, // + platform_data, // + shell->GetSettings(), // + std::move(animator), // + weak_io_manager_future.get(), // + unref_queue_future.get(), // + snapshot_delegate_future.get(), // + shell->volatile_path_tracker_)); })); if (!shell->Setup(std::move(platform_view), // @@ -349,11 +355,15 @@ std::unique_ptr Shell::Create( return shell; } -Shell::Shell(DartVMRef vm, TaskRunners task_runners, Settings settings) +Shell::Shell(DartVMRef vm, + TaskRunners task_runners, + Settings settings, + std::shared_ptr volatile_path_tracker) : task_runners_(std::move(task_runners)), settings_(std::move(settings)), vm_(std::move(vm)), is_gpu_disabled_sync_switch_(new fml::SyncSwitch()), + volatile_path_tracker_(std::move(volatile_path_tracker)), weak_factory_gpu_(nullptr), weak_factory_(this) { FML_CHECK(vm_) << "Must have access to VM to create a shell."; @@ -481,7 +491,8 @@ std::unique_ptr Shell::Spawn( Settings settings, std::unique_ptr animator, fml::WeakPtr io_manager, fml::RefPtr unref_queue, - fml::WeakPtr snapshot_delegate) { + fml::WeakPtr snapshot_delegate, + std::shared_ptr volatile_path_tracker) { return engine->Spawn(/*delegate=*/delegate, /*dispatcher_maker=*/dispatcher_maker, /*settings=*/settings, @@ -1088,6 +1099,7 @@ void Shell::OnAnimatorNotifyIdle(int64_t deadline) { if (engine_) { engine_->NotifyIdle(deadline); + volatile_path_tracker_->OnFrame(); } } diff --git a/shell/common/shell.h b/shell/common/shell.h index 4c9b688eb2ae0f2c7ab19224682f5dac045bda5e..3749d4727bca71fec2996dc346bc52bde27497cb 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -27,6 +27,7 @@ #include "flutter/fml/time/time_point.h" #include "flutter/lib/ui/semantics/custom_accessibility_action.h" #include "flutter/lib/ui/semantics/semantics_node.h" +#include "flutter/lib/ui/volatile_path_tracker.h" #include "flutter/lib/ui/window/platform_message.h" #include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/runtime/service_protocol.h" @@ -108,7 +109,8 @@ class Shell final : public PlatformView::Delegate, std::unique_ptr animator, fml::WeakPtr io_manager, fml::RefPtr unref_queue, - fml::WeakPtr snapshot_delegate)> + fml::WeakPtr snapshot_delegate, + std::shared_ptr volatile_path_tracker)> EngineCreateCallback; //---------------------------------------------------------------------------- @@ -410,6 +412,7 @@ class Shell final : public PlatformView::Delegate, std::unique_ptr rasterizer_; // on raster task runner std::unique_ptr io_manager_; // on IO task runner std::shared_ptr is_gpu_disabled_sync_switch_; + std::shared_ptr volatile_path_tracker_; fml::WeakPtr weak_engine_; // to be shared across threads fml::TaskRunnerAffineWeakPtr @@ -461,7 +464,10 @@ class Shell final : public PlatformView::Delegate, sk_sp shared_resource_context_; - Shell(DartVMRef vm, TaskRunners task_runners, Settings settings); + Shell(DartVMRef vm, + TaskRunners task_runners, + Settings settings, + std::shared_ptr volatile_path_tracker); static std::unique_ptr CreateShellOnPlatformThread( DartVMRef vm, diff --git a/testing/dart_isolate_runner.cc b/testing/dart_isolate_runner.cc index 43d77c0165c28215ee891ed79795d0eb5a82bd30..86f6c66892e9f00c6315b1fa24d4706d1b7078c0 100644 --- a/testing/dart_isolate_runner.cc +++ b/testing/dart_isolate_runner.cc @@ -56,7 +56,8 @@ std::unique_ptr RunDartCodeInIsolateOnUITaskRunner( std::string entrypoint, const std::vector& args, const std::string& fixtures_path, - fml::WeakPtr io_manager) { + fml::WeakPtr io_manager, + std::shared_ptr volatile_path_tracker) { FML_CHECK(task_runners.GetUITaskRunner()->RunsTasksOnCurrentThread()); if (!vm_ref) { @@ -126,7 +127,8 @@ std::unique_ptr RunDartCodeInIsolateOnUITaskRunner( settings.isolate_shutdown_callback, // isolate shutdown callback entrypoint, // entrypoint std::nullopt, // library - std::move(isolate_configuration) // isolate configuration + std::move(isolate_configuration), // isolate configuration + std::move(volatile_path_tracker) // volatile path tracker ) .lock(); @@ -146,14 +148,15 @@ std::unique_ptr RunDartCodeInIsolate( std::string entrypoint, const std::vector& args, const std::string& fixtures_path, - fml::WeakPtr io_manager) { + fml::WeakPtr io_manager, + std::shared_ptr volatile_path_tracker) { std::unique_ptr result; fml::AutoResetWaitableEvent latch; fml::TaskRunner::RunNowOrPostTask( task_runners.GetUITaskRunner(), fml::MakeCopyable([&]() mutable { result = RunDartCodeInIsolateOnUITaskRunner( vm_ref, settings, task_runners, entrypoint, args, fixtures_path, - io_manager); + io_manager, std::move(volatile_path_tracker)); latch.Signal(); })); latch.Wait(); diff --git a/testing/dart_isolate_runner.h b/testing/dart_isolate_runner.h index 83c91a1e5c3a80652f294d6430ab64b02b764031..ab6029e38b02b07af37b7b214e52788cc9919fbb 100644 --- a/testing/dart_isolate_runner.h +++ b/testing/dart_isolate_runner.h @@ -42,14 +42,16 @@ class AutoIsolateShutdown { FML_DISALLOW_COPY_AND_ASSIGN(AutoIsolateShutdown); }; -void RunDartCodeInIsolate(DartVMRef& vm_ref, - std::unique_ptr& result, - const Settings& settings, - const TaskRunners& task_runners, - std::string entrypoint, - const std::vector& args, - const std::string& fixtures_path, - fml::WeakPtr io_manager = {}); +void RunDartCodeInIsolate( + DartVMRef& vm_ref, + std::unique_ptr& result, + const Settings& settings, + const TaskRunners& task_runners, + std::string entrypoint, + const std::vector& args, + const std::string& fixtures_path, + fml::WeakPtr io_manager = {}, + std::shared_ptr volatile_path_tracker = nullptr); std::unique_ptr RunDartCodeInIsolate( DartVMRef& vm_ref, @@ -58,7 +60,8 @@ std::unique_ptr RunDartCodeInIsolate( std::string entrypoint, const std::vector& args, const std::string& fixtures_path, - fml::WeakPtr io_manager = {}); + fml::WeakPtr io_manager = {}, + std::shared_ptr volatile_path_tracker = nullptr); } // namespace testing } // namespace flutter diff --git a/third_party/tonic/tests/dart_state_unittest.cc b/third_party/tonic/tests/dart_state_unittest.cc index 3d053de40e75b17d6c29cad52107142635253533..8a91cc20b41aedfad309f306ecf027ab5f98af55 100644 --- a/third_party/tonic/tests/dart_state_unittest.cc +++ b/third_party/tonic/tests/dart_state_unittest.cc @@ -44,7 +44,8 @@ TEST_F(DartState, IsShuttingDown) { settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint std::nullopt, // dart entrypoint library - std::move(isolate_configuration) // isolate configuration + std::move(isolate_configuration), // isolate configuration + nullptr // Volatile path tracker ); auto root_isolate = weak_isolate.lock(); ASSERT_TRUE(root_isolate);