From 3cb5ce3f73539e8ac6dd84b2b69002f01f224592 Mon Sep 17 00:00:00 2001 From: Yuqian Li Date: Fri, 13 Sep 2019 14:32:19 -0700 Subject: [PATCH] Revert "Revert "Smooth out iOS irregular input events delivery (#11817)" (#12251)" This reverts commit 3c6383f2db5f8e3df1088e3e17f688842351422a. --- ci/licenses_golden/licenses_flutter | 3 + shell/common/BUILD.gn | 3 + shell/common/engine.cc | 16 +- shell/common/engine.h | 12 +- shell/common/fixtures/shell_test.dart | 11 + shell/common/input_events_unittests.cc | 236 ++++++++++++++++++ shell/common/platform_view.cc | 7 + shell/common/platform_view.h | 89 ++++--- shell/common/pointer_data_dispatcher.cc | 75 ++++++ shell/common/pointer_data_dispatcher.h | 175 +++++++++++++ shell/common/shell.cc | 14 +- shell/common/shell.h | 1 + shell/common/shell_test.cc | 19 ++ shell/common/shell_test.h | 4 + shell/platform/darwin/ios/platform_view_ios.h | 3 + .../platform/darwin/ios/platform_view_ios.mm | 6 + 16 files changed, 625 insertions(+), 49 deletions(-) create mode 100644 shell/common/input_events_unittests.cc create mode 100644 shell/common/pointer_data_dispatcher.cc create mode 100644 shell/common/pointer_data_dispatcher.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index dbb8643280c0d..5d04beaa2c0fe 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -485,6 +485,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 @@ -494,6 +495,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/shell/common/BUILD.gn b/shell/common/BUILD.gn index 34ce4a61bc31d..b3745aef2f262 100644 --- a/shell/common/BUILD.gn +++ b/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/shell/common/engine.cc b/shell/common/engine.cc index fa3fb65c8172d..44dc8f00638cb 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -36,6 +36,7 @@ static constexpr char kSettingsChannel[] = "flutter/settings"; static constexpr char kIsolateChannel[] = "flutter/isolate"; Engine::Engine(Delegate& delegate, + PointerDataDispatcherMaker& dispatcher_maker, DartVM& vm, fml::RefPtr isolate_snapshot, fml::RefPtr shared_snapshot, @@ -60,7 +61,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 +70,9 @@ Engine::Engine(Delegate& delegate, settings_.isolate_create_callback, // isolate create callback settings_.isolate_shutdown_callback // isolate shutdown callback ); + + pointer_data_dispatcher_ = dispatcher_maker(*animator_, *runtime_controller_, + std::move(task_runners)); } 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, @@ -434,6 +438,8 @@ void Engine::Render(std::unique_ptr layer_tree) { layer_tree->set_frame_size(frame_size); animator_->Render(std::move(layer_tree)); + + pointer_data_dispatcher_->OnFrameLayerTreeReceived(); } void Engine::UpdateSemantics(SemanticsNodeUpdates update, diff --git a/shell/common/engine.h b/shell/common/engine.h index e2fa9adf323a3..cb4ccbb53d08b 100644 --- a/shell/common/engine.h +++ b/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" @@ -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 `std::function` 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, + 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); //---------------------------------------------------------------------------- @@ -705,6 +714,7 @@ class Engine final : public RuntimeDelegate { const Settings settings_; std::unique_ptr animator_; std::unique_ptr runtime_controller_; + std::unique_ptr pointer_data_dispatcher_; std::string initial_route_; ViewportMetrics viewport_metrics_; std::shared_ptr asset_manager_; diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index abfd14b56d41c..9ba950d83b57b 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/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,16 @@ void onBeginFrameMain() { }; } +@pragma('vm:entry-point') +void onPointerDataPacketMain() { + window.onPointerDataPacket = (PointerDataPacket packet) { + nativeOnPointerDataPacket(); + }; + window.onBeginFrame = (Duration beginTime) { + nativeOnBeginFrame(beginTime.inMicroseconds); + }; +} + @pragma('vm:entry-point') void emptyMain() {} diff --git a/shell/common/input_events_unittests.cc b/shell/common/input_events_unittests.cc new file mode 100644 index 0000000000000..efa4908976b35 --- /dev/null +++ b/shell/common/input_events_unittests.cc @@ -0,0 +1,236 @@ +// 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) { + ///// Begin constructing shell /////////////////////////////////////////////// + auto settings = fixture->CreateSettingsForFixture(); + std::unique_ptr shell = fixture->CreateShell(settings); + + 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)); + + auto nativeOnBeginFrame = [&will_draw_new_frame](Dart_NativeArguments args) { + will_draw_new_frame = true; + }; + fixture->AddNativeCallback("NativeOnBeginFrame", + CREATE_NATIVE_ENTRY(nativeOnBeginFrame)); + + ASSERT_TRUE(configuration.IsValid()); + fixture->RunEngine(shell.get(), std::move(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); + } + + // 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::PumpOneFrame(shell.get()); + } + + shell.reset(); +} + +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); +} + +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/shell/common/platform_view.cc b/shell/common/platform_view.cc index 4a84f27f4e456..7f4efd7fa5ba6 100644 --- a/shell/common/platform_view.cc +++ b/shell/common/platform_view.cc @@ -88,6 +88,13 @@ sk_sp PlatformView::CreateResourceContext() const { void PlatformView::ReleaseResourceContext() const {} +PointerDataDispatcherMaker PlatformView::GetDispatcherMaker() { + return [](Animator& animator, RuntimeController& controller, + TaskRunners task_runners) { + return std::make_unique(animator, controller); + }; +} + fml::WeakPtr PlatformView::GetWeakPtr() const { return weak_factory_.GetWeakPtr(); } diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index 414399150511b..9bbc0a6baf935 100644 --- a/shell/common/platform_view.h +++ b/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 + /// `Engien` 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/shell/common/pointer_data_dispatcher.cc b/shell/common/pointer_data_dispatcher.cc new file mode 100644 index 0000000000000..e3c41aad485d4 --- /dev/null +++ b/shell/common/pointer_data_dispatcher.cc @@ -0,0 +1,75 @@ +// 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() = default; + +void DefaultPointerDataDispatcher::DispatchPacket( + std::unique_ptr packet, + uint64_t trace_flow_id) { + animator_.EnqueueTraceFlowId(trace_flow_id); + runtime_controller_.DispatchPointerDataPacket(*packet); +} + +// Intentional no-op. +void DefaultPointerDataDispatcher::OnFrameLayerTreeReceived() {} + +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; +} + +void SmoothPointerDataDispatcher::OnFrameLayerTreeReceived() { + if (is_pointer_data_in_progress_) { + if (pending_packet_ != nullptr) { + // This is already in the UI thread. However, method + // `OnFrameLayerTreeReceived` is called by `Engine::Render` (a part of the + // `VSYNC` UI thread task) which is in Flutter framework's + // `SchedulerPhase.persistentCallbacks` phase. In that phase, no pointer + // data packet is allowed to be fired because the framework requires such + // phase to be executed synchronously without being interrupted. Hence + // we'll post a new UI thread task to fire the packet after `VSYNC` task + // is done. When a non-VSYNC UI thread task (like the following one) is + // run, the Flutter framework is always in `SchedulerPhase.idle` phase). + task_runners_.GetUITaskRunner()->PostTask( + // Use and validate a `fml::WeakPtr` because this dispatcher might + // have been destructed with engine when the task is run. + [dispatcher = weak_factory_.GetWeakPtr()]() { + if (dispatcher) { + dispatcher->DispatchPendingPacket(); + } + }); + } else { + 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; +} + +} // namespace flutter diff --git a/shell/common/pointer_data_dispatcher.h b/shell/common/pointer_data_dispatcher.h new file mode 100644 index 0000000000000..3a2afef094554 --- /dev/null +++ b/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; + +//-------------------------------------------------------------------------- +/// @brief Signature for constructing PointerDataDispatcher. +/// +/// @param[in] animator the animator of `Flutter::Engine` +/// @param[in] controller the runtime controller of `Flutter::Engine` +/// @param[in] task_runners the task runners of `Flutter::Engine` +/// +using PointerDataDispatcherMaker = + std::function( + Animator& animator, + RuntimeController& runtime_controller, + TaskRunners task_runners)>; + +//------------------------------------------------------------------------------ +/// 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 send it to the UI thread for the final +/// construction of the `PointerDataDispatcher`. +class PointerDataDispatcher { + public: + //---------------------------------------------------------------------------- + /// @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 Signal that the engine has received a frame layer tree for + /// rendering. This signal is supposed to be regularly delivered + /// at the same frequency of VSYNC. Such frame layer tree is + /// usually a result of a previous packet sent to the Flutter + /// framework. + virtual void OnFrameLayerTreeReceived() = 0; + + //---------------------------------------------------------------------------- + /// @brief Default destructor. + virtual ~PointerDataDispatcher(); +}; + +//------------------------------------------------------------------------------ +/// The default dispatcher that forwards the packet without any modification. +/// +class DefaultPointerDataDispatcher : public PointerDataDispatcher { + public: + DefaultPointerDataDispatcher(Animator& animator, + RuntimeController& runtime_controller) + : runtime_controller_(runtime_controller), animator_(animator) {} + + // |PointerDataDispatcer| + void DispatchPacket(std::unique_ptr packet, + uint64_t trace_flow_id) override; + + // |PointerDataDispatcer| + void OnFrameLayerTreeReceived() override; + + virtual ~DefaultPointerDataDispatcher(); + + protected: + RuntimeController& runtime_controller_; + Animator& animator_; + + FML_DISALLOW_COPY_AND_ASSIGN(DefaultPointerDataDispatcher); +}; + +//------------------------------------------------------------------------------ +/// The dispatcher that filters out irregular input events delivery to provide +/// a smooth scroll on iPhone X/Xs. +/// +/// This fixes https://github.com/flutter/flutter/issues/31086. +/// +/// 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 `OnFrameLayerTreeReceived`. 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(Animator& animator, + RuntimeController& runtime_controller, + TaskRunners task_runners) + : DefaultPointerDataDispatcher(animator, runtime_controller), + task_runners_(task_runners), + weak_factory_(this) {} + + // |PointerDataDispatcer| + void DispatchPacket(std::unique_ptr packet, + uint64_t trace_flow_id) override; + + // |PointerDataDispatcer| + void OnFrameLayerTreeReceived() override; + + virtual ~SmoothPointerDataDispatcher(); + + private: + TaskRunners task_runners_; + + // 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(); + + FML_DISALLOW_COPY_AND_ASSIGN(SmoothPointerDataDispatcher); +}; + +} // namespace flutter + +#endif // POINTER_DATA_DISPATCHER_H_ diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 30feb7746c02d..7631ac7d8a4f8 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -102,6 +102,10 @@ std::unique_ptr Shell::CreateShellOnPlatformThread( gpu_latch.Wait(); + // 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. fml::AutoResetWaitableEvent ui_latch; std::unique_ptr engine; @@ -110,6 +114,7 @@ std::unique_ptr Shell::CreateShellOnPlatformThread( fml::MakeCopyable([&ui_latch, // &engine, // shell = shell.get(), // + &dispatcher_maker, // isolate_snapshot = std::move(isolate_snapshot), // shared_snapshot = std::move(shared_snapshot), // vsync_waiter = std::move(vsync_waiter), // @@ -124,6 +129,7 @@ std::unique_ptr Shell::CreateShellOnPlatformThread( std::move(vsync_waiter)); engine = std::make_unique(*shell, // + dispatcher_maker, // *shell->GetDartVM(), // std::move(isolate_snapshot), // std::move(shared_snapshot), // @@ -724,11 +730,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/shell/common/shell.h b/shell/common/shell.h index f8f0c67fee2e5..f33558562bd0d 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -29,6 +29,7 @@ #include "flutter/shell/common/animator.h" #include "flutter/shell/common/engine.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/shell_io_manager.h" #include "flutter/shell/common/surface.h" diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index 9230b83133439..3c4dcefe8aced 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -132,6 +132,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(); } @@ -221,6 +231,15 @@ std::unique_ptr ShellTestPlatformView::CreateRenderingSurface() { return std::make_unique(this, true); } +// |PlatformView| +PointerDataDispatcherMaker ShellTestPlatformView::GetDispatcherMaker() { + return [](Animator& animator, RuntimeController& controller, + TaskRunners task_runners) { + return std::make_unique(animator, controller, + task_runners); + }; +} + // |GPUSurfaceGLDelegate| bool ShellTestPlatformView::GLContextMakeCurrent() { return gl_surface_.MakeCurrent(); diff --git a/shell/common/shell_test.h b/shell/common/shell_test.h index 55436a82a29ed..b3ce96f117017 100644 --- a/shell/common/shell_test.h +++ b/shell/common/shell_test.h @@ -43,6 +43,7 @@ class ShellTest : public ThreadTest { static void RunEngine(Shell* shell, RunConfiguration configuration); static void PumpOneFrame(Shell* shell); + static void DispatchFakePointerData(Shell* shell); // Declare |UnreportedTimingsCount|, |GetNeedsReportTimings| and // |SetNeedsReportTimings| inside |ShellTest| mainly for easier friend class @@ -84,6 +85,9 @@ class ShellTestPlatformView : public PlatformView, public GPUSurfaceGLDelegate { // |PlatformView| std::unique_ptr CreateRenderingSurface() override; + // |PlatformView| + PointerDataDispatcherMaker GetDispatcherMaker() override; + // |GPUSurfaceGLDelegate| bool GLContextMakeCurrent() override; diff --git a/shell/platform/darwin/ios/platform_view_ios.h b/shell/platform/darwin/ios/platform_view_ios.h index 38fc2a9f30b28..dbd93bb3dde39 100644 --- a/shell/platform/darwin/ios/platform_view_ios.h +++ b/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/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index c80ae377c03a1..acc7de58f5fb6 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -66,6 +66,12 @@ new AccessibilityBridge(static_cast(owner_controller_.get().view), } } +PointerDataDispatcherMaker PlatformViewIOS::GetDispatcherMaker() { + return [](Animator& animator, RuntimeController& controller, TaskRunners task_runners) { + return std::make_unique(animator, controller, task_runners); + }; +} + void PlatformViewIOS::RegisterExternalTexture(int64_t texture_id, NSObject* texture) { RegisterTexture(std::make_shared(texture_id, texture));