cc: Unify use of DidSwapBuffers() and did_request_swap

cc/scheduler/scheduler_state_machine.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_state_machine.h"
 
 #include "base/debug/trace_event.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
 #include "ui/gfx/frame_time.h"
 
 namespace cc {
 
 SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
     : settings_(settings),
       output_surface_state_(OUTPUT_SURFACE_LOST),
       begin_impl_frame_state_(BEGIN_IMPL_FRAME_STATE_IDLE),
       commit_state_(COMMIT_STATE_IDLE),
       forced_redraw_state_(FORCED_REDRAW_STATE_IDLE),
       readback_state_(READBACK_STATE_IDLE),
       commit_count_(0),
       current_frame_number_(0),
       last_frame_number_animate_performed_(-1),
       last_frame_number_swap_performed_(-1),
+      last_frame_number_swap_request_sent_(-1),
       last_frame_number_begin_main_frame_sent_(-1),
+      last_frame_number_draw_request_sent_(-1),
       last_frame_number_update_visible_tiles_was_called_(-1),
       manage_tiles_funnel_(0),
       consecutive_checkerboard_animations_(0),
       max_pending_swaps_(1),
       pending_swaps_(0),
       needs_redraw_(false),
       needs_animate_(false),
       needs_manage_tiles_(false),
       swap_used_incomplete_tile_(false),
       needs_commit_(false),
@@ skipping 179 matching lines @@
   state->Set("major_timestamps_in_ms", timestamps_state.release());
 
   scoped_ptr<base::DictionaryValue> minor_state(new base::DictionaryValue);
   minor_state->SetInteger("commit_count", commit_count_);
   minor_state->SetInteger("current_frame_number", current_frame_number_);
 
   minor_state->SetInteger("last_frame_number_animate_performed",
                           last_frame_number_animate_performed_);
   minor_state->SetInteger("last_frame_number_swap_performed",
                           last_frame_number_swap_performed_);
+  minor_state->SetInteger("last_frame_number_swap_request_sent",
+                          last_frame_number_swap_request_sent_);
   minor_state->SetInteger(
       "last_frame_number_begin_main_frame_sent",
       last_frame_number_begin_main_frame_sent_);
   minor_state->SetInteger(
+      "last_frame_number_draw_request_sent",
+      last_frame_number_draw_request_sent_);
+  minor_state->SetInteger(
       "last_frame_number_update_visible_tiles_was_called",
       last_frame_number_update_visible_tiles_was_called_);
 
   minor_state->SetInteger("manage_tiles_funnel", manage_tiles_funnel_);
   minor_state->SetInteger("consecutive_checkerboard_animations",
                           consecutive_checkerboard_animations_);
   minor_state->SetInteger("max_pending_swaps_", max_pending_swaps_);
   minor_state->SetInteger("pending_swaps_", pending_swaps_);
   minor_state->SetBoolean("needs_redraw", needs_redraw_);
   minor_state->SetBoolean("needs_animate_", needs_animate_);
@@ skipping 44 matching lines @@
 
 bool SchedulerStateMachine::HasUpdatedVisibleTilesThisFrame() const {
   return current_frame_number_ ==
          last_frame_number_update_visible_tiles_was_called_;
 }
 
 bool SchedulerStateMachine::HasSwappedThisFrame() const {
   return current_frame_number_ == last_frame_number_swap_performed_;
 }
 
+bool SchedulerStateMachine::HasSentSwapRequestThisFrame() const {

[brianderson, 2014/05/08 02:24:13]

HasRequestedSwapThisFrame?

[simonhong, 2014/05/08 03:13:31]

Done.

+  return current_frame_number_ == last_frame_number_swap_request_sent_;
+}
+
+bool SchedulerStateMachine::HasSentDrawRequestThisFrame() const {

[brianderson, 2014/05/08 02:24:13]

HasRequestedDrawThisFrame?

[simonhong, 2014/05/08 03:13:31]

Done.

+  return current_frame_number_ == last_frame_number_draw_request_sent_;
+}
+
 bool SchedulerStateMachine::PendingDrawsShouldBeAborted() const {
   // These are all the cases where we normally cannot or do not want to draw
   // but, if needs_redraw_ is true and we do not draw to make forward progress,
   // we might deadlock with the main thread.
   // This should be a superset of PendingActivationsShouldBeForced() since
   // activation of the pending tree is blocked by drawing of the active tree and
   // the main thread might be blocked on activation of the most recent commit.
   if (PendingActivationsShouldBeForced())
     return true;
 
@@ skipping 58 matching lines @@
     return true;
 
   // If we need to abort draws, we should do so ASAP since the draw could
   // be blocking other important actions (like output surface initialization),
   // from occuring. If we are waiting for the first draw, then perfom the
   // aborted draw to keep things moving. If we are not waiting for the first
   // draw however, we don't want to abort for no reason.
   if (PendingDrawsShouldBeAborted())
     return active_tree_needs_first_draw_;
 
-  // After this line, we only want to swap once per frame.
-  if (HasSwappedThisFrame())
+  // After this line, we only want to send draw & swap request once per frame.
+  if (HasSentDrawRequestThisFrame() && HasSentSwapRequestThisFrame())

[brianderson, 2014/05/08 02:24:13]

HasSentSwapRequestThisFrame should be good enough - is it not?

[simonhong, 2014/05/08 03:13:31]

Yep, you're right.
Using it alone is enough.

     return false;
 
   // Do not queue too many swaps.
   if (pending_swaps_ >= max_pending_swaps_)
     return false;
 
   // Except for the cases above, do not draw outside of the BeginImplFrame
   // deadline.
   if (begin_impl_frame_state_ != BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE)
     return false;
@@ skipping 236 matching lines @@
       UpdateStateOnCommit(commit_was_aborted);
       return;
     }
 
     case ACTION_DRAW_AND_SWAP_FORCED:
     case ACTION_DRAW_AND_SWAP_IF_POSSIBLE: {
       bool did_request_swap = true;
       UpdateStateOnDraw(did_request_swap);
       return;
     }
-

[brianderson, 2014/05/08 02:24:13]

Accident?

[simonhong, 2014/05/08 03:13:31]

Oops!
blank line restored.

     case ACTION_DRAW_AND_SWAP_ABORT:
     case ACTION_DRAW_AND_READBACK: {
       bool did_request_swap = false;
       UpdateStateOnDraw(did_request_swap);
       return;
     }
 
     case ACTION_BEGIN_OUTPUT_SURFACE_CREATION:
       DCHECK_EQ(output_surface_state_, OUTPUT_SURFACE_LOST);
       output_surface_state_ = OUTPUT_SURFACE_CREATING;
@@ skipping 140 matching lines @@
     readback_state_ = READBACK_STATE_WAITING_FOR_REPLACEMENT_COMMIT;
   } else if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_DRAW) {
     forced_redraw_state_ = FORCED_REDRAW_STATE_IDLE;
   }
 
   if (commit_state_ == COMMIT_STATE_WAITING_FOR_FIRST_DRAW)
     commit_state_ = COMMIT_STATE_IDLE;
 
   needs_redraw_ = false;
   active_tree_needs_first_draw_ = false;
