cc: Redraw incomplete frames when new texture uploads finish

cc/tile_manager.cc

 // Copyright 2012 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/tile_manager.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
@@ skipping 207 matching lines @@
   // Sort by bin.
   std::sort(tiles_.begin(), tiles_.end(), BinComparator());
 
   // Assign gpu memory and determine what tiles need to be rasterized.
   AssignGpuMemoryToTiles();
 
   // Finally, kick the rasterizer.
   DispatchMoreTasks();
 }
 
-void TileManager::CheckForCompletedSetPixels() {
+bool TileManager::CheckForCompletedSetPixels() {
+  bool visible_highres_tile_was_completed = false;
+
   while (!tiles_with_pending_set_pixels_.empty()) {
     Tile* tile = tiles_with_pending_set_pixels_.front();
     DCHECK(tile->managed_state().resource);
 
     // Set pixel tasks complete in the order they are posted.
     if (!resource_pool_->resource_provider()->didSetPixelsComplete(
           tile->managed_state().resource->id())) {
       break;
     }
 
+    if (tile->priority(ACTIVE_TREE).distance_to_visible_in_pixels == 0 &&
+        tile->priority(ACTIVE_TREE).resolution == HIGH_RESOLUTION)
+      visible_highres_tile_was_completed = true;

[nduca, 2013/01/12 09:14:02]

why not just trigger a setNeedsRedraw here? In this case, I think its okay for
the TileManager to just call m_client->didCompleteTileOnActiveTree() and then
have lthi convert it into setNeedsRedraw.

The distinction between this and the scheduledAction patch you just did may seem
a bit obscure to you: in this case, we aren't needing special behavior on the
redraw, as in it doesn't need to be combiend with other states. So it doesn't
need to push down to the rest of the machine.

[brianderson, 2013/01/12 18:48:06]

I did it this way for two reasons:

1) After the decoupling patch goes through, triggering setNeedsRedraw directly
from here will cause spurious draws.  This function will be called within
scheduledActionDrawAndSwap*, after the setNeedsRedraw has been cleared but
before we actually draw.  So it's possible the upcoming draw is complete
(because all the uploads just finished) and we needlessly setNeedsRedraw for the
next vsync here.

2) If we are in the ACTION_CHECK_FOR_NEW_VISIBLE_TEXTURES state and this
function returns false, the caller can decide to stay in that state without
doing another tree traversal.

[nduca, 2013/01/12 22:04:40]

But look at this function. At this specific line, inside the active-tree,
distance to visible = 0, resolution=high, we KNOW its onscreen. As in, ITS ON
SCREEN. **It would be drawn**

How on earth can the draw be complete yet us get this condition?

[brianderson, 2013/01/14 18:51:12]

The problem is that there can be a missmatch between the state machine's state
and the actual state when we are in this function during an
ACTION_DRAW_IF_POSSIBLE.

SchedulerStateMachine::updateState is called before an action is taken, which
for ACTION_DRAW_IF_POSSIBLE means that m_needsRedraw is set to false before
we've actually drawn the current frame.

Currently, CheckForCompletedSetPixels is always called right before drawing to
make sure we have the most up-to-date information. If CheckForCompletedSetPixels
triggers a setNeedsRedraw directly, it's possible that the frame we are about to
draw for the current vsync is complete and we are just telling the state machine
to redraw an identical frame on the next vsync.

I will admit this is not very clear and that part of the confusion comes from
the way I have the decoupling patch always call CheckForCompletedSetPixels
before drawing. There are a few options we can take to make things clearer:

1) Keep the patch as-is and add a fat comment.
2) Add an explicit state/action that checks for completed pixels before drawing
and remove the check for completed pixels in the draw action.
3) Call SchedulerStateMachine::updateState after taking an action (instead of
before) so m_needsRedraw isn't cleared until after we've actually drawn.  Will
probably have side-effects that affect a lot of the state machine code.

Although I think 2 will be the most verbose, I think it will also be the
clearest and most explicit.  I plan to implement 2, unless anyone has a better
solution.

+
     // It's now safe to release the pixel buffer.
     resource_pool_->resource_provider()->releasePixelBuffer(
         tile->managed_state().resource->id());
 
     DidFinishTileInitialization(tile);
     tiles_with_pending_set_pixels_.pop();
   }
+
+  return visible_highres_tile_was_completed;
 }
 
 void TileManager::GetRenderingStats(RenderingStats* stats) {
   raster_worker_->GetRenderingStats(stats);
   stats->totalDeferredImageCacheHitCount =
       rendering_stats_.totalDeferredImageCacheHitCount;
   stats->totalImageGatheringCount = rendering_stats_.totalImageGatheringCount;
   stats->totalImageGatheringTimeInSeconds =
       rendering_stats_.totalImageGatheringTimeInSeconds;
 }
@@ skipping 258 matching lines @@
 }
 
 void TileManager::DidFinishTileInitialization(Tile* tile) {
   ManagedTileState& managed_tile_state = tile->managed_state();
   DCHECK(managed_tile_state.resource);
   managed_tile_state.resource_is_being_initialized = false;
   managed_tile_state.can_be_freed = true;
 }
 
 }  // namespace cc