From 1950e72cd034ce8b6ddb7821f5d2d2bb4a4d4f64 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Mon, 12 Feb 2024 12:01:28 -0800 Subject: [PATCH 01/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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 8731e4759d14b214eb59161d922dcb8b4eb767d0 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 20 Feb 2024 14:32:27 -0800 Subject: [PATCH 20/23] 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 5d9f48bd7bef8ce9d6bc206453ac976fa45b4832 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 20 Feb 2024 17:42:58 -0800 Subject: [PATCH 21/23] Web comments --- lib/ui/platform_dispatcher.dart | 11 ++++++----- lib/web_ui/lib/src/engine/initialization.dart | 3 ++- lib/web_ui/lib/src/engine/platform_dispatcher.dart | 12 +++++++++--- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index dc82eb911419c..63d7211dccba2 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -811,11 +811,12 @@ class PlatformDispatcher { /// 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 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 platforms and situations, the warm up - /// frame might or might not be actually rendered onto the screen. + /// The application can call this method as soon as it starts up 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 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/lib/web_ui/lib/src/engine/initialization.dart b/lib/web_ui/lib/src/engine/initialization.dart index 78352cf091af3..6448bf7d918e2 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. + // implement it properly. (Same to-do as 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 057d3bcfefb98..592c748e61da2 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -784,9 +784,15 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { @override void scheduleWarmUpFrame({required ui.VoidCallback beginFrame, required ui.VoidCallback drawFrame}) { - Timer.run(beginFrame); - // We use timers here to ensure that microtasks flush in between. - Timer.run(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 to-do as in + // `initializeEngineServices`). + drawFrame(); + }); } /// Updates the application's rendering on the GPU with the newly provided From 556984f108e8115426cb5c290a04407047262cad Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 20 Feb 2024 18:38:37 -0800 Subject: [PATCH 22/23] Fix test --- .../platform_dispatcher_test.dart | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) 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 0d3d646a76c4f..0530dddfb18aa 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 @@ -419,31 +419,21 @@ void testMain() { }); - test('scheduleWarmupFrame should call both callbacks and flush microtasks', () async { - bool microtaskFlushed = false; + test('scheduleWarmupFrame should call both callbacks', () async { 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 2b6d3b2aa4a13b29e6482775cedc5c36909a2ecf Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 21 Feb 2024 10:49:13 -0800 Subject: [PATCH 23/23] Revert timer run change --- lib/web_ui/lib/src/engine/initialization.dart | 2 +- .../lib/src/engine/platform_dispatcher.dart | 18 +++++++++--------- .../platform_dispatcher_test.dart | 1 - 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/web_ui/lib/src/engine/initialization.dart b/lib/web_ui/lib/src/engine/initialization.dart index 6448bf7d918e2..0dab016be43ba 100644 --- a/lib/web_ui/lib/src/engine/initialization.dart +++ b/lib/web_ui/lib/src/engine/initialization.dart @@ -187,7 +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. (Same to-do as in + // implement it properly. (Also see the to-do 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 592c748e61da2..277fe73018ade 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -784,15 +784,15 @@ 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. (Same to-do as in - // `initializeEngineServices`). - drawFrame(); - }); + Timer.run(beginFrame); + // We use timers here to ensure that microtasks flush in between. + // + // TODO(dkwingsmt): This logic was moved from the framework and is different + // from how Web renders a regular frame, which doesn't flush microtasks + // between the callbacks at all (see `initializeEngineServices`). We might + // want to change this. See the to-do in `initializeEngineServices` and + // https://github.com/flutter/engine/pull/50570#discussion_r1496671676 + Timer.run(drawFrame); } /// Updates the application's rendering on the GPU with the newly provided 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 0530dddfb18aa..09d64757b5324 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 @@ -418,7 +418,6 @@ void testMain() { expect(dispatcher.accessibilityPlaceholder.isConnected, isFalse); }); - test('scheduleWarmupFrame should call both callbacks', () async { bool beginFrameCalled = false; final Completer drawFrameCalled = Completer();