+  last_frame_number_draw_request_sent_ = current_frame_number_;
 
   if (did_request_swap)
-    last_frame_number_swap_performed_ = current_frame_number_;
+    last_frame_number_swap_request_sent_ = current_frame_number_;
 }
 
 void SchedulerStateMachine::UpdateStateOnManageTiles() {
   needs_manage_tiles_ = false;
 }
 
 void SchedulerStateMachine::SetSkipNextBeginMainFrameToReduceLatency() {
   skip_next_begin_main_frame_to_reduce_latency_ = true;
 }
 
@@ skipping 87 matching lines @@
   // If the pending tree activates quickly, we'll want a BeginImplFrame soon
   // to draw the new active tree.
   if (has_pending_tree_)
     return true;
 
   // Changing priorities may allow us to activate (given the new priorities),
   // which may result in a new frame.
   if (needs_manage_tiles_)
     return true;
 
-  // If we just swapped, it's likely that we are going to produce another
-  // frame soon. This helps avoid negative glitches in our
+  // If we just sent draw request, it's likely that we are going to produce
+  // another frame soon. This helps avoid negative glitches in our
   // SetNeedsBeginFrame requests, which may propagate to the BeginImplFrame
   // provider and get sampled at an inopportune time, delaying the next
   // BeginImplFrame.
