diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index a7484675f1278..40c9989d69c1d 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -486,7 +486,6 @@ 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,8 +495,6 @@ 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 b3745aef2f262..34ce4a61bc31d 100644 --- a/shell/common/BUILD.gn +++ b/shell/common/BUILD.gn @@ -74,8 +74,6 @@ 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", @@ -158,7 +156,6 @@ 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 18dd07cc82751..fa3fb65c8172d 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -36,7 +36,6 @@ 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, @@ -52,7 +51,6 @@ 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 @@ -62,7 +60,7 @@ Engine::Engine(Delegate& delegate, &vm, // VM std::move(isolate_snapshot), // isolate snapshot std::move(shared_snapshot), // shared snapshot - task_runners_, // task runners + std::move(task_runners), // task runners std::move(io_manager), // io manager image_decoder_.GetWeakPtr(), // image decoder settings_.advisory_script_uri, // advisory script uri @@ -71,8 +69,6 @@ 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; @@ -385,12 +381,12 @@ void Engine::HandleSettingsPlatformMessage(PlatformMessage* message) { } } -void Engine::DispatchPointerDataPacket( - std::unique_ptr packet, - uint64_t trace_flow_id) { +void Engine::DispatchPointerDataPacket(const PointerDataPacket& packet, + uint64_t trace_flow_id) { TRACE_EVENT0("flutter", "Engine::DispatchPointerDataPacket"); TRACE_FLOW_STEP("flutter", "PointerEvent", trace_flow_id); - pointer_data_dispatcher_->DispatchPacket(std::move(packet), trace_flow_id); + animator_->EnqueueTraceFlowId(trace_flow_id); + runtime_controller_->DispatchPointerDataPacket(packet); } void Engine::DispatchSemanticsAction(int id, @@ -438,8 +434,6 @@ 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, @@ -468,14 +462,6 @@ 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::HandleAssetPlatformMessage(fml::RefPtr message) { fml::RefPtr response = message->response(); if (!response) { diff --git a/shell/common/engine.h b/shell/common/engine.h index cc874c237b7f3..e2fa9adf323a3 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -22,8 +22,6 @@ #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" @@ -67,7 +65,7 @@ namespace flutter { /// name and it does happen to be one of the older classes in the /// repository. /// -class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { +class Engine final : public RuntimeDelegate { public: //---------------------------------------------------------------------------- /// @brief Indicates the result of the call to `Engine::Run`. @@ -236,12 +234,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// 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 @@ -273,7 +265,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// GPU. /// Engine(Delegate& delegate, - PointerDataDispatcherMaker& dispatcher_maker, DartVM& vm, fml::RefPtr isolate_snapshot, fml::RefPtr shared_snapshot, @@ -658,7 +649,7 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// timeline and allow grouping frames and input /// events into logical chunks. /// - void DispatchPointerDataPacket(std::unique_ptr packet, + void DispatchPointerDataPacket(const PointerDataPacket& packet, uint64_t trace_flow_id); //---------------------------------------------------------------------------- @@ -709,23 +700,11 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { // |RuntimeDelegate| FontCollection& GetFontCollection() override; - // |PointerDataDispatcher::Delegate| - void DoDispatchPacket(std::unique_ptr packet, - uint64_t trace_flow_id) override; - - TaskRunners& task_runners() override { return task_runners_; } - 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_; @@ -733,7 +712,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { bool have_surface_; FontCollection font_collection_; ImageDecoder image_decoder_; - TaskRunners task_runners_; fml::WeakPtrFactory weak_factory_; // |RuntimeDelegate| diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index 9ba950d83b57b..abfd14b56d41c 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -11,7 +11,6 @@ void main() {} void nativeReportTimingsCallback(List timings) native 'NativeReportTimingsCallback'; void nativeOnBeginFrame(int microseconds) native 'NativeOnBeginFrame'; -void nativeOnPointerDataPacket() native 'NativeOnPointerDataPacket'; @pragma('vm:entry-point') void reportTimingsMain() { @@ -33,16 +32,6 @@ 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 deleted file mode 100644 index 361c237ef3dfa..0000000000000 --- a/shell/common/input_events_unittests.cc +++ /dev/null @@ -1,250 +0,0 @@ -// 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); - - 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)); - - 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); - } - - // 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); - - // 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/shell/common/platform_view.cc b/shell/common/platform_view.cc index 058ad55d0c801..4a84f27f4e456 100644 --- a/shell/common/platform_view.cc +++ b/shell/common/platform_view.cc @@ -88,12 +88,6 @@ 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/shell/common/platform_view.h b/shell/common/platform_view.h index 9bbc0a6baf935..414399150511b 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -16,7 +16,6 @@ #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" @@ -419,23 +418,17 @@ 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. /// @@ -453,14 +446,13 @@ 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. @@ -473,8 +465,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. /// @@ -483,17 +475,16 @@ 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 /// @@ -503,11 +494,10 @@ 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. /// @@ -524,14 +514,13 @@ 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 /// @@ -547,8 +536,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 deleted file mode 100644 index 0e1101b94cf4a..0000000000000 --- a/shell/common/pointer_data_dispatcher.cc +++ /dev/null @@ -1,74 +0,0 @@ -// 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) { - delegate_.DoDispatchPacket(std::move(packet), trace_flow_id); -} - -// 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). - delegate_.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 deleted file mode 100644 index 86f865d7f5460..0000000000000 --- a/shell/common/pointer_data_dispatcher.h +++ /dev/null @@ -1,174 +0,0 @@ -// 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 send 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; - - /// Get the task runners to schedule tasks on specific threads. - virtual TaskRunners& task_runners() = 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 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(Delegate& delegate) : delegate_(delegate) {} - - // |PointerDataDispatcer| - void DispatchPacket(std::unique_ptr packet, - uint64_t trace_flow_id) override; - - // |PointerDataDispatcer| - void OnFrameLayerTreeReceived() override; - - virtual ~DefaultPointerDataDispatcher(); - - protected: - Delegate& delegate_; - - 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(Delegate& delegate) - : DefaultPointerDataDispatcher(delegate), weak_factory_(this) {} - - // |PointerDataDispatcer| - void DispatchPacket(std::unique_ptr packet, - uint64_t trace_flow_id) override; - - // |PointerDataDispatcer| - void OnFrameLayerTreeReceived() 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(); - - 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/shell/common/shell.cc b/shell/common/shell.cc index 804dcb2b722a7..93c6c1042286d 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -98,10 +98,6 @@ 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(); @@ -109,7 +105,6 @@ 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), // @@ -125,7 +120,6 @@ 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 +715,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 = weak_engine_, packet = std::move(packet), - flow_id = next_pointer_flow_id_]() mutable { + task_runners_.GetUITaskRunner()->PostTask(fml::MakeCopyable( + [engine = engine_->GetWeakPtr(), packet = std::move(packet), + flow_id = next_pointer_flow_id_] { if (engine) { - engine->DispatchPointerDataPacket(std::move(packet), flow_id); + engine->DispatchPointerDataPacket(*packet, flow_id); } })); next_pointer_flow_id_++; diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index 3ede4058be9a6..e541d908c5586 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -101,18 +101,6 @@ void ShellTest::RunEngine(Shell* shell, RunConfiguration configuration) { latch.Wait(); } -void ShellTest::RestartEngine(Shell* shell, RunConfiguration configuration) { - fml::AutoResetWaitableEvent latch; - fml::TaskRunner::RunNowOrPostTask( - shell->GetTaskRunners().GetUITaskRunner(), - [shell, &latch, &configuration]() { - bool restarted = shell->engine_->Restart(std::move(configuration)); - ASSERT_TRUE(restarted); - 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 @@ -144,16 +132,6 @@ 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(); } @@ -244,13 +222,6 @@ 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/shell/common/shell_test.h b/shell/common/shell_test.h index ae6c92560fa09..55436a82a29ed 100644 --- a/shell/common/shell_test.h +++ b/shell/common/shell_test.h @@ -41,10 +41,8 @@ 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); static void PumpOneFrame(Shell* shell); - static void DispatchFakePointerData(Shell* shell); // Declare |UnreportedTimingsCount|, |GetNeedsReportTimings| and // |SetNeedsReportTimings| inside |ShellTest| mainly for easier friend class @@ -86,9 +84,6 @@ 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 51d00e86370b1..eeaca3dd9df50 100644 --- a/shell/platform/darwin/ios/platform_view_ios.h +++ b/shell/platform/darwin/ios/platform_view_ios.h @@ -37,9 +37,6 @@ 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 ce3f25d6ca8db..778823493977a 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -98,12 +98,6 @@ new AccessibilityBridge(static_cast(owner_controller_.get().view), } } -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));