Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(997)

Unified Diff: cc/scheduler/scheduler_state_machine.cc

Issue 22926024: cc: Control activation from the Scheduler (Closed) Base URL: http://git.chromium.org/chromium/src.git@schedOutputSurface4
Patch Set: remove hacks, address comments, force activation to prevent deadlock Created 7 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: cc/scheduler/scheduler_state_machine.cc
diff --git a/cc/scheduler/scheduler_state_machine.cc b/cc/scheduler/scheduler_state_machine.cc
index 1d77e09b3008c45af4978fa5199085aa8ee3a9e1..24a59d0122a99591c1cc2aae1363285ca5e696c3 100644
--- a/cc/scheduler/scheduler_state_machine.cc
+++ b/cc/scheduler/scheduler_state_machine.cc
@@ -19,7 +19,6 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
current_frame_number_(0),
last_frame_number_where_begin_frame_sent_to_main_thread_(-1),
last_frame_number_where_draw_was_called_(-1),
- last_frame_number_where_tree_activation_attempted_(-1),
last_frame_number_where_update_visible_tiles_was_called_(-1),
consecutive_failed_draws_(0),
maximum_number_of_failed_draws_before_draw_is_forced_(3),
@@ -36,6 +35,8 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
can_start_(false),
can_draw_(false),
has_pending_tree_(false),
+ pending_tree_is_ready_for_activation_(false),
+ active_tree_has_been_drawn_(false),
draw_if_possible_failed_(false),
texture_state_(LAYER_TEXTURE_STATE_UNLOCKED),
did_create_and_initialize_first_output_surface_(false) {}
@@ -98,8 +99,8 @@ const char* SchedulerStateMachine::ActionToString(Action action) {
return "ACTION_COMMIT";
case ACTION_UPDATE_VISIBLE_TILES:
return "ACTION_UPDATE_VISIBLE_TILES";
- case ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED:
- return "ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED";
+ case ACTION_ACTIVATE_PENDING_TREE:
+ return "ACTION_ACTIVATE_PENDING_TREE";
case ACTION_DRAW_IF_POSSIBLE:
return "ACTION_DRAW_IF_POSSIBLE";
case ACTION_DRAW_FORCED:
@@ -162,8 +163,6 @@ scoped_ptr<base::Value> SchedulerStateMachine::AsValue() const {
last_frame_number_where_begin_frame_sent_to_main_thread_);
minor_state->SetInteger("last_frame_number_where_draw_was_called",
last_frame_number_where_draw_was_called_);
- minor_state->SetInteger("last_frame_number_where_tree_activation_attempted",
- last_frame_number_where_tree_activation_attempted_);
minor_state->SetInteger(
"last_frame_number_where_update_visible_tiles_was_called",
last_frame_number_where_update_visible_tiles_was_called_);
@@ -189,6 +188,10 @@ scoped_ptr<base::Value> SchedulerStateMachine::AsValue() const {
minor_state->SetBoolean("can_start", can_start_);
minor_state->SetBoolean("can_draw", can_draw_);
minor_state->SetBoolean("has_pending_tree", has_pending_tree_);
+ minor_state->SetBoolean("pending_tree_is_ready_for_activation_",
+ pending_tree_is_ready_for_activation_);
+ minor_state->SetBoolean("active_tree_has_been_drawn_",
+ active_tree_has_been_drawn_);
minor_state->SetBoolean("draw_if_possible_failed", draw_if_possible_failed_);
minor_state->SetBoolean("did_create_and_initialize_first_output_surface",
did_create_and_initialize_first_output_surface_);
@@ -201,11 +204,6 @@ bool SchedulerStateMachine::HasDrawnThisFrame() const {
return current_frame_number_ == last_frame_number_where_draw_was_called_;
}
-bool SchedulerStateMachine::HasAttemptedTreeActivationThisFrame() const {
- return current_frame_number_ ==
- last_frame_number_where_tree_activation_attempted_;
-}
-
bool SchedulerStateMachine::HasUpdatedVisibleTilesThisFrame() const {
return current_frame_number_ ==
last_frame_number_where_update_visible_tiles_was_called_;
@@ -244,12 +242,11 @@ void SchedulerStateMachine::HandleCommitInternal(bool commit_was_aborted) {
}
}
- // if we don't have to wait for activation, update needs_redraw now.
- if (!commit_results_in_pending_tree) {
- if (!commit_was_aborted)
- needs_redraw_ = true;
- if (expect_immediate_begin_frame_for_main_thread_)
+ // Update state if we have a new active tree to draw.
+ if (!commit_results_in_pending_tree &&
+ (!commit_was_aborted || expect_immediate_begin_frame_for_main_thread_)) {
needs_redraw_ = true;
+ active_tree_has_been_drawn_ = false;
}
// This post-commit work is common to both completed and aborted commits.
@@ -272,14 +269,32 @@ void SchedulerStateMachine::HandleCommitInternal(bool commit_was_aborted) {
bool SchedulerStateMachine::PendingDrawsShouldBeAborted() const {
// These are all the cases where, if we do not abort draws to make
// forward progress, we might deadlock with the main thread.
+ // This should be a superset of PendingActivationsShouldBeForced().
+ if (PendingActivationsShouldBeForced())
+ return true;
+
+ // Additional states where we should abort draws.
+ // Note: We don't force activation in these cases because doing so would
+ // result in checkerboarding on resize, becoming visible, etc.
if (!can_draw_)
return true;
if (!visible_)
return true;
- if (output_surface_state_ != OUTPUT_SURFACE_ACTIVE)
+ if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION)
return true;
+ return false;
+}
+
+bool SchedulerStateMachine::PendingActivationsShouldBeForced() const {
+ // These are all the cases where, if we do not force activations to make
+ // forward progress, we might deadlock with the main thread.
+ // This should be a subset PendingDrawsShouldBeAborted().
if (texture_state_ == LAYER_TEXTURE_STATE_ACQUIRED_BY_MAIN_THREAD)
return true;
+ if (output_surface_state_ == OUTPUT_SURFACE_LOST ||
+ output_surface_state_ == OUTPUT_SURFACE_CREATING ||
+ output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_COMMIT)
+ return true;
return false;
}
@@ -289,13 +304,8 @@ bool SchedulerStateMachine::ShouldDraw() const {
return true;
// If we are going to abort draws, we should do so ASAP.
- if (PendingDrawsShouldBeAborted()) {
- // TODO(brianderson): Remove the !has_pending_tree_ condition once
- // the Scheduler controls activation. It's dangerous for us to rely on
- // an eventual activation if we've lost the output surface.
- return commit_state_ == COMMIT_STATE_WAITING_FOR_FIRST_DRAW &&
- !has_pending_tree_;
- }
+ if (PendingDrawsShouldBeAborted())
+ return commit_state_ == COMMIT_STATE_WAITING_FOR_FIRST_DRAW;
// After this line, we only want to draw once per frame.
if (HasDrawnThisFrame())
@@ -308,9 +318,25 @@ bool SchedulerStateMachine::ShouldDraw() const {
return needs_redraw_;
}
-bool SchedulerStateMachine::ShouldAttemptTreeActivation() const {
- return has_pending_tree_ && inside_begin_frame_ &&
- !HasAttemptedTreeActivationThisFrame();
+bool SchedulerStateMachine::ShouldActivatePendingTree() const {
+ // There is nothing to activate.
+ if (!has_pending_tree_)
+ return false;
+
+ // We don't want to activate a second tree before drawing the first one.
+ // Note: It is possible that there is no active tree to draw when
+ // output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION,
+ // so we don't block activation on draw in that case.
+ if (!active_tree_has_been_drawn_ &&
+ output_surface_state_ != OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION)
+ return false;
+
+ // If we want to force activation, do so ASAP.
+ if (PendingActivationsShouldBeForced())
+ return true;
+
+ // At this point, only activate if we are ready to activate.
+ return pending_tree_is_ready_for_activation_;
}
bool SchedulerStateMachine::ShouldUpdateVisibleTiles() const {
@@ -319,7 +345,7 @@ bool SchedulerStateMachine::ShouldUpdateVisibleTiles() const {
if (HasUpdatedVisibleTilesThisFrame())
return false;
- return ShouldAttemptTreeActivation() || ShouldDraw() ||
+ return ShouldActivatePendingTree() || ShouldDraw() ||
swap_used_incomplete_tile_;
}
@@ -388,8 +414,8 @@ SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
return ACTION_NONE;
if (ShouldUpdateVisibleTiles())
return ACTION_UPDATE_VISIBLE_TILES;
- if (ShouldAttemptTreeActivation())
- return ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED;
+ if (ShouldActivatePendingTree())
+ return ACTION_ACTIVATE_PENDING_TREE;
if (ShouldDraw()) {
return needs_forced_redraw_ ? ACTION_DRAW_FORCED
: ACTION_DRAW_IF_POSSIBLE;
@@ -401,8 +427,8 @@ SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
case COMMIT_STATE_FRAME_IN_PROGRESS:
if (ShouldUpdateVisibleTiles())
return ACTION_UPDATE_VISIBLE_TILES;
- if (ShouldAttemptTreeActivation())
- return ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED;
+ if (ShouldActivatePendingTree())
+ return ACTION_ACTIVATE_PENDING_TREE;
if (ShouldDraw()) {
return needs_forced_redraw_ ? ACTION_DRAW_FORCED
: ACTION_DRAW_IF_POSSIBLE;
@@ -415,8 +441,8 @@ SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
case COMMIT_STATE_WAITING_FOR_FIRST_DRAW: {
if (ShouldUpdateVisibleTiles())
return ACTION_UPDATE_VISIBLE_TILES;
- if (ShouldAttemptTreeActivation())
- return ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED;
+ if (ShouldActivatePendingTree())
+ return ACTION_ACTIVATE_PENDING_TREE;
if (ShouldDraw()) {
if (needs_forced_redraw_)
return ACTION_DRAW_FORCED;
@@ -431,8 +457,8 @@ SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
case COMMIT_STATE_WAITING_FOR_FIRST_FORCED_DRAW:
if (ShouldUpdateVisibleTiles())
return ACTION_UPDATE_VISIBLE_TILES;
- if (ShouldAttemptTreeActivation())
- return ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED;
+ if (ShouldActivatePendingTree())
+ return ACTION_ACTIVATE_PENDING_TREE;
if (needs_forced_redraw_)
return ACTION_DRAW_FORCED;
return ACTION_NONE;
@@ -451,9 +477,7 @@ void SchedulerStateMachine::UpdateState(Action action) {
current_frame_number_;
return;
- case ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED:
- last_frame_number_where_tree_activation_attempted_ =
- current_frame_number_;
+ case ACTION_ACTIVATE_PENDING_TREE:
return;
case ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD:
@@ -519,6 +543,7 @@ void SchedulerStateMachine::UpdateStateOnDraw(bool did_swap) {
needs_redraw_ = false;
needs_forced_redraw_ = false;
draw_if_possible_failed_ = false;
+ active_tree_has_been_drawn_ = true;
if (did_swap)
swap_used_incomplete_tile_ = false;
@@ -531,10 +556,6 @@ void SchedulerStateMachine::SetMainThreadNeedsLayerTextures() {
}
bool SchedulerStateMachine::BeginFrameNeededToDrawByImplThread() const {
- // TODO(brianderson): Remove this hack once the Scheduler controls activation.
- if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION)
- return true;
-
// If we can't draw, don't tick until we are notified that we can draw again.
if (!can_draw_)
return false;
@@ -550,7 +571,7 @@ bool SchedulerStateMachine::BeginFrameNeededToDrawByImplThread() const {
bool SchedulerStateMachine::ProactiveBeginFrameWantedByImplThread() const {
// Do not be proactive when invisible.
- if (!visible_ || output_surface_state_ != OUTPUT_SURFACE_ACTIVE)
+ if (!visible_ || !HasInitializedOutputSurface())
return false;
// We should proactively request a BeginFrame if a commit or a tree activation
@@ -636,13 +657,23 @@ void SchedulerStateMachine::DidLoseOutputSurface() {
needs_redraw_ = false;
}
+void SchedulerStateMachine::NotifyReadyToActivate() {
+ if (has_pending_tree_)
+ pending_tree_is_ready_for_activation_ = true;
+}
+
void SchedulerStateMachine::SetHasPendingTree(bool has_pending_tree) {
if (has_pending_tree_ && !has_pending_tree) {
// There is a new active tree.
if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION)
output_surface_state_ = OUTPUT_SURFACE_ACTIVE;
+ pending_tree_is_ready_for_activation_ = false;
+ active_tree_has_been_drawn_ = false;
needs_redraw_ = true;
+ } else if (!has_pending_tree_ && has_pending_tree) {
+ // There is a new pending tree.
+ pending_tree_is_ready_for_activation_ = false;
}
has_pending_tree_ = has_pending_tree;
}

Powered by Google App Engine
This is Rietveld 408576698