-  if (last_frame_number_swap_performed_ == current_frame_number_)
+  if (HasSentDrawRequestThisFrame())
     return true;
 
   return false;
 }
 
 void SchedulerStateMachine::OnBeginImplFrame(const BeginFrameArgs& args) {
   AdvanceCurrentFrameNumber();
   begin_impl_frame_args_ = args;
   DCHECK_EQ(begin_impl_frame_state_, BEGIN_IMPL_FRAME_STATE_IDLE) << *AsValue();
   begin_impl_frame_state_ = BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING;
@@ skipping 60 matching lines @@
 }
 
 bool SchedulerStateMachine::MainThreadIsInHighLatencyMode() const {
   // If a commit is pending before the previous commit has been drawn, we
   // are definitely in a high latency mode.
   if (CommitPending() && (active_tree_needs_first_draw_ || has_pending_tree_))
     return true;
 
   // If we just sent a BeginMainFrame and haven't hit the deadline yet, the main
   // thread is in a low latency mode.
-  if (last_frame_number_begin_main_frame_sent_ == current_frame_number_ &&
+  if (HasSentBeginMainFrameThisFrame() &&
       (begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING ||
        begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_INSIDE_BEGIN_FRAME))
     return false;
 
   // If there's a commit in progress it must either be from the previous frame
   // or it started after the impl thread's deadline. In either case the main
   // thread is in high latency mode.
-  if (commit_state_ == COMMIT_STATE_BEGIN_MAIN_FRAME_SENT ||
-      commit_state_ == COMMIT_STATE_BEGIN_MAIN_FRAME_STARTED ||
-      commit_state_ == COMMIT_STATE_READY_TO_COMMIT)
+  if (CommitPending())
     return true;
 
   // Similarly, if there's a pending tree the main thread is in high latency
   // mode, because either
   //   it's from the previous frame
   // or
   //   we're currently drawing the active tree and the pending tree will thus
   //   only be drawn in the next frame.
   if (has_pending_tree_)
     return true;
 
   if (begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE) {
-    // Even if there's a new active tree to draw at the deadline or we've just
-    // drawn it, it may have been triggered by a previous BeginImplFrame, in
-    // which case the main thread is in a high latency mode.
-    return (active_tree_needs_first_draw_ ||
-            last_frame_number_swap_performed_ == current_frame_number_) &&
-           last_frame_number_begin_main_frame_sent_ != current_frame_number_;
+    // Even if we've just drawn it, it may have been triggered by a previous
+    // BeginImplFrame, in which case the main thread is in a high latency mode.
+    return HasSwappedThisFrame() && !HasSentBeginMainFrameThisFrame();

[brianderson, 2014/05/08 02:24:13]

Why was the active_tree_needs_first_draw_ condition removed?

[simonhong, 2014/05/08 03:13:31]

My mistake. restored!

   }
 
   // If the active tree needs its first draw in any other state, we know the
   // main thread is in a high latency mode.
   return active_tree_needs_first_draw_;
 }
 
 void SchedulerStateMachine::DidEnterPollForAnticipatedDrawTriggers() {
   AdvanceCurrentFrameNumber();
   inside_poll_for_anticipated_draw_triggers_ = true;
@@ skipping 21 matching lines @@
   }
 }
 
 void SchedulerStateMachine::SetMaxSwapsPending(int max) {
   max_pending_swaps_ = max;
 }
 
 void SchedulerStateMachine::DidSwapBuffers() {
   pending_swaps_++;
   DCHECK_LE(pending_swaps_, max_pending_swaps_);
+
+  last_frame_number_swap_performed_ = current_frame_number_;
 }
 
 void SchedulerStateMachine::SetSwapUsedIncompleteTile(
     bool used_incomplete_tile) {
   swap_used_incomplete_tile_ = used_incomplete_tile;
 }
 
 void SchedulerStateMachine::DidSwapBuffersComplete() {
   DCHECK_GT(pending_swaps_, 0);
   pending_swaps_--;
 }
 
 void SchedulerStateMachine::SetSmoothnessTakesPriority(
     bool smoothness_takes_priority) {
   smoothness_takes_priority_ = smoothness_takes_priority;
 }
 
