Consider occluded tiles during eviction with occluded as Tile property.

cc/layers/picture_layer_impl_unittest.cc

Index: cc/layers/picture_layer_impl_unittest.cc
diff --git a/cc/layers/picture_layer_impl_unittest.cc b/cc/layers/picture_layer_impl_unittest.cc
index 2258655dbf6f678b4858290e86c6d30abbfed1d8..f03208af0a3f6f8e6c14bcc2a400923b0f79baa6 100644
--- a/cc/layers/picture_layer_impl_unittest.cc
+++ b/cc/layers/picture_layer_impl_unittest.cc
@@ -3204,5 +3204,317 @@ TEST_F(OcclusionTrackingPictureLayerImplTest, DifferentOcclusionOnTrees) {
     }
   }
 }
+
+TEST_F(OcclusionTrackingPictureLayerImplTest,
+       OccludedTilesConsideredDuringEviction) {
+  gfx::Size tile_size(102, 102);
+  gfx::Size layer_bounds(1000, 1000);
+  gfx::Size viewport_size(500, 500);
+  gfx::Point pending_occluding_layer_position(310, 0);
+  gfx::Point active_occluding_layer_position(0, 310);
+  gfx::Rect invalidation_rect(230, 230, 102, 102);
+
+  scoped_refptr<FakePicturePileImpl> pending_pile =
+      FakePicturePileImpl::CreateFilledPile(tile_size, layer_bounds);
+  scoped_refptr<FakePicturePileImpl> active_pile =
+      FakePicturePileImpl::CreateFilledPile(tile_size, layer_bounds);
+  SetupTrees(pending_pile, active_pile);
+
+  pending_layer_->set_fixed_tile_size(tile_size);
+  active_layer_->set_fixed_tile_size(tile_size);
+
+  float low_res_factor = host_impl_.settings().low_res_contents_scale_factor;
+
+  std::vector<PictureLayerTiling*> tilings;
+  tilings.push_back(pending_layer_->AddTiling(low_res_factor));
+  tilings.push_back(pending_layer_->AddTiling(0.3f));
+  tilings.push_back(pending_layer_->AddTiling(0.7f));
+  tilings.push_back(pending_layer_->AddTiling(1.0f));
+  tilings.push_back(pending_layer_->AddTiling(2.0f));
+
+  EXPECT_EQ(5u, pending_layer_->num_tilings());
+  EXPECT_EQ(5u, active_layer_->num_tilings());
+
+  // Partially occlude the pending layer.
+  pending_layer_->AddChild(LayerImpl::Create(host_impl_.pending_tree(), 1));
+  LayerImpl* pending_occluding_layer = pending_layer_->children()[0];
+  pending_occluding_layer->SetBounds(layer_bounds);
+  pending_occluding_layer->SetContentBounds(layer_bounds);
+  pending_occluding_layer->SetDrawsContent(true);
+  pending_occluding_layer->SetContentsOpaque(true);
+  pending_occluding_layer->SetPosition(pending_occluding_layer_position);
+
+  // Partially occlude the active layer.
+  active_layer_->AddChild(LayerImpl::Create(host_impl_.active_tree(), 2));
+  LayerImpl* active_occluding_layer = active_layer_->children()[0];
+  active_occluding_layer->SetBounds(layer_bounds);
+  active_occluding_layer->SetContentBounds(layer_bounds);
+  active_occluding_layer->SetDrawsContent(true);
+  active_occluding_layer->SetContentsOpaque(true);
+  active_occluding_layer->SetPosition(active_occluding_layer_position);
+
+  // Partially invalidate the pending layer. Tiles inside the invalidation rect
+  // are not shared between trees.
+  pending_layer_->set_invalidation(invalidation_rect);
+
+  host_impl_.SetViewportSize(viewport_size);
+  host_impl_.active_tree()->UpdateDrawProperties();
+  host_impl_.pending_tree()->UpdateDrawProperties();
+
+  size_t expected_occluded_on_pending_count[] = {30u, 5u, 4u, 2u, 2u};
+  size_t expected_occluded_on_active_count[] = {30u, 5u, 4u, 2u, 2u};
+  size_t expected_occluded_on_both_count[] = {9u, 1u, 1u, 1u, 1u};
+  size_t expected_occluded_on_pending_sum = 43u;
+  size_t expected_occluded_on_active_sum = 43u;
+  size_t expected_occluded_on_both_sum = 13u;
+
+  // Verify number of occluded tiles on the pending layer for each tiling.
+  for (size_t i = 0; i < pending_layer_->num_tilings(); ++i) {
+    PictureLayerTiling* tiling = pending_layer_->tilings()->tiling_at(i);
+    tiling->CreateAllTilesForTesting();
+
+    size_t occluded_on_pending_count = 0u;
+    size_t occluded_on_active_count = 0u;
+    size_t occluded_on_both_count = 0u;
+    for (PictureLayerTiling::CoverageIterator iter(
+             tiling,
+             pending_layer_->contents_scale_x(),
+             gfx::Rect(layer_bounds));
+         iter;
+         ++iter) {
+      Tile* tile = *iter;
+
+      if (tile->is_occluded(PENDING_TREE))
+        occluded_on_pending_count++;
+      if (tile->is_occluded(ACTIVE_TREE))
+        occluded_on_active_count++;
+      if (tile->is_occluded(PENDING_TREE) && tile->is_occluded(ACTIVE_TREE))
+        occluded_on_both_count++;
+    }
+    EXPECT_EQ(expected_occluded_on_pending_count[i], occluded_on_pending_count);
+    EXPECT_EQ(expected_occluded_on_active_count[i], occluded_on_active_count);
+    EXPECT_EQ(expected_occluded_on_both_count[i], occluded_on_both_count);
+  }
+
+  // Verify number of occluded tiles on the active layer for each tiling.
+  for (size_t i = 0; i < active_layer_->num_tilings(); ++i) {
+    PictureLayerTiling* tiling = active_layer_->tilings()->tiling_at(i);
+    tiling->CreateAllTilesForTesting();
+
+    size_t occluded_on_pending_count = 0u;
+    size_t occluded_on_active_count = 0u;
+    size_t occluded_on_both_count = 0u;
+    for (PictureLayerTiling::CoverageIterator iter(
+             tiling,
+             active_layer_->contents_scale_x(),
+             gfx::Rect(layer_bounds));
+         iter;
+         ++iter) {
+      Tile* tile = *iter;
+
+      if (tile->is_occluded(PENDING_TREE))
+        occluded_on_pending_count++;
+      if (tile->is_occluded(ACTIVE_TREE))
+        occluded_on_active_count++;
+      if (tile->is_occluded(PENDING_TREE) && tile->is_occluded(ACTIVE_TREE))
+        occluded_on_both_count++;
+    }
+    EXPECT_EQ(expected_occluded_on_pending_count[i], occluded_on_pending_count);
+    EXPECT_EQ(expected_occluded_on_active_count[i], occluded_on_active_count);
+    EXPECT_EQ(expected_occluded_on_both_count[i], occluded_on_both_count);
+  }
+
+  std::vector<Tile*> all_tiles;
+  for (std::vector<PictureLayerTiling*>::iterator tiling_iterator =
+           tilings.begin();
+       tiling_iterator != tilings.end();
+       ++tiling_iterator) {
+    std::vector<Tile*> tiles = (*tiling_iterator)->AllTilesForTesting();
+    std::copy(tiles.begin(), tiles.end(), std::back_inserter(all_tiles));
+  }
+
+  host_impl_.tile_manager()->InitializeTilesWithResourcesForTesting(all_tiles);
+
+  // Iterate over the pending layer considering occlusion on the pending tree.
+  TreePriority tree_priority = NEW_CONTENT_TAKES_PRIORITY;
+  size_t occluded_tile_count = 0u;
+  Tile* last_tile = NULL;
+  for (PictureLayerImpl::LayerEvictionTileIterator it =
+           PictureLayerImpl::LayerEvictionTileIterator(pending_layer_,
+                                                       tree_priority);
+       it;
+       ++it) {
+    Tile* tile = *it;
+    if (!last_tile)
+      last_tile = tile;
+
+    // Since occlusion is considered after priority bin, requirement for
+    // activation, and resolution, the only way we will encounter an occluded
+    // tile after an unoccluded tile is if at least one of those three
+    // properties differs between the two tiles.
+    if (tile->is_occluded_for_tree_priority(tree_priority)) {
+      occluded_tile_count++;
+      if (!last_tile->is_occluded_for_tree_priority(tree_priority)) {
+        EXPECT_TRUE(
+            (tile->priority_for_tree_priority(tree_priority).priority_bin !=

[vmpstr, 2014/07/11 00:44:31]

Can you get the priority first, then check the bin in an effort to clean up this
statement a bit? Or get all three parts into a bool first, something like
"bin_differs, activation_differs, scale_differs" and then have
EXPECT_TRUE(bin_differs || activation_differs || scale_differs);

[jbedley, 2014/07/11 18:00:30]

How does it look now?

+             last_tile->priority_for_tree_priority(tree_priority)
+                 .priority_bin) ||
+            (tile->required_for_activation() !=

[vmpstr, 2014/07/11 00:44:31]

Why is activation a part of this condition? That is, what is the configuration
where we return an unoccluded tile after an occluded one with the reason that
their activation flag differs? (IIUC occluded tiles aren't marked required for
activation)

[jbedley, 2014/07/11 18:00:30]

Tiles that are occluded on the pending tree cannot be marked as required for
activation, but if the tile is only occluded on the active tree then it can be
required for activation. So if the TreePriority is SMOOTHNESS_TAKES_PRIORITY
then we could see a tile that is occluded (on the active tree) and required for
activation, in which case it could be evicted after an unoccluded tile that is
not required for activation since requirement for activation takes precedence.

I think it is ok for a tile that is only occluded on the active tree to be
marked as required for activation. I also think it is right for the
TileEvictionOrder to consider requirement for activation regardless of
TreePriority to make sure we don't evict shared tiles that are required for
activation. If either of these assumptions are wrong, please let me know and I
will change the behaviour.

[vmpstr, 2014/07/11 18:13:36]

Ah, I forgot that we have occlusion for both trees separately. Thanks for the
explanation!

+             last_tile->required_for_activation()) ||
+            (tile->contents_scale() != last_tile->contents_scale()));

[vmpstr, 2014/07/11 00:44:32]

I would pull this check up and just bypass the whole EXPECT_TRUE thing

[jbedley, 2014/07/11 18:00:30]

Sorry, could you explain what you mean by this?

[vmpstr, 2014/07/11 18:13:36]

I just meant something like

if (one_scale != other_scale)
 continue;

EXPECT_TRUE(...)

or something along those lines, but don't worry about it now. I just wanted
EXPECT_TRUE statement to look clean.

The current version looks nice! Thanks.

+      }
+    }
+
+    last_tile = tile;
+  }
+  EXPECT_EQ(expected_occluded_on_pending_sum, occluded_tile_count);
+
+  // Iterate over the active layer considering occlusion on the pending tree.
+  occluded_tile_count = 0u;
+  last_tile = NULL;
+  for (PictureLayerImpl::LayerEvictionTileIterator it =
+           PictureLayerImpl::LayerEvictionTileIterator(active_layer_,
+                                                       tree_priority);
+       it;
+       ++it) {
+    Tile* tile = *it;
+    if (!last_tile)
+      last_tile = tile;
+
+    if (tile->is_occluded_for_tree_priority(tree_priority)) {
+      occluded_tile_count++;
+      if (!last_tile->is_occluded_for_tree_priority(tree_priority)) {
+        EXPECT_TRUE(
+            (tile->priority_for_tree_priority(tree_priority).priority_bin !=
+             last_tile->priority_for_tree_priority(tree_priority)
+                 .priority_bin) ||
+            (tile->required_for_activation() !=
+             last_tile->required_for_activation()) ||
+            (tile->contents_scale() != last_tile->contents_scale()));
+      }
+    }
+
+    last_tile = tile;
+  }
+  EXPECT_EQ(expected_occluded_on_pending_sum, occluded_tile_count);
+
+  // Iterate over the pending layer considering occlusion on the active tree.
+  tree_priority = SMOOTHNESS_TAKES_PRIORITY;

[vmpstr, 2014/07/11 00:44:32]

Can you compress the test a bit by making a loop:

for (int priority_count = 0; priority_count < 3; ++priority_count) {
  tree_priority = static_cast<TreePriority>(priority_count);
  ...
}

Otherwise, it's hard to tell if the tree priority is the only thing that differs
or there's a subtle difference. Likewise, pending/active layer thing can be a
helper function that takes a layer and does the loop. 

(all this assuming that nothing differs... If something is different that
precludes this, then you should put a comment explaining the differences between
each block)

[jbedley, 2014/07/11 18:00:30]

Done.

+  occluded_tile_count = 0u;
+  last_tile = NULL;
+  for (PictureLayerImpl::LayerEvictionTileIterator it =
+           PictureLayerImpl::LayerEvictionTileIterator(pending_layer_,
+                                                       tree_priority);
+       it;
+       ++it) {
+    Tile* tile = *it;
+    if (!last_tile)
+      last_tile = tile;
+
+    if (tile->is_occluded_for_tree_priority(tree_priority)) {
+      occluded_tile_count++;
+      if (!last_tile->is_occluded_for_tree_priority(tree_priority)) {
+        EXPECT_TRUE(
+            (tile->priority_for_tree_priority(tree_priority).priority_bin !=
+             last_tile->priority_for_tree_priority(tree_priority)
+                 .priority_bin) ||
+            (tile->required_for_activation() !=
+             last_tile->required_for_activation()) ||
+            (tile->contents_scale() != last_tile->contents_scale()));
+      }
+    }
+
+    last_tile = tile;
+  }
+  EXPECT_EQ(expected_occluded_on_active_sum, occluded_tile_count);
+
+  // Iterate over the active layer considering occlusion on the active tree.
+  occluded_tile_count = 0u;
+  last_tile = NULL;
+  for (PictureLayerImpl::LayerEvictionTileIterator it =
+           PictureLayerImpl::LayerEvictionTileIterator(active_layer_,
+                                                       tree_priority);
+       it;
+       ++it) {
+    Tile* tile = *it;
+    if (!last_tile)
+      last_tile = tile;
+
+    if (tile->is_occluded_for_tree_priority(tree_priority)) {
+      occluded_tile_count++;
+      if (!last_tile->is_occluded_for_tree_priority(tree_priority)) {
+        EXPECT_TRUE(
+            (tile->priority_for_tree_priority(tree_priority).priority_bin !=
+             last_tile->priority_for_tree_priority(tree_priority)
+                 .priority_bin) ||
+            (tile->required_for_activation() !=
+             last_tile->required_for_activation()) ||
+            (tile->contents_scale() != last_tile->contents_scale()));
+      }
+    }
+
+    last_tile = tile;
+  }
+  EXPECT_EQ(expected_occluded_on_active_sum, occluded_tile_count);
+
+  // Iterate over the pending layer considering occlusion on both trees.
+  tree_priority = SAME_PRIORITY_FOR_BOTH_TREES;
+  occluded_tile_count = 0u;
+  last_tile = NULL;
+  for (PictureLayerImpl::LayerEvictionTileIterator it =
+           PictureLayerImpl::LayerEvictionTileIterator(pending_layer_,
+                                                       tree_priority);
+       it;
+       ++it) {
+    Tile* tile = *it;
+    if (!last_tile)
+      last_tile = tile;
+
+    if (tile->is_occluded_for_tree_priority(tree_priority)) {
+      occluded_tile_count++;
+      if (!last_tile->is_occluded_for_tree_priority(tree_priority)) {
+        EXPECT_TRUE(
+            (tile->priority_for_tree_priority(tree_priority).priority_bin !=
+             last_tile->priority_for_tree_priority(tree_priority)
+                 .priority_bin) ||
+            (tile->required_for_activation() !=
+             last_tile->required_for_activation()) ||
+            (tile->contents_scale() != last_tile->contents_scale()));
+      }
+    }
+
+    last_tile = tile;
+  }
+  EXPECT_EQ(expected_occluded_on_both_sum, occluded_tile_count);
+
+  // Iterate over the active layer considering occlusion on both trees.
+  occluded_tile_count = 0u;
+  last_tile = NULL;
+  for (PictureLayerImpl::LayerEvictionTileIterator it =
+           PictureLayerImpl::LayerEvictionTileIterator(active_layer_,
+                                                       tree_priority);
+       it;
+       ++it) {
+    Tile* tile = *it;
+    if (!last_tile)
+      last_tile = tile;
+
+    if (tile->is_occluded_for_tree_priority(tree_priority)) {
+      occluded_tile_count++;
+      if (!last_tile->is_occluded_for_tree_priority(tree_priority)) {
+        EXPECT_TRUE(
+            (tile->priority_for_tree_priority(tree_priority).priority_bin !=
+             last_tile->priority_for_tree_priority(tree_priority)
+                 .priority_bin) ||
+            (tile->required_for_activation() !=
+             last_tile->required_for_activation()) ||
+            (tile->contents_scale() != last_tile->contents_scale()));
+      }
+    }
+
+    last_tile = tile;
+  }
+  EXPECT_EQ(expected_occluded_on_both_sum, occluded_tile_count);
+}
 }  // namespace
 }  // namespace cc