CC: Add a scheduled action for ManageTiles

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/format_macros.h"
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
@@ skipping 128 matching lines @@
     case ACTION_DRAW_AND_SWAP_FORCED:
       return "ACTION_DRAW_AND_SWAP_FORCED";
     case ACTION_DRAW_AND_SWAP_ABORT:
       return "ACTION_DRAW_AND_SWAP_ABORT";
     case ACTION_DRAW_AND_READBACK:
       return "ACTION_DRAW_AND_READBACK";
     case ACTION_BEGIN_OUTPUT_SURFACE_CREATION:
       return "ACTION_BEGIN_OUTPUT_SURFACE_CREATION";
     case ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD:
       return "ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD";
+    case ACTION_MANAGE_TILES:
+      return "ACTION_MANAGE_TILES";
   }
   NOTREACHED();
   return "???";
 }
 
 scoped_ptr<base::Value> SchedulerStateMachine::AsValue() const  {
   scoped_ptr<base::DictionaryValue> state(new base::DictionaryValue);
 
   scoped_ptr<base::DictionaryValue> major_state(new base::DictionaryValue);
   major_state->SetString("next_action", ActionToString(NextAction()));
@@ skipping 43 matching lines @@
       "last_frame_number_where_begin_frame_sent_to_main_thread",
       last_frame_number_where_begin_frame_sent_to_main_thread_);
   minor_state->SetInteger("last_frame_number_swap_performed_",
                           last_frame_number_swap_performed_);
   minor_state->SetInteger(
       "last_frame_number_where_update_visible_tiles_was_called",
       last_frame_number_where_update_visible_tiles_was_called_);
   minor_state->SetInteger("consecutive_failed_draws",
                           consecutive_failed_draws_);
   minor_state->SetBoolean("needs_redraw", needs_redraw_);
+  minor_state->SetBoolean("needs_manage_tiles", needs_manage_tiles_);
   minor_state->SetBoolean("swap_used_incomplete_tile",
                           swap_used_incomplete_tile_);
   minor_state->SetBoolean("needs_commit", needs_commit_);
   minor_state->SetBoolean("main_thread_needs_layer_textures",
                           main_thread_needs_layer_textures_);
   minor_state->SetBoolean("inside_begin_frame", inside_begin_frame_);
   minor_state->SetBoolean("visible", visible_);
   minor_state->SetBoolean("can_start", can_start_);
   minor_state->SetBoolean("can_draw", can_draw_);
   minor_state->SetBoolean("has_pending_tree", has_pending_tree_);
@@ skipping 213 matching lines @@
   if (!HasInitializedOutputSurface())
     return false;
 
   return true;
 }
 
 bool SchedulerStateMachine::ShouldCommit() const {
   return commit_state_ == COMMIT_STATE_READY_TO_COMMIT;
 }
 
+bool SchedulerStateMachine::ShouldManageTiles() const {

[enne (OOO), 2013/09/11 23:45:11]

Yeah, I agree strongly with brianderson's suggestion about throttling this to
happen once per frame.  Without that, this seems like this patch could
accidentally regress current behavior.

It'd also be nice if the ManageTiles from commit could be wrapped into the
scheduler too so that the manual ManageTiles there wouldn't happen in the same
frame as a scheduler-initiated one.  I think you could do this if you added a
needs_post_commit_manage_tiles_ variable.  You could update its value to
needs_manage_tiles_ in OnUpdateState, and then check it here before checking
inside_begin_frame.  What do you think?

[epennerAtGoogle, 2013/09/12 00:09:59]

Just for clarity: The extra throttling is only required for the synchronous
compositor right? IIUC throttling by inside_begin_frame_ is enough in other
cases?

Regarding the commit ManageTiles: I initially thought we could merge the
synchronous commit ManageTiles... However, I think there is a problem: The
commit changes tile priorities, so the last ManageTiles isn't sufficient.
Therefore, waiting for the next ManageTiles effectively delays commits due to
delaying painting the new tiles etc.

So unless my understanding above is off, we can't skip the commit ManageTiles
unless we are in an accelerate gesture (don't care about commit latency)...
Alternatively maybe we could skip the next post-draw ManageTiles, but I think
that has the same problem (CC may have changed scroll offsets etc. and we would
miss those for one frame).

[brianderson, 2013/09/12 00:24:08]

Yes, the extra throttling is only required for the synchronous compositor, but
should also work without the synchronous compositor and make the check for
inside_begin_frame unnecessary.

I agree that we should not skip the ManageTiles after the commit as it would
delay activation. It would be nice if the ManageTiles after the commit cleared
the Scheduler's needs_manage_tiles and also blocked future ManageTiles from the
Scheduler until the next frame though.

+  if (!inside_begin_frame_)
+    return false;
+  return needs_manage_tiles_;
+}
+
 SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
   if (ShouldAcquireLayerTexturesForMainThread())
     return ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD;
   if (ShouldUpdateVisibleTiles())
     return ACTION_UPDATE_VISIBLE_TILES;
   if (ShouldActivatePendingTree())
     return ACTION_ACTIVATE_PENDING_TREE;
   if (ShouldCommit())
     return ACTION_COMMIT;
   if (ShouldDraw()) {
     if (readback_state_ == READBACK_STATE_WAITING_FOR_DRAW_AND_READBACK)
       return ACTION_DRAW_AND_READBACK;
     else if (PendingDrawsShouldBeAborted())
       return ACTION_DRAW_AND_SWAP_ABORT;
     else if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_DRAW)
       return ACTION_DRAW_AND_SWAP_FORCED;
     else
       return ACTION_DRAW_AND_SWAP_IF_POSSIBLE;
   }
