Embedder VsyncWaiter must schedule a frame for the right time (#29408) (#29508) * Embedder VsyncWaiter must schedule a frame for the right time (#29408) * Ensure vsync callback completes before resetting the engine. (#29488) Fixes: https://github.com/flutter/flutter/issues/92934 Co-authored-by: Kaushik Iska <iska.kaushik@gmail.com>
diff --git a/shell/platform/embedder/embedder_engine.cc b/shell/platform/embedder/embedder_engine.cc index 7d1f74b..d96fa0c 100644 --- a/shell/platform/embedder/embedder_engine.cc +++ b/shell/platform/embedder/embedder_engine.cc
@@ -229,8 +229,8 @@ 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 ac6c86a..d04188e 100644 --- a/shell/platform/embedder/tests/embedder_config_builder.cc +++ b/shell/platform/embedder/tests/embedder_config_builder.cc
@@ -261,6 +261,13 @@ 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<EmbedderTestContext*>(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 40f9bfc..17a1dd4 100644 --- a/shell/platform/embedder/tests/embedder_config_builder.h +++ b/shell/platform/embedder/tests/embedder_config_builder.h
@@ -104,6 +104,10 @@ 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 44d39a3..79eb749 100644 --- a/shell/platform/embedder/tests/embedder_test_context.cc +++ b/shell/platform/embedder/tests/embedder_test_context.cc
@@ -224,5 +224,14 @@ callback(image_callback()); } +void EmbedderTestContext::SetVsyncCallback( + std::function<void(intptr_t)> 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 366b5b0..6efca54 100644 --- a/shell/platform/embedder/tests/embedder_test_context.h +++ b/shell/platform/embedder/tests/embedder_test_context.h
@@ -89,6 +89,15 @@ 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<void(intptr_t)> 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 @@ std::unique_ptr<EmbedderTestCompositor> compositor_; NextSceneCallback next_scene_callback_; SkMatrix root_surface_transformation_; + std::function<void(intptr_t)> 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 ea9904d..1998ebe 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 <string> @@ -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,63 @@ 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; + 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 + // 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, &vsync_latch]() { + FlutterEngineOnVsync(engine.get(), baton, NanosFromEpoch(16), + NanosFromEpoch(32)); + vsync_latch.Signal(); + }); + }); + 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); + }); + + vsync_latch.Wait(); + 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 a3d483f..bf56692 100644 --- a/shell/platform/embedder/vsync_waiter_embedder.cc +++ b/shell/platform/embedder/vsync_waiter_embedder.cc
@@ -17,26 +17,35 @@ // |VsyncWaiter| void VsyncWaiterEmbedder::AwaitVSync() { auto* weak_waiter = new std::weak_ptr<VsyncWaiter>(shared_from_this()); - vsync_callback_(reinterpret_cast<intptr_t>(weak_waiter)); + intptr_t baton = reinterpret_cast<intptr_t>(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<std::weak_ptr<VsyncWaiter>*>(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<VsyncWaiter>* weak_waiter = + reinterpret_cast<std::weak_ptr<VsyncWaiter>*>(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 c1b0632..24e98e4 100644 --- a/shell/platform/embedder/vsync_waiter_embedder.h +++ b/shell/platform/embedder/vsync_waiter_embedder.h
@@ -19,7 +19,8 @@ ~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);