Commit 0e90c5f5 authored by Adrienne Walker's avatar Adrienne Walker Committed by Commit Bot

cc: Separate out task vs gpu TileManager signals

Signals in the TileManager are somewhat muddled.  Prior to this patch:

* PrepareTiles resets the signals.
* When a task set completes (draw/activate/all), it sets the signal
  and then schedules CheckAndIssueSignals
* CheckAndIssueSignals unconditionally calls CheckPendingGpuWorKTiles
* CPGWT clobbers the value of ready to draw and ready to activate
  based on whether or not there is pending gpu work.
* If not in smoothness mode, then there are no pending gpu work tiles
  and so finishing activate tiles calling CheckAndIssueSignals will
  also set ready to draw, but the THIRD IsReadyToDraw check will fail
  and so it never does anything.
* Sending a signal to the client also unsets ready to draw / activate / etc.

This patch separates out tile task completion from gpu work completion,
which means that no values are ever unset except for PrepareTiles and
we only do ready to draw/activate checks when both the tile work and
the gpu work is done.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I77db37729761d99daa7ce747a4b3809a6d033363
Reviewed-on: https://chromium-review.googlesource.com/993157Reviewed-by: default avatarKhushal <khushalsagar@chromium.org>
Commit-Queue: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547840}
parent 6b2841f3
...@@ -442,7 +442,7 @@ void TileManager::DidFinishRunningTileTasksRequiredForActivation() { ...@@ -442,7 +442,7 @@ void TileManager::DidFinishRunningTileTasksRequiredForActivation() {
ScheduledTasksStateAsValue()); ScheduledTasksStateAsValue());
// TODO(vmpstr): Temporary check to debug crbug.com/642927. // TODO(vmpstr): Temporary check to debug crbug.com/642927.
CHECK(tile_task_manager_); CHECK(tile_task_manager_);
signals_.ready_to_activate = true; signals_.activate_tile_tasks_completed = true;
signals_check_notifier_.Schedule(); signals_check_notifier_.Schedule();
} }
...@@ -452,7 +452,7 @@ void TileManager::DidFinishRunningTileTasksRequiredForDraw() { ...@@ -452,7 +452,7 @@ void TileManager::DidFinishRunningTileTasksRequiredForDraw() {
ScheduledTasksStateAsValue()); ScheduledTasksStateAsValue());
// TODO(vmpstr): Temporary check to debug crbug.com/642927. // TODO(vmpstr): Temporary check to debug crbug.com/642927.
CHECK(tile_task_manager_); CHECK(tile_task_manager_);
signals_.ready_to_draw = true; signals_.draw_tile_tasks_completed = true;
signals_check_notifier_.Schedule(); signals_check_notifier_.Schedule();
} }
...@@ -492,7 +492,7 @@ bool TileManager::PrepareTiles( ...@@ -492,7 +492,7 @@ bool TileManager::PrepareTiles(
return false; return false;
} }
signals_.reset(); signals_ = Signals();
global_state_ = state; global_state_ = state;
// Ensure that we don't schedule any decode work for checkered images until // Ensure that we don't schedule any decode work for checkered images until
...@@ -1212,7 +1212,7 @@ scoped_refptr<TileTask> TileManager::CreateRasterTask( ...@@ -1212,7 +1212,7 @@ scoped_refptr<TileTask> TileManager::CreateRasterTask(
} }
void TileManager::ResetSignalsForTesting() { void TileManager::ResetSignalsForTesting() {
signals_.reset(); signals_ = Signals();
} }
void TileManager::OnRasterTaskCompleted( void TileManager::OnRasterTaskCompleted(
...@@ -1361,8 +1361,9 @@ void TileManager::CheckAndIssueSignals() { ...@@ -1361,8 +1361,9 @@ void TileManager::CheckAndIssueSignals() {
CheckPendingGpuWorkTiles(false /* issue_signals */); CheckPendingGpuWorkTiles(false /* issue_signals */);
// Ready to activate. // Ready to activate.
if (signals_.ready_to_activate && !signals_.did_notify_ready_to_activate) { if (signals_.activate_tile_tasks_completed &&
signals_.ready_to_activate = false; signals_.activate_gpu_work_completed &&
!signals_.did_notify_ready_to_activate) {
if (IsReadyToActivate()) { if (IsReadyToActivate()) {
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"), TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
"TileManager::CheckAndIssueSignals - ready to activate"); "TileManager::CheckAndIssueSignals - ready to activate");
...@@ -1372,8 +1373,8 @@ void TileManager::CheckAndIssueSignals() { ...@@ -1372,8 +1373,8 @@ void TileManager::CheckAndIssueSignals() {
} }
// Ready to draw. // Ready to draw.
if (signals_.ready_to_draw && !signals_.did_notify_ready_to_draw) { if (signals_.draw_tile_tasks_completed && signals_.draw_gpu_work_completed &&
signals_.ready_to_draw = false; !signals_.did_notify_ready_to_draw) {
if (IsReadyToDraw()) { if (IsReadyToDraw()) {
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"), TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
"TileManager::CheckAndIssueSignals - ready to draw"); "TileManager::CheckAndIssueSignals - ready to draw");
...@@ -1385,7 +1386,6 @@ void TileManager::CheckAndIssueSignals() { ...@@ -1385,7 +1386,6 @@ void TileManager::CheckAndIssueSignals() {
// All tile tasks completed. // All tile tasks completed.
if (signals_.all_tile_tasks_completed && if (signals_.all_tile_tasks_completed &&
!signals_.did_notify_all_tile_tasks_completed) { !signals_.did_notify_all_tile_tasks_completed) {
signals_.all_tile_tasks_completed = false;
if (!has_scheduled_tile_tasks_) { if (!has_scheduled_tile_tasks_) {
TRACE_EVENT0( TRACE_EVENT0(
TRACE_DISABLED_BY_DEFAULT("cc.debug"), TRACE_DISABLED_BY_DEFAULT("cc.debug"),
...@@ -1445,8 +1445,8 @@ void TileManager::CheckIfMoreTilesNeedToBePrepared() { ...@@ -1445,8 +1445,8 @@ void TileManager::CheckIfMoreTilesNeedToBePrepared() {
CHECK(tile_task_manager_); CHECK(tile_task_manager_);
// Schedule all checks in case we're left with solid color tiles only. // Schedule all checks in case we're left with solid color tiles only.
signals_.ready_to_activate = true; signals_.activate_tile_tasks_completed = true;
signals_.ready_to_draw = true; signals_.draw_tile_tasks_completed = true;
signals_.all_tile_tasks_completed = true; signals_.all_tile_tasks_completed = true;
signals_check_notifier_.Schedule(); signals_check_notifier_.Schedule();
...@@ -1527,8 +1527,10 @@ TileManager::ScheduledTasksStateAsValue() const { ...@@ -1527,8 +1527,10 @@ TileManager::ScheduledTasksStateAsValue() const {
std::unique_ptr<base::trace_event::TracedValue> state( std::unique_ptr<base::trace_event::TracedValue> state(
new base::trace_event::TracedValue()); new base::trace_event::TracedValue());
state->BeginDictionary("tasks_pending"); state->BeginDictionary("tasks_pending");
state->SetBoolean("ready_to_activate", signals_.ready_to_activate); state->SetBoolean("activate_tile_tasks_completed",
state->SetBoolean("ready_to_draw", signals_.ready_to_draw); signals_.activate_tile_tasks_completed);
state->SetBoolean("draw_tile_tasks_completed",
signals_.draw_tile_tasks_completed);
state->SetBoolean("all_tile_tasks_completed", state->SetBoolean("all_tile_tasks_completed",
signals_.all_tile_tasks_completed); signals_.all_tile_tasks_completed);
state->EndDictionary(); state->EndDictionary();
...@@ -1602,15 +1604,23 @@ void TileManager::CheckPendingGpuWorkTiles(bool issue_signals) { ...@@ -1602,15 +1604,23 @@ void TileManager::CheckPendingGpuWorkTiles(bool issue_signals) {
} }
// Update our signals now that we know whether we have pending resources. // Update our signals now that we know whether we have pending resources.
signals_.ready_to_activate = signals_.activate_gpu_work_completed =
(pending_required_for_activation_callback_id_ == 0); (pending_required_for_activation_callback_id_ == 0);
signals_.ready_to_draw = (pending_required_for_draw_callback_id_ == 0); signals_.draw_gpu_work_completed =
(pending_required_for_draw_callback_id_ == 0);
if (issue_signals && (signals_.ready_to_activate || signals_.ready_to_draw))
signals_check_notifier_.Schedule();
// We've just updated all pending tile requirements if necessary. // We've just updated all pending tile requirements if necessary.
pending_tile_requirements_dirty_ = false; pending_tile_requirements_dirty_ = false;
if (!issue_signals)
return;
bool can_activate = signals_.activate_gpu_work_completed &&
signals_.activate_tile_tasks_completed;
bool can_draw =
signals_.draw_gpu_work_completed && signals_.draw_tile_tasks_completed;
if (can_activate || can_draw)
signals_check_notifier_.Schedule();
} }
// Utility function that can be used to create a "Task set finished" task that // Utility function that can be used to create a "Task set finished" task that
...@@ -1751,19 +1761,6 @@ bool TileManager::MemoryUsage::Exceeds(const MemoryUsage& limit) const { ...@@ -1751,19 +1761,6 @@ bool TileManager::MemoryUsage::Exceeds(const MemoryUsage& limit) const {
resource_count_ > limit.resource_count_; resource_count_ > limit.resource_count_;
} }
TileManager::Signals::Signals() {
reset();
}
void TileManager::Signals::reset() {
ready_to_activate = false;
did_notify_ready_to_activate = false;
ready_to_draw = false;
did_notify_ready_to_draw = false;
all_tile_tasks_completed = false;
did_notify_all_tile_tasks_completed = false;
}
TileManager::PrioritizedWorkToSchedule::PrioritizedWorkToSchedule() = default; TileManager::PrioritizedWorkToSchedule::PrioritizedWorkToSchedule() = default;
TileManager::PrioritizedWorkToSchedule::PrioritizedWorkToSchedule( TileManager::PrioritizedWorkToSchedule::PrioritizedWorkToSchedule(
PrioritizedWorkToSchedule&& other) = default; PrioritizedWorkToSchedule&& other) = default;
......
...@@ -312,16 +312,16 @@ class CC_EXPORT TileManager : CheckerImageTrackerClient { ...@@ -312,16 +312,16 @@ class CC_EXPORT TileManager : CheckerImageTrackerClient {
}; };
struct Signals { struct Signals {
Signals(); bool activate_tile_tasks_completed = false;
bool draw_tile_tasks_completed = false;
bool all_tile_tasks_completed = false;
void reset(); bool activate_gpu_work_completed = false;
bool draw_gpu_work_completed = false;
bool ready_to_activate; bool did_notify_ready_to_activate = false;
bool did_notify_ready_to_activate; bool did_notify_ready_to_draw = false;
bool ready_to_draw; bool did_notify_all_tile_tasks_completed = false;
bool did_notify_ready_to_draw;
bool all_tile_tasks_completed;
bool did_notify_all_tile_tasks_completed;
}; };
struct PrioritizedWorkToSchedule { struct PrioritizedWorkToSchedule {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment