cc: Move render surface ownership from layers to the effect tree

cc/trees/property_tree.cc

Index: cc/trees/property_tree.cc
diff --git a/cc/trees/property_tree.cc b/cc/trees/property_tree.cc
index 9850577ff4ed778e9f6fd3ce8eb87e7cd57ef092..b3454943e5d96c165b575bb040160498165fa711 100644
--- a/cc/trees/property_tree.cc
+++ b/cc/trees/property_tree.cc
@@ -749,13 +749,25 @@ StickyPositionNodeData* TransformTree::StickyPositionData(int node_id) {
   return &sticky_position_data_[node->sticky_position_constraint_id];
 }
 
-EffectTree::EffectTree() {}
+EffectTree::EffectTree() : needs_update_render_surfaces_(false) {
+  render_surfaces_.push_back(nullptr);
+}
 
 EffectTree::~EffectTree() {}
 
+int EffectTree::Insert(const EffectNode& tree_node, int parent_id) {
+  int node_id = PropertyTree<EffectNode>::Insert(tree_node, parent_id);
+  DCHECK_EQ(node_id, static_cast<int>(render_surfaces_.size()));
+
+  render_surfaces_.push_back(nullptr);
+  return node_id;
+}
+
 void EffectTree::clear() {
   PropertyTree<EffectNode>::clear();
   mask_layer_ids_.clear();
+  render_surfaces_.clear();
+  render_surfaces_.push_back(nullptr);
 
 #if DCHECK_IS_ON()
   EffectTree tree;
@@ -977,6 +989,27 @@ void EffectTree::AddMaskLayerId(int id) {
   mask_layer_ids_.push_back(id);
 }
 
+void EffectTree::UpdateRenderSurfaces(LayerTreeImpl* layer_tree_impl,

[jaydasika, 2017/02/22 01:29:50]

I am confused. Why do we need UpdateRenderSurfaces ? Since, they are on effect
tree and not on layers, don't they get copied(updated) when property trees are
copied ?

[ajuma, 2017/02/22 22:07:00]

Good catch. That was happening for a couple reasons: 1) when property trees are
copied, no-longer-needed surfaces were getting destroyed but new ones weren't
getting created;  2) if non_root_surfaces_enabled changes, we need to
create/destroy surfaces. I've fixed (1), which meant moving calls to
create/reuse render surfaces out of EffectTree::operator= and into
LayerTreeImpl::SetPropertyTrees (since we need a pointer to LayerTreeImpl in
order to create render surfaces). So now UpdateRenderSurfaces only gets called
if non_root_surfaces_enabled changes (it doesn't change anymore on ToT, so this
can be deleted as part of https://codereview.chromium.org/2693703010/)

+                                      bool non_root_surfaces_enabled) {
+  for (int id = kContentsRootNodeId; id < static_cast<int>(size()); ++id) {
+    EffectNode* effect_node = Node(id);
+    bool needs_render_surface =
+        id == kContentsRootNodeId ||
+        (non_root_surfaces_enabled && effect_node->has_render_surface);
+    if (needs_render_surface == !!render_surfaces_[id])
+      continue;
+
+    if (needs_render_surface) {
+      render_surfaces_[id] = base::MakeUnique<RenderSurfaceImpl>(
+          layer_tree_impl, effect_node->owning_layer_id);
+      render_surfaces_[id]->set_effect_tree_index(id);
+    } else {
+      render_surfaces_[id].reset();
+    }
+  }
+  needs_update_render_surfaces_ = false;
+}
+
 bool EffectTree::ContributesToDrawnSurface(int id) {
   // All drawn nodes contribute to drawn surface.
   // Exception : Nodes that are hidden and are drawn only for the sake of
@@ -989,34 +1022,28 @@ bool EffectTree::ContributesToDrawnSurface(int id) {
 void EffectTree::ResetChangeTracking() {
   for (int id = EffectTree::kContentsRootNodeId; id < static_cast<int>(size());
        ++id) {
-    EffectNode* node = Node(id);
-    node->effect_changed = false;
+    Node(id)->effect_changed = false;
+    if (render_surfaces_[id])
+      render_surfaces_[id]->ResetPropertyChangedFlags();
   }
 }
 
-EffectTree::StableIdRenderSurfaceList
-EffectTree::CreateStableIdRenderSurfaceList() const {
-  StableIdRenderSurfaceList stable_id_render_surface_list;
+void EffectTree::TakeRenderSurfaces(
+    std::vector<std::unique_ptr<RenderSurfaceImpl>>* render_surfaces) {
   for (int id = kContentsRootNodeId; id < static_cast<int>(size()); ++id) {
-    const EffectNode* node = Node(id);
-    if (node->render_surface) {
-      stable_id_render_surface_list.push_back(
-          std::make_pair(node->owning_layer_id, node->render_surface));
+    if (render_surfaces_[id]) {
+      render_surfaces->push_back(std::move(render_surfaces_[id]));
     }
   }
-  std::sort(stable_id_render_surface_list.begin(),
-            stable_id_render_surface_list.end());
-  return stable_id_render_surface_list;
 }
 
-void EffectTree::UpdateRenderSurfaceEffectIds(
-    const EffectTree::StableIdRenderSurfaceList& stable_id_render_surface_list,
-    LayerTreeImpl* layer_tree_impl) {
+void EffectTree::ReuseRenderSurfaces(
+    std::vector<std::unique_ptr<RenderSurfaceImpl>>* render_surfaces) {
   // Make a list of {stable id, node id} pairs for nodes that are supposed to
   // have surfaces.
   std::vector<std::pair<int, int>> stable_id_node_id_list;
   for (int id = kContentsRootNodeId; id < static_cast<int>(size()); ++id) {
-    const EffectNode* node = Node(id);
+    EffectNode* node = Node(id);
     if (node->has_render_surface) {
       stable_id_node_id_list.push_back(
           std::make_pair(node->owning_layer_id, node->id));
@@ -1025,45 +1052,36 @@ void EffectTree::UpdateRenderSurfaceEffectIds(
 
   // Sort by stable id so that we can process the two lists cosequentially.
   std::sort(stable_id_node_id_list.begin(), stable_id_node_id_list.end());
-
-  auto surface_list_it = stable_id_render_surface_list.begin();
-  auto node_id_list_it = stable_id_node_id_list.begin();
-  while (surface_list_it != stable_id_render_surface_list.end() &&
-         node_id_list_it != stable_id_node_id_list.end()) {
-    if (surface_list_it->first == node_id_list_it->first) {
-      RenderSurfaceImpl* surface = surface_list_it->second;
-      int node_id = node_id_list_it->second;
-      Node(node_id)->render_surface = surface;
-      surface->set_effect_tree_index(node_id);
-      surface_list_it++;
-      node_id_list_it++;
+  std::sort(render_surfaces->begin(), render_surfaces->end(),
+            [](const std::unique_ptr<RenderSurfaceImpl>& a,
+               const std::unique_ptr<RenderSurfaceImpl>& b) {
+              return a->id() < b->id();
+            });
+
+  auto surfaces_list_it = render_surfaces->begin();
+  auto id_list_it = stable_id_node_id_list.begin();
+  while (surfaces_list_it != render_surfaces->end() &&
+         id_list_it != stable_id_node_id_list.end()) {
+    if ((*surfaces_list_it)->id() == id_list_it->first) {
+      int new_node_id = id_list_it->second;
+      render_surfaces_[new_node_id] = std::move(*surfaces_list_it);
+      render_surfaces_[new_node_id]->set_effect_tree_index(new_node_id);
+      surfaces_list_it++;
+      id_list_it++;
       continue;
     }
 
-    if (surface_list_it->first > node_id_list_it->first) {
-      node_id_list_it++;
-      continue;
-    }
+    needs_update_render_surfaces_ = true;
 
-    // If we reach here, there's no longer an effect node with stable id
-    // |surface_list_it->first| that has a render surface. If there's no longer
-    // any corresponding layer either, there's nothing more to do since the
-    // surface owned by that layer would have been destroyed when the layer was
-    // destroyed. But if the layer still exists, we need to destroy the surface
-    // since it now has an invalid effect node id.
-    if (LayerImpl* layer_impl =
-            layer_tree_impl->LayerById(surface_list_it->first)) {
-      layer_impl->SetHasRenderSurface(false);
-    }
-    surface_list_it++;
+    if ((*surfaces_list_it)->id() > id_list_it->first)
+      id_list_it++;
+    else
+      surfaces_list_it++;
   }
 
-  while (surface_list_it != stable_id_render_surface_list.end()) {
-    if (LayerImpl* layer_impl =
-            layer_tree_impl->LayerById(surface_list_it->first)) {
-      layer_impl->SetHasRenderSurface(false);
-    }
-    surface_list_it++;
+  if (surfaces_list_it != render_surfaces->end() ||
+      id_list_it != stable_id_node_id_list.end()) {
+    needs_update_render_surfaces_ = true;
   }
 }
 
@@ -1097,10 +1115,17 @@ bool ClipTree::operator==(const ClipTree& other) const {
 }
 
 EffectTree& EffectTree::operator=(const EffectTree& from) {
+  // Keep existing render surfaces that still have a corresponding node in the
+  // new tree, and delete the rest.
+  std::vector<std::unique_ptr<RenderSurfaceImpl>> old_render_surfaces;
+  TakeRenderSurfaces(&old_render_surfaces);
   PropertyTree::operator=(from);
+  render_surfaces_.resize(size());
+  ReuseRenderSurfaces(&old_render_surfaces);
   mask_layer_ids_ = from.mask_layer_ids_;
   // copy_requests_ are omitted here, since these need to be moved rather
   // than copied or assigned.
+
   return *this;
 }