cc: Emulate BeginFrame in OutputSurfaces that don't support it natively

cc/scheduler/scheduler.cc

Index: cc/scheduler/scheduler.cc
diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index 737ffc1bb1222b2af5fec3d72e7a468abd4fa2ad..7f410a7ed5937fc228e2de1dc24b473c5ce57cc5 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -7,23 +7,33 @@
 #include "base/auto_reset.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
+#include "cc/base/thread.h"
 
 namespace cc {
 
 Scheduler::Scheduler(SchedulerClient* client,
-                     scoped_ptr<FrameRateController> frame_rate_controller,
-                     const SchedulerSettings& scheduler_settings)
+                     const SchedulerSettings& scheduler_settings,
+                     Thread *thread)
     : settings_(scheduler_settings),
       client_(client),
-      frame_rate_controller_(frame_rate_controller.Pass()),
+      weak_factory_(this),
+      thread_(thread),
+      last_set_needs_begin_frame_(false),
+      pending_begin_frames_(0),
+      incomplete_swaps_(0),
+      last_begin_frame_time_(base::TimeTicks()),
+      anticipated_draw_time_(base::TimeTicks::Now()),
+      //TODO(brianderson): Pass with BeginFrame in the near future.
+      interval_(base::TimeDelta::FromMicroseconds(16666)),
       state_machine_(scheduler_settings),
       inside_process_scheduled_actions_(false) {
   DCHECK(client_);
-  frame_rate_controller_->SetClient(this);
   DCHECK(!state_machine_.BeginFrameNeededByImplThread());
 }
 