-void SchedulerStateMachine::DidDrawIfPossibleCompleted(
-    DrawSwapReadbackResult::DrawResult result) {
+void SchedulerStateMachine::DidDrawIfPossibleCompleted(DrawResult result) {
   switch (result) {
-    case DrawSwapReadbackResult::INVALID_RESULT:
-      NOTREACHED() << "Uninitialized DrawSwapReadbackResult.";
+    case INVALID_RESULT:
+      NOTREACHED() << "Uninitialized DrawResult.";
       break;
-    case DrawSwapReadbackResult::DRAW_ABORTED_CANT_DRAW:
-    case DrawSwapReadbackResult::DRAW_ABORTED_CANT_READBACK:
-    case DrawSwapReadbackResult::DRAW_ABORTED_CONTEXT_LOST:
+    case DRAW_ABORTED_CANT_DRAW:
+    case DRAW_ABORTED_CANT_READBACK:
+    case DRAW_ABORTED_CONTEXT_LOST:
       NOTREACHED() << "Invalid return value from DrawAndSwapIfPossible:"
                    << result;
       break;
-    case DrawSwapReadbackResult::DRAW_SUCCESS:
+    case DRAW_SUCCESS:
       consecutive_checkerboard_animations_ = 0;
       forced_redraw_state_ = FORCED_REDRAW_STATE_IDLE;
       break;
-    case DrawSwapReadbackResult::DRAW_ABORTED_CHECKERBOARD_ANIMATIONS:
+    case DRAW_ABORTED_CHECKERBOARD_ANIMATIONS:
       needs_redraw_ = true;
 
       // If we're already in the middle of a redraw, we don't need to
       // restart it.
       if (forced_redraw_state_ != FORCED_REDRAW_STATE_IDLE)
         return;
 
       needs_commit_ = true;
       consecutive_checkerboard_animations_++;
       if (settings_.timeout_and_draw_when_animation_checkerboards &&
           consecutive_checkerboard_animations_ >=
               settings_.maximum_number_of_failed_draws_before_draw_is_forced_) {
         consecutive_checkerboard_animations_ = 0;
         // We need to force a draw, but it doesn't make sense to do this until
         // we've committed and have new textures.
         forced_redraw_state_ = FORCED_REDRAW_STATE_WAITING_FOR_COMMIT;
       }
       break;
-    case DrawSwapReadbackResult::DRAW_ABORTED_MISSING_HIGH_RES_CONTENT:
+    case DRAW_ABORTED_MISSING_HIGH_RES_CONTENT:
       // It's not clear whether this missing content is because of missing
       // pictures (which requires a commit) or because of memory pressure
       // removing textures (which might not).  To be safe, request a commit
       // anyway.
       needs_commit_ = true;
       break;
   }
 }
 
 void SchedulerStateMachine::SetNeedsCommit() { needs_commit_ = true; }
@@ skipping 99 matching lines @@
     case OUTPUT_SURFACE_ACTIVE:
     case OUTPUT_SURFACE_WAITING_FOR_FIRST_COMMIT:
     case OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION:
       return true;
   }
   NOTREACHED();
   return false;
 }
 
 }  // namespace cc