[fuchsia] Restructure Flatland vsync loop

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()) {