cc: Pass priority rect information from iterators to tilings.

cc/resources/tiling_set_raster_queue_all.cc

 // Copyright 2014 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/resources/tiling_set_raster_queue_all.h"
 
 #include <utility>
 
 #include "cc/resources/picture_layer_tiling_set.h"
 #include "cc/resources/tile.h"
@@ skipping 135 matching lines @@
     TilePriority::PriorityBin tile_type = stages_[current_stage_].tile_type;
 
     if (!iterators_[index].done() && iterators_[index].type() == tile_type)
       break;
     ++current_stage_;
   }
 }
 
 // OnePriorityRectIterator
 TilingSetRasterQueueAll::OnePriorityRectIterator::OnePriorityRectIterator()
-    : tile_(nullptr), tiling_(nullptr), tiling_data_(nullptr) {
+    : tile_(nullptr),
+      tiling_(nullptr),
+      pending_twin_tiling_(nullptr),
+      tiling_data_(nullptr) {
 }
 
 TilingSetRasterQueueAll::OnePriorityRectIterator::OnePriorityRectIterator(
     PictureLayerTiling* tiling,
-    TilingData* tiling_data)
-    : tile_(nullptr), tiling_(tiling), tiling_data_(tiling_data) {
+    TilingData* tiling_data,
+    PictureLayerTiling::PriorityRect priority_rect)
+    : tile_(nullptr),
+      tiling_(tiling),
+      pending_twin_tiling_(

[vmpstr, 2015/05/05 18:04:02]

Why are you adding this here? Why is getting tiling->pending_visible_rect not
sufficient? It should return the same value that you need if this is an active
tree, and it would return empty rect if this is the pending tree.

[USE eero AT chromium.org, 2015/05/06 12:05:20]

As a premature (size) optimisation. I removed it.

+          tiling->tree() == ACTIVE_TREE
+              ? tiling->client()->GetPendingOrActiveTwinTiling(tiling)
+              : nullptr),
+      tiling_data_(tiling_data),
+      priority_rect_(priority_rect) {
 }
 
 template <typename TilingIteratorType>
 void TilingSetRasterQueueAll::OnePriorityRectIterator::AdvanceToNextTile(
     TilingIteratorType* iterator) {
-  tile_ = nullptr;
-  while (!tile_ || !TileNeedsRaster(tile_)) {
+  bool found_tile = false;
+  while (!found_tile) {
     ++(*iterator);
     if (!(*iterator)) {
       tile_ = nullptr;
-      return;
+      break;
     }
-    tile_ = tiling_->TileAt(iterator->index_x(), iterator->index_y());
+    found_tile = GetFirstTileAndCheckIfValid(iterator);
   }
-  tiling_->UpdateTilePriority(tile_);
 }
 
 template <typename TilingIteratorType>
 bool TilingSetRasterQueueAll::OnePriorityRectIterator::
     GetFirstTileAndCheckIfValid(TilingIteratorType* iterator) {
   tile_ = tiling_->TileAt(iterator->index_x(), iterator->index_y());
   if (!tile_ || !TileNeedsRaster(tile_)) {
     tile_ = nullptr;
     return false;
   }
-  tiling_->UpdateTilePriority(tile_);
+  if (priority_rect_ > PictureLayerTiling::PriorityRect::PENDING_VISIBLE_RECT &&
+      pending_twin_tiling_ &&
+      pending_twin_tiling_->current_visible_rect().Intersects(
+          tile_->content_rect())) {
+    return false;
+  }
+  tiling_->UpdateTilePriority(tile_, priority_rect_);
   return true;
 }
 
 // VisibleTilingIterator.
 TilingSetRasterQueueAll::VisibleTilingIterator::VisibleTilingIterator(
     PictureLayerTiling* tiling,
     TilingData* tiling_data)
-    : OnePriorityRectIterator(tiling, tiling_data) {
+    : OnePriorityRectIterator(tiling,
+                              tiling_data,
+                              PictureLayerTiling::VISIBLE_RECT) {
   if (!tiling_->has_visible_rect_tiles())
     return;
   iterator_ =
       TilingData::Iterator(tiling_data_, tiling_->current_visible_rect(),
                            false /* include_borders */);
   if (!iterator_)
     return;
   if (!GetFirstTileAndCheckIfValid(&iterator_))
     ++(*this);
 }
 
 TilingSetRasterQueueAll::VisibleTilingIterator&
     TilingSetRasterQueueAll::VisibleTilingIterator::
     operator++() {
   AdvanceToNextTile(&iterator_);
   return *this;
 }
 
 // PendingVisibleTilingIterator.
 TilingSetRasterQueueAll::PendingVisibleTilingIterator::
     PendingVisibleTilingIterator(PictureLayerTiling* tiling,
                                  TilingData* tiling_data)
-    : OnePriorityRectIterator(tiling, tiling_data) {
-  iterator_ = TilingData::DifferenceIterator(tiling_data_,
-                                             tiling_->pending_visible_rect(),
-                                             tiling_->current_visible_rect());
+    : OnePriorityRectIterator(tiling,
+                              tiling_data,
+                              PictureLayerTiling::PENDING_VISIBLE_RECT) {
+  if (!pending_twin_tiling_ || !pending_twin_tiling_->has_visible_rect_tiles())
+    return;
+  iterator_ = TilingData::DifferenceIterator(
+      tiling_data_, pending_twin_tiling_->current_visible_rect(),
+      tiling_->current_visible_rect());
   if (!iterator_)
     return;
   if (!GetFirstTileAndCheckIfValid(&iterator_))
     ++(*this);
 }
 
 TilingSetRasterQueueAll::PendingVisibleTilingIterator&
     TilingSetRasterQueueAll::PendingVisibleTilingIterator::
     operator++() {
   AdvanceToNextTile(&iterator_);
   return *this;
 }
 
 // SkewportTilingIterator.
 TilingSetRasterQueueAll::SkewportTilingIterator::SkewportTilingIterator(
     PictureLayerTiling* tiling,
     TilingData* tiling_data)
-    : OnePriorityRectIterator(tiling, tiling_data),
-      pending_visible_rect_(tiling->pending_visible_rect()) {
+    : OnePriorityRectIterator(tiling,
+                              tiling_data,
+                              PictureLayerTiling::SKEWPORT_RECT) {
   if (!tiling_->has_skewport_rect_tiles())
     return;
   iterator_ = TilingData::SpiralDifferenceIterator(
       tiling_data_, tiling_->current_skewport_rect(),
       tiling_->current_visible_rect(), tiling_->current_visible_rect());
   if (!iterator_)
     return;
-  if (!GetFirstTileAndCheckIfValid(&iterator_)) {
-    ++(*this);
-    return;
-  }
-  if (tile_->content_rect().Intersects(pending_visible_rect_))
+  if (!GetFirstTileAndCheckIfValid(&iterator_))
     ++(*this);
 }
 
 TilingSetRasterQueueAll::SkewportTilingIterator&
     TilingSetRasterQueueAll::SkewportTilingIterator::
     operator++() {
   AdvanceToNextTile(&iterator_);
-  while (!done()) {
-    if (!tile_->content_rect().Intersects(pending_visible_rect_))
-      break;
-    AdvanceToNextTile(&iterator_);
-  }
   return *this;
 }
 
 // SoonBorderTilingIterator.
 TilingSetRasterQueueAll::SoonBorderTilingIterator::SoonBorderTilingIterator(
     PictureLayerTiling* tiling,
     TilingData* tiling_data)
-    : OnePriorityRectIterator(tiling, tiling_data),
-      pending_visible_rect_(tiling->pending_visible_rect()) {
+    : OnePriorityRectIterator(tiling,
+                              tiling_data,
+                              PictureLayerTiling::SOON_BORDER_RECT) {
   if (!tiling_->has_soon_border_rect_tiles())
     return;
   iterator_ = TilingData::SpiralDifferenceIterator(
       tiling_data_, tiling_->current_soon_border_rect(),
       tiling_->current_skewport_rect(), tiling_->current_visible_rect());
   if (!iterator_)
     return;
-  if (!GetFirstTileAndCheckIfValid(&iterator_)) {
-    ++(*this);
-    return;
-  }
-  if (tile_->content_rect().Intersects(pending_visible_rect_))
+  if (!GetFirstTileAndCheckIfValid(&iterator_))
     ++(*this);
 }
 
 TilingSetRasterQueueAll::SoonBorderTilingIterator&
     TilingSetRasterQueueAll::SoonBorderTilingIterator::
     operator++() {
   AdvanceToNextTile(&iterator_);
-  while (!done()) {
-    if (!tile_->content_rect().Intersects(pending_visible_rect_))
-      break;
-    AdvanceToNextTile(&iterator_);
-  }
   return *this;
 }
 
 // EventuallyTilingIterator.
 TilingSetRasterQueueAll::EventuallyTilingIterator::EventuallyTilingIterator(
     PictureLayerTiling* tiling,
     TilingData* tiling_data)
-    : OnePriorityRectIterator(tiling, tiling_data),
-      pending_visible_rect_(tiling->pending_visible_rect()) {
+    : OnePriorityRectIterator(tiling,
+                              tiling_data,
+                              PictureLayerTiling::EVENTUALLY_RECT) {
   if (!tiling_->has_eventually_rect_tiles())
     return;
   iterator_ = TilingData::SpiralDifferenceIterator(
       tiling_data_, tiling_->current_eventually_rect(),
       tiling_->current_skewport_rect(), tiling_->current_soon_border_rect());
   if (!iterator_)
     return;
-  if (!GetFirstTileAndCheckIfValid(&iterator_)) {
-    ++(*this);
-    return;
-  }
-  if (tile_->content_rect().Intersects(pending_visible_rect_))

[vmpstr, 2015/05/05 18:04:02]

I see that this is less code, but can you maybe do this as a separate patch? I'd
rather be a bit more explicit about checking pending visible rect in each
iterator for now. 

I don't think your main change here to not intersect in priority rects depends
on this, right? That is, we still have this intersect check in GetFirstTile...
function

[USE eero AT chromium.org, 2015/05/06 12:05:20]

Yes, of course.
My main change does not depend on removing these code blocks which lie after
GetFirstTileAndCheckIfValid and AdvanceToNextTile calls. However, my main change
does depend on adding the skipping logic to GetFirstTileAndCheckIfValid.

Previously, tile priories for pending visible tiles which were not visible tiles
were calculated twice. First as pending visible tiles and then as
skewport/soon-border/eventually tiles.

But now I want DCHECK in PictureLayerTiling::ComputePriorityForTile that the
function is called with a correct priority rect type (see
https://codereview.chromium.org/1126813002/diff/1/cc/resources/picture_layer_...).
For that DCHECK to pass, the tile priorities must be computed only once, thus
GetFirstTileAndCheckIfValid must skip pending visible tiles before updating tile
priorities.

You are correct that these code removals are not mandatory. So I left them
intact and added only TODO comments. Let's clean up them later.

+  if (!GetFirstTileAndCheckIfValid(&iterator_))
     ++(*this);
 }
 
 TilingSetRasterQueueAll::EventuallyTilingIterator&
     TilingSetRasterQueueAll::EventuallyTilingIterator::
     operator++() {
   AdvanceToNextTile(&iterator_);
-  while (!done()) {
-    if (!tile_->content_rect().Intersects(pending_visible_rect_))
-      break;
-    AdvanceToNextTile(&iterator_);
-  }
   return *this;
 }
 
 // TilingIterator
 TilingSetRasterQueueAll::TilingIterator::TilingIterator()
     : tiling_(NULL), current_tile_(NULL) {
 }
 
 TilingSetRasterQueueAll::TilingIterator::TilingIterator(
     PictureLayerTiling* tiling,
     TilingData* tiling_data)
     : tiling_(tiling),
       tiling_data_(tiling_data),
-      phase_(VISIBLE_RECT),
+      phase_(Phase::VISIBLE_RECT),
       current_tile_(NULL) {
   visible_iterator_ = VisibleTilingIterator(tiling_, tiling_data_);
   if (visible_iterator_.done()) {
     AdvancePhase();
     return;
   }
   current_tile_ = *visible_iterator_;
 }
 
 TilingSetRasterQueueAll::TilingIterator::~TilingIterator() {
 }
 
 void TilingSetRasterQueueAll::TilingIterator::AdvancePhase() {
-  DCHECK_LT(phase_, EVENTUALLY_RECT);
+  DCHECK_LT(phase_, Phase::EVENTUALLY_RECT);
 
   current_tile_ = nullptr;
-  while (!current_tile_ && phase_ < EVENTUALLY_RECT) {
+  while (!current_tile_ && phase_ < Phase::EVENTUALLY_RECT) {
     phase_ = static_cast<Phase>(phase_ + 1);
     switch (phase_) {
-      case VISIBLE_RECT:
+      case Phase::VISIBLE_RECT:
         NOTREACHED();
         return;
-      case PENDING_VISIBLE_RECT:
+      case Phase::PENDING_VISIBLE_RECT:
         pending_visible_iterator_ =
             PendingVisibleTilingIterator(tiling_, tiling_data_);
         if (!pending_visible_iterator_.done())
           current_tile_ = *pending_visible_iterator_;
         break;
-      case SKEWPORT_RECT:
+      case Phase::SKEWPORT_RECT:
         skewport_iterator_ = SkewportTilingIterator(tiling_, tiling_data_);
         if (!skewport_iterator_.done())
           current_tile_ = *skewport_iterator_;
         break;
-      case SOON_BORDER_RECT:
+      case Phase::SOON_BORDER_RECT:
         soon_border_iterator_ = SoonBorderTilingIterator(tiling_, tiling_data_);
         if (!soon_border_iterator_.done())
           current_tile_ = *soon_border_iterator_;
         break;
-      case EVENTUALLY_RECT:
+      case Phase::EVENTUALLY_RECT:
         eventually_iterator_ = EventuallyTilingIterator(tiling_, tiling_data_);
         if (!eventually_iterator_.done())
           current_tile_ = *eventually_iterator_;
         break;
     }
   }
 }
 
 TilingSetRasterQueueAll::TilingIterator&
     TilingSetRasterQueueAll::TilingIterator::
     operator++() {
   switch (phase_) {
-    case VISIBLE_RECT:
+    case Phase::VISIBLE_RECT:
       ++visible_iterator_;
       if (visible_iterator_.done()) {
         AdvancePhase();
         return *this;
       }
       current_tile_ = *visible_iterator_;
       break;
-    case PENDING_VISIBLE_RECT:
+    case Phase::PENDING_VISIBLE_RECT:
       ++pending_visible_iterator_;
       if (pending_visible_iterator_.done()) {
         AdvancePhase();
         return *this;
       }
       current_tile_ = *pending_visible_iterator_;
       break;
-    case SKEWPORT_RECT:
+    case Phase::SKEWPORT_RECT:
       ++skewport_iterator_;
       if (skewport_iterator_.done()) {
         AdvancePhase();
         return *this;
       }
       current_tile_ = *skewport_iterator_;
       break;
-    case SOON_BORDER_RECT:
+    case Phase::SOON_BORDER_RECT:
       ++soon_border_iterator_;
       if (soon_border_iterator_.done()) {
         AdvancePhase();
         return *this;
       }
       current_tile_ = *soon_border_iterator_;
       break;
-    case EVENTUALLY_RECT:
+    case Phase::EVENTUALLY_RECT:
       ++eventually_iterator_;
       if (eventually_iterator_.done()) {
         current_tile_ = nullptr;
         return *this;
       }
       current_tile_ = *eventually_iterator_;
       break;
   }
   return *this;
 }
 
 }  // namespace cc