-Scheduler::~Scheduler() { frame_rate_controller_->SetActive(false); }
+Scheduler::~Scheduler() {
+  client_->SetNeedsBeginFrameOnImplThread(false);
+}
 
 void Scheduler::SetCanStart() {
   state_machine_.SetCanStart();
@@ -88,27 +98,6 @@ void Scheduler::BeginFrameAbortedByMainThread() {
   ProcessScheduledActions();
 }
 
-void Scheduler::SetMaxFramesPending(int max_frames_pending) {
-  frame_rate_controller_->SetMaxFramesPending(max_frames_pending);
-}
-
-int Scheduler::MaxFramesPending() const {
-  return frame_rate_controller_->MaxFramesPending();
-}
-
-int Scheduler::NumFramesPendingForTesting() const {
-  return frame_rate_controller_->NumFramesPendingForTesting();
-}
-
-void Scheduler::SetSwapBuffersCompleteSupported(bool supported) {
-  frame_rate_controller_->SetSwapBuffersCompleteSupported(supported);
-}
-
-void Scheduler::DidSwapBuffersComplete() {
-  TRACE_EVENT0("cc", "Scheduler::DidSwapBuffersComplete");
-  frame_rate_controller_->DidSwapBuffersComplete();
-}
-
 void Scheduler::DidLoseOutputSurface() {
   TRACE_EVENT0("cc", "Scheduler::DidLoseOutputSurface");
   state_machine_.DidLoseOutputSurface();
@@ -117,31 +106,78 @@ void Scheduler::DidLoseOutputSurface() {
 
 void Scheduler::DidCreateAndInitializeOutputSurface() {
   TRACE_EVENT0("cc", "Scheduler::DidCreateAndInitializeOutputSurface");
-  frame_rate_controller_->DidAbortAllPendingFrames();
   state_machine_.DidCreateAndInitializeOutputSurface();
+  last_set_needs_begin_frame_ = false;
   ProcessScheduledActions();
 }
 
-void Scheduler::SetTimebaseAndInterval(base::TimeTicks timebase,
-                                       base::TimeDelta interval) {
-  frame_rate_controller_->SetTimebaseAndInterval(timebase, interval);
-}
-
 base::TimeTicks Scheduler::AnticipatedDrawTime() {
-  return frame_rate_controller_->NextTickTime();
+  TRACE_EVENT0("cc", "Scheduler::AnticipatedDrawTime");
+  base::TimeTicks now = base::TimeTicks::Now();
+  while (anticipated_draw_time_ < now) {

[nduca, 2013/06/04 04:53:59]

i think delay based timesource has some crap in it to compute this without a
loop

iirw, its  floor((now - antipcated draw time) / interval_) * interval_  +
anticipated_draw_time_;

[brianderson, 2013/06/04 18:33:27]

Will change this.

+    anticipated_draw_time_ += interval_;
+  }
+
+  return anticipated_draw_time_;
 }
 
 base::TimeTicks Scheduler::LastBeginFrameOnImplThreadTime() {
-  return frame_rate_controller_->LastTickTime();
+  return last_begin_frame_time_;
+}
+
+void Scheduler::SetupNextBeginFrameIfNeeded() {
+  // Determine if we need BeginFrame notifications.
+  // If we do, always request the BeginFrame immediately.
+  // If not, only disable on the next BeginFrame to avoid unnecessary toggles.
+  // The synchronous renderer compositor requires immediate disables though.
+  bool needs_begin_frame = state_machine_.BeginFrameNeededByImplThread();
+  if ((needs_begin_frame ||
+       state_machine_.inside_begin_frame() ||
+       settings_.using_synchronous_renderer_compositor) &&
+      (needs_begin_frame != last_set_needs_begin_frame_)) {
+    client_->SetNeedsBeginFrameOnImplThread(needs_begin_frame);
+    last_set_needs_begin_frame_ = needs_begin_frame;
+  }
+
+  if (state_machine_.inside_begin_frame() && pending_begin_frames_) {
+    // Self tick to retry a BeginFrame where we did not draw.
+    // TODO(brianderson): Implement smarter polling when deadlines are added.
+    thread_->PostDelayedTask(
+        base::Bind(&Scheduler::BeginFrameRetry, weak_factory_.GetWeakPtr()),
+        interval_);
+  }
+}
+
+void Scheduler::BeginFrame(base::TimeTicks frame_time) {
+  TRACE_EVENT0("cc", "Scheduler::BeginFrame");
+  pending_begin_frames_++;

[nduca, 2013/06/04 04:53:59]

okay this bit i dont get. why would we ever have this?

[brianderson, 2013/06/04 18:33:27]

In this patch, pending_begin_frames_ will only ever be 0 or 1. I kept it a
counter though, in case we ever wanted to have more than 1 BeginFrame pending
for pipelining reasons.

Using it as a counter now implies something that isn't true (that there can be
more than one BeginFrame pending), which is confusing, so I will change it to a
bool

+  last_begin_frame_time_ = frame_time;
+  anticipated_draw_time_ = frame_time + interval_;
+  state_machine_.DidEnterBeginFrame();
+  ProcessScheduledActions();
+  state_machine_.DidLeaveBeginFrame();
 }
 
-void Scheduler::BeginFrame(bool throttled) {
-  TRACE_EVENT1("cc", "Scheduler::BeginFrame", "throttled", throttled);
-  if (!throttled)
-    state_machine_.DidEnterBeginFrame();
+void Scheduler::BeginFrameRetry() {
+  TRACE_EVENT0("cc", "Scheduler::BeginFrameRetry");
+  state_machine_.DidEnterBeginFrame();
   ProcessScheduledActions();
-  if (!throttled)
-    state_machine_.DidLeaveBeginFrame();
+  state_machine_.DidLeaveBeginFrame();
+}
+
+void Scheduler::DrawAndSwapIfPossible() {
+  ScheduledActionDrawAndSwapResult result =
+      client_->ScheduledActionDrawAndSwapIfPossible();
+  state_machine_.DidDrawIfPossibleCompleted(result.did_draw);
+  if (result.did_swap)
+    pending_begin_frames_--;
+}
+
+void Scheduler::DrawAndSwapForced() {
+  ScheduledActionDrawAndSwapResult result =
+      client_->ScheduledActionDrawAndSwapForced();
+  if (result.did_swap)
+    pending_begin_frames_--;
 }
 
 void Scheduler::ProcessScheduledActions() {
@@ -155,8 +191,10 @@ void Scheduler::ProcessScheduledActions() {
   SchedulerStateMachine::Action action = state_machine_.NextAction();
   while (action != SchedulerStateMachine::ACTION_NONE) {
     state_machine_.UpdateState(action);
-    TRACE_EVENT1(
-        "cc", "Scheduler::ProcessScheduledActions()", "action", action);
+    TRACE_EVENT2(
+        "cc", "Scheduler::ProcessScheduledActions()",
+        "action", action,
+        "state", state_machine_.ToString());
 
     switch (action) {
       case SchedulerStateMachine::ACTION_NONE:
@@ -173,21 +211,12 @@ void Scheduler::ProcessScheduledActions() {
       case SchedulerStateMachine::ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED:
         client_->ScheduledActionActivatePendingTreeIfNeeded();
         break;
-      case SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE: {
-        ScheduledActionDrawAndSwapResult result =
-            client_->ScheduledActionDrawAndSwapIfPossible();
-        state_machine_.DidDrawIfPossibleCompleted(result.did_draw);
-        if (result.did_swap)
-          frame_rate_controller_->DidSwapBuffers();
+      case SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE:
+        DrawAndSwapIfPossible();
         break;
-      }
-      case SchedulerStateMachine::ACTION_DRAW_FORCED: {
-        ScheduledActionDrawAndSwapResult result =
-            client_->ScheduledActionDrawAndSwapForced();
-        if (result.did_swap)
-          frame_rate_controller_->DidSwapBuffers();
+      case SchedulerStateMachine::ACTION_DRAW_FORCED:
+        DrawAndSwapForced();
         break;
-      }
       case SchedulerStateMachine::ACTION_BEGIN_OUTPUT_SURFACE_CREATION:
         client_->ScheduledActionBeginOutputSurfaceCreation();
         break;
@@ -198,10 +227,8 @@ void Scheduler::ProcessScheduledActions() {
     action = state_machine_.NextAction();
   }
 
-  // Activate or deactivate the frame rate controller.
-  frame_rate_controller_->SetActive(
-      state_machine_.BeginFrameNeededByImplThread());
-  client_->DidAnticipatedDrawTimeChange(frame_rate_controller_->NextTickTime());
+  SetupNextBeginFrameIfNeeded();

[nduca, 2013/06/04 04:53:59]

This feels like more data suggesting that the tasks should live in the output
surface impl [and maybe all the existing output surfaces should have a output
surface impl base class so we can have common code]

[brianderson, 2013/06/04 18:33:27]

I will revisit this idea in future patches that add the deadline.

+  client_->DidAnticipatedDrawTimeChange(AnticipatedDrawTime());

[nduca, 2013/06/04 04:53:59]

so, if you actually look at who needs this, its the resource updater. And,
whereas in the scheduler we have to think that this might change anytime, when
youre in the output surface, you know exactly when the output time changes. THis
is yet more data to me that suggests output surface should just call
BeginFrame(ts, draw deadline, etc) and then that calls to scheduler, and to
resource update controller.

[brianderson, 2013/06/04 18:33:27]

This patch is already pretty big, so I opened another bug to fix this in a
future patch:
https://code.google.com/p/chromium/issues/detail?id=246714

 }
 
 bool Scheduler::WillDrawIfNeeded() const {