add vsync_waiter comments

Author: farchond

shell/platform/fuchsia/flutter/vsync_waiter.cc

@@ -76,6 +76,31 @@ fml::TimePoint VsyncWaiter::SnapToNextPhase(
   }
 }
 
+// This function takes in all relevant information to determining when should
+// the next frame be scheduled. It returns a pair of (frame_start_time,
+// frame_end_time) to be passed into FireCallback().
+//
+// Importantly, there are two invariants for correct and performant scheduling
+// that this function upholds:
+// 1. Schedule the next frame at least half a vsync interval from the previous
+// one. In practice, this means that every vsync interval Flutter produces
+// exactly one frame in steady state behavior.
+// 2. Only produce frames beginning in the future.
+//
+// |vsync_offset| - the time from the next vsync that the animator should begin
+// working on the next frame. For instance if vsyncs are at 0ms, 16ms, and 33ms,
+// and the |vsync_offset| is 5ms, then frames should begin at 11ms and 28ms.
+//
+// |vsync_interval| - the interval between vsyncs. Would be 16.6ms for a 60Hz
+// display.
+//
+// |last_targetted_vsync| - the last vsync targetted, which is usually the
+// frame_end_time returned from the last invocation of this function
+//
+// |now| - the current time
+//
+// |next_vsync| - the next vsync after |now|. This can be generated using the
+// SnapToNextPhase function.
 FrameTargetTimes VsyncWaiter::GetTargetTimes(
     fml::TimeDelta vsync_offset,
     fml::TimeDelta vsync_interval,
@@ -94,7 +119,6 @@ FrameTargetTimes VsyncWaiter::GetTargetTimes(
 
   // Start the frame after Scenic has finished its CPU work. This is
   // accomplished using the vsync_offset.
-  // fml::TimePoint frame_start_time = next_vsync - vsync_offset;
   fml::TimeDelta vsync_offset2 = vsync_interval - vsync_offset;
   fml::TimePoint frame_start_time =
       (next_vsync - vsync_interval) + vsync_offset2;
@@ -103,11 +127,7 @@ FrameTargetTimes VsyncWaiter::GetTargetTimes(
 
   std::pair<fml::TimePoint, fml::TimePoint> pair;
 
-  // Advance to next available slot. There are only two invariants:
-  // 1. Schedule the next frame at least half a vsync interval from the previous
-  // one. In practice, this means that every vsync interval Flutter produces
-  // exactly one frame in steady state behavior.
-  // 2. Only produce frames beginning in the future.
+  // Advance to next available slot, keeping in mind the two invariants.
   while (frame_end_time < (last_targetted_vsync + (vsync_interval / 2)) ||
          frame_start_time < now) {
     frame_start_time = frame_start_time + vsync_interval;
@@ -117,14 +137,15 @@ FrameTargetTimes VsyncWaiter::GetTargetTimes(
   pair.first = frame_start_time;
   pair.second = frame_end_time;
 
+  // Useful knowledge for analyzing traces.
   fml::TimePoint previous_vsync = next_vsync - vsync_interval;
   TRACE_DURATION(
-      "flutter", "VsyncWaiter::GetTargetTimes", "previous_vsync",
-      previous_vsync.ToEpochDelta().ToMilliseconds(), "last_targetted",
-      last_targetted_vsync.ToEpochDelta().ToMilliseconds(), "now",
-      fml::TimePoint::Now().ToEpochDelta().ToMilliseconds(), "next_vsync",
-      next_vsync.ToEpochDelta().ToMilliseconds(), "frame_start_time",
-      frame_start_time.ToEpochDelta().ToMilliseconds(), "frame_end_time",
+      "flutter", "VsyncWaiter::GetTargetTimes", "previous_vsync(ms)",
+      previous_vsync.ToEpochDelta().ToMilliseconds(), "last_targetted(ms)",
+      last_targetted_vsync.ToEpochDelta().ToMilliseconds(), "now(ms)",
+      fml::TimePoint::Now().ToEpochDelta().ToMilliseconds(), "next_vsync(ms))",
+      next_vsync.ToEpochDelta().ToMilliseconds(), "frame_start_time(ms)",
+      frame_start_time.ToEpochDelta().ToMilliseconds(), "frame_end_time(ms)",
       frame_end_time.ToEpochDelta().ToMilliseconds());
 
   return pair;
@@ -184,19 +205,15 @@ VsyncWaiter::~VsyncWaiter() {
   ui_latch.Wait();
 }
 
-// When the Animator wants to produce the next frame, it calls AwaitVsync() in
-// order for it to know when the best time to do that is.
-//
-// There are two cases: either we are able to do immedatitely, or we have
-// produced too many frames in a recent interval and need to wait for the
-// underlying system, namely Scenic, to signal that the previous enqueued work
-// has been processed and we can produce more work.
+// This function is called when the Animator wishes to schedule a new frame.
 void VsyncWaiter::AwaitVSync() {
   TRACE_DURATION("flutter", "VsyncWaiter::AwaitVsync()",
                  "request_already_pending", frame_request_pending_);
-  RequestAFrameToBeScheduled();
+  FireCallbackMaybe();
 }
 
+// This function is called when the Animator wants to know about the next vsync,
+// but not for frame scheduling purposes.
 void VsyncWaiter::AwaitVSyncForSecondaryCallback() {
   TRACE_DURATION("flutter", "VsyncWaiter::AwaitVsyncForSecondaryCallback()",
                  "request_already_pending", frame_request_pending_);
@@ -206,9 +223,9 @@ void VsyncWaiter::AwaitVSyncForSecondaryCallback() {
 }
 
 // Postcondition: Either a frame is scheduled or frame_request_pending_ is set
-// to true.
-void VsyncWaiter::RequestAFrameToBeScheduled() {
-  TRACE_DURATION("flutter", "RequestAFrameToBeScheduled");
+// to true, meaning we will attempt to schedule a frame on the next |OnVsync|.
+void VsyncWaiter::FireCallbackMaybe() {
+  TRACE_DURATION("flutter", "FireCallbackMaybe");
 
   if (session_connection_->CanRequestNewFrames()) {
     FrameTargetTimes times = GetTargetTimesHelper(/*secondary_callback=*/false);
@@ -223,15 +240,19 @@ void VsyncWaiter::RequestAFrameToBeScheduled() {
 }
 
 // This function is called when the SessionConnection signals us to let us know
-// that one or more frames Flutter has produced have been displayed.
+// that one or more frames Flutter has produced have been displayed. Note that
+// in practice this will be called several milliseconds after vsync, due to
+// CPU contention.
 void VsyncWaiter::OnVsync() {
   TRACE_DURATION("flutter", "VsyncWaiter::OnVsync");
 
   if (frame_request_pending_) {
-    RequestAFrameToBeScheduled();
+    FireCallbackMaybe();
   }
 }
 
+// A helper function for GetTargetTimes(), since many of the fields it takes
+// have to be derived from other state.
 FrameTargetTimes VsyncWaiter::GetTargetTimesHelper(bool secondary_callback) {
   fml::TimeDelta presentation_interval =
       VsyncRecorder::GetInstance().GetCurrentVsyncInfo().presentation_interval;

shell/platform/fuchsia/flutter/vsync_waiter.h

@@ -44,7 +44,13 @@ class VsyncWaiter final : public flutter::VsyncWaiter {
   ~VsyncWaiter() override;
 
  private:
-  void RequestAFrameToBeScheduled();
+  // |flutter::VsyncWaiter|
+  void AwaitVSync() override;
+
+  // |flutter::VsyncWaiter|
+  void AwaitVSyncForSecondaryCallback() override;
+
+  void FireCallbackMaybe();
   void OnVsync();
 
   FrameTargetTimes GetTargetTimesHelper(bool secondary_callback);
@@ -61,20 +67,12 @@ class VsyncWaiter final : public flutter::VsyncWaiter {
   // and that Flutter is not producing multiple frames in a small interval.
   fml::TimePoint last_targetted_vsync_;
 
-  // This is true iff AwaitVsync() was called but we could not schedule a frame.
+  // This is true iff AwaitVSync() was called but we could not schedule a frame.
   bool frame_request_pending_ = false;
 
-  // For accessing the VsyncWaiter via the UI thread, necessary for the callback
-  // for AwaitVSync()
   std::unique_ptr<fml::WeakPtrFactory<VsyncWaiter>> weak_factory_ui_;
   fml::WeakPtrFactory<VsyncWaiter> weak_factory_;
 
-  // |flutter::VsyncWaiter|
-  void AwaitVSync() override;
-
-  // |flutter::VsyncWaiter|
-  void AwaitVSyncForSecondaryCallback() override;
-
   FML_DISALLOW_COPY_AND_ASSIGN(VsyncWaiter);
 };