diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_connection.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_connection.cc index 895b374da8..22af7db0c0 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_connection.cc +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_connection.cc @@ -37,6 +37,10 @@ FlatlandConnection::~FlatlandConnection() = default; // This method is called from the raster thread. void FlatlandConnection::Present() { + if (!threadsafe_state_.first_present_called_) { + std::scoped_lock lock(threadsafe_state_.mutex_); + threadsafe_state_.first_present_called_ = true; + } if (present_credits_ > 0) { DoPresent(); } else { @@ -67,25 +71,26 @@ void FlatlandConnection::DoPresent() { // This method is called from the UI thread. void FlatlandConnection::AwaitVsync(FireCallbackCallback callback) { - if (first_call_to_await_vsync_) { + std::scoped_lock lock(threadsafe_state_.mutex_); + + // Immediately fire callbacks until the first Present. We might receive + // multiple requests for AwaitVsync() until the first Present, which relies on + // receiving size on FlatlandPlatformView::OnGetLayout() at an uncertain time. + if (!threadsafe_state_.first_present_called_) { fml::TimePoint now = fml::TimePoint::Now(); callback(now, now + kDefaultFlatlandPresentationInterval); - first_call_to_await_vsync_ = false; return; } - { - std::scoped_lock lock(threadsafe_state_.mutex_); - threadsafe_state_.fire_callback_ = callback; + threadsafe_state_.fire_callback_ = callback; - if (threadsafe_state_.fire_callback_pending_) { - fml::TimePoint now = fml::TimePoint::Now(); - // TODO(fxbug.dev/64201): Calculate correct frame times. - threadsafe_state_.fire_callback_( - now, now + kDefaultFlatlandPresentationInterval); - threadsafe_state_.fire_callback_ = nullptr; - threadsafe_state_.fire_callback_pending_ = false; - } + if (threadsafe_state_.fire_callback_pending_) { + fml::TimePoint now = fml::TimePoint::Now(); + // TODO(fxbug.dev/64201): Calculate correct frame times. + threadsafe_state_.fire_callback_( + now, now + kDefaultFlatlandPresentationInterval); + threadsafe_state_.fire_callback_ = nullptr; + threadsafe_state_.fire_callback_pending_ = false; } } diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_connection.h b/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_connection.h index 89f3a01206..72940ace24 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_connection.h +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_connection.h @@ -80,16 +80,15 @@ class FlatlandConnection final { bool present_pending_ = false; // This struct contains state that is accessed from both from the UI thread - // (in AwaitVsync) and the raster thread (in OnNextFrameBegin). You should - // always lock mutex_ before touching anything in this struct + // (in AwaitVsync) and the raster thread (in OnNextFrameBegin and Present). + // You should always lock mutex_ before touching anything in this struct struct { std::mutex mutex_; FireCallbackCallback fire_callback_; bool fire_callback_pending_ = false; + bool first_present_called_ = false; } threadsafe_state_; - bool first_call_to_await_vsync_ = true; - std::vector acquire_fences_; std::vector current_present_release_fences_; std::vector previous_present_release_fences_; diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/flatland_connection_unittests.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/flatland_connection_unittests.cc index c161fe6c02..5da43cc8e1 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/flatland_connection_unittests.cc +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/flatland_connection_unittests.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -194,17 +195,52 @@ TEST_F(FlatlandConnectionTest, BasicPresent) { EXPECT_EQ(release_fence_handle, first_release_fence_handle); } +TEST_F(FlatlandConnectionTest, AwaitVsyncBeforePresent) { + // Set up callbacks which allow sensing of how many presents were handled. + size_t presents_called = 0u; + fake_flatland().SetPresentHandler( + [&presents_called](auto present_args) { presents_called++; }); + + // Create the FlatlandConnection but don't pump the loop. No FIDL calls are + // completed yet. + flutter_runner::FlatlandConnection flatland_connection( + GetCurrentTestName(), TakeFlatlandHandle(), []() { FAIL(); }, + [](auto...) {}, 1, fml::TimeDelta::Zero()); + EXPECT_EQ(presents_called, 0u); + + // Pump the loop. Nothing is called. + loop().RunUntilIdle(); + EXPECT_EQ(presents_called, 0u); + + // Simulate an AwaitVsync that comes before the first Present. + bool await_vsync_callback_fired = false; + AwaitVsyncChecked(flatland_connection, await_vsync_callback_fired, + kDefaultFlatlandPresentationInterval); + EXPECT_TRUE(await_vsync_callback_fired); + + // Another AwaitVsync that comes before the first Present. + await_vsync_callback_fired = false; + AwaitVsyncChecked(flatland_connection, await_vsync_callback_fired, + kDefaultFlatlandPresentationInterval); + EXPECT_TRUE(await_vsync_callback_fired); + + // Queue Present. + flatland_connection.Present(); + loop().RunUntilIdle(); + EXPECT_EQ(presents_called, 1u); + + // Set the callback with AwaitVsync, callback should not be fired + await_vsync_callback_fired = false; + AwaitVsyncChecked(flatland_connection, await_vsync_callback_fired, + kDefaultFlatlandPresentationInterval); + EXPECT_FALSE(await_vsync_callback_fired); +} + TEST_F(FlatlandConnectionTest, OutOfOrderAwait) { // Set up callbacks which allow sensing of how many presents were handled. size_t presents_called = 0u; - zx_handle_t release_fence_handle; - fake_flatland().SetPresentHandler([&presents_called, - &release_fence_handle](auto present_args) { - presents_called++; - release_fence_handle = present_args.release_fences().empty() - ? ZX_HANDLE_INVALID - : present_args.release_fences().front().get(); - }); + fake_flatland().SetPresentHandler( + [&presents_called](auto present_args) { presents_called++; }); // Set up a callback which allows sensing of how many vsync's // (`OnFramePresented` events) were handled. @@ -226,12 +262,17 @@ TEST_F(FlatlandConnectionTest, OutOfOrderAwait) { EXPECT_EQ(presents_called, 0u); EXPECT_EQ(vsyncs_handled, 0u); - // Simulate an AwaitVsync that comes after the first call. + // Simulate an AwaitVsync that comes before the first Present. bool await_vsync_callback_fired = false; AwaitVsyncChecked(flatland_connection, await_vsync_callback_fired, kDefaultFlatlandPresentationInterval); EXPECT_TRUE(await_vsync_callback_fired); + // Queue Present. + flatland_connection.Present(); + loop().RunUntilIdle(); + EXPECT_EQ(presents_called, 1u); + // Set the callback with AwaitVsync, callback should not be fired await_vsync_callback_fired = false; AwaitVsyncChecked(flatland_connection, await_vsync_callback_fired, @@ -304,7 +345,7 @@ TEST_F(FlatlandConnectionTest, PresentCreditExhaustion) { loop().RunUntilIdle(); EXPECT_EQ(num_presents_called, 0u); - // Simulate an AwaitVsync that comes after the first call. + // Simulate an AwaitVsync that comes before the first Present. flatland_connection.AwaitVsync([](fml::TimePoint, fml::TimePoint) {}); loop().RunUntilIdle(); EXPECT_EQ(num_presents_called, 0u); @@ -312,28 +353,25 @@ TEST_F(FlatlandConnectionTest, PresentCreditExhaustion) { // This test uses a fire callback that triggers Present() with a single // acquire and release fence in order to approximate the behavior of the real // flutter fire callback and let us drive presents with ONFBs - auto fire_callback = [&flatland_connection](fml::TimePoint frame_start, - fml::TimePoint frame_end) { - zx::event acquire_fence; - zx::event::create(0, &acquire_fence); - zx::event release_fence; - zx::event::create(0, &release_fence); - flatland_connection.EnqueueAcquireFence(std::move(acquire_fence)); - flatland_connection.EnqueueReleaseFence(std::move(release_fence)); - flatland_connection.Present(); + auto fire_callback = [dispatcher = loop().dispatcher(), &flatland_connection]( + fml::TimePoint frame_start, + fml::TimePoint frame_end) { + async::PostTask(dispatcher, [&flatland_connection]() { + zx::event acquire_fence; + zx::event::create(0, &acquire_fence); + zx::event release_fence; + zx::event::create(0, &release_fence); + flatland_connection.EnqueueAcquireFence(std::move(acquire_fence)); + flatland_connection.EnqueueReleaseFence(std::move(release_fence)); + flatland_connection.Present(); + }); }; - // Call Await Vsync with a callback that triggers Present, but this should not - // present until ONFB is delivered below. + // Call Await Vsync with a callback that triggers Present and consumes the one + // and only present credit we start with. reset_test_counters(); flatland_connection.AwaitVsync(fire_callback); loop().RunUntilIdle(); - EXPECT_EQ(num_presents_called, 0u); - - // Call ONFB ands supply 0 present credits. This causes `Present` to be - // called and consumes the one and only present credit we start with. - OnNextFrameBegin(0); - loop().RunUntilIdle(); EXPECT_EQ(num_presents_called, 1u); EXPECT_EQ(num_acquire_fences, 1u); EXPECT_EQ(num_release_fences, 0u);