From ffca51fcdd2ec125df3f2658af328aee25672cfd Mon Sep 17 00:00:00 2001 From: liyuqian Date: Mon, 30 Sep 2019 11:25:50 -0700 Subject: [PATCH] Reland "Smooth out iOS irregular input events delivery (#12280)" (flutter/engine#12385) This reverts commit 56bb40c0179628e37ba3614534552441642c0492. Additionally, we fix https://github.com/flutter/flutter/issues/40863 by adding a secondary VSYNC callback. Unit tests are updated to provide VSYNC mocking and check the fix of https://github.com/flutter/flutter/issues/40863. The root cause of having https://github.com/flutter/flutter/issues/40863 is the false assumption that each input event must trigger a new frame. That was true in the framework PR https://github.com/flutter/flutter/pull/36616 because the input events there are all scrolling move events. When the PR was ported to the engine, we can no longer distinguish different types of events, and tap events may no longer trigger a new frame. Therefore, this PR directly hooks into the `VsyncWaiter` and uses its (newly added) secondary callback to dispatch the pending input event. --- .../ci/licenses_golden/licenses_flutter | 3 + engine/src/flutter/shell/common/BUILD.gn | 3 + engine/src/flutter/shell/common/animator.cc | 4 + engine/src/flutter/shell/common/animator.h | 17 ++ engine/src/flutter/shell/common/engine.cc | 26 +- engine/src/flutter/shell/common/engine.h | 27 +- .../shell/common/fixtures/shell_test.dart | 8 + .../shell/common/input_events_unittests.cc | 256 ++++++++++++++++++ .../src/flutter/shell/common/platform_view.cc | 6 + .../src/flutter/shell/common/platform_view.h | 89 +++--- .../shell/common/pointer_data_dispatcher.cc | 63 +++++ .../shell/common/pointer_data_dispatcher.h | 175 ++++++++++++ engine/src/flutter/shell/common/shell.cc | 14 +- engine/src/flutter/shell/common/shell_test.cc | 122 ++++++++- engine/src/flutter/shell/common/shell_test.h | 53 +++- .../src/flutter/shell/common/vsync_waiter.cc | 79 ++++-- .../src/flutter/shell/common/vsync_waiter.h | 10 +- .../platform/darwin/ios/platform_view_ios.h | 3 + .../platform/darwin/ios/platform_view_ios.mm | 6 + 19 files changed, 883 insertions(+), 81 deletions(-) create mode 100644 engine/src/flutter/shell/common/input_events_unittests.cc create mode 100644 engine/src/flutter/shell/common/pointer_data_dispatcher.cc create mode 100644 engine/src/flutter/shell/common/pointer_data_dispatcher.h diff --git a/engine/src/flutter/ci/licenses_golden/licenses_flutter b/engine/src/flutter/ci/licenses_golden/licenses_flutter index 6b5b591b48f..cccbe6a7537 100644 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter @@ -487,6 +487,7 @@ FILE: ../../../flutter/shell/common/canvas_spy_unittests.cc FILE: ../../../flutter/shell/common/engine.cc FILE: ../../../flutter/shell/common/engine.h FILE: ../../../flutter/shell/common/fixtures/shell_test.dart +FILE: ../../../flutter/shell/common/input_events_unittests.cc FILE: ../../../flutter/shell/common/isolate_configuration.cc FILE: ../../../flutter/shell/common/isolate_configuration.h FILE: ../../../flutter/shell/common/persistent_cache.cc @@ -496,6 +497,8 @@ FILE: ../../../flutter/shell/common/pipeline.h FILE: ../../../flutter/shell/common/pipeline_unittests.cc FILE: ../../../flutter/shell/common/platform_view.cc FILE: ../../../flutter/shell/common/platform_view.h +FILE: ../../../flutter/shell/common/pointer_data_dispatcher.cc +FILE: ../../../flutter/shell/common/pointer_data_dispatcher.h FILE: ../../../flutter/shell/common/rasterizer.cc FILE: ../../../flutter/shell/common/rasterizer.h FILE: ../../../flutter/shell/common/run_configuration.cc diff --git a/engine/src/flutter/shell/common/BUILD.gn b/engine/src/flutter/shell/common/BUILD.gn index 34ce4a61bc3..b3745aef2f2 100644 --- a/engine/src/flutter/shell/common/BUILD.gn +++ b/engine/src/flutter/shell/common/BUILD.gn @@ -74,6 +74,8 @@ source_set("common") { "pipeline.h", "platform_view.cc", "platform_view.h", + "pointer_data_dispatcher.cc", + "pointer_data_dispatcher.h", "rasterizer.cc", "rasterizer.h", "run_configuration.cc", @@ -156,6 +158,7 @@ if (current_toolchain == host_toolchain) { shell_host_executable("shell_unittests") { sources = [ "canvas_spy_unittests.cc", + "input_events_unittests.cc", "pipeline_unittests.cc", "shell_test.cc", "shell_test.h", diff --git a/engine/src/flutter/shell/common/animator.cc b/engine/src/flutter/shell/common/animator.cc index c987805784c..b0ef6fca098 100644 --- a/engine/src/flutter/shell/common/animator.cc +++ b/engine/src/flutter/shell/common/animator.cc @@ -244,4 +244,8 @@ void Animator::AwaitVSync() { delegate_.OnAnimatorNotifyIdle(dart_frame_deadline_); } +void Animator::ScheduleSecondaryVsyncCallback(fml::closure callback) { + waiter_->ScheduleSecondaryCallback(std::move(callback)); +} + } // namespace flutter diff --git a/engine/src/flutter/shell/common/animator.h b/engine/src/flutter/shell/common/animator.h index 9f84468c01b..dddc70b145b 100644 --- a/engine/src/flutter/shell/common/animator.h +++ b/engine/src/flutter/shell/common/animator.h @@ -52,6 +52,23 @@ class Animator final { void Render(std::unique_ptr layer_tree); + //-------------------------------------------------------------------------- + /// @brief Schedule a secondary callback to be executed right after the + /// main `VsyncWaiter::AsyncWaitForVsync` callback (which is added + /// by `Animator::RequestFrame`). + /// + /// Like the callback in `AsyncWaitForVsync`, this callback is + /// only scheduled to be called once, and it's supposed to be + /// called in the UI thread. If there is no AsyncWaitForVsync + /// callback (`Animator::RequestFrame` is not called), this + /// secondary callback will still be executed at vsync. + /// + /// This callback is used to provide the vsync signal needed by + /// `SmoothPointerDataDispatcher`. + /// + /// @see `PointerDataDispatcher::ScheduleSecondaryVsyncCallback`. + void ScheduleSecondaryVsyncCallback(fml::closure callback); + void Start(); void Stop(); diff --git a/engine/src/flutter/shell/common/engine.cc b/engine/src/flutter/shell/common/engine.cc index fa3fb65c817..f106c662daf 100644 --- a/engine/src/flutter/shell/common/engine.cc +++ b/engine/src/flutter/shell/common/engine.cc @@ -36,6 +36,7 @@ static constexpr char kSettingsChannel[] = "flutter/settings"; static constexpr char kIsolateChannel[] = "flutter/isolate"; Engine::Engine(Delegate& delegate, + const PointerDataDispatcherMaker& dispatcher_maker, DartVM& vm, fml::RefPtr isolate_snapshot, fml::RefPtr shared_snapshot, @@ -51,6 +52,7 @@ Engine::Engine(Delegate& delegate, image_decoder_(task_runners, vm.GetConcurrentWorkerTaskRunner(), io_manager), + task_runners_(std::move(task_runners)), weak_factory_(this) { // Runtime controller is initialized here because it takes a reference to this // object as its delegate. The delegate may be called in the constructor and @@ -60,7 +62,7 @@ Engine::Engine(Delegate& delegate, &vm, // VM std::move(isolate_snapshot), // isolate snapshot std::move(shared_snapshot), // shared snapshot - std::move(task_runners), // task runners + task_runners_, // task runners std::move(io_manager), // io manager image_decoder_.GetWeakPtr(), // image decoder settings_.advisory_script_uri, // advisory script uri @@ -69,6 +71,8 @@ Engine::Engine(Delegate& delegate, settings_.isolate_create_callback, // isolate create callback settings_.isolate_shutdown_callback // isolate shutdown callback ); + + pointer_data_dispatcher_ = dispatcher_maker(*this); } Engine::~Engine() = default; @@ -381,12 +385,12 @@ void Engine::HandleSettingsPlatformMessage(PlatformMessage* message) { } } -void Engine::DispatchPointerDataPacket(const PointerDataPacket& packet, - uint64_t trace_flow_id) { +void Engine::DispatchPointerDataPacket( + std::unique_ptr packet, + uint64_t trace_flow_id) { TRACE_EVENT0("flutter", "Engine::DispatchPointerDataPacket"); TRACE_FLOW_STEP("flutter", "PointerEvent", trace_flow_id); - animator_->EnqueueTraceFlowId(trace_flow_id); - runtime_controller_->DispatchPointerDataPacket(packet); + pointer_data_dispatcher_->DispatchPacket(std::move(packet), trace_flow_id); } void Engine::DispatchSemanticsAction(int id, @@ -462,6 +466,18 @@ FontCollection& Engine::GetFontCollection() { return font_collection_; } +void Engine::DoDispatchPacket(std::unique_ptr packet, + uint64_t trace_flow_id) { + animator_->EnqueueTraceFlowId(trace_flow_id); + if (runtime_controller_) { + runtime_controller_->DispatchPointerDataPacket(*packet); + } +} + +void Engine::ScheduleSecondaryVsyncCallback(fml::closure callback) { + animator_->ScheduleSecondaryVsyncCallback(std::move(callback)); +} + void Engine::HandleAssetPlatformMessage(fml::RefPtr message) { fml::RefPtr response = message->response(); if (!response) { diff --git a/engine/src/flutter/shell/common/engine.h b/engine/src/flutter/shell/common/engine.h index e2fa9adf323..2e4fd41964b 100644 --- a/engine/src/flutter/shell/common/engine.h +++ b/engine/src/flutter/shell/common/engine.h @@ -22,6 +22,8 @@ #include "flutter/runtime/runtime_controller.h" #include "flutter/runtime/runtime_delegate.h" #include "flutter/shell/common/animator.h" +#include "flutter/shell/common/platform_view.h" +#include "flutter/shell/common/pointer_data_dispatcher.h" #include "flutter/shell/common/rasterizer.h" #include "flutter/shell/common/run_configuration.h" #include "flutter/shell/common/shell_io_manager.h" @@ -65,7 +67,7 @@ namespace flutter { /// name and it does happen to be one of the older classes in the /// repository. /// -class Engine final : public RuntimeDelegate { +class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { public: //---------------------------------------------------------------------------- /// @brief Indicates the result of the call to `Engine::Run`. @@ -234,6 +236,12 @@ class Engine final : public RuntimeDelegate { /// tasks that require access to components /// that cannot be safely accessed by the /// engine. This is the shell. + /// @param dispatcher_maker The callback provided by `PlatformView` for + /// engine to create the pointer data + /// dispatcher. Similar to other engine + /// resources, this dispatcher_maker and its + /// returned dispatcher is only safe to be + /// called from the UI thread. /// @param vm An instance of the running Dart VM. /// @param[in] isolate_snapshot The snapshot used to create the root /// isolate. Even though the isolate is not @@ -265,6 +273,7 @@ class Engine final : public RuntimeDelegate { /// GPU. /// Engine(Delegate& delegate, + const PointerDataDispatcherMaker& dispatcher_maker, DartVM& vm, fml::RefPtr isolate_snapshot, fml::RefPtr shared_snapshot, @@ -649,7 +658,7 @@ class Engine final : public RuntimeDelegate { /// timeline and allow grouping frames and input /// events into logical chunks. /// - void DispatchPointerDataPacket(const PointerDataPacket& packet, + void DispatchPointerDataPacket(std::unique_ptr packet, uint64_t trace_flow_id); //---------------------------------------------------------------------------- @@ -700,11 +709,24 @@ class Engine final : public RuntimeDelegate { // |RuntimeDelegate| FontCollection& GetFontCollection() override; + // |PointerDataDispatcher::Delegate| + void DoDispatchPacket(std::unique_ptr packet, + uint64_t trace_flow_id) override; + + // |PointerDataDispatcher::Delegate| + void ScheduleSecondaryVsyncCallback(fml::closure callback) override; + private: Engine::Delegate& delegate_; const Settings settings_; std::unique_ptr animator_; std::unique_ptr runtime_controller_; + + // The pointer_data_dispatcher_ depends on animator_ and runtime_controller_. + // So it should be defined after them to ensure that pointer_data_dispatcher_ + // is destructed first. + std::unique_ptr pointer_data_dispatcher_; + std::string initial_route_; ViewportMetrics viewport_metrics_; std::shared_ptr asset_manager_; @@ -712,6 +734,7 @@ class Engine final : public RuntimeDelegate { bool have_surface_; FontCollection font_collection_; ImageDecoder image_decoder_; + TaskRunners task_runners_; fml::WeakPtrFactory weak_factory_; // |RuntimeDelegate| diff --git a/engine/src/flutter/shell/common/fixtures/shell_test.dart b/engine/src/flutter/shell/common/fixtures/shell_test.dart index abfd14b56d4..05f4782fc64 100644 --- a/engine/src/flutter/shell/common/fixtures/shell_test.dart +++ b/engine/src/flutter/shell/common/fixtures/shell_test.dart @@ -11,6 +11,7 @@ void main() {} void nativeReportTimingsCallback(List timings) native 'NativeReportTimingsCallback'; void nativeOnBeginFrame(int microseconds) native 'NativeOnBeginFrame'; +void nativeOnPointerDataPacket() native 'NativeOnPointerDataPacket'; @pragma('vm:entry-point') void reportTimingsMain() { @@ -32,6 +33,13 @@ void onBeginFrameMain() { }; } +@pragma('vm:entry-point') +void onPointerDataPacketMain() { + window.onPointerDataPacket = (PointerDataPacket packet) { + nativeOnPointerDataPacket(); + }; +} + @pragma('vm:entry-point') void emptyMain() {} diff --git a/engine/src/flutter/shell/common/input_events_unittests.cc b/engine/src/flutter/shell/common/input_events_unittests.cc new file mode 100644 index 00000000000..69688cce4a5 --- /dev/null +++ b/engine/src/flutter/shell/common/input_events_unittests.cc @@ -0,0 +1,256 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/shell/common/shell_test.h" +#include "flutter/testing/testing.h" + +namespace flutter { +namespace testing { + +// Throughout these tests, the choice of time unit is irrelevant as long as all +// times have the same units. +using UnitlessTime = int; + +// Signature of a generator function that takes the frame index as input and +// returns the time of that frame. +using Generator = std::function; + +//---------------------------------------------------------------------------- +/// Simulate n input events where the i-th one is delivered at delivery_time(i). +/// +/// Simulation results will be written into events_consumed_at_frame whose +/// length will be equal to the number of frames drawn. Each element in the +/// vector is the number of input events consumed up to that frame. (We can't +/// return such vector because ASSERT_TRUE requires return type of void.) +/// +/// We assume (and check) that the delivery latency is some base latency plus a +/// random latency where the random latency must be within one frame: +/// +/// 1. latency = delivery_time(i) - j * frame_time = base_latency + +/// random_latency +/// 2. 0 <= base_latency, 0 <= random_latency < frame_time +/// +/// We also assume that there will be at least one input event per frame if +/// there were no latency. Let j = floor( (delivery_time(i) - base_latency) / +/// frame_time ) be the frame index if there were no latency. Then the set of j +/// should be all integers from 0 to continuous_frame_count - 1 for some +/// integer continuous_frame_count. +/// +/// (Note that there coulds be multiple input events within one frame.) +/// +/// The test here is insensitive to the choice of time unit as long as +/// delivery_time and frame_time are in the same unit. +static void TestSimulatedInputEvents( + ShellTest* fixture, + int num_events, + UnitlessTime base_latency, + Generator delivery_time, + UnitlessTime frame_time, + std::vector& events_consumed_at_frame, + bool restart_engine = false) { + ///// Begin constructing shell /////////////////////////////////////////////// + auto settings = fixture->CreateSettingsForFixture(); + std::unique_ptr shell = fixture->CreateShell(settings, true); + + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("onPointerDataPacketMain"); + + // The following 4 variables are only accessed in the UI thread by + // nativeOnPointerDataPacket and nativeOnBeginFrame between their + // initializations and `shell.reset()`. + events_consumed_at_frame.clear(); + bool will_draw_new_frame = true; + int events_consumed = 0; + int frame_drawn = 0; + auto nativeOnPointerDataPacket = [&events_consumed_at_frame, + &will_draw_new_frame, &events_consumed, + &frame_drawn](Dart_NativeArguments args) { + events_consumed += 1; + if (will_draw_new_frame) { + frame_drawn += 1; + will_draw_new_frame = false; + events_consumed_at_frame.push_back(events_consumed); + } else { + events_consumed_at_frame.back() = events_consumed; + } + }; + fixture->AddNativeCallback("NativeOnPointerDataPacket", + CREATE_NATIVE_ENTRY(nativeOnPointerDataPacket)); + + ASSERT_TRUE(configuration.IsValid()); + fixture->RunEngine(shell.get(), std::move(configuration)); + + if (restart_engine) { + auto new_configuration = RunConfiguration::InferFromSettings(settings); + new_configuration.SetEntrypoint("onPointerDataPacketMain"); + ASSERT_TRUE(new_configuration.IsValid()); + fixture->RestartEngine(shell.get(), std::move(new_configuration)); + } + ///// End constructing shell ///////////////////////////////////////////////// + + ASSERT_GE(base_latency, 0); + + // Check that delivery_time satisfies our assumptions. + int continuous_frame_count = 0; + for (int i = 0; i < num_events; i += 1) { + // j is the frame index of event i if there were no latency. + int j = static_cast((delivery_time(i) - base_latency) / frame_time); + if (j == continuous_frame_count) { + continuous_frame_count += 1; + } + double random_latency = delivery_time(i) - j * frame_time - base_latency; + ASSERT_GE(random_latency, 0); + ASSERT_LT(random_latency, frame_time); + + // If there were no latency, there should be at least one event per frame. + // Hence j should never skip any integer less than continuous_frame_count. + ASSERT_LT(j, continuous_frame_count); + } + + // This has to be running on a different thread than Platform thread to avoid + // dead locks. + auto simulation = std::async(std::launch::async, [&]() { + // i is the input event's index. + // j is the frame's index. + for (int i = 0, j = 0; i < num_events; j += 1) { + double t = j * frame_time; + while (i < num_events && delivery_time(i) <= t) { + ShellTest::DispatchFakePointerData(shell.get()); + i += 1; + } + ShellTest::VSyncFlush(shell.get(), will_draw_new_frame); + } + // Finally, issue a vsync for the pending event that may be generated duing + // the last vsync. + ShellTest::VSyncFlush(shell.get(), will_draw_new_frame); + }); + + simulation.wait(); + shell.reset(); + + // Make sure that all events have been consumed so + // https://github.com/flutter/flutter/issues/40863 won't happen again. + ASSERT_EQ(events_consumed_at_frame.back(), num_events); +} + +TEST_F(ShellTest, MissAtMostOneFrameForIrregularInputEvents) { + // We don't use `constexpr int frame_time` here because MSVC doesn't handle + // it well with lambda capture. + UnitlessTime frame_time = 10; + UnitlessTime base_latency = 0.5 * frame_time; + Generator extreme = [frame_time, base_latency](int i) { + return static_cast( + i * frame_time + base_latency + + (i % 2 == 0 ? 0.1 * frame_time : 0.9 * frame_time)); + }; + constexpr int n = 40; + std::vector events_consumed_at_frame; + TestSimulatedInputEvents(this, n, base_latency, extreme, frame_time, + events_consumed_at_frame); + int frame_drawn = events_consumed_at_frame.size(); + ASSERT_GE(frame_drawn, n - 1); + + // Make sure that it also works after an engine restart. + TestSimulatedInputEvents(this, n, base_latency, extreme, frame_time, + events_consumed_at_frame, true /* restart_engine */); + int frame_drawn_after_restart = events_consumed_at_frame.size(); + ASSERT_GE(frame_drawn_after_restart, n - 1); +} + +TEST_F(ShellTest, DelayAtMostOneEventForFasterThanVSyncInputEvents) { + // We don't use `constexpr int frame_time` here because MSVC doesn't handle + // it well with lambda capture. + UnitlessTime frame_time = 10; + UnitlessTime base_latency = 0.2 * frame_time; + Generator double_sampling = [frame_time, base_latency](int i) { + return static_cast(i * 0.5 * frame_time + base_latency); + }; + constexpr int n = 40; + std::vector events_consumed_at_frame; + TestSimulatedInputEvents(this, n, base_latency, double_sampling, frame_time, + events_consumed_at_frame); + + // Draw one extra frame due to delaying a pending packet for the next frame. + int frame_drawn = events_consumed_at_frame.size(); + ASSERT_EQ(frame_drawn, n / 2 + 1); + + for (int i = 0; i < n / 2; i += 1) { + ASSERT_GE(events_consumed_at_frame[i], 2 * i - 1); + } +} + +TEST_F(ShellTest, HandlesActualIphoneXsInputEvents) { + // Actual delivery times measured on iPhone Xs, in the unit of frame_time + // (16.67ms for 60Hz). + constexpr double iphone_xs_times[] = {0.15, + 1.0773046874999999, + 2.1738720703124996, + 3.0579052734374996, + 4.0890087890624995, + 5.0952685546875, + 6.1251708984375, + 7.1253076171875, + 8.125927734374999, + 9.37248046875, + 10.133950195312499, + 11.161201171875, + 12.226992187499999, + 13.1443798828125, + 14.440327148437499, + 15.091684570312498, + 16.138681640625, + 17.126469726562497, + 18.1592431640625, + 19.371372070312496, + 20.033774414062496, + 21.021782226562497, + 22.070053710937497, + 23.325541992187496, + 24.119648437499997, + 25.084262695312496, + 26.077866210937497, + 27.036547851562496, + 28.035073242187497, + 29.081411132812498, + 30.066064453124998, + 31.089360351562497, + 32.086142578125, + 33.4618798828125, + 34.14697265624999, + 35.0513525390625, + 36.136025390624994, + 37.1618408203125, + 38.144472656249995, + 39.201123046875, + 40.4339501953125, + 41.1552099609375, + 42.102128906249995, + 43.0426318359375, + 44.070131835937495, + 45.08862304687499, + 46.091469726562494}; + constexpr int n = sizeof(iphone_xs_times) / sizeof(iphone_xs_times[0]); + // We don't use `constexpr int frame_time` here because MSVC doesn't handle + // it well with lambda capture. + UnitlessTime frame_time = 10000; + for (double base_latency_f = 0; base_latency_f < 1; base_latency_f += 0.1) { + // Everything is converted to int to avoid floating point error in + // TestSimulatedInputEvents. + UnitlessTime base_latency = + static_cast(base_latency_f * frame_time); + Generator iphone_xs_generator = [frame_time, iphone_xs_times, + base_latency](int i) { + return base_latency + + static_cast(iphone_xs_times[i] * frame_time); + }; + std::vector events_consumed_at_frame; + TestSimulatedInputEvents(this, n, base_latency, iphone_xs_generator, + frame_time, events_consumed_at_frame); + int frame_drawn = events_consumed_at_frame.size(); + ASSERT_GE(frame_drawn, n - 1); + } +} + +} // namespace testing +} // namespace flutter diff --git a/engine/src/flutter/shell/common/platform_view.cc b/engine/src/flutter/shell/common/platform_view.cc index 4a84f27f4e4..058ad55d0c8 100644 --- a/engine/src/flutter/shell/common/platform_view.cc +++ b/engine/src/flutter/shell/common/platform_view.cc @@ -88,6 +88,12 @@ sk_sp PlatformView::CreateResourceContext() const { void PlatformView::ReleaseResourceContext() const {} +PointerDataDispatcherMaker PlatformView::GetDispatcherMaker() { + return [](DefaultPointerDataDispatcher::Delegate& delegate) { + return std::make_unique(delegate); + }; +} + fml::WeakPtr PlatformView::GetWeakPtr() const { return weak_factory_.GetWeakPtr(); } diff --git a/engine/src/flutter/shell/common/platform_view.h b/engine/src/flutter/shell/common/platform_view.h index 41439915051..ad4170f776a 100644 --- a/engine/src/flutter/shell/common/platform_view.h +++ b/engine/src/flutter/shell/common/platform_view.h @@ -16,6 +16,7 @@ #include "flutter/lib/ui/window/platform_message.h" #include "flutter/lib/ui/window/pointer_data_packet.h" #include "flutter/lib/ui/window/viewport_metrics.h" +#include "flutter/shell/common/pointer_data_dispatcher.h" #include "flutter/shell/common/surface.h" #include "flutter/shell/common/vsync_waiter.h" #include "third_party/skia/include/core/SkSize.h" @@ -418,17 +419,23 @@ class PlatformView { /// virtual void ReleaseResourceContext() const; + //-------------------------------------------------------------------------- + /// @brief Returns a platform-specific PointerDataDispatcherMaker so the + /// `Engine` can construct the PointerDataPacketDispatcher based + /// on platforms. + virtual PointerDataDispatcherMaker GetDispatcherMaker(); + //---------------------------------------------------------------------------- /// @brief Returns a weak pointer to the platform view. Since the - /// platform view may only be created, accessed and destroyed on - /// the platform thread, any access to the platform view from a - /// non-platform task runner needs a weak pointer to the platform - /// view along with a reference to the platform task runner. A - /// task must be posted to the platform task runner with the weak - /// pointer captured in the same. The platform view method may - /// only be called in the posted task once the weak pointer - /// validity has been checked. This method is used by callers to - /// obtain that weak pointer. + /// platform view may only be created, accessed and destroyed + /// on the platform thread, any access to the platform view + /// from a non-platform task runner needs a weak pointer to + /// the platform view along with a reference to the platform + /// task runner. A task must be posted to the platform task + /// runner with the weak pointer captured in the same. The + /// platform view method may only be called in the posted task + /// once the weak pointer validity has been checked. This + /// method is used by callers to obtain that weak pointer. /// /// @return The weak pointer to the platform view. /// @@ -446,13 +453,14 @@ class PlatformView { //---------------------------------------------------------------------------- /// @brief Sets a callback that gets executed when the rasterizer renders - /// the next frame. Due to the asynchronous nature of rendering in - /// Flutter, embedders usually add a placeholder over the - /// contents in which Flutter is going to render when Flutter is - /// first initialized. This callback may be used as a signal to - /// remove that placeholder. The callback is executed on the - /// render task runner and not the platform task runner. It is - /// the embedder's responsibility to re-thread as necessary. + /// the next frame. Due to the asynchronous nature of + /// rendering in Flutter, embedders usually add a placeholder + /// over the contents in which Flutter is going to render when + /// Flutter is first initialized. This callback may be used as + /// a signal to remove that placeholder. The callback is + /// executed on the render task runner and not the platform + /// task runner. It is the embedder's responsibility to + /// re-thread as necessary. /// /// @attention The callback is executed on the render task runner and not the /// platform task runner. Embedders must re-thread as necessary. @@ -465,8 +473,8 @@ class PlatformView { //---------------------------------------------------------------------------- /// @brief Dispatches pointer events from the embedder to the /// framework. Each pointer data packet may contain multiple - /// pointer input events. Each call to this method wakes up the UI - /// thread. + /// pointer input events. Each call to this method wakes up + /// the UI thread. /// /// @param[in] packet The pointer data packet to dispatch to the framework. /// @@ -475,16 +483,17 @@ class PlatformView { //-------------------------------------------------------------------------- /// @brief Used by the embedder to specify a texture that it wants the /// rasterizer to composite within the Flutter layer tree. All - /// textures must have a unique identifier. When the rasterizer - /// encounters an external texture within its hierarchy, it gives - /// the embedder a chance to update that texture on the GPU thread - /// before it composites the same on-screen. + /// textures must have a unique identifier. When the + /// rasterizer encounters an external texture within its + /// hierarchy, it gives the embedder a chance to update that + /// texture on the GPU thread before it composites the same + /// on-screen. /// /// @attention This method must only be called once per texture. When the - /// texture is updated, calling `MarkTextureFrameAvailable` with - /// the specified texture identifier is sufficient to make Flutter - /// re-render the frame with the updated texture composited - /// in-line. + /// texture is updated, calling `MarkTextureFrameAvailable` + /// with the specified texture identifier is sufficient to + /// make Flutter re-render the frame with the updated texture + /// composited in-line. /// /// @see UnregisterTexture, MarkTextureFrameAvailable /// @@ -494,10 +503,11 @@ class PlatformView { void RegisterTexture(std::shared_ptr texture); //-------------------------------------------------------------------------- - /// @brief Used by the embedder to notify the rasterizer that it will no - /// longer attempt to composite the specified texture within the - /// layer tree. This allows the rasterizer to collect associated - /// resources. + /// @brief Used by the embedder to notify the rasterizer that it will + /// no + /// longer attempt to composite the specified texture within + /// the layer tree. This allows the rasterizer to collect + /// associated resources. /// /// @attention This call must only be called once per texture identifier. /// @@ -514,13 +524,14 @@ class PlatformView { /// of the previously registered texture have been updated. /// Typically, Flutter will only render a frame if there is an /// updated layer tree. However, in cases where the layer tree - /// is static but one of the externally composited textures has - /// been updated by the embedder, the embedder needs to notify - /// the rasterizer to render a new frame. In such cases, the - /// existing layer tree may be reused with the frame re-composited - /// with all updated external textures. Unlike the calls to - /// register and unregister the texture, this call must be made - /// each time a new texture frame is available. + /// is static but one of the externally composited textures + /// has been updated by the embedder, the embedder needs to + /// notify the rasterizer to render a new frame. In such + /// cases, the existing layer tree may be reused with the + /// frame re-composited with all updated external textures. + /// Unlike the calls to register and unregister the texture, + /// this call must be made each time a new texture frame is + /// available. /// /// @see RegisterTexture, UnregisterTexture /// @@ -536,8 +547,8 @@ class PlatformView { SkISize size_; fml::WeakPtrFactory weak_factory_; - // Unlike all other methods on the platform view, this is called on the GPU - // task runner. + // Unlike all other methods on the platform view, this is called on the + // GPU task runner. virtual std::unique_ptr CreateRenderingSurface(); private: diff --git a/engine/src/flutter/shell/common/pointer_data_dispatcher.cc b/engine/src/flutter/shell/common/pointer_data_dispatcher.cc new file mode 100644 index 00000000000..8341ecec10a --- /dev/null +++ b/engine/src/flutter/shell/common/pointer_data_dispatcher.cc @@ -0,0 +1,63 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/shell/common/pointer_data_dispatcher.h" + +namespace flutter { + +PointerDataDispatcher::~PointerDataDispatcher() = default; +DefaultPointerDataDispatcher::~DefaultPointerDataDispatcher() = default; + +SmoothPointerDataDispatcher::SmoothPointerDataDispatcher(Delegate& delegate) + : DefaultPointerDataDispatcher(delegate), weak_factory_(this) {} +SmoothPointerDataDispatcher::~SmoothPointerDataDispatcher() = default; + +void DefaultPointerDataDispatcher::DispatchPacket( + std::unique_ptr packet, + uint64_t trace_flow_id) { + delegate_.DoDispatchPacket(std::move(packet), trace_flow_id); +} + +void SmoothPointerDataDispatcher::DispatchPacket( + std::unique_ptr packet, + uint64_t trace_flow_id) { + if (is_pointer_data_in_progress_) { + if (pending_packet_ != nullptr) { + DispatchPendingPacket(); + } + pending_packet_ = std::move(packet); + pending_trace_flow_id_ = trace_flow_id; + } else { + FML_DCHECK(pending_packet_ == nullptr); + DefaultPointerDataDispatcher::DispatchPacket(std::move(packet), + trace_flow_id); + } + is_pointer_data_in_progress_ = true; + ScheduleSecondaryVsyncCallback(); +} + +void SmoothPointerDataDispatcher::ScheduleSecondaryVsyncCallback() { + delegate_.ScheduleSecondaryVsyncCallback( + [dispatcher = weak_factory_.GetWeakPtr()]() { + if (dispatcher && dispatcher->is_pointer_data_in_progress_) { + if (dispatcher->pending_packet_ != nullptr) { + dispatcher->DispatchPendingPacket(); + } else { + dispatcher->is_pointer_data_in_progress_ = false; + } + } + }); +} + +void SmoothPointerDataDispatcher::DispatchPendingPacket() { + FML_DCHECK(pending_packet_ != nullptr); + FML_DCHECK(is_pointer_data_in_progress_); + DefaultPointerDataDispatcher::DispatchPacket(std::move(pending_packet_), + pending_trace_flow_id_); + pending_packet_ = nullptr; + pending_trace_flow_id_ = -1; + ScheduleSecondaryVsyncCallback(); +} + +} // namespace flutter diff --git a/engine/src/flutter/shell/common/pointer_data_dispatcher.h b/engine/src/flutter/shell/common/pointer_data_dispatcher.h new file mode 100644 index 00000000000..d67a5b6fe7f --- /dev/null +++ b/engine/src/flutter/shell/common/pointer_data_dispatcher.h @@ -0,0 +1,175 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef POINTER_DATA_DISPATCHER_H_ +#define POINTER_DATA_DISPATCHER_H_ + +#include "flutter/runtime/runtime_controller.h" +#include "flutter/shell/common/animator.h" + +namespace flutter { + +class PointerDataDispatcher; + +//------------------------------------------------------------------------------ +/// The `Engine` pointer data dispatcher that forwards the packet received from +/// `PlatformView::DispatchPointerDataPacket` on the platform thread, to +/// `Window::DispatchPointerDataPacket` on the UI thread. +/// +/// This class is used to filter the packets so the Flutter framework on the UI +/// thread will receive packets with some desired properties. See +/// `SmoothPointerDataDispatcher` for an example which filters irregularly +/// delivered packets, and dispatches them in sync with the VSYNC signal. +/// +/// This object will be owned by the engine because it relies on the engine's +/// `Animator` (which owns `VsyncWaiter`) and `RuntomeController` to do the +/// filtering. This object is currently designed to be only called from the UI +/// thread (no thread safety is guaranteed). +/// +/// The `PlatformView` decides which subclass of `PointerDataDispatcher` is +/// constructed by sending a `PointerDataDispatcherMaker` to the engine's +/// constructor in `Shell::CreateShellOnPlatformThread`. This is needed because: +/// (1) Different platforms (e.g., Android, iOS) have different dispatchers +/// so the decision has to be made per `PlatformView`. +/// (2) The `PlatformView` can only be accessed from the PlatformThread while +/// this class (as owned by engine) can only be accessed in the UI thread. +/// Hence `PlatformView` creates a `PointerDataDispatchMaker` on the +/// platform thread, and sends it to the UI thread for the final +/// construction of the `PointerDataDispatcher`. +class PointerDataDispatcher { + public: + /// The interface for Engine to implement. + class Delegate { + public: + /// Actually dispatch the packet using Engine's `animator_` and + /// `runtime_controller_`. + virtual void DoDispatchPacket(std::unique_ptr packet, + uint64_t trace_flow_id) = 0; + + //-------------------------------------------------------------------------- + /// @brief Schedule a secondary callback to be executed right after the + /// main `VsyncWaiter::AsyncWaitForVsync` callback (which is added + /// by `Animator::RequestFrame`). + /// + /// Like the callback in `AsyncWaitForVsync`, this callback is + /// only scheduled to be called once, and it will be called in the + /// UI thread. If there is no AsyncWaitForVsync callback + /// (`Animator::RequestFrame` is not called), this secondary + /// callback will still be executed at vsync. + /// + /// This callback is used to provide the vsync signal needed by + /// `SmoothPointerDataDispatcher`. + virtual void ScheduleSecondaryVsyncCallback(fml::closure callback) = 0; + }; + + //---------------------------------------------------------------------------- + /// @brief Signal that `PlatformView` has a packet to be dispatched. + /// + /// @param[in] packet The `PointerDataPacket` to be dispatched. + /// @param[in] trace_flow_id The id for `Animator::EnqueueTraceFlowId`. + virtual void DispatchPacket(std::unique_ptr packet, + uint64_t trace_flow_id) = 0; + + //---------------------------------------------------------------------------- + /// @brief Default destructor. + virtual ~PointerDataDispatcher(); +}; + +//------------------------------------------------------------------------------ +/// The default dispatcher that forwards the packet without any modification. +/// +class DefaultPointerDataDispatcher : public PointerDataDispatcher { + public: + DefaultPointerDataDispatcher(Delegate& delegate) : delegate_(delegate) {} + + // |PointerDataDispatcer| + void DispatchPacket(std::unique_ptr packet, + uint64_t trace_flow_id) override; + + virtual ~DefaultPointerDataDispatcher(); + + protected: + Delegate& delegate_; + + FML_DISALLOW_COPY_AND_ASSIGN(DefaultPointerDataDispatcher); +}; + +//------------------------------------------------------------------------------ +/// A dispatcher that may temporarily store and defer the last received +/// PointerDataPacket if multiple packets are received in one VSYNC. The +/// deferred packet will be sent in the next vsync in order to smooth out the +/// events. This filters out irregular input events delivery to provide a smooth +/// scroll on iPhone X/Xs. +/// +/// It works as follows: +/// +/// When `DispatchPacket` is called while a preivous pointer data dispatch is +/// still in progress (its frame isn't finished yet), it means that an input +/// event is delivered to us too fast. That potentially means a later event will +/// be too late which could cause the missing of a frame. Hence we'll cache it +/// in `pending_packet_` for the next frame to smooth it out. +/// +/// If the input event is sent to us regularly at the same rate of VSYNC (say +/// at 60Hz), this would be identical to `DefaultPointerDataDispatcher` where +/// `runtime_controller_->DispatchPointerDataPacket` is always called right +/// away. That's because `is_pointer_data_in_progress_` will always be false +/// when `DispatchPacket` is called since it will be cleared by the end of a +/// frame through `ScheduleSecondaryVsyncCallback`. This is the case for all +/// Android/iOS devices before iPhone X/XS. +/// +/// If the input event is irregular, but with a random latency of no more than +/// one frame, this would guarantee that we'll miss at most 1 frame. Without +/// this, we could miss half of the frames. +/// +/// If the input event is delivered at a higher rate than that of VSYNC, this +/// would at most add a latency of one event delivery. For example, if the +/// input event is delivered at 120Hz (this is only true for iPad pro, not even +/// iPhone X), this may delay the handling of an input event by 8ms. +/// +/// The assumption of this solution is that the sampling itself is still +/// regular. Only the event delivery is allowed to be irregular. So far this +/// assumption seems to hold on all devices. If it's changed in the future, +/// we'll need a different solution. +/// +/// See also input_events_unittests.cc where we test all our claims above. +class SmoothPointerDataDispatcher : public DefaultPointerDataDispatcher { + public: + SmoothPointerDataDispatcher(Delegate& delegate); + + // |PointerDataDispatcer| + void DispatchPacket(std::unique_ptr packet, + uint64_t trace_flow_id) override; + + virtual ~SmoothPointerDataDispatcher(); + + private: + // If non-null, this will be a pending pointer data packet for the next frame + // to consume. This is used to smooth out the irregular drag events delivery. + // See also `DispatchPointerDataPacket` and input_events_unittests.cc. + std::unique_ptr pending_packet_; + int pending_trace_flow_id_ = -1; + + bool is_pointer_data_in_progress_ = false; + + fml::WeakPtrFactory weak_factory_; + + void DispatchPendingPacket(); + + void ScheduleSecondaryVsyncCallback(); + + FML_DISALLOW_COPY_AND_ASSIGN(SmoothPointerDataDispatcher); +}; + +//-------------------------------------------------------------------------- +/// @brief Signature for constructing PointerDataDispatcher. +/// +/// @param[in] delegate the `Flutter::Engine` +/// +using PointerDataDispatcherMaker = + std::function( + PointerDataDispatcher::Delegate&)>; + +} // namespace flutter + +#endif // POINTER_DATA_DISPATCHER_H_ diff --git a/engine/src/flutter/shell/common/shell.cc b/engine/src/flutter/shell/common/shell.cc index c22478680b9..9c97908c399 100644 --- a/engine/src/flutter/shell/common/shell.cc +++ b/engine/src/flutter/shell/common/shell.cc @@ -104,6 +104,10 @@ std::unique_ptr Shell::CreateShellOnPlatformThread( io_manager_promise.set_value(std::move(io_manager)); }); + // Send dispatcher_maker to the engine constructor because shell won't have + // platform_view set until Shell::Setup is called later. + auto dispatcher_maker = platform_view->GetDispatcherMaker(); + // Create the engine on the UI thread. std::promise> engine_promise; auto engine_future = engine_promise.get_future(); @@ -111,6 +115,7 @@ std::unique_ptr Shell::CreateShellOnPlatformThread( shell->GetTaskRunners().GetUITaskRunner(), fml::MakeCopyable([&engine_promise, // shell = shell.get(), // + &dispatcher_maker, // isolate_snapshot = std::move(isolate_snapshot), // shared_snapshot = std::move(shared_snapshot), // vsync_waiter = std::move(vsync_waiter), // @@ -126,6 +131,7 @@ std::unique_ptr Shell::CreateShellOnPlatformThread( engine_promise.set_value(std::make_unique( *shell, // + dispatcher_maker, // *shell->GetDartVM(), // std::move(isolate_snapshot), // std::move(shared_snapshot), // @@ -721,11 +727,11 @@ void Shell::OnPlatformViewDispatchPointerDataPacket( TRACE_FLOW_BEGIN("flutter", "PointerEvent", next_pointer_flow_id_); FML_DCHECK(is_setup_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); - task_runners_.GetUITaskRunner()->PostTask(fml::MakeCopyable( - [engine = engine_->GetWeakPtr(), packet = std::move(packet), - flow_id = next_pointer_flow_id_] { + task_runners_.GetUITaskRunner()->PostTask( + fml::MakeCopyable([engine = weak_engine_, packet = std::move(packet), + flow_id = next_pointer_flow_id_]() mutable { if (engine) { - engine->DispatchPointerDataPacket(*packet, flow_id); + engine->DispatchPointerDataPacket(std::move(packet), flow_id); } })); next_pointer_flow_id_++; diff --git a/engine/src/flutter/shell/common/shell_test.cc b/engine/src/flutter/shell/common/shell_test.cc index 6887b704ee3..1e7d7c4b1d8 100644 --- a/engine/src/flutter/shell/common/shell_test.cc +++ b/engine/src/flutter/shell/common/shell_test.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include #define FML_USED_ON_EMBEDDER #include "flutter/shell/common/shell_test.h" @@ -101,6 +102,39 @@ void ShellTest::RunEngine(Shell* shell, RunConfiguration configuration) { latch.Wait(); } +void ShellTest::RestartEngine(Shell* shell, RunConfiguration configuration) { + std::promise restarted; + fml::TaskRunner::RunNowOrPostTask( + shell->GetTaskRunners().GetUITaskRunner(), + [shell, &restarted, &configuration]() { + restarted.set_value(shell->engine_->Restart(std::move(configuration))); + }); + ASSERT_TRUE(restarted.get_future().get()); +} + +void ShellTest::VSyncFlush(Shell* shell, bool& will_draw_new_frame) { + fml::AutoResetWaitableEvent latch; + shell->GetTaskRunners().GetPlatformTaskRunner()->PostTask( + [shell, &will_draw_new_frame, &latch] { + // The following UI task ensures that all previous UI tasks are flushed. + fml::AutoResetWaitableEvent ui_latch; + shell->GetTaskRunners().GetUITaskRunner()->PostTask( + [&ui_latch, &will_draw_new_frame]() { + will_draw_new_frame = true; + ui_latch.Signal(); + }); + + ShellTestPlatformView* test_platform_view = + static_cast(shell->GetPlatformView().get()); + do { + test_platform_view->SimulateVSync(); + } while (ui_latch.WaitWithTimeout(fml::TimeDelta::FromMilliseconds(1))); + + latch.Signal(); + }); + latch.Wait(); +} + void ShellTest::PumpOneFrame(Shell* shell) { // Set viewport to nonempty, and call Animator::BeginFrame to make the layer // tree pipeline nonempty. Without either of this, the layer tree below @@ -132,6 +166,16 @@ void ShellTest::PumpOneFrame(Shell* shell) { latch.Wait(); } +void ShellTest::DispatchFakePointerData(Shell* shell) { + fml::AutoResetWaitableEvent latch; + shell->GetTaskRunners().GetPlatformTaskRunner()->PostTask([&latch, shell]() { + auto packet = std::make_unique(1); + shell->OnPlatformViewDispatchPointerDataPacket(std::move(packet)); + latch.Signal(); + }); + latch.Wait(); +} + int ShellTest::UnreportedTimingsCount(Shell* shell) { return shell->unreported_timings_.size(); } @@ -175,17 +219,20 @@ TaskRunners ShellTest::GetTaskRunnersForFixture() { }; } -std::unique_ptr ShellTest::CreateShell(Settings settings) { - return CreateShell(std::move(settings), GetTaskRunnersForFixture()); +std::unique_ptr ShellTest::CreateShell(Settings settings, + bool simulate_vsync) { + return CreateShell(std::move(settings), GetTaskRunnersForFixture(), + simulate_vsync); } std::unique_ptr ShellTest::CreateShell(Settings settings, - TaskRunners task_runners) { + TaskRunners task_runners, + bool simulate_vsync) { return Shell::Create( task_runners, settings, - [](Shell& shell) { - return std::make_unique(shell, - shell.GetTaskRunners()); + [simulate_vsync](Shell& shell) { + return std::make_unique( + shell, shell.GetTaskRunners(), simulate_vsync); }, [](Shell& shell) { return std::make_unique(shell, shell.GetTaskRunners()); @@ -215,18 +262,77 @@ void ShellTest::AddNativeCallback(std::string name, native_resolver_->AddNativeCallback(std::move(name), callback); } +void ShellTestVsyncClock::SimulateVSync() { + std::scoped_lock lock(mutex_); + if (vsync_issued_ >= vsync_promised_.size()) { + vsync_promised_.emplace_back(); + } + FML_CHECK(vsync_issued_ < vsync_promised_.size()); + vsync_promised_[vsync_issued_].set_value(vsync_issued_); + vsync_issued_ += 1; +} + +std::future ShellTestVsyncClock::NextVSync() { + std::scoped_lock lock(mutex_); + vsync_promised_.emplace_back(); + return vsync_promised_.back().get_future(); +} + +void ShellTestVsyncWaiter::AwaitVSync() { + FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); + auto vsync_future = clock_.NextVSync(); + + auto async_wait = std::async([&vsync_future, this]() { + vsync_future.wait(); + + // Post the `FireCallback` to the Platform thread so earlier Platform tasks + // (specifically, the `VSyncFlush` call) will be finished before + // `FireCallback` is executed. This is only needed for our unit tests. + // + // Without this, the repeated VSYNC signals in `VSyncFlush` may start both + // the current frame in the UI thread and the next frame in the secondary + // callback (both of them are waiting for VSYNCs). That breaks the unit + // test's assumption that each frame's VSYNC must be issued by different + // `VSyncFlush` call (which resets the `will_draw_new_frame` bit). + // + // For example, HandlesActualIphoneXsInputEvents will fail without this. + task_runners_.GetPlatformTaskRunner()->PostTask([this]() { + FireCallback(fml::TimePoint::Now(), fml::TimePoint::Now()); + }); + }); +} + ShellTestPlatformView::ShellTestPlatformView(PlatformView::Delegate& delegate, - TaskRunners task_runners) + TaskRunners task_runners, + bool simulate_vsync) : PlatformView(delegate, std::move(task_runners)), - gl_surface_(SkISize::Make(800, 600)) {} + gl_surface_(SkISize::Make(800, 600)), + simulate_vsync_(simulate_vsync) {} ShellTestPlatformView::~ShellTestPlatformView() = default; +std::unique_ptr ShellTestPlatformView::CreateVSyncWaiter() { + return simulate_vsync_ ? std::make_unique(task_runners_, + vsync_clock_) + : PlatformView::CreateVSyncWaiter(); +} + +void ShellTestPlatformView::SimulateVSync() { + vsync_clock_.SimulateVSync(); +} + // |PlatformView| std::unique_ptr ShellTestPlatformView::CreateRenderingSurface() { return std::make_unique(this, true); } +// |PlatformView| +PointerDataDispatcherMaker ShellTestPlatformView::GetDispatcherMaker() { + return [](DefaultPointerDataDispatcher::Delegate& delegate) { + return std::make_unique(delegate); + }; +} + // |GPUSurfaceGLDelegate| bool ShellTestPlatformView::GLContextMakeCurrent() { return gl_surface_.MakeCurrent(); diff --git a/engine/src/flutter/shell/common/shell_test.h b/engine/src/flutter/shell/common/shell_test.h index 04d653e6079..ca486230a42 100644 --- a/engine/src/flutter/shell/common/shell_test.h +++ b/engine/src/flutter/shell/common/shell_test.h @@ -9,6 +9,7 @@ #include "flutter/common/settings.h" #include "flutter/fml/macros.h" +#include "flutter/fml/synchronization/thread_annotations.h" #include "flutter/lib/ui/window/platform_message.h" #include "flutter/shell/common/run_configuration.h" #include "flutter/shell/common/shell.h" @@ -28,9 +29,11 @@ class ShellTest : public ThreadTest { ~ShellTest(); Settings CreateSettingsForFixture(); - std::unique_ptr CreateShell(Settings settings); std::unique_ptr CreateShell(Settings settings, - TaskRunners task_runners); + bool simulate_vsync = false); + std::unique_ptr CreateShell(Settings settings, + TaskRunners task_runners, + bool simulate_vsync = false); TaskRunners GetTaskRunnersForFixture(); void SendEnginePlatformMessage(Shell* shell, @@ -41,8 +44,14 @@ class ShellTest : public ThreadTest { static void PlatformViewNotifyCreated( Shell* shell); // This creates the surface static void RunEngine(Shell* shell, RunConfiguration configuration); + static void RestartEngine(Shell* shell, RunConfiguration configuration); + + /// Issue as many VSYNC as needed to flush the UI tasks so far, and reset + /// the `will_draw_new_frame` to true. + static void VSyncFlush(Shell* shell, bool& will_draw_new_frame); static void PumpOneFrame(Shell* shell); + static void DispatchFakePointerData(Shell* shell); // Declare |UnreportedTimingsCount|, |GetNeedsReportTimings| and // |SetNeedsReportTimings| inside |ShellTest| mainly for easier friend class @@ -73,19 +82,57 @@ class ShellTest : public ThreadTest { void SetSnapshotsAndAssets(Settings& settings); }; +class ShellTestVsyncClock { + public: + /// Simulate that a vsync signal is triggered. + void SimulateVSync(); + + /// A future that will return the index the next vsync signal. + std::future NextVSync(); + + private: + std::mutex mutex_; + std::vector> vsync_promised_ FML_GUARDED_BY(mutex_); + size_t vsync_issued_ FML_GUARDED_BY(mutex_) = 0; +}; + +class ShellTestVsyncWaiter : public VsyncWaiter { + public: + ShellTestVsyncWaiter(TaskRunners task_runners, ShellTestVsyncClock& clock) + : VsyncWaiter(std::move(task_runners)), clock_(clock) {} + + protected: + void AwaitVSync() override; + + private: + ShellTestVsyncClock& clock_; +}; + class ShellTestPlatformView : public PlatformView, public GPUSurfaceGLDelegate { public: ShellTestPlatformView(PlatformView::Delegate& delegate, - TaskRunners task_runners); + TaskRunners task_runners, + bool simulate_vsync = false); ~ShellTestPlatformView() override; + void SimulateVSync(); + private: TestGLSurface gl_surface_; + bool simulate_vsync_ = false; + ShellTestVsyncClock vsync_clock_; + // |PlatformView| std::unique_ptr CreateRenderingSurface() override; + // |PlatformView| + std::unique_ptr CreateVSyncWaiter() override; + + // |PlatformView| + PointerDataDispatcherMaker GetDispatcherMaker() override; + // |GPUSurfaceGLDelegate| bool GLContextMakeCurrent() override; diff --git a/engine/src/flutter/shell/common/vsync_waiter.cc b/engine/src/flutter/shell/common/vsync_waiter.cc index aa3637221df..c191a134900 100644 --- a/engine/src/flutter/shell/common/vsync_waiter.cc +++ b/engine/src/flutter/shell/common/vsync_waiter.cc @@ -46,6 +46,38 @@ void VsyncWaiter::AsyncWaitForVsync(Callback callback) { return; } callback_ = std::move(callback); + if (secondary_callback_) { + // Return directly as `AwaitVSync` is already called by + // `ScheduleSecondaryCallback`. + return; + } + } + AwaitVSync(); +} + +void VsyncWaiter::ScheduleSecondaryCallback(fml::closure callback) { + FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); + + if (!callback) { + return; + } + + TRACE_EVENT0("flutter", "ScheduleSecondaryCallback"); + + { + std::scoped_lock lock(callback_mutex_); + if (secondary_callback_) { + // Multiple schedules must result in a single callback per frame interval. + TRACE_EVENT_INSTANT0("flutter", + "MultipleCallsToSecondaryVsyncInFrameInterval"); + return; + } + secondary_callback_ = std::move(callback); + if (callback_) { + // Return directly as `AwaitVSync` is already called by + // `AsyncWaitForVsync`. + return; + } } AwaitVSync(); } @@ -53,13 +85,15 @@ void VsyncWaiter::AsyncWaitForVsync(Callback callback) { void VsyncWaiter::FireCallback(fml::TimePoint frame_start_time, fml::TimePoint frame_target_time) { Callback callback; + fml::closure secondary_callback; { std::scoped_lock lock(callback_mutex_); callback = std::move(callback_); + secondary_callback = std::move(secondary_callback_); } - if (!callback) { + if (!callback && !secondary_callback) { // This means that the vsync waiter implementation fired a callback for a // request we did not make. This is a paranoid check but we still want to // make sure we catch misbehaving vsync implementations. @@ -67,27 +101,34 @@ void VsyncWaiter::FireCallback(fml::TimePoint frame_start_time, return; } - auto flow_identifier = fml::tracing::TraceNonce(); + if (callback) { + auto flow_identifier = fml::tracing::TraceNonce(); - // The base trace ensures that flows have a root to begin from if one does not - // exist. The trace viewer will ignore traces that have no base event trace. - // While all our message loops insert a base trace trace - // (MessageLoop::RunExpiredTasks), embedders may not. - TRACE_EVENT0("flutter", "VsyncFireCallback"); + // The base trace ensures that flows have a root to begin from if one does + // not exist. The trace viewer will ignore traces that have no base event + // trace. While all our message loops insert a base trace trace + // (MessageLoop::RunExpiredTasks), embedders may not. + TRACE_EVENT0("flutter", "VsyncFireCallback"); - TRACE_FLOW_BEGIN("flutter", kVsyncFlowName, flow_identifier); + TRACE_FLOW_BEGIN("flutter", kVsyncFlowName, flow_identifier); - task_runners_.GetUITaskRunner()->PostTaskForTime( - [callback, flow_identifier, frame_start_time, frame_target_time]() { - FML_TRACE_EVENT("flutter", kVsyncTraceName, "StartTime", - frame_start_time, "TargetTime", frame_target_time); - fml::tracing::TraceEventAsyncComplete( - "flutter", "VsyncSchedulingOverhead", fml::TimePoint::Now(), - frame_start_time); - callback(frame_start_time, frame_target_time); - TRACE_FLOW_END("flutter", kVsyncFlowName, flow_identifier); - }, - frame_start_time); + task_runners_.GetUITaskRunner()->PostTaskForTime( + [callback, flow_identifier, frame_start_time, frame_target_time]() { + FML_TRACE_EVENT("flutter", kVsyncTraceName, "StartTime", + frame_start_time, "TargetTime", frame_target_time); + fml::tracing::TraceEventAsyncComplete( + "flutter", "VsyncSchedulingOverhead", fml::TimePoint::Now(), + frame_start_time); + callback(frame_start_time, frame_target_time); + TRACE_FLOW_END("flutter", kVsyncFlowName, flow_identifier); + }, + frame_start_time); + } + + if (secondary_callback) { + task_runners_.GetUITaskRunner()->PostTaskForTime( + std::move(secondary_callback), frame_start_time); + } } float VsyncWaiter::GetDisplayRefreshRate() const { diff --git a/engine/src/flutter/shell/common/vsync_waiter.h b/engine/src/flutter/shell/common/vsync_waiter.h index 5e80dd4c09c..e93b730fe7f 100644 --- a/engine/src/flutter/shell/common/vsync_waiter.h +++ b/engine/src/flutter/shell/common/vsync_waiter.h @@ -26,6 +26,11 @@ class VsyncWaiter : public std::enable_shared_from_this { void AsyncWaitForVsync(Callback callback); + /// Add a secondary callback for the next vsync. + /// + /// See also |PointerDataDispatcher::ScheduleSecondaryVsyncCallback|. + void ScheduleSecondaryCallback(fml::closure callback); + static constexpr float kUnknownRefreshRateFPS = 0.0; // Get the display's maximum refresh rate in the unit of frame per second. @@ -45,7 +50,7 @@ class VsyncWaiter : public std::enable_shared_from_this { // Implementations are meant to override this method and arm their vsync // latches when in response to this invocation. On vsync, they are meant to // invoke the |FireCallback| method once (and only once) with the appropriate - // arguments. + // arguments. This method should not block the current thread. virtual void AwaitVSync() = 0; void FireCallback(fml::TimePoint frame_start_time, @@ -55,6 +60,9 @@ class VsyncWaiter : public std::enable_shared_from_this { std::mutex callback_mutex_; Callback callback_ FML_GUARDED_BY(callback_mutex_); + std::mutex secondary_callback_mutex_; + fml::closure secondary_callback_ FML_GUARDED_BY(callback_mutex_); + FML_DISALLOW_COPY_AND_ASSIGN(VsyncWaiter); }; diff --git a/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.h b/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.h index eeaca3dd9df..51d00e86370 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.h +++ b/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.h @@ -37,6 +37,9 @@ class PlatformViewIOS final : public PlatformView { void RegisterExternalTexture(int64_t id, NSObject* texture); + // |PlatformView| + PointerDataDispatcherMaker GetDispatcherMaker() override; + fml::scoped_nsprotocol GetTextInputPlugin() const; void SetTextInputPlugin(fml::scoped_nsprotocol plugin); diff --git a/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.mm b/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.mm index 77882349397..ce3f25d6ca8 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.mm @@ -98,6 +98,12 @@ void PlatformViewIOS::SetOwnerViewController(fml::WeakPtr } } +PointerDataDispatcherMaker PlatformViewIOS::GetDispatcherMaker() { + return [](DefaultPointerDataDispatcher::Delegate& delegate) { + return std::make_unique(delegate); + }; +} + void PlatformViewIOS::RegisterExternalTexture(int64_t texture_id, NSObject* texture) { RegisterTexture(std::make_shared(texture_id, texture));