cc: Start using raster/eviction iterators.

cc/layers/picture_layer_impl.cc

Index: cc/layers/picture_layer_impl.cc
diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc
index 3a9bdd9af3e651b105e589c058bda8d530a96d50..472634e622af58de6dab870c678634774899c680 100644
--- a/cc/layers/picture_layer_impl.cc
+++ b/cc/layers/picture_layer_impl.cc
@@ -97,15 +97,6 @@ void PictureLayerImpl::PushPropertiesTo(LayerImpl* base_layer) {
   DoPostCommitInitializationIfNeeded();
   PictureLayerImpl* layer_impl = static_cast<PictureLayerImpl*>(base_layer);
 
-  // We have already synced the important bits from the the active layer, and
-  // we will soon swap out its tilings and use them for recycling. However,
-  // there are now tiles in this layer's tilings that were unref'd and replaced
-  // with new tiles (due to invalidation). This resets all active priorities on
-  // the to-be-recycled tiling to ensure replaced tiles don't linger and take
-  // memory (due to a stale 'active' priority).
-  if (layer_impl->tilings_)
-    layer_impl->tilings_->DidBecomeRecycled();
-
   LayerImpl::PushPropertiesTo(base_layer);
 
   // When the pending tree pushes to the active tree, the pending twin
@@ -229,10 +220,10 @@ void PictureLayerImpl::AppendQuads(
         } else if (mode == ManagedTileState::TileVersion::PICTURE_PILE_MODE) {
           color = DebugColors::PictureTileBorderColor();
           width = DebugColors::PictureTileBorderWidth(layer_tree_impl());
-        } else if (iter->priority(ACTIVE_TREE).resolution == HIGH_RESOLUTION) {
+        } else if (iter.resolution() == HIGH_RESOLUTION) {
           color = DebugColors::HighResTileBorderColor();
           width = DebugColors::HighResTileBorderWidth(layer_tree_impl());
-        } else if (iter->priority(ACTIVE_TREE).resolution == LOW_RESOLUTION) {
+        } else if (iter.resolution() == LOW_RESOLUTION) {
           color = DebugColors::LowResTileBorderColor();
           width = DebugColors::LowResTileBorderWidth(layer_tree_impl());
         } else if (iter->contents_scale() > max_contents_scale) {
@@ -390,7 +381,7 @@ void PictureLayerImpl::AppendQuads(
       continue;
     }
 
-    if (iter->priority(ACTIVE_TREE).resolution != HIGH_RESOLUTION) {
+    if (iter.resolution() != HIGH_RESOLUTION) {
       append_quads_data->approximated_visible_content_area +=
           visible_geometry_rect.width() * visible_geometry_rect.height();
     }
@@ -459,9 +450,6 @@ void PictureLayerImpl::UpdateTiles(
   should_update_tile_priorities_ = true;
 
   UpdateTilePriorities(occlusion_in_content_space);
-
-  if (layer_tree_impl()->IsPendingTree())
-    MarkVisibleResourcesAsRequired();
 }
 
 void PictureLayerImpl::UpdateTilePriorities(
@@ -541,7 +529,6 @@ void PictureLayerImpl::NotifyTileStateChanged(const Tile* tile) {
 
 void PictureLayerImpl::DidBecomeActive() {
   LayerImpl::DidBecomeActive();
-  tilings_->DidBecomeActive();
   layer_tree_impl()->DidModifyTilePriorities();

[danakj, 2014/09/19 01:41:46]

Can we do this from LayerTreeHostImpl directly and remove this virtual method
from existance? You can leave a TODO if you like.

[vmpstr, 2014/09/19 21:22:52]

I've tried to just call DidModifyTilePriorities from LTHI, but that failed
mostly because of tests, I suspect (a crash in unittests). I left a todo for
now, but will go back and try to make it work.

 }
 
@@ -634,6 +621,15 @@ int PictureLayerImpl::GetSkewportExtrapolationLimitInContentPixels() const {
       .skewport_extrapolation_limit_in_content_pixels;
 }
 
+bool PictureLayerImpl::RequiresHighResToDraw() const {
+  const PictureLayerImpl* layer = this;
+  if (GetTree() == PENDING_TREE)

[danakj, 2014/09/19 01:41:46]

GetTree() is a virtual function call, how about just ask the layer_tree_impl()
directly?

[vmpstr, 2014/09/19 21:22:52]

Done.

+    layer = twin_layer_;
+  if (layer)
+    return layer->layer_tree_impl()->RequiresHighResToDraw();
+  return false;
+}
+
 gfx::Size PictureLayerImpl::CalculateTileSize(
     const gfx::Size& content_bounds) const {
   int max_texture_size =
@@ -772,158 +768,6 @@ ResourceProvider::ResourceId PictureLayerImpl::ContentsResourceId() const {
   return tile_version.get_resource_id();
 }
 
-void PictureLayerImpl::MarkVisibleResourcesAsRequired() const {
-  DCHECK(layer_tree_impl()->IsPendingTree());
-  DCHECK(ideal_contents_scale_);
-  DCHECK_GT(tilings_->num_tilings(), 0u);
-
-  // The goal of this function is to find the minimum set of tiles that need to
-  // be ready to draw in order to activate without flashing content from a
-  // higher res on the active tree to a lower res on the pending tree.
-
-  // First, early out for layers with no visible content.
-  if (visible_content_rect().IsEmpty())
-    return;
-
-  // Only mark tiles inside the viewport for tile priority as required for
-  // activation. This viewport is normally the same as the draw viewport but
-  // can be independently overridden by embedders like Android WebView with
-  // SetExternalDrawConstraints.
-  gfx::Rect rect = GetViewportForTilePriorityInContentSpace();
-
-  float min_acceptable_scale =
-      std::min(raster_contents_scale_, ideal_contents_scale_);
-
-  if (PictureLayerImpl* twin = twin_layer_) {
-    float twin_min_acceptable_scale =
-        std::min(twin->ideal_contents_scale_, twin->raster_contents_scale_);
-    // Ignore 0 scale in case CalculateContentsScale() has never been
-    // called for active twin.
-    if (twin_min_acceptable_scale != 0.0f) {
-      min_acceptable_scale =
-          std::min(min_acceptable_scale, twin_min_acceptable_scale);
-    }
-  }
-
-  PictureLayerTiling* high_res = NULL;
-  PictureLayerTiling* low_res = NULL;
-
-  // First pass: ready to draw tiles in acceptable but non-ideal tilings are
-  // marked as required for activation so that their textures are not thrown
-  // away; any non-ready tiles are not marked as required.
-  Region missing_region = rect;
-  for (size_t i = 0; i < tilings_->num_tilings(); ++i) {
-    PictureLayerTiling* tiling = tilings_->tiling_at(i);
-    DCHECK(tiling->has_ever_been_updated());
-
-    if (tiling->resolution() == LOW_RESOLUTION) {
-      DCHECK(!low_res) << "There can only be one low res tiling";
-      low_res = tiling;
-    }
-    if (tiling->contents_scale() < min_acceptable_scale)
-      continue;
-    if (tiling->resolution() == HIGH_RESOLUTION) {
-      DCHECK(!high_res) << "There can only be one high res tiling";
-      high_res = tiling;
-      continue;
-    }
-    for (PictureLayerTiling::CoverageIterator iter(tiling, 1.f, rect); iter;
-         ++iter) {
-      if (!*iter || !iter->IsReadyToDraw())
-        continue;
-
-      missing_region.Subtract(iter.geometry_rect());
-      iter->MarkRequiredForActivation();
-    }
-  }
-  DCHECK(high_res) << "There must be one high res tiling";
-
-  // If these pointers are null (because no twin, no matching tiling, or the
-  // simpification just below), then high res tiles will be required to fill any
-  // holes left by the first pass above.  If the pointers are valid, then this
-  // layer is allowed to skip any tiles that are not ready on its twin.
-  const PictureLayerTiling* twin_high_res = NULL;
-  const PictureLayerTiling* twin_low_res = NULL;
-
-  if (twin_layer_) {
-    // As a simplification, only allow activating to skip twin tiles that the
-    // active layer is also missing when both this layer and its twin have
-    // "simple" sets of tilings: only 2 tilings (high and low) or only 1 high
-    // res tiling. This avoids having to iterate/track coverage of non-ideal
-    // tilings during the last draw call on the active layer.
-    if (tilings_->num_tilings() <= 2 &&
-        twin_layer_->tilings_->num_tilings() <= tilings_->num_tilings()) {
-      twin_low_res = low_res ? GetTwinTiling(low_res) : NULL;
-      twin_high_res = high_res ? GetTwinTiling(high_res) : NULL;
-    }
-
-    // If this layer and its twin have different transforms, then don't compare
-    // them and only allow activating to high res tiles, since tiles on each
-    // layer will be in different places on screen.
-    if (twin_layer_->layer_tree_impl()->RequiresHighResToDraw() ||
-        bounds() != twin_layer_->bounds() ||
-        draw_properties().screen_space_transform !=
-            twin_layer_->draw_properties().screen_space_transform) {
-      twin_high_res = NULL;
-      twin_low_res = NULL;
-    }
-  }
-
-  // As a second pass, mark as required any visible high res tiles not filled in
-  // by acceptable non-ideal tiles from the first pass.
-  if (MarkVisibleTilesAsRequired(
-          high_res, twin_high_res, rect, missing_region)) {
-    // As an optional third pass, if a high res tile was skipped because its
-    // twin was also missing, then fall back to mark low res tiles as required
-    // in case the active twin is substituting those for missing high res
-    // content. Only suitable, when low res is enabled.
-    if (low_res) {
-      MarkVisibleTilesAsRequired(low_res, twin_low_res, rect, missing_region);
-    }
-  }
-}
-
-bool PictureLayerImpl::MarkVisibleTilesAsRequired(
-    PictureLayerTiling* tiling,
-    const PictureLayerTiling* optional_twin_tiling,
-    const gfx::Rect& rect,
-    const Region& missing_region) const {
-  bool twin_had_missing_tile = false;
-  for (PictureLayerTiling::CoverageIterator iter(tiling, 1.f, rect); iter;
-       ++iter) {
-    Tile* tile = *iter;
-    // A null tile (i.e. missing recording) can just be skipped.
-    if (!tile)
-      continue;
-
-    // If the tile is occluded, don't mark it as required for activation.
-    if (tile->is_occluded(PENDING_TREE))
-      continue;
-
-    // If the missing region doesn't cover it, this tile is fully
-    // covered by acceptable tiles at other scales.
-    if (!missing_region.Intersects(iter.geometry_rect()))
-      continue;
-
-    // If the twin tile doesn't exist (i.e. missing recording or so far away
-    // that it is outside the visible tile rect) or this tile is shared between
-    // with the twin, then this tile isn't required to prevent flashing.
-    if (optional_twin_tiling) {
-      Tile* twin_tile = optional_twin_tiling->TileAt(iter.i(), iter.j());
-      if (!twin_tile || twin_tile == tile) {
-        // However if the shared tile is being used on the active tree, then
-        // there's no missing content in this place, and low res is not needed.
-        if (!twin_tile || !twin_tile->IsReadyToDraw())
-          twin_had_missing_tile = true;
-        continue;
-      }
-    }
-
-    tile->MarkRequiredForActivation();
-  }
-  return twin_had_missing_tile;
-}
-
 void PictureLayerImpl::DoPostCommitInitialization() {
   DCHECK(needs_post_commit_initialization_);
   DCHECK(layer_tree_impl()->IsPendingTree());
@@ -1319,8 +1163,7 @@ void PictureLayerImpl::SanityCheckTilingState() const {
   if (tilings_->num_tilings() == 0)
     return;
 
-  // MarkVisibleResourcesAsRequired depends on having exactly 1 high res
-  // tiling to mark its tiles as being required for activation.
+  // We should only have one high res tiling.
   DCHECK_EQ(1, tilings_->NumHighResTilings());
 #endif
 }
@@ -1479,10 +1322,12 @@ bool PictureLayerImpl::AllTilesRequiredForActivationAreReadyToDraw() const {
          ++iter) {
       const Tile* tile = *iter;
       // A null tile (i.e. missing recording) can just be skipped.
+      // TODO(vmpstr): Verify this is true if we create tiles in raster
+      // iterators.
       if (!tile)
         continue;
 
-      if (tile->required_for_activation() && !tile->IsReadyToDraw())
+      if (tiling->IsTileRequiredForActivation(tile) && !tile->IsReadyToDraw())

[danakj, 2014/09/19 01:41:46]

The pieces all look good to me, but can you explain how they fit together? What
is doing the iterator walk and setting the priority on all the tiles before we
come here?

[vmpstr, 2014/09/19 21:22:52]

The priority of the tile here isn't accessed (IsTileRequiredForActivation
doesn't care and IsReadyToDraw is independent of priority). If we were to update
the priority, then we'd find that it's in the NOW bin. That's done, because we
only do the coverage over the visible rect.

[danakj, 2014/09/19 22:30:06]

Oh ok. It'd be nice if I could tell from the function name if it uses the tile's
priority or not, maybe. Or maybe this gets more clear when priority is always
through the iterators, so okay, let's work on that next.

[vmpstr, 2014/09/22 18:01:59]

Yeah, I want to start kind of assuming that we never use tile priority and make
it explicit when we do. I would prefer something like
GetIsTileRequiredForActivationFromTilePriority if it accessed priority. You know
what I mean? I don't know if that's the case right now, since I just came up
with that, but I think that would be the direction I prefer.

         return false;
     }
   }