From e02e209f6ddc3633c92b3125c8565c557ae5e8f1 Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Thu, 25 Mar 2021 18:15:33 -0400 Subject: [PATCH 01/16] re-implement VsyncWaiter - all tests and rebased --- ci/licenses_golden/licenses_flutter | 2 +- shell/common/animator.cc | 1 + shell/platform/fuchsia/flutter/BUILD.gn | 3 +- .../flutter/default_session_connection.cc | 41 +-- .../flutter/default_session_connection.h | 37 +- shell/platform/fuchsia/flutter/engine.cc | 23 +- shell/platform/fuchsia/flutter/engine.h | 2 +- .../platform/fuchsia/flutter/platform_view.cc | 6 +- .../platform/fuchsia/flutter/platform_view.h | 4 +- .../fuchsia/flutter/platform_view_unittest.cc | 3 +- .../fuchsia/flutter/session_connection.h | 24 ++ .../default_session_connection_unittests.cc | 6 +- .../flutter/tests/fake_session_connection.h | 36 ++ .../flutter/tests/vsync_waiter_unittests.cc | 340 ++++++++++++++++++ .../platform/fuchsia/flutter/vsync_recorder.h | 2 +- .../platform/fuchsia/flutter/vsync_waiter.cc | 283 +++++++++------ shell/platform/fuchsia/flutter/vsync_waiter.h | 59 ++- .../fuchsia/flutter/vsync_waiter_unittests.cc | 137 ------- 18 files changed, 685 insertions(+), 324 deletions(-) create mode 100644 shell/platform/fuchsia/flutter/session_connection.h create mode 100644 shell/platform/fuchsia/flutter/tests/fake_session_connection.h create mode 100644 shell/platform/fuchsia/flutter/tests/vsync_waiter_unittests.cc delete mode 100644 shell/platform/fuchsia/flutter/vsync_waiter_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index ef9a420aae284..9fc5d26858e5e 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -1360,6 +1360,7 @@ FILE: ../../../flutter/shell/platform/fuchsia/flutter/runner.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/runner.h FILE: ../../../flutter/shell/platform/fuchsia/flutter/runner_tzdata_unittest.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/runner_unittest.cc +FILE: ../../../flutter/shell/platform/fuchsia/flutter/session_connection.h FILE: ../../../flutter/shell/platform/fuchsia/flutter/surface.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/surface.h FILE: ../../../flutter/shell/platform/fuchsia/flutter/task_observers.cc @@ -1373,7 +1374,6 @@ FILE: ../../../flutter/shell/platform/fuchsia/flutter/vsync_recorder.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/vsync_recorder.h FILE: ../../../flutter/shell/platform/fuchsia/flutter/vsync_waiter.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/vsync_waiter.h -FILE: ../../../flutter/shell/platform/fuchsia/flutter/vsync_waiter_unittests.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/vulkan_surface.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/vulkan_surface.h FILE: ../../../flutter/shell/platform/fuchsia/flutter/vulkan_surface_pool.cc diff --git a/shell/common/animator.cc b/shell/common/animator.cc index 2fbd6fa772f1e..41b5a44e60f7f 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -128,6 +128,7 @@ void Animator::BeginFrame( // If we still don't have valid continuation, the pipeline is currently // full because the consumer is being too slow. Try again at the next // frame interval. + TRACE_EVENT0("flutter", "PipelineFull"); RequestFrame(); return; } diff --git a/shell/platform/fuchsia/flutter/BUILD.gn b/shell/platform/fuchsia/flutter/BUILD.gn index 3e03505bf1ecf..9b9feba36e013 100644 --- a/shell/platform/fuchsia/flutter/BUILD.gn +++ b/shell/platform/fuchsia/flutter/BUILD.gn @@ -454,9 +454,10 @@ executable("flutter_runner_unittests") { "platform_view_unittest.cc", "runner_unittest.cc", "tests/engine_unittests.cc", + "tests/fake_session_connection.h", "tests/flutter_runner_product_configuration_unittests.cc", "tests/vsync_recorder_unittests.cc", - "vsync_waiter_unittests.cc", + "tests/vsync_waiter_unittests.cc", ] # This is needed for //third_party/googletest for linking zircon symbols. diff --git a/shell/platform/fuchsia/flutter/default_session_connection.cc b/shell/platform/fuchsia/flutter/default_session_connection.cc index 1b72d7a990567..3afa3ebd7780e 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.cc +++ b/shell/platform/fuchsia/flutter/default_session_connection.cc @@ -17,11 +17,9 @@ DefaultSessionConnection::DefaultSessionConnection( fidl::InterfaceHandle session, fml::closure session_error_callback, on_frame_presented_event on_frame_presented_callback, - zx_handle_t vsync_event_handle, uint64_t max_frames_in_flight) : session_wrapper_(session.Bind(), nullptr), on_frame_presented_callback_(std::move(on_frame_presented_callback)), - vsync_event_handle_(vsync_event_handle), kMaxFramesInFlight(max_frames_in_flight) { session_wrapper_.set_error_handler( [callback = session_error_callback](zx_status_t status) { callback(); }); @@ -29,8 +27,7 @@ DefaultSessionConnection::DefaultSessionConnection( // Set the |fuchsia::ui::scenic::OnFramePresented()| event handler that will // fire every time a set of one or more frames is presented. session_wrapper_.set_on_frame_presented_handler( - [this, handle = vsync_event_handle_]( - fuchsia::scenic::scheduling::FramePresentedInfo info) { + [this](fuchsia::scenic::scheduling::FramePresentedInfo info) { // Update Scenic's limit for our remaining frames in flight allowed. size_t num_presents_handled = info.presentation_infos.size(); frames_in_flight_allowed_ = info.num_presents_allowed; @@ -38,9 +35,10 @@ DefaultSessionConnection::DefaultSessionConnection( // A frame was presented: Update our |frames_in_flight| to match the // updated unfinalized present requests. frames_in_flight_ -= num_presents_handled; - TRACE_EVENT2("gfx", "DSC::OnFramePresented", "frames_in_flight", - frames_in_flight_, "max_frames_in_flight", - kMaxFramesInFlight); + TRACE_DURATION("gfx", "OnFramePresented", "frames_in_flight", + frames_in_flight_, "max_frames_in_flight", + kMaxFramesInFlight, "num_presents_handled", + num_presents_handled); FML_DCHECK(frames_in_flight_ >= 0); VsyncRecorder::GetInstance().UpdateFramePresentedInfo( @@ -52,7 +50,10 @@ DefaultSessionConnection::DefaultSessionConnection( if (present_session_pending_) { PresentSession(); } - ToggleSignal(handle, true); + + if (vsync_waiter_initialized_) { + vsync_waiter_callback_(); + } } // callback ); @@ -70,8 +71,6 @@ DefaultSessionConnection::DefaultSessionConnection( VsyncRecorder::GetInstance().UpdateNextPresentationInfo( std::move(info)); - // Signal is initially high indicating availability of the session. - ToggleSignal(vsync_event_handle_, true); initialized_ = true; PresentSession(); @@ -81,8 +80,8 @@ DefaultSessionConnection::DefaultSessionConnection( DefaultSessionConnection::~DefaultSessionConnection() = default; void DefaultSessionConnection::Present() { - TRACE_EVENT2("gfx", "DefaultSessionConnection::Present", "frames_in_flight", - frames_in_flight_, "max_frames_in_flight", kMaxFramesInFlight); + TRACE_DURATION("gfx", "DefaultSessionConnection::Present", "frames_in_flight", + frames_in_flight_, "max_frames_in_flight", kMaxFramesInFlight); TRACE_FLOW_BEGIN("gfx", "DefaultSessionConnection::PresentSession", next_present_session_trace_id_); @@ -101,7 +100,6 @@ void DefaultSessionConnection::Present() { FML_CHECK(frames_in_flight_ <= kMaxFramesInFlight); present_session_pending_ = true; - ToggleSignal(vsync_event_handle_, false); } } @@ -135,7 +133,7 @@ fml::TimePoint DefaultSessionConnection::CalculateNextLatchPoint( } void DefaultSessionConnection::PresentSession() { - TRACE_EVENT0("gfx", "DefaultSessionConnection::PresentSession"); + TRACE_DURATION("gfx", "DefaultSessionConnection::PresentSession"); // If we cannot call Present2() because we have no more Scenic frame budget, // then we must wait until the OnFramePresented() event fires so we can @@ -197,15 +195,12 @@ void DefaultSessionConnection::PresentSession() { }); } -void DefaultSessionConnection::ToggleSignal(zx_handle_t handle, bool set) { - const auto signal = VsyncWaiter::SessionPresentSignal; - auto status = zx_object_signal(handle, // handle - set ? 0 : signal, // clear mask - set ? signal : 0 // set mask - ); - if (status != ZX_OK) { - FML_LOG(ERROR) << "Could not toggle vsync signal: " << set; - } +void DefaultSessionConnection::InitializeVsyncWaiterCallback( + VsyncWaiterCallback callback) { + FML_DCHECK(!vsync_waiter_initialized_); + + vsync_waiter_callback_ = callback; + vsync_waiter_initialized_ = true; } } // namespace flutter_runner diff --git a/shell/platform/fuchsia/flutter/default_session_connection.h b/shell/platform/fuchsia/flutter/default_session_connection.h index 97ca5637e4084..3debfd9873d3d 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.h +++ b/shell/platform/fuchsia/flutter/default_session_connection.h @@ -14,6 +14,8 @@ #include "flutter/fml/closure.h" #include "flutter/fml/macros.h" +#include "session_connection.h" + namespace flutter_runner { using on_frame_presented_event = @@ -21,29 +23,41 @@ using on_frame_presented_event = // The component residing on the raster thread that is responsible for // maintaining the Scenic session connection and presenting node updates. -class DefaultSessionConnection final : public flutter::SessionWrapper { +class DefaultSessionConnection final + : public flutter::SessionWrapper, + public flutter_runner::SessionConnection { public: + static fml::TimePoint CalculateNextLatchPoint( + fml::TimePoint present_requested_time, + fml::TimePoint now, + fml::TimePoint last_latch_point_targeted, + fml::TimeDelta flutter_frame_build_time, + fml::TimeDelta vsync_interval, + std::deque>& + future_presentation_infos); + DefaultSessionConnection( std::string debug_label, fidl::InterfaceHandle session, fml::closure session_error_callback, on_frame_presented_event on_frame_presented_callback, - zx_handle_t vsync_event_handle, uint64_t max_frames_in_flight); ~DefaultSessionConnection(); + // |SessionWrapper| scenic::Session* get() override { return &session_wrapper_; } + + // |SessionWrapper| void Present() override; - static fml::TimePoint CalculateNextLatchPoint( - fml::TimePoint present_requested_time, - fml::TimePoint now, - fml::TimePoint last_latch_point_targeted, - fml::TimeDelta flutter_frame_build_time, - fml::TimeDelta vsync_interval, - std::deque>& - future_presentation_infos); + // |SessionConnection| + void InitializeVsyncWaiterCallback(VsyncWaiterCallback callback) override; + + // |SessionConnection| + bool CanRequestNewFrames() override { + return frames_in_flight_ < kMaxFramesInFlight; + } private: scenic::Session session_wrapper_; @@ -80,6 +94,9 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { bool present_session_pending_ = false; + bool vsync_waiter_initialized_ = false; + VsyncWaiterCallback vsync_waiter_callback_; + void PresentSession(); static void ToggleSignal(zx_handle_t handle, bool raise); diff --git a/shell/platform/fuchsia/flutter/engine.cc b/shell/platform/fuchsia/flutter/engine.cc index 62751e336834c..001d96328dfd6 100644 --- a/shell/platform/fuchsia/flutter/engine.cc +++ b/shell/platform/fuchsia/flutter/engine.cc @@ -76,10 +76,6 @@ Engine::Engine(Delegate& delegate, #endif intercept_all_input_(product_config.get_intercept_all_input()), weak_factory_(this) { - if (zx::event::create(0, &vsync_event_) != ZX_OK) { - FML_DLOG(ERROR) << "Could not create the vsync event."; - return; - } // Get the task runners from the managed threads. The current thread will be // used as the "platform" thread. @@ -138,10 +134,10 @@ Engine::Engine(Delegate& delegate, view_token = std::move(view_token), view_ref_pair = std::move(view_ref_pair), max_frames_in_flight = product_config.get_max_frames_in_flight(), - vsync_handle = vsync_event_.get(), &view_embedder_latch]() mutable { - session_connection_.emplace( + &view_embedder_latch]() mutable { + session_connection_ = std::make_shared( thread_label_, std::move(session), - std::move(session_error_callback), [](auto) {}, vsync_handle, + std::move(session_error_callback), [](auto) {}, max_frames_in_flight); surface_producer_.emplace(session_connection_->get()); #if defined(LEGACY_FUCHSIA_EMBEDDER) @@ -149,7 +145,7 @@ Engine::Engine(Delegate& delegate, legacy_external_view_embedder_ = std::make_shared( thread_label_, std::move(view_token), - std::move(view_ref_pair), session_connection_.value(), + std::move(view_ref_pair), *(session_connection_.get()), intercept_all_input_); } else #endif @@ -157,7 +153,7 @@ Engine::Engine(Delegate& delegate, external_view_embedder_ = std::make_shared( thread_label_, std::move(view_token), - std::move(view_ref_pair), session_connection_.value(), + std::move(view_ref_pair), *session_connection_.get(), surface_producer_.value(), intercept_all_input_); } view_embedder_latch.Signal(); @@ -232,8 +228,8 @@ Engine::Engine(Delegate& delegate, // Setup the callback that will instantiate the platform view. flutter::Shell::CreateCallback on_create_platform_view = fml::MakeCopyable( - [debug_label = thread_label_, view_ref = std::move(platform_view_ref), - runner_services, + [this, debug_label = thread_label_, + view_ref = std::move(platform_view_ref), runner_services, parent_environment_service_provider = std::move(parent_environment_service_provider), session_listener_request = std::move(session_listener_request), @@ -248,7 +244,6 @@ Engine::Engine(Delegate& delegate, on_create_surface_callback = std::move(on_create_surface_callback), external_view_embedder = GetExternalViewEmbedder(), vsync_offset = product_config.get_vsync_offset(), - vsync_handle = vsync_event_.get(), keyboard_listener_request = std::move(keyboard_listener_request)]( flutter::Shell& shell) mutable { return std::make_unique( @@ -271,7 +266,7 @@ Engine::Engine(Delegate& delegate, std::move(on_create_surface_callback), external_view_embedder, // external view embedder std::move(vsync_offset), // vsync offset - vsync_handle); + session_connection_); }); // Setup the callback that will instantiate the rasterizer. @@ -295,7 +290,7 @@ Engine::Engine(Delegate& delegate, auto compositor_context = std::make_unique( - session_connection_.value(), surface_producer_.value(), + *(session_connection_.get()), surface_producer_.value(), legacy_external_view_embedder_); return std::make_unique( shell, std::move(compositor_context)); diff --git a/shell/platform/fuchsia/flutter/engine.h b/shell/platform/fuchsia/flutter/engine.h index 73b47074179bd..f6303274fe078 100644 --- a/shell/platform/fuchsia/flutter/engine.h +++ b/shell/platform/fuchsia/flutter/engine.h @@ -74,7 +74,7 @@ class Engine final { const std::string thread_label_; std::array threads_; - std::optional session_connection_; + std::shared_ptr session_connection_; std::optional surface_producer_; std::shared_ptr external_view_embedder_; #if defined(LEGACY_FUCHSIA_EMBEDDER) diff --git a/shell/platform/fuchsia/flutter/platform_view.cc b/shell/platform/fuchsia/flutter/platform_view.cc index 5061240162b85..190ed9b5a9d3b 100644 --- a/shell/platform/fuchsia/flutter/platform_view.cc +++ b/shell/platform/fuchsia/flutter/platform_view.cc @@ -74,7 +74,7 @@ PlatformView::PlatformView( OnCreateSurface on_create_surface_callback, std::shared_ptr external_view_embedder, fml::TimeDelta vsync_offset, - zx_handle_t vsync_event_handle) + std::shared_ptr session_connection) : flutter::PlatformView(delegate, std::move(task_runners)), debug_label_(std::move(debug_label)), view_ref_(std::move(view_ref)), @@ -90,7 +90,7 @@ PlatformView::PlatformView( external_view_embedder_(external_view_embedder), ime_client_(this), vsync_offset_(std::move(vsync_offset)), - vsync_event_handle_(vsync_event_handle), + session_connection_(session_connection), keyboard_listener_binding_(this, std::move(keyboard_listener_request)), weak_factory_(this) { // Register all error handlers. @@ -692,7 +692,7 @@ void PlatformView::DeactivateIme() { // |flutter::PlatformView| std::unique_ptr PlatformView::CreateVSyncWaiter() { return std::make_unique( - debug_label_, vsync_event_handle_, task_runners_, vsync_offset_); + session_connection_, task_runners_, vsync_offset_); } // |flutter::PlatformView| diff --git a/shell/platform/fuchsia/flutter/platform_view.h b/shell/platform/fuchsia/flutter/platform_view.h index a549f1d6e48b6..118e82784b803 100644 --- a/shell/platform/fuchsia/flutter/platform_view.h +++ b/shell/platform/fuchsia/flutter/platform_view.h @@ -72,7 +72,7 @@ class PlatformView final : public flutter::PlatformView, OnCreateSurface on_create_surface_callback, std::shared_ptr view_embedder, fml::TimeDelta vsync_offset, - zx_handle_t vsync_event_handle); + std::shared_ptr session_connection); ~PlatformView(); @@ -218,7 +218,7 @@ class PlatformView final : public flutter::PlatformView, std::set unregistered_channels_; fml::TimeDelta vsync_offset_; - zx_handle_t vsync_event_handle_ = 0; + std::shared_ptr session_connection_; // The registered binding for serving the keyboard listener server endpoint. fidl::Binding diff --git a/shell/platform/fuchsia/flutter/platform_view_unittest.cc b/shell/platform/fuchsia/flutter/platform_view_unittest.cc index 63cdec25319a6..9fa6e88356637 100644 --- a/shell/platform/fuchsia/flutter/platform_view_unittest.cc +++ b/shell/platform/fuchsia/flutter/platform_view_unittest.cc @@ -269,7 +269,7 @@ class PlatformViewBuilder { std::move(on_update_view_callback_), std::move(on_destroy_view_callback_), std::move(on_create_surface_callback_), view_embedder_, - std::move(vsync_offset_), vsync_event_handle_); + std::move(vsync_offset_), nullptr); } private: @@ -300,7 +300,6 @@ class PlatformViewBuilder { OnCreateSurface on_create_surface_callback_{nullptr}; std::shared_ptr view_embedder_{nullptr}; fml::TimeDelta vsync_offset_{fml::TimeDelta::Zero()}; - zx_handle_t vsync_event_handle_{ZX_HANDLE_INVALID}; }; } // namespace diff --git a/shell/platform/fuchsia/flutter/session_connection.h b/shell/platform/fuchsia/flutter/session_connection.h new file mode 100644 index 0000000000000..cd105d5ef513d --- /dev/null +++ b/shell/platform/fuchsia/flutter/session_connection.h @@ -0,0 +1,24 @@ +// 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 FLUTTER_SHELL_PLATFORM_FUCHSIA_SESSION_CONNECTION_H_ +#define FLUTTER_SHELL_PLATFORM_FUCHSIA_SESSION_CONNECTION_H_ + +namespace flutter_runner { +using VsyncWaiterCallback = std::function; + +// The component residing on the raster thread that is responsible for +// maintaining the Scenic session connection and presenting node updates. +class SessionConnection { + public: + virtual ~SessionConnection() = default; + + virtual void InitializeVsyncWaiterCallback(VsyncWaiterCallback callback) = 0; + + virtual bool CanRequestNewFrames() = 0; +}; + +} // namespace flutter_runner + +#endif // FLUTTER_SHELL_PLATFORM_FUCHSIA_SESSION_CONNECTION_H_ diff --git a/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc b/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc index d684f0bdd86c0..e5b078b1ddfb8 100644 --- a/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc +++ b/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc @@ -30,7 +30,6 @@ class DefaultSessionConnectionTest : public ::testing::Test { auto session_listener_request = session_listener_.NewRequest(); scenic_->CreateSession(session_.NewRequest(), session_listener_.Bind()); - FML_CHECK(zx::event::create(0, &vsync_event_) == ZX_OK); // Ensure Scenic has had time to wake up before the test logic begins. // TODO(61768) Find a better solution than sleeping periodically checking a @@ -54,7 +53,6 @@ class DefaultSessionConnectionTest : public ::testing::Test { fidl::InterfaceHandle session_; fidl::InterfaceHandle session_listener_; - zx::event vsync_event_; thrd_t fidl_thread_; }; @@ -70,7 +68,7 @@ TEST_F(DefaultSessionConnectionTest, SimplePresentTest) { flutter_runner::DefaultSessionConnection session_connection( "debug label", std::move(session_), on_session_error_callback, - on_frame_presented_callback, vsync_event_.get(), 3); + on_frame_presented_callback, 3); for (int i = 0; i < 200; ++i) { session_connection.Present(); @@ -92,7 +90,7 @@ TEST_F(DefaultSessionConnectionTest, BatchedPresentTest) { flutter_runner::DefaultSessionConnection session_connection( "debug label", std::move(session_), on_session_error_callback, - on_frame_presented_callback, vsync_event_.get(), 3); + on_frame_presented_callback, 3); for (int i = 0; i < 200; ++i) { session_connection.Present(); diff --git a/shell/platform/fuchsia/flutter/tests/fake_session_connection.h b/shell/platform/fuchsia/flutter/tests/fake_session_connection.h new file mode 100644 index 0000000000000..8854ca700e76f --- /dev/null +++ b/shell/platform/fuchsia/flutter/tests/fake_session_connection.h @@ -0,0 +1,36 @@ +// 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 FLUTTER_SHELL_PLATFORM_FUCHSIA_TESTS_FAKE_SESSION_CONNECTION_H_ +#define FLUTTER_SHELL_PLATFORM_FUCHSIA_TESTS_FAKE_SESSION_CONNECTION_H_ + +namespace flutter_runner_test { + +#include "flutter/shell/platform/fuchsia/flutter/session_connection.h" + +class FakeSessionConnection : public flutter_runner::SessionConnection { + public: + FakeSessionConnection() {} + + void InitializeVsyncWaiterCallback( + flutter_runner::VsyncWaiterCallback callback) override { + callback_ = callback; + } + + bool CanRequestNewFrames() override { return can_request_new_frames_; } + + void FireVsyncWaiterCallback() { callback_(); } + + void SetCanRequestNewFrames(bool new_bool) { + can_request_new_frames_ = new_bool; + } + + private: + bool can_request_new_frames_ = true; + flutter_runner::VsyncWaiterCallback callback_; +}; + +} // namespace flutter_runner_test + +#endif // FLUTTER_SHELL_PLATFORM_FUCHSIA_TESTS_FAKE_SESSION_CONNECTION_H_ diff --git a/shell/platform/fuchsia/flutter/tests/vsync_waiter_unittests.cc b/shell/platform/fuchsia/flutter/tests/vsync_waiter_unittests.cc new file mode 100644 index 0000000000000..e474246c25c16 --- /dev/null +++ b/shell/platform/fuchsia/flutter/tests/vsync_waiter_unittests.cc @@ -0,0 +1,340 @@ +// 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 +#include +#include +#include + +#include + +#include "flutter/fml/time/time_delta.h" +#include "flutter/fml/time/time_point.h" +#include "flutter/shell/common/thread_host.h" +#include "flutter/shell/common/vsync_waiter.h" +#include "flutter/shell/platform/fuchsia/flutter/flutter_runner_product_configuration.h" +#include "flutter/shell/platform/fuchsia/flutter/task_runner_adapter.h" +#include "flutter/shell/platform/fuchsia/flutter/thread.h" +#include "flutter/shell/platform/fuchsia/flutter/vsync_waiter.h" + +#include "fake_session_connection.h" + +namespace flutter_runner_test { + +static fml::TimePoint TimePointFromInt(int i) { + return fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromNanoseconds(i)); +} +static fml::TimeDelta TimeDeltaFromInt(int i) { + return fml::TimeDelta::FromNanoseconds(i); +} +static int TimePointToInt(fml::TimePoint time) { + return time.ToEpochDelta().ToNanoseconds(); +} + +class VsyncWaiterTest : public testing::Test { + public: + VsyncWaiterTest() { + for (auto& thread : threads_) { + thread.reset(new flutter_runner::Thread()); + } + + const flutter::TaskRunners task_runners( + "VsyncWaiterTests", // Dart thread labels + flutter_runner::CreateFMLTaskRunner( + async_get_default_dispatcher()), // platform + flutter_runner::CreateFMLTaskRunner( + threads_[0]->dispatcher()), // raster + flutter_runner::CreateFMLTaskRunner(threads_[1]->dispatcher()), // ui + flutter_runner::CreateFMLTaskRunner(threads_[2]->dispatcher()) // io + ); + + session_connection_ = std::make_shared(); + vsync_waiter_ = std::make_unique( + session_connection_, task_runners, fml::TimeDelta::Zero()); + } + + flutter_runner::VsyncWaiter* get_vsync_waiter() { + return vsync_waiter_.get(); + } + FakeSessionConnection* get_session_connection() { + return session_connection_.get(); + } + + ~VsyncWaiterTest() { + vsync_waiter_.reset(); + for (const auto& thread : threads_) { + thread->Quit(); + } + } + + private: + async::Loop loop_ = async::Loop(&kAsyncLoopConfigAttachToCurrentThread); + std::shared_ptr session_connection_; + std::unique_ptr vsync_waiter_; + std::array, 3> threads_; +}; + +TEST_F(VsyncWaiterTest, SimpleVsyncCallback) { + auto vsync_waiter = get_vsync_waiter(); + auto session_connection = get_session_connection(); + + bool flag = false; + vsync_waiter->AsyncWaitForVsync([&flag](auto) { flag = true; }); + + session_connection->FireVsyncWaiterCallback(); + + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + + EXPECT_TRUE(flag); +} + +TEST_F(VsyncWaiterTest, EnsureBackPressure) { + auto vsync_waiter = get_vsync_waiter(); + auto session_connection = get_session_connection(); + + session_connection->SetCanRequestNewFrames(false); + bool flag = false; + vsync_waiter->AsyncWaitForVsync([&flag](auto) { flag = true; }); + + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + + EXPECT_FALSE(flag); +} + +TEST_F(VsyncWaiterTest, BackPressureRelief) { + auto vsync_waiter = get_vsync_waiter(); + auto session_connection = get_session_connection(); + + session_connection->SetCanRequestNewFrames(false); + bool flag = false; + vsync_waiter->AsyncWaitForVsync([&flag](auto) { flag = true; }); + + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + + EXPECT_FALSE(flag); + + session_connection->SetCanRequestNewFrames(true); + session_connection->FireVsyncWaiterCallback(); + + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + + EXPECT_TRUE(flag); +} + +TEST_F(VsyncWaiterTest, SteadyStateBehavior) { + auto vsync_waiter = get_vsync_waiter(); + auto session_connection = get_session_connection(); + + session_connection->SetCanRequestNewFrames(true); + + int count = 0; + const int expected_iterations = 100; + + for (int i = 0; i < expected_iterations; ++i) { + vsync_waiter->AsyncWaitForVsync([&count](auto) { count++; }); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } + + EXPECT_EQ(count, expected_iterations); +} + +TEST(SnapToNextPhaseTest, SnapOverlapsWithNow) { + const auto now = fml::TimePoint::Now(); + const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(10); + const auto delta = fml::TimeDelta::FromNanoseconds(10); + const auto next_vsync = flutter_runner::VsyncWaiter::SnapToNextPhase( + now, last_presentation_time, delta); + + EXPECT_EQ(now + delta, next_vsync); +} + +TEST(SnapToNextPhaseTest, SnapAfterNow) { + const auto now = fml::TimePoint::Now(); + const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(9); + const auto delta = fml::TimeDelta::FromNanoseconds(10); + const auto next_vsync = flutter_runner::VsyncWaiter::SnapToNextPhase( + now, last_presentation_time, delta); + + // math here: 10 - 9 = 1 + EXPECT_EQ(now + fml::TimeDelta::FromNanoseconds(1), next_vsync); +} + +TEST(SnapToNextPhaseTest, SnapAfterNowMultiJump) { + const auto now = fml::TimePoint::Now(); + const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(34); + const auto delta = fml::TimeDelta::FromNanoseconds(10); + const auto next_vsync = flutter_runner::VsyncWaiter::SnapToNextPhase( + now, last_presentation_time, delta); + + // zeroes: -34, -24, -14, -4, 6, ... + EXPECT_EQ(now + fml::TimeDelta::FromNanoseconds(6), next_vsync); +} + +TEST(SnapToNextPhaseTest, SnapAfterNowMultiJumpAccountForCeils) { + const auto now = fml::TimePoint::Now(); + const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(20); + const auto delta = fml::TimeDelta::FromNanoseconds(16); + const auto next_vsync = flutter_runner::VsyncWaiter::SnapToNextPhase( + now, last_presentation_time, delta); + + // zeroes: -20, -4, 12, 28, ... + EXPECT_EQ(now + fml::TimeDelta::FromNanoseconds(12), next_vsync); +} + +TEST(GetTargetTimesTest, ScheduleForNextVsync) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + const fml::TimePoint last_targetted_vsync = TimePointFromInt(10); + const fml::TimePoint now = TimePointFromInt(9); + const fml::TimePoint next_vsync = TimePointFromInt(10); + + const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 10); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 20); +} + +TEST(GetTargetTimesTest, ScheduleForCurrentVsync_DueToOffset) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(3); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + const fml::TimePoint last_targetted_vsync = TimePointFromInt(0); + const fml::TimePoint now = TimePointFromInt(6); + const fml::TimePoint next_vsync = TimePointFromInt(10); + + const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 7); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 10); +} + +TEST(GetTargetTimesTest, ScheduleForFollowingVsync_BecauseOfNow) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + const fml::TimePoint last_targetted_vsync = TimePointFromInt(10); + const fml::TimePoint now = TimePointFromInt(15); + const fml::TimePoint next_vsync = TimePointFromInt(10); + + const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 20); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 30); +} + +TEST(GetTargetTimesTest, ScheduleForFollowingVsync_BecauseOfTargettedTime) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + const fml::TimePoint last_targetted_vsync = TimePointFromInt(20); + const fml::TimePoint now = TimePointFromInt(9); + const fml::TimePoint next_vsync = TimePointFromInt(10); + + const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 20); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 30); +} + +TEST(GetTargetTimesTest, ScheduleForDistantVsync_BecauseOfTargettedTime) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + const fml::TimePoint last_targetted_vsync = TimePointFromInt(60); + const fml::TimePoint now = TimePointFromInt(9); + const fml::TimePoint next_vsync = TimePointFromInt(10); + + const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 60); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 70); +} + +TEST(GetTargetTimesTest, ScheduleForFollowingVsync_WithSlightVsyncDrift) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + + // Even though it appears as if the next vsync is at time 40, we should still + // present at time 50. + const fml::TimePoint last_targetted_vsync = TimePointFromInt(37); + const fml::TimePoint now = TimePointFromInt(9); + const fml::TimePoint next_vsync = TimePointFromInt(10); + + const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 40); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 50); +} + +TEST(GetTargetTimesTest, ScheduleForAnOffsetFromVsync) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(4); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + const fml::TimePoint last_targetted_vsync = TimePointFromInt(10); + const fml::TimePoint now = TimePointFromInt(9); + const fml::TimePoint next_vsync = TimePointFromInt(10); + + const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 16); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 20); +} + +TEST(GetTargetTimesTest, ScheduleMultipleTimes) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + + fml::TimePoint last_targetted_vsync = TimePointFromInt(0); + fml::TimePoint now = TimePointFromInt(5); + fml::TimePoint next_vsync = TimePointFromInt(10); + + for (int i = 0; i < 100; ++i) { + const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 10 * (i + 1)); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 10 * (i + 2)); + + // Simulate the passage of time. + now = now + vsync_interval; + next_vsync = next_vsync + vsync_interval; + last_targetted_vsync = target_times.frame_target; + } +} + +TEST(GetTargetTimesTest, ScheduleMultipleTimes_WithDelayedWakeups) { + // It is often the case that Flutter does not wake up when it intends to due + // to CPU contention. This test has VsyncWaiter wake up to schedule 0-4ns + // after when |now| should be - and we verify that the results should be the + // same as if there were no delay. + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + + fml::TimePoint last_targetted_vsync = TimePointFromInt(0); + fml::TimePoint now = TimePointFromInt(5); + fml::TimePoint next_vsync = TimePointFromInt(10); + + for (int i = 0; i < 100; ++i) { + const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + const auto target_times_delay = flutter_runner::VsyncWaiter::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, + now + TimeDeltaFromInt(i % 5), next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), + TimePointToInt(target_times_delay.frame_start)); + EXPECT_EQ(TimePointToInt(target_times.frame_target), + TimePointToInt(target_times_delay.frame_target)); + + // Simulate the passage of time. + now = now + vsync_interval; + next_vsync = next_vsync + vsync_interval; + last_targetted_vsync = target_times.frame_target; + } +} + +} // namespace flutter_runner_test diff --git a/shell/platform/fuchsia/flutter/vsync_recorder.h b/shell/platform/fuchsia/flutter/vsync_recorder.h index c2a7f081d2462..35151ce1ceff9 100644 --- a/shell/platform/fuchsia/flutter/vsync_recorder.h +++ b/shell/platform/fuchsia/flutter/vsync_recorder.h @@ -30,7 +30,7 @@ class VsyncRecorder { // to be called in |scenic::Session::Present2| immedaite callbacks with the // presentation info provided by Scenic. Only the next vsync // information will be saved (in order to handle edge cases involving - // multiple Scenic sessions in the same process). This function is safe to + // multiple Scenic sessions in the same process). This function is safe to // call from any thread. void UpdateNextPresentationInfo( fuchsia::scenic::scheduling::FuturePresentationTimes info); diff --git a/shell/platform/fuchsia/flutter/vsync_waiter.cc b/shell/platform/fuchsia/flutter/vsync_waiter.cc index fd8f60122a0fa..ce468fafc4b27 100644 --- a/shell/platform/fuchsia/flutter/vsync_waiter.cc +++ b/shell/platform/fuchsia/flutter/vsync_waiter.cc @@ -19,64 +19,6 @@ namespace flutter_runner { -VsyncWaiter::VsyncWaiter(std::string debug_label, - zx_handle_t session_present_handle, - flutter::TaskRunners task_runners, - fml::TimeDelta vsync_offset) - : flutter::VsyncWaiter(task_runners), - debug_label_(std::move(debug_label)), - session_wait_(session_present_handle, SessionPresentSignal), - vsync_offset_(vsync_offset), - weak_factory_ui_(nullptr), - weak_factory_(this) { - auto wait_handler = [&](async_dispatcher_t* dispatcher, // - async::Wait* wait, // - zx_status_t status, // - const zx_packet_signal_t* signal // - ) { - if (status != ZX_OK) { - FML_LOG(ERROR) << "Vsync wait failed."; - return; - } - - wait->Cancel(); - - FireCallbackNow(); - }; - - // Generate a WeakPtrFactory for use with the UI thread. This does not need - // to wait on a latch because we only ever use the WeakPtrFactory on the UI - // thread so we have ordering guarantees (see ::AwaitVSync()) - fml::TaskRunner::RunNowOrPostTask( - task_runners_.GetUITaskRunner(), fml::MakeCopyable([this]() mutable { - this->weak_factory_ui_ = - std::make_unique>(this); - })); - session_wait_.set_handler(wait_handler); - - if (vsync_offset_ >= fml::TimeDelta::FromSeconds(1)) { - FML_LOG(WARNING) << "Given vsync_offset is extremely high: " - << vsync_offset_.ToMilliseconds() << "ms"; - } else { - FML_LOG(INFO) << "Set vsync_offset to " << vsync_offset_.ToMicroseconds() - << "us"; - } -} - -VsyncWaiter::~VsyncWaiter() { - session_wait_.Cancel(); - - fml::AutoResetWaitableEvent ui_latch; - fml::TaskRunner::RunNowOrPostTask( - task_runners_.GetUITaskRunner(), - fml::MakeCopyable( - [weak_factory_ui = std::move(weak_factory_ui_), &ui_latch]() mutable { - weak_factory_ui.reset(); - ui_latch.Signal(); - })); - ui_latch.Wait(); -} - /// Returns the system time at which the next frame is likely to be presented. /// /// Consider the following scenarios, where in both the @@ -109,13 +51,14 @@ fml::TimePoint VsyncWaiter::SnapToNextPhase( const fml::TimePoint last_frame_presentation_time, const fml::TimeDelta presentation_interval) { if (presentation_interval <= fml::TimeDelta::Zero()) { - FML_LOG(ERROR) << "Presentation interval must be positive. The value was: " - << presentation_interval.ToMilliseconds() << "ms."; + FML_LOG(WARNING) + << "Presentation interval must be positive. The value was: " + << presentation_interval.ToMilliseconds() << "ms."; return now; } if (last_frame_presentation_time >= now) { - FML_LOG(ERROR) + FML_LOG(WARNING) << "Last frame was presented in the future. Clamping to now."; return now + presentation_interval; } @@ -134,70 +77,196 @@ fml::TimePoint VsyncWaiter::SnapToNextPhase( } } -void VsyncWaiter::AwaitVSync() { - VsyncInfo vsync_info = VsyncRecorder::GetInstance().GetCurrentVsyncInfo(); +// This function takes in all relevant information to determining when should +// the next frame be scheduled. It returns a pair of (frame_start_time, +// frame_end_time) to be passed into FireCallback(). +// +// Importantly, there are two invariants for correct and performant scheduling +// that this function upholds: +// 1. Schedule the next frame at least half a vsync interval from the previous +// one. In practice, this means that every vsync interval Flutter produces +// exactly one frame in steady state behavior. +// 2. Only produce frames beginning in the future. +// +// |vsync_offset| - the time from the next vsync that the animator should begin +// working on the next frame. For instance if vsyncs are at 0ms, 16ms, and 33ms, +// and the |vsync_offset| is 5ms, then frames should begin at 11ms and 28ms. +// +// |vsync_interval| - the interval between vsyncs. Would be 16.6ms for a 60Hz +// display. +// +// |last_targetted_vsync| - the last vsync targetted, which is usually the +// frame_end_time returned from the last invocation of this function +// +// |now| - the current time +// +// |next_vsync| - the next vsync after |now|. This can be generated using the +// SnapToNextPhase function. +FlutterFrameTimes VsyncWaiter::GetTargetTimes( + fml::TimeDelta vsync_offset, + fml::TimeDelta vsync_interval, + fml::TimePoint last_targetted_vsync, + fml::TimePoint now, + fml::TimePoint next_vsync) { + FML_DCHECK(vsync_offset <= vsync_interval); + FML_DCHECK(vsync_interval > fml::TimeDelta::FromMilliseconds(0)); + FML_DCHECK(now < next_vsync && next_vsync < now + vsync_interval); - fml::TimePoint now = fml::TimePoint::Now(); - fml::TimePoint last_presentation_time = - VsyncRecorder::GetInstance().GetLastPresentationTime(); + // This makes the math much easier below, since we live in a (mod + // vsync_interval) world. + if (vsync_offset == fml::TimeDelta::FromNanoseconds(0)) { + vsync_offset = vsync_interval; + } - fml::TimePoint next_vsync = - VsyncRecorder::GetInstance().GetCurrentVsyncInfo().presentation_time; + // Start the frame after Scenic has finished its CPU work. This is + // accomplished using the vsync_offset. + fml::TimeDelta vsync_offset2 = vsync_interval - vsync_offset; + fml::TimePoint frame_start_time = + (next_vsync - vsync_interval) + vsync_offset2; - if (next_vsync <= now) { - next_vsync = SnapToNextPhase(now, last_presentation_time, - vsync_info.presentation_interval); + fml::TimePoint frame_end_time = next_vsync; + + // Advance to next available slot, keeping in mind the two invariants. + while (frame_end_time < (last_targetted_vsync + (vsync_interval / 2)) || + frame_start_time < now) { + frame_start_time = frame_start_time + vsync_interval; + frame_end_time = frame_end_time + vsync_interval; } - auto next_vsync_start_time = next_vsync - vsync_offset_; - - if (now >= next_vsync_start_time) - next_vsync_start_time = - next_vsync_start_time + vsync_info.presentation_interval; - - fml::TimeDelta delta = next_vsync_start_time - now; - - task_runners_.GetUITaskRunner()->PostDelayedTask( - [&weak_factory_ui = this->weak_factory_ui_] { - if (!weak_factory_ui) { - FML_LOG(WARNING) << "WeakPtrFactory for VsyncWaiter is null, likely " - "due to the VsyncWaiter being destroyed."; - return; - } - auto self = weak_factory_ui->GetWeakPtr(); - if (self) { - self->FireCallbackWhenSessionAvailable(); - } - }, - delta); + // Useful knowledge for analyzing traces. + fml::TimePoint previous_vsync = next_vsync - vsync_interval; + TRACE_DURATION( + "flutter", "VsyncWaiter::GetTargetTimes", "previous_vsync(ms)", + previous_vsync.ToEpochDelta().ToMilliseconds(), "last_targetted(ms)", + last_targetted_vsync.ToEpochDelta().ToMilliseconds(), "now(ms)", + fml::TimePoint::Now().ToEpochDelta().ToMilliseconds(), "next_vsync(ms))", + next_vsync.ToEpochDelta().ToMilliseconds(), "frame_start_time(ms)", + frame_start_time.ToEpochDelta().ToMilliseconds(), "frame_end_time(ms)", + frame_end_time.ToEpochDelta().ToMilliseconds()); + + return {frame_start_time, frame_end_time}; } -void VsyncWaiter::FireCallbackWhenSessionAvailable() { - TRACE_EVENT0("flutter", "VsyncWaiter::FireCallbackWhenSessionAvailable"); - FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); - if (session_wait_.Begin(async_get_default_dispatcher()) != ZX_OK) { - FML_LOG(ERROR) << "Could not begin wait for Vsync."; +VsyncWaiter::VsyncWaiter(std::shared_ptr session_connection, + flutter::TaskRunners task_runners, + fml::TimeDelta vsync_offset) + : flutter::VsyncWaiter(task_runners), + session_connection_(session_connection), + vsync_offset_(vsync_offset), + weak_factory_ui_(nullptr), + weak_factory_(this) { + FML_CHECK(session_connection_); + + // Generate a WeakPtrFactory for use with the UI thread. This does not need + // to wait on a latch because we only ever use the WeakPtrFactory on the UI + // thread so we have ordering guarantees (see ::AwaitVSync()) + fml::TaskRunner::RunNowOrPostTask( + task_runners_.GetUITaskRunner(), fml::MakeCopyable([this]() mutable { + this->weak_factory_ui_ = + std::make_unique>(this); + })); + + // Initialize the callback that will run every time the SessionConnection + // receives one or more frame presented events on Vsync. + session_connection_->InitializeVsyncWaiterCallback([this]() { + task_runners_.GetUITaskRunner()->PostTask([&weak_factory_ui = + this->weak_factory_ui_] { + if (!weak_factory_ui) { + FML_LOG(WARNING) << "WeakPtrFactory for VsyncWaiter is null, likely " + "due to the VsyncWaiter being destroyed."; + return; + } + + auto self = weak_factory_ui->GetWeakPtr(); + if (self) { + self->OnVsync(); + } + }); + }); + + FML_LOG(INFO) << "Flutter VsyncWaiter: Set vsync_offset to " + << vsync_offset_.ToMicroseconds() << "us"; +} + +VsyncWaiter::~VsyncWaiter() { + fml::AutoResetWaitableEvent ui_latch; + fml::TaskRunner::RunNowOrPostTask( + task_runners_.GetUITaskRunner(), + fml::MakeCopyable( + [weak_factory_ui = std::move(weak_factory_ui_), &ui_latch]() mutable { + weak_factory_ui.reset(); + ui_latch.Signal(); + })); + ui_latch.Wait(); +} + +// This function is called when the Animator wishes to schedule a new frame. +void VsyncWaiter::AwaitVSync() { + TRACE_DURATION("flutter", "VsyncWaiter::AwaitVsync()", + "request_already_pending", frame_request_pending_); + FireCallbackMaybe(); +} + +// This function is called when the Animator wants to know about the next vsync, +// but not for frame scheduling purposes. +void VsyncWaiter::AwaitVSyncForSecondaryCallback() { + TRACE_DURATION("flutter", "VsyncWaiter::AwaitVsyncForSecondaryCallback()", + "request_already_pending", frame_request_pending_); + + FlutterFrameTimes times = GetTargetTimesHelper(/*secondary_callback=*/true); + FireCallback(times.frame_start, times.frame_target, false); +} + +// Postcondition: Either a frame is scheduled or frame_request_pending_ is set +// to true, meaning we will attempt to schedule a frame on the next |OnVsync|. +void VsyncWaiter::FireCallbackMaybe() { + TRACE_DURATION("flutter", "FireCallbackMaybe"); + + if (session_connection_->CanRequestNewFrames()) { + FlutterFrameTimes times = + GetTargetTimesHelper(/*secondary_callback=*/false); + + last_targetted_vsync_ = times.frame_target; + frame_request_pending_ = false; + + FireCallback(times.frame_start, times.frame_target, false); + } else { + frame_request_pending_ = true; } } -void VsyncWaiter::FireCallbackNow() { - FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); +// This function is called when the SessionConnection signals us to let us know +// that one or more frames Flutter has produced have been displayed. Note that +// in practice this will be called several milliseconds after vsync, due to +// CPU contention. +void VsyncWaiter::OnVsync() { + TRACE_DURATION("flutter", "VsyncWaiter::OnVsync"); + + if (frame_request_pending_) { + FireCallbackMaybe(); + } +} - VsyncInfo vsync_info = VsyncRecorder::GetInstance().GetCurrentVsyncInfo(); +// A helper function for GetTargetTimes(), since many of the fields it takes +// have to be derived from other state. +FlutterFrameTimes VsyncWaiter::GetTargetTimesHelper(bool secondary_callback) { + fml::TimeDelta presentation_interval = + VsyncRecorder::GetInstance().GetCurrentVsyncInfo().presentation_interval; + fml::TimePoint next_vsync = + VsyncRecorder::GetInstance().GetCurrentVsyncInfo().presentation_time; fml::TimePoint now = fml::TimePoint::Now(); fml::TimePoint last_presentation_time = VsyncRecorder::GetInstance().GetLastPresentationTime(); - fml::TimePoint next_vsync = - VsyncRecorder::GetInstance().GetCurrentVsyncInfo().presentation_time; - if (next_vsync <= now) { - next_vsync = SnapToNextPhase(now, last_presentation_time, - vsync_info.presentation_interval); + next_vsync = + SnapToNextPhase(now, last_presentation_time, presentation_interval); } - fml::TimePoint previous_vsync = next_vsync - vsync_info.presentation_interval; - FireCallback(previous_vsync, next_vsync, false); + fml::TimePoint last_targetted_vsync = + secondary_callback ? fml::TimePoint::Min() : last_targetted_vsync_; + return GetTargetTimes(vsync_offset_, presentation_interval, + last_targetted_vsync, now, next_vsync); } } // namespace flutter_runner diff --git a/shell/platform/fuchsia/flutter/vsync_waiter.h b/shell/platform/fuchsia/flutter/vsync_waiter.h index 117153c491085..79f76963d2c0a 100644 --- a/shell/platform/fuchsia/flutter/vsync_waiter.h +++ b/shell/platform/fuchsia/flutter/vsync_waiter.h @@ -14,40 +14,63 @@ #include "flutter/shell/common/vsync_waiter.h" #include "flutter_runner_product_configuration.h" +#include "session_connection.h" + namespace flutter_runner { +struct FlutterFrameTimes { + fml::TimePoint frame_start; + fml::TimePoint frame_target; +}; + class VsyncWaiter final : public flutter::VsyncWaiter { public: - static constexpr zx_signals_t SessionPresentSignal = ZX_EVENT_SIGNALED; - - VsyncWaiter(std::string debug_label, - zx_handle_t session_present_handle, - flutter::TaskRunners task_runners, - fml::TimeDelta vsync_offset); - - ~VsyncWaiter() override; + static FlutterFrameTimes GetTargetTimes(fml::TimeDelta vsync_offset, + fml::TimeDelta vsync_interval, + fml::TimePoint last_targetted_vsync, + fml::TimePoint now, + fml::TimePoint next_vsync); static fml::TimePoint SnapToNextPhase( const fml::TimePoint now, const fml::TimePoint last_frame_presentation_time, const fml::TimeDelta presentation_interval); - private: - const std::string debug_label_; - async::Wait session_wait_; - fml::TimeDelta vsync_offset_ = fml::TimeDelta::FromMicroseconds(0); + VsyncWaiter(std::shared_ptr session_connection, + flutter::TaskRunners task_runners, + fml::TimeDelta vsync_offset); - // For accessing the VsyncWaiter via the UI thread, necessary for the callback - // for AwaitVSync() - std::unique_ptr> weak_factory_ui_; - fml::WeakPtrFactory weak_factory_; + ~VsyncWaiter() override; + private: // |flutter::VsyncWaiter| void AwaitVSync() override; - void FireCallbackWhenSessionAvailable(); + // |flutter::VsyncWaiter| + void AwaitVSyncForSecondaryCallback() override; + + void FireCallbackMaybe(); + void OnVsync(); + + FlutterFrameTimes GetTargetTimesHelper(bool secondary_callback); - void FireCallbackNow(); + std::shared_ptr session_connection_; + + // This is the offset from vsync that Flutter should wake up at to begin work + // on the next frame. + fml::TimeDelta vsync_offset_; + + // This is the last Vsync we submitted as the frame_target_time to + // FireCallback(). This value should be strictly increasing in order to + // guarantee that animation code that relies on target vsyncs works correctly, + // and that Flutter is not producing multiple frames in a small interval. + fml::TimePoint last_targetted_vsync_; + + // This is true iff AwaitVSync() was called but we could not schedule a frame. + bool frame_request_pending_ = false; + + std::unique_ptr> weak_factory_ui_; + fml::WeakPtrFactory weak_factory_; FML_DISALLOW_COPY_AND_ASSIGN(VsyncWaiter); }; diff --git a/shell/platform/fuchsia/flutter/vsync_waiter_unittests.cc b/shell/platform/fuchsia/flutter/vsync_waiter_unittests.cc deleted file mode 100644 index 85a820b62c10d..0000000000000 --- a/shell/platform/fuchsia/flutter/vsync_waiter_unittests.cc +++ /dev/null @@ -1,137 +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 -#include -#include -#include - -#include - -#include "flutter/flow/frame_timings.h" -#include "flutter/fml/synchronization/waitable_event.h" -#include "flutter/fml/time/time_delta.h" -#include "flutter/fml/time/time_point.h" -#include "flutter/shell/common/thread_host.h" -#include "flutter/shell/common/vsync_waiter.h" -#include "flutter/shell/platform/fuchsia/flutter/flutter_runner_product_configuration.h" -#include "flutter/shell/platform/fuchsia/flutter/task_runner_adapter.h" -#include "flutter/shell/platform/fuchsia/flutter/thread.h" -#include "flutter/shell/platform/fuchsia/flutter/vsync_waiter.h" - -namespace flutter_runner_test { - -class VsyncWaiterTest : public testing::Test { - public: - VsyncWaiterTest() {} - - ~VsyncWaiterTest() = default; - - std::unique_ptr CreateVsyncWaiter( - flutter::TaskRunners task_runners) { - return std::make_unique( - "VsyncWaiterTest", vsync_event_.get(), task_runners, - fml::TimeDelta::Zero()); - } - - void SignalVsyncEvent() { - auto status = - zx_object_signal(vsync_event_.get(), 0, - flutter_runner::VsyncWaiter::SessionPresentSignal); - EXPECT_EQ(status, ZX_OK); - } - - protected: - void SetUp() override { - auto status = zx::event::create(0, &vsync_event_); - EXPECT_EQ(status, ZX_OK); - } - - private: - zx::event vsync_event_; -}; - -TEST_F(VsyncWaiterTest, AwaitVsync) { - std::array, 3> threads; - - for (auto& thread : threads) { - thread.reset(new flutter_runner::Thread()); - } - - async::Loop loop(&kAsyncLoopConfigAttachToCurrentThread); - - const flutter::TaskRunners task_runners( - "VsyncWaiterTests", // Dart thread labels - flutter_runner::CreateFMLTaskRunner( - async_get_default_dispatcher()), // platform - flutter_runner::CreateFMLTaskRunner(threads[0]->dispatcher()), // raster - flutter_runner::CreateFMLTaskRunner(threads[1]->dispatcher()), // ui - flutter_runner::CreateFMLTaskRunner(threads[2]->dispatcher()) // io - ); - - auto vsync_waiter = CreateVsyncWaiter(std::move(task_runners)); - - fml::AutoResetWaitableEvent latch; - vsync_waiter->AsyncWaitForVsync( - [&latch](std::unique_ptr recorder) { - latch.Signal(); - }); - SignalVsyncEvent(); - - bool did_timeout = - latch.WaitWithTimeout(fml::TimeDelta::FromMilliseconds(5000)); - - // False indicates we were signalled rather than timed out - EXPECT_FALSE(did_timeout); - - vsync_waiter.reset(); - for (const auto& thread : threads) { - thread->Quit(); - } -} - -TEST_F(VsyncWaiterTest, SnapToNextPhaseOverlapsWithNow) { - const auto now = fml::TimePoint::Now(); - const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(10); - const auto delta = fml::TimeDelta::FromNanoseconds(10); - const auto next_vsync = flutter_runner::VsyncWaiter::SnapToNextPhase( - now, last_presentation_time, delta); - - EXPECT_EQ(now + delta, next_vsync); -} - -TEST_F(VsyncWaiterTest, SnapToNextPhaseAfterNow) { - const auto now = fml::TimePoint::Now(); - const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(9); - const auto delta = fml::TimeDelta::FromNanoseconds(10); - const auto next_vsync = flutter_runner::VsyncWaiter::SnapToNextPhase( - now, last_presentation_time, delta); - - // math here: 10 - 9 = 1 - EXPECT_EQ(now + fml::TimeDelta::FromNanoseconds(1), next_vsync); -} - -TEST_F(VsyncWaiterTest, SnapToNextPhaseAfterNowMultiJump) { - const auto now = fml::TimePoint::Now(); - const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(34); - const auto delta = fml::TimeDelta::FromNanoseconds(10); - const auto next_vsync = flutter_runner::VsyncWaiter::SnapToNextPhase( - now, last_presentation_time, delta); - - // zeroes: -34, -24, -14, -4, 6, ... - EXPECT_EQ(now + fml::TimeDelta::FromNanoseconds(6), next_vsync); -} - -TEST_F(VsyncWaiterTest, SnapToNextPhaseAfterNowMultiJumpAccountForCeils) { - const auto now = fml::TimePoint::Now(); - const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(20); - const auto delta = fml::TimeDelta::FromNanoseconds(16); - const auto next_vsync = flutter_runner::VsyncWaiter::SnapToNextPhase( - now, last_presentation_time, delta); - - // zeroes: -20, -4, 12, 28, ... - EXPECT_EQ(now + fml::TimeDelta::FromNanoseconds(12), next_vsync); -} - -} // namespace flutter_runner_test From 21b89ee928905cdf71574f58bc6ca483ca58d6b9 Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Tue, 18 May 2021 16:45:08 -0400 Subject: [PATCH 02/16] moved to pure callback only mode unittests are deactivated --- shell/platform/fuchsia/flutter/BUILD.gn | 7 +- .../flutter/default_session_connection.cc | 201 +++++++++++++++++- .../flutter/default_session_connection.h | 49 ++++- shell/platform/fuchsia/flutter/engine.cc | 30 ++- .../platform/fuchsia/flutter/platform_view.cc | 13 +- .../platform/fuchsia/flutter/platform_view.h | 13 +- .../platform/fuchsia/flutter/vsync_waiter.cc | 198 +++-------------- shell/platform/fuchsia/flutter/vsync_waiter.h | 46 ++-- 8 files changed, 322 insertions(+), 235 deletions(-) diff --git a/shell/platform/fuchsia/flutter/BUILD.gn b/shell/platform/fuchsia/flutter/BUILD.gn index 9b9feba36e013..94287ef01f23f 100644 --- a/shell/platform/fuchsia/flutter/BUILD.gn +++ b/shell/platform/fuchsia/flutter/BUILD.gn @@ -451,13 +451,13 @@ executable("flutter_runner_unittests") { "flutter_runner_fakes.h", "fuchsia_intl_unittest.cc", "keyboard_unittest.cc", - "platform_view_unittest.cc", + #"platform_view_unittest.cc", "runner_unittest.cc", "tests/engine_unittests.cc", "tests/fake_session_connection.h", "tests/flutter_runner_product_configuration_unittests.cc", "tests/vsync_recorder_unittests.cc", - "tests/vsync_waiter_unittests.cc", + #"tests/vsync_waiter_unittests.cc", ] # This is needed for //third_party/googletest for linking zircon symbols. @@ -513,7 +513,8 @@ executable("flutter_runner_scenic_unittests") { output_name = "flutter_runner_scenic_tests" - sources = [ "tests/default_session_connection_unittests.cc" ] + sources = [ #"tests/default_session_connection_unittests.cc" +] # This is needed for //third_party/googletest for linking zircon symbols. libs = [ "//fuchsia/sdk/$host_os/arch/$target_cpu/sysroot/lib/libzircon.so" ] diff --git a/shell/platform/fuchsia/flutter/default_session_connection.cc b/shell/platform/fuchsia/flutter/default_session_connection.cc index 3afa3ebd7780e..d947d725924d0 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.cc +++ b/shell/platform/fuchsia/flutter/default_session_connection.cc @@ -12,15 +12,145 @@ namespace flutter_runner { +// This function takes in all relevant information to determining when should +// the next frame be scheduled. It returns a pair of (frame_start_time, +// frame_end_time) to be passed into FireCallback(). +// +// Importantly, there are two invariants for correct and performant scheduling +// that this function upholds: +// 1. Schedule the next frame at least half a vsync interval from the previous +// one. In practice, this means that every vsync interval Flutter produces +// exactly one frame in steady state behavior. +// 2. Only produce frames beginning in the future. +// +// |vsync_offset| - the time from the next vsync that the animator should begin +// working on the next frame. For instance if vsyncs are at 0ms, 16ms, and 33ms, +// and the |vsync_offset| is 5ms, then frames should begin at 11ms and 28ms. +// +// |vsync_interval| - the interval between vsyncs. Would be 16.6ms for a 60Hz +// display. +// +// |last_targetted_vsync| - the last vsync targetted, which is usually the +// frame_end_time returned from the last invocation of this function +// +// |now| - the current time +// +// |next_vsync| - the next vsync after |now|. This can be generated using the +// SnapToNextPhase function. +FlutterFrameTimes DefaultSessionConnection::GetTargetTimes( + fml::TimeDelta vsync_offset, + fml::TimeDelta vsync_interval, + fml::TimePoint last_targetted_vsync, + fml::TimePoint now, + fml::TimePoint next_vsync) { + FML_DCHECK(vsync_offset <= vsync_interval); + FML_DCHECK(vsync_interval > fml::TimeDelta::FromMilliseconds(0)); + FML_DCHECK(now < next_vsync && next_vsync < now + vsync_interval); + + // This makes the math much easier below, since we live in a (mod + // vsync_interval) world. + if (vsync_offset == fml::TimeDelta::FromNanoseconds(0)) { + vsync_offset = vsync_interval; + } + + // Start the frame after Scenic has finished its CPU work. This is + // accomplished using the vsync_offset. + fml::TimeDelta vsync_offset2 = vsync_interval - vsync_offset; + fml::TimePoint frame_start_time = + (next_vsync - vsync_interval) + vsync_offset2; + + fml::TimePoint frame_end_time = next_vsync; + + // Advance to next available slot, keeping in mind the two invariants. + while (frame_end_time < (last_targetted_vsync + (vsync_interval / 2)) || + frame_start_time < now) { + frame_start_time = frame_start_time + vsync_interval; + frame_end_time = frame_end_time + vsync_interval; + } + + // Useful knowledge for analyzing traces. + fml::TimePoint previous_vsync = next_vsync - vsync_interval; + TRACE_DURATION( + "flutter", "VsyncWaiter::GetTargetTimes", "previous_vsync(ms)", + previous_vsync.ToEpochDelta().ToMilliseconds(), "last_targetted(ms)", + last_targetted_vsync.ToEpochDelta().ToMilliseconds(), "now(ms)", + fml::TimePoint::Now().ToEpochDelta().ToMilliseconds(), "next_vsync(ms))", + next_vsync.ToEpochDelta().ToMilliseconds(), "frame_start_time(ms)", + frame_start_time.ToEpochDelta().ToMilliseconds(), "frame_end_time(ms)", + frame_end_time.ToEpochDelta().ToMilliseconds()); + + return {frame_start_time, frame_end_time}; +} + +/// Returns the system time at which the next frame is likely to be presented. +/// +/// Consider the following scenarios, where in both the +/// scenarios the result will be the same. +/// +/// Scenario 1: +/// presentation_interval is 2 +/// ^ ^ ^ ^ ^ +/// + + + + + +/// 0--1--2--3--4--5--6--7--8--9-- +/// + + + +/// | | +---------> result: next_presentation_time +/// | v +/// v now +/// last_presentation_time +/// +/// Scenario 2: +/// presentation_interval is 2 +/// ^ ^ ^ ^ ^ +/// + + + + + +/// 0--1--2--3--4--5--6--7--8--9-- +/// + + + +/// | | +--------->result: next_presentation_time +/// | | +/// | +>now +/// | +/// +->last_presentation_time +fml::TimePoint DefaultSessionConnection::SnapToNextPhase( + const fml::TimePoint now, + const fml::TimePoint last_frame_presentation_time, + const fml::TimeDelta presentation_interval) { + if (presentation_interval <= fml::TimeDelta::Zero()) { + FML_LOG(WARNING) + << "Presentation interval must be positive. The value was: " + << presentation_interval.ToMilliseconds() << "ms."; + return now; + } + + if (last_frame_presentation_time >= now) { + FML_LOG(WARNING) + << "Last frame was presented in the future. Clamping to now."; + return now + presentation_interval; + } + + const fml::TimeDelta time_since_last_presentation = + now - last_frame_presentation_time; + // this will be the most likely scenario if we are rendering at a good + // frame rate, short circuiting the other checks in this case. + if (time_since_last_presentation < presentation_interval) { + return last_frame_presentation_time + presentation_interval; + } else { + const int64_t num_phases_passed = + (time_since_last_presentation / presentation_interval); + return last_frame_presentation_time + + (presentation_interval * (num_phases_passed + 1)); + } +} + DefaultSessionConnection::DefaultSessionConnection( std::string debug_label, fidl::InterfaceHandle session, fml::closure session_error_callback, on_frame_presented_event on_frame_presented_callback, - uint64_t max_frames_in_flight) + uint64_t max_frames_in_flight, + fml::TimeDelta vsync_offset) : session_wrapper_(session.Bind(), nullptr), on_frame_presented_callback_(std::move(on_frame_presented_callback)), - kMaxFramesInFlight(max_frames_in_flight) { + kMaxFramesInFlight(max_frames_in_flight), + vsync_offset_(vsync_offset) { session_wrapper_.set_error_handler( [callback = session_error_callback](zx_status_t status) { callback(); }); @@ -51,8 +181,8 @@ DefaultSessionConnection::DefaultSessionConnection( PresentSession(); } - if (vsync_waiter_initialized_) { - vsync_waiter_callback_(); + if (fire_callback_request_pending_) { + FireCallbackMaybe(); } } // callback ); @@ -75,6 +205,8 @@ DefaultSessionConnection::DefaultSessionConnection( PresentSession(); }); + FML_LOG(INFO) << "Flutter DefaultSessionConnection: Set vsync_offset to " + << vsync_offset_.ToMicroseconds() << "us"; } DefaultSessionConnection::~DefaultSessionConnection() = default; @@ -195,12 +327,63 @@ void DefaultSessionConnection::PresentSession() { }); } -void DefaultSessionConnection::InitializeVsyncWaiterCallback( - VsyncWaiterCallback callback) { - FML_DCHECK(!vsync_waiter_initialized_); +void DefaultSessionConnection::AwaitVsync(FireCallbackCallback callback) { + // FML_LOG(INFO) << "CRASH:: DSC::AwaitVsync"; + fire_callback_ = callback; + + FireCallbackMaybe(); +} + +void DefaultSessionConnection::AwaitVsyncForSecondaryCallback( + FireCallbackCallback callback) { + // FML_LOG(INFO) << "CRASH:: DSC::AwaitVsyncForSecondaryCallback"; + fire_callback_ = callback; + + FlutterFrameTimes times = GetTargetTimesHelper(/*secondary_callback=*/true); + fire_callback_(times.frame_start, times.frame_target); +} + +// Postcondition: Either a frame is scheduled or fire_callback_request_pending_ +// is set to true, meaning we will attempt to schedule a frame on the next +// |OnVsync|. +void DefaultSessionConnection::FireCallbackMaybe() { + TRACE_DURATION("flutter", "FireCallbackMaybe"); + // FML_LOG(INFO) << "CRASH:: DSC::FireCallbackMaybe"; + + if (frames_in_flight_ < kMaxFramesInFlight) { + FlutterFrameTimes times = + GetTargetTimesHelper(/*secondary_callback=*/false); + + last_targetted_vsync_ = times.frame_target; + fire_callback_request_pending_ = false; + + fire_callback_(times.frame_start, times.frame_target); + } else { + fire_callback_request_pending_ = true; + } +} + +// A helper function for GetTargetTimes(), since many of the fields it takes +// have to be derived from other state. +FlutterFrameTimes DefaultSessionConnection::GetTargetTimesHelper( + bool secondary_callback) { + fml::TimeDelta presentation_interval = + VsyncRecorder::GetInstance().GetCurrentVsyncInfo().presentation_interval; + + fml::TimePoint next_vsync = + VsyncRecorder::GetInstance().GetCurrentVsyncInfo().presentation_time; + fml::TimePoint now = fml::TimePoint::Now(); + fml::TimePoint last_presentation_time = + VsyncRecorder::GetInstance().GetLastPresentationTime(); + if (next_vsync <= now) { + next_vsync = + SnapToNextPhase(now, last_presentation_time, presentation_interval); + } - vsync_waiter_callback_ = callback; - vsync_waiter_initialized_ = true; + fml::TimePoint last_targetted_vsync = + secondary_callback ? fml::TimePoint::Min() : last_targetted_vsync_; + return GetTargetTimes(vsync_offset_, presentation_interval, + last_targetted_vsync, now, next_vsync); } } // namespace flutter_runner diff --git a/shell/platform/fuchsia/flutter/default_session_connection.h b/shell/platform/fuchsia/flutter/default_session_connection.h index 3debfd9873d3d..5b5c4e34db361 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.h +++ b/shell/platform/fuchsia/flutter/default_session_connection.h @@ -15,17 +15,21 @@ #include "flutter/fml/macros.h" #include "session_connection.h" +#include "vsync_waiter.h" namespace flutter_runner { using on_frame_presented_event = std::function; +struct FlutterFrameTimes { + fml::TimePoint frame_start; + fml::TimePoint frame_target; +}; + // The component residing on the raster thread that is responsible for // maintaining the Scenic session connection and presenting node updates. -class DefaultSessionConnection final - : public flutter::SessionWrapper, - public flutter_runner::SessionConnection { +class DefaultSessionConnection final : public flutter::SessionWrapper { public: static fml::TimePoint CalculateNextLatchPoint( fml::TimePoint present_requested_time, @@ -36,12 +40,24 @@ class DefaultSessionConnection final std::deque>& future_presentation_infos); + static FlutterFrameTimes GetTargetTimes(fml::TimeDelta vsync_offset, + fml::TimeDelta vsync_interval, + fml::TimePoint last_targetted_vsync, + fml::TimePoint now, + fml::TimePoint next_vsync); + + static fml::TimePoint SnapToNextPhase( + const fml::TimePoint now, + const fml::TimePoint last_frame_presentation_time, + const fml::TimeDelta presentation_interval); + DefaultSessionConnection( std::string debug_label, fidl::InterfaceHandle session, fml::closure session_error_callback, on_frame_presented_event on_frame_presented_callback, - uint64_t max_frames_in_flight); + uint64_t max_frames_in_flight, + fml::TimeDelta vsync_offset); ~DefaultSessionConnection(); @@ -51,6 +67,7 @@ class DefaultSessionConnection final // |SessionWrapper| void Present() override; + /* // |SessionConnection| void InitializeVsyncWaiterCallback(VsyncWaiterCallback callback) override; @@ -58,14 +75,17 @@ class DefaultSessionConnection final bool CanRequestNewFrames() override { return frames_in_flight_ < kMaxFramesInFlight; } + */ + + void AwaitVsync(FireCallbackCallback callback); + + void AwaitVsyncForSecondaryCallback(FireCallbackCallback callback); private: scenic::Session session_wrapper_; on_frame_presented_event on_frame_presented_callback_; - zx_handle_t vsync_event_handle_; - fml::TimePoint last_latch_point_targeted_ = fml::TimePoint::FromEpochDelta(fml::TimeDelta::Zero()); fml::TimePoint present_requested_time_ = @@ -88,18 +108,29 @@ class DefaultSessionConnection final // outstanding at any time. This is equivalent to how many times it has // called Present2() before receiving an OnFramePresented() event. const int kMaxFramesInFlight; + fml::TimeDelta vsync_offset_; + int frames_in_flight_ = 0; int frames_in_flight_allowed_ = 0; bool present_session_pending_ = false; - bool vsync_waiter_initialized_ = false; - VsyncWaiterCallback vsync_waiter_callback_; + // This is the last Vsync we submitted as the frame_target_time to + // FireCallback(). This value should be strictly increasing in order to + // guarantee that animation code that relies on target vsyncs works correctly, + // and that Flutter is not producing multiple frames in a small interval. + fml::TimePoint last_targetted_vsync_; + + // This is true iff AwaitVSync() was called but we could not schedule a frame. + bool fire_callback_request_pending_ = false; + + FireCallbackCallback fire_callback_; void PresentSession(); - static void ToggleSignal(zx_handle_t handle, bool raise); + void FireCallbackMaybe(); + FlutterFrameTimes GetTargetTimesHelper(bool secondary_callback); FML_DISALLOW_COPY_AND_ASSIGN(DefaultSessionConnection); }; diff --git a/shell/platform/fuchsia/flutter/engine.cc b/shell/platform/fuchsia/flutter/engine.cc index 001d96328dfd6..020c110ca745e 100644 --- a/shell/platform/fuchsia/flutter/engine.cc +++ b/shell/platform/fuchsia/flutter/engine.cc @@ -25,6 +25,7 @@ #include "platform_view.h" #include "surface.h" #include "task_runner_adapter.h" +#include "vsync_waiter.h" #if defined(LEGACY_FUCHSIA_EMBEDDER) #include "compositor_context.h" // nogncheck @@ -134,11 +135,12 @@ Engine::Engine(Delegate& delegate, view_token = std::move(view_token), view_ref_pair = std::move(view_ref_pair), max_frames_in_flight = product_config.get_max_frames_in_flight(), - &view_embedder_latch]() mutable { + &view_embedder_latch, + vsync_offset = product_config.get_vsync_offset()]() mutable { session_connection_ = std::make_shared( thread_label_, std::move(session), std::move(session_error_callback), [](auto) {}, - max_frames_in_flight); + max_frames_in_flight, vsync_offset); surface_producer_.emplace(session_connection_->get()); #if defined(LEGACY_FUCHSIA_EMBEDDER) if (use_legacy_renderer_) { @@ -228,8 +230,8 @@ Engine::Engine(Delegate& delegate, // Setup the callback that will instantiate the platform view. flutter::Shell::CreateCallback on_create_platform_view = fml::MakeCopyable( - [this, debug_label = thread_label_, - view_ref = std::move(platform_view_ref), runner_services, + [debug_label = thread_label_, view_ref = std::move(platform_view_ref), + runner_services, parent_environment_service_provider = std::move(parent_environment_service_provider), session_listener_request = std::move(session_listener_request), @@ -243,9 +245,15 @@ Engine::Engine(Delegate& delegate, on_destroy_view_callback = std::move(on_destroy_view_callback), on_create_surface_callback = std::move(on_create_surface_callback), external_view_embedder = GetExternalViewEmbedder(), - vsync_offset = product_config.get_vsync_offset(), - keyboard_listener_request = std::move(keyboard_listener_request)]( - flutter::Shell& shell) mutable { + keyboard_listener_request = std::move(keyboard_listener_request), + await_vsync_callback = + [this](FireCallbackCallback cb) { + session_connection_->AwaitVsync(cb); + }, + await_vsync_for_secondary_callback_callback = + [this](FireCallbackCallback cb) { + session_connection_->AwaitVsyncForSecondaryCallback(cb); + }](flutter::Shell& shell) mutable { return std::make_unique( shell, // delegate debug_label, // debug label @@ -263,10 +271,10 @@ Engine::Engine(Delegate& delegate, std::move(on_create_view_callback), std::move(on_update_view_callback), std::move(on_destroy_view_callback), - std::move(on_create_surface_callback), - external_view_embedder, // external view embedder - std::move(vsync_offset), // vsync offset - session_connection_); + std::move(on_create_surface_callback), external_view_embedder, + // Callbacks for VsyncWaiter to call into SessionConnection. + await_vsync_callback, + await_vsync_for_secondary_callback_callback); }); // Setup the callback that will instantiate the rasterizer. diff --git a/shell/platform/fuchsia/flutter/platform_view.cc b/shell/platform/fuchsia/flutter/platform_view.cc index 190ed9b5a9d3b..4a0d345b554ed 100644 --- a/shell/platform/fuchsia/flutter/platform_view.cc +++ b/shell/platform/fuchsia/flutter/platform_view.cc @@ -73,8 +73,9 @@ PlatformView::PlatformView( OnDestroyView on_destroy_view_callback, OnCreateSurface on_create_surface_callback, std::shared_ptr external_view_embedder, - fml::TimeDelta vsync_offset, - std::shared_ptr session_connection) + AwaitVsyncCallback await_vsync_callback, + AwaitVsyncForSecondaryCallbackCallback + await_vsync_for_secondary_callback_callback) : flutter::PlatformView(delegate, std::move(task_runners)), debug_label_(std::move(debug_label)), view_ref_(std::move(view_ref)), @@ -89,9 +90,10 @@ PlatformView::PlatformView( on_create_surface_callback_(std::move(on_create_surface_callback)), external_view_embedder_(external_view_embedder), ime_client_(this), - vsync_offset_(std::move(vsync_offset)), - session_connection_(session_connection), keyboard_listener_binding_(this, std::move(keyboard_listener_request)), + await_vsync_callback_(await_vsync_callback), + await_vsync_for_secondary_callback_callback_( + await_vsync_for_secondary_callback_callback), weak_factory_(this) { // Register all error handlers. SetInterfaceErrorHandler(session_listener_binding_, "SessionListener"); @@ -692,7 +694,8 @@ void PlatformView::DeactivateIme() { // |flutter::PlatformView| std::unique_ptr PlatformView::CreateVSyncWaiter() { return std::make_unique( - session_connection_, task_runners_, vsync_offset_); + await_vsync_callback_, await_vsync_for_secondary_callback_callback_, + task_runners_); } // |flutter::PlatformView| diff --git a/shell/platform/fuchsia/flutter/platform_view.h b/shell/platform/fuchsia/flutter/platform_view.h index 118e82784b803..940a60132cbb9 100644 --- a/shell/platform/fuchsia/flutter/platform_view.h +++ b/shell/platform/fuchsia/flutter/platform_view.h @@ -23,6 +23,7 @@ #include "flutter/shell/common/platform_view.h" #include "flutter/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h" #include "flutter/shell/platform/fuchsia/flutter/keyboard.h" +#include "flutter/shell/platform/fuchsia/flutter/vsync_waiter.h" #include "accessibility_bridge.h" @@ -71,8 +72,9 @@ class PlatformView final : public flutter::PlatformView, OnDestroyView on_destroy_view_callback, OnCreateSurface on_create_surface_callback, std::shared_ptr view_embedder, - fml::TimeDelta vsync_offset, - std::shared_ptr session_connection); + AwaitVsyncCallback await_vsync_callback, + AwaitVsyncForSecondaryCallbackCallback + await_vsync_for_secondary_callback_callback); ~PlatformView(); @@ -217,9 +219,6 @@ class PlatformView final : public flutter::PlatformView, // https://github.com/flutter/flutter/issues/55966 std::set unregistered_channels_; - fml::TimeDelta vsync_offset_; - std::shared_ptr session_connection_; - // The registered binding for serving the keyboard listener server endpoint. fidl::Binding keyboard_listener_binding_; @@ -227,6 +226,10 @@ class PlatformView final : public flutter::PlatformView, // The keyboard translation for fuchsia.ui.input3.KeyEvent. Keyboard keyboard_; + AwaitVsyncCallback await_vsync_callback_; + AwaitVsyncForSecondaryCallbackCallback + await_vsync_for_secondary_callback_callback_; + fml::WeakPtrFactory weak_factory_; // Must be the last member. FML_DISALLOW_COPY_AND_ASSIGN(PlatformView); diff --git a/shell/platform/fuchsia/flutter/vsync_waiter.cc b/shell/platform/fuchsia/flutter/vsync_waiter.cc index ce468fafc4b27..097d6a87f5570 100644 --- a/shell/platform/fuchsia/flutter/vsync_waiter.cc +++ b/shell/platform/fuchsia/flutter/vsync_waiter.cc @@ -19,143 +19,22 @@ namespace flutter_runner { -/// Returns the system time at which the next frame is likely to be presented. -/// -/// Consider the following scenarios, where in both the -/// scenarios the result will be the same. -/// -/// Scenario 1: -/// presentation_interval is 2 -/// ^ ^ ^ ^ ^ -/// + + + + + -/// 0--1--2--3--4--5--6--7--8--9-- -/// + + + -/// | | +---------> result: next_presentation_time -/// | v -/// v now -/// last_presentation_time -/// -/// Scenario 2: -/// presentation_interval is 2 -/// ^ ^ ^ ^ ^ -/// + + + + + -/// 0--1--2--3--4--5--6--7--8--9-- -/// + + + -/// | | +--------->result: next_presentation_time -/// | | -/// | +>now -/// | -/// +->last_presentation_time -fml::TimePoint VsyncWaiter::SnapToNextPhase( - const fml::TimePoint now, - const fml::TimePoint last_frame_presentation_time, - const fml::TimeDelta presentation_interval) { - if (presentation_interval <= fml::TimeDelta::Zero()) { - FML_LOG(WARNING) - << "Presentation interval must be positive. The value was: " - << presentation_interval.ToMilliseconds() << "ms."; - return now; - } - - if (last_frame_presentation_time >= now) { - FML_LOG(WARNING) - << "Last frame was presented in the future. Clamping to now."; - return now + presentation_interval; - } - - const fml::TimeDelta time_since_last_presentation = - now - last_frame_presentation_time; - // this will be the most likely scenario if we are rendering at a good - // frame rate, short circuiting the other checks in this case. - if (time_since_last_presentation < presentation_interval) { - return last_frame_presentation_time + presentation_interval; - } else { - const int64_t num_phases_passed = - (time_since_last_presentation / presentation_interval); - return last_frame_presentation_time + - (presentation_interval * (num_phases_passed + 1)); - } -} - -// This function takes in all relevant information to determining when should -// the next frame be scheduled. It returns a pair of (frame_start_time, -// frame_end_time) to be passed into FireCallback(). -// -// Importantly, there are two invariants for correct and performant scheduling -// that this function upholds: -// 1. Schedule the next frame at least half a vsync interval from the previous -// one. In practice, this means that every vsync interval Flutter produces -// exactly one frame in steady state behavior. -// 2. Only produce frames beginning in the future. -// -// |vsync_offset| - the time from the next vsync that the animator should begin -// working on the next frame. For instance if vsyncs are at 0ms, 16ms, and 33ms, -// and the |vsync_offset| is 5ms, then frames should begin at 11ms and 28ms. -// -// |vsync_interval| - the interval between vsyncs. Would be 16.6ms for a 60Hz -// display. -// -// |last_targetted_vsync| - the last vsync targetted, which is usually the -// frame_end_time returned from the last invocation of this function -// -// |now| - the current time -// -// |next_vsync| - the next vsync after |now|. This can be generated using the -// SnapToNextPhase function. -FlutterFrameTimes VsyncWaiter::GetTargetTimes( - fml::TimeDelta vsync_offset, - fml::TimeDelta vsync_interval, - fml::TimePoint last_targetted_vsync, - fml::TimePoint now, - fml::TimePoint next_vsync) { - FML_DCHECK(vsync_offset <= vsync_interval); - FML_DCHECK(vsync_interval > fml::TimeDelta::FromMilliseconds(0)); - FML_DCHECK(now < next_vsync && next_vsync < now + vsync_interval); - - // This makes the math much easier below, since we live in a (mod - // vsync_interval) world. - if (vsync_offset == fml::TimeDelta::FromNanoseconds(0)) { - vsync_offset = vsync_interval; - } - - // Start the frame after Scenic has finished its CPU work. This is - // accomplished using the vsync_offset. - fml::TimeDelta vsync_offset2 = vsync_interval - vsync_offset; - fml::TimePoint frame_start_time = - (next_vsync - vsync_interval) + vsync_offset2; - - fml::TimePoint frame_end_time = next_vsync; - - // Advance to next available slot, keeping in mind the two invariants. - while (frame_end_time < (last_targetted_vsync + (vsync_interval / 2)) || - frame_start_time < now) { - frame_start_time = frame_start_time + vsync_interval; - frame_end_time = frame_end_time + vsync_interval; - } - - // Useful knowledge for analyzing traces. - fml::TimePoint previous_vsync = next_vsync - vsync_interval; - TRACE_DURATION( - "flutter", "VsyncWaiter::GetTargetTimes", "previous_vsync(ms)", - previous_vsync.ToEpochDelta().ToMilliseconds(), "last_targetted(ms)", - last_targetted_vsync.ToEpochDelta().ToMilliseconds(), "now(ms)", - fml::TimePoint::Now().ToEpochDelta().ToMilliseconds(), "next_vsync(ms))", - next_vsync.ToEpochDelta().ToMilliseconds(), "frame_start_time(ms)", - frame_start_time.ToEpochDelta().ToMilliseconds(), "frame_end_time(ms)", - frame_end_time.ToEpochDelta().ToMilliseconds()); - - return {frame_start_time, frame_end_time}; -} - -VsyncWaiter::VsyncWaiter(std::shared_ptr session_connection, - flutter::TaskRunners task_runners, - fml::TimeDelta vsync_offset) +VsyncWaiter::VsyncWaiter( + AwaitVsyncCallback await_vsync, + AwaitVsyncForSecondaryCallbackCallback await_vsync_for_secondary_callback, + flutter::TaskRunners task_runners) : flutter::VsyncWaiter(task_runners), - session_connection_(session_connection), - vsync_offset_(vsync_offset), + await_vsync_(await_vsync), + await_vsync_for_secondary_callback_(await_vsync_for_secondary_callback), weak_factory_ui_(nullptr), weak_factory_(this) { - FML_CHECK(session_connection_); + fire_callback_callback_ = [this](fml::TimePoint frame_start, + fml::TimePoint frame_end) { + // Note: It is VERY important to set |pause_secondary_tasks| to false, else + // Animator will almost immediately crash on Fuchsia. + // FML_LOG(INFO) << "CRASH:: VsyncWaiter about to FireCallback"; + FireCallback(frame_start, frame_end, /*pause_secondary_tasks=*/false); + }; // Generate a WeakPtrFactory for use with the UI thread. This does not need // to wait on a latch because we only ever use the WeakPtrFactory on the UI @@ -168,6 +47,7 @@ VsyncWaiter::VsyncWaiter(std::shared_ptr session_connection, // Initialize the callback that will run every time the SessionConnection // receives one or more frame presented events on Vsync. + /* session_connection_->InitializeVsyncWaiterCallback([this]() { task_runners_.GetUITaskRunner()->PostTask([&weak_factory_ui = this->weak_factory_ui_] { @@ -184,8 +64,9 @@ VsyncWaiter::VsyncWaiter(std::shared_ptr session_connection, }); }); - FML_LOG(INFO) << "Flutter VsyncWaiter: Set vsync_offset to " - << vsync_offset_.ToMicroseconds() << "us"; + */ + + FML_LOG(INFO) << "CRASH: VsyncWaiter init"; } VsyncWaiter::~VsyncWaiter() { @@ -202,21 +83,25 @@ VsyncWaiter::~VsyncWaiter() { // This function is called when the Animator wishes to schedule a new frame. void VsyncWaiter::AwaitVSync() { - TRACE_DURATION("flutter", "VsyncWaiter::AwaitVsync()", - "request_already_pending", frame_request_pending_); - FireCallbackMaybe(); + // FML_LOG(INFO) << "CRASH: VsyncWaiter::AwaitVsync"; + await_vsync_(fire_callback_callback_); } // This function is called when the Animator wants to know about the next vsync, // but not for frame scheduling purposes. void VsyncWaiter::AwaitVSyncForSecondaryCallback() { - TRACE_DURATION("flutter", "VsyncWaiter::AwaitVsyncForSecondaryCallback()", - "request_already_pending", frame_request_pending_); - - FlutterFrameTimes times = GetTargetTimesHelper(/*secondary_callback=*/true); - FireCallback(times.frame_start, times.frame_target, false); + // FML_LOG(INFO) << "CRASH: VsyncWaiter::AwaitVsyncForSecondaryCallback"; + await_vsync_for_secondary_callback_(fire_callback_callback_); + // + // + // session_connection_->AwaitVsyncForSecondaryCallback(); + + // FlutterFrameTimes times = + // GetTargetTimesHelper(/*secondary_callback=*/true); + // FireCallback(times.frame_start, times.frame_target, false); } +/* // Postcondition: Either a frame is scheduled or frame_request_pending_ is set // to true, meaning we will attempt to schedule a frame on the next |OnVsync|. void VsyncWaiter::FireCallbackMaybe() { @@ -224,7 +109,7 @@ void VsyncWaiter::FireCallbackMaybe() { if (session_connection_->CanRequestNewFrames()) { FlutterFrameTimes times = - GetTargetTimesHelper(/*secondary_callback=*/false); + GetTargetTimesHelper(secondary_callback=false); last_targetted_vsync_ = times.frame_target; frame_request_pending_ = false; @@ -246,27 +131,6 @@ void VsyncWaiter::OnVsync() { FireCallbackMaybe(); } } - -// A helper function for GetTargetTimes(), since many of the fields it takes -// have to be derived from other state. -FlutterFrameTimes VsyncWaiter::GetTargetTimesHelper(bool secondary_callback) { - fml::TimeDelta presentation_interval = - VsyncRecorder::GetInstance().GetCurrentVsyncInfo().presentation_interval; - - fml::TimePoint next_vsync = - VsyncRecorder::GetInstance().GetCurrentVsyncInfo().presentation_time; - fml::TimePoint now = fml::TimePoint::Now(); - fml::TimePoint last_presentation_time = - VsyncRecorder::GetInstance().GetLastPresentationTime(); - if (next_vsync <= now) { - next_vsync = - SnapToNextPhase(now, last_presentation_time, presentation_interval); - } - - fml::TimePoint last_targetted_vsync = - secondary_callback ? fml::TimePoint::Min() : last_targetted_vsync_; - return GetTargetTimes(vsync_offset_, presentation_interval, - last_targetted_vsync, now, next_vsync); -} +*/ } // namespace flutter_runner diff --git a/shell/platform/fuchsia/flutter/vsync_waiter.h b/shell/platform/fuchsia/flutter/vsync_waiter.h index 79f76963d2c0a..f3ef5fd0a5c27 100644 --- a/shell/platform/fuchsia/flutter/vsync_waiter.h +++ b/shell/platform/fuchsia/flutter/vsync_waiter.h @@ -18,27 +18,21 @@ namespace flutter_runner { -struct FlutterFrameTimes { - fml::TimePoint frame_start; - fml::TimePoint frame_target; -}; +using FireCallbackCallback = + std::function; -class VsyncWaiter final : public flutter::VsyncWaiter { - public: - static FlutterFrameTimes GetTargetTimes(fml::TimeDelta vsync_offset, - fml::TimeDelta vsync_interval, - fml::TimePoint last_targetted_vsync, - fml::TimePoint now, - fml::TimePoint next_vsync); +using AwaitVsyncCallback = std::function; - static fml::TimePoint SnapToNextPhase( - const fml::TimePoint now, - const fml::TimePoint last_frame_presentation_time, - const fml::TimeDelta presentation_interval); +using AwaitVsyncForSecondaryCallbackCallback = + std::function; - VsyncWaiter(std::shared_ptr session_connection, - flutter::TaskRunners task_runners, - fml::TimeDelta vsync_offset); +class VsyncWaiter final : public flutter::VsyncWaiter { + public: + VsyncWaiter( + AwaitVsyncCallback await_vsync, + AwaitVsyncForSecondaryCallbackCallback await_vsync_for_secondary_callback, + flutter::TaskRunners task_runners); + // fml::TimeDelta vsync_offset); ~VsyncWaiter() override; @@ -49,25 +43,25 @@ class VsyncWaiter final : public flutter::VsyncWaiter { // |flutter::VsyncWaiter| void AwaitVSyncForSecondaryCallback() override; - void FireCallbackMaybe(); - void OnVsync(); - - FlutterFrameTimes GetTargetTimesHelper(bool secondary_callback); + // void FireCallbackMaybe(); + // void OnVsync(); + FireCallbackCallback fire_callback_callback_; - std::shared_ptr session_connection_; + AwaitVsyncCallback await_vsync_; + AwaitVsyncForSecondaryCallbackCallback await_vsync_for_secondary_callback_; // This is the offset from vsync that Flutter should wake up at to begin work // on the next frame. - fml::TimeDelta vsync_offset_; + // fml::TimeDelta vsync_offset_; // This is the last Vsync we submitted as the frame_target_time to // FireCallback(). This value should be strictly increasing in order to // guarantee that animation code that relies on target vsyncs works correctly, // and that Flutter is not producing multiple frames in a small interval. - fml::TimePoint last_targetted_vsync_; + // fml::TimePoint last_targetted_vsync_; // This is true iff AwaitVSync() was called but we could not schedule a frame. - bool frame_request_pending_ = false; + // bool frame_request_pending_ = false; std::unique_ptr> weak_factory_ui_; fml::WeakPtrFactory weak_factory_; From a4e0f5ecc12cee4698f7981ac546dd58ef5f26af Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Tue, 18 May 2021 17:03:34 -0400 Subject: [PATCH 03/16] add traces and clean up dead code --- .../flutter/default_session_connection.cc | 4 +- .../flutter/default_session_connection.h | 13 ---- .../platform/fuchsia/flutter/vsync_waiter.cc | 72 ++----------------- shell/platform/fuchsia/flutter/vsync_waiter.h | 24 ++----- 4 files changed, 13 insertions(+), 100 deletions(-) diff --git a/shell/platform/fuchsia/flutter/default_session_connection.cc b/shell/platform/fuchsia/flutter/default_session_connection.cc index d947d725924d0..e07dd64f82384 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.cc +++ b/shell/platform/fuchsia/flutter/default_session_connection.cc @@ -71,7 +71,7 @@ FlutterFrameTimes DefaultSessionConnection::GetTargetTimes( // Useful knowledge for analyzing traces. fml::TimePoint previous_vsync = next_vsync - vsync_interval; TRACE_DURATION( - "flutter", "VsyncWaiter::GetTargetTimes", "previous_vsync(ms)", + "flutter", "DefaultSessionConnection::GetTargetTimes", "previous_vsync(ms)", previous_vsync.ToEpochDelta().ToMilliseconds(), "last_targetted(ms)", last_targetted_vsync.ToEpochDelta().ToMilliseconds(), "now(ms)", fml::TimePoint::Now().ToEpochDelta().ToMilliseconds(), "next_vsync(ms))", @@ -328,6 +328,7 @@ void DefaultSessionConnection::PresentSession() { } void DefaultSessionConnection::AwaitVsync(FireCallbackCallback callback) { + TRACE_DURATION("flutter", "DefaultSessionConnection::AwaitVsync"); // FML_LOG(INFO) << "CRASH:: DSC::AwaitVsync"; fire_callback_ = callback; @@ -336,6 +337,7 @@ void DefaultSessionConnection::AwaitVsync(FireCallbackCallback callback) { void DefaultSessionConnection::AwaitVsyncForSecondaryCallback( FireCallbackCallback callback) { + TRACE_DURATION("flutter", "DefaultSessionConnection::AwaitVsyncForSecondaryCallback"); // FML_LOG(INFO) << "CRASH:: DSC::AwaitVsyncForSecondaryCallback"; fire_callback_ = callback; diff --git a/shell/platform/fuchsia/flutter/default_session_connection.h b/shell/platform/fuchsia/flutter/default_session_connection.h index 5b5c4e34db361..d93ef14152cc7 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.h +++ b/shell/platform/fuchsia/flutter/default_session_connection.h @@ -67,18 +67,7 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { // |SessionWrapper| void Present() override; - /* - // |SessionConnection| - void InitializeVsyncWaiterCallback(VsyncWaiterCallback callback) override; - - // |SessionConnection| - bool CanRequestNewFrames() override { - return frames_in_flight_ < kMaxFramesInFlight; - } - */ - void AwaitVsync(FireCallbackCallback callback); - void AwaitVsyncForSecondaryCallback(FireCallbackCallback callback); private: @@ -111,9 +100,7 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { fml::TimeDelta vsync_offset_; int frames_in_flight_ = 0; - int frames_in_flight_allowed_ = 0; - bool present_session_pending_ = false; // This is the last Vsync we submitted as the frame_target_time to diff --git a/shell/platform/fuchsia/flutter/vsync_waiter.cc b/shell/platform/fuchsia/flutter/vsync_waiter.cc index 097d6a87f5570..b2c5c3bfe178a 100644 --- a/shell/platform/fuchsia/flutter/vsync_waiter.cc +++ b/shell/platform/fuchsia/flutter/vsync_waiter.cc @@ -20,12 +20,12 @@ namespace flutter_runner { VsyncWaiter::VsyncWaiter( - AwaitVsyncCallback await_vsync, - AwaitVsyncForSecondaryCallbackCallback await_vsync_for_secondary_callback, + AwaitVsyncCallback await_vsync_callback, + AwaitVsyncForSecondaryCallbackCallback await_vsync_for_secondary_callback_callback, flutter::TaskRunners task_runners) : flutter::VsyncWaiter(task_runners), - await_vsync_(await_vsync), - await_vsync_for_secondary_callback_(await_vsync_for_secondary_callback), + await_vsync_callback_(await_vsync_callback), + await_vsync_for_secondary_callback_callback_(await_vsync_for_secondary_callback_callback), weak_factory_ui_(nullptr), weak_factory_(this) { fire_callback_callback_ = [this](fml::TimePoint frame_start, @@ -45,27 +45,6 @@ VsyncWaiter::VsyncWaiter( std::make_unique>(this); })); - // Initialize the callback that will run every time the SessionConnection - // receives one or more frame presented events on Vsync. - /* - session_connection_->InitializeVsyncWaiterCallback([this]() { - task_runners_.GetUITaskRunner()->PostTask([&weak_factory_ui = - this->weak_factory_ui_] { - if (!weak_factory_ui) { - FML_LOG(WARNING) << "WeakPtrFactory for VsyncWaiter is null, likely " - "due to the VsyncWaiter being destroyed."; - return; - } - - auto self = weak_factory_ui->GetWeakPtr(); - if (self) { - self->OnVsync(); - } - }); - }); - - */ - FML_LOG(INFO) << "CRASH: VsyncWaiter init"; } @@ -84,53 +63,14 @@ VsyncWaiter::~VsyncWaiter() { // This function is called when the Animator wishes to schedule a new frame. void VsyncWaiter::AwaitVSync() { // FML_LOG(INFO) << "CRASH: VsyncWaiter::AwaitVsync"; - await_vsync_(fire_callback_callback_); + await_vsync_callback_(fire_callback_callback_); } // This function is called when the Animator wants to know about the next vsync, // but not for frame scheduling purposes. void VsyncWaiter::AwaitVSyncForSecondaryCallback() { // FML_LOG(INFO) << "CRASH: VsyncWaiter::AwaitVsyncForSecondaryCallback"; - await_vsync_for_secondary_callback_(fire_callback_callback_); - // - // - // session_connection_->AwaitVsyncForSecondaryCallback(); - - // FlutterFrameTimes times = - // GetTargetTimesHelper(/*secondary_callback=*/true); - // FireCallback(times.frame_start, times.frame_target, false); -} - -/* -// Postcondition: Either a frame is scheduled or frame_request_pending_ is set -// to true, meaning we will attempt to schedule a frame on the next |OnVsync|. -void VsyncWaiter::FireCallbackMaybe() { - TRACE_DURATION("flutter", "FireCallbackMaybe"); - - if (session_connection_->CanRequestNewFrames()) { - FlutterFrameTimes times = - GetTargetTimesHelper(secondary_callback=false); - - last_targetted_vsync_ = times.frame_target; - frame_request_pending_ = false; - - FireCallback(times.frame_start, times.frame_target, false); - } else { - frame_request_pending_ = true; - } -} - -// This function is called when the SessionConnection signals us to let us know -// that one or more frames Flutter has produced have been displayed. Note that -// in practice this will be called several milliseconds after vsync, due to -// CPU contention. -void VsyncWaiter::OnVsync() { - TRACE_DURATION("flutter", "VsyncWaiter::OnVsync"); - - if (frame_request_pending_) { - FireCallbackMaybe(); - } + await_vsync_for_secondary_callback_callback_(fire_callback_callback_); } -*/ } // namespace flutter_runner diff --git a/shell/platform/fuchsia/flutter/vsync_waiter.h b/shell/platform/fuchsia/flutter/vsync_waiter.h index f3ef5fd0a5c27..896da0dbf3b83 100644 --- a/shell/platform/fuchsia/flutter/vsync_waiter.h +++ b/shell/platform/fuchsia/flutter/vsync_waiter.h @@ -29,10 +29,9 @@ using AwaitVsyncForSecondaryCallbackCallback = class VsyncWaiter final : public flutter::VsyncWaiter { public: VsyncWaiter( - AwaitVsyncCallback await_vsync, - AwaitVsyncForSecondaryCallbackCallback await_vsync_for_secondary_callback, + AwaitVsyncCallback await_vsync_callback, + AwaitVsyncForSecondaryCallbackCallback await_vsync_for_secondary_callback_callback, flutter::TaskRunners task_runners); - // fml::TimeDelta vsync_offset); ~VsyncWaiter() override; @@ -43,25 +42,10 @@ class VsyncWaiter final : public flutter::VsyncWaiter { // |flutter::VsyncWaiter| void AwaitVSyncForSecondaryCallback() override; - // void FireCallbackMaybe(); - // void OnVsync(); FireCallbackCallback fire_callback_callback_; - AwaitVsyncCallback await_vsync_; - AwaitVsyncForSecondaryCallbackCallback await_vsync_for_secondary_callback_; - - // This is the offset from vsync that Flutter should wake up at to begin work - // on the next frame. - // fml::TimeDelta vsync_offset_; - - // This is the last Vsync we submitted as the frame_target_time to - // FireCallback(). This value should be strictly increasing in order to - // guarantee that animation code that relies on target vsyncs works correctly, - // and that Flutter is not producing multiple frames in a small interval. - // fml::TimePoint last_targetted_vsync_; - - // This is true iff AwaitVSync() was called but we could not schedule a frame. - // bool frame_request_pending_ = false; + AwaitVsyncCallback await_vsync_callback_; + AwaitVsyncForSecondaryCallbackCallback await_vsync_for_secondary_callback_callback_; std::unique_ptr> weak_factory_ui_; fml::WeakPtrFactory weak_factory_; From 7c67c73010ddb0d92ecfa8ba00bba5da70ac7f4b Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Tue, 18 May 2021 17:27:13 -0400 Subject: [PATCH 04/16] add mutex and trace --- shell/common/animator.cc | 9 +++++++++ .../fuchsia/flutter/default_session_connection.cc | 3 +++ .../fuchsia/flutter/default_session_connection.h | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/shell/common/animator.cc b/shell/common/animator.cc index 41b5a44e60f7f..26c71a45877da 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -9,6 +9,7 @@ #include "flutter/fml/trace_event.h" #include "third_party/dart/runtime/include/dart_tools_api.h" +#include namespace flutter { namespace { @@ -102,6 +103,10 @@ void Animator::BeginFrame( std::unique_ptr frame_timings_recorder) { TRACE_EVENT_ASYNC_END0("flutter", "Frame Request Pending", frame_timings_recorder->GetFrameNumber()); + int num = frame_timings_recorder->GetFrameNumber(); + std::string numa = std::to_string(num); + auto numab = numa.c_str(); + TRACE_EVENT_INSTANT1("flutter", "TRACE_END", "number", numab); frame_timings_recorder_ = std::move(frame_timings_recorder); frame_timings_recorder_->RecordBuildStart(fml::TimePoint::Now()); @@ -255,6 +260,10 @@ void Animator::RequestFrame(bool regenerate_layer_tree) { return; } TRACE_EVENT_ASYNC_BEGIN0("flutter", "Frame Request Pending", frame_number); + int num = frame_number; + std::string numa = std::to_string(num); + auto numab = numa.c_str(); + TRACE_EVENT_INSTANT1("flutter", "TRACE_BEGIN", "number", numab); self->AwaitVSync(); }); frame_scheduled_ = true; diff --git a/shell/platform/fuchsia/flutter/default_session_connection.cc b/shell/platform/fuchsia/flutter/default_session_connection.cc index e07dd64f82384..58f0a18ca1a68 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.cc +++ b/shell/platform/fuchsia/flutter/default_session_connection.cc @@ -182,6 +182,7 @@ DefaultSessionConnection::DefaultSessionConnection( } if (fire_callback_request_pending_) { + std::lock_guard lock(mutex_); FireCallbackMaybe(); } } // callback @@ -328,6 +329,7 @@ void DefaultSessionConnection::PresentSession() { } void DefaultSessionConnection::AwaitVsync(FireCallbackCallback callback) { + std::lock_guard lock(mutex_); TRACE_DURATION("flutter", "DefaultSessionConnection::AwaitVsync"); // FML_LOG(INFO) << "CRASH:: DSC::AwaitVsync"; fire_callback_ = callback; @@ -337,6 +339,7 @@ void DefaultSessionConnection::AwaitVsync(FireCallbackCallback callback) { void DefaultSessionConnection::AwaitVsyncForSecondaryCallback( FireCallbackCallback callback) { + std::lock_guard lock(mutex_); TRACE_DURATION("flutter", "DefaultSessionConnection::AwaitVsyncForSecondaryCallback"); // FML_LOG(INFO) << "CRASH:: DSC::AwaitVsyncForSecondaryCallback"; fire_callback_ = callback; diff --git a/shell/platform/fuchsia/flutter/default_session_connection.h b/shell/platform/fuchsia/flutter/default_session_connection.h index d93ef14152cc7..0c4535ff3e5aa 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.h +++ b/shell/platform/fuchsia/flutter/default_session_connection.h @@ -17,6 +17,8 @@ #include "session_connection.h" #include "vsync_waiter.h" +#include + namespace flutter_runner { using on_frame_presented_event = @@ -103,6 +105,10 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { int frames_in_flight_allowed_ = 0; bool present_session_pending_ = false; + ////// Flutter Animator logic. + + std::mutex mutex_; + // This is the last Vsync we submitted as the frame_target_time to // FireCallback(). This value should be strictly increasing in order to // guarantee that animation code that relies on target vsyncs works correctly, From 401a086ea7fccc5b54d3892d9f37abd9f95dc5ec Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Tue, 18 May 2021 18:15:35 -0400 Subject: [PATCH 05/16] made locking easier to prove correct --- .../fuchsia/flutter/default_session_connection.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/shell/platform/fuchsia/flutter/default_session_connection.cc b/shell/platform/fuchsia/flutter/default_session_connection.cc index 58f0a18ca1a68..a51ff33ea4109 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.cc +++ b/shell/platform/fuchsia/flutter/default_session_connection.cc @@ -181,9 +181,11 @@ DefaultSessionConnection::DefaultSessionConnection( PresentSession(); } - if (fire_callback_request_pending_) { + { std::lock_guard lock(mutex_); - FireCallbackMaybe(); + if (fire_callback_request_pending_) { + FireCallbackMaybe(); + } } } // callback ); @@ -331,7 +333,6 @@ void DefaultSessionConnection::PresentSession() { void DefaultSessionConnection::AwaitVsync(FireCallbackCallback callback) { std::lock_guard lock(mutex_); TRACE_DURATION("flutter", "DefaultSessionConnection::AwaitVsync"); - // FML_LOG(INFO) << "CRASH:: DSC::AwaitVsync"; fire_callback_ = callback; FireCallbackMaybe(); @@ -341,7 +342,6 @@ void DefaultSessionConnection::AwaitVsyncForSecondaryCallback( FireCallbackCallback callback) { std::lock_guard lock(mutex_); TRACE_DURATION("flutter", "DefaultSessionConnection::AwaitVsyncForSecondaryCallback"); - // FML_LOG(INFO) << "CRASH:: DSC::AwaitVsyncForSecondaryCallback"; fire_callback_ = callback; FlutterFrameTimes times = GetTargetTimesHelper(/*secondary_callback=*/true); @@ -353,7 +353,6 @@ void DefaultSessionConnection::AwaitVsyncForSecondaryCallback( // |OnVsync|. void DefaultSessionConnection::FireCallbackMaybe() { TRACE_DURATION("flutter", "FireCallbackMaybe"); - // FML_LOG(INFO) << "CRASH:: DSC::FireCallbackMaybe"; if (frames_in_flight_ < kMaxFramesInFlight) { FlutterFrameTimes times = From 7fe1efd75b533311a76be3ee2e5a0fd96c49aa03 Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Tue, 18 May 2021 19:09:35 -0400 Subject: [PATCH 06/16] fix BUILD.gn tests remove session_connection.h remove vsync_waitr_unittests --- shell/common/animator.cc | 8 +- shell/platform/fuchsia/flutter/BUILD.gn | 6 +- .../flutter/default_session_connection.cc | 8 +- .../flutter/default_session_connection.h | 4 +- .../fuchsia/flutter/platform_view_unittest.cc | 24 +- .../fuchsia/flutter/session_connection.h | 24 -- .../default_session_connection_unittests.cc | 238 +++++++++++++++++- .../flutter/tests/fake_session_connection.h | 36 --- .../flutter/tests/vsync_waiter_unittests.cc | 200 --------------- .../platform/fuchsia/flutter/vsync_waiter.cc | 11 +- shell/platform/fuchsia/flutter/vsync_waiter.h | 13 +- 11 files changed, 263 insertions(+), 309 deletions(-) delete mode 100644 shell/platform/fuchsia/flutter/session_connection.h delete mode 100644 shell/platform/fuchsia/flutter/tests/fake_session_connection.h diff --git a/shell/common/animator.cc b/shell/common/animator.cc index 26c71a45877da..d2067217d75bd 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -260,10 +260,10 @@ void Animator::RequestFrame(bool regenerate_layer_tree) { return; } TRACE_EVENT_ASYNC_BEGIN0("flutter", "Frame Request Pending", frame_number); - int num = frame_number; - std::string numa = std::to_string(num); - auto numab = numa.c_str(); - TRACE_EVENT_INSTANT1("flutter", "TRACE_BEGIN", "number", numab); + int num = frame_number; + std::string numa = std::to_string(num); + auto numab = numa.c_str(); + TRACE_EVENT_INSTANT1("flutter", "TRACE_BEGIN", "number", numab); self->AwaitVSync(); }); frame_scheduled_ = true; diff --git a/shell/platform/fuchsia/flutter/BUILD.gn b/shell/platform/fuchsia/flutter/BUILD.gn index 94287ef01f23f..2fe04f0f1cde1 100644 --- a/shell/platform/fuchsia/flutter/BUILD.gn +++ b/shell/platform/fuchsia/flutter/BUILD.gn @@ -451,13 +451,11 @@ executable("flutter_runner_unittests") { "flutter_runner_fakes.h", "fuchsia_intl_unittest.cc", "keyboard_unittest.cc", - #"platform_view_unittest.cc", + "platform_view_unittest.cc", "runner_unittest.cc", "tests/engine_unittests.cc", - "tests/fake_session_connection.h", "tests/flutter_runner_product_configuration_unittests.cc", "tests/vsync_recorder_unittests.cc", - #"tests/vsync_waiter_unittests.cc", ] # This is needed for //third_party/googletest for linking zircon symbols. @@ -513,7 +511,7 @@ executable("flutter_runner_scenic_unittests") { output_name = "flutter_runner_scenic_tests" - sources = [ #"tests/default_session_connection_unittests.cc" + sources = [ "tests/default_session_connection_unittests.cc" ] # This is needed for //third_party/googletest for linking zircon symbols. diff --git a/shell/platform/fuchsia/flutter/default_session_connection.cc b/shell/platform/fuchsia/flutter/default_session_connection.cc index a51ff33ea4109..04b1c47b8bbba 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.cc +++ b/shell/platform/fuchsia/flutter/default_session_connection.cc @@ -71,8 +71,9 @@ FlutterFrameTimes DefaultSessionConnection::GetTargetTimes( // Useful knowledge for analyzing traces. fml::TimePoint previous_vsync = next_vsync - vsync_interval; TRACE_DURATION( - "flutter", "DefaultSessionConnection::GetTargetTimes", "previous_vsync(ms)", - previous_vsync.ToEpochDelta().ToMilliseconds(), "last_targetted(ms)", + "flutter", "DefaultSessionConnection::GetTargetTimes", + "previous_vsync(ms)", previous_vsync.ToEpochDelta().ToMilliseconds(), + "last_targetted(ms)", last_targetted_vsync.ToEpochDelta().ToMilliseconds(), "now(ms)", fml::TimePoint::Now().ToEpochDelta().ToMilliseconds(), "next_vsync(ms))", next_vsync.ToEpochDelta().ToMilliseconds(), "frame_start_time(ms)", @@ -341,7 +342,8 @@ void DefaultSessionConnection::AwaitVsync(FireCallbackCallback callback) { void DefaultSessionConnection::AwaitVsyncForSecondaryCallback( FireCallbackCallback callback) { std::lock_guard lock(mutex_); - TRACE_DURATION("flutter", "DefaultSessionConnection::AwaitVsyncForSecondaryCallback"); + TRACE_DURATION("flutter", + "DefaultSessionConnection::AwaitVsyncForSecondaryCallback"); fire_callback_ = callback; FlutterFrameTimes times = GetTargetTimesHelper(/*secondary_callback=*/true); diff --git a/shell/platform/fuchsia/flutter/default_session_connection.h b/shell/platform/fuchsia/flutter/default_session_connection.h index 0c4535ff3e5aa..856d4f43794e7 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.h +++ b/shell/platform/fuchsia/flutter/default_session_connection.h @@ -14,7 +14,6 @@ #include "flutter/fml/closure.h" #include "flutter/fml/macros.h" -#include "session_connection.h" #include "vsync_waiter.h" #include @@ -69,6 +68,7 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { // |SessionWrapper| void Present() override; + // Used to implement VsyncWaiter functionality. void AwaitVsync(FireCallbackCallback callback); void AwaitVsyncForSecondaryCallback(FireCallbackCallback callback); @@ -106,7 +106,7 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { bool present_session_pending_ = false; ////// Flutter Animator logic. - + std::mutex mutex_; // This is the last Vsync we submitted as the frame_target_time to diff --git a/shell/platform/fuchsia/flutter/platform_view_unittest.cc b/shell/platform/fuchsia/flutter/platform_view_unittest.cc index 9fa6e88356637..0da6735fa01e4 100644 --- a/shell/platform/fuchsia/flutter/platform_view_unittest.cc +++ b/shell/platform/fuchsia/flutter/platform_view_unittest.cc @@ -258,18 +258,18 @@ class PlatformViewBuilder { EXPECT_EQ(false, built_) << "Build() was already called, this buider is good for one use only."; built_ = true; - return PlatformView(delegate_, debug_label_, std::move(view_ref_), - task_runners_, runner_services_, - std::move(parent_environment_service_provider_), - std::move(session_listener_request_), - std::move(focuser_), std::move(keyboard_listener_), - std::move(on_session_listener_error_callback_), - std::move(wireframe_enabled_callback_), - std::move(on_create_view_callback_), - std::move(on_update_view_callback_), - std::move(on_destroy_view_callback_), - std::move(on_create_surface_callback_), view_embedder_, - std::move(vsync_offset_), nullptr); + return PlatformView( + delegate_, debug_label_, std::move(view_ref_), task_runners_, + runner_services_, std::move(parent_environment_service_provider_), + std::move(session_listener_request_), std::move(focuser_), + std::move(keyboard_listener_), + std::move(on_session_listener_error_callback_), + std::move(wireframe_enabled_callback_), + std::move(on_create_view_callback_), + std::move(on_update_view_callback_), + std::move(on_destroy_view_callback_), + std::move(on_create_surface_callback_), view_embedder_, [](auto...) {}, + [](auto...) {}); } private: diff --git a/shell/platform/fuchsia/flutter/session_connection.h b/shell/platform/fuchsia/flutter/session_connection.h deleted file mode 100644 index cd105d5ef513d..0000000000000 --- a/shell/platform/fuchsia/flutter/session_connection.h +++ /dev/null @@ -1,24 +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 FLUTTER_SHELL_PLATFORM_FUCHSIA_SESSION_CONNECTION_H_ -#define FLUTTER_SHELL_PLATFORM_FUCHSIA_SESSION_CONNECTION_H_ - -namespace flutter_runner { -using VsyncWaiterCallback = std::function; - -// The component residing on the raster thread that is responsible for -// maintaining the Scenic session connection and presenting node updates. -class SessionConnection { - public: - virtual ~SessionConnection() = default; - - virtual void InitializeVsyncWaiterCallback(VsyncWaiterCallback callback) = 0; - - virtual bool CanRequestNewFrames() = 0; -}; - -} // namespace flutter_runner - -#endif // FLUTTER_SHELL_PLATFORM_FUCHSIA_SESSION_CONNECTION_H_ diff --git a/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc b/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc index e5b078b1ddfb8..18dcaffb1063f 100644 --- a/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc +++ b/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc @@ -17,6 +17,16 @@ using namespace flutter_runner; namespace flutter_runner_test { +static fml::TimePoint TimePointFromInt(int i) { + return fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromNanoseconds(i)); +} +static fml::TimeDelta TimeDeltaFromInt(int i) { + return fml::TimeDelta::FromNanoseconds(i); +} +static int TimePointToInt(fml::TimePoint time) { + return time.ToEpochDelta().ToNanoseconds(); +} + class DefaultSessionConnectionTest : public ::testing::Test { public: void SetUp() override { @@ -68,7 +78,7 @@ TEST_F(DefaultSessionConnectionTest, SimplePresentTest) { flutter_runner::DefaultSessionConnection session_connection( "debug label", std::move(session_), on_session_error_callback, - on_frame_presented_callback, 3); + on_frame_presented_callback, 3, fml::TimeDelta::FromSeconds(0)); for (int i = 0; i < 200; ++i) { session_connection.Present(); @@ -90,7 +100,7 @@ TEST_F(DefaultSessionConnectionTest, BatchedPresentTest) { flutter_runner::DefaultSessionConnection session_connection( "debug label", std::move(session_), on_session_error_callback, - on_frame_presented_callback, 3); + on_frame_presented_callback, 3, fml::TimeDelta::FromSeconds(0)); for (int i = 0; i < 200; ++i) { session_connection.Present(); @@ -102,16 +112,6 @@ TEST_F(DefaultSessionConnectionTest, BatchedPresentTest) { EXPECT_GT(num_presents_handled, 0u); } -static fml::TimePoint TimePointFromInt(int i) { - return fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromNanoseconds(i)); -} -static fml::TimeDelta TimeDeltaFromInt(int i) { - return fml::TimeDelta::FromNanoseconds(i); -} -static int TimePointToInt(fml::TimePoint time) { - return time.ToEpochDelta().ToNanoseconds(); -} - // The first set of tests has an empty |future_presentation_infos| passed in. // Therefore these tests are to ensure that on startup and after not presenting // for some time that we have correct, reasonable behavior. @@ -469,4 +469,218 @@ TEST(CalculateNextLatchPointTest, SteadyState_FuzzyLatchPointsAfterTarget) { EXPECT_GE(TimePointToInt(calculated_latch_point), TimePointToInt(last_latch_point_targeted)); } + +TEST(SnapToNextPhaseTest, SnapOverlapsWithNow) { + const auto now = fml::TimePoint::Now(); + const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(10); + const auto delta = fml::TimeDelta::FromNanoseconds(10); + const auto next_vsync = + flutter_runner::DefaultSessionConnection::SnapToNextPhase( + now, last_presentation_time, delta); + + EXPECT_EQ(now + delta, next_vsync); +} + +TEST(SnapToNextPhaseTest, SnapAfterNow) { + const auto now = fml::TimePoint::Now(); + const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(9); + const auto delta = fml::TimeDelta::FromNanoseconds(10); + const auto next_vsync = + flutter_runner::DefaultSessionConnection::SnapToNextPhase( + now, last_presentation_time, delta); + + // math here: 10 - 9 = 1 + EXPECT_EQ(now + fml::TimeDelta::FromNanoseconds(1), next_vsync); +} + +TEST(SnapToNextPhaseTest, SnapAfterNowMultiJump) { + const auto now = fml::TimePoint::Now(); + const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(34); + const auto delta = fml::TimeDelta::FromNanoseconds(10); + const auto next_vsync = + flutter_runner::DefaultSessionConnection::SnapToNextPhase( + now, last_presentation_time, delta); + + // zeroes: -34, -24, -14, -4, 6, ... + EXPECT_EQ(now + fml::TimeDelta::FromNanoseconds(6), next_vsync); +} + +TEST(SnapToNextPhaseTest, SnapAfterNowMultiJumpAccountForCeils) { + const auto now = fml::TimePoint::Now(); + const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(20); + const auto delta = fml::TimeDelta::FromNanoseconds(16); + const auto next_vsync = + flutter_runner::DefaultSessionConnection::SnapToNextPhase( + now, last_presentation_time, delta); + + // zeroes: -20, -4, 12, 28, ... + EXPECT_EQ(now + fml::TimeDelta::FromNanoseconds(12), next_vsync); +} + +TEST(GetTargetTimesTest, ScheduleForNextVsync) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + const fml::TimePoint last_targetted_vsync = TimePointFromInt(10); + const fml::TimePoint now = TimePointFromInt(9); + const fml::TimePoint next_vsync = TimePointFromInt(10); + + const auto target_times = + flutter_runner::DefaultSessionConnection::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 10); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 20); +} + +TEST(GetTargetTimesTest, ScheduleForCurrentVsync_DueToOffset) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(3); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + const fml::TimePoint last_targetted_vsync = TimePointFromInt(0); + const fml::TimePoint now = TimePointFromInt(6); + const fml::TimePoint next_vsync = TimePointFromInt(10); + + const auto target_times = + flutter_runner::DefaultSessionConnection::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 7); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 10); +} + +TEST(GetTargetTimesTest, ScheduleForFollowingVsync_BecauseOfNow) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + const fml::TimePoint last_targetted_vsync = TimePointFromInt(10); + const fml::TimePoint now = TimePointFromInt(15); + const fml::TimePoint next_vsync = TimePointFromInt(10); + + const auto target_times = + flutter_runner::DefaultSessionConnection::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 20); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 30); +} + +TEST(GetTargetTimesTest, ScheduleForFollowingVsync_BecauseOfTargettedTime) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + const fml::TimePoint last_targetted_vsync = TimePointFromInt(20); + const fml::TimePoint now = TimePointFromInt(9); + const fml::TimePoint next_vsync = TimePointFromInt(10); + + const auto target_times = + flutter_runner::DefaultSessionConnection::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 20); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 30); +} + +TEST(GetTargetTimesTest, ScheduleForDistantVsync_BecauseOfTargettedTime) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + const fml::TimePoint last_targetted_vsync = TimePointFromInt(60); + const fml::TimePoint now = TimePointFromInt(9); + const fml::TimePoint next_vsync = TimePointFromInt(10); + + const auto target_times = + flutter_runner::DefaultSessionConnection::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 60); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 70); +} + +TEST(GetTargetTimesTest, ScheduleForFollowingVsync_WithSlightVsyncDrift) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + + // Even though it appears as if the next vsync is at time 40, we should still + // present at time 50. + const fml::TimePoint last_targetted_vsync = TimePointFromInt(37); + const fml::TimePoint now = TimePointFromInt(9); + const fml::TimePoint next_vsync = TimePointFromInt(10); + + const auto target_times = + flutter_runner::DefaultSessionConnection::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 40); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 50); +} + +TEST(GetTargetTimesTest, ScheduleForAnOffsetFromVsync) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(4); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + const fml::TimePoint last_targetted_vsync = TimePointFromInt(10); + const fml::TimePoint now = TimePointFromInt(9); + const fml::TimePoint next_vsync = TimePointFromInt(10); + + const auto target_times = + flutter_runner::DefaultSessionConnection::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 16); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 20); +} + +TEST(GetTargetTimesTest, ScheduleMultipleTimes) { + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + + fml::TimePoint last_targetted_vsync = TimePointFromInt(0); + fml::TimePoint now = TimePointFromInt(5); + fml::TimePoint next_vsync = TimePointFromInt(10); + + for (int i = 0; i < 100; ++i) { + const auto target_times = + flutter_runner::DefaultSessionConnection::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, + next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), 10 * (i + 1)); + EXPECT_EQ(TimePointToInt(target_times.frame_target), 10 * (i + 2)); + + // Simulate the passage of time. + now = now + vsync_interval; + next_vsync = next_vsync + vsync_interval; + last_targetted_vsync = target_times.frame_target; + } +} + +TEST(GetTargetTimesTest, ScheduleMultipleTimes_WithDelayedWakeups) { + // It is often the case that Flutter does not wake up when it intends to due + // to CPU contention. This test has DefaultSessionConnection wake up to + // schedule 0-4ns after when |now| should be - and we verify that the results + // should be the same as if there were no delay. + const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); + const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); + + fml::TimePoint last_targetted_vsync = TimePointFromInt(0); + fml::TimePoint now = TimePointFromInt(5); + fml::TimePoint next_vsync = TimePointFromInt(10); + + for (int i = 0; i < 100; ++i) { + const auto target_times = + flutter_runner::DefaultSessionConnection::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, now, + next_vsync); + + const auto target_times_delay = + flutter_runner::DefaultSessionConnection::GetTargetTimes( + vsync_offset, vsync_interval, last_targetted_vsync, + now + TimeDeltaFromInt(i % 5), next_vsync); + + EXPECT_EQ(TimePointToInt(target_times.frame_start), + TimePointToInt(target_times_delay.frame_start)); + EXPECT_EQ(TimePointToInt(target_times.frame_target), + TimePointToInt(target_times_delay.frame_target)); + + // Simulate the passage of time. + now = now + vsync_interval; + next_vsync = next_vsync + vsync_interval; + last_targetted_vsync = target_times.frame_target; + } +} } // namespace flutter_runner_test diff --git a/shell/platform/fuchsia/flutter/tests/fake_session_connection.h b/shell/platform/fuchsia/flutter/tests/fake_session_connection.h deleted file mode 100644 index 8854ca700e76f..0000000000000 --- a/shell/platform/fuchsia/flutter/tests/fake_session_connection.h +++ /dev/null @@ -1,36 +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 FLUTTER_SHELL_PLATFORM_FUCHSIA_TESTS_FAKE_SESSION_CONNECTION_H_ -#define FLUTTER_SHELL_PLATFORM_FUCHSIA_TESTS_FAKE_SESSION_CONNECTION_H_ - -namespace flutter_runner_test { - -#include "flutter/shell/platform/fuchsia/flutter/session_connection.h" - -class FakeSessionConnection : public flutter_runner::SessionConnection { - public: - FakeSessionConnection() {} - - void InitializeVsyncWaiterCallback( - flutter_runner::VsyncWaiterCallback callback) override { - callback_ = callback; - } - - bool CanRequestNewFrames() override { return can_request_new_frames_; } - - void FireVsyncWaiterCallback() { callback_(); } - - void SetCanRequestNewFrames(bool new_bool) { - can_request_new_frames_ = new_bool; - } - - private: - bool can_request_new_frames_ = true; - flutter_runner::VsyncWaiterCallback callback_; -}; - -} // namespace flutter_runner_test - -#endif // FLUTTER_SHELL_PLATFORM_FUCHSIA_TESTS_FAKE_SESSION_CONNECTION_H_ diff --git a/shell/platform/fuchsia/flutter/tests/vsync_waiter_unittests.cc b/shell/platform/fuchsia/flutter/tests/vsync_waiter_unittests.cc index e474246c25c16..cbf28d885ae31 100644 --- a/shell/platform/fuchsia/flutter/tests/vsync_waiter_unittests.cc +++ b/shell/platform/fuchsia/flutter/tests/vsync_waiter_unittests.cc @@ -18,8 +18,6 @@ #include "flutter/shell/platform/fuchsia/flutter/thread.h" #include "flutter/shell/platform/fuchsia/flutter/vsync_waiter.h" -#include "fake_session_connection.h" - namespace flutter_runner_test { static fml::TimePoint TimePointFromInt(int i) { @@ -139,202 +137,4 @@ TEST_F(VsyncWaiterTest, SteadyStateBehavior) { EXPECT_EQ(count, expected_iterations); } -TEST(SnapToNextPhaseTest, SnapOverlapsWithNow) { - const auto now = fml::TimePoint::Now(); - const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(10); - const auto delta = fml::TimeDelta::FromNanoseconds(10); - const auto next_vsync = flutter_runner::VsyncWaiter::SnapToNextPhase( - now, last_presentation_time, delta); - - EXPECT_EQ(now + delta, next_vsync); -} - -TEST(SnapToNextPhaseTest, SnapAfterNow) { - const auto now = fml::TimePoint::Now(); - const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(9); - const auto delta = fml::TimeDelta::FromNanoseconds(10); - const auto next_vsync = flutter_runner::VsyncWaiter::SnapToNextPhase( - now, last_presentation_time, delta); - - // math here: 10 - 9 = 1 - EXPECT_EQ(now + fml::TimeDelta::FromNanoseconds(1), next_vsync); -} - -TEST(SnapToNextPhaseTest, SnapAfterNowMultiJump) { - const auto now = fml::TimePoint::Now(); - const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(34); - const auto delta = fml::TimeDelta::FromNanoseconds(10); - const auto next_vsync = flutter_runner::VsyncWaiter::SnapToNextPhase( - now, last_presentation_time, delta); - - // zeroes: -34, -24, -14, -4, 6, ... - EXPECT_EQ(now + fml::TimeDelta::FromNanoseconds(6), next_vsync); -} - -TEST(SnapToNextPhaseTest, SnapAfterNowMultiJumpAccountForCeils) { - const auto now = fml::TimePoint::Now(); - const auto last_presentation_time = now - fml::TimeDelta::FromNanoseconds(20); - const auto delta = fml::TimeDelta::FromNanoseconds(16); - const auto next_vsync = flutter_runner::VsyncWaiter::SnapToNextPhase( - now, last_presentation_time, delta); - - // zeroes: -20, -4, 12, 28, ... - EXPECT_EQ(now + fml::TimeDelta::FromNanoseconds(12), next_vsync); -} - -TEST(GetTargetTimesTest, ScheduleForNextVsync) { - const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); - const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - const fml::TimePoint last_targetted_vsync = TimePointFromInt(10); - const fml::TimePoint now = TimePointFromInt(9); - const fml::TimePoint next_vsync = TimePointFromInt(10); - - const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); - - EXPECT_EQ(TimePointToInt(target_times.frame_start), 10); - EXPECT_EQ(TimePointToInt(target_times.frame_target), 20); -} - -TEST(GetTargetTimesTest, ScheduleForCurrentVsync_DueToOffset) { - const fml::TimeDelta vsync_offset = TimeDeltaFromInt(3); - const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - const fml::TimePoint last_targetted_vsync = TimePointFromInt(0); - const fml::TimePoint now = TimePointFromInt(6); - const fml::TimePoint next_vsync = TimePointFromInt(10); - - const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); - - EXPECT_EQ(TimePointToInt(target_times.frame_start), 7); - EXPECT_EQ(TimePointToInt(target_times.frame_target), 10); -} - -TEST(GetTargetTimesTest, ScheduleForFollowingVsync_BecauseOfNow) { - const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); - const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - const fml::TimePoint last_targetted_vsync = TimePointFromInt(10); - const fml::TimePoint now = TimePointFromInt(15); - const fml::TimePoint next_vsync = TimePointFromInt(10); - - const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); - - EXPECT_EQ(TimePointToInt(target_times.frame_start), 20); - EXPECT_EQ(TimePointToInt(target_times.frame_target), 30); -} - -TEST(GetTargetTimesTest, ScheduleForFollowingVsync_BecauseOfTargettedTime) { - const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); - const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - const fml::TimePoint last_targetted_vsync = TimePointFromInt(20); - const fml::TimePoint now = TimePointFromInt(9); - const fml::TimePoint next_vsync = TimePointFromInt(10); - - const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); - - EXPECT_EQ(TimePointToInt(target_times.frame_start), 20); - EXPECT_EQ(TimePointToInt(target_times.frame_target), 30); -} - -TEST(GetTargetTimesTest, ScheduleForDistantVsync_BecauseOfTargettedTime) { - const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); - const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - const fml::TimePoint last_targetted_vsync = TimePointFromInt(60); - const fml::TimePoint now = TimePointFromInt(9); - const fml::TimePoint next_vsync = TimePointFromInt(10); - - const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); - - EXPECT_EQ(TimePointToInt(target_times.frame_start), 60); - EXPECT_EQ(TimePointToInt(target_times.frame_target), 70); -} - -TEST(GetTargetTimesTest, ScheduleForFollowingVsync_WithSlightVsyncDrift) { - const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); - const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - - // Even though it appears as if the next vsync is at time 40, we should still - // present at time 50. - const fml::TimePoint last_targetted_vsync = TimePointFromInt(37); - const fml::TimePoint now = TimePointFromInt(9); - const fml::TimePoint next_vsync = TimePointFromInt(10); - - const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); - - EXPECT_EQ(TimePointToInt(target_times.frame_start), 40); - EXPECT_EQ(TimePointToInt(target_times.frame_target), 50); -} - -TEST(GetTargetTimesTest, ScheduleForAnOffsetFromVsync) { - const fml::TimeDelta vsync_offset = TimeDeltaFromInt(4); - const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - const fml::TimePoint last_targetted_vsync = TimePointFromInt(10); - const fml::TimePoint now = TimePointFromInt(9); - const fml::TimePoint next_vsync = TimePointFromInt(10); - - const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); - - EXPECT_EQ(TimePointToInt(target_times.frame_start), 16); - EXPECT_EQ(TimePointToInt(target_times.frame_target), 20); -} - -TEST(GetTargetTimesTest, ScheduleMultipleTimes) { - const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); - const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - - fml::TimePoint last_targetted_vsync = TimePointFromInt(0); - fml::TimePoint now = TimePointFromInt(5); - fml::TimePoint next_vsync = TimePointFromInt(10); - - for (int i = 0; i < 100; ++i) { - const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); - - EXPECT_EQ(TimePointToInt(target_times.frame_start), 10 * (i + 1)); - EXPECT_EQ(TimePointToInt(target_times.frame_target), 10 * (i + 2)); - - // Simulate the passage of time. - now = now + vsync_interval; - next_vsync = next_vsync + vsync_interval; - last_targetted_vsync = target_times.frame_target; - } -} - -TEST(GetTargetTimesTest, ScheduleMultipleTimes_WithDelayedWakeups) { - // It is often the case that Flutter does not wake up when it intends to due - // to CPU contention. This test has VsyncWaiter wake up to schedule 0-4ns - // after when |now| should be - and we verify that the results should be the - // same as if there were no delay. - const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); - const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - - fml::TimePoint last_targetted_vsync = TimePointFromInt(0); - fml::TimePoint now = TimePointFromInt(5); - fml::TimePoint next_vsync = TimePointFromInt(10); - - for (int i = 0; i < 100; ++i) { - const auto target_times = flutter_runner::VsyncWaiter::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); - - const auto target_times_delay = flutter_runner::VsyncWaiter::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, - now + TimeDeltaFromInt(i % 5), next_vsync); - - EXPECT_EQ(TimePointToInt(target_times.frame_start), - TimePointToInt(target_times_delay.frame_start)); - EXPECT_EQ(TimePointToInt(target_times.frame_target), - TimePointToInt(target_times_delay.frame_target)); - - // Simulate the passage of time. - now = now + vsync_interval; - next_vsync = next_vsync + vsync_interval; - last_targetted_vsync = target_times.frame_target; - } -} - } // namespace flutter_runner_test diff --git a/shell/platform/fuchsia/flutter/vsync_waiter.cc b/shell/platform/fuchsia/flutter/vsync_waiter.cc index b2c5c3bfe178a..a36124fb4f20a 100644 --- a/shell/platform/fuchsia/flutter/vsync_waiter.cc +++ b/shell/platform/fuchsia/flutter/vsync_waiter.cc @@ -19,13 +19,14 @@ namespace flutter_runner { -VsyncWaiter::VsyncWaiter( - AwaitVsyncCallback await_vsync_callback, - AwaitVsyncForSecondaryCallbackCallback await_vsync_for_secondary_callback_callback, - flutter::TaskRunners task_runners) +VsyncWaiter::VsyncWaiter(AwaitVsyncCallback await_vsync_callback, + AwaitVsyncForSecondaryCallbackCallback + await_vsync_for_secondary_callback_callback, + flutter::TaskRunners task_runners) : flutter::VsyncWaiter(task_runners), await_vsync_callback_(await_vsync_callback), - await_vsync_for_secondary_callback_callback_(await_vsync_for_secondary_callback_callback), + await_vsync_for_secondary_callback_callback_( + await_vsync_for_secondary_callback_callback), weak_factory_ui_(nullptr), weak_factory_(this) { fire_callback_callback_ = [this](fml::TimePoint frame_start, diff --git a/shell/platform/fuchsia/flutter/vsync_waiter.h b/shell/platform/fuchsia/flutter/vsync_waiter.h index 896da0dbf3b83..1e92064c8735b 100644 --- a/shell/platform/fuchsia/flutter/vsync_waiter.h +++ b/shell/platform/fuchsia/flutter/vsync_waiter.h @@ -14,8 +14,6 @@ #include "flutter/shell/common/vsync_waiter.h" #include "flutter_runner_product_configuration.h" -#include "session_connection.h" - namespace flutter_runner { using FireCallbackCallback = @@ -28,10 +26,10 @@ using AwaitVsyncForSecondaryCallbackCallback = class VsyncWaiter final : public flutter::VsyncWaiter { public: - VsyncWaiter( - AwaitVsyncCallback await_vsync_callback, - AwaitVsyncForSecondaryCallbackCallback await_vsync_for_secondary_callback_callback, - flutter::TaskRunners task_runners); + VsyncWaiter(AwaitVsyncCallback await_vsync_callback, + AwaitVsyncForSecondaryCallbackCallback + await_vsync_for_secondary_callback_callback, + flutter::TaskRunners task_runners); ~VsyncWaiter() override; @@ -45,7 +43,8 @@ class VsyncWaiter final : public flutter::VsyncWaiter { FireCallbackCallback fire_callback_callback_; AwaitVsyncCallback await_vsync_callback_; - AwaitVsyncForSecondaryCallbackCallback await_vsync_for_secondary_callback_callback_; + AwaitVsyncForSecondaryCallbackCallback + await_vsync_for_secondary_callback_callback_; std::unique_ptr> weak_factory_ui_; fml::WeakPtrFactory weak_factory_; From 7eef75934f88818fe8866cd62d0ea7e6d49e7e68 Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Tue, 18 May 2021 19:15:02 -0400 Subject: [PATCH 07/16] fix BUILD.gn formatting and DSC method order --- shell/platform/fuchsia/flutter/BUILD.gn | 3 +- .../flutter/default_session_connection.cc | 85 +++++++++---------- .../flutter/default_session_connection.h | 23 ++--- 3 files changed, 55 insertions(+), 56 deletions(-) diff --git a/shell/platform/fuchsia/flutter/BUILD.gn b/shell/platform/fuchsia/flutter/BUILD.gn index 2fe04f0f1cde1..549e353cfaf4a 100644 --- a/shell/platform/fuchsia/flutter/BUILD.gn +++ b/shell/platform/fuchsia/flutter/BUILD.gn @@ -511,8 +511,7 @@ executable("flutter_runner_scenic_unittests") { output_name = "flutter_runner_scenic_tests" - sources = [ "tests/default_session_connection_unittests.cc" -] + sources = [ "tests/default_session_connection_unittests.cc" ] # This is needed for //third_party/googletest for linking zircon symbols. libs = [ "//fuchsia/sdk/$host_os/arch/$target_cpu/sysroot/lib/libzircon.so" ] diff --git a/shell/platform/fuchsia/flutter/default_session_connection.cc b/shell/platform/fuchsia/flutter/default_session_connection.cc index 04b1c47b8bbba..9b9a8cf4519e2 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.cc +++ b/shell/platform/fuchsia/flutter/default_session_connection.cc @@ -83,6 +83,34 @@ FlutterFrameTimes DefaultSessionConnection::GetTargetTimes( return {frame_start_time, frame_end_time}; } +fml::TimePoint DefaultSessionConnection::CalculateNextLatchPoint( + fml::TimePoint present_requested_time, + fml::TimePoint now, + fml::TimePoint last_latch_point_targeted, + fml::TimeDelta flutter_frame_build_time, + fml::TimeDelta vsync_interval, + std::deque>& + future_presentation_infos) { + // The minimum latch point is the largest of: + // Now + // When we expect the Flutter work for the frame to be completed + // The last latch point targeted + fml::TimePoint minimum_latch_point_to_target = + std::max(std::max(now, present_requested_time + flutter_frame_build_time), + last_latch_point_targeted); + + for (auto& info : future_presentation_infos) { + fml::TimePoint latch_point = info.first; + + if (latch_point >= minimum_latch_point_to_target) { + return latch_point; + } + } + + // We could not find a suitable latch point in the list given to us from + // Scenic, so aim for the smallest safe value. + return minimum_latch_point_to_target; +} /// Returns the system time at which the next frame is likely to be presented. /// /// Consider the following scenarios, where in both the @@ -239,33 +267,23 @@ void DefaultSessionConnection::Present() { } } -fml::TimePoint DefaultSessionConnection::CalculateNextLatchPoint( - fml::TimePoint present_requested_time, - fml::TimePoint now, - fml::TimePoint last_latch_point_targeted, - fml::TimeDelta flutter_frame_build_time, - fml::TimeDelta vsync_interval, - std::deque>& - future_presentation_infos) { - // The minimum latch point is the largest of: - // Now - // When we expect the Flutter work for the frame to be completed - // The last latch point targeted - fml::TimePoint minimum_latch_point_to_target = - std::max(std::max(now, present_requested_time + flutter_frame_build_time), - last_latch_point_targeted); +void DefaultSessionConnection::AwaitVsync(FireCallbackCallback callback) { + std::lock_guard lock(mutex_); + TRACE_DURATION("flutter", "DefaultSessionConnection::AwaitVsync"); + fire_callback_ = callback; - for (auto& info : future_presentation_infos) { - fml::TimePoint latch_point = info.first; + FireCallbackMaybe(); +} - if (latch_point >= minimum_latch_point_to_target) { - return latch_point; - } - } +void DefaultSessionConnection::AwaitVsyncForSecondaryCallback( + FireCallbackCallback callback) { + std::lock_guard lock(mutex_); + TRACE_DURATION("flutter", + "DefaultSessionConnection::AwaitVsyncForSecondaryCallback"); + fire_callback_ = callback; - // We could not find a suitable latch point in the list given to us from - // Scenic, so aim for the smallest safe value. - return minimum_latch_point_to_target; + FlutterFrameTimes times = GetTargetTimesHelper(/*secondary_callback=*/true); + fire_callback_(times.frame_start, times.frame_target); } void DefaultSessionConnection::PresentSession() { @@ -331,25 +349,6 @@ void DefaultSessionConnection::PresentSession() { }); } -void DefaultSessionConnection::AwaitVsync(FireCallbackCallback callback) { - std::lock_guard lock(mutex_); - TRACE_DURATION("flutter", "DefaultSessionConnection::AwaitVsync"); - fire_callback_ = callback; - - FireCallbackMaybe(); -} - -void DefaultSessionConnection::AwaitVsyncForSecondaryCallback( - FireCallbackCallback callback) { - std::lock_guard lock(mutex_); - TRACE_DURATION("flutter", - "DefaultSessionConnection::AwaitVsyncForSecondaryCallback"); - fire_callback_ = callback; - - FlutterFrameTimes times = GetTargetTimesHelper(/*secondary_callback=*/true); - fire_callback_(times.frame_start, times.frame_target); -} - // Postcondition: Either a frame is scheduled or fire_callback_request_pending_ // is set to true, meaning we will attempt to schedule a frame on the next // |OnVsync|. diff --git a/shell/platform/fuchsia/flutter/default_session_connection.h b/shell/platform/fuchsia/flutter/default_session_connection.h index 856d4f43794e7..4553f1f54556d 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.h +++ b/shell/platform/fuchsia/flutter/default_session_connection.h @@ -32,6 +32,12 @@ struct FlutterFrameTimes { // maintaining the Scenic session connection and presenting node updates. class DefaultSessionConnection final : public flutter::SessionWrapper { public: + static FlutterFrameTimes GetTargetTimes(fml::TimeDelta vsync_offset, + fml::TimeDelta vsync_interval, + fml::TimePoint last_targetted_vsync, + fml::TimePoint now, + fml::TimePoint next_vsync); + static fml::TimePoint CalculateNextLatchPoint( fml::TimePoint present_requested_time, fml::TimePoint now, @@ -41,12 +47,6 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { std::deque>& future_presentation_infos); - static FlutterFrameTimes GetTargetTimes(fml::TimeDelta vsync_offset, - fml::TimeDelta vsync_interval, - fml::TimePoint last_targetted_vsync, - fml::TimePoint now, - fml::TimePoint next_vsync); - static fml::TimePoint SnapToNextPhase( const fml::TimePoint now, const fml::TimePoint last_frame_presentation_time, @@ -73,6 +73,12 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { void AwaitVsyncForSecondaryCallback(FireCallbackCallback callback); private: + void PresentSession(); + + void FireCallbackMaybe(); + + FlutterFrameTimes GetTargetTimesHelper(bool secondary_callback); + scenic::Session session_wrapper_; on_frame_presented_event on_frame_presented_callback_; @@ -120,11 +126,6 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { FireCallbackCallback fire_callback_; - void PresentSession(); - - void FireCallbackMaybe(); - FlutterFrameTimes GetTargetTimesHelper(bool secondary_callback); - FML_DISALLOW_COPY_AND_ASSIGN(DefaultSessionConnection); }; From 32c20965a62ea590c43e0f7084f4337ec35f8ef7 Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Tue, 18 May 2021 19:54:35 -0400 Subject: [PATCH 08/16] cleanup licenses, animator, vsync_waiter --- ci/licenses_golden/licenses_flutter | 1 - shell/common/animator.cc | 9 --------- shell/platform/fuchsia/flutter/vsync_waiter.cc | 7 ------- 3 files changed, 17 deletions(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 9fc5d26858e5e..2bcf7252fd226 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -1360,7 +1360,6 @@ FILE: ../../../flutter/shell/platform/fuchsia/flutter/runner.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/runner.h FILE: ../../../flutter/shell/platform/fuchsia/flutter/runner_tzdata_unittest.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/runner_unittest.cc -FILE: ../../../flutter/shell/platform/fuchsia/flutter/session_connection.h FILE: ../../../flutter/shell/platform/fuchsia/flutter/surface.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/surface.h FILE: ../../../flutter/shell/platform/fuchsia/flutter/task_observers.cc diff --git a/shell/common/animator.cc b/shell/common/animator.cc index d2067217d75bd..41b5a44e60f7f 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -9,7 +9,6 @@ #include "flutter/fml/trace_event.h" #include "third_party/dart/runtime/include/dart_tools_api.h" -#include namespace flutter { namespace { @@ -103,10 +102,6 @@ void Animator::BeginFrame( std::unique_ptr frame_timings_recorder) { TRACE_EVENT_ASYNC_END0("flutter", "Frame Request Pending", frame_timings_recorder->GetFrameNumber()); - int num = frame_timings_recorder->GetFrameNumber(); - std::string numa = std::to_string(num); - auto numab = numa.c_str(); - TRACE_EVENT_INSTANT1("flutter", "TRACE_END", "number", numab); frame_timings_recorder_ = std::move(frame_timings_recorder); frame_timings_recorder_->RecordBuildStart(fml::TimePoint::Now()); @@ -260,10 +255,6 @@ void Animator::RequestFrame(bool regenerate_layer_tree) { return; } TRACE_EVENT_ASYNC_BEGIN0("flutter", "Frame Request Pending", frame_number); - int num = frame_number; - std::string numa = std::to_string(num); - auto numab = numa.c_str(); - TRACE_EVENT_INSTANT1("flutter", "TRACE_BEGIN", "number", numab); self->AwaitVSync(); }); frame_scheduled_ = true; diff --git a/shell/platform/fuchsia/flutter/vsync_waiter.cc b/shell/platform/fuchsia/flutter/vsync_waiter.cc index a36124fb4f20a..ccc0d16d7f729 100644 --- a/shell/platform/fuchsia/flutter/vsync_waiter.cc +++ b/shell/platform/fuchsia/flutter/vsync_waiter.cc @@ -45,8 +45,6 @@ VsyncWaiter::VsyncWaiter(AwaitVsyncCallback await_vsync_callback, this->weak_factory_ui_ = std::make_unique>(this); })); - - FML_LOG(INFO) << "CRASH: VsyncWaiter init"; } VsyncWaiter::~VsyncWaiter() { @@ -61,16 +59,11 @@ VsyncWaiter::~VsyncWaiter() { ui_latch.Wait(); } -// This function is called when the Animator wishes to schedule a new frame. void VsyncWaiter::AwaitVSync() { - // FML_LOG(INFO) << "CRASH: VsyncWaiter::AwaitVsync"; await_vsync_callback_(fire_callback_callback_); } -// This function is called when the Animator wants to know about the next vsync, -// but not for frame scheduling purposes. void VsyncWaiter::AwaitVSyncForSecondaryCallback() { - // FML_LOG(INFO) << "CRASH: VsyncWaiter::AwaitVsyncForSecondaryCallback"; await_vsync_for_secondary_callback_callback_(fire_callback_callback_); } From 682b744b69c3a3f96b23ef5bc2454d058d24ec3c Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Tue, 18 May 2021 19:55:18 -0400 Subject: [PATCH 09/16] delete vsync_waiter_unittests --- .../flutter/tests/vsync_waiter_unittests.cc | 140 ------------------ 1 file changed, 140 deletions(-) delete mode 100644 shell/platform/fuchsia/flutter/tests/vsync_waiter_unittests.cc diff --git a/shell/platform/fuchsia/flutter/tests/vsync_waiter_unittests.cc b/shell/platform/fuchsia/flutter/tests/vsync_waiter_unittests.cc deleted file mode 100644 index cbf28d885ae31..0000000000000 --- a/shell/platform/fuchsia/flutter/tests/vsync_waiter_unittests.cc +++ /dev/null @@ -1,140 +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 -#include -#include -#include - -#include - -#include "flutter/fml/time/time_delta.h" -#include "flutter/fml/time/time_point.h" -#include "flutter/shell/common/thread_host.h" -#include "flutter/shell/common/vsync_waiter.h" -#include "flutter/shell/platform/fuchsia/flutter/flutter_runner_product_configuration.h" -#include "flutter/shell/platform/fuchsia/flutter/task_runner_adapter.h" -#include "flutter/shell/platform/fuchsia/flutter/thread.h" -#include "flutter/shell/platform/fuchsia/flutter/vsync_waiter.h" - -namespace flutter_runner_test { - -static fml::TimePoint TimePointFromInt(int i) { - return fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromNanoseconds(i)); -} -static fml::TimeDelta TimeDeltaFromInt(int i) { - return fml::TimeDelta::FromNanoseconds(i); -} -static int TimePointToInt(fml::TimePoint time) { - return time.ToEpochDelta().ToNanoseconds(); -} - -class VsyncWaiterTest : public testing::Test { - public: - VsyncWaiterTest() { - for (auto& thread : threads_) { - thread.reset(new flutter_runner::Thread()); - } - - const flutter::TaskRunners task_runners( - "VsyncWaiterTests", // Dart thread labels - flutter_runner::CreateFMLTaskRunner( - async_get_default_dispatcher()), // platform - flutter_runner::CreateFMLTaskRunner( - threads_[0]->dispatcher()), // raster - flutter_runner::CreateFMLTaskRunner(threads_[1]->dispatcher()), // ui - flutter_runner::CreateFMLTaskRunner(threads_[2]->dispatcher()) // io - ); - - session_connection_ = std::make_shared(); - vsync_waiter_ = std::make_unique( - session_connection_, task_runners, fml::TimeDelta::Zero()); - } - - flutter_runner::VsyncWaiter* get_vsync_waiter() { - return vsync_waiter_.get(); - } - FakeSessionConnection* get_session_connection() { - return session_connection_.get(); - } - - ~VsyncWaiterTest() { - vsync_waiter_.reset(); - for (const auto& thread : threads_) { - thread->Quit(); - } - } - - private: - async::Loop loop_ = async::Loop(&kAsyncLoopConfigAttachToCurrentThread); - std::shared_ptr session_connection_; - std::unique_ptr vsync_waiter_; - std::array, 3> threads_; -}; - -TEST_F(VsyncWaiterTest, SimpleVsyncCallback) { - auto vsync_waiter = get_vsync_waiter(); - auto session_connection = get_session_connection(); - - bool flag = false; - vsync_waiter->AsyncWaitForVsync([&flag](auto) { flag = true; }); - - session_connection->FireVsyncWaiterCallback(); - - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - - EXPECT_TRUE(flag); -} - -TEST_F(VsyncWaiterTest, EnsureBackPressure) { - auto vsync_waiter = get_vsync_waiter(); - auto session_connection = get_session_connection(); - - session_connection->SetCanRequestNewFrames(false); - bool flag = false; - vsync_waiter->AsyncWaitForVsync([&flag](auto) { flag = true; }); - - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - - EXPECT_FALSE(flag); -} - -TEST_F(VsyncWaiterTest, BackPressureRelief) { - auto vsync_waiter = get_vsync_waiter(); - auto session_connection = get_session_connection(); - - session_connection->SetCanRequestNewFrames(false); - bool flag = false; - vsync_waiter->AsyncWaitForVsync([&flag](auto) { flag = true; }); - - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - - EXPECT_FALSE(flag); - - session_connection->SetCanRequestNewFrames(true); - session_connection->FireVsyncWaiterCallback(); - - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - - EXPECT_TRUE(flag); -} - -TEST_F(VsyncWaiterTest, SteadyStateBehavior) { - auto vsync_waiter = get_vsync_waiter(); - auto session_connection = get_session_connection(); - - session_connection->SetCanRequestNewFrames(true); - - int count = 0; - const int expected_iterations = 100; - - for (int i = 0; i < expected_iterations; ++i) { - vsync_waiter->AsyncWaitForVsync([&count](auto) { count++; }); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - } - - EXPECT_EQ(count, expected_iterations); -} - -} // namespace flutter_runner_test From d3f740e1fed2c5daef2f597f4f0b5dbece2e6035 Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Wed, 19 May 2021 15:57:00 -0400 Subject: [PATCH 10/16] add defaultsessionconnection tests --- .../default_session_connection_unittests.cc | 124 +++++++++++++++++- 1 file changed, 122 insertions(+), 2 deletions(-) diff --git a/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc b/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc index 18dcaffb1063f..e06577461cca0 100644 --- a/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc +++ b/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc @@ -66,7 +66,7 @@ class DefaultSessionConnectionTest : public ::testing::Test { thrd_t fidl_thread_; }; -TEST_F(DefaultSessionConnectionTest, SimplePresentTest) { +TEST_F(DefaultSessionConnectionTest, SimplePresent) { fml::closure on_session_error_callback = []() { FML_CHECK(false); }; uint64_t num_presents_handled = 0; @@ -88,7 +88,7 @@ TEST_F(DefaultSessionConnectionTest, SimplePresentTest) { EXPECT_GT(num_presents_handled, 0u); } -TEST_F(DefaultSessionConnectionTest, BatchedPresentTest) { +TEST_F(DefaultSessionConnectionTest, BatchedPresent) { fml::closure on_session_error_callback = []() { FML_CHECK(false); }; uint64_t num_presents_handled = 0; @@ -112,6 +112,126 @@ TEST_F(DefaultSessionConnectionTest, BatchedPresentTest) { EXPECT_GT(num_presents_handled, 0u); } +TEST_F(DefaultSessionConnectionTest, AwaitVsync) { + fml::closure on_session_error_callback = []() { FML_CHECK(false); }; + + uint64_t num_presents_handled = 0; + on_frame_presented_event on_frame_presented_callback = + [&num_presents_handled]( + fuchsia::scenic::scheduling::FramePresentedInfo info) { + num_presents_handled += info.presentation_infos.size(); + }; + + flutter_runner::DefaultSessionConnection session_connection( + "debug label", std::move(session_), on_session_error_callback, + on_frame_presented_callback, 3, fml::TimeDelta::FromSeconds(0)); + + uint64_t await_vsyncs_handled = 0; + + for (int i = 0; i < 5; ++i) { + session_connection.Present(); + session_connection.AwaitVsync( + [&await_vsyncs_handled](auto...) { await_vsyncs_handled++; }); + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + } + + EXPECT_GT(num_presents_handled, 0u); + EXPECT_GT(await_vsyncs_handled, 0u); +} + +TEST_F(DefaultSessionConnectionTest, EnsureBackpressureForAwaitVsync) { + fml::closure on_session_error_callback = []() { FML_CHECK(false); }; + + uint64_t num_presents_handled = 0; + on_frame_presented_event on_frame_presented_callback = + [&num_presents_handled]( + fuchsia::scenic::scheduling::FramePresentedInfo info) { + num_presents_handled += info.presentation_infos.size(); + }; + + flutter_runner::DefaultSessionConnection session_connection( + "debug label", std::move(session_), on_session_error_callback, + on_frame_presented_callback, 0, fml::TimeDelta::FromSeconds(0)); + + uint64_t await_vsyncs_handled = 0; + + for (int i = 0; i < 5; ++i) { + session_connection.Present(); + session_connection.AwaitVsync( + [&await_vsyncs_handled](auto...) { await_vsyncs_handled++; }); + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + } + + EXPECT_EQ(num_presents_handled, 1u); + EXPECT_EQ(await_vsyncs_handled, 0u); +} + +TEST_F(DefaultSessionConnectionTest, SecondaryCallbackShouldFireRegardless) { + fml::closure on_session_error_callback = []() { FML_CHECK(false); }; + + uint64_t num_presents_handled = 0; + on_frame_presented_event on_frame_presented_callback = + [&num_presents_handled]( + fuchsia::scenic::scheduling::FramePresentedInfo info) { + num_presents_handled += info.presentation_infos.size(); + }; + + flutter_runner::DefaultSessionConnection session_connection( + "debug label", std::move(session_), on_session_error_callback, + on_frame_presented_callback, 0, fml::TimeDelta::FromSeconds(0)); + + // We're going to expect *only* secondary callbacks to be triggered. + uint64_t await_vsyncs_handled = 0; + uint64_t await_vsync_for_secondary_callbacks_handled = 0; + + for (int i = 0; i < 5; ++i) { + session_connection.Present(); + session_connection.AwaitVsync( + [&await_vsyncs_handled](auto...) { await_vsyncs_handled++; }); + session_connection.AwaitVsyncForSecondaryCallback( + [&await_vsync_for_secondary_callbacks_handled](auto...) { + await_vsync_for_secondary_callbacks_handled++; + }); + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + } + + EXPECT_EQ(num_presents_handled, 1u); + EXPECT_EQ(await_vsyncs_handled, 0u); + EXPECT_GT(await_vsync_for_secondary_callbacks_handled, 0u); +} + +TEST_F(DefaultSessionConnectionTest, AwaitVsyncBackpressureRelief) { + fml::closure on_session_error_callback = []() { FML_CHECK(false); }; + + uint64_t num_presents_handled = 0; + on_frame_presented_event on_frame_presented_callback = + [&num_presents_handled]( + fuchsia::scenic::scheduling::FramePresentedInfo info) { + num_presents_handled += info.presentation_infos.size(); + }; + + flutter_runner::DefaultSessionConnection session_connection( + "debug label", std::move(session_), on_session_error_callback, + on_frame_presented_callback, 1, fml::TimeDelta::FromSeconds(0)); + + uint64_t await_vsyncs_handled = 0; + + // Max out our present budget. + for (int i = 0; i < 5; ++i) { + session_connection.Present(); + } + + // AwaitVsyncs(). + for (int i = 0; i < 5; ++i) { + session_connection.AwaitVsync( + [&await_vsyncs_handled](auto...) { await_vsyncs_handled++; }); + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + } + + EXPECT_GT(num_presents_handled, 0u); + EXPECT_GT(await_vsyncs_handled, 0u); +} + // The first set of tests has an empty |future_presentation_infos| passed in. // Therefore these tests are to ensure that on startup and after not presenting // for some time that we have correct, reasonable behavior. From 99166f250bda950d4f529f056d06c42a7d5aa404 Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Wed, 19 May 2021 16:22:22 -0400 Subject: [PATCH 11/16] remove vsync_recorder --- shell/platform/fuchsia/flutter/BUILD.gn | 3 - .../flutter/default_session_connection.cc | 41 +++++++++---- .../flutter/default_session_connection.h | 33 +++++++++- .../fuchsia/flutter/vsync_recorder.cc | 61 ------------------- .../platform/fuchsia/flutter/vsync_recorder.h | 55 ----------------- .../platform/fuchsia/flutter/vsync_waiter.cc | 1 - 6 files changed, 60 insertions(+), 134 deletions(-) delete mode 100644 shell/platform/fuchsia/flutter/vsync_recorder.cc delete mode 100644 shell/platform/fuchsia/flutter/vsync_recorder.h diff --git a/shell/platform/fuchsia/flutter/BUILD.gn b/shell/platform/fuchsia/flutter/BUILD.gn index 549e353cfaf4a..771b8ff94688d 100644 --- a/shell/platform/fuchsia/flutter/BUILD.gn +++ b/shell/platform/fuchsia/flutter/BUILD.gn @@ -83,8 +83,6 @@ template("runner_sources") { "thread.cc", "thread.h", "unique_fdio_ns.h", - "vsync_recorder.cc", - "vsync_recorder.h", "vsync_waiter.cc", "vsync_waiter.h", "vulkan_surface.cc", @@ -455,7 +453,6 @@ executable("flutter_runner_unittests") { "runner_unittest.cc", "tests/engine_unittests.cc", "tests/flutter_runner_product_configuration_unittests.cc", - "tests/vsync_recorder_unittests.cc", ] # This is needed for //third_party/googletest for linking zircon symbols. diff --git a/shell/platform/fuchsia/flutter/default_session_connection.cc b/shell/platform/fuchsia/flutter/default_session_connection.cc index 9b9a8cf4519e2..75c5f5de85304 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.cc +++ b/shell/platform/fuchsia/flutter/default_session_connection.cc @@ -7,7 +7,6 @@ #include "flutter/fml/make_copyable.h" #include "flutter/fml/trace_event.h" -#include "vsync_recorder.h" #include "vsync_waiter.h" namespace flutter_runner { @@ -200,8 +199,8 @@ DefaultSessionConnection::DefaultSessionConnection( num_presents_handled); FML_DCHECK(frames_in_flight_ >= 0); - VsyncRecorder::GetInstance().UpdateFramePresentedInfo( - zx::time(info.actual_presentation_time)); + last_presentation_time_ = fml::TimePoint::FromEpochDelta( + fml::TimeDelta::FromNanoseconds(info.actual_presentation_time)); // Call the client-provided callback once we are done using |info|. on_frame_presented_callback_(std::move(info)); @@ -230,8 +229,7 @@ DefaultSessionConnection::DefaultSessionConnection( // If Scenic alloted us 0 frames to begin with, we should fail here. FML_CHECK(frames_in_flight_allowed_ > 0); - VsyncRecorder::GetInstance().UpdateNextPresentationInfo( - std::move(info)); + UpdateNextPresentationInfo(std::move(info)); initialized_ = true; @@ -314,7 +312,7 @@ void DefaultSessionConnection::PresentSession() { // tasks. fml::TimeDelta presentation_interval = - VsyncRecorder::GetInstance().GetCurrentVsyncInfo().presentation_interval; + GetCurrentVsyncInfo().presentation_interval; fml::TimePoint next_latch_point = CalculateNextLatchPoint( fml::TimePoint::Now(), present_requested_time_, @@ -344,8 +342,7 @@ void DefaultSessionConnection::PresentSession() { } frames_in_flight_allowed_ = info.remaining_presents_in_flight_allowed; - VsyncRecorder::GetInstance().UpdateNextPresentationInfo( - std::move(info)); + UpdateNextPresentationInfo(std::move(info)); }); } @@ -373,13 +370,11 @@ void DefaultSessionConnection::FireCallbackMaybe() { FlutterFrameTimes DefaultSessionConnection::GetTargetTimesHelper( bool secondary_callback) { fml::TimeDelta presentation_interval = - VsyncRecorder::GetInstance().GetCurrentVsyncInfo().presentation_interval; + GetCurrentVsyncInfo().presentation_interval; - fml::TimePoint next_vsync = - VsyncRecorder::GetInstance().GetCurrentVsyncInfo().presentation_time; + fml::TimePoint next_vsync = GetCurrentVsyncInfo().presentation_time; fml::TimePoint now = fml::TimePoint::Now(); - fml::TimePoint last_presentation_time = - VsyncRecorder::GetInstance().GetLastPresentationTime(); + fml::TimePoint last_presentation_time = last_presentation_time_; if (next_vsync <= now) { next_vsync = SnapToNextPhase(now, last_presentation_time, presentation_interval); @@ -391,4 +386,24 @@ FlutterFrameTimes DefaultSessionConnection::GetTargetTimesHelper( last_targetted_vsync, now, next_vsync); } +void DefaultSessionConnection::UpdateNextPresentationInfo( + fuchsia::scenic::scheduling::FuturePresentationTimes info) { + auto next_time = next_presentation_info_.presentation_time(); + // Get the earliest vsync time that is after our recorded |presentation_time|. + for (auto& presentation_info : info.future_presentations) { + auto current_time = presentation_info.presentation_time(); + + if (current_time > next_time) { + next_presentation_info_.set_presentation_time(current_time); + return; + } + } +} + +VsyncInfo DefaultSessionConnection::GetCurrentVsyncInfo() const { + return {fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromNanoseconds( + next_presentation_info_.presentation_time())), + kDefaultPresentationInterval}; +} + } // namespace flutter_runner diff --git a/shell/platform/fuchsia/flutter/default_session_connection.h b/shell/platform/fuchsia/flutter/default_session_connection.h index 4553f1f54556d..ce4128e8fbf9a 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.h +++ b/shell/platform/fuchsia/flutter/default_session_connection.h @@ -28,6 +28,16 @@ struct FlutterFrameTimes { fml::TimePoint frame_target; }; +struct VsyncInfo { + fml::TimePoint presentation_time; + fml::TimeDelta presentation_interval; +}; + +// Assume a 60hz refresh rate before we have enough past +// |fuchsia::scenic::scheduling::PresentationInfo|s to calculate it ourselves. +static constexpr fml::TimeDelta kDefaultPresentationInterval = + fml::TimeDelta::FromSecondsF(1.0 / 60.0); + // The component residing on the raster thread that is responsible for // maintaining the Scenic session connection and presenting node updates. class DefaultSessionConnection final : public flutter::SessionWrapper { @@ -111,8 +121,11 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { int frames_in_flight_allowed_ = 0; bool present_session_pending_ = false; - ////// Flutter Animator logic. + // Flutter framework pipeline logic. + // The following fields can be accessed from both the raster and UI threads, + // so guard them with this mutex. If performance dictates, this could probably + // be made lock-free, but it's much easier to reason about with this mutex. std::mutex mutex_; // This is the last Vsync we submitted as the frame_target_time to @@ -124,8 +137,26 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { // This is true iff AwaitVSync() was called but we could not schedule a frame. bool fire_callback_request_pending_ = false; + // The callback passed in from VsyncWaiter which eventually runs on the UI + // thread. FireCallbackCallback fire_callback_; + // VsyncRecorder logic. + + // Update the next Vsync info to |next_presentation_info_|. This is expected + // to be called in |scenic::Session::Present2| immedaite callbacks with the + // presentation info provided by Scenic. Only the next vsync + // information will be saved (in order to handle edge cases involving + // multiple Scenic sessions in the same process). This function is safe to + // call from any thread. + void UpdateNextPresentationInfo( + fuchsia::scenic::scheduling::FuturePresentationTimes info); + + VsyncInfo GetCurrentVsyncInfo() const; + + fuchsia::scenic::scheduling::PresentationInfo next_presentation_info_; + fml::TimePoint last_presentation_time_ = fml::TimePoint::Now(); + FML_DISALLOW_COPY_AND_ASSIGN(DefaultSessionConnection); }; diff --git a/shell/platform/fuchsia/flutter/vsync_recorder.cc b/shell/platform/fuchsia/flutter/vsync_recorder.cc deleted file mode 100644 index 3880cb0ac26cd..0000000000000 --- a/shell/platform/fuchsia/flutter/vsync_recorder.cc +++ /dev/null @@ -1,61 +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 "vsync_recorder.h" - -#include - -namespace flutter_runner { - -namespace { - -std::mutex g_mutex; - -// Assume a 60hz refresh rate before we have enough past -// |fuchsia::scenic::scheduling::PresentationInfo|s to calculate it ourselves. -static constexpr fml::TimeDelta kDefaultPresentationInterval = - fml::TimeDelta::FromSecondsF(1.0 / 60.0); - -} // namespace - -VsyncRecorder& VsyncRecorder::GetInstance() { - static VsyncRecorder vsync_recorder; - return vsync_recorder; -} - -VsyncInfo VsyncRecorder::GetCurrentVsyncInfo() const { - { - std::unique_lock lock(g_mutex); - return {fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromNanoseconds( - next_presentation_info_.presentation_time())), - kDefaultPresentationInterval}; - } -} - -void VsyncRecorder::UpdateNextPresentationInfo( - fuchsia::scenic::scheduling::FuturePresentationTimes info) { - std::unique_lock lock(g_mutex); - - auto next_time = next_presentation_info_.presentation_time(); - // Get the earliest vsync time that is after our recorded |presentation_time|. - for (auto& presentation_info : info.future_presentations) { - auto current_time = presentation_info.presentation_time(); - - if (current_time > next_time) { - next_presentation_info_.set_presentation_time(current_time); - return; - } - } -} - -void VsyncRecorder::UpdateFramePresentedInfo(zx::time presentation_time) { - last_presentation_time_ = fml::TimePoint::FromEpochDelta( - fml::TimeDelta::FromNanoseconds(presentation_time.get())); -} - -fml::TimePoint VsyncRecorder::GetLastPresentationTime() const { - return last_presentation_time_; -} - -} // namespace flutter_runner diff --git a/shell/platform/fuchsia/flutter/vsync_recorder.h b/shell/platform/fuchsia/flutter/vsync_recorder.h deleted file mode 100644 index 35151ce1ceff9..0000000000000 --- a/shell/platform/fuchsia/flutter/vsync_recorder.h +++ /dev/null @@ -1,55 +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 FLUTTER_SHELL_PLATFORM_FUCHSIA_VSYNC_RECORDER_H_ -#define FLUTTER_SHELL_PLATFORM_FUCHSIA_VSYNC_RECORDER_H_ - -#include - -#include "flutter/fml/time/time_delta.h" -#include "flutter/fml/time/time_point.h" -#include "lib/ui/scenic/cpp/session.h" - -namespace flutter_runner { - -struct VsyncInfo { - fml::TimePoint presentation_time; - fml::TimeDelta presentation_interval; -}; - -class VsyncRecorder { - public: - static VsyncRecorder& GetInstance(); - - // Retrieve the most recent |PresentationInfo| provided to us by scenic. - // This function is safe to call from any thread. - VsyncInfo GetCurrentVsyncInfo() const; - - // Update the next Vsync info to |next_presentation_info_|. This is expected - // to be called in |scenic::Session::Present2| immedaite callbacks with the - // presentation info provided by Scenic. Only the next vsync - // information will be saved (in order to handle edge cases involving - // multiple Scenic sessions in the same process). This function is safe to - // call from any thread. - void UpdateNextPresentationInfo( - fuchsia::scenic::scheduling::FuturePresentationTimes info); - - void UpdateFramePresentedInfo(zx::time presentation_time); - - fml::TimePoint GetLastPresentationTime() const; - - private: - VsyncRecorder() { next_presentation_info_.set_presentation_time(0); } - - fuchsia::scenic::scheduling::PresentationInfo next_presentation_info_; - fml::TimePoint last_presentation_time_ = fml::TimePoint::Now(); - - // Disallow copy and assignment. - VsyncRecorder(const VsyncRecorder&) = delete; - VsyncRecorder& operator=(const VsyncRecorder&) = delete; -}; - -} // namespace flutter_runner - -#endif // FLUTTER_SHELL_PLATFORM_FUCHSIA_VSYNC_RECORDER_H_ diff --git a/shell/platform/fuchsia/flutter/vsync_waiter.cc b/shell/platform/fuchsia/flutter/vsync_waiter.cc index ccc0d16d7f729..c66e12690e310 100644 --- a/shell/platform/fuchsia/flutter/vsync_waiter.cc +++ b/shell/platform/fuchsia/flutter/vsync_waiter.cc @@ -15,7 +15,6 @@ #include "flutter/fml/trace_event.h" #include "flutter_runner_product_configuration.h" -#include "vsync_recorder.h" namespace flutter_runner { From 9a332280b438b195d2b8d5ddd96425aa2cb92e41 Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Wed, 19 May 2021 16:33:34 -0400 Subject: [PATCH 12/16] fix vsync_recorder licenses --- ci/licenses_golden/licenses_flutter | 2 -- 1 file changed, 2 deletions(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 2bcf7252fd226..7da6cdc6cb36e 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -1369,8 +1369,6 @@ FILE: ../../../flutter/shell/platform/fuchsia/flutter/task_runner_adapter.h FILE: ../../../flutter/shell/platform/fuchsia/flutter/thread.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/thread.h FILE: ../../../flutter/shell/platform/fuchsia/flutter/unique_fdio_ns.h -FILE: ../../../flutter/shell/platform/fuchsia/flutter/vsync_recorder.cc -FILE: ../../../flutter/shell/platform/fuchsia/flutter/vsync_recorder.h FILE: ../../../flutter/shell/platform/fuchsia/flutter/vsync_waiter.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/vsync_waiter.h FILE: ../../../flutter/shell/platform/fuchsia/flutter/vulkan_surface.cc From 2d9d00c64a0b7f023d0e09fd63854b4dc4569c43 Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Wed, 19 May 2021 18:55:59 -0400 Subject: [PATCH 13/16] finish vsync_recorder tests --- .../flutter/default_session_connection.cc | 29 +++++-- .../flutter/default_session_connection.h | 19 ++--- .../default_session_connection_unittests.cc | 78 +++++++++++++++++++ 3 files changed, 109 insertions(+), 17 deletions(-) diff --git a/shell/platform/fuchsia/flutter/default_session_connection.cc b/shell/platform/fuchsia/flutter/default_session_connection.cc index 75c5f5de85304..55102f47d5de5 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.cc +++ b/shell/platform/fuchsia/flutter/default_session_connection.cc @@ -179,6 +179,8 @@ DefaultSessionConnection::DefaultSessionConnection( on_frame_presented_callback_(std::move(on_frame_presented_callback)), kMaxFramesInFlight(max_frames_in_flight), vsync_offset_(vsync_offset) { + next_presentation_info_.set_presentation_time(0); + session_wrapper_.set_error_handler( [callback = session_error_callback](zx_status_t status) { callback(); }); @@ -229,7 +231,8 @@ DefaultSessionConnection::DefaultSessionConnection( // If Scenic alloted us 0 frames to begin with, we should fail here. FML_CHECK(frames_in_flight_allowed_ > 0); - UpdateNextPresentationInfo(std::move(info)); + next_presentation_info_ = + UpdatePresentationInfo(std::move(info), next_presentation_info_); initialized_ = true; @@ -342,7 +345,8 @@ void DefaultSessionConnection::PresentSession() { } frames_in_flight_allowed_ = info.remaining_presents_in_flight_allowed; - UpdateNextPresentationInfo(std::move(info)); + next_presentation_info_ = + UpdatePresentationInfo(std::move(info), next_presentation_info_); }); } @@ -386,18 +390,27 @@ FlutterFrameTimes DefaultSessionConnection::GetTargetTimesHelper( last_targetted_vsync, now, next_vsync); } -void DefaultSessionConnection::UpdateNextPresentationInfo( - fuchsia::scenic::scheduling::FuturePresentationTimes info) { - auto next_time = next_presentation_info_.presentation_time(); +fuchsia::scenic::scheduling::PresentationInfo + +DefaultSessionConnection::UpdatePresentationInfo( + fuchsia::scenic::scheduling::FuturePresentationTimes future_info, + fuchsia::scenic::scheduling::PresentationInfo& presentation_info) { + fuchsia::scenic::scheduling::PresentationInfo new_presentation_info; + new_presentation_info.set_presentation_time( + presentation_info.presentation_time()); + + auto next_time = presentation_info.presentation_time(); // Get the earliest vsync time that is after our recorded |presentation_time|. - for (auto& presentation_info : info.future_presentations) { + for (auto& presentation_info : future_info.future_presentations) { auto current_time = presentation_info.presentation_time(); if (current_time > next_time) { - next_presentation_info_.set_presentation_time(current_time); - return; + new_presentation_info.set_presentation_time(current_time); + return new_presentation_info; } } + + return new_presentation_info; } VsyncInfo DefaultSessionConnection::GetCurrentVsyncInfo() const { diff --git a/shell/platform/fuchsia/flutter/default_session_connection.h b/shell/platform/fuchsia/flutter/default_session_connection.h index ce4128e8fbf9a..b17d443657a4e 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.h +++ b/shell/platform/fuchsia/flutter/default_session_connection.h @@ -62,6 +62,16 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { const fml::TimePoint last_frame_presentation_time, const fml::TimeDelta presentation_interval); + // Update the next Vsync info to |next_presentation_info_|. This is expected + // to be called in |scenic::Session::Present2| immedaite callbacks with the + // presentation info provided by Scenic. Only the next vsync + // information will be saved (in order to handle edge cases involving + // multiple Scenic sessions in the same process). This function is safe to + // call from any thread. + static fuchsia::scenic::scheduling::PresentationInfo UpdatePresentationInfo( + fuchsia::scenic::scheduling::FuturePresentationTimes future_info, + fuchsia::scenic::scheduling::PresentationInfo& presentation_info); + DefaultSessionConnection( std::string debug_label, fidl::InterfaceHandle session, @@ -143,15 +153,6 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { // VsyncRecorder logic. - // Update the next Vsync info to |next_presentation_info_|. This is expected - // to be called in |scenic::Session::Present2| immedaite callbacks with the - // presentation info provided by Scenic. Only the next vsync - // information will be saved (in order to handle edge cases involving - // multiple Scenic sessions in the same process). This function is safe to - // call from any thread. - void UpdateNextPresentationInfo( - fuchsia::scenic::scheduling::FuturePresentationTimes info); - VsyncInfo GetCurrentVsyncInfo() const; fuchsia::scenic::scheduling::PresentationInfo next_presentation_info_; diff --git a/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc b/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc index e06577461cca0..bdb98b911362d 100644 --- a/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc +++ b/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc @@ -27,6 +27,16 @@ static int TimePointToInt(fml::TimePoint time) { return time.ToEpochDelta().ToNanoseconds(); } +static fuchsia::scenic::scheduling::PresentationInfo CreatePresentationInfo( + zx_time_t latch_point, + zx_time_t presentation_time) { + fuchsia::scenic::scheduling::PresentationInfo info; + + info.set_latch_point(latch_point); + info.set_presentation_time(presentation_time); + return info; +} + class DefaultSessionConnectionTest : public ::testing::Test { public: void SetUp() override { @@ -803,4 +813,72 @@ TEST(GetTargetTimesTest, ScheduleMultipleTimes_WithDelayedWakeups) { last_targetted_vsync = target_times.frame_target; } } +// static fuchsia::scenic::scheduling::PresentationInfo UpdatePresentationInfo( +// fuchsia::scenic::scheduling::FuturePresentationTimes future_info, +// fuchsia::scenic::scheduling::PresentationInfo& presentation_info); + +TEST(UpdatePresentationInfoTest, SingleUpdate) { + std::vector + future_presentations = {}; + + // Update the |vsync_info|. + future_presentations.push_back( + CreatePresentationInfo(/*latch_point=*/5, /*presentation_time=*/10)); + + fuchsia::scenic::scheduling::FuturePresentationTimes future_info; + future_info.future_presentations = std::move(future_presentations); + future_info.remaining_presents_in_flight_allowed = 1; + + fuchsia::scenic::scheduling::PresentationInfo presentation_info; + presentation_info.set_presentation_time(0); + + fuchsia::scenic::scheduling::PresentationInfo new_presentation_info = + flutter_runner::DefaultSessionConnection::UpdatePresentationInfo( + std::move(future_info), presentation_info); + + EXPECT_EQ(new_presentation_info.presentation_time(), 10); +} + +TEST(UpdatePresentationInfoTest, MultipleUpdates) { + std::vector + future_presentations = {}; + + // Update the |vsync_info|. + future_presentations.push_back( + CreatePresentationInfo(/*latch_point=*/15, /*presentation_time=*/20)); + future_presentations.push_back( + CreatePresentationInfo(/*latch_point=*/25, /*presentation_time=*/30)); + fuchsia::scenic::scheduling::FuturePresentationTimes future_info; + future_info.future_presentations = std::move(future_presentations); + future_info.remaining_presents_in_flight_allowed = 1; + + fuchsia::scenic::scheduling::PresentationInfo presentation_info; + presentation_info.set_presentation_time(0); + + fuchsia::scenic::scheduling::PresentationInfo new_presentation_info = + flutter_runner::DefaultSessionConnection::UpdatePresentationInfo( + std::move(future_info), presentation_info); + + EXPECT_EQ(new_presentation_info.presentation_time(), 20); + + // Clear and re-try with more future times! + future_presentations.clear(); + future_presentations.push_back( + CreatePresentationInfo(/*latch_point=*/15, /*presentation_time=*/20)); + future_presentations.push_back( + CreatePresentationInfo(/*latch_point=*/25, /*presentation_time=*/30)); + future_presentations.push_back( + CreatePresentationInfo(/*latch_point=*/35, /*presentation_time=*/40)); + future_presentations.push_back( + CreatePresentationInfo(/*latch_point=*/45, /*presentation_time=*/50)); + future_info.future_presentations = std::move(future_presentations); + future_info.remaining_presents_in_flight_allowed = 1; + + new_presentation_info = + flutter_runner::DefaultSessionConnection::UpdatePresentationInfo( + std::move(future_info), new_presentation_info); + + EXPECT_EQ(new_presentation_info.presentation_time(), 30); +} + } // namespace flutter_runner_test From 08bf0fb8afbbdb973a738f1317f468be3b3ae265 Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Wed, 19 May 2021 19:08:20 -0400 Subject: [PATCH 14/16] remove old vsync_recorder_unittests --- .../flutter/tests/vsync_recorder_unittests.cc | 134 ------------------ 1 file changed, 134 deletions(-) delete mode 100644 shell/platform/fuchsia/flutter/tests/vsync_recorder_unittests.cc diff --git a/shell/platform/fuchsia/flutter/tests/vsync_recorder_unittests.cc b/shell/platform/fuchsia/flutter/tests/vsync_recorder_unittests.cc deleted file mode 100644 index 200ad80732d5c..0000000000000 --- a/shell/platform/fuchsia/flutter/tests/vsync_recorder_unittests.cc +++ /dev/null @@ -1,134 +0,0 @@ -// Copyright 2019 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 - -#include -#include - -#include "flutter/shell/platform/fuchsia/flutter/logging.h" -#include "flutter/shell/platform/fuchsia/flutter/vsync_recorder.h" - -#include "flutter/shell/platform/fuchsia/flutter/default_session_connection.h" -#include "flutter/shell/platform/fuchsia/flutter/runner.h" - -using namespace flutter_runner; - -namespace flutter_runner_test { - -static fuchsia::scenic::scheduling::PresentationInfo CreatePresentationInfo( - zx_time_t latch_point, - zx_time_t presentation_time) { - fuchsia::scenic::scheduling::PresentationInfo info; - - info.set_latch_point(latch_point); - info.set_presentation_time(presentation_time); - return info; -} - -// IMPORTANT NOTE: Because there only exists one VsyncRecorder, the order of -// these tests matter. - -TEST(VsyncRecorderTest, DefaultVsyncInfoValues_AreReasonable) { - VsyncInfo vsync_info = VsyncRecorder::GetInstance().GetCurrentVsyncInfo(); - - EXPECT_GE(vsync_info.presentation_time, - fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromNanoseconds(0))); - - EXPECT_GE(vsync_info.presentation_interval, - fml::TimeDelta::FromMilliseconds(10)); -} - -TEST(VsyncRecorderTest, DefaultLastPresentationTime_IsReasonable) { - fml::TimePoint last_presentation_time = - VsyncRecorder::GetInstance().GetLastPresentationTime(); - - EXPECT_LE(last_presentation_time, fml::TimePoint::Now()); -} - -TEST(VsyncRecorderTest, SinglePresentationInfo_IsUpdatedCorrectly) { - std::vector - future_presentations = {}; - - // Update the |vsync_info|. - future_presentations.push_back( - CreatePresentationInfo(/*latch_point=*/5, /*presentation_time=*/10)); - VsyncRecorder::GetInstance().UpdateNextPresentationInfo( - {.future_presentations = std::move(future_presentations), - .remaining_presents_in_flight_allowed = 1}); - - // Check that |vsync_info| was correctly updated. - VsyncInfo vsync_info = VsyncRecorder::GetInstance().GetCurrentVsyncInfo(); - EXPECT_EQ( - vsync_info.presentation_time, - fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromNanoseconds(10))); - - EXPECT_GE(vsync_info.presentation_interval, - fml::TimeDelta::FromMilliseconds(10)); -} - -TEST(VsyncRecorderTest, MultiplePresentationInfos_AreUpdatedCorrectly) { - std::vector - future_presentations = {}; - - // Update the |vsync_info|. - future_presentations.push_back( - CreatePresentationInfo(/*latch_point=*/15, /*presentation_time=*/20)); - future_presentations.push_back( - CreatePresentationInfo(/*latch_point=*/25, /*presentation_time=*/30)); - VsyncRecorder::GetInstance().UpdateNextPresentationInfo( - {.future_presentations = std::move(future_presentations), - .remaining_presents_in_flight_allowed = 1}); - - // Check that |vsync_info| was correctly updated with the first time. - VsyncInfo vsync_info = VsyncRecorder::GetInstance().GetCurrentVsyncInfo(); - EXPECT_EQ( - vsync_info.presentation_time, - fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromNanoseconds(20))); - EXPECT_GE(vsync_info.presentation_interval, - fml::TimeDelta::FromMilliseconds(10)); - - // Clear and re-try with more future times! - future_presentations.clear(); - future_presentations.push_back( - CreatePresentationInfo(/*latch_point=*/15, /*presentation_time=*/20)); - future_presentations.push_back( - CreatePresentationInfo(/*latch_point=*/25, /*presentation_time=*/30)); - future_presentations.push_back( - CreatePresentationInfo(/*latch_point=*/35, /*presentation_time=*/40)); - future_presentations.push_back( - CreatePresentationInfo(/*latch_point=*/45, /*presentation_time=*/50)); - VsyncRecorder::GetInstance().UpdateNextPresentationInfo( - {.future_presentations = std::move(future_presentations), - .remaining_presents_in_flight_allowed = 1}); - - // Check that |vsync_info| was correctly updated with the first time. - vsync_info = VsyncRecorder::GetInstance().GetCurrentVsyncInfo(); - EXPECT_EQ( - vsync_info.presentation_time, - fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromNanoseconds(30))); - EXPECT_GE(vsync_info.presentation_interval, - fml::TimeDelta::FromMilliseconds(10)); -} - -TEST(VsyncRecorderTest, FramePresentedInfo_IsUpdatedCorrectly) { - int64_t time1 = 10; - int64_t time2 = 30; - int64_t time3 = 35; - - VsyncRecorder::GetInstance().UpdateFramePresentedInfo(zx::time(time1)); - - EXPECT_EQ( - VsyncRecorder::GetInstance().GetLastPresentationTime(), - fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromNanoseconds(time1))); - - VsyncRecorder::GetInstance().UpdateFramePresentedInfo(zx::time(time2)); - VsyncRecorder::GetInstance().UpdateFramePresentedInfo(zx::time(time3)); - - EXPECT_EQ( - VsyncRecorder::GetInstance().GetLastPresentationTime(), - fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromNanoseconds(time3))); -} - -} // namespace flutter_runner_test From 05c50d8aa14b620baf0ba67bc27e91343627769b Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Wed, 19 May 2021 19:12:23 -0400 Subject: [PATCH 15/16] s/targetted/targeted --- .../flutter/default_session_connection.cc | 25 +++++------ .../flutter/default_session_connection.h | 4 +- .../default_session_connection_unittests.cc | 44 +++++++++---------- 3 files changed, 35 insertions(+), 38 deletions(-) diff --git a/shell/platform/fuchsia/flutter/default_session_connection.cc b/shell/platform/fuchsia/flutter/default_session_connection.cc index 55102f47d5de5..7acb3b2cb1067 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.cc +++ b/shell/platform/fuchsia/flutter/default_session_connection.cc @@ -29,7 +29,7 @@ namespace flutter_runner { // |vsync_interval| - the interval between vsyncs. Would be 16.6ms for a 60Hz // display. // -// |last_targetted_vsync| - the last vsync targetted, which is usually the +// |last_targeted_vsync| - the last vsync targeted, which is usually the // frame_end_time returned from the last invocation of this function // // |now| - the current time @@ -39,7 +39,7 @@ namespace flutter_runner { FlutterFrameTimes DefaultSessionConnection::GetTargetTimes( fml::TimeDelta vsync_offset, fml::TimeDelta vsync_interval, - fml::TimePoint last_targetted_vsync, + fml::TimePoint last_targeted_vsync, fml::TimePoint now, fml::TimePoint next_vsync) { FML_DCHECK(vsync_offset <= vsync_interval); @@ -61,7 +61,7 @@ FlutterFrameTimes DefaultSessionConnection::GetTargetTimes( fml::TimePoint frame_end_time = next_vsync; // Advance to next available slot, keeping in mind the two invariants. - while (frame_end_time < (last_targetted_vsync + (vsync_interval / 2)) || + while (frame_end_time < (last_targeted_vsync + (vsync_interval / 2)) || frame_start_time < now) { frame_start_time = frame_start_time + vsync_interval; frame_end_time = frame_end_time + vsync_interval; @@ -72,12 +72,11 @@ FlutterFrameTimes DefaultSessionConnection::GetTargetTimes( TRACE_DURATION( "flutter", "DefaultSessionConnection::GetTargetTimes", "previous_vsync(ms)", previous_vsync.ToEpochDelta().ToMilliseconds(), - "last_targetted(ms)", - last_targetted_vsync.ToEpochDelta().ToMilliseconds(), "now(ms)", - fml::TimePoint::Now().ToEpochDelta().ToMilliseconds(), "next_vsync(ms))", - next_vsync.ToEpochDelta().ToMilliseconds(), "frame_start_time(ms)", - frame_start_time.ToEpochDelta().ToMilliseconds(), "frame_end_time(ms)", - frame_end_time.ToEpochDelta().ToMilliseconds()); + "last_targeted(ms)", last_targeted_vsync.ToEpochDelta().ToMilliseconds(), + "now(ms)", fml::TimePoint::Now().ToEpochDelta().ToMilliseconds(), + "next_vsync(ms))", next_vsync.ToEpochDelta().ToMilliseconds(), + "frame_start_time(ms)", frame_start_time.ToEpochDelta().ToMilliseconds(), + "frame_end_time(ms)", frame_end_time.ToEpochDelta().ToMilliseconds()); return {frame_start_time, frame_end_time}; } @@ -360,7 +359,7 @@ void DefaultSessionConnection::FireCallbackMaybe() { FlutterFrameTimes times = GetTargetTimesHelper(/*secondary_callback=*/false); - last_targetted_vsync_ = times.frame_target; + last_targeted_vsync_ = times.frame_target; fire_callback_request_pending_ = false; fire_callback_(times.frame_start, times.frame_target); @@ -384,10 +383,10 @@ FlutterFrameTimes DefaultSessionConnection::GetTargetTimesHelper( SnapToNextPhase(now, last_presentation_time, presentation_interval); } - fml::TimePoint last_targetted_vsync = - secondary_callback ? fml::TimePoint::Min() : last_targetted_vsync_; + fml::TimePoint last_targeted_vsync = + secondary_callback ? fml::TimePoint::Min() : last_targeted_vsync_; return GetTargetTimes(vsync_offset_, presentation_interval, - last_targetted_vsync, now, next_vsync); + last_targeted_vsync, now, next_vsync); } fuchsia::scenic::scheduling::PresentationInfo diff --git a/shell/platform/fuchsia/flutter/default_session_connection.h b/shell/platform/fuchsia/flutter/default_session_connection.h index b17d443657a4e..2f14224031d11 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.h +++ b/shell/platform/fuchsia/flutter/default_session_connection.h @@ -44,7 +44,7 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { public: static FlutterFrameTimes GetTargetTimes(fml::TimeDelta vsync_offset, fml::TimeDelta vsync_interval, - fml::TimePoint last_targetted_vsync, + fml::TimePoint last_targeted_vsync, fml::TimePoint now, fml::TimePoint next_vsync); @@ -142,7 +142,7 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { // FireCallback(). This value should be strictly increasing in order to // guarantee that animation code that relies on target vsyncs works correctly, // and that Flutter is not producing multiple frames in a small interval. - fml::TimePoint last_targetted_vsync_; + fml::TimePoint last_targeted_vsync_; // This is true iff AwaitVSync() was called but we could not schedule a frame. bool fire_callback_request_pending_ = false; diff --git a/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc b/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc index bdb98b911362d..75b1ac41c0c6b 100644 --- a/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc +++ b/shell/platform/fuchsia/flutter/tests/default_session_connection_unittests.cc @@ -650,13 +650,13 @@ TEST(SnapToNextPhaseTest, SnapAfterNowMultiJumpAccountForCeils) { TEST(GetTargetTimesTest, ScheduleForNextVsync) { const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - const fml::TimePoint last_targetted_vsync = TimePointFromInt(10); + const fml::TimePoint last_targeted_vsync = TimePointFromInt(10); const fml::TimePoint now = TimePointFromInt(9); const fml::TimePoint next_vsync = TimePointFromInt(10); const auto target_times = flutter_runner::DefaultSessionConnection::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + vsync_offset, vsync_interval, last_targeted_vsync, now, next_vsync); EXPECT_EQ(TimePointToInt(target_times.frame_start), 10); EXPECT_EQ(TimePointToInt(target_times.frame_target), 20); @@ -665,13 +665,13 @@ TEST(GetTargetTimesTest, ScheduleForNextVsync) { TEST(GetTargetTimesTest, ScheduleForCurrentVsync_DueToOffset) { const fml::TimeDelta vsync_offset = TimeDeltaFromInt(3); const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - const fml::TimePoint last_targetted_vsync = TimePointFromInt(0); + const fml::TimePoint last_targeted_vsync = TimePointFromInt(0); const fml::TimePoint now = TimePointFromInt(6); const fml::TimePoint next_vsync = TimePointFromInt(10); const auto target_times = flutter_runner::DefaultSessionConnection::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + vsync_offset, vsync_interval, last_targeted_vsync, now, next_vsync); EXPECT_EQ(TimePointToInt(target_times.frame_start), 7); EXPECT_EQ(TimePointToInt(target_times.frame_target), 10); @@ -680,13 +680,13 @@ TEST(GetTargetTimesTest, ScheduleForCurrentVsync_DueToOffset) { TEST(GetTargetTimesTest, ScheduleForFollowingVsync_BecauseOfNow) { const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - const fml::TimePoint last_targetted_vsync = TimePointFromInt(10); + const fml::TimePoint last_targeted_vsync = TimePointFromInt(10); const fml::TimePoint now = TimePointFromInt(15); const fml::TimePoint next_vsync = TimePointFromInt(10); const auto target_times = flutter_runner::DefaultSessionConnection::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + vsync_offset, vsync_interval, last_targeted_vsync, now, next_vsync); EXPECT_EQ(TimePointToInt(target_times.frame_start), 20); EXPECT_EQ(TimePointToInt(target_times.frame_target), 30); @@ -695,13 +695,13 @@ TEST(GetTargetTimesTest, ScheduleForFollowingVsync_BecauseOfNow) { TEST(GetTargetTimesTest, ScheduleForFollowingVsync_BecauseOfTargettedTime) { const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - const fml::TimePoint last_targetted_vsync = TimePointFromInt(20); + const fml::TimePoint last_targeted_vsync = TimePointFromInt(20); const fml::TimePoint now = TimePointFromInt(9); const fml::TimePoint next_vsync = TimePointFromInt(10); const auto target_times = flutter_runner::DefaultSessionConnection::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + vsync_offset, vsync_interval, last_targeted_vsync, now, next_vsync); EXPECT_EQ(TimePointToInt(target_times.frame_start), 20); EXPECT_EQ(TimePointToInt(target_times.frame_target), 30); @@ -710,13 +710,13 @@ TEST(GetTargetTimesTest, ScheduleForFollowingVsync_BecauseOfTargettedTime) { TEST(GetTargetTimesTest, ScheduleForDistantVsync_BecauseOfTargettedTime) { const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - const fml::TimePoint last_targetted_vsync = TimePointFromInt(60); + const fml::TimePoint last_targeted_vsync = TimePointFromInt(60); const fml::TimePoint now = TimePointFromInt(9); const fml::TimePoint next_vsync = TimePointFromInt(10); const auto target_times = flutter_runner::DefaultSessionConnection::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + vsync_offset, vsync_interval, last_targeted_vsync, now, next_vsync); EXPECT_EQ(TimePointToInt(target_times.frame_start), 60); EXPECT_EQ(TimePointToInt(target_times.frame_target), 70); @@ -728,13 +728,13 @@ TEST(GetTargetTimesTest, ScheduleForFollowingVsync_WithSlightVsyncDrift) { // Even though it appears as if the next vsync is at time 40, we should still // present at time 50. - const fml::TimePoint last_targetted_vsync = TimePointFromInt(37); + const fml::TimePoint last_targeted_vsync = TimePointFromInt(37); const fml::TimePoint now = TimePointFromInt(9); const fml::TimePoint next_vsync = TimePointFromInt(10); const auto target_times = flutter_runner::DefaultSessionConnection::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + vsync_offset, vsync_interval, last_targeted_vsync, now, next_vsync); EXPECT_EQ(TimePointToInt(target_times.frame_start), 40); EXPECT_EQ(TimePointToInt(target_times.frame_target), 50); @@ -743,13 +743,13 @@ TEST(GetTargetTimesTest, ScheduleForFollowingVsync_WithSlightVsyncDrift) { TEST(GetTargetTimesTest, ScheduleForAnOffsetFromVsync) { const fml::TimeDelta vsync_offset = TimeDeltaFromInt(4); const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - const fml::TimePoint last_targetted_vsync = TimePointFromInt(10); + const fml::TimePoint last_targeted_vsync = TimePointFromInt(10); const fml::TimePoint now = TimePointFromInt(9); const fml::TimePoint next_vsync = TimePointFromInt(10); const auto target_times = flutter_runner::DefaultSessionConnection::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, next_vsync); + vsync_offset, vsync_interval, last_targeted_vsync, now, next_vsync); EXPECT_EQ(TimePointToInt(target_times.frame_start), 16); EXPECT_EQ(TimePointToInt(target_times.frame_target), 20); @@ -759,15 +759,14 @@ TEST(GetTargetTimesTest, ScheduleMultipleTimes) { const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - fml::TimePoint last_targetted_vsync = TimePointFromInt(0); + fml::TimePoint last_targeted_vsync = TimePointFromInt(0); fml::TimePoint now = TimePointFromInt(5); fml::TimePoint next_vsync = TimePointFromInt(10); for (int i = 0; i < 100; ++i) { const auto target_times = flutter_runner::DefaultSessionConnection::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, - next_vsync); + vsync_offset, vsync_interval, last_targeted_vsync, now, next_vsync); EXPECT_EQ(TimePointToInt(target_times.frame_start), 10 * (i + 1)); EXPECT_EQ(TimePointToInt(target_times.frame_target), 10 * (i + 2)); @@ -775,7 +774,7 @@ TEST(GetTargetTimesTest, ScheduleMultipleTimes) { // Simulate the passage of time. now = now + vsync_interval; next_vsync = next_vsync + vsync_interval; - last_targetted_vsync = target_times.frame_target; + last_targeted_vsync = target_times.frame_target; } } @@ -787,19 +786,18 @@ TEST(GetTargetTimesTest, ScheduleMultipleTimes_WithDelayedWakeups) { const fml::TimeDelta vsync_offset = TimeDeltaFromInt(0); const fml::TimeDelta vsync_interval = TimeDeltaFromInt(10); - fml::TimePoint last_targetted_vsync = TimePointFromInt(0); + fml::TimePoint last_targeted_vsync = TimePointFromInt(0); fml::TimePoint now = TimePointFromInt(5); fml::TimePoint next_vsync = TimePointFromInt(10); for (int i = 0; i < 100; ++i) { const auto target_times = flutter_runner::DefaultSessionConnection::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, now, - next_vsync); + vsync_offset, vsync_interval, last_targeted_vsync, now, next_vsync); const auto target_times_delay = flutter_runner::DefaultSessionConnection::GetTargetTimes( - vsync_offset, vsync_interval, last_targetted_vsync, + vsync_offset, vsync_interval, last_targeted_vsync, now + TimeDeltaFromInt(i % 5), next_vsync); EXPECT_EQ(TimePointToInt(target_times.frame_start), @@ -810,7 +808,7 @@ TEST(GetTargetTimesTest, ScheduleMultipleTimes_WithDelayedWakeups) { // Simulate the passage of time. now = now + vsync_interval; next_vsync = next_vsync + vsync_interval; - last_targetted_vsync = target_times.frame_target; + last_targeted_vsync = target_times.frame_target; } } // static fuchsia::scenic::scheduling::PresentationInfo UpdatePresentationInfo( From fadb7dc84851602aaa904ff6149934ae366b09cd Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Thu, 20 May 2021 14:42:40 -0400 Subject: [PATCH 16/16] clean up session_connection.h --- .../flutter/default_session_connection.h | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/shell/platform/fuchsia/flutter/default_session_connection.h b/shell/platform/fuchsia/flutter/default_session_connection.h index 2f14224031d11..c6bef21902955 100644 --- a/shell/platform/fuchsia/flutter/default_session_connection.h +++ b/shell/platform/fuchsia/flutter/default_session_connection.h @@ -99,6 +99,8 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { FlutterFrameTimes GetTargetTimesHelper(bool secondary_callback); + VsyncInfo GetCurrentVsyncInfo() const; + scenic::Session session_wrapper_; on_frame_presented_event on_frame_presented_callback_; @@ -125,12 +127,19 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { // outstanding at any time. This is equivalent to how many times it has // called Present2() before receiving an OnFramePresented() event. const int kMaxFramesInFlight; - fml::TimeDelta vsync_offset_; int frames_in_flight_ = 0; int frames_in_flight_allowed_ = 0; bool present_session_pending_ = false; + // The time from vsync that the Flutter animator should begin its frames. This + // is non-zero so that Flutter and Scenic compete less for CPU and GPU time. + fml::TimeDelta vsync_offset_; + + // Variables for recording past and future vsync info, as reported by Scenic. + fml::TimePoint last_presentation_time_ = fml::TimePoint::Now(); + fuchsia::scenic::scheduling::PresentationInfo next_presentation_info_; + // Flutter framework pipeline logic. // The following fields can be accessed from both the raster and UI threads, @@ -151,13 +160,6 @@ class DefaultSessionConnection final : public flutter::SessionWrapper { // thread. FireCallbackCallback fire_callback_; - // VsyncRecorder logic. - - VsyncInfo GetCurrentVsyncInfo() const; - - fuchsia::scenic::scheduling::PresentationInfo next_presentation_info_; - fml::TimePoint last_presentation_time_ = fml::TimePoint::Now(); - FML_DISALLOW_COPY_AND_ASSIGN(DefaultSessionConnection); };