Unified BeginFrame scheduling

cc/scheduler/scheduler.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/scheduler/scheduler.h"
 
 #include <algorithm>
 #include "base/auto_reset.h"
 #include "base/debug/trace_event.h"
 #include "base/debug/trace_event_argument.h"
@@ skipping 73 matching lines @@
       client_(client),
       layer_tree_host_id_(layer_tree_host_id),
       task_runner_(task_runner),
       vsync_interval_(BeginFrameArgs::DefaultInterval()),
       last_set_needs_begin_frame_(false),
       begin_unthrottled_frame_posted_(false),
       begin_retro_frame_posted_(false),
       state_machine_(scheduler_settings),
       inside_process_scheduled_actions_(false),
       inside_action_(SchedulerStateMachine::ACTION_NONE),
+      begin_frame_requested_(false),
       weak_factory_(this) {
   TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler"),
                "Scheduler::Scheduler",
                "settings",
                settings_.AsValue());
   DCHECK(client_);
   DCHECK(!state_machine_.BeginFrameNeeded());
   if (settings_.main_frame_before_activation_enabled) {
     DCHECK(settings_.main_frame_before_draw_enabled);
   }
@@ skipping 188 matching lines @@
 void Scheduler::SetupNextBeginFrameWhenVSyncThrottlingEnabled(
     bool needs_begin_frame) {
   bool at_end_of_deadline =
       state_machine_.begin_impl_frame_state() ==
           SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE;
 
   bool should_call_set_needs_begin_frame =
       // Always request the BeginFrame immediately if it wasn't needed before.
       (needs_begin_frame && !last_set_needs_begin_frame_) ||
       // Only stop requesting BeginFrames after a deadline.
-      (!needs_begin_frame && last_set_needs_begin_frame_ && at_end_of_deadline);
+      (!needs_begin_frame &&
+       last_set_needs_begin_frame_ &&
+       at_end_of_deadline &&
+       !begin_frame_requested_);

[brianderson, 2014/08/21 00:13:59]

Instead of taking begin_frame_requested_ into account here, would it make sense
to take it into account from inside SchedulerStateMachine::BeginFrameNeeded?

[simonhong, 2014/08/26 08:24:47]

Yeap, handling this state by SchdulerStateMachine would make sense!

 
   if (should_call_set_needs_begin_frame) {
     if (settings_.begin_frame_scheduling_enabled) {
       client_->SetNeedsBeginFrame(needs_begin_frame);
     } else {
       synthetic_begin_frame_source_->SetNeedsBeginFrame(
           needs_begin_frame, &begin_retro_frame_args_);
     }
     last_set_needs_begin_frame_ = needs_begin_frame;
   }
@@ skipping 88 matching lines @@
 
 // BeginFrame is the mechanism that tells us that now is a good time to start
 // making a frame. Usually this means that user input for the frame is complete.
 // If the scheduler is busy, we queue the BeginFrame to be handled later as
 // a BeginRetroFrame.
 void Scheduler::BeginFrame(const BeginFrameArgs& args) {
   TRACE_EVENT1("cc", "Scheduler::BeginFrame", "args", args.AsValue());
   DCHECK(settings_.throttle_frame_production);
 
   BeginFrameArgs adjusted_args(args);
+
+  if (settings_.begin_frame_publisher && begin_frame_requested_) {
+    // |arg.deadline| should be adjusted by subscribed Scheduler.
+    client_->SendBeginFrame(args);
+    begin_frame_requested_ = false;

[brianderson, 2014/08/21 00:13:59]

The child Schedulers should be setting begin_frame_requested explicitly to false
and the parent Scheduler should not set it to false implicitly like this. That
way we reduce the number of messages between the Renderer and Browser.

[simonhong, 2014/08/26 08:24:47]

Yeap, calling SetChildrenNeedBeginFrames() from BeginFrameManager can make much
simpler :)

+  }
+
   adjusted_args.deadline -= EstimatedParentDrawTime();
 
   bool should_defer_begin_frame;
   if (settings_.using_synchronous_renderer_compositor) {
     should_defer_begin_frame = false;
   } else {
     should_defer_begin_frame =
         !begin_retro_frame_args_.empty() || begin_retro_frame_posted_ ||
         !last_set_needs_begin_frame_ ||
         (state_machine_.begin_impl_frame_state() !=
@@ skipping 184 matching lines @@
   TRACE_EVENT0("cc", "Scheduler::PollToAdvanceCommitState");
   advance_commit_state_task_.Cancel();
   ProcessScheduledActions();
 }
 
 bool Scheduler::IsBeginMainFrameSent() const {
   return state_machine_.commit_state() ==
          SchedulerStateMachine::COMMIT_STATE_BEGIN_MAIN_FRAME_SENT;
 }
 
+void Scheduler::BeginFrameRequested() {
+  DCHECK(settings_.begin_frame_publisher);
+  DCHECK(!begin_frame_requested_);
+
+  begin_frame_requested_ = true;
+
+  // If frame source is already activated, next BeginFrame will be scheduled.
+  if (synthetic_begin_frame_source_->IsActive())
+    return;
+
+  synthetic_begin_frame_source_->SetNeedsBeginFrame(true,

[brianderson, 2014/08/21 00:13:59]

Instead of calling SetNeedsBeginFrame direcly here, please call
ProcessScheduledActions instead, which will go through the common way of setting
up the value for SetNeedsBeginFrame.

That will properly handle the vsync disabled case.

[simonhong, 2014/08/26 08:24:47]

Done.

+                                                    &begin_retro_frame_args_);
+}
+
 void Scheduler::DrawAndSwapIfPossible() {
   DrawResult result = client_->ScheduledActionDrawAndSwapIfPossible();
   state_machine_.DidDrawIfPossibleCompleted(result);
 }
 
 void Scheduler::ProcessScheduledActions() {
   // We do not allow ProcessScheduledActions to be recursive.
   // The top-level call will iteratively execute the next action for us anyway.
   if (inside_process_scheduled_actions_)
     return;
@@ skipping 130 matching lines @@
 }
 
 bool Scheduler::IsBeginMainFrameSentOrStarted() const {
   return (state_machine_.commit_state() ==
               SchedulerStateMachine::COMMIT_STATE_BEGIN_MAIN_FRAME_SENT ||
           state_machine_.commit_state() ==
               SchedulerStateMachine::COMMIT_STATE_BEGIN_MAIN_FRAME_STARTED);
 }
 
 }  // namespace cc