[fuchsia] Restructure Flatland vsync loop (#45531)

This CL restructures Flatland vsync loop to fire for each vsync instead
of each OnNextFrameBegin. As shown in the traces attached to the bug,
the current implementation of firing callbacks on each OnNextFrameBegin
causes skips when Flutter has longer draw calls. By scheduling frames in
between, we are increasing the chance of sending one before the latch
point. OnNextFrameBegin is now used to keep track of present credits and
future presentation times as well as when to start frame, replacing the
need for max_frames_in_flight and vsync_offset fields.

Bug: b/296272449
(cherry picked from commit 633ba42)

Author: uysalere

shell/platform/fuchsia/flutter/engine.cc

@@ -354,8 +354,6 @@ void Engine::Initialize(
        view_protocols = std::move(view_protocols),
        request = parent_viewport_watcher.NewRequest(),
        view_ref_pair = std::move(view_ref_pair),
-       max_frames_in_flight = product_config.get_max_frames_in_flight(),
-       vsync_offset = product_config.get_vsync_offset(),
        software_rendering = product_config.software_rendering()]() mutable {
         if (software_rendering) {
           surface_producer_ = std::make_shared<SoftwareSurfaceProducer>();
@@ -365,8 +363,7 @@ void Engine::Initialize(
 
         flatland_connection_ = std::make_shared<FlatlandConnection>(
             thread_label_, std::move(flatland),
-            std::move(session_error_callback), [](auto) {},
-            max_frames_in_flight, vsync_offset);
+            std::move(session_error_callback), [](auto) {});
 
         fuchsia::ui::views::ViewIdentityOnCreation view_identity = {
             .view_ref = std::move(view_ref_pair.second),

shell/platform/fuchsia/flutter/flatland_connection.cc

@@ -13,11 +13,10 @@ namespace flutter_runner {
 
 namespace {
 
-fml::TimePoint GetNextPresentationTime(fml::TimePoint now,
-                                       fml::TimePoint next_presentation_time) {
-  return now > next_presentation_time
-             ? now + flutter_runner::kDefaultFlatlandPresentationInterval
-             : next_presentation_time;
+// Helper function for traces.
+double DeltaFromNowInNanoseconds(const fml::TimePoint& now,
+                                 const fml::TimePoint& time) {
+  return (time - now).ToNanoseconds();
 }
 
 }  // namespace
@@ -26,9 +25,7 @@ FlatlandConnection::FlatlandConnection(
     std::string debug_label,
     fuchsia::ui::composition::FlatlandHandle flatland,
     fml::closure error_callback,
-    on_frame_presented_event on_frame_presented_callback,
-    uint64_t max_frames_in_flight,
-    fml::TimeDelta vsync_offset)
+    on_frame_presented_event on_frame_presented_callback)
     : flatland_(flatland.Bind()),
       error_callback_(error_callback),
       on_frame_presented_callback_(std::move(on_frame_presented_callback)) {
@@ -49,11 +46,9 @@ FlatlandConnection::~FlatlandConnection() = default;
 
 // This method is called from the raster thread.
 void FlatlandConnection::Present() {
-  if (!threadsafe_state_.first_present_called_) {
-    std::scoped_lock<std::mutex> lock(threadsafe_state_.mutex_);
-    threadsafe_state_.first_present_called_ = true;
-  }
-  if (present_credits_ > 0) {
+  TRACE_DURATION("flutter", "FlatlandConnection::Present");
+  std::scoped_lock<std::mutex> lock(threadsafe_state_.mutex_);
+  if (threadsafe_state_.present_credits_ > 0) {
     DoPresent();
   } else {
     present_waiting_for_credit_ = true;
@@ -66,14 +61,15 @@ void FlatlandConnection::DoPresent() {
   TRACE_FLOW_BEGIN("gfx", "Flatland::Present", next_present_trace_id_);
   ++next_present_trace_id_;
 
-  FML_CHECK(present_credits_ > 0);
-  --present_credits_;
+  FML_CHECK(threadsafe_state_.present_credits_ > 0);
+  --threadsafe_state_.present_credits_;
 
   fuchsia::ui::composition::PresentArgs present_args;
   present_args.set_requested_presentation_time(0);
   present_args.set_acquire_fences(std::move(acquire_fences_));
   present_args.set_release_fences(std::move(previous_present_release_fences_));
-  present_args.set_unsquashable(false);
+  // Frame rate over latency.
+  present_args.set_unsquashable(true);
   flatland_->Present(std::move(present_args));
 
   // In Flatland, release fences apply to the content of the previous present.
@@ -86,38 +82,44 @@ void FlatlandConnection::DoPresent() {
 
 // This method is called from the UI thread.
 void FlatlandConnection::AwaitVsync(FireCallbackCallback callback) {
+  TRACE_DURATION("flutter", "FlatlandConnection::AwaitVsync");
+
   std::scoped_lock<std::mutex> lock(threadsafe_state_.mutex_);
-  const fml::TimePoint now = fml::TimePoint::Now();
+  threadsafe_state_.pending_fire_callback_ = nullptr;
+  const auto now = fml::TimePoint::Now();
 
-  // Immediately fire callbacks until the first Present. We might receive
-  // multiple requests for AwaitVsync() until the first Present, which relies on
-  // receiving size on PlatformView::OnGetLayout() at an uncertain time.
-  if (!threadsafe_state_.first_present_called_) {
-    callback(now, now + kDefaultFlatlandPresentationInterval);
+  // Initial case.
+  if (MaybeRunInitialVsyncCallback(now, callback))
     return;
-  }
-
-  threadsafe_state_.fire_callback_ = callback;
 
-  // Immediately fire callback if OnNextFrameBegin() is already called.
-  if (threadsafe_state_.on_next_frame_pending_) {
-    threadsafe_state_.fire_callback_(
-        now, GetNextPresentationTime(
-                 now, threadsafe_state_.next_presentation_time_));
-    threadsafe_state_.fire_callback_ = nullptr;
-    threadsafe_state_.on_next_frame_pending_ = false;
+  // Throttle case.
+  if (threadsafe_state_.present_credits_ == 0) {
+    threadsafe_state_.pending_fire_callback_ = callback;
+    return;
   }
+
+  // Regular case.
+  RunVsyncCallback(now, callback);
 }
 
 // This method is called from the UI thread.
 void FlatlandConnection::AwaitVsyncForSecondaryCallback(
     FireCallbackCallback callback) {
-  const fml::TimePoint now = fml::TimePoint::Now();
+  TRACE_DURATION("flutter",
+                 "FlatlandConnection::AwaitVsyncForSecondaryCallback");
+
   std::scoped_lock<std::mutex> lock(threadsafe_state_.mutex_);
-  callback(now, GetNextPresentationTime(
-                    now, threadsafe_state_.next_presentation_time_));
+  const auto now = fml::TimePoint::Now();
+
+  // Initial case.
+  if (MaybeRunInitialVsyncCallback(now, callback))
+    return;
+
+  // Regular case.
+  RunVsyncCallback(now, callback);
 }
 
+// This method is called from the raster thread.
 void FlatlandConnection::OnError(
     fuchsia::ui::composition::FlatlandError error) {
   FML_LOG(ERROR) << "Flatland error: " << static_cast<int>(error);
@@ -127,30 +129,62 @@ void FlatlandConnection::OnError(
 // This method is called from the raster thread.
 void FlatlandConnection::OnNextFrameBegin(
     fuchsia::ui::composition::OnNextFrameBeginValues values) {
-  present_credits_ += values.additional_present_credits();
+  // Collect now before locking because this is an important timing information
+  // from Scenic.
+  const auto now = fml::TimePoint::Now();
+
+  std::scoped_lock<std::mutex> lock(threadsafe_state_.mutex_);
+  threadsafe_state_.first_feedback_received_ = true;
+  threadsafe_state_.present_credits_ += values.additional_present_credits();
+  TRACE_DURATION("flutter", "FlatlandConnection::OnNextFrameBegin",
+                 "present_credits", threadsafe_state_.present_credits_);
 
-  if (present_waiting_for_credit_ && present_credits_ > 0) {
+  if (present_waiting_for_credit_ && threadsafe_state_.present_credits_ > 0) {
     DoPresent();
     present_waiting_for_credit_ = false;
   }
 
-  if (present_credits_ > 0) {
-    FML_CHECK(values.has_future_presentation_infos() &&
-              !values.future_presentation_infos().empty());
-    const auto next_presentation_time =
-        fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromNanoseconds(
-            values.future_presentation_infos().front().presentation_time()));
-
-    std::scoped_lock<std::mutex> lock(threadsafe_state_.mutex_);
-    if (threadsafe_state_.fire_callback_) {
-      threadsafe_state_.fire_callback_(
-          /*frame_start=*/fml::TimePoint::Now(),
-          /*frame_target=*/next_presentation_time);
-      threadsafe_state_.fire_callback_ = nullptr;
-    } else {
-      threadsafe_state_.on_next_frame_pending_ = true;
-    }
-    threadsafe_state_.next_presentation_time_ = next_presentation_time;
+  // Update vsync_interval_ by calculating the difference between the first two
+  // presentation times. Flatland always returns >1 presentation_infos, so this
+  // check is to guard against any changes to this assumption.
+  if (values.has_future_presentation_infos() &&
+      values.future_presentation_infos().size() > 1) {
+    threadsafe_state_.vsync_interval_ = fml::TimeDelta::FromNanoseconds(
+        values.future_presentation_infos().at(1).presentation_time() -
+        values.future_presentation_infos().at(0).presentation_time());
+  } else {
+    FML_LOG(WARNING)
+        << "Flatland didn't send enough future_presentation_infos to update "
+           "vsync interval.";
+  }
+
+  // Update next_presentation_times_.
+  std::queue<fml::TimePoint> new_times;
+  for (const auto& info : values.future_presentation_infos()) {
+    new_times.emplace(fml::TimePoint::FromEpochDelta(
+        fml::TimeDelta::FromNanoseconds(info.presentation_time())));
+  }
+  threadsafe_state_.next_presentation_times_.swap(new_times);
+
+  // Update vsync_offset_.
+  // We use modulo here because Flatland may point to the following vsync if
+  // OnNextFrameBegin() is called after the current frame's latch point.
+  auto vsync_offset =
+      (fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromNanoseconds(
+           values.future_presentation_infos().front().presentation_time())) -
+       now) %
+      threadsafe_state_.vsync_interval_;
+  // Thread contention may result in OnNextFrameBegin() being called after the
+  // presentation time. Ignore these outliers.
+  if (vsync_offset > fml::TimeDelta::Zero()) {
+    threadsafe_state_.vsync_offset_ = vsync_offset;
+  }
+
+  // Throttle case.
+  if (threadsafe_state_.pending_fire_callback_ &&
+      threadsafe_state_.present_credits_ > 0) {
+    RunVsyncCallback(now, threadsafe_state_.pending_fire_callback_);
+    threadsafe_state_.pending_fire_callback_ = nullptr;
   }
 }
 
@@ -160,6 +194,68 @@ void FlatlandConnection::OnFramePresented(
   on_frame_presented_callback_(std::move(info));
 }
 
+// Parses and updates next_presentation_times_.
+fml::TimePoint FlatlandConnection::GetNextPresentationTime(
+    const fml::TimePoint& now) {
+  const fml::TimePoint& cutoff =
+      now > threadsafe_state_.last_presentation_time_
+          ? now
+          : threadsafe_state_.last_presentation_time_;
+
+  // Remove presentation times that may have been passed. This may happen after
+  // a long draw call.
+  while (!threadsafe_state_.next_presentation_times_.empty() &&
+         threadsafe_state_.next_presentation_times_.front() <= cutoff) {
+    threadsafe_state_.next_presentation_times_.pop();
+  }
+
+  // Calculate a presentation time based on
+  // |threadsafe_state_.last_presentation_time_| that is later than cutoff using
+  // |vsync_interval| increments if we don't have any future presentation times
+  // left.
+  if (threadsafe_state_.next_presentation_times_.empty()) {
+    auto result = threadsafe_state_.last_presentation_time_;
+    while (result <= cutoff) {
+      result = result + threadsafe_state_.vsync_interval_;
+    }
+    return result;
+  }
+
+  // Return the next presentation time in the queue for the regular case.
+  const auto result = threadsafe_state_.next_presentation_times_.front();
+  threadsafe_state_.next_presentation_times_.pop();
+  return result;
+}
+
+// This method is called from the UI thread.
+bool FlatlandConnection::MaybeRunInitialVsyncCallback(
+    const fml::TimePoint& now,
+    FireCallbackCallback& callback) {
+  if (!threadsafe_state_.first_feedback_received_) {
+    TRACE_DURATION("flutter",
+                   "FlatlandConnection::MaybeRunInitialVsyncCallback");
+    const auto frame_end = now + kInitialFlatlandVsyncOffset;
+    threadsafe_state_.last_presentation_time_ = frame_end;
+    callback(now, frame_end);
+    return true;
+  }
+  return false;
+}
+
+// This method may be called from the raster or UI thread, but it is safe
+// because VsyncWaiter posts the vsync callback on UI thread.
+void FlatlandConnection::RunVsyncCallback(const fml::TimePoint& now,
+                                          FireCallbackCallback& callback) {
+  const auto& frame_end = GetNextPresentationTime(now);
+  const auto& frame_start = frame_end - threadsafe_state_.vsync_offset_;
+  threadsafe_state_.last_presentation_time_ = frame_end;
+  TRACE_DURATION("flutter", "FlatlandConnection::RunVsyncCallback",
+                 "frame_start_delta",
+                 DeltaFromNowInNanoseconds(now, frame_start), "frame_end_delta",
+                 DeltaFromNowInNanoseconds(now, frame_end));
+  callback(frame_start, frame_end);
+}
+
 // This method is called from the raster thread.
 void FlatlandConnection::EnqueueAcquireFence(zx::event fence) {
   acquire_fences_.push_back(std::move(fence));

shell/platform/fuchsia/flutter/flatland_connection.h

@@ -16,17 +16,18 @@
 
 #include <cstdint>
 #include <mutex>
+#include <queue>
 #include <string>
 
 namespace flutter_runner {
 
 using on_frame_presented_event =
     std::function<void(fuchsia::scenic::scheduling::FramePresentedInfo)>;
 
-// 2ms interval to target vsync is only used until Scenic sends presentation
-// feedback, or when we run out of present credits.
-static constexpr fml::TimeDelta kDefaultFlatlandPresentationInterval =
-    fml::TimeDelta::FromMilliseconds(2);
+// 10ms interval to target vsync is only used until Scenic sends presentation
+// feedback.
+static constexpr fml::TimeDelta kInitialFlatlandVsyncOffset =
+    fml::TimeDelta::FromMilliseconds(10);
 
 // The component residing on the raster thread that is responsible for
 // maintaining the Flatland instance connection and presenting updates.
@@ -35,9 +36,7 @@ class FlatlandConnection final {
   FlatlandConnection(std::string debug_label,
                      fuchsia::ui::composition::FlatlandHandle flatland,
                      fml::closure error_callback,
-                     on_frame_presented_event on_frame_presented_callback,
-                     uint64_t max_frames_in_flight,
-                     fml::TimeDelta vsync_offset);
+                     on_frame_presented_event on_frame_presented_callback);
 
   ~FlatlandConnection();
 
@@ -70,6 +69,12 @@ class FlatlandConnection final {
   void OnFramePresented(fuchsia::scenic::scheduling::FramePresentedInfo info);
   void DoPresent();
 
+  fml::TimePoint GetNextPresentationTime(const fml::TimePoint& now);
+  bool MaybeRunInitialVsyncCallback(const fml::TimePoint& now,
+                                    FireCallbackCallback& callback);
+  void RunVsyncCallback(const fml::TimePoint& now,
+                        FireCallbackCallback& callback);
+
   fuchsia::ui::composition::FlatlandPtr flatland_;
 
   fml::closure error_callback_;
@@ -78,7 +83,6 @@ class FlatlandConnection final {
   uint64_t next_content_id_ = 0;
 
   on_frame_presented_event on_frame_presented_callback_;
-  uint32_t present_credits_ = 1;
   bool present_waiting_for_credit_ = false;
 
   // A flow event trace id for following |Flatland::Present| calls into Scenic.
@@ -89,10 +93,13 @@ class FlatlandConnection final {
   // You should always lock mutex_ before touching anything in this struct
   struct {
     std::mutex mutex_;
-    FireCallbackCallback fire_callback_;
-    bool first_present_called_ = false;
-    bool on_next_frame_pending_ = false;
-    fml::TimePoint next_presentation_time_;
+    std::queue<fml::TimePoint> next_presentation_times_;
+    fml::TimeDelta vsync_interval_ = kInitialFlatlandVsyncOffset;
+    fml::TimeDelta vsync_offset_ = kInitialFlatlandVsyncOffset;
+    fml::TimePoint last_presentation_time_;
+    FireCallbackCallback pending_fire_callback_;
+    uint32_t present_credits_ = 1;
+    bool first_feedback_received_ = false;
   } threadsafe_state_;
 
   std::vector<zx::event> acquire_fences_;

shell/platform/fuchsia/flutter/flutter_runner_product_configuration.cc

@@ -23,18 +23,6 @@ FlutterRunnerProductConfiguration::FlutterRunnerProductConfiguration(
   }
 
   // Parse out all values we're expecting.
-  if (document.HasMember("vsync_offset_in_us")) {
-    auto& val = document["vsync_offset_in_us"];
-    if (val.IsInt()) {
-      vsync_offset_ = fml::TimeDelta::FromMicroseconds(val.GetInt());
-    }
-  }
-  if (document.HasMember("max_frames_in_flight")) {
-    auto& val = document["max_frames_in_flight"];
-    if (val.IsInt()) {
-      max_frames_in_flight_ = val.GetInt();
-    }
-  }
   if (document.HasMember("intercept_all_input")) {
     auto& val = document["intercept_all_input"];
     if (val.IsBool()) {