From 3797b8f6086774039d2680e54b2568077eeb72d7 Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Fri, 29 Oct 2021 07:26:45 -0700 Subject: [PATCH] Embedder VsyncWaiter must schedule a frame for the right time Prior to https://github.com/flutter/engine/pull/28817 super class vsync waiter would post the tast to the future if the frame start time is at a later poit. This change made it so that the sub classes are responsible for posting the task at the right time. This change makes it so that vsync waiter embedder respects that contract. Fixes https://github.com/flutter/flutter/issues/92603 --- shell/platform/embedder/embedder_engine.cc | 4 +- .../embedder/tests/embedder_config_builder.cc | 7 ++ .../embedder/tests/embedder_config_builder.h | 4 ++ .../embedder/tests/embedder_test_context.cc | 9 +++ .../embedder/tests/embedder_test_context.h | 10 +++ .../embedder/tests/embedder_unittests.cc | 69 ++++++++++++++++++- .../embedder/vsync_waiter_embedder.cc | 33 +++++---- .../platform/embedder/vsync_waiter_embedder.h | 3 +- 8 files changed, 123 insertions(+), 16 deletions(-) diff --git a/shell/platform/embedder/embedder_engine.cc b/shell/platform/embedder/embedder_engine.cc index 7d1f74bdb3897..d96fa0c81b31e 100644 --- a/shell/platform/embedder/embedder_engine.cc +++ b/shell/platform/embedder/embedder_engine.cc @@ -229,8 +229,8 @@ bool EmbedderEngine::OnVsyncEvent(intptr_t baton, return false; } - return VsyncWaiterEmbedder::OnEmbedderVsync(baton, frame_start_time, - frame_target_time); + return VsyncWaiterEmbedder::OnEmbedderVsync( + task_runners_, baton, frame_start_time, frame_target_time); } bool EmbedderEngine::ReloadSystemFonts() { diff --git a/shell/platform/embedder/tests/embedder_config_builder.cc b/shell/platform/embedder/tests/embedder_config_builder.cc index ac6c86a7e5089..d04188e0d3070 100644 --- a/shell/platform/embedder/tests/embedder_config_builder.cc +++ b/shell/platform/embedder/tests/embedder_config_builder.cc @@ -261,6 +261,13 @@ void EmbedderConfigBuilder::SetPlatformTaskRunner( project_args_.custom_task_runners = &custom_task_runners_; } +void EmbedderConfigBuilder::SetupVsyncCallback() { + project_args_.vsync_callback = [](void* user_data, intptr_t baton) { + auto context = reinterpret_cast(user_data); + context->RunVsyncCallback(baton); + }; +} + void EmbedderConfigBuilder::SetRenderTaskRunner( const FlutterTaskRunnerDescription* runner) { if (runner == nullptr) { diff --git a/shell/platform/embedder/tests/embedder_config_builder.h b/shell/platform/embedder/tests/embedder_config_builder.h index 40f9bfc85a560..17a1dd4161389 100644 --- a/shell/platform/embedder/tests/embedder_config_builder.h +++ b/shell/platform/embedder/tests/embedder_config_builder.h @@ -104,6 +104,10 @@ class EmbedderConfigBuilder { UniqueEngine InitializeEngine() const; + // Sets up the callback for vsync, the callbacks needs to be specified on the + // text context vis `SetVsyncCallback`. + void SetupVsyncCallback(); + private: EmbedderTestContext& context_; FlutterProjectArgs project_args_ = {}; diff --git a/shell/platform/embedder/tests/embedder_test_context.cc b/shell/platform/embedder/tests/embedder_test_context.cc index 44d39a3326a96..79eb749e632a9 100644 --- a/shell/platform/embedder/tests/embedder_test_context.cc +++ b/shell/platform/embedder/tests/embedder_test_context.cc @@ -224,5 +224,14 @@ void EmbedderTestContext::FireRootSurfacePresentCallbackIfPresent( callback(image_callback()); } +void EmbedderTestContext::SetVsyncCallback( + std::function callback) { + vsync_callback_ = callback; +} + +void EmbedderTestContext::RunVsyncCallback(intptr_t baton) { + vsync_callback_(baton); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/embedder/tests/embedder_test_context.h b/shell/platform/embedder/tests/embedder_test_context.h index 366b5b0b52cfe..6efca542be181 100644 --- a/shell/platform/embedder/tests/embedder_test_context.h +++ b/shell/platform/embedder/tests/embedder_test_context.h @@ -89,6 +89,15 @@ class EmbedderTestContext { virtual EmbedderTestContextType GetContextType() const = 0; + // Sets up the callback for vsync. This callback will be invoked + // for every vsync. This should be used in conjunction with SetupVsyncCallback + // on the EmbedderConfigBuilder. Any callback setup here must call + // `FlutterEngineOnVsync` from the platform task runner. + void SetVsyncCallback(std::function callback); + + // Runs the vsync callback. + void RunVsyncCallback(intptr_t baton); + // TODO(gw280): encapsulate these properly for subclasses to use protected: // This allows the builder to access the hooks. @@ -112,6 +121,7 @@ class EmbedderTestContext { std::unique_ptr compositor_; NextSceneCallback next_scene_callback_; SkMatrix root_surface_transformation_; + std::function vsync_callback_ = nullptr; static VoidCallback GetIsolateCreateCallbackHook(); diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index ea9904d9c0e1f..929cb264b5b38 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "fml/task_runner.h" #define FML_USED_ON_EMBEDDER #include @@ -18,7 +17,10 @@ #include "flutter/fml/paths.h" #include "flutter/fml/synchronization/count_down_latch.h" #include "flutter/fml/synchronization/waitable_event.h" +#include "flutter/fml/task_runner.h" #include "flutter/fml/thread.h" +#include "flutter/fml/time/time_delta.h" +#include "flutter/fml/time/time_point.h" #include "flutter/runtime/dart_vm.h" #include "flutter/shell/platform/embedder/tests/embedder_assertions.h" #include "flutter/shell/platform/embedder/tests/embedder_config_builder.h" @@ -29,6 +31,16 @@ #include "third_party/skia/include/core/SkSurface.h" #include "third_party/tonic/converter/dart_converter.h" +namespace { + +static uint64_t NanosFromEpoch(int millis_from_now) { + const auto now = fml::TimePoint::Now(); + const auto delta = fml::TimeDelta::FromMilliseconds(millis_from_now); + return (now + delta).ToEpochDelta().ToNanoseconds(); +} + +} // namespace + namespace flutter { namespace testing { @@ -1575,5 +1587,60 @@ TEST_F(EmbedderTest, BackToBackKeyEventResponsesCorrectlyInvoked) { shutdown_latch.Wait(); } +// This test schedules a frame for the future and asserts that vsync waiter +// posts the event at the right frame start time (which is in the future). +TEST_F(EmbedderTest, VsyncCallbackPostedIntoFuture) { + UniqueEngine engine; + fml::AutoResetWaitableEvent present_latch; + + // One of the threads that the callback (FlutterEngineOnVsync) will be posted + // to is the platform thread. So we cannot wait for assertions to complete on + // the platform thread. Create a new thread to manage the engine instance and + // wait for assertions on the test thread. + auto platform_task_runner = CreateNewThread("platform_thread"); + + platform_task_runner->PostTask([&]() { + auto& context = + GetEmbedderContext(EmbedderTestContextType::kSoftwareContext); + + context.SetVsyncCallback([&](intptr_t baton) { + platform_task_runner->PostTask([baton = baton, engine = engine.get()]() { + FlutterEngineOnVsync(engine, baton, NanosFromEpoch(16), + NanosFromEpoch(32)); + }); + }); + context.AddNativeCallback( + "SignalNativeTest", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + present_latch.Signal(); + })); + + EmbedderConfigBuilder builder(context); + builder.SetSoftwareRendererConfig(); + builder.SetupVsyncCallback(); + builder.SetDartEntrypoint("empty_scene"); + engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + + // Send a window metrics events so frames may be scheduled. + FlutterWindowMetricsEvent event = {}; + event.struct_size = sizeof(event); + event.width = 800; + event.height = 600; + event.pixel_ratio = 1.0; + + ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), + kSuccess); + }); + + present_latch.Wait(); + + fml::AutoResetWaitableEvent shutdown_latch; + platform_task_runner->PostTask([&]() { + engine.reset(); + shutdown_latch.Signal(); + }); + shutdown_latch.Wait(); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/embedder/vsync_waiter_embedder.cc b/shell/platform/embedder/vsync_waiter_embedder.cc index a3d483f51b41f..bf566929e5a6f 100644 --- a/shell/platform/embedder/vsync_waiter_embedder.cc +++ b/shell/platform/embedder/vsync_waiter_embedder.cc @@ -17,26 +17,35 @@ VsyncWaiterEmbedder::~VsyncWaiterEmbedder() = default; // |VsyncWaiter| void VsyncWaiterEmbedder::AwaitVSync() { auto* weak_waiter = new std::weak_ptr(shared_from_this()); - vsync_callback_(reinterpret_cast(weak_waiter)); + intptr_t baton = reinterpret_cast(weak_waiter); + vsync_callback_(baton); } // static -bool VsyncWaiterEmbedder::OnEmbedderVsync(intptr_t baton, - fml::TimePoint frame_start_time, - fml::TimePoint frame_target_time) { +bool VsyncWaiterEmbedder::OnEmbedderVsync( + const flutter::TaskRunners& task_runners, + intptr_t baton, + fml::TimePoint frame_start_time, + fml::TimePoint frame_target_time) { if (baton == 0) { return false; } - auto* weak_waiter = reinterpret_cast*>(baton); - auto strong_waiter = weak_waiter->lock(); - delete weak_waiter; + // If the time here is in the future, the contract for `FlutterEngineOnVsync` + // says that the engine will only process the frame when the time becomes + // current. + task_runners.GetUITaskRunner()->PostTaskForTime( + [frame_start_time, frame_target_time, baton]() { + std::weak_ptr* weak_waiter = + reinterpret_cast*>(baton); + auto vsync_waiter = weak_waiter->lock(); + delete weak_waiter; + if (vsync_waiter) { + vsync_waiter->FireCallback(frame_start_time, frame_target_time); + } + }, + frame_start_time); - if (!strong_waiter) { - return false; - } - - strong_waiter->FireCallback(frame_start_time, frame_target_time); return true; } diff --git a/shell/platform/embedder/vsync_waiter_embedder.h b/shell/platform/embedder/vsync_waiter_embedder.h index c1b06327c8fc3..24e98e433fd70 100644 --- a/shell/platform/embedder/vsync_waiter_embedder.h +++ b/shell/platform/embedder/vsync_waiter_embedder.h @@ -19,7 +19,8 @@ class VsyncWaiterEmbedder final : public VsyncWaiter { ~VsyncWaiterEmbedder() override; - static bool OnEmbedderVsync(intptr_t baton, + static bool OnEmbedderVsync(const flutter::TaskRunners& task_runners, + intptr_t baton, fml::TimePoint frame_start_time, fml::TimePoint frame_target_time);