From 1950e72cd034ce8b6ddb7821f5d2d2bb4a4d4f64 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Mon, 12 Feb 2024 12:01:28 -0800 Subject: [PATCH 01/24] Impl engine --- lib/ui/dart_ui.cc | 1 + lib/ui/platform_dispatcher.dart | 25 +++++++++++++++++++ lib/ui/window/platform_configuration.cc | 5 ++++ lib/ui/window/platform_configuration.h | 7 ++++++ lib/web_ui/lib/platform_dispatcher.dart | 2 ++ .../lib/src/engine/platform_dispatcher.dart | 6 +++++ runtime/runtime_controller.cc | 5 ++++ runtime/runtime_controller.h | 3 +++ runtime/runtime_delegate.h | 2 ++ shell/common/animator.cc | 14 +++++++++++ shell/common/animator.h | 2 ++ shell/common/engine.cc | 4 +++ shell/common/engine.h | 3 +++ shell/common/engine_unittests.cc | 1 + 14 files changed, 80 insertions(+) diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index a1ba414316245..205df18059879 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -98,6 +98,7 @@ typedef CanvasPath Path; V(NativeStringAttribute::initSpellOutStringAttribute) \ V(PlatformConfigurationNativeApi::DefaultRouteName) \ V(PlatformConfigurationNativeApi::ScheduleFrame) \ + V(PlatformConfigurationNativeApi::ForceSyncFrame) \ V(PlatformConfigurationNativeApi::Render) \ V(PlatformConfigurationNativeApi::UpdateSemantics) \ V(PlatformConfigurationNativeApi::SetNeedsReportTimings) \ diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 250ea04d6aa74..4153f8f3cd018 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -801,11 +801,36 @@ class PlatformDispatcher { /// /// * [SchedulerBinding], the Flutter framework class which manages the /// scheduling of frames. + /// * [forceSyncFrame], a similar method that is used in rare cases that + /// a frame must be rendered immediately. void scheduleFrame() => _scheduleFrame(); @Native(symbol: 'PlatformConfigurationNativeApi::ScheduleFrame') external static void _scheduleFrame(); + /// Immediately render a frame by invoking the [onBeginFrame] and + /// [onDrawFrame] callbacks synchronously. + /// + /// This method performs the same computation for a frame as [scheduleFrame] + /// does, but instead of doing so at an appropriate opportunity, the render is + /// completed synchronously within this call. + /// + /// Prefer [scheduleFrame] to update the display in normal operation. The + /// [forceSyncFrame] method is designed for situations that require a frame is + /// rendered as soon as possible, even at the cost of rendering quality. An + /// example of using this method is [SchedulerBinding.scheduleWarmUpFrame], + /// which is called during application startup so that the first frame can be + /// presented to the screen a few extra milliseconds earlier. + /// + /// See also: + /// + /// * [SchedulerBinding.scheduleWarmUpFrame], which uses this method. + /// * [scheduleFrame]. + void forceSyncFrame() => _forceSyncFrame(); + + @Native(symbol: 'PlatformConfigurationNativeApi::ForceSyncFrame') + external static void _forceSyncFrame(); + /// Additional accessibility features that may be enabled by the platform. AccessibilityFeatures get accessibilityFeatures => _configuration.accessibilityFeatures; diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index 3ecadac12fe6f..b5f7d99b27a6c 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -589,6 +589,11 @@ void PlatformConfigurationNativeApi::ScheduleFrame() { UIDartState::Current()->platform_configuration()->client()->ScheduleFrame(); } +void PlatformConfigurationNativeApi::ForceSyncFrame() { + UIDartState::ThrowIfUIOperationsProhibited(); + UIDartState::Current()->platform_configuration()->client()->ForceSyncFrame(); +} + void PlatformConfigurationNativeApi::UpdateSemantics(SemanticsUpdate* update) { UIDartState::ThrowIfUIOperationsProhibited(); UIDartState::Current()->platform_configuration()->client()->UpdateSemantics( diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index 5f21051110472..f4f8252181705 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -65,6 +65,11 @@ class PlatformConfigurationClient { /// virtual void ScheduleFrame() = 0; + //-------------------------------------------------------------------------- + /// @brief + /// + virtual void ForceSyncFrame() = 0; + //-------------------------------------------------------------------------- /// @brief Updates the client's rendering on the GPU with the newly /// provided Scene. @@ -557,6 +562,8 @@ class PlatformConfigurationNativeApi { static void ScheduleFrame(); + static void ForceSyncFrame(); + static void Render(int64_t view_id, Scene* scene, double width, diff --git a/lib/web_ui/lib/platform_dispatcher.dart b/lib/web_ui/lib/platform_dispatcher.dart index cf9e15661446d..875318eb61bc2 100644 --- a/lib/web_ui/lib/platform_dispatcher.dart +++ b/lib/web_ui/lib/platform_dispatcher.dart @@ -90,6 +90,8 @@ abstract class PlatformDispatcher { void scheduleFrame(); + void forceSyncFrame(); + Future render(Scene scene, [FlutterView view]); AccessibilityFeatures get accessibilityFeatures; diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index 5dac8c3a0250d..de8fe33a72fcc 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -771,6 +771,12 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { scheduleFrameCallback!(); } + @override + void forceSyncFrame() { + // TODO(dkwingsmt): Call beginFrame and drawFrame, since the framework + // will no longer call them once it switches to forceSyncFrame. + } + /// Updates the application's rendering on the GPU with the newly provided /// [Scene]. This function must be called within the scope of the /// [onBeginFrame] or [onDrawFrame] callbacks being invoked. If this function diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 71ac779e881eb..fc9ea5fbca93d 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -340,6 +340,11 @@ void RuntimeController::ScheduleFrame() { client_.ScheduleFrame(); } +// |PlatformConfigurationClient| +void RuntimeController::ForceSyncFrame() { + client_.ForceSyncFrame(); +} + // |PlatformConfigurationClient| void RuntimeController::Render(Scene* scene, double width, double height) { // TODO(dkwingsmt): Currently only supports a single window. diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index e9e1335cbbb75..3dad9f661163b 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -657,6 +657,9 @@ class RuntimeController : public PlatformConfigurationClient { // |PlatformConfigurationClient| void ScheduleFrame() override; + // |PlatformConfigurationClient| + void ForceSyncFrame() override; + // |PlatformConfigurationClient| void Render(Scene* scene, double width, double height) override; diff --git a/runtime/runtime_delegate.h b/runtime/runtime_delegate.h index bc3de031f4411..98b59b51b12d6 100644 --- a/runtime/runtime_delegate.h +++ b/runtime/runtime_delegate.h @@ -25,6 +25,8 @@ class RuntimeDelegate { virtual void ScheduleFrame(bool regenerate_layer_trees = true) = 0; + virtual void ForceSyncFrame() = 0; + virtual void Render(std::unique_ptr layer_tree, float device_pixel_ratio) = 0; diff --git a/shell/common/animator.cc b/shell/common/animator.cc index 3dd925cee6213..19f0a1388a913 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -264,6 +264,20 @@ void Animator::AwaitVSync() { } } +void Animator::ForceSyncFrame() { + TRACE_EVENT_ASYNC_BEGIN0("flutter", "Forced Frame Request Pending", + frame_request_number_); + regenerate_layer_trees_ = true; + + const fml::TimePoint frame_start_time = fml::TimePoint::Now(); + const fml::TimePoint frame_target_time = frame_start_time; + + std::unique_ptr frame_timings_recorder = + std::make_unique(); + frame_timings_recorder->RecordVsync(frame_start_time, frame_target_time); + BeginFrame(std::move(frame_timings_recorder)); +} + void Animator::ScheduleSecondaryVsyncCallback(uintptr_t id, const fml::closure& callback) { waiter_->ScheduleSecondaryCallback(id, callback); diff --git a/shell/common/animator.h b/shell/common/animator.h index be15a76b765b1..e12315317e2a6 100644 --- a/shell/common/animator.h +++ b/shell/common/animator.h @@ -53,6 +53,8 @@ class Animator final { void RequestFrame(bool regenerate_layer_trees = true); + void ForceSyncFrame(); + //-------------------------------------------------------------------------- /// @brief Tells the Animator that this frame needs to render another view. /// diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 3cc83228feea8..2e1ac3f20cdbf 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -462,6 +462,10 @@ void Engine::ScheduleFrame(bool regenerate_layer_trees) { animator_->RequestFrame(regenerate_layer_trees); } +void Engine::ForceSyncFrame() { + animator_->ForceSyncFrame(); +} + void Engine::Render(std::unique_ptr layer_tree, float device_pixel_ratio) { if (!layer_tree) { diff --git a/shell/common/engine.h b/shell/common/engine.h index 4a4b3318b149a..3cfe8b223d0e5 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -837,6 +837,9 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// tree. void ScheduleFrame() { ScheduleFrame(true); } + // |RuntimeDelegate| + void ForceSyncFrame() override; + // |RuntimeDelegate| FontCollection& GetFontCollection() override; diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 8a6cc7834552e..998a8d3c64a32 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -63,6 +63,7 @@ class MockRuntimeDelegate : public RuntimeDelegate { public: MOCK_METHOD(std::string, DefaultRouteName, (), (override)); MOCK_METHOD(void, ScheduleFrame, (bool), (override)); + MOCK_METHOD(void, ForceSyncFrame, (), (override)); MOCK_METHOD(void, Render, (std::unique_ptr, float), From f55dfdf0be8df111e854983df02bca0b7cd4afe3 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Mon, 12 Feb 2024 14:39:37 -0800 Subject: [PATCH 02/24] Web --- lib/web_ui/lib/src/engine/initialization.dart | 51 +++++++++++-------- .../lib/src/engine/platform_dispatcher.dart | 13 ++++- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/lib/web_ui/lib/src/engine/initialization.dart b/lib/web_ui/lib/src/engine/initialization.dart index 78352cf091af3..01eaad015a8f5 100644 --- a/lib/web_ui/lib/src/engine/initialization.dart +++ b/lib/web_ui/lib/src/engine/initialization.dart @@ -102,6 +102,30 @@ void debugResetEngineInitializationState() { _initializationState = DebugEngineInitializationState.uninitialized; } +void renderFrame(int timeInMicroseconds) { + frameTimingsOnVsync(); + + // In Flutter terminology "building a frame" consists of "beginning + // frame" and "drawing frame". + // + // We do not call `frameTimingsOnBuildFinish` from here because + // part of the rasterization process, particularly in the HTML + // renderer, takes place in the `SceneBuilder.build()`. + frameTimingsOnBuildStart(); + if (EnginePlatformDispatcher.instance.onBeginFrame != null) { + EnginePlatformDispatcher.instance.invokeOnBeginFrame( + Duration(microseconds: timeInMicroseconds)); + } + + if (EnginePlatformDispatcher.instance.onDrawFrame != null) { + // TODO(yjbanov): technically Flutter flushes microtasks between + // onBeginFrame and onDrawFrame. We don't, which hasn't + // been an issue yet, but eventually we'll have to + // implement it properly. + EnginePlatformDispatcher.instance.invokeOnDrawFrame(); + } +} + /// Initializes non-UI engine services. /// /// Does not put any UI onto the page. It is therefore safe to call this @@ -158,8 +182,6 @@ Future initializeEngineServices({ if (!waitingForAnimation) { waitingForAnimation = true; domWindow.requestAnimationFrame((JSNumber highResTime) { - frameTimingsOnVsync(); - // Reset immediately, because `frameHandler` can schedule more frames. waitingForAnimation = false; @@ -170,29 +192,14 @@ Future initializeEngineServices({ // microsecond precision, and only then convert to `int`. final int highResTimeMicroseconds = (1000 * highResTime.toDartDouble).toInt(); - - // In Flutter terminology "building a frame" consists of "beginning - // frame" and "drawing frame". - // - // We do not call `frameTimingsOnBuildFinish` from here because - // part of the rasterization process, particularly in the HTML - // renderer, takes place in the `SceneBuilder.build()`. - frameTimingsOnBuildStart(); - if (EnginePlatformDispatcher.instance.onBeginFrame != null) { - EnginePlatformDispatcher.instance.invokeOnBeginFrame( - Duration(microseconds: highResTimeMicroseconds)); - } - - if (EnginePlatformDispatcher.instance.onDrawFrame != null) { - // TODO(yjbanov): technically Flutter flushes microtasks between - // onBeginFrame and onDrawFrame. We don't, which hasn't - // been an issue yet, but eventually we'll have to - // implement it properly. - EnginePlatformDispatcher.instance.invokeOnDrawFrame(); - } + renderFrame(highResTimeMicroseconds); }); } }; + warmUpFrameCallback = () { + // TODO(dkwingsmt): Can we give it some time value? + renderFrame(0); + }; assetManager ??= ui_web.AssetManager(assetBase: configuration.assetBase); _setAssetManager(assetManager); diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index de8fe33a72fcc..0a7f167fa0d92 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -17,6 +17,13 @@ import '../engine.dart'; /// This may be overridden in tests, for example, to pump fake frames. ui.VoidCallback? scheduleFrameCallback; +/// Requests that the framework tries to render a frame immediately. +/// +/// Since this will probably call [PlatformWindow.render] outside of an +/// animation frame, the render will not be actually presented, but just to warm +/// up the framework. +ui.VoidCallback? warmUpFrameCallback; + /// Signature of functions added as a listener to high contrast changes typedef HighContrastListener = void Function(bool enabled); typedef _KeyDataResponseCallback = void Function(bool handled); @@ -773,8 +780,10 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { @override void forceSyncFrame() { - // TODO(dkwingsmt): Call beginFrame and drawFrame, since the framework - // will no longer call them once it switches to forceSyncFrame. + if (warmUpFrameCallback == null) { + throw Exception('warmUpFrameCallback must be initialized first.'); + } + warmUpFrameCallback!(); } /// Updates the application's rendering on the GPU with the newly provided From 7f76c5fa61ac599b2c328d1fb91fa7d70cab7afb Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Mon, 12 Feb 2024 22:28:59 -0800 Subject: [PATCH 03/24] Rename to RequestWarmUpFrame and add web time --- lib/ui/dart_ui.cc | 2 +- lib/ui/platform_dispatcher.dart | 10 +++++----- lib/ui/window/platform_configuration.cc | 7 +++++-- lib/ui/window/platform_configuration.h | 4 ++-- lib/web_ui/lib/platform_dispatcher.dart | 2 +- lib/web_ui/lib/src/engine/initialization.dart | 10 +++++----- lib/web_ui/lib/src/engine/platform_dispatcher.dart | 10 +++++----- runtime/runtime_controller.cc | 4 ++-- runtime/runtime_controller.h | 2 +- runtime/runtime_delegate.h | 2 +- shell/common/animator.cc | 2 +- shell/common/animator.h | 2 +- shell/common/engine.cc | 4 ++-- shell/common/engine.h | 2 +- shell/common/engine_unittests.cc | 2 +- 15 files changed, 34 insertions(+), 31 deletions(-) diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index 205df18059879..a78aa30764367 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -98,7 +98,7 @@ typedef CanvasPath Path; V(NativeStringAttribute::initSpellOutStringAttribute) \ V(PlatformConfigurationNativeApi::DefaultRouteName) \ V(PlatformConfigurationNativeApi::ScheduleFrame) \ - V(PlatformConfigurationNativeApi::ForceSyncFrame) \ + V(PlatformConfigurationNativeApi::RequestWarmUpFrame) \ V(PlatformConfigurationNativeApi::Render) \ V(PlatformConfigurationNativeApi::UpdateSemantics) \ V(PlatformConfigurationNativeApi::SetNeedsReportTimings) \ diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 4153f8f3cd018..376ae7f3ecf76 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -801,7 +801,7 @@ class PlatformDispatcher { /// /// * [SchedulerBinding], the Flutter framework class which manages the /// scheduling of frames. - /// * [forceSyncFrame], a similar method that is used in rare cases that + /// * [requestWarmUpFrame], a similar method that is used in rare cases that /// a frame must be rendered immediately. void scheduleFrame() => _scheduleFrame(); @@ -816,7 +816,7 @@ class PlatformDispatcher { /// completed synchronously within this call. /// /// Prefer [scheduleFrame] to update the display in normal operation. The - /// [forceSyncFrame] method is designed for situations that require a frame is + /// [requestWarmUpFrame] method is designed for situations that require a frame is /// rendered as soon as possible, even at the cost of rendering quality. An /// example of using this method is [SchedulerBinding.scheduleWarmUpFrame], /// which is called during application startup so that the first frame can be @@ -826,10 +826,10 @@ class PlatformDispatcher { /// /// * [SchedulerBinding.scheduleWarmUpFrame], which uses this method. /// * [scheduleFrame]. - void forceSyncFrame() => _forceSyncFrame(); + void requestWarmUpFrame() => _requestWarmUpFrame(); - @Native(symbol: 'PlatformConfigurationNativeApi::ForceSyncFrame') - external static void _forceSyncFrame(); + @Native(symbol: 'PlatformConfigurationNativeApi::RequestWarmUpFrame') + external static void _requestWarmUpFrame(); /// Additional accessibility features that may be enabled by the platform. AccessibilityFeatures get accessibilityFeatures => _configuration.accessibilityFeatures; diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index b5f7d99b27a6c..f025d5ac4fb8b 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -589,9 +589,12 @@ void PlatformConfigurationNativeApi::ScheduleFrame() { UIDartState::Current()->platform_configuration()->client()->ScheduleFrame(); } -void PlatformConfigurationNativeApi::ForceSyncFrame() { +void PlatformConfigurationNativeApi::RequestWarmUpFrame() { UIDartState::ThrowIfUIOperationsProhibited(); - UIDartState::Current()->platform_configuration()->client()->ForceSyncFrame(); + UIDartState::Current() + ->platform_configuration() + ->client() + ->RequestWarmUpFrame(); } void PlatformConfigurationNativeApi::UpdateSemantics(SemanticsUpdate* update) { diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index f4f8252181705..6211949251b94 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -68,7 +68,7 @@ class PlatformConfigurationClient { //-------------------------------------------------------------------------- /// @brief /// - virtual void ForceSyncFrame() = 0; + virtual void RequestWarmUpFrame() = 0; //-------------------------------------------------------------------------- /// @brief Updates the client's rendering on the GPU with the newly @@ -562,7 +562,7 @@ class PlatformConfigurationNativeApi { static void ScheduleFrame(); - static void ForceSyncFrame(); + static void RequestWarmUpFrame(); static void Render(int64_t view_id, Scene* scene, diff --git a/lib/web_ui/lib/platform_dispatcher.dart b/lib/web_ui/lib/platform_dispatcher.dart index 875318eb61bc2..0738602f0eb97 100644 --- a/lib/web_ui/lib/platform_dispatcher.dart +++ b/lib/web_ui/lib/platform_dispatcher.dart @@ -90,7 +90,7 @@ abstract class PlatformDispatcher { void scheduleFrame(); - void forceSyncFrame(); + void requestWarmUpFrame(); Future render(Scene scene, [FlutterView view]); diff --git a/lib/web_ui/lib/src/engine/initialization.dart b/lib/web_ui/lib/src/engine/initialization.dart index 01eaad015a8f5..0c009b0a29204 100644 --- a/lib/web_ui/lib/src/engine/initialization.dart +++ b/lib/web_ui/lib/src/engine/initialization.dart @@ -102,7 +102,7 @@ void debugResetEngineInitializationState() { _initializationState = DebugEngineInitializationState.uninitialized; } -void renderFrame(int timeInMicroseconds) { +void renderFrame(int timeMicroseconds) { frameTimingsOnVsync(); // In Flutter terminology "building a frame" consists of "beginning @@ -114,7 +114,7 @@ void renderFrame(int timeInMicroseconds) { frameTimingsOnBuildStart(); if (EnginePlatformDispatcher.instance.onBeginFrame != null) { EnginePlatformDispatcher.instance.invokeOnBeginFrame( - Duration(microseconds: timeInMicroseconds)); + Duration(microseconds: timeMicroseconds)); } if (EnginePlatformDispatcher.instance.onDrawFrame != null) { @@ -196,9 +196,9 @@ Future initializeEngineServices({ }); } }; - warmUpFrameCallback = () { - // TODO(dkwingsmt): Can we give it some time value? - renderFrame(0); + requestWarmUpFrameCallback = () { + final int timeMicroseconds = (1000 * domWindow.performance.now()).toInt(); + renderFrame(timeMicroseconds); }; assetManager ??= ui_web.AssetManager(assetBase: configuration.assetBase); diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index 0a7f167fa0d92..c0eea35290600 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -22,7 +22,7 @@ ui.VoidCallback? scheduleFrameCallback; /// Since this will probably call [PlatformWindow.render] outside of an /// animation frame, the render will not be actually presented, but just to warm /// up the framework. -ui.VoidCallback? warmUpFrameCallback; +ui.VoidCallback? requestWarmUpFrameCallback; /// Signature of functions added as a listener to high contrast changes typedef HighContrastListener = void Function(bool enabled); @@ -779,11 +779,11 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { } @override - void forceSyncFrame() { - if (warmUpFrameCallback == null) { - throw Exception('warmUpFrameCallback must be initialized first.'); + void requestWarmUpFrame() { + if (requestWarmUpFrameCallback == null) { + throw Exception('requestWarmUpFrameCallback must be initialized first.'); } - warmUpFrameCallback!(); + requestWarmUpFrameCallback!(); } /// Updates the application's rendering on the GPU with the newly provided diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index fc9ea5fbca93d..64911a900d2cf 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -341,8 +341,8 @@ void RuntimeController::ScheduleFrame() { } // |PlatformConfigurationClient| -void RuntimeController::ForceSyncFrame() { - client_.ForceSyncFrame(); +void RuntimeController::RequestWarmUpFrame() { + client_.RequestWarmUpFrame(); } // |PlatformConfigurationClient| diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 3dad9f661163b..6e0394e6f7669 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -658,7 +658,7 @@ class RuntimeController : public PlatformConfigurationClient { void ScheduleFrame() override; // |PlatformConfigurationClient| - void ForceSyncFrame() override; + void RequestWarmUpFrame() override; // |PlatformConfigurationClient| void Render(Scene* scene, double width, double height) override; diff --git a/runtime/runtime_delegate.h b/runtime/runtime_delegate.h index 98b59b51b12d6..22e9d3942732c 100644 --- a/runtime/runtime_delegate.h +++ b/runtime/runtime_delegate.h @@ -25,7 +25,7 @@ class RuntimeDelegate { virtual void ScheduleFrame(bool regenerate_layer_trees = true) = 0; - virtual void ForceSyncFrame() = 0; + virtual void RequestWarmUpFrame() = 0; virtual void Render(std::unique_ptr layer_tree, float device_pixel_ratio) = 0; diff --git a/shell/common/animator.cc b/shell/common/animator.cc index 19f0a1388a913..572a0804e57e0 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -264,7 +264,7 @@ void Animator::AwaitVSync() { } } -void Animator::ForceSyncFrame() { +void Animator::RequestWarmUpFrame() { TRACE_EVENT_ASYNC_BEGIN0("flutter", "Forced Frame Request Pending", frame_request_number_); regenerate_layer_trees_ = true; diff --git a/shell/common/animator.h b/shell/common/animator.h index e12315317e2a6..95f5476e83e07 100644 --- a/shell/common/animator.h +++ b/shell/common/animator.h @@ -53,7 +53,7 @@ class Animator final { void RequestFrame(bool regenerate_layer_trees = true); - void ForceSyncFrame(); + void RequestWarmUpFrame(); //-------------------------------------------------------------------------- /// @brief Tells the Animator that this frame needs to render another view. diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 2e1ac3f20cdbf..1ec234410264b 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -462,8 +462,8 @@ void Engine::ScheduleFrame(bool regenerate_layer_trees) { animator_->RequestFrame(regenerate_layer_trees); } -void Engine::ForceSyncFrame() { - animator_->ForceSyncFrame(); +void Engine::RequestWarmUpFrame() { + animator_->RequestWarmUpFrame(); } void Engine::Render(std::unique_ptr layer_tree, diff --git a/shell/common/engine.h b/shell/common/engine.h index 3cfe8b223d0e5..7cd4e9069eb63 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -838,7 +838,7 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { void ScheduleFrame() { ScheduleFrame(true); } // |RuntimeDelegate| - void ForceSyncFrame() override; + void RequestWarmUpFrame() override; // |RuntimeDelegate| FontCollection& GetFontCollection() override; diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 998a8d3c64a32..bf9c3a90ac334 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -63,7 +63,7 @@ class MockRuntimeDelegate : public RuntimeDelegate { public: MOCK_METHOD(std::string, DefaultRouteName, (), (override)); MOCK_METHOD(void, ScheduleFrame, (bool), (override)); - MOCK_METHOD(void, ForceSyncFrame, (), (override)); + MOCK_METHOD(void, RequestWarmUpFrame, (), (override)); MOCK_METHOD(void, Render, (std::unique_ptr, float), From 31a5ea333a0d775672b5e2a1a75c2ae4bf17ce4c Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 13 Feb 2024 14:25:41 -0800 Subject: [PATCH 04/24] Change to scheduleWarmUpFrame and EndWarmUpFrame --- lib/ui/dart_ui.cc | 2 +- lib/ui/platform_dispatcher.dart | 16 ++++-- lib/ui/window/platform_configuration.cc | 7 +-- lib/ui/window/platform_configuration.h | 4 +- lib/web_ui/lib/platform_dispatcher.dart | 2 +- lib/web_ui/lib/src/engine/initialization.dart | 54 +++++++++---------- .../lib/src/engine/platform_dispatcher.dart | 14 +++-- runtime/runtime_controller.cc | 4 +- runtime/runtime_controller.h | 2 +- runtime/runtime_delegate.h | 2 +- shell/common/animator.cc | 16 ++---- shell/common/animator.h | 2 +- shell/common/engine.cc | 4 +- shell/common/engine.h | 2 +- shell/common/engine_unittests.cc | 2 +- 15 files changed, 65 insertions(+), 68 deletions(-) diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index a78aa30764367..59d141852f654 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -98,7 +98,7 @@ typedef CanvasPath Path; V(NativeStringAttribute::initSpellOutStringAttribute) \ V(PlatformConfigurationNativeApi::DefaultRouteName) \ V(PlatformConfigurationNativeApi::ScheduleFrame) \ - V(PlatformConfigurationNativeApi::RequestWarmUpFrame) \ + V(PlatformConfigurationNativeApi::EndWarmUpFrame) \ V(PlatformConfigurationNativeApi::Render) \ V(PlatformConfigurationNativeApi::UpdateSemantics) \ V(PlatformConfigurationNativeApi::SetNeedsReportTimings) \ diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 376ae7f3ecf76..f71e84a051212 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -801,7 +801,7 @@ class PlatformDispatcher { /// /// * [SchedulerBinding], the Flutter framework class which manages the /// scheduling of frames. - /// * [requestWarmUpFrame], a similar method that is used in rare cases that + /// * [scheduleWarmUpFrame], a similar method that is used in rare cases that /// a frame must be rendered immediately. void scheduleFrame() => _scheduleFrame(); @@ -816,7 +816,7 @@ class PlatformDispatcher { /// completed synchronously within this call. /// /// Prefer [scheduleFrame] to update the display in normal operation. The - /// [requestWarmUpFrame] method is designed for situations that require a frame is + /// [scheduleWarmUpFrame] method is designed for situations that require a frame is /// rendered as soon as possible, even at the cost of rendering quality. An /// example of using this method is [SchedulerBinding.scheduleWarmUpFrame], /// which is called during application startup so that the first frame can be @@ -826,10 +826,16 @@ class PlatformDispatcher { /// /// * [SchedulerBinding.scheduleWarmUpFrame], which uses this method. /// * [scheduleFrame]. - void requestWarmUpFrame() => _requestWarmUpFrame(); + void scheduleWarmUpFrame(VoidCallback beginFrameCallback, VoidCallback drawFrameCallback) { + Timer.run(beginFrameCallback); + Timer.run(() { + drawFrameCallback(); + _endWarmUpFrame(); + }); + } - @Native(symbol: 'PlatformConfigurationNativeApi::RequestWarmUpFrame') - external static void _requestWarmUpFrame(); + @Native(symbol: 'PlatformConfigurationNativeApi::EndWarmUpFrame') + external static void _endWarmUpFrame(); /// Additional accessibility features that may be enabled by the platform. AccessibilityFeatures get accessibilityFeatures => _configuration.accessibilityFeatures; diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index f025d5ac4fb8b..5e325fbdf4b10 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -589,12 +589,9 @@ void PlatformConfigurationNativeApi::ScheduleFrame() { UIDartState::Current()->platform_configuration()->client()->ScheduleFrame(); } -void PlatformConfigurationNativeApi::RequestWarmUpFrame() { +void PlatformConfigurationNativeApi::EndWarmUpFrame() { UIDartState::ThrowIfUIOperationsProhibited(); - UIDartState::Current() - ->platform_configuration() - ->client() - ->RequestWarmUpFrame(); + UIDartState::Current()->platform_configuration()->client()->EndWarmUpFrame(); } void PlatformConfigurationNativeApi::UpdateSemantics(SemanticsUpdate* update) { diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index 6211949251b94..ba40dad1fe990 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -68,7 +68,7 @@ class PlatformConfigurationClient { //-------------------------------------------------------------------------- /// @brief /// - virtual void RequestWarmUpFrame() = 0; + virtual void EndWarmUpFrame() = 0; //-------------------------------------------------------------------------- /// @brief Updates the client's rendering on the GPU with the newly @@ -562,7 +562,7 @@ class PlatformConfigurationNativeApi { static void ScheduleFrame(); - static void RequestWarmUpFrame(); + static void EndWarmUpFrame(); static void Render(int64_t view_id, Scene* scene, diff --git a/lib/web_ui/lib/platform_dispatcher.dart b/lib/web_ui/lib/platform_dispatcher.dart index 0738602f0eb97..a8347f76cf0ab 100644 --- a/lib/web_ui/lib/platform_dispatcher.dart +++ b/lib/web_ui/lib/platform_dispatcher.dart @@ -90,7 +90,7 @@ abstract class PlatformDispatcher { void scheduleFrame(); - void requestWarmUpFrame(); + void scheduleWarmUpFrame(VoidCallback beginFrameCallback, VoidCallback drawFrameCallback); Future render(Scene scene, [FlutterView view]); diff --git a/lib/web_ui/lib/src/engine/initialization.dart b/lib/web_ui/lib/src/engine/initialization.dart index 0c009b0a29204..10b71219a9dc9 100644 --- a/lib/web_ui/lib/src/engine/initialization.dart +++ b/lib/web_ui/lib/src/engine/initialization.dart @@ -102,30 +102,6 @@ void debugResetEngineInitializationState() { _initializationState = DebugEngineInitializationState.uninitialized; } -void renderFrame(int timeMicroseconds) { - frameTimingsOnVsync(); - - // In Flutter terminology "building a frame" consists of "beginning - // frame" and "drawing frame". - // - // We do not call `frameTimingsOnBuildFinish` from here because - // part of the rasterization process, particularly in the HTML - // renderer, takes place in the `SceneBuilder.build()`. - frameTimingsOnBuildStart(); - if (EnginePlatformDispatcher.instance.onBeginFrame != null) { - EnginePlatformDispatcher.instance.invokeOnBeginFrame( - Duration(microseconds: timeMicroseconds)); - } - - if (EnginePlatformDispatcher.instance.onDrawFrame != null) { - // TODO(yjbanov): technically Flutter flushes microtasks between - // onBeginFrame and onDrawFrame. We don't, which hasn't - // been an issue yet, but eventually we'll have to - // implement it properly. - EnginePlatformDispatcher.instance.invokeOnDrawFrame(); - } -} - /// Initializes non-UI engine services. /// /// Does not put any UI onto the page. It is therefore safe to call this @@ -182,6 +158,8 @@ Future initializeEngineServices({ if (!waitingForAnimation) { waitingForAnimation = true; domWindow.requestAnimationFrame((JSNumber highResTime) { + frameTimingsOnVsync(); + // Reset immediately, because `frameHandler` can schedule more frames. waitingForAnimation = false; @@ -192,13 +170,33 @@ Future initializeEngineServices({ // microsecond precision, and only then convert to `int`. final int highResTimeMicroseconds = (1000 * highResTime.toDartDouble).toInt(); - renderFrame(highResTimeMicroseconds); + + // In Flutter terminology "building a frame" consists of "beginning + // frame" and "drawing frame". + // + // We do not call `frameTimingsOnBuildFinish` from here because + // part of the rasterization process, particularly in the HTML + // renderer, takes place in the `SceneBuilder.build()`. + frameTimingsOnBuildStart(); + if (EnginePlatformDispatcher.instance.onBeginFrame != null) { + EnginePlatformDispatcher.instance.invokeOnBeginFrame( + Duration(microseconds: highResTimeMicroseconds)); + } + + if (EnginePlatformDispatcher.instance.onDrawFrame != null) { + // TODO(yjbanov): technically Flutter flushes microtasks between + // onBeginFrame and onDrawFrame. We don't, which hasn't + // been an issue yet, but eventually we'll have to + // implement it properly. + EnginePlatformDispatcher.instance.invokeOnDrawFrame(); + } }); } }; - requestWarmUpFrameCallback = () { - final int timeMicroseconds = (1000 * domWindow.performance.now()).toInt(); - renderFrame(timeMicroseconds); + + scheduleWarmUpFrameCallback = (ui.VoidCallback beginFrameCallback, ui.VoidCallback drawFrameCallback) { + Timer.run(beginFrameCallback); + Timer.run(drawFrameCallback); }; assetManager ??= ui_web.AssetManager(assetBase: configuration.assetBase); diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index c0eea35290600..3c87693757154 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -17,12 +17,16 @@ import '../engine.dart'; /// This may be overridden in tests, for example, to pump fake frames. ui.VoidCallback? scheduleFrameCallback; +/// Signature of the function to schedule a warm up frame with the specified +/// frame callbacks. +typedef ScheduleWarmUpFrameCallback = void Function(ui.VoidCallback beginFrameCallback, ui.VoidCallback drawFrameCallback); + /// Requests that the framework tries to render a frame immediately. /// /// Since this will probably call [PlatformWindow.render] outside of an /// animation frame, the render will not be actually presented, but just to warm /// up the framework. -ui.VoidCallback? requestWarmUpFrameCallback; +ScheduleWarmUpFrameCallback? scheduleWarmUpFrameCallback; /// Signature of functions added as a listener to high contrast changes typedef HighContrastListener = void Function(bool enabled); @@ -779,11 +783,11 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { } @override - void requestWarmUpFrame() { - if (requestWarmUpFrameCallback == null) { - throw Exception('requestWarmUpFrameCallback must be initialized first.'); + void scheduleWarmUpFrame(ui.VoidCallback beginFrameCallback, ui.VoidCallback drawFrameCallback) { + if (scheduleWarmUpFrameCallback == null) { + throw Exception('scheduleWarmUpFrameCallback must be initialized first.'); } - requestWarmUpFrameCallback!(); + scheduleWarmUpFrameCallback!(beginFrameCallback, drawFrameCallback); } /// Updates the application's rendering on the GPU with the newly provided diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 64911a900d2cf..85a3bd9efaac5 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -341,8 +341,8 @@ void RuntimeController::ScheduleFrame() { } // |PlatformConfigurationClient| -void RuntimeController::RequestWarmUpFrame() { - client_.RequestWarmUpFrame(); +void RuntimeController::EndWarmUpFrame() { + client_.EndWarmUpFrame(); } // |PlatformConfigurationClient| diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 6e0394e6f7669..b68750df8751e 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -658,7 +658,7 @@ class RuntimeController : public PlatformConfigurationClient { void ScheduleFrame() override; // |PlatformConfigurationClient| - void RequestWarmUpFrame() override; + void EndWarmUpFrame() override; // |PlatformConfigurationClient| void Render(Scene* scene, double width, double height) override; diff --git a/runtime/runtime_delegate.h b/runtime/runtime_delegate.h index 22e9d3942732c..6d3707d27737a 100644 --- a/runtime/runtime_delegate.h +++ b/runtime/runtime_delegate.h @@ -25,7 +25,7 @@ class RuntimeDelegate { virtual void ScheduleFrame(bool regenerate_layer_trees = true) = 0; - virtual void RequestWarmUpFrame() = 0; + virtual void EndWarmUpFrame() = 0; virtual void Render(std::unique_ptr layer_tree, float device_pixel_ratio) = 0; diff --git a/shell/common/animator.cc b/shell/common/animator.cc index 572a0804e57e0..ad3cea4d1adf2 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -264,18 +264,10 @@ void Animator::AwaitVSync() { } } -void Animator::RequestWarmUpFrame() { - TRACE_EVENT_ASYNC_BEGIN0("flutter", "Forced Frame Request Pending", - frame_request_number_); - regenerate_layer_trees_ = true; - - const fml::TimePoint frame_start_time = fml::TimePoint::Now(); - const fml::TimePoint frame_target_time = frame_start_time; - - std::unique_ptr frame_timings_recorder = - std::make_unique(); - frame_timings_recorder->RecordVsync(frame_start_time, frame_target_time); - BeginFrame(std::move(frame_timings_recorder)); +void Animator::EndWarmUpFrame() { + // Do nothing. The warm up frame does not need any additional work to end the + // frame for now. This will change once the pipeline supports multi-view. + // https://github.com/flutter/flutter/issues/142851 } void Animator::ScheduleSecondaryVsyncCallback(uintptr_t id, diff --git a/shell/common/animator.h b/shell/common/animator.h index 95f5476e83e07..cee8c3cdbe01a 100644 --- a/shell/common/animator.h +++ b/shell/common/animator.h @@ -53,7 +53,7 @@ class Animator final { void RequestFrame(bool regenerate_layer_trees = true); - void RequestWarmUpFrame(); + void EndWarmUpFrame(); //-------------------------------------------------------------------------- /// @brief Tells the Animator that this frame needs to render another view. diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 1ec234410264b..c7af2131373a7 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -462,8 +462,8 @@ void Engine::ScheduleFrame(bool regenerate_layer_trees) { animator_->RequestFrame(regenerate_layer_trees); } -void Engine::RequestWarmUpFrame() { - animator_->RequestWarmUpFrame(); +void Engine::EndWarmUpFrame() { + animator_->EndWarmUpFrame(); } void Engine::Render(std::unique_ptr layer_tree, diff --git a/shell/common/engine.h b/shell/common/engine.h index 7cd4e9069eb63..050101773766f 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -838,7 +838,7 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { void ScheduleFrame() { ScheduleFrame(true); } // |RuntimeDelegate| - void RequestWarmUpFrame() override; + void EndWarmUpFrame() override; // |RuntimeDelegate| FontCollection& GetFontCollection() override; diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index bf9c3a90ac334..35212111c50c0 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -63,7 +63,7 @@ class MockRuntimeDelegate : public RuntimeDelegate { public: MOCK_METHOD(std::string, DefaultRouteName, (), (override)); MOCK_METHOD(void, ScheduleFrame, (bool), (override)); - MOCK_METHOD(void, RequestWarmUpFrame, (), (override)); + MOCK_METHOD(void, EndWarmUpFrame, (), (override)); MOCK_METHOD(void, Render, (std::unique_ptr, float), From 590287dd445f5a52050d8e622cb7bcfce78310b2 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 13 Feb 2024 15:08:08 -0800 Subject: [PATCH 05/24] Comment --- lib/ui/platform_dispatcher.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index f71e84a051212..bcd0febc48097 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -827,6 +827,7 @@ class PlatformDispatcher { /// * [SchedulerBinding.scheduleWarmUpFrame], which uses this method. /// * [scheduleFrame]. void scheduleWarmUpFrame(VoidCallback beginFrameCallback, VoidCallback drawFrameCallback) { + // We use timers here to ensure that microtasks flush in between. Timer.run(beginFrameCallback); Timer.run(() { drawFrameCallback(); From 40b269ba013734fb6a5b03669b2f45e490ebdbd4 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 13 Feb 2024 15:30:13 -0800 Subject: [PATCH 06/24] Doc --- lib/ui/platform_dispatcher.dart | 35 ++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index bcd0febc48097..7f2c54e51fbcc 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -808,29 +808,32 @@ class PlatformDispatcher { @Native(symbol: 'PlatformConfigurationNativeApi::ScheduleFrame') external static void _scheduleFrame(); - /// Immediately render a frame by invoking the [onBeginFrame] and - /// [onDrawFrame] callbacks synchronously. + /// Schedule a frame to run as soon as possible, rather than waiting for the + /// engine to request a frame in response to a system "Vsync" signal. /// - /// This method performs the same computation for a frame as [scheduleFrame] - /// does, but instead of doing so at an appropriate opportunity, the render is - /// completed synchronously within this call. + /// This method is used during application startup so that the first frame + /// (which is likely to be quite expensive) can start a few extra milliseconds + /// earlier. Using it in other situations might lead to unintended results, + /// such as screen tearing. Depending on the platform, the warm up frame + /// might or might not be actually rendered to the screen. /// - /// Prefer [scheduleFrame] to update the display in normal operation. The - /// [scheduleWarmUpFrame] method is designed for situations that require a frame is - /// rendered as soon as possible, even at the cost of rendering quality. An - /// example of using this method is [SchedulerBinding.scheduleWarmUpFrame], - /// which is called during application startup so that the first frame can be - /// presented to the screen a few extra milliseconds earlier. + /// For more introduction to the warm up frame, see + /// [SchedulerBinding.scheduleWarmUpFrame]. + /// + /// This method uses the provided callbacks as the begin frame callback and + /// the draw frame callback instead of [onBeginFrame] and [onDrawFrame]. /// /// See also: /// - /// * [SchedulerBinding.scheduleWarmUpFrame], which uses this method. - /// * [scheduleFrame]. - void scheduleWarmUpFrame(VoidCallback beginFrameCallback, VoidCallback drawFrameCallback) { + /// * [SchedulerBinding.scheduleWarmUpFrame], which uses this method, and + /// introduces the warm up frame in more details. + /// * [scheduleFrame], which schedules the frame at the next appropriate + /// opportunity and should be used to render regular frames. + void scheduleWarmUpFrame({required VoidCallback beginFrame, required VoidCallback drawFrame}) { // We use timers here to ensure that microtasks flush in between. - Timer.run(beginFrameCallback); + Timer.run(beginFrame); Timer.run(() { - drawFrameCallback(); + drawFrame(); _endWarmUpFrame(); }); } From 2d70a0f3b8caf3ef39922ac09c62b7f90e5fcaaf Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 13 Feb 2024 16:32:33 -0800 Subject: [PATCH 07/24] Simplify web implementation --- lib/web_ui/lib/platform_dispatcher.dart | 2 +- lib/web_ui/lib/src/engine/initialization.dart | 8 ++----- .../lib/src/engine/platform_dispatcher.dart | 24 +++++++------------ 3 files changed, 11 insertions(+), 23 deletions(-) diff --git a/lib/web_ui/lib/platform_dispatcher.dart b/lib/web_ui/lib/platform_dispatcher.dart index a8347f76cf0ab..18fa3d02a3ebd 100644 --- a/lib/web_ui/lib/platform_dispatcher.dart +++ b/lib/web_ui/lib/platform_dispatcher.dart @@ -90,7 +90,7 @@ abstract class PlatformDispatcher { void scheduleFrame(); - void scheduleWarmUpFrame(VoidCallback beginFrameCallback, VoidCallback drawFrameCallback); + void scheduleWarmUpFrame({required VoidCallback beginFrame, required VoidCallback drawFrame}); Future render(Scene scene, [FlutterView view]); diff --git a/lib/web_ui/lib/src/engine/initialization.dart b/lib/web_ui/lib/src/engine/initialization.dart index 10b71219a9dc9..d43fe955427bd 100644 --- a/lib/web_ui/lib/src/engine/initialization.dart +++ b/lib/web_ui/lib/src/engine/initialization.dart @@ -187,18 +187,14 @@ Future initializeEngineServices({ // TODO(yjbanov): technically Flutter flushes microtasks between // onBeginFrame and onDrawFrame. We don't, which hasn't // been an issue yet, but eventually we'll have to - // implement it properly. + // implement it properly. (Same TODO as in + // `EnginePlatformDispatcher.scheduleWarmUpFrame`). EnginePlatformDispatcher.instance.invokeOnDrawFrame(); } }); } }; - scheduleWarmUpFrameCallback = (ui.VoidCallback beginFrameCallback, ui.VoidCallback drawFrameCallback) { - Timer.run(beginFrameCallback); - Timer.run(drawFrameCallback); - }; - assetManager ??= ui_web.AssetManager(assetBase: configuration.assetBase); _setAssetManager(assetManager); diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index 3c87693757154..4657c4a2518aa 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -17,17 +17,6 @@ import '../engine.dart'; /// This may be overridden in tests, for example, to pump fake frames. ui.VoidCallback? scheduleFrameCallback; -/// Signature of the function to schedule a warm up frame with the specified -/// frame callbacks. -typedef ScheduleWarmUpFrameCallback = void Function(ui.VoidCallback beginFrameCallback, ui.VoidCallback drawFrameCallback); - -/// Requests that the framework tries to render a frame immediately. -/// -/// Since this will probably call [PlatformWindow.render] outside of an -/// animation frame, the render will not be actually presented, but just to warm -/// up the framework. -ScheduleWarmUpFrameCallback? scheduleWarmUpFrameCallback; - /// Signature of functions added as a listener to high contrast changes typedef HighContrastListener = void Function(bool enabled); typedef _KeyDataResponseCallback = void Function(bool handled); @@ -783,11 +772,14 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { } @override - void scheduleWarmUpFrame(ui.VoidCallback beginFrameCallback, ui.VoidCallback drawFrameCallback) { - if (scheduleWarmUpFrameCallback == null) { - throw Exception('scheduleWarmUpFrameCallback must be initialized first.'); - } - scheduleWarmUpFrameCallback!(beginFrameCallback, drawFrameCallback); + void scheduleWarmUpFrame({required ui.VoidCallback beginFrame, required ui.VoidCallback drawFrame}) { + Timer.run(beginFrame); + // TODO(yjbanov): technically Flutter flushes microtasks between + // onBeginFrame and onDrawFrame. We don't, which hasn't been + // an issue yet, but eventually we'll have to implement it + // properly. (Same TODO as in `initializeEngineServices` in + // initialization.dart). + Timer.run(drawFrame); } /// Updates the application's rendering on the GPU with the newly provided From 4af9f069e1b70aaf7206f69ef6f363d9fd9c2073 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 13 Feb 2024 16:38:05 -0800 Subject: [PATCH 08/24] Fix comment --- lib/ui/platform_dispatcher.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 7f2c54e51fbcc..342f2525acc9e 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -801,8 +801,8 @@ class PlatformDispatcher { /// /// * [SchedulerBinding], the Flutter framework class which manages the /// scheduling of frames. - /// * [scheduleWarmUpFrame], a similar method that is used in rare cases that - /// a frame must be rendered immediately. + /// * [scheduleWarmUpFrame], which should only be used to schedule warm up + /// frames. void scheduleFrame() => _scheduleFrame(); @Native(symbol: 'PlatformConfigurationNativeApi::ScheduleFrame') From bfe55701bb141badbcf88f85df87dd8d14812513 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 13 Feb 2024 16:49:09 -0800 Subject: [PATCH 09/24] More doc --- lib/ui/platform_dispatcher.dart | 4 ++-- shell/common/animator.h | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 342f2525acc9e..09ed91ebd1444 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -814,8 +814,8 @@ class PlatformDispatcher { /// This method is used during application startup so that the first frame /// (which is likely to be quite expensive) can start a few extra milliseconds /// earlier. Using it in other situations might lead to unintended results, - /// such as screen tearing. Depending on the platform, the warm up frame - /// might or might not be actually rendered to the screen. + /// such as screen tearing. Depending on platforms and situations, the warm up + /// frame might or might not be actually rendered onto the screen. /// /// For more introduction to the warm up frame, see /// [SchedulerBinding.scheduleWarmUpFrame]. diff --git a/shell/common/animator.h b/shell/common/animator.h index cee8c3cdbe01a..b2aa53c9d672f 100644 --- a/shell/common/animator.h +++ b/shell/common/animator.h @@ -53,6 +53,20 @@ class Animator final { void RequestFrame(bool regenerate_layer_trees = true); + //-------------------------------------------------------------------------- + /// @brief Tells the Animator that a warm up frame has ended. + /// + /// In a warm up frame, Animator::Render is called out of a vsync + /// task, and Animator requires this explicit end-of-frame call to + /// know when to send the layer trees to the pipeline. + /// + /// This is different from regular frames, where Animator::Render is + /// always called within a vsync task, and Animator can send + /// the views at the end of the vsync task. + /// + /// For more about warm up frames, see + /// `PlatformDispatcher.scheduleWarmUpFrame`. + /// void EndWarmUpFrame(); //-------------------------------------------------------------------------- From 21439c4c53a2e9ff22d2933959c3da0179d5c69d Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 13 Feb 2024 16:49:57 -0800 Subject: [PATCH 10/24] Fix doc --- shell/common/animator.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/common/animator.cc b/shell/common/animator.cc index ad3cea4d1adf2..e5e37be779be6 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -265,7 +265,7 @@ void Animator::AwaitVSync() { } void Animator::EndWarmUpFrame() { - // Do nothing. The warm up frame does not need any additional work to end the + // Do nothing. The warm up frame does not need any additional work for end the // frame for now. This will change once the pipeline supports multi-view. // https://github.com/flutter/flutter/issues/142851 } From 1bd21db9b72bf35a24b1b16054d843bd5796e649 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 13 Feb 2024 16:54:03 -0800 Subject: [PATCH 11/24] More doc --- lib/ui/window/platform_configuration.h | 4 +++- shell/common/animator.h | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index ba40dad1fe990..8914bbb756d40 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -66,7 +66,9 @@ class PlatformConfigurationClient { virtual void ScheduleFrame() = 0; //-------------------------------------------------------------------------- - /// @brief + /// @brief Called when a warm up frame has ended. + /// + /// For more introduction, see `Animator::EndWarmUpFrame`. /// virtual void EndWarmUpFrame() = 0; diff --git a/shell/common/animator.h b/shell/common/animator.h index b2aa53c9d672f..51bea1d273871 100644 --- a/shell/common/animator.h +++ b/shell/common/animator.h @@ -56,8 +56,8 @@ class Animator final { //-------------------------------------------------------------------------- /// @brief Tells the Animator that a warm up frame has ended. /// - /// In a warm up frame, Animator::Render is called out of a vsync - /// task, and Animator requires this explicit end-of-frame call to + /// In a warm up frame, `Animator::Render` is called out of vsync + /// tasks, and Animator requires an explicit end-of-frame call to /// know when to send the layer trees to the pipeline. /// /// This is different from regular frames, where Animator::Render is From 4f41658a4e2603fa1304044d26be1568d03ea21e Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 13 Feb 2024 23:20:13 -0800 Subject: [PATCH 12/24] Fix linter --- lib/web_ui/lib/src/engine/initialization.dart | 3 ++- lib/web_ui/lib/src/engine/platform_dispatcher.dart | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/web_ui/lib/src/engine/initialization.dart b/lib/web_ui/lib/src/engine/initialization.dart index d43fe955427bd..1e852018b26f8 100644 --- a/lib/web_ui/lib/src/engine/initialization.dart +++ b/lib/web_ui/lib/src/engine/initialization.dart @@ -187,7 +187,8 @@ Future initializeEngineServices({ // TODO(yjbanov): technically Flutter flushes microtasks between // onBeginFrame and onDrawFrame. We don't, which hasn't // been an issue yet, but eventually we'll have to - // implement it properly. (Same TODO as in + // implement it properly. (This is the same to-do as + // the one in // `EnginePlatformDispatcher.scheduleWarmUpFrame`). EnginePlatformDispatcher.instance.invokeOnDrawFrame(); } diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index 4657c4a2518aa..cb5f41739358b 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -777,8 +777,8 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { // TODO(yjbanov): technically Flutter flushes microtasks between // onBeginFrame and onDrawFrame. We don't, which hasn't been // an issue yet, but eventually we'll have to implement it - // properly. (Same TODO as in `initializeEngineServices` in - // initialization.dart). + // properly. (This is the same to-do as the one in + // `initializeEngineServices` in initialization.dart). Timer.run(drawFrame); } From eeac064cd62c95803ebb9240961bd4538ca14392 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 14 Feb 2024 11:38:23 -0800 Subject: [PATCH 13/24] Fix comment --- lib/web_ui/lib/src/engine/initialization.dart | 4 +--- lib/web_ui/lib/src/engine/platform_dispatcher.dart | 6 +----- shell/common/animator.cc | 2 +- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/lib/web_ui/lib/src/engine/initialization.dart b/lib/web_ui/lib/src/engine/initialization.dart index 1e852018b26f8..78352cf091af3 100644 --- a/lib/web_ui/lib/src/engine/initialization.dart +++ b/lib/web_ui/lib/src/engine/initialization.dart @@ -187,9 +187,7 @@ Future initializeEngineServices({ // TODO(yjbanov): technically Flutter flushes microtasks between // onBeginFrame and onDrawFrame. We don't, which hasn't // been an issue yet, but eventually we'll have to - // implement it properly. (This is the same to-do as - // the one in - // `EnginePlatformDispatcher.scheduleWarmUpFrame`). + // implement it properly. EnginePlatformDispatcher.instance.invokeOnDrawFrame(); } }); diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index cb5f41739358b..2fc4fbe7550a1 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -774,11 +774,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { @override void scheduleWarmUpFrame({required ui.VoidCallback beginFrame, required ui.VoidCallback drawFrame}) { Timer.run(beginFrame); - // TODO(yjbanov): technically Flutter flushes microtasks between - // onBeginFrame and onDrawFrame. We don't, which hasn't been - // an issue yet, but eventually we'll have to implement it - // properly. (This is the same to-do as the one in - // `initializeEngineServices` in initialization.dart). + // We use timers here to ensure that microtasks flush in between. Timer.run(drawFrame); } diff --git a/shell/common/animator.cc b/shell/common/animator.cc index e5e37be779be6..ad3cea4d1adf2 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -265,7 +265,7 @@ void Animator::AwaitVSync() { } void Animator::EndWarmUpFrame() { - // Do nothing. The warm up frame does not need any additional work for end the + // Do nothing. The warm up frame does not need any additional work to end the // frame for now. This will change once the pipeline supports multi-view. // https://github.com/flutter/flutter/issues/142851 } From 9c5bce4def4d2feaaf79d6d4d9ae432b78253b1f Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 14 Feb 2024 23:00:17 -0800 Subject: [PATCH 14/24] Add test --- shell/common/engine_unittests.cc | 217 ++++++++++++++++++++++++++ shell/common/fixtures/shell_test.dart | 62 +++++++- 2 files changed, 273 insertions(+), 6 deletions(-) diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 35212111c50c0..94555979e1f8b 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -7,6 +7,9 @@ #include #include "flutter/runtime/dart_vm_lifecycle.h" +#include "flutter/common/constants.h" +#include "flutter/shell/common/shell.h" +#include "flutter/shell/common/shell_test.h" #include "flutter/shell/common/thread_host.h" #include "flutter/testing/fixture_test.h" #include "flutter/testing/testing.h" @@ -19,6 +22,19 @@ namespace flutter { namespace { +using ::testing::Invoke; +using ::testing::ReturnRef; + +static void PostSync(const fml::RefPtr& task_runner, + const fml::closure& task) { + fml::AutoResetWaitableEvent latch; + fml::TaskRunner::RunNowOrPostTask(task_runner, [&latch, &task] { + task(); + latch.Signal(); + }); + latch.Wait(); +} + class MockDelegate : public Engine::Delegate { public: MOCK_METHOD(void, @@ -118,6 +134,51 @@ class MockRuntimeController : public RuntimeController { MOCK_METHOD(bool, NotifyIdle, (fml::TimeDelta), (override)); }; +class MockAnimatorDelegate : public Animator::Delegate { + public: + /* Animator::Delegate */ + MOCK_METHOD(void, + OnAnimatorBeginFrame, + (fml::TimePoint frame_target_time, uint64_t frame_number), + (override)); + MOCK_METHOD(void, + OnAnimatorNotifyIdle, + (fml::TimeDelta deadline), + (override)); + MOCK_METHOD(void, + OnAnimatorUpdateLatestFrameTargetTime, + (fml::TimePoint frame_target_time), + (override)); + MOCK_METHOD(void, + OnAnimatorDraw, + (std::shared_ptr pipeline), + (override)); + MOCK_METHOD(void, + OnAnimatorDrawLastLayerTrees, + (std::unique_ptr frame_timings_recorder), + (override)); +}; + +class MockPlatformMessageHandler : public PlatformMessageHandler { + public: + MOCK_METHOD(void, + HandlePlatformMessage, + (std::unique_ptr message), + (override)); + MOCK_METHOD(bool, + DoesHandlePlatformMessageOnPlatformThread, + (), + (const, override)); + MOCK_METHOD(void, + InvokePlatformMessageResponseCallback, + (int response_id, std::unique_ptr mapping), + (override)); + MOCK_METHOD(void, + InvokePlatformMessageEmptyResponseCallback, + (int response_id), + (override)); +}; + std::unique_ptr MakePlatformMessage( const std::string& channel, const std::map& values, @@ -186,6 +247,96 @@ class EngineTest : public testing::FixtureTest { std::shared_ptr image_decoder_task_runner_; fml::TaskRunnerAffineWeakPtr snapshot_delegate_; }; + +// A class that can launch an Engine with the specified Engine::Delegate. +// +// To use this class, contruct this class with Create, call Run, and use the +// engine with EngineTaskSync(). +class EngineContext { + public: + using EngineCallback = std::function; + + [[nodiscard]] static std::unique_ptr Create( + Engine::Delegate& delegate, // + Settings settings, // + const TaskRunners& task_runners, // + std::unique_ptr animator) { + auto [vm, isolate_snapshot] = Shell::InferVmInitDataFromSettings(settings); + FML_CHECK(vm) << "Must be able to initialize the VM."; + // Construct the class with `new` because `make_unique` has no access to the + // private constructor. + EngineContext* raw_pointer = + new EngineContext(delegate, settings, task_runners, std::move(animator), + std::move(vm), isolate_snapshot); + return std::unique_ptr(raw_pointer); + } + + void Run(RunConfiguration configuration) { + PostSync(task_runners_.GetUITaskRunner(), [this, &configuration] { + Engine::RunStatus run_status = engine_->Run(std::move(configuration)); + FML_CHECK(run_status == Engine::RunStatus::Success) + << "Engine failed to run."; + (void)run_status; // Suppress unused-variable warning + }); + } + + // Run a task that operates the Engine on the UI thread, and wait for the + // task to end. + // + // If called on the UI thread, the task is executed synchronously. + void EngineTaskSync(EngineCallback task) { + ASSERT_TRUE(engine_); + ASSERT_TRUE(task); + auto runner = task_runners_.GetUITaskRunner(); + if (runner->RunsTasksOnCurrentThread()) { + task(*engine_); + } else { + PostSync(task_runners_.GetUITaskRunner(), [&]() { task(*engine_); }); + } + } + + ~EngineContext() { + PostSync(task_runners_.GetUITaskRunner(), [this] { engine_.reset(); }); + } + + private: + EngineContext(Engine::Delegate& delegate, // + Settings settings, // + const TaskRunners& task_runners, // + std::unique_ptr animator, // + DartVMRef vm, // + fml::RefPtr isolate_snapshot) + : task_runners_(task_runners), vm_(std::move(vm)) { + PostSync(task_runners.GetUITaskRunner(), [this, &settings, &animator, + &delegate, &isolate_snapshot] { + auto dispatcher_maker = + [](DefaultPointerDataDispatcher::Delegate& delegate) { + return std::make_unique(delegate); + }; + engine_ = std::make_unique( + /*delegate=*/delegate, + /*dispatcher_maker=*/dispatcher_maker, + /*vm=*/*&vm_, + /*isolate_snapshot=*/std::move(isolate_snapshot), + /*task_runners=*/task_runners_, + /*platform_data=*/PlatformData(), + /*settings=*/settings, + /*animator=*/std::move(animator), + /*io_manager=*/io_manager_, + /*unref_queue=*/nullptr, + /*snapshot_delegate=*/snapshot_delegate_, + /*volatile_path_tracker=*/nullptr, + /*gpu_disabled_switch=*/std::make_shared()); + }); + } + + TaskRunners task_runners_; + DartVMRef vm_; + std::unique_ptr engine_; + + fml::WeakPtr io_manager_; + fml::TaskRunnerAffineWeakPtr snapshot_delegate_; +}; } // namespace TEST_F(EngineTest, Create) { @@ -419,4 +570,70 @@ TEST_F(EngineTest, PassesLoadDartDeferredLibraryErrorToRuntime) { }); } +// The animator should submit to the pipeline the implicit view rendered in a +// warm up frame if there's already a continuation (i.e. Animator::BeginFrame +// has been called) +TEST_F(EngineTest, AnimatorSubmitWarmUpImplicitView) { + MockAnimatorDelegate animator_delegate; + std::unique_ptr engine_context; + + std::shared_ptr platform_message_handler = + std::make_shared(); + EXPECT_CALL(delegate_, GetPlatformMessageHandler) + .WillOnce(ReturnRef(platform_message_handler)); + + fml::AutoResetWaitableEvent draw_latch; + EXPECT_CALL(animator_delegate, OnAnimatorDraw) + .WillOnce( + Invoke([&draw_latch](const std::shared_ptr& pipeline) { + auto status = + pipeline->Consume([&](std::unique_ptr item) { + EXPECT_EQ(item->layer_tree_tasks.size(), 1u); + EXPECT_EQ(item->layer_tree_tasks[0]->view_id, kFlutterImplicitViewId); + }); + EXPECT_EQ(status, PipelineConsumeResult::Done); + draw_latch.Signal(); + })); + EXPECT_CALL(animator_delegate, OnAnimatorBeginFrame) + .Times(2) + .WillRepeatedly(Invoke([&engine_context](fml::TimePoint frame_target_time, + uint64_t frame_number) { + engine_context->EngineTaskSync([&](Engine& engine) { + engine.BeginFrame(frame_target_time, frame_number); + }); + })); + + static fml::AutoResetWaitableEvent continuation_ready_latch; + continuation_ready_latch.Reset(); + AddNativeCallback("NotifyNative", + [](auto args) { continuation_ready_latch.Signal(); }); + + std::unique_ptr animator; + PostSync(task_runners_.GetUITaskRunner(), + [&animator, &animator_delegate, &task_runners = task_runners_] { + animator = std::make_unique( + animator_delegate, task_runners, + static_cast>( + std::make_unique( + task_runners))); + }); + + engine_context = EngineContext::Create(delegate_, settings_, task_runners_, + std::move(animator)); + + auto configuration = RunConfiguration::InferFromSettings(settings_); + configuration.SetEntrypoint("renderWarmUpImplicitViewAfterMetricsChanged"); + engine_context->Run(std::move(configuration)); + + continuation_ready_latch.Wait(); + + + // Set metrics, which notifies the Dart isolate to render the views. + engine_context->EngineTaskSync([](Engine& engine) { + engine.AddView(kFlutterImplicitViewId, ViewportMetrics{}); + }); + + draw_latch.Wait(); +} + } // namespace flutter diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index 19e91c63869e8..3541abd2ec229 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -2,11 +2,18 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:convert' show utf8, json; +import 'dart:async' show scheduleMicrotask; +import 'dart:convert' show json, utf8; import 'dart:isolate'; import 'dart:typed_data'; import 'dart:ui'; +void expect(Object? a, Object? b) { + if (a != b) { + throw AssertionError('Expected $a to == $b'); + } +} + void main() {} @pragma('vm:entry-point') @@ -349,11 +356,6 @@ Future toImageSync() async { onBeforeToImageSync(); final Image image = picture.toImageSync(20, 25); - void expect(Object? a, Object? b) { - if (a != b) { - throw 'Expected $a to == $b'; - } - } expect(image.width, 20); expect(image.height, 25); @@ -529,3 +531,51 @@ void testReportViewWidths() { nativeReportViewWidthsCallback(getCurrentViewWidths()); }; } + +@pragma('vm:entry-point') +void renderWarmUpImplicitViewAfterMetricsChanged() { + PlatformDispatcher.instance.onBeginFrame = (Duration beginTime) { + // Make sure onBeginFrame and onDrawFrame are not used later. + PlatformDispatcher.instance.onBeginFrame = null; + PlatformDispatcher.instance.onDrawFrame = null; + // Let the test know that a continuation is available. + notifyNative(); + }; + + bool microtaskDone = false; + + // As soon as 2 views are added, render these views. + PlatformDispatcher.instance.onMetricsChanged = () { + PlatformDispatcher.instance.scheduleWarmUpFrame( + beginFrame: () { + expect(microtaskDone, false); + scheduleMicrotask(() { + microtaskDone = true; + }); + }, + drawFrame: () { + // TODO(dkwingsmt): According to the document in + // [ScheduleBinding.scheduleWarmUpFrame], the microtasks should be + // executed between two `Timer`s. It doesn't, at least with the current + // set up. We should examine more closely. + expect(microtaskDone, false); + + final SceneBuilder builder = SceneBuilder(); + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + canvas.drawPaint(Paint()..color = const Color(0xFFABCDEF)); + final Picture picture = recorder.endRecording(); + builder.addPicture(Offset.zero, picture); + + final Scene scene = builder.build(); + PlatformDispatcher.instance.implicitView!.render(scene); + + scene.dispose(); + picture.dispose(); + }, + ); + }; + + // Schedule a frame to produce an Animator continuation. + PlatformDispatcher.instance.scheduleFrame(); +} From cdcf71ab8eaec2d3c443ac97d8473c0b832e6345 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 14 Feb 2024 23:37:21 -0800 Subject: [PATCH 15/24] Fix test --- shell/common/engine_unittests.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 94555979e1f8b..ec2bd5f55f37d 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -595,7 +595,6 @@ TEST_F(EngineTest, AnimatorSubmitWarmUpImplicitView) { draw_latch.Signal(); })); EXPECT_CALL(animator_delegate, OnAnimatorBeginFrame) - .Times(2) .WillRepeatedly(Invoke([&engine_context](fml::TimePoint frame_target_time, uint64_t frame_number) { engine_context->EngineTaskSync([&](Engine& engine) { @@ -627,10 +626,9 @@ TEST_F(EngineTest, AnimatorSubmitWarmUpImplicitView) { continuation_ready_latch.Wait(); - // Set metrics, which notifies the Dart isolate to render the views. engine_context->EngineTaskSync([](Engine& engine) { - engine.AddView(kFlutterImplicitViewId, ViewportMetrics{}); + engine.AddView(kFlutterImplicitViewId, ViewportMetrics{1.0, 10, 10, 1, 0}); }); draw_latch.Wait(); From 7ab7d8eb00fc796a7afb1de2c551fc90d92f3766 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 14 Feb 2024 23:43:27 -0800 Subject: [PATCH 16/24] Better test --- shell/common/engine_unittests.cc | 23 +++++++++++------------ shell/common/fixtures/shell_test.dart | 14 ++++---------- 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index ec2bd5f55f37d..7152257c78c79 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -6,8 +6,8 @@ #include -#include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/common/constants.h" +#include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/shell/common/shell.h" #include "flutter/shell/common/shell_test.h" #include "flutter/shell/common/thread_host.h" @@ -584,19 +584,18 @@ TEST_F(EngineTest, AnimatorSubmitWarmUpImplicitView) { fml::AutoResetWaitableEvent draw_latch; EXPECT_CALL(animator_delegate, OnAnimatorDraw) - .WillOnce( - Invoke([&draw_latch](const std::shared_ptr& pipeline) { - auto status = - pipeline->Consume([&](std::unique_ptr item) { - EXPECT_EQ(item->layer_tree_tasks.size(), 1u); - EXPECT_EQ(item->layer_tree_tasks[0]->view_id, kFlutterImplicitViewId); - }); - EXPECT_EQ(status, PipelineConsumeResult::Done); - draw_latch.Signal(); - })); + .WillOnce(Invoke([&draw_latch]( + const std::shared_ptr& pipeline) { + auto status = pipeline->Consume([&](std::unique_ptr item) { + EXPECT_EQ(item->layer_tree_tasks.size(), 1u); + EXPECT_EQ(item->layer_tree_tasks[0]->view_id, kFlutterImplicitViewId); + }); + EXPECT_EQ(status, PipelineConsumeResult::Done); + draw_latch.Signal(); + })); EXPECT_CALL(animator_delegate, OnAnimatorBeginFrame) .WillRepeatedly(Invoke([&engine_context](fml::TimePoint frame_target_time, - uint64_t frame_number) { + uint64_t frame_number) { engine_context->EngineTaskSync([&](Engine& engine) { engine.BeginFrame(frame_target_time, frame_number); }); diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index 3541abd2ec229..bd73bbac8cc75 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -542,23 +542,17 @@ void renderWarmUpImplicitViewAfterMetricsChanged() { notifyNative(); }; - bool microtaskDone = false; + bool beginFrameDone = false; // As soon as 2 views are added, render these views. PlatformDispatcher.instance.onMetricsChanged = () { PlatformDispatcher.instance.scheduleWarmUpFrame( beginFrame: () { - expect(microtaskDone, false); - scheduleMicrotask(() { - microtaskDone = true; - }); + expect(beginFrameDone, false); + beginFrameDone = true; }, drawFrame: () { - // TODO(dkwingsmt): According to the document in - // [ScheduleBinding.scheduleWarmUpFrame], the microtasks should be - // executed between two `Timer`s. It doesn't, at least with the current - // set up. We should examine more closely. - expect(microtaskDone, false); + expect(beginFrameDone, true); final SceneBuilder builder = SceneBuilder(); final PictureRecorder recorder = PictureRecorder(); From 7381087ac5acf78802308e4b0ce9e4362a8b1847 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 15 Feb 2024 12:56:21 -0800 Subject: [PATCH 17/24] Simplify test and add platformdispatcher test --- shell/common/engine_unittests.cc | 29 +++++------ shell/common/fixtures/shell_test.dart | 58 ++++++++-------------- testing/dart/platform_dispatcher_test.dart | 28 +++++++++++ 3 files changed, 65 insertions(+), 50 deletions(-) diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 7152257c78c79..456a3feda0913 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -582,6 +582,7 @@ TEST_F(EngineTest, AnimatorSubmitWarmUpImplicitView) { EXPECT_CALL(delegate_, GetPlatformMessageHandler) .WillOnce(ReturnRef(platform_message_handler)); + fml::AutoResetWaitableEvent continuation_ready_latch; fml::AutoResetWaitableEvent draw_latch; EXPECT_CALL(animator_delegate, OnAnimatorDraw) .WillOnce(Invoke([&draw_latch]( @@ -594,17 +595,14 @@ TEST_F(EngineTest, AnimatorSubmitWarmUpImplicitView) { draw_latch.Signal(); })); EXPECT_CALL(animator_delegate, OnAnimatorBeginFrame) - .WillRepeatedly(Invoke([&engine_context](fml::TimePoint frame_target_time, - uint64_t frame_number) { - engine_context->EngineTaskSync([&](Engine& engine) { - engine.BeginFrame(frame_target_time, frame_number); - }); - })); - - static fml::AutoResetWaitableEvent continuation_ready_latch; - continuation_ready_latch.Reset(); - AddNativeCallback("NotifyNative", - [](auto args) { continuation_ready_latch.Signal(); }); + .WillRepeatedly( + Invoke([&engine_context, &continuation_ready_latch]( + fml::TimePoint frame_target_time, uint64_t frame_number) { + continuation_ready_latch.Signal(); + engine_context->EngineTaskSync([&](Engine& engine) { + engine.BeginFrame(frame_target_time, frame_number); + }); + })); std::unique_ptr animator; PostSync(task_runners_.GetUITaskRunner(), @@ -619,12 +617,15 @@ TEST_F(EngineTest, AnimatorSubmitWarmUpImplicitView) { engine_context = EngineContext::Create(delegate_, settings_, task_runners_, std::move(animator)); - auto configuration = RunConfiguration::InferFromSettings(settings_); - configuration.SetEntrypoint("renderWarmUpImplicitViewAfterMetricsChanged"); - engine_context->Run(std::move(configuration)); + engine_context->EngineTaskSync( + [](Engine& engine) { engine.ScheduleFrame(true); }); continuation_ready_latch.Wait(); + auto configuration = RunConfiguration::InferFromSettings(settings_); + configuration.SetEntrypoint("renderWarmUpImplicitView"); + engine_context->Run(std::move(configuration)); + // Set metrics, which notifies the Dart isolate to render the views. engine_context->EngineTaskSync([](Engine& engine) { engine.AddView(kFlutterImplicitViewId, ViewportMetrics{1.0, 10, 10, 1, 0}); diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index bd73bbac8cc75..ae681d892e54d 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -533,43 +533,29 @@ void testReportViewWidths() { } @pragma('vm:entry-point') -void renderWarmUpImplicitViewAfterMetricsChanged() { - PlatformDispatcher.instance.onBeginFrame = (Duration beginTime) { - // Make sure onBeginFrame and onDrawFrame are not used later. - PlatformDispatcher.instance.onBeginFrame = null; - PlatformDispatcher.instance.onDrawFrame = null; - // Let the test know that a continuation is available. - notifyNative(); - }; - +void renderWarmUpImplicitView() { bool beginFrameDone = false; - // As soon as 2 views are added, render these views. - PlatformDispatcher.instance.onMetricsChanged = () { - PlatformDispatcher.instance.scheduleWarmUpFrame( - beginFrame: () { - expect(beginFrameDone, false); - beginFrameDone = true; - }, - drawFrame: () { - expect(beginFrameDone, true); - - final SceneBuilder builder = SceneBuilder(); - final PictureRecorder recorder = PictureRecorder(); - final Canvas canvas = Canvas(recorder); - canvas.drawPaint(Paint()..color = const Color(0xFFABCDEF)); - final Picture picture = recorder.endRecording(); - builder.addPicture(Offset.zero, picture); - - final Scene scene = builder.build(); - PlatformDispatcher.instance.implicitView!.render(scene); - - scene.dispose(); - picture.dispose(); - }, - ); - }; + PlatformDispatcher.instance.scheduleWarmUpFrame( + beginFrame: () { + expect(beginFrameDone, false); + beginFrameDone = true; + }, + drawFrame: () { + expect(beginFrameDone, true); - // Schedule a frame to produce an Animator continuation. - PlatformDispatcher.instance.scheduleFrame(); + final SceneBuilder builder = SceneBuilder(); + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + canvas.drawPaint(Paint()..color = const Color(0xFFABCDEF)); + final Picture picture = recorder.endRecording(); + builder.addPicture(Offset.zero, picture); + + final Scene scene = builder.build(); + PlatformDispatcher.instance.implicitView!.render(scene); + + scene.dispose(); + picture.dispose(); + }, + ); } diff --git a/testing/dart/platform_dispatcher_test.dart b/testing/dart/platform_dispatcher_test.dart index 0d3d8802cd6b2..e7ff1f1aa5646 100644 --- a/testing/dart/platform_dispatcher_test.dart +++ b/testing/dart/platform_dispatcher_test.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; import 'dart:ui'; import 'package:litetest/litetest.dart'; @@ -46,4 +47,31 @@ void main() { expect(constraints.isSatisfiedBy(const Size(400, 500)), false); expect(constraints / 2, const ViewConstraints(minWidth: 50, maxWidth: 100, minHeight: 150, maxHeight: 200)); }); + + test('scheduleWarmupFrame should call both callbacks and flush microtasks', () async { + bool microtaskFlushed = false; + bool beginFrameCalled = false; + final Completer drawFrameCalled = Completer(); + PlatformDispatcher.instance.scheduleWarmUpFrame(beginFrame: () { + expect(microtaskFlushed, false); + expect(drawFrameCalled.isCompleted, false); + expect(beginFrameCalled, false); + beginFrameCalled = true; + scheduleMicrotask(() { + expect(microtaskFlushed, false); + expect(drawFrameCalled.isCompleted, false); + microtaskFlushed = true; + }); + expect(microtaskFlushed, false); + }, drawFrame: () { + expect(beginFrameCalled, true); + expect(microtaskFlushed, true); + expect(drawFrameCalled.isCompleted, false); + drawFrameCalled.complete(); + }); + await drawFrameCalled.future; + expect(beginFrameCalled, true); + expect(drawFrameCalled.isCompleted, true); + expect(microtaskFlushed, true); + }); } From 05d3d2dcf2aa5ae2444a425a74d8a2969f0d0403 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 15 Feb 2024 13:05:33 -0800 Subject: [PATCH 18/24] Better structure --- shell/common/engine_unittests.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 456a3feda0913..182d8423adfeb 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -617,20 +617,21 @@ TEST_F(EngineTest, AnimatorSubmitWarmUpImplicitView) { engine_context = EngineContext::Create(delegate_, settings_, task_runners_, std::move(animator)); - engine_context->EngineTaskSync( - [](Engine& engine) { engine.ScheduleFrame(true); }); - + engine_context->EngineTaskSync([](Engine& engine) { + // Schedule a frame to trigger Animator::BeginFrame to create a + // continuation. The continuation needs to be available before `Engine::Run` + // since the Dart program immediately schedules a warm up frame. + engine.ScheduleFrame(true); + // Add the implicit view so that the engine recognizes it and that its + // metrics is not empty. + engine.AddView(kFlutterImplicitViewId, ViewportMetrics{1.0, 10, 10, 1, 0}); + }); continuation_ready_latch.Wait(); auto configuration = RunConfiguration::InferFromSettings(settings_); configuration.SetEntrypoint("renderWarmUpImplicitView"); engine_context->Run(std::move(configuration)); - // Set metrics, which notifies the Dart isolate to render the views. - engine_context->EngineTaskSync([](Engine& engine) { - engine.AddView(kFlutterImplicitViewId, ViewportMetrics{1.0, 10, 10, 1, 0}); - }); - draw_latch.Wait(); } From 06957bb16ef2212c8edb41644b2952199739834d Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 15 Feb 2024 13:06:15 -0800 Subject: [PATCH 19/24] Better name --- shell/common/fixtures/shell_test.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index ae681d892e54d..1b52fabe1efe7 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -534,15 +534,15 @@ void testReportViewWidths() { @pragma('vm:entry-point') void renderWarmUpImplicitView() { - bool beginFrameDone = false; + bool beginFrameCalled = false; PlatformDispatcher.instance.scheduleWarmUpFrame( beginFrame: () { - expect(beginFrameDone, false); - beginFrameDone = true; + expect(beginFrameCalled, false); + beginFrameCalled = true; }, drawFrame: () { - expect(beginFrameDone, true); + expect(beginFrameCalled, true); final SceneBuilder builder = SceneBuilder(); final PictureRecorder recorder = PictureRecorder(); From 7789d11757ee4e44db5d30d198badc6908cdf2aa Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 20 Feb 2024 11:03:57 -0800 Subject: [PATCH 20/24] Original changes --- flow/frame_timings.cc | 24 +- flow/frame_timings.h | 3 + lib/ui/painting/image_dispose_unittests.cc | 10 +- lib/ui/window/platform_configuration.cc | 2 +- lib/ui/window/platform_configuration.h | 5 +- .../platform_configuration_unittests.cc | 158 +++++++++++++ runtime/runtime_controller.cc | 9 +- runtime/runtime_controller.h | 5 +- runtime/runtime_delegate.h | 3 +- shell/common/animator.cc | 77 ++++--- shell/common/animator.h | 10 +- shell/common/animator_unittests.cc | 37 +-- shell/common/engine.cc | 5 +- shell/common/engine.h | 3 +- shell/common/engine_unittests.cc | 217 +++++++++++++++++- shell/common/fixtures/shell_test.dart | 21 ++ shell/common/input_events_unittests.cc | 10 +- shell/common/rasterizer.cc | 1 + shell/common/shell_test.cc | 103 ++++++--- shell/common/shell_test.h | 50 ++-- shell/common/shell_unittests.cc | 80 ++++--- testing/dart/platform_view_test.dart | 10 +- 22 files changed, 692 insertions(+), 151 deletions(-) diff --git a/flow/frame_timings.cc b/flow/frame_timings.cc index 339374d77b837..2943e850e17d0 100644 --- a/flow/frame_timings.cc +++ b/flow/frame_timings.cc @@ -254,8 +254,30 @@ const char* FrameTimingsRecorder::GetFrameNumberTraceArg() const { return frame_number_trace_arg_val_.c_str(); } +static const char* StateToString(FrameTimingsRecorder::State state) { +#ifndef NDEBUG + switch (state) { + case FrameTimingsRecorder::State::kUninitialized: + return "kUninitialized"; + case FrameTimingsRecorder::State::kVsync: + return "kVsync"; + case FrameTimingsRecorder::State::kBuildStart: + return "kBuildStart"; + case FrameTimingsRecorder::State::kBuildEnd: + return "kBuildEnd"; + case FrameTimingsRecorder::State::kRasterStart: + return "kRasterStart"; + case FrameTimingsRecorder::State::kRasterEnd: + return "kRasterEnd"; + }; + FML_UNREACHABLE(); +#endif + return ""; +} + void FrameTimingsRecorder::AssertInState(State state) const { - FML_DCHECK(state_ == state); + FML_DCHECK(state_ == state) << "Expected state " << StateToString(state) + << ", actual state " << StateToString(state_); } } // namespace flutter diff --git a/flow/frame_timings.h b/flow/frame_timings.h index c81bcc3362298..ac5a7e470215e 100644 --- a/flow/frame_timings.h +++ b/flow/frame_timings.h @@ -31,6 +31,7 @@ class FrameTimingsRecorder { public: /// Various states that the recorder can be in. When created the recorder is /// in an unitialized state and transtions in sequential order of the states. + // After adding an item to this enum, modify StateToString accordingly. enum class State : uint32_t { kUninitialized, kVsync, @@ -121,6 +122,8 @@ class FrameTimingsRecorder { /// /// Instead of adding a `GetState` method and asserting on the result, this /// method prevents other logic from relying on the state. + /// + /// In opt builds, this call is a no-op. void AssertInState(State state) const; private: diff --git a/lib/ui/painting/image_dispose_unittests.cc b/lib/ui/painting/image_dispose_unittests.cc index 0f8bb6d027062..93600ca83c93d 100644 --- a/lib/ui/painting/image_dispose_unittests.cc +++ b/lib/ui/painting/image_dispose_unittests.cc @@ -5,6 +5,7 @@ #define FML_USED_ON_EMBEDDER #include "flutter/common/task_runners.h" +#include "flutter/fml/synchronization/count_down_latch.h" #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/lib/ui/painting/canvas.h" #include "flutter/lib/ui/painting/image.h" @@ -57,6 +58,10 @@ TEST_F(ImageDisposeTest, ImageReleasedAfterFrameAndDisposePictureAndLayer) { }; Settings settings = CreateSettingsForFixture(); + fml::CountDownLatch frame_latch{2}; + settings.frame_rasterized_callback = [&frame_latch](const FrameTiming& t) { + frame_latch.CountDown(); + }; auto task_runner = CreateNewThread(); TaskRunners task_runners("test", // label GetCurrentTaskRunner(), // platform @@ -83,12 +88,15 @@ TEST_F(ImageDisposeTest, ImageReleasedAfterFrameAndDisposePictureAndLayer) { shell->RunEngine(std::move(configuration), [&](auto result) { ASSERT_EQ(result, Engine::RunStatus::Success); }); - message_latch_.Wait(); ASSERT_TRUE(current_display_list_); ASSERT_TRUE(current_image_); + // Wait for 2 frames to be rasterized. The 2nd frame releases resources of the + // 1st frame. + frame_latch.Wait(); + // Force a drain the SkiaUnrefQueue. The engine does this normally as frames // pump, but we force it here to make the test more deterministic. message_latch_.Reset(); diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index 3ecadac12fe6f..fde45f2a231d2 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -458,7 +458,7 @@ void PlatformConfigurationNativeApi::Render(int64_t view_id, FML_DCHECK(view_id == kFlutterImplicitViewId); UIDartState::ThrowIfUIOperationsProhibited(); UIDartState::Current()->platform_configuration()->client()->Render( - scene, width, height); + view_id, scene, width, height); } void PlatformConfigurationNativeApi::SetNeedsReportTimings(bool value) { diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index 5f21051110472..a1f9ccca59367 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -69,7 +69,10 @@ class PlatformConfigurationClient { /// @brief Updates the client's rendering on the GPU with the newly /// provided Scene. /// - virtual void Render(Scene* scene, double width, double height) = 0; + virtual void Render(int64_t view_id, + Scene* scene, + double width, + double height) = 0; //-------------------------------------------------------------------------- /// @brief Receives an updated semantics tree from the Framework. diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 7410caeb66d6c..916f47c7026e8 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -15,8 +15,166 @@ #include "flutter/shell/common/shell_test.h" #include "flutter/shell/common/thread_host.h" #include "flutter/testing/testing.h" +#include "gmock/gmock.h" namespace flutter { + +namespace { + +static constexpr int64_t kImplicitViewId = 0; + +static void PostSync(const fml::RefPtr& task_runner, + const fml::closure& task) { + fml::AutoResetWaitableEvent latch; + fml::TaskRunner::RunNowOrPostTask(task_runner, [&latch, &task] { + task(); + latch.Signal(); + }); + latch.Wait(); +} + +class MockRuntimeDelegate : public RuntimeDelegate { + public: + MOCK_METHOD(std::string, DefaultRouteName, (), (override)); + MOCK_METHOD(void, ScheduleFrame, (bool), (override)); + MOCK_METHOD(void, + Render, + (int64_t, std::unique_ptr, float), + (override)); + MOCK_METHOD(void, + UpdateSemantics, + (SemanticsNodeUpdates, CustomAccessibilityActionUpdates), + (override)); + MOCK_METHOD(void, + HandlePlatformMessage, + (std::unique_ptr), + (override)); + MOCK_METHOD(FontCollection&, GetFontCollection, (), (override)); + MOCK_METHOD(std::shared_ptr, GetAssetManager, (), (override)); + MOCK_METHOD(void, OnRootIsolateCreated, (), (override)); + MOCK_METHOD(void, + UpdateIsolateDescription, + (const std::string, int64_t), + (override)); + MOCK_METHOD(void, SetNeedsReportTimings, (bool), (override)); + MOCK_METHOD(std::unique_ptr>, + ComputePlatformResolvedLocale, + (const std::vector&), + (override)); + MOCK_METHOD(void, RequestDartDeferredLibrary, (intptr_t), (override)); + MOCK_METHOD(std::weak_ptr, + GetPlatformMessageHandler, + (), + (const, override)); + MOCK_METHOD(void, SendChannelUpdate, (std::string, bool), (override)); + MOCK_METHOD(double, + GetScaledFontSize, + (double font_size, int configuration_id), + (const, override)); +}; + +class MockPlatformMessageHandler : public PlatformMessageHandler { + public: + MOCK_METHOD(void, + HandlePlatformMessage, + (std::unique_ptr message), + (override)); + MOCK_METHOD(bool, + DoesHandlePlatformMessageOnPlatformThread, + (), + (const, override)); + MOCK_METHOD(void, + InvokePlatformMessageResponseCallback, + (int response_id, std::unique_ptr mapping), + (override)); + MOCK_METHOD(void, + InvokePlatformMessageEmptyResponseCallback, + (int response_id), + (override)); +}; + +// A class that can launch a RuntimeController with the specified +// RuntimeDelegate. +// +// To use this class, contruct this class with Create, call LaunchRootIsolate, +// and use the controller with ControllerTaskSync(). +class RuntimeControllerContext { + public: + using ControllerCallback = std::function; + + [[nodiscard]] static std::unique_ptr Create( + Settings settings, // + const TaskRunners& task_runners, // + RuntimeDelegate& client) { + auto [vm, isolate_snapshot] = Shell::InferVmInitDataFromSettings(settings); + FML_CHECK(vm) << "Must be able to initialize the VM."; + // Construct the class with `new` because `make_unique` has no access to the + // private constructor. + RuntimeControllerContext* raw_pointer = new RuntimeControllerContext( + settings, task_runners, client, std::move(vm), isolate_snapshot); + return std::unique_ptr(raw_pointer); + } + + ~RuntimeControllerContext() { + PostSync(task_runners_.GetUITaskRunner(), + [&]() { runtime_controller_.reset(); }); + } + + // Launch the root isolate. The post_launch callback will be executed in the + // same UI task, which can be used to create initial views. + void LaunchRootIsolate(RunConfiguration& configuration, + ControllerCallback post_launch) { + PostSync(task_runners_.GetUITaskRunner(), [&]() { + bool launch_success = runtime_controller_->LaunchRootIsolate( + settings_, // + []() {}, // + configuration.GetEntrypoint(), // + configuration.GetEntrypointLibrary(), // + configuration.GetEntrypointArgs(), // + configuration.TakeIsolateConfiguration()); // + ASSERT_TRUE(launch_success); + post_launch(*runtime_controller_); + }); + } + + // Run a task that operates the RuntimeController on the UI thread, and wait + // for the task to end. + void ControllerTaskSync(ControllerCallback task) { + ASSERT_TRUE(runtime_controller_); + ASSERT_TRUE(task); + PostSync(task_runners_.GetUITaskRunner(), + [&]() { task(*runtime_controller_); }); + } + + private: + RuntimeControllerContext(const Settings& settings, + const TaskRunners& task_runners, + RuntimeDelegate& client, + DartVMRef vm, + fml::RefPtr isolate_snapshot) + : settings_(settings), + task_runners_(task_runners), + isolate_snapshot_(std::move(isolate_snapshot)), + vm_(std::move(vm)), + runtime_controller_(std::make_unique( + client, + &vm_, + std::move(isolate_snapshot_), + settings.idle_notification_callback, // idle notification callback + flutter::PlatformData(), // platform data + settings.isolate_create_callback, // isolate create callback + settings.isolate_shutdown_callback, // isolate shutdown callback + settings.persistent_isolate_data, // persistent isolate data + UIDartState::Context{task_runners})) {} + + Settings settings_; + TaskRunners task_runners_; + fml::RefPtr isolate_snapshot_; + DartVMRef vm_; + std::unique_ptr runtime_controller_; +}; +} // namespace + namespace testing { class PlatformConfigurationTest : public ShellTest {}; diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 71ac779e881eb..5e65dd3e3d41b 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -341,15 +341,16 @@ void RuntimeController::ScheduleFrame() { } // |PlatformConfigurationClient| -void RuntimeController::Render(Scene* scene, double width, double height) { - // TODO(dkwingsmt): Currently only supports a single window. - int64_t view_id = kFlutterImplicitViewId; +void RuntimeController::Render(int64_t view_id, + Scene* scene, + double width, + double height) { const ViewportMetrics* view_metrics = UIDartState::Current()->platform_configuration()->GetMetrics(view_id); if (view_metrics == nullptr) { return; } - client_.Render(scene->takeLayerTree(width, height), + client_.Render(view_id, scene->takeLayerTree(width, height), view_metrics->device_pixel_ratio); } diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index e9e1335cbbb75..44af6bd44f390 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -658,7 +658,10 @@ class RuntimeController : public PlatformConfigurationClient { void ScheduleFrame() override; // |PlatformConfigurationClient| - void Render(Scene* scene, double width, double height) override; + void Render(int64_t view_id, + Scene* scene, + double width, + double height) override; // |PlatformConfigurationClient| void UpdateSemantics(SemanticsUpdate* update) override; diff --git a/runtime/runtime_delegate.h b/runtime/runtime_delegate.h index bc3de031f4411..809fa46ac0aff 100644 --- a/runtime/runtime_delegate.h +++ b/runtime/runtime_delegate.h @@ -25,7 +25,8 @@ class RuntimeDelegate { virtual void ScheduleFrame(bool regenerate_layer_trees = true) = 0; - virtual void Render(std::unique_ptr layer_tree, + virtual void Render(int64_t view_id, + std::unique_ptr layer_tree, float device_pixel_ratio) = 0; virtual void UpdateSemantics(SemanticsNodeUpdates update, diff --git a/shell/common/animator.cc b/shell/common/animator.cc index 3dd925cee6213..1494ca2832afa 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -60,6 +60,10 @@ void Animator::EnqueueTraceFlowId(uint64_t trace_flow_id) { void Animator::BeginFrame( std::unique_ptr frame_timings_recorder) { + // Both frame_timings_recorder_ and layer_trees_tasks_ must be empty if not + // between BeginFrame and EndFrame. + FML_DCHECK(frame_timings_recorder_ == nullptr); + FML_DCHECK(layer_trees_tasks_.empty()); TRACE_EVENT_ASYNC_END0("flutter", "Frame Request Pending", frame_request_number_); frame_request_number_++; @@ -112,6 +116,33 @@ void Animator::BeginFrame( dart_frame_deadline_ = frame_target_time.ToEpochDelta(); uint64_t frame_number = frame_timings_recorder_->GetFrameNumber(); delegate_.OnAnimatorBeginFrame(frame_target_time, frame_number); +} + +void Animator::EndFrame() { + FML_CHECK(frame_timings_recorder_ != nullptr); + if (!layer_trees_tasks_.empty()) { + // The build is completed in OnAnimatorBeginFrame. + frame_timings_recorder_->RecordBuildEnd(fml::TimePoint::Now()); + + delegate_.OnAnimatorUpdateLatestFrameTargetTime( + frame_timings_recorder_->GetVsyncTargetTime()); + + // Commit the pending continuation. + PipelineProduceResult result = + producer_continuation_.Complete(std::make_unique( + std::move(layer_trees_tasks_), std::move(frame_timings_recorder_))); + + if (!result.success) { + FML_DLOG(INFO) << "Failed to commit to the pipeline"; + } else if (!result.is_first_item) { + // Do nothing. It has been successfully pushed to the pipeline but not as + // the first item. Eventually the 'Rasterizer' will consume it, so we + // don't need to notify the delegate. + } else { + delegate_.OnAnimatorDraw(layer_tree_pipeline_); + } + } + frame_timings_recorder_ = nullptr; // Ensure it's cleared. if (!frame_scheduled_ && has_rendered_) { // Wait a tad more than 3 60hz frames before reporting a big idle period. @@ -139,52 +170,25 @@ void Animator::BeginFrame( }, kNotifyIdleTaskWaitTime); } + // Both frame_timings_recorder_ and layer_trees_tasks_ must be empty if not + // between BeginFrame and EndFrame. + FML_DCHECK(layer_trees_tasks_.empty()); + FML_DCHECK(frame_timings_recorder_ == nullptr); } -void Animator::Render(std::unique_ptr layer_tree, +void Animator::Render(int64_t view_id, + std::unique_ptr layer_tree, float device_pixel_ratio) { - has_rendered_ = true; + FML_CHECK(frame_timings_recorder_ != nullptr); - if (!frame_timings_recorder_) { - // Framework can directly call render with a built scene. - frame_timings_recorder_ = std::make_unique(); - const fml::TimePoint placeholder_time = fml::TimePoint::Now(); - frame_timings_recorder_->RecordVsync(placeholder_time, placeholder_time); - frame_timings_recorder_->RecordBuildStart(placeholder_time); - } + has_rendered_ = true; TRACE_EVENT_WITH_FRAME_NUMBER(frame_timings_recorder_, "flutter", "Animator::Render", /*flow_id_count=*/0, /*flow_ids=*/nullptr); - frame_timings_recorder_->RecordBuildEnd(fml::TimePoint::Now()); - - delegate_.OnAnimatorUpdateLatestFrameTargetTime( - frame_timings_recorder_->GetVsyncTargetTime()); - // TODO(dkwingsmt): Currently only supports a single window. - // See https://github.com/flutter/flutter/issues/135530, item 2. - int64_t view_id = kFlutterImplicitViewId; - std::vector> layer_trees_tasks; - layer_trees_tasks.push_back(std::make_unique( + layer_trees_tasks_.push_back(std::make_unique( view_id, std::move(layer_tree), device_pixel_ratio)); - // Commit the pending continuation. - PipelineProduceResult result = - producer_continuation_.Complete(std::make_unique( - std::move(layer_trees_tasks), std::move(frame_timings_recorder_))); - - if (!result.success) { - FML_DLOG(INFO) << "No pending continuation to commit"; - return; - } - - if (!result.is_first_item) { - // It has been successfully pushed to the pipeline but not as the first - // item. Eventually the 'Rasterizer' will consume it, so we don't need to - // notify the delegate. - return; - } - - delegate_.OnAnimatorDraw(layer_tree_pipeline_); } const std::weak_ptr Animator::GetVsyncWaiter() const { @@ -256,6 +260,7 @@ void Animator::AwaitVSync() { self->DrawLastLayerTrees(std::move(frame_timings_recorder)); } else { self->BeginFrame(std::move(frame_timings_recorder)); + self->EndFrame(); } } }); diff --git a/shell/common/animator.h b/shell/common/animator.h index be15a76b765b1..5358ed6b14001 100644 --- a/shell/common/animator.h +++ b/shell/common/animator.h @@ -60,7 +60,8 @@ class Animator final { /// technically, between Animator::BeginFrame and Animator::EndFrame /// (both private methods). Otherwise, this call will be ignored. /// - void Render(std::unique_ptr layer_tree, + void Render(int64_t view_id, + std::unique_ptr layer_tree, float device_pixel_ratio); const std::weak_ptr GetVsyncWaiter() const; @@ -89,7 +90,13 @@ class Animator final { void EnqueueTraceFlowId(uint64_t trace_flow_id); private: + // Animator's work during a vsync is split into two methods, BeginFrame and + // EndFrame. The two methods should be called synchronously back-to-back to + // avoid being interrupted by a regular vsync. The reason to split them is to + // allow ShellTest::PumpOneFrame to insert a Render in between. + void BeginFrame(std::unique_ptr frame_timings_recorder); + void EndFrame(); bool CanReuseLastLayerTrees(); @@ -106,6 +113,7 @@ class Animator final { std::shared_ptr waiter_; std::unique_ptr frame_timings_recorder_; + std::vector> layer_trees_tasks_; uint64_t frame_request_number_ = 1; fml::TimeDelta dart_frame_deadline_; std::shared_ptr layer_tree_pipeline_; diff --git a/shell/common/animator_unittests.cc b/shell/common/animator_unittests.cc index 19a851a864c74..a1f96e7992376 100644 --- a/shell/common/animator_unittests.cc +++ b/shell/common/animator_unittests.cc @@ -23,6 +23,8 @@ namespace flutter { namespace testing { +constexpr int64_t kImplicitViewId = 0; + class FakeAnimatorDelegate : public Animator::Delegate { public: MOCK_METHOD(void, @@ -158,20 +160,30 @@ TEST_F(ShellTest, AnimatorDoesNotNotifyIdleBeforeRender) { latch.Wait(); ASSERT_FALSE(delegate.notify_idle_called_); + fml::AutoResetWaitableEvent render_latch; // Validate it has not notified idle and try to render. task_runners.GetUITaskRunner()->PostDelayedTask( [&] { ASSERT_FALSE(delegate.notify_idle_called_); - auto layer_tree = std::make_unique(LayerTree::Config(), - SkISize::Make(600, 800)); - animator->Render(std::move(layer_tree), 1.0); + EXPECT_CALL(delegate, OnAnimatorBeginFrame).WillOnce([&] { + auto layer_tree = std::make_unique( + LayerTree::Config(), SkISize::Make(600, 800)); + animator->Render(kImplicitViewId, std::move(layer_tree), 1.0); + render_latch.Signal(); + }); + // Request a frame that builds a layer tree and renders a frame. + // When the frame is rendered, render_latch will be signaled. + animator->RequestFrame(true); task_runners.GetPlatformTaskRunner()->PostTask(flush_vsync_task); }, // See kNotifyIdleTaskWaitTime in animator.cc. fml::TimeDelta::FromMilliseconds(60)); latch.Wait(); + render_latch.Wait(); - // Still hasn't notified idle because there has been no frame request. + // A frame has been rendered, and the next frame request will notify idle. + // But at the moment there isn't another frame request, therefore it still + // hasn't notified idle. task_runners.GetUITaskRunner()->PostTask([&] { ASSERT_FALSE(delegate.notify_idle_called_); // False to avoid getting cals to BeginFrame that will request more frames @@ -224,11 +236,6 @@ TEST_F(ShellTest, AnimatorDoesNotNotifyDelegateIfPipelineIsNotEmpty) { }); fml::AutoResetWaitableEvent begin_frame_latch; - EXPECT_CALL(delegate, OnAnimatorBeginFrame) - .WillRepeatedly( - [&](fml::TimePoint frame_target_time, uint64_t frame_number) { - begin_frame_latch.Signal(); - }); // It must always be called when the method 'Animator::Render' is called, // regardless of whether the pipeline is empty or not. EXPECT_CALL(delegate, OnAnimatorUpdateLatestFrameTargetTime).Times(2); @@ -239,16 +246,16 @@ TEST_F(ShellTest, AnimatorDoesNotNotifyDelegateIfPipelineIsNotEmpty) { for (int i = 0; i < 2; i++) { task_runners.GetUITaskRunner()->PostTask([&] { + EXPECT_CALL(delegate, OnAnimatorBeginFrame).WillOnce([&] { + auto layer_tree = std::make_unique(LayerTree::Config(), + SkISize::Make(600, 800)); + animator->Render(kImplicitViewId, std::move(layer_tree), 1.0); + begin_frame_latch.Signal(); + }); animator->RequestFrame(); task_runners.GetPlatformTaskRunner()->PostTask(flush_vsync_task); }); begin_frame_latch.Wait(); - - PostTaskSync(task_runners.GetUITaskRunner(), [&] { - auto layer_tree = std::make_unique(LayerTree::Config(), - SkISize::Make(600, 800)); - animator->Render(std::move(layer_tree), 1.0); - }); } PostTaskSync(task_runners.GetUITaskRunner(), [&] { animator.reset(); }); diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 3cc83228feea8..038df0a093746 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -462,7 +462,8 @@ void Engine::ScheduleFrame(bool regenerate_layer_trees) { animator_->RequestFrame(regenerate_layer_trees); } -void Engine::Render(std::unique_ptr layer_tree, +void Engine::Render(int64_t view_id, + std::unique_ptr layer_tree, float device_pixel_ratio) { if (!layer_tree) { return; @@ -473,7 +474,7 @@ void Engine::Render(std::unique_ptr layer_tree, return; } - animator_->Render(std::move(layer_tree), device_pixel_ratio); + animator_->Render(view_id, std::move(layer_tree), device_pixel_ratio); } void Engine::UpdateSemantics(SemanticsNodeUpdates update, diff --git a/shell/common/engine.h b/shell/common/engine.h index 4a4b3318b149a..f5c8524bec75d 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -963,7 +963,8 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { std::string DefaultRouteName() override; // |RuntimeDelegate| - void Render(std::unique_ptr layer_tree, + void Render(int64_t view_id, + std::unique_ptr layer_tree, float device_pixel_ratio) override; // |RuntimeDelegate| diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 8a6cc7834552e..55e895d165dca 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -7,6 +7,8 @@ #include #include "flutter/runtime/dart_vm_lifecycle.h" +#include "flutter/shell/common/shell.h" +#include "flutter/shell/common/shell_test.h" #include "flutter/shell/common/thread_host.h" #include "flutter/testing/fixture_test.h" #include "flutter/testing/testing.h" @@ -19,6 +21,19 @@ namespace flutter { namespace { +using ::testing::Invoke; +using ::testing::ReturnRef; + +static void PostSync(const fml::RefPtr& task_runner, + const fml::closure& task) { + fml::AutoResetWaitableEvent latch; + fml::TaskRunner::RunNowOrPostTask(task_runner, [&latch, &task] { + task(); + latch.Signal(); + }); + latch.Wait(); +} + class MockDelegate : public Engine::Delegate { public: MOCK_METHOD(void, @@ -65,7 +80,7 @@ class MockRuntimeDelegate : public RuntimeDelegate { MOCK_METHOD(void, ScheduleFrame, (bool), (override)); MOCK_METHOD(void, Render, - (std::unique_ptr, float), + (int64_t, std::unique_ptr, float), (override)); MOCK_METHOD(void, UpdateSemantics, @@ -117,6 +132,51 @@ class MockRuntimeController : public RuntimeController { MOCK_METHOD(bool, NotifyIdle, (fml::TimeDelta), (override)); }; +class MockAnimatorDelegate : public Animator::Delegate { + public: + /* Animator::Delegate */ + MOCK_METHOD(void, + OnAnimatorBeginFrame, + (fml::TimePoint frame_target_time, uint64_t frame_number), + (override)); + MOCK_METHOD(void, + OnAnimatorNotifyIdle, + (fml::TimeDelta deadline), + (override)); + MOCK_METHOD(void, + OnAnimatorUpdateLatestFrameTargetTime, + (fml::TimePoint frame_target_time), + (override)); + MOCK_METHOD(void, + OnAnimatorDraw, + (std::shared_ptr pipeline), + (override)); + MOCK_METHOD(void, + OnAnimatorDrawLastLayerTrees, + (std::unique_ptr frame_timings_recorder), + (override)); +}; + +class MockPlatformMessageHandler : public PlatformMessageHandler { + public: + MOCK_METHOD(void, + HandlePlatformMessage, + (std::unique_ptr message), + (override)); + MOCK_METHOD(bool, + DoesHandlePlatformMessageOnPlatformThread, + (), + (const, override)); + MOCK_METHOD(void, + InvokePlatformMessageResponseCallback, + (int response_id, std::unique_ptr mapping), + (override)); + MOCK_METHOD(void, + InvokePlatformMessageEmptyResponseCallback, + (int response_id), + (override)); +}; + std::unique_ptr MakePlatformMessage( const std::string& channel, const std::map& values, @@ -185,6 +245,97 @@ class EngineTest : public testing::FixtureTest { std::shared_ptr image_decoder_task_runner_; fml::TaskRunnerAffineWeakPtr snapshot_delegate_; }; + +// A class that can launch an Engine with the specified Engine::Delegate. +// +// To use this class, contruct this class with Create, call Run, and use the +// engine with EngineTaskSync(). +class EngineContext { + public: + using EngineCallback = std::function; + + [[nodiscard]] static std::unique_ptr Create( + Engine::Delegate& delegate, // + Settings settings, // + const TaskRunners& task_runners, // + std::unique_ptr animator) { + auto [vm, isolate_snapshot] = Shell::InferVmInitDataFromSettings(settings); + FML_CHECK(vm) << "Must be able to initialize the VM."; + // Construct the class with `new` because `make_unique` has no access to the + // private constructor. + EngineContext* raw_pointer = + new EngineContext(delegate, settings, task_runners, std::move(animator), + std::move(vm), isolate_snapshot); + return std::unique_ptr(raw_pointer); + } + + void Run(RunConfiguration configuration) { + PostSync(task_runners_.GetUITaskRunner(), [this, &configuration] { + Engine::RunStatus run_status = engine_->Run(std::move(configuration)); + FML_CHECK(run_status == Engine::RunStatus::Success) + << "Engine failed to run."; + (void)run_status; // Suppress unused-variable warning + }); + } + + // Run a task that operates the Engine on the UI thread, and wait for the + // task to end. + // + // If called on the UI thread, the task is executed synchronously. + void EngineTaskSync(EngineCallback task) { + ASSERT_TRUE(engine_); + ASSERT_TRUE(task); + auto runner = task_runners_.GetUITaskRunner(); + if (runner->RunsTasksOnCurrentThread()) { + task(*engine_); + } else { + PostSync(task_runners_.GetUITaskRunner(), [&]() { task(*engine_); }); + } + } + + ~EngineContext() { + PostSync(task_runners_.GetUITaskRunner(), [this] { engine_.reset(); }); + } + + private: + EngineContext(Engine::Delegate& delegate, // + Settings settings, // + const TaskRunners& task_runners, // + std::unique_ptr animator, // + DartVMRef vm, // + fml::RefPtr isolate_snapshot) + : task_runners_(task_runners), vm_(std::move(vm)) { + PostSync(task_runners.GetUITaskRunner(), [this, &settings, &animator, + &delegate, &isolate_snapshot] { + auto dispatcher_maker = + [](DefaultPointerDataDispatcher::Delegate& delegate) { + return std::make_unique(delegate); + }; + engine_ = std::make_unique( + /*delegate=*/delegate, + /*dispatcher_maker=*/dispatcher_maker, + /*vm=*/*&vm_, + /*isolate_snapshot=*/std::move(isolate_snapshot), + /*task_runners=*/task_runners_, + /*platform_data=*/PlatformData(), + /*settings=*/settings, + /*animator=*/std::move(animator), + /*io_manager=*/io_manager_, + /*unref_queue=*/nullptr, + /*snapshot_delegate=*/snapshot_delegate_, + /*volatile_path_tracker=*/nullptr, + /*gpu_disabled_switch=*/std::make_shared()); + }); + } + + TaskRunners task_runners_; + DartVMRef vm_; + std::unique_ptr engine_; + + fml::WeakPtr io_manager_; + fml::TaskRunnerAffineWeakPtr snapshot_delegate_; +}; + } // namespace TEST_F(EngineTest, Create) { @@ -418,4 +569,68 @@ TEST_F(EngineTest, PassesLoadDartDeferredLibraryErrorToRuntime) { }); } +TEST_F(EngineTest, AnimatorAcceptsMultipleRenders) { + MockAnimatorDelegate animator_delegate; + std::unique_ptr engine_context; + + std::shared_ptr platform_message_handler = + std::make_shared(); + EXPECT_CALL(delegate_, GetPlatformMessageHandler) + .WillOnce(ReturnRef(platform_message_handler)); + + fml::AutoResetWaitableEvent draw_latch; + EXPECT_CALL(animator_delegate, OnAnimatorDraw) + .WillOnce( + Invoke([&draw_latch](const std::shared_ptr& pipeline) { + auto status = + pipeline->Consume([&](std::unique_ptr item) { + EXPECT_EQ(item->layer_tree_tasks.size(), 2u); + EXPECT_EQ(item->layer_tree_tasks[0]->view_id, 1); + EXPECT_EQ(item->layer_tree_tasks[1]->view_id, 2); + }); + EXPECT_EQ(status, PipelineConsumeResult::Done); + draw_latch.Signal(); + })); + EXPECT_CALL(animator_delegate, OnAnimatorBeginFrame) + .WillOnce(Invoke([&engine_context](fml::TimePoint frame_target_time, + uint64_t frame_number) { + engine_context->EngineTaskSync([&](Engine& engine) { + engine.BeginFrame(frame_target_time, frame_number); + }); + })); + + static fml::AutoResetWaitableEvent callback_ready_latch; + callback_ready_latch.Reset(); + AddNativeCallback("NotifyNative", + [](auto args) { callback_ready_latch.Signal(); }); + + std::unique_ptr animator; + PostSync(task_runners_.GetUITaskRunner(), + [&animator, &animator_delegate, &task_runners = task_runners_] { + animator = std::make_unique( + animator_delegate, task_runners, + static_cast>( + std::make_unique( + task_runners))); + }); + + engine_context = EngineContext::Create(delegate_, settings_, task_runners_, + std::move(animator)); + + auto configuration = RunConfiguration::InferFromSettings(settings_); + configuration.SetEntrypoint("onBeginFrameRendersMultipleViews"); + engine_context->Run(std::move(configuration)); + + engine_context->EngineTaskSync([](Engine& engine) { + engine.AddView(1, {1, 10, 10, 22, 0}); + engine.AddView(2, {1, 10, 10, 22, 0}); + }); + + callback_ready_latch.Wait(); + + engine_context->EngineTaskSync( + [](Engine& engine) { engine.ScheduleFrame(); }); + draw_latch.Wait(); +} + } // namespace flutter diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index 19e91c63869e8..5cc2df586b981 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -529,3 +529,24 @@ void testReportViewWidths() { nativeReportViewWidthsCallback(getCurrentViewWidths()); }; } + +@pragma('vm:entry-point') +void onBeginFrameRendersMultipleViews() { + PlatformDispatcher.instance.onBeginFrame = (Duration beginTime) { + for (final FlutterView view in PlatformDispatcher.instance.views) { + final SceneBuilder builder = SceneBuilder(); + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + canvas.drawPaint(Paint()..color = const Color(0xFFABCDEF)); + final Picture picture = recorder.endRecording(); + builder.addPicture(Offset.zero, picture); + + final Scene scene = builder.build(); + view.render(scene); + + scene.dispose(); + picture.dispose(); + } + }; + notifyNative(); +} diff --git a/shell/common/input_events_unittests.cc b/shell/common/input_events_unittests.cc index 3824dc4d92bae..66a2b64a39e02 100644 --- a/shell/common/input_events_unittests.cc +++ b/shell/common/input_events_unittests.cc @@ -127,11 +127,11 @@ static void TestSimulatedInputEvents( ShellTest::DispatchFakePointerData(shell.get()); i += 1; } - ShellTest::VSyncFlush(shell.get(), will_draw_new_frame); + ShellTest::VSyncFlush(shell.get(), &will_draw_new_frame); } // Finally, issue a vsync for the pending event that may be generated duing // the last vsync. - ShellTest::VSyncFlush(shell.get(), will_draw_new_frame); + ShellTest::VSyncFlush(shell.get(), &will_draw_new_frame); }); simulation.wait(); @@ -345,8 +345,7 @@ TEST_F(ShellTest, CanCorrectlyPipePointerPacket) { CreateSimulatedPointerData(data, PointerData::Change::kRemove, 3.0, 4.0); packet->SetPointerData(5, data); ShellTest::DispatchPointerData(shell.get(), std::move(packet)); - bool will_draw_new_frame; - ShellTest::VSyncFlush(shell.get(), will_draw_new_frame); + ShellTest::VSyncFlush(shell.get()); reportLatch.Wait(); size_t expect_length = 6; @@ -407,8 +406,7 @@ TEST_F(ShellTest, CanCorrectlySynthesizePointerPacket) { CreateSimulatedPointerData(data, PointerData::Change::kRemove, 3.0, 4.0); packet->SetPointerData(3, data); ShellTest::DispatchPointerData(shell.get(), std::move(packet)); - bool will_draw_new_frame; - ShellTest::VSyncFlush(shell.get(), will_draw_new_frame); + ShellTest::VSyncFlush(shell.get()); reportLatch.Wait(); size_t expect_length = 6; diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 8d6edce8d958a..ed7bee5a9376c 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -261,6 +261,7 @@ DrawStatus Rasterizer::Draw(const std::shared_ptr& pipeline) { bool should_resubmit_frame = ShouldResubmitFrame(draw_result); if (should_resubmit_frame) { + FML_CHECK(draw_result.resubmitted_item); auto front_continuation = pipeline->ProduceIfEmpty(); PipelineProduceResult pipeline_result = front_continuation.Complete(std::move(draw_result.resubmitted_item)); diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index e321826ca1615..efcf43ce82668 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -22,6 +22,39 @@ namespace testing { constexpr int64_t kImplicitViewId = 0; +FrameContent ViewContent::NoViews() { + return std::map(); +} + +FrameContent ViewContent::DummyView(double width, double height) { + FrameContent result; + result[kImplicitViewId] = ViewContent{ + .viewport_metrics = {1.0, width, height, 22, 0}, + .builder = {}, + }; + return result; +} + +FrameContent ViewContent::DummyView(flutter::ViewportMetrics viewport_metrics) { + FrameContent result; + result[kImplicitViewId] = ViewContent{ + .viewport_metrics = std::move(viewport_metrics), + .builder = {}, + }; + return result; +} + +FrameContent ViewContent::ImplicitView(double width, + double height, + LayerTreeBuilder builder) { + FrameContent result; + result[kImplicitViewId] = ViewContent{ + .viewport_metrics = {1.0, width, height, 22, 0}, + .builder = std::move(builder), + }; + return result; +} + ShellTest::ShellTest() : thread_host_("io.flutter.test." + GetCurrentTestName() + ".", ThreadHost::Type::kPlatform | ThreadHost::Type::kIo | @@ -92,16 +125,18 @@ void ShellTest::RestartEngine(Shell* shell, RunConfiguration configuration) { ASSERT_TRUE(restarted.get_future().get()); } -void ShellTest::VSyncFlush(Shell* shell, bool& will_draw_new_frame) { +void ShellTest::VSyncFlush(Shell* shell, bool* will_draw_new_frame) { fml::AutoResetWaitableEvent latch; fml::TaskRunner::RunNowOrPostTask( shell->GetTaskRunners().GetPlatformTaskRunner(), - [shell, &will_draw_new_frame, &latch] { + [shell, will_draw_new_frame, &latch] { // The following UI task ensures that all previous UI tasks are flushed. fml::AutoResetWaitableEvent ui_latch; shell->GetTaskRunners().GetUITaskRunner()->PostTask( - [&ui_latch, &will_draw_new_frame]() { - will_draw_new_frame = true; + [&ui_latch, will_draw_new_frame]() { + if (will_draw_new_frame != nullptr) { + *will_draw_new_frame = true; + } ui_latch.Signal(); }); @@ -154,6 +189,7 @@ void ShellTest::SetViewportMetrics(Shell* shell, double width, double height) { std::make_unique(); recorder->RecordVsync(frame_begin_time, frame_end_time); engine->animator_->BeginFrame(std::move(recorder)); + engine->animator_->EndFrame(); } latch.Signal(); }); @@ -172,23 +208,22 @@ void ShellTest::NotifyIdle(Shell* shell, fml::TimeDelta deadline) { latch.Wait(); } -void ShellTest::PumpOneFrame(Shell* shell, - double width, - double height, - LayerTreeBuilder builder) { - PumpOneFrame(shell, {1.0, width, height, 22, 0}, std::move(builder)); +void ShellTest::PumpOneFrame(Shell* shell) { + PumpOneFrame(shell, ViewContent::DummyView()); } -void ShellTest::PumpOneFrame(Shell* shell, - const flutter::ViewportMetrics& viewport_metrics, - LayerTreeBuilder builder) { +void ShellTest::PumpOneFrame(Shell* shell, FrameContent frame_content) { // Set viewport to nonempty, and call Animator::BeginFrame to make the layer // tree pipeline nonempty. Without either of this, the layer tree below // won't be rasterized. fml::AutoResetWaitableEvent latch; + fml::WeakPtr runtime_delegate = shell->weak_engine_; shell->GetTaskRunners().GetUITaskRunner()->PostTask( - [&latch, engine = shell->weak_engine_, viewport_metrics]() { - engine->SetViewportMetrics(kImplicitViewId, viewport_metrics); + [&latch, engine = shell->weak_engine_, &frame_content, + runtime_delegate]() { + for (auto& [view_id, view_content] : frame_content) { + engine->SetViewportMetrics(view_id, view_content.viewport_metrics); + } const auto frame_begin_time = fml::TimePoint::Now(); const auto frame_end_time = frame_begin_time + fml::TimeDelta::FromSecondsF(1.0 / 60.0); @@ -196,28 +231,28 @@ void ShellTest::PumpOneFrame(Shell* shell, std::make_unique(); recorder->RecordVsync(frame_begin_time, frame_end_time); engine->animator_->BeginFrame(std::move(recorder)); - latch.Signal(); - }); - latch.Wait(); - latch.Reset(); - // Call |Render| to rasterize a layer tree and trigger |OnFrameRasterized| - fml::WeakPtr runtime_delegate = shell->weak_engine_; - shell->GetTaskRunners().GetUITaskRunner()->PostTask( - [&latch, runtime_delegate, &builder, viewport_metrics]() { - SkMatrix identity; - identity.setIdentity(); - auto root_layer = std::make_shared(identity); - auto layer_tree = std::make_unique( - LayerTree::Config{.root_layer = root_layer}, - SkISize::Make(viewport_metrics.physical_width, - viewport_metrics.physical_height)); - float device_pixel_ratio = - static_cast(viewport_metrics.device_pixel_ratio); - if (builder) { - builder(root_layer); + // The BeginFrame phase and the EndFrame phase must be performed in a + // single task, otherwise a normal vsync might be inserted in between, + // causing flaky assertion errors. + + for (auto& [view_id, view_content] : frame_content) { + SkMatrix identity; + identity.setIdentity(); + auto root_layer = std::make_shared(identity); + auto layer_tree = std::make_unique( + LayerTree::Config{.root_layer = root_layer}, + SkISize::Make(view_content.viewport_metrics.physical_width, + view_content.viewport_metrics.physical_height)); + float device_pixel_ratio = static_cast( + view_content.viewport_metrics.device_pixel_ratio); + if (view_content.builder) { + view_content.builder(root_layer); + } + runtime_delegate->Render(view_id, std::move(layer_tree), + device_pixel_ratio); } - runtime_delegate->Render(std::move(layer_tree), device_pixel_ratio); + engine->animator_->EndFrame(); latch.Signal(); }); latch.Wait(); diff --git a/shell/common/shell_test.h b/shell/common/shell_test.h index 7ded997fbcdc5..c11ad1174dc88 100644 --- a/shell/common/shell_test.h +++ b/shell/common/shell_test.h @@ -29,6 +29,38 @@ namespace flutter { namespace testing { +// The signature of ViewContent::builder. +using LayerTreeBuilder = + std::function root)>; +struct ViewContent; +// Defines the content to be rendered to all views of a frame in PumpOneFrame. +using FrameContent = std::map; +// Defines the content to be rendered to a view in PumpOneFrame. +struct ViewContent { + flutter::ViewportMetrics viewport_metrics; + // Given the root layer, this callback builds the layer tree to be rasterized + // in PumpOneFrame. + LayerTreeBuilder builder; + + // Build a frame with no views. This is useful when PumpOneFrame is used just + // to schedule the frame while the frame content is defined by other means. + static FrameContent NoViews(); + + // Build a frame with a single implicit view with the specific size and no + // content. + static FrameContent DummyView(double width = 1, double height = 1); + + // Build a frame with a single implicit view with the specific viewport + // metrics and no content. + static FrameContent DummyView(flutter::ViewportMetrics viewport_metrics); + + // Build a frame with a single implicit view with the specific size and + // content. + static FrameContent ImplicitView(double width, + double height, + LayerTreeBuilder builder); +}; + class ShellTest : public FixtureTest { public: struct Config { @@ -70,24 +102,14 @@ class ShellTest : public FixtureTest { static void RestartEngine(Shell* shell, RunConfiguration configuration); /// Issue as many VSYNC as needed to flush the UI tasks so far, and reset - /// the `will_draw_new_frame` to true. - static void VSyncFlush(Shell* shell, bool& will_draw_new_frame); - - /// Given the root layer, this callback builds the layer tree to be rasterized - /// in PumpOneFrame. - using LayerTreeBuilder = - std::function root)>; + /// the content of `will_draw_new_frame` to true if it's not nullptr. + static void VSyncFlush(Shell* shell, bool* will_draw_new_frame = nullptr); static void SetViewportMetrics(Shell* shell, double width, double height); static void NotifyIdle(Shell* shell, fml::TimeDelta deadline); - static void PumpOneFrame(Shell* shell, - double width = 1, - double height = 1, - LayerTreeBuilder = {}); - static void PumpOneFrame(Shell* shell, - const flutter::ViewportMetrics& viewport_metrics, - LayerTreeBuilder = {}); + static void PumpOneFrame(Shell* shell); + static void PumpOneFrame(Shell* shell, FrameContent frame_content); static void DispatchFakePointerData(Shell* shell); static void DispatchPointerData(Shell* shell, std::unique_ptr packet); diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 7c334eeebe807..57dc489ada4f4 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -42,6 +42,7 @@ #include "flutter/shell/common/switches.h" #include "flutter/shell/common/thread_host.h" #include "flutter/shell/common/vsync_waiter_fallback.h" +#include "flutter/shell/common/vsync_waiters_test.h" #include "flutter/shell/version/version.h" #include "flutter/testing/mock_canvas.h" #include "flutter/testing/testing.h" @@ -875,7 +876,7 @@ TEST_F(ShellTest, ExternalEmbedderNoThreadMerger) { root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), 100, 100, builder); + PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); end_frame_latch.Wait(); ASSERT_TRUE(end_frame_called); @@ -949,7 +950,7 @@ TEST_F(ShellTest, PushBackdropFilterToVisitedPlatformViews) { backdrop_filter_layer->Add(platform_view_layer2); }; - PumpOneFrame(shell.get(), 100, 100, builder); + PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); end_frame_latch.Wait(); ASSERT_EQ(visited_platform_views, (std::vector{50, 75})); ASSERT_TRUE(stack_75.is_empty()); @@ -1010,7 +1011,7 @@ TEST_F(ShellTest, root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), 100, 100, builder); + PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); end_frame_latch.Wait(); ASSERT_TRUE(end_frame_called); @@ -1056,9 +1057,12 @@ TEST_F(ShellTest, OnPlatformViewDestroyDisablesThreadMerger) { root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), 100, 100, builder); + PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); auto result = shell->WaitForFirstFrame(fml::TimeDelta::Max()); + // Wait for the rasterizer to process the frame. WaitForFirstFrame only waits + // for the Animator, but end_frame_callback is called by the Rasterizer. + PostSync(shell->GetTaskRunners().GetRasterTaskRunner(), [] {}); ASSERT_TRUE(result.ok()) << "Result: " << static_cast(result.code()) << ": " << result.message(); @@ -1123,12 +1127,12 @@ TEST_F(ShellTest, OnPlatformViewDestroyAfterMergingThreads) { root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), 100, 100, builder); + PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); // Pump one frame to trigger thread merging. end_frame_latch.Wait(); // Pump another frame to ensure threads are merged and a regular layer tree is // submitted. - PumpOneFrame(shell.get(), 100, 100, builder); + PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); // Threads are merged here. PlatformViewNotifyDestroy should be executed // successfully. ASSERT_TRUE(fml::TaskRunnerChecker::RunsOnTheSameThread( @@ -1192,7 +1196,7 @@ TEST_F(ShellTest, OnPlatformViewDestroyWhenThreadsAreMerging) { root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), 100, 100, builder); + PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); // Pump one frame and threads aren't merged end_frame_latch.Wait(); ASSERT_FALSE(fml::TaskRunnerChecker::RunsOnTheSameThread( @@ -1203,7 +1207,7 @@ TEST_F(ShellTest, OnPlatformViewDestroyWhenThreadsAreMerging) { // threads external_view_embedder->UpdatePostPrerollResult( PostPrerollResult::kResubmitFrame); - PumpOneFrame(shell.get(), 100, 100, builder); + PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); // Now destroy the platform view immediately. // Two things can happen here: @@ -1259,7 +1263,7 @@ TEST_F(ShellTest, SkPoint::Make(10, 10), MakeSizedDisplayList(80, 80), false, false); root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), 100, 100, builder); + PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); end_frame_latch.Wait(); // Threads should not be merged. @@ -1298,7 +1302,7 @@ TEST_F(ShellTest, OnPlatformViewDestroyWithoutRasterThreadMerger) { SkPoint::Make(10, 10), MakeSizedDisplayList(80, 80), false, false); root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), 100, 100, builder); + PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); // Threads should not be merged. ASSERT_FALSE(fml::TaskRunnerChecker::RunsOnTheSameThread( @@ -1364,7 +1368,7 @@ TEST_F(ShellTest, OnPlatformViewDestroyWithStaticThreadMerging) { SkPoint::Make(10, 10), MakeSizedDisplayList(80, 80), false, false); root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), 100, 100, builder); + PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); end_frame_latch.Wait(); ValidateDestroyPlatformView(shell.get()); @@ -1410,7 +1414,7 @@ TEST_F(ShellTest, GetUsedThisFrameShouldBeSetBeforeEndFrame) { SkPoint::Make(10, 10), MakeSizedDisplayList(80, 80), false, false); root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), 100, 100, builder); + PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); end_frame_latch.Wait(); ASSERT_FALSE(used_this_frame); @@ -1560,10 +1564,11 @@ TEST_F(ShellTest, WaitForFirstFrameZeroSizeFrame) { configuration.SetEntrypoint("emptyMain"); RunEngine(shell.get(), std::move(configuration)); - PumpOneFrame(shell.get(), {1.0, 0.0, 0.0, 22, 0}); + PumpOneFrame(shell.get(), ViewContent::DummyView({1.0, 0.0, 0.0, 22, 0})); fml::Status result = shell->WaitForFirstFrame(fml::TimeDelta::Zero()); - ASSERT_FALSE(result.ok()); - ASSERT_EQ(result.code(), fml::StatusCode::kDeadlineExceeded); + EXPECT_FALSE(result.ok()); + EXPECT_EQ(result.message(), "timeout"); + EXPECT_EQ(result.code(), fml::StatusCode::kDeadlineExceeded); DestroyShell(std::move(shell)); } @@ -2079,6 +2084,7 @@ TEST_F(ShellTest, CanScheduleFrameFromPlatform) { TEST_F(ShellTest, SecondaryVsyncCallbackShouldBeCalledAfterVsyncCallback) { bool is_on_begin_frame_called = false; bool is_secondary_callback_called = false; + bool test_started = false; Settings settings = CreateSettingsForFixture(); TaskRunners task_runners = GetTaskRunnersForFixture(); fml::AutoResetWaitableEvent latch; @@ -2088,12 +2094,18 @@ TEST_F(ShellTest, SecondaryVsyncCallbackShouldBeCalledAfterVsyncCallback) { fml::CountDownLatch count_down_latch(2); AddNativeCallback("NativeOnBeginFrame", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + if (!test_started) { + return; + } EXPECT_FALSE(is_on_begin_frame_called); EXPECT_FALSE(is_secondary_callback_called); is_on_begin_frame_called = true; count_down_latch.CountDown(); })); - std::unique_ptr shell = CreateShell(settings, task_runners); + std::unique_ptr shell = CreateShell({ + .settings = settings, + .task_runners = task_runners, + }); ASSERT_TRUE(shell->IsSetup()); auto configuration = RunConfiguration::InferFromSettings(settings); @@ -2106,12 +2118,16 @@ TEST_F(ShellTest, SecondaryVsyncCallbackShouldBeCalledAfterVsyncCallback) { fml::TaskRunner::RunNowOrPostTask( shell->GetTaskRunners().GetUITaskRunner(), [&]() { shell->GetEngine()->ScheduleSecondaryVsyncCallback(0, [&]() { + if (!test_started) { + return; + } EXPECT_TRUE(is_on_begin_frame_called); EXPECT_FALSE(is_secondary_callback_called); is_secondary_callback_called = true; count_down_latch.CountDown(); }); shell->GetEngine()->ScheduleFrame(); + test_started = true; }); count_down_latch.Wait(); EXPECT_TRUE(is_on_begin_frame_called); @@ -2156,7 +2172,7 @@ TEST_F(ShellTest, Screenshot) { root->Add(display_list_layer); }; - PumpOneFrame(shell.get(), 100, 100, builder); + PumpOneFrame(shell.get(), ViewContent::ImplicitView(100, 100, builder)); firstFrameLatch.Wait(); std::promise screenshot_promise; @@ -2541,7 +2557,13 @@ TEST_F(ShellTest, OnServiceProtocolRenderFrameWithRasterStatsWorks) { configuration.SetEntrypoint("scene_with_red_box"); RunEngine(shell.get(), std::move(configuration)); - PumpOneFrame(shell.get()); + // Set a non-zero viewport metrics, otherwise the scene would be discarded. + PostSync(shell->GetTaskRunners().GetUITaskRunner(), + [engine = shell->GetEngine()]() { + engine->SetViewportMetrics(kImplicitViewId, + ViewportMetrics{1, 1, 1, 22, 0}); + }); + PumpOneFrame(shell.get(), ViewContent::NoViews()); ServiceProtocol::Handler::ServiceProtocolMap empty_params; rapidjson::Document document; @@ -2653,7 +2675,7 @@ TEST_F(ShellTest, OnServiceProtocolRenderFrameWithRasterStatsDisableImpeller) { configuration.SetEntrypoint("scene_with_red_box"); RunEngine(shell.get(), std::move(configuration)); - PumpOneFrame(shell.get()); + PumpOneFrame(shell.get(), ViewContent::NoViews()); ServiceProtocol::Handler::ServiceProtocolMap empty_params; rapidjson::Document document; @@ -2717,14 +2739,16 @@ TEST_F(ShellTest, DISABLED_DiscardLayerTreeOnResize) { RunEngine(shell.get(), std::move(configuration)); - PumpOneFrame(shell.get(), static_cast(wrong_size.width()), - static_cast(wrong_size.height())); + PumpOneFrame(shell.get(), ViewContent::DummyView( + static_cast(wrong_size.width()), + static_cast(wrong_size.height()))); end_frame_latch.Wait(); // Wrong size, no frames are submitted. ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount()); - PumpOneFrame(shell.get(), static_cast(expected_size.width()), - static_cast(expected_size.height())); + PumpOneFrame(shell.get(), ViewContent::DummyView( + static_cast(expected_size.width()), + static_cast(expected_size.height()))); end_frame_latch.Wait(); // Expected size, 1 frame submitted. ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount()); @@ -2795,8 +2819,9 @@ TEST_F(ShellTest, DISABLED_DiscardResubmittedLayerTreeOnResize) { RunEngine(shell.get(), std::move(configuration)); - PumpOneFrame(shell.get(), static_cast(origin_size.width()), - static_cast(origin_size.height())); + PumpOneFrame(shell.get(), ViewContent::DummyView( + static_cast(origin_size.width()), + static_cast(origin_size.height()))); end_frame_latch.Wait(); ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount()); @@ -2817,8 +2842,9 @@ TEST_F(ShellTest, DISABLED_DiscardResubmittedLayerTreeOnResize) { ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount()); // Threads will be merged at the end of this frame. - PumpOneFrame(shell.get(), static_cast(new_size.width()), - static_cast(new_size.height())); + PumpOneFrame(shell.get(), + ViewContent::DummyView(static_cast(new_size.width()), + static_cast(new_size.height()))); end_frame_latch.Wait(); ASSERT_TRUE(raster_thread_merger_ref->IsMerged()); diff --git a/testing/dart/platform_view_test.dart b/testing/dart/platform_view_test.dart index 146c865899952..cd581a22d581d 100644 --- a/testing/dart/platform_view_test.dart +++ b/testing/dart/platform_view_test.dart @@ -10,10 +10,12 @@ void main() { test('PlatformView layers do not emit errors from tester', () async { final SceneBuilder builder = SceneBuilder(); builder.addPlatformView(1); - final Scene scene = builder.build(); - - PlatformDispatcher.instance.implicitView!.render(scene); - scene.dispose(); + PlatformDispatcher.instance.onBeginFrame = (Duration duration) { + final Scene scene = builder.build(); + PlatformDispatcher.instance.implicitView!.render(scene); + scene.dispose(); + }; + PlatformDispatcher.instance.scheduleFrame(); // Test harness asserts that this does not emit an error from the shell logs. }); } From 3464c0583b9367c707293d9879120b40d752fa8a Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 20 Feb 2024 11:13:38 -0800 Subject: [PATCH 21/24] Additional changes --- .../platform_configuration_unittests.cc | 158 ------------------ shell/common/animator.cc | 31 ++-- shell/common/engine_unittests.cc | 131 +++++++++++++++ shell/common/fixtures/shell_test.dart | 53 ++++++ 4 files changed, 202 insertions(+), 171 deletions(-) diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 916f47c7026e8..7410caeb66d6c 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -15,166 +15,8 @@ #include "flutter/shell/common/shell_test.h" #include "flutter/shell/common/thread_host.h" #include "flutter/testing/testing.h" -#include "gmock/gmock.h" namespace flutter { - -namespace { - -static constexpr int64_t kImplicitViewId = 0; - -static void PostSync(const fml::RefPtr& task_runner, - const fml::closure& task) { - fml::AutoResetWaitableEvent latch; - fml::TaskRunner::RunNowOrPostTask(task_runner, [&latch, &task] { - task(); - latch.Signal(); - }); - latch.Wait(); -} - -class MockRuntimeDelegate : public RuntimeDelegate { - public: - MOCK_METHOD(std::string, DefaultRouteName, (), (override)); - MOCK_METHOD(void, ScheduleFrame, (bool), (override)); - MOCK_METHOD(void, - Render, - (int64_t, std::unique_ptr, float), - (override)); - MOCK_METHOD(void, - UpdateSemantics, - (SemanticsNodeUpdates, CustomAccessibilityActionUpdates), - (override)); - MOCK_METHOD(void, - HandlePlatformMessage, - (std::unique_ptr), - (override)); - MOCK_METHOD(FontCollection&, GetFontCollection, (), (override)); - MOCK_METHOD(std::shared_ptr, GetAssetManager, (), (override)); - MOCK_METHOD(void, OnRootIsolateCreated, (), (override)); - MOCK_METHOD(void, - UpdateIsolateDescription, - (const std::string, int64_t), - (override)); - MOCK_METHOD(void, SetNeedsReportTimings, (bool), (override)); - MOCK_METHOD(std::unique_ptr>, - ComputePlatformResolvedLocale, - (const std::vector&), - (override)); - MOCK_METHOD(void, RequestDartDeferredLibrary, (intptr_t), (override)); - MOCK_METHOD(std::weak_ptr, - GetPlatformMessageHandler, - (), - (const, override)); - MOCK_METHOD(void, SendChannelUpdate, (std::string, bool), (override)); - MOCK_METHOD(double, - GetScaledFontSize, - (double font_size, int configuration_id), - (const, override)); -}; - -class MockPlatformMessageHandler : public PlatformMessageHandler { - public: - MOCK_METHOD(void, - HandlePlatformMessage, - (std::unique_ptr message), - (override)); - MOCK_METHOD(bool, - DoesHandlePlatformMessageOnPlatformThread, - (), - (const, override)); - MOCK_METHOD(void, - InvokePlatformMessageResponseCallback, - (int response_id, std::unique_ptr mapping), - (override)); - MOCK_METHOD(void, - InvokePlatformMessageEmptyResponseCallback, - (int response_id), - (override)); -}; - -// A class that can launch a RuntimeController with the specified -// RuntimeDelegate. -// -// To use this class, contruct this class with Create, call LaunchRootIsolate, -// and use the controller with ControllerTaskSync(). -class RuntimeControllerContext { - public: - using ControllerCallback = std::function; - - [[nodiscard]] static std::unique_ptr Create( - Settings settings, // - const TaskRunners& task_runners, // - RuntimeDelegate& client) { - auto [vm, isolate_snapshot] = Shell::InferVmInitDataFromSettings(settings); - FML_CHECK(vm) << "Must be able to initialize the VM."; - // Construct the class with `new` because `make_unique` has no access to the - // private constructor. - RuntimeControllerContext* raw_pointer = new RuntimeControllerContext( - settings, task_runners, client, std::move(vm), isolate_snapshot); - return std::unique_ptr(raw_pointer); - } - - ~RuntimeControllerContext() { - PostSync(task_runners_.GetUITaskRunner(), - [&]() { runtime_controller_.reset(); }); - } - - // Launch the root isolate. The post_launch callback will be executed in the - // same UI task, which can be used to create initial views. - void LaunchRootIsolate(RunConfiguration& configuration, - ControllerCallback post_launch) { - PostSync(task_runners_.GetUITaskRunner(), [&]() { - bool launch_success = runtime_controller_->LaunchRootIsolate( - settings_, // - []() {}, // - configuration.GetEntrypoint(), // - configuration.GetEntrypointLibrary(), // - configuration.GetEntrypointArgs(), // - configuration.TakeIsolateConfiguration()); // - ASSERT_TRUE(launch_success); - post_launch(*runtime_controller_); - }); - } - - // Run a task that operates the RuntimeController on the UI thread, and wait - // for the task to end. - void ControllerTaskSync(ControllerCallback task) { - ASSERT_TRUE(runtime_controller_); - ASSERT_TRUE(task); - PostSync(task_runners_.GetUITaskRunner(), - [&]() { task(*runtime_controller_); }); - } - - private: - RuntimeControllerContext(const Settings& settings, - const TaskRunners& task_runners, - RuntimeDelegate& client, - DartVMRef vm, - fml::RefPtr isolate_snapshot) - : settings_(settings), - task_runners_(task_runners), - isolate_snapshot_(std::move(isolate_snapshot)), - vm_(std::move(vm)), - runtime_controller_(std::make_unique( - client, - &vm_, - std::move(isolate_snapshot_), - settings.idle_notification_callback, // idle notification callback - flutter::PlatformData(), // platform data - settings.isolate_create_callback, // isolate create callback - settings.isolate_shutdown_callback, // isolate shutdown callback - settings.persistent_isolate_data, // persistent isolate data - UIDartState::Context{task_runners})) {} - - Settings settings_; - TaskRunners task_runners_; - fml::RefPtr isolate_snapshot_; - DartVMRef vm_; - std::unique_ptr runtime_controller_; -}; -} // namespace - namespace testing { class PlatformConfigurationTest : public ShellTest {}; diff --git a/shell/common/animator.cc b/shell/common/animator.cc index 94f1b8747627d..dbcf6e22eadd4 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -60,12 +60,12 @@ void Animator::EnqueueTraceFlowId(uint64_t trace_flow_id) { void Animator::BeginFrame( std::unique_ptr frame_timings_recorder) { - // Both frame_timings_recorder_ and layer_trees_tasks_ must be empty if not - // between BeginFrame and EndFrame. - FML_DCHECK(frame_timings_recorder_ == nullptr); - FML_DCHECK(layer_trees_tasks_.empty()); TRACE_EVENT_ASYNC_END0("flutter", "Frame Request Pending", frame_request_number_); + // Clear layer trees rendered out of a frame. Only Animator::Render called + // within a frame is used. + layer_trees_tasks_.clear(); + frame_request_number_++; frame_timings_recorder_ = std::move(frame_timings_recorder); @@ -119,7 +119,7 @@ void Animator::BeginFrame( } void Animator::EndFrame() { - FML_CHECK(frame_timings_recorder_ != nullptr); + FML_DCHECK(frame_timings_recorder_ != nullptr); if (!layer_trees_tasks_.empty()) { // The build is completed in OnAnimatorBeginFrame. frame_timings_recorder_->RecordBuildEnd(fml::TimePoint::Now()); @@ -142,7 +142,7 @@ void Animator::EndFrame() { delegate_.OnAnimatorDraw(layer_tree_pipeline_); } } - frame_timings_recorder_ = nullptr; // Ensure it's cleared. + frame_timings_recorder_ = nullptr; if (!frame_scheduled_ && has_rendered_) { // Wait a tad more than 3 60hz frames before reporting a big idle period. @@ -170,8 +170,6 @@ void Animator::EndFrame() { }, kNotifyIdleTaskWaitTime); } - // Both frame_timings_recorder_ and layer_trees_tasks_ must be empty if not - // between BeginFrame and EndFrame. FML_DCHECK(layer_trees_tasks_.empty()); FML_DCHECK(frame_timings_recorder_ == nullptr); } @@ -179,10 +177,17 @@ void Animator::EndFrame() { void Animator::Render(int64_t view_id, std::unique_ptr layer_tree, float device_pixel_ratio) { - FML_CHECK(frame_timings_recorder_ != nullptr); - has_rendered_ = true; + if (!frame_timings_recorder_) { + // Framework can directly call render with a built scene. A major reason is + // to render warm up frames. + frame_timings_recorder_ = std::make_unique(); + const fml::TimePoint placeholder_time = fml::TimePoint::Now(); + frame_timings_recorder_->RecordVsync(placeholder_time, placeholder_time); + frame_timings_recorder_->RecordBuildStart(placeholder_time); + } + TRACE_EVENT_WITH_FRAME_NUMBER(frame_timings_recorder_, "flutter", "Animator::Render", /*flow_id_count=*/0, /*flow_ids=*/nullptr); @@ -270,9 +275,9 @@ void Animator::AwaitVSync() { } void Animator::EndWarmUpFrame() { - // Do nothing. The warm up frame does not need any additional work to end the - // frame for now. This will change once the pipeline supports multi-view. - // https://github.com/flutter/flutter/issues/142851 + if (!layer_trees_tasks_.empty()) { + EndFrame(); + } } void Animator::ScheduleSecondaryVsyncCallback(uintptr_t id, diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 11cdc0bd3d5ea..d8f0c4f7d46fe 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -337,6 +337,7 @@ class EngineContext { fml::WeakPtr io_manager_; fml::TaskRunnerAffineWeakPtr snapshot_delegate_; }; + } // namespace TEST_F(EngineTest, Create) { @@ -570,6 +571,68 @@ TEST_F(EngineTest, PassesLoadDartDeferredLibraryErrorToRuntime) { }); } +TEST_F(EngineTest, AnimatorAcceptsMultipleRenders) { + MockAnimatorDelegate animator_delegate; + std::unique_ptr engine_context; + + std::shared_ptr platform_message_handler = + std::make_shared(); + EXPECT_CALL(delegate_, GetPlatformMessageHandler) + .WillOnce(ReturnRef(platform_message_handler)); + fml::AutoResetWaitableEvent draw_latch; + EXPECT_CALL(animator_delegate, OnAnimatorDraw) + .WillOnce( + Invoke([&draw_latch](const std::shared_ptr& pipeline) { + auto status = + pipeline->Consume([&](std::unique_ptr item) { + EXPECT_EQ(item->layer_tree_tasks.size(), 2u); + EXPECT_EQ(item->layer_tree_tasks[0]->view_id, 1); + EXPECT_EQ(item->layer_tree_tasks[1]->view_id, 2); + }); + EXPECT_EQ(status, PipelineConsumeResult::Done); + draw_latch.Signal(); + })); + EXPECT_CALL(animator_delegate, OnAnimatorBeginFrame) + .WillOnce(Invoke([&engine_context](fml::TimePoint frame_target_time, + uint64_t frame_number) { + engine_context->EngineTaskSync([&](Engine& engine) { + engine.BeginFrame(frame_target_time, frame_number); + }); + })); + + static fml::AutoResetWaitableEvent callback_ready_latch; + callback_ready_latch.Reset(); + AddNativeCallback("NotifyNative", + [](auto args) { callback_ready_latch.Signal(); }); + + std::unique_ptr animator; + PostSync(task_runners_.GetUITaskRunner(), + [&animator, &animator_delegate, &task_runners = task_runners_] { + animator = std::make_unique( + animator_delegate, task_runners, + static_cast>( + std::make_unique( + task_runners))); + }); + + engine_context = EngineContext::Create(delegate_, settings_, task_runners_, + std::move(animator)); + auto configuration = RunConfiguration::InferFromSettings(settings_); + configuration.SetEntrypoint("onDrawFrameRenderAllViews"); + engine_context->Run(std::move(configuration)); + + engine_context->EngineTaskSync([](Engine& engine) { + engine.AddView(1, {1, 10, 10, 22, 0}); + engine.AddView(2, {1, 10, 10, 22, 0}); + }); + + callback_ready_latch.Wait(); + + engine_context->EngineTaskSync( + [](Engine& engine) { engine.ScheduleFrame(); }); + draw_latch.Wait(); +} + // The animator should submit to the pipeline the implicit view rendered in a // warm up frame if there's already a continuation (i.e. Animator::BeginFrame // has been called) @@ -635,4 +698,72 @@ TEST_F(EngineTest, AnimatorSubmitWarmUpImplicitView) { draw_latch.Wait(); } +// The warm up frame should work if only some of the registered views are +// included. +// +// This test also verifies that the warm up frame can render multiple views. +TEST_F(EngineTest, AnimatorSubmitPartialViewsForWarmUp) { + MockAnimatorDelegate animator_delegate; + std::unique_ptr engine_context; + + std::shared_ptr platform_message_handler = + std::make_shared(); + EXPECT_CALL(delegate_, GetPlatformMessageHandler) + .WillOnce(ReturnRef(platform_message_handler)); + + fml::AutoResetWaitableEvent continuation_ready_latch; + fml::AutoResetWaitableEvent draw_latch; + EXPECT_CALL(animator_delegate, OnAnimatorDraw) + .WillOnce( + Invoke([&draw_latch](const std::shared_ptr& pipeline) { + auto status = + pipeline->Consume([&](std::unique_ptr item) { + EXPECT_EQ(item->layer_tree_tasks.size(), 2u); + EXPECT_EQ(item->layer_tree_tasks[0]->view_id, 1); + EXPECT_EQ(item->layer_tree_tasks[1]->view_id, 2); + }); + EXPECT_EQ(status, PipelineConsumeResult::Done); + draw_latch.Signal(); + })); + EXPECT_CALL(animator_delegate, OnAnimatorBeginFrame) + .WillRepeatedly( + Invoke([&engine_context, &continuation_ready_latch]( + fml::TimePoint frame_target_time, uint64_t frame_number) { + continuation_ready_latch.Signal(); + engine_context->EngineTaskSync([&](Engine& engine) { + engine.BeginFrame(frame_target_time, frame_number); + }); + })); + + std::unique_ptr animator; + PostSync(task_runners_.GetUITaskRunner(), + [&animator, &animator_delegate, &task_runners = task_runners_] { + animator = std::make_unique( + animator_delegate, task_runners, + static_cast>( + std::make_unique( + task_runners))); + }); + + engine_context = EngineContext::Create(delegate_, settings_, task_runners_, + std::move(animator)); + + engine_context->EngineTaskSync([](Engine& engine) { + // Schedule a frame to make the animator create a continuation. + engine.ScheduleFrame(true); + // Add multiple views. + engine.AddView(0, {1, 10, 10, 22, 0}); + engine.AddView(1, {1, 10, 10, 22, 0}); + engine.AddView(2, {1, 10, 10, 22, 0}); + }); + + continuation_ready_latch.Wait(); + + auto configuration = RunConfiguration::InferFromSettings(settings_); + configuration.SetEntrypoint("renderWarmUpView1and2"); + engine_context->Run(std::move(configuration)); + + draw_latch.Wait(); +} + } // namespace flutter diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index 1b52fabe1efe7..c537cbf2ecfcf 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -532,6 +532,27 @@ void testReportViewWidths() { }; } +@pragma('vm:entry-point') +void onDrawFrameRenderAllViews() { + PlatformDispatcher.instance.onDrawFrame = () { + for (final FlutterView view in PlatformDispatcher.instance.views) { + final SceneBuilder builder = SceneBuilder(); + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + canvas.drawPaint(Paint()..color = const Color(0xFFABCDEF)); + final Picture picture = recorder.endRecording(); + builder.addPicture(Offset.zero, picture); + + final Scene scene = builder.build(); + view.render(scene); + + scene.dispose(); + picture.dispose(); + } + }; + notifyNative(); +} + @pragma('vm:entry-point') void renderWarmUpImplicitView() { bool beginFrameCalled = false; @@ -559,3 +580,35 @@ void renderWarmUpImplicitView() { }, ); } + +@pragma('vm:entry-point') +void renderWarmUpView1and2() { + bool beginFrameCalled = false; + + PlatformDispatcher.instance.scheduleWarmUpFrame( + beginFrame: () { + expect(beginFrameCalled, false); + beginFrameCalled = true; + }, + drawFrame: () { + expect(beginFrameCalled, true); + + for (final int viewId in [1, 2]) { + final FlutterView view = PlatformDispatcher.instance.view(id: viewId)!; + + final SceneBuilder builder = SceneBuilder(); + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + canvas.drawPaint(Paint()..color = const Color(0xFFABCDEF)); + final Picture picture = recorder.endRecording(); + builder.addPicture(Offset.zero, picture); + + final Scene scene = builder.build(); + view.render(scene); + + scene.dispose(); + picture.dispose(); + } + } + ); +} From fe143117c4070f54be3768b0a5df58970cc0579c Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 20 Feb 2024 13:27:58 -0800 Subject: [PATCH 22/24] Comments --- flow/frame_timings.cc | 46 +++++++++++++++++--------------- flow/frame_timings.h | 2 +- shell/common/engine_unittests.cc | 20 +++++++------- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/flow/frame_timings.cc b/flow/frame_timings.cc index 2943e850e17d0..07a398a81c42e 100644 --- a/flow/frame_timings.cc +++ b/flow/frame_timings.cc @@ -13,6 +13,31 @@ namespace flutter { +namespace { + +const char* StateToString(FrameTimingsRecorder::State state) { +#ifndef NDEBUG + switch (state) { + case FrameTimingsRecorder::State::kUninitialized: + return "kUninitialized"; + case FrameTimingsRecorder::State::kVsync: + return "kVsync"; + case FrameTimingsRecorder::State::kBuildStart: + return "kBuildStart"; + case FrameTimingsRecorder::State::kBuildEnd: + return "kBuildEnd"; + case FrameTimingsRecorder::State::kRasterStart: + return "kRasterStart"; + case FrameTimingsRecorder::State::kRasterEnd: + return "kRasterEnd"; + }; + FML_UNREACHABLE(); +#endif + return ""; +} + +} // namespace + std::atomic FrameTimingsRecorder::frame_number_gen_ = {1}; FrameTimingsRecorder::FrameTimingsRecorder() @@ -254,27 +279,6 @@ const char* FrameTimingsRecorder::GetFrameNumberTraceArg() const { return frame_number_trace_arg_val_.c_str(); } -static const char* StateToString(FrameTimingsRecorder::State state) { -#ifndef NDEBUG - switch (state) { - case FrameTimingsRecorder::State::kUninitialized: - return "kUninitialized"; - case FrameTimingsRecorder::State::kVsync: - return "kVsync"; - case FrameTimingsRecorder::State::kBuildStart: - return "kBuildStart"; - case FrameTimingsRecorder::State::kBuildEnd: - return "kBuildEnd"; - case FrameTimingsRecorder::State::kRasterStart: - return "kRasterStart"; - case FrameTimingsRecorder::State::kRasterEnd: - return "kRasterEnd"; - }; - FML_UNREACHABLE(); -#endif - return ""; -} - void FrameTimingsRecorder::AssertInState(State state) const { FML_DCHECK(state_ == state) << "Expected state " << StateToString(state) << ", actual state " << StateToString(state_); diff --git a/flow/frame_timings.h b/flow/frame_timings.h index ac5a7e470215e..ccccd89dddb88 100644 --- a/flow/frame_timings.h +++ b/flow/frame_timings.h @@ -123,7 +123,7 @@ class FrameTimingsRecorder { /// Instead of adding a `GetState` method and asserting on the result, this /// method prevents other logic from relying on the state. /// - /// In opt builds, this call is a no-op. + /// In release builds, this call is a no-op. void AssertInState(State state) const; private: diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index d8f0c4f7d46fe..0b812af0d6e98 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -25,8 +25,8 @@ namespace { using ::testing::Invoke; using ::testing::ReturnRef; -static void PostSync(const fml::RefPtr& task_runner, - const fml::closure& task) { +void PostSync(const fml::RefPtr& task_runner, + const fml::closure& task) { fml::AutoResetWaitableEvent latch; fml::TaskRunner::RunNowOrPostTask(task_runner, [&latch, &task] { task(); @@ -600,10 +600,12 @@ TEST_F(EngineTest, AnimatorAcceptsMultipleRenders) { }); })); - static fml::AutoResetWaitableEvent callback_ready_latch; + fml::AutoResetWaitableEvent callback_ready_latch; callback_ready_latch.Reset(); AddNativeCallback("NotifyNative", - [](auto args) { callback_ready_latch.Signal(); }); + CREATE_NATIVE_ENTRY([&callback_ready_latch](auto args) { + callback_ready_latch.Signal(); + })); std::unique_ptr animator; PostSync(task_runners_.GetUITaskRunner(), @@ -622,8 +624,8 @@ TEST_F(EngineTest, AnimatorAcceptsMultipleRenders) { engine_context->Run(std::move(configuration)); engine_context->EngineTaskSync([](Engine& engine) { - engine.AddView(1, {1, 10, 10, 22, 0}); - engine.AddView(2, {1, 10, 10, 22, 0}); + engine.AddView(1, ViewportMetrics{1, 10, 10, 22, 0}); + engine.AddView(2, ViewportMetrics{1, 10, 10, 22, 0}); }); callback_ready_latch.Wait(); @@ -752,9 +754,9 @@ TEST_F(EngineTest, AnimatorSubmitPartialViewsForWarmUp) { // Schedule a frame to make the animator create a continuation. engine.ScheduleFrame(true); // Add multiple views. - engine.AddView(0, {1, 10, 10, 22, 0}); - engine.AddView(1, {1, 10, 10, 22, 0}); - engine.AddView(2, {1, 10, 10, 22, 0}); + engine.AddView(0, ViewportMetrics{1, 10, 10, 22, 0}); + engine.AddView(1, ViewportMetrics{1, 10, 10, 22, 0}); + engine.AddView(2, ViewportMetrics{1, 10, 10, 22, 0}); }); continuation_ready_latch.Wait(); From 8731e4759d14b214eb59161d922dcb8b4eb767d0 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 20 Feb 2024 14:32:27 -0800 Subject: [PATCH 23/24] Add web platform dispatcher test --- .../platform_dispatcher_test.dart | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/lib/web_ui/test/engine/platform_dispatcher/platform_dispatcher_test.dart b/lib/web_ui/test/engine/platform_dispatcher/platform_dispatcher_test.dart index 89cf4c8388302..0d3d646a76c4f 100644 --- a/lib/web_ui/test/engine/platform_dispatcher/platform_dispatcher_test.dart +++ b/lib/web_ui/test/engine/platform_dispatcher/platform_dispatcher_test.dart @@ -417,6 +417,34 @@ void testMain() { dispatcher.dispose(); expect(dispatcher.accessibilityPlaceholder.isConnected, isFalse); }); + + + test('scheduleWarmupFrame should call both callbacks and flush microtasks', () async { + bool microtaskFlushed = false; + bool beginFrameCalled = false; + final Completer drawFrameCalled = Completer(); + dispatcher.scheduleWarmUpFrame(beginFrame: () { + expect(microtaskFlushed, false); + expect(drawFrameCalled.isCompleted, false); + expect(beginFrameCalled, false); + beginFrameCalled = true; + scheduleMicrotask(() { + expect(microtaskFlushed, false); + expect(drawFrameCalled.isCompleted, false); + microtaskFlushed = true; + }); + expect(microtaskFlushed, false); + }, drawFrame: () { + expect(beginFrameCalled, true); + expect(microtaskFlushed, true); + expect(drawFrameCalled.isCompleted, false); + drawFrameCalled.complete(); + }); + await drawFrameCalled.future; + expect(beginFrameCalled, true); + expect(drawFrameCalled.isCompleted, true); + expect(microtaskFlushed, true); + }); }); } From 34998fa6ecc35bf3a07689e95b1e43bb5537a867 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 20 Feb 2024 14:49:29 -0800 Subject: [PATCH 24/24] Fix lint --- shell/common/engine_unittests.cc | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 0b812af0d6e98..db31e8c0a339b 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -25,6 +25,8 @@ namespace { using ::testing::Invoke; using ::testing::ReturnRef; +fml::AutoResetWaitableEvent native_latch; + void PostSync(const fml::RefPtr& task_runner, const fml::closure& task) { fml::AutoResetWaitableEvent latch; @@ -600,12 +602,8 @@ TEST_F(EngineTest, AnimatorAcceptsMultipleRenders) { }); })); - fml::AutoResetWaitableEvent callback_ready_latch; - callback_ready_latch.Reset(); - AddNativeCallback("NotifyNative", - CREATE_NATIVE_ENTRY([&callback_ready_latch](auto args) { - callback_ready_latch.Signal(); - })); + native_latch.Reset(); + AddNativeCallback("NotifyNative", [](auto args) { native_latch.Signal(); }); std::unique_ptr animator; PostSync(task_runners_.GetUITaskRunner(), @@ -628,7 +626,7 @@ TEST_F(EngineTest, AnimatorAcceptsMultipleRenders) { engine.AddView(2, ViewportMetrics{1, 10, 10, 22, 0}); }); - callback_ready_latch.Wait(); + native_latch.Wait(); engine_context->EngineTaskSync( [](Engine& engine) { engine.ScheduleFrame(); });