+  if (ShouldManageTiles())
+    return ACTION_MANAGE_TILES;
   if (ShouldSendBeginFrameToMainThread())
     return ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD;
   if (ShouldBeginOutputSurfaceCreation())
     return ACTION_BEGIN_OUTPUT_SURFACE_CREATION;
   return ACTION_NONE;
 }
 
 void SchedulerStateMachine::CheckInvariants() {
   // We should never try to perform a draw for readback and forced draw due to
   // timeout simultaneously.
@@ skipping 55 matching lines @@
       // surface creation to avoid complicated corner cases.
       DCHECK_EQ(commit_state_, COMMIT_STATE_IDLE);
       DCHECK(!has_pending_tree_);
       DCHECK(!active_tree_needs_first_draw_);
       return;
 
     case ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD:
       texture_state_ = LAYER_TEXTURE_STATE_ACQUIRED_BY_MAIN_THREAD;
       main_thread_needs_layer_textures_ = false;
       return;
+
+    case ACTION_MANAGE_TILES:
+      UpdateStateOnManageTiles();
+      return;
   }
 }
 
 void SchedulerStateMachine::UpdateStateOnCommit(bool commit_was_aborted) {
   commit_count_++;
 
   // If we are impl-side-painting but the commit was aborted, then we behave
   // mostly as if we are not impl-side-painting since there is no pending tree.
   has_pending_tree_ = settings_.impl_side_painting && !commit_was_aborted;
 
@@ skipping 121 matching lines @@
     texture_state_ = LAYER_TEXTURE_STATE_UNLOCKED;
 
   needs_redraw_ = false;
   draw_if_possible_failed_ = false;
   active_tree_needs_first_draw_ = false;
 
   if (did_swap)
     last_frame_number_swap_performed_ = current_frame_number_;
 }
 
+void SchedulerStateMachine::UpdateStateOnManageTiles() {
+  needs_manage_tiles_ = false;
+}
+
 void SchedulerStateMachine::SetMainThreadNeedsLayerTextures() {
   DCHECK(!main_thread_needs_layer_textures_);
   DCHECK_NE(texture_state_, LAYER_TEXTURE_STATE_ACQUIRED_BY_MAIN_THREAD);
   main_thread_needs_layer_textures_ = true;
 }
 
 // These are the cases where we definitely (or almost definitely) have a
 // new frame to draw and can draw.
 bool SchedulerStateMachine::BeginFrameNeededToDrawByImplThread() const {
   // The output surface is the provider of BeginFrames for the impl thread,
@@ skipping 43 matching lines @@
   // We should proactively request a BeginFrame if a commit is pending
   // because we will want to draw if the commit completes quickly.
   if (needs_commit_ || commit_state_ != COMMIT_STATE_IDLE)
     return true;
 
   // If the pending tree activates quickly, we'll want a BeginFrame 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;

[epenner, 2013/09/11 22:55:47]

Brianderson@, is there anything I need to worry about here regarding the
synchronous compositor. The comment references this being an issue, but I don't
know understand how that case should be handled (or whether it's handled in
general somewhere else).

[brianderson, 2013/09/11 23:14:37]

I'm working on a patch to fix issues like this, when we need to poll for
actions/state that might produce a new frame with the synchronous compositor:
https://codereview.chromium.org/23463014/

For that to work with this patch, you will need to throttle ShouldManageTiles by
the current_frame_number_.

+
   return false;
 }
 
 void SchedulerStateMachine::DidEnterBeginFrame(const BeginFrameArgs& args) {
   current_frame_number_++;
   inside_begin_frame_ = true;
   last_begin_frame_args_ = args;
 }
 
 void SchedulerStateMachine::DidLeaveBeginFrame() {
   inside_begin_frame_ = false;
 }
 
 void SchedulerStateMachine::SetVisible(bool visible) { visible_ = visible; }
 
 void SchedulerStateMachine::SetNeedsRedraw() { needs_redraw_ = true; }
 
+void SchedulerStateMachine::SetNeedsManageTiles() {
+  needs_manage_tiles_ = true;
+}
+
 void SchedulerStateMachine::SetSwapUsedIncompleteTile(
     bool used_incomplete_tile) {
   swap_used_incomplete_tile_ = used_incomplete_tile;
 }
 
 void SchedulerStateMachine::DidDrawIfPossibleCompleted(bool success) {
   draw_if_possible_failed_ = !success;
   if (draw_if_possible_failed_) {
     needs_redraw_ = true;
     needs_commit_ = true;
@@ skipping 87 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