From 0128075225f23992f784572cca42051acaea24ab Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Mon, 1 Nov 2021 13:34:41 -0400 Subject: [PATCH 1/2] Embedder VsyncWaiter must schedule a frame for the right time (#29408) --- 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); From 97cbf38f37a16c0114ddcac744538425d37d5fc1 Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Tue, 2 Nov 2021 18:38:42 -0400 Subject: [PATCH 2/2] Ensure vsync callback completes before resetting the engine. (#29488) Fixes: https://github.com/flutter/flutter/issues/92934 --- shell/platform/embedder/tests/embedder_unittests.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 929cb264b5b38..1998ebec4359f 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -1592,6 +1592,7 @@ TEST_F(EmbedderTest, BackToBackKeyEventResponsesCorrectlyInvoked) { TEST_F(EmbedderTest, VsyncCallbackPostedIntoFuture) { UniqueEngine engine; fml::AutoResetWaitableEvent present_latch; + fml::AutoResetWaitableEvent vsync_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 @@ -1604,9 +1605,10 @@ TEST_F(EmbedderTest, VsyncCallbackPostedIntoFuture) { GetEmbedderContext(EmbedderTestContextType::kSoftwareContext); context.SetVsyncCallback([&](intptr_t baton) { - platform_task_runner->PostTask([baton = baton, engine = engine.get()]() { - FlutterEngineOnVsync(engine, baton, NanosFromEpoch(16), + platform_task_runner->PostTask([baton = baton, &engine, &vsync_latch]() { + FlutterEngineOnVsync(engine.get(), baton, NanosFromEpoch(16), NanosFromEpoch(32)); + vsync_latch.Signal(); }); }); context.AddNativeCallback( @@ -1632,6 +1634,7 @@ TEST_F(EmbedderTest, VsyncCallbackPostedIntoFuture) { kSuccess); }); + vsync_latch.Wait(); present_latch.Wait(); fml::AutoResetWaitableEvent shutdown_latch;