cc: Remove DrawQuad::IterateResoruces

cc/surfaces/surface_aggregator.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/surfaces/surface_aggregator.h"
 
 #include <map>
 
 #include "base/bind.h"
 #include "base/containers/hash_tables.h"
@@ skipping 113 matching lines @@
       provider_->SetChildNeedsSyncPoints(
           child_id, surface->factory()->needs_sync_points());
     }
     surface_id_to_resource_child_id_[surface->surface_id()] = child_id;
     return child_id;
   } else {
     return it->second;
   }
 }
 
-static ResourceId ResourceRemapHelper(
-    const ResourceProvider::ResourceIdMap& child_to_parent_map,
-    ResourceId id) {
-  ResourceProvider::ResourceIdMap::const_iterator it =
-      child_to_parent_map.find(id);
-  DCHECK(it != child_to_parent_map.end());
-
-  DCHECK_EQ(it->first, id);
-  ResourceId remapped_id = it->second;
-  return remapped_id;
-}
-
-static ResourceId ValidateResourceHelper(
-    bool* invalid_frame,
-    const ResourceProvider::ResourceIdMap& child_to_parent_map,
-    ResourceProvider::ResourceIdSet* resources_in_frame,
-    ResourceId id) {
-  ResourceProvider::ResourceIdMap::const_iterator it =
-      child_to_parent_map.find(id);
-  if (it == child_to_parent_map.end()) {
-    *invalid_frame = true;
-    return id;
-  }
-  resources_in_frame->insert(id);
-  return id;
-}
-
 bool SurfaceAggregator::ValidateResources(
     Surface* surface,
     const DelegatedFrameData* frame_data) {
   if (!provider_)  // TODO(jamesr): hack for unit tests that don't set up rp
     return false;
 
   int child_id = ChildIdForSurface(surface);
   if (surface->factory())
     surface->factory()->RefResources(frame_data->resource_list);
   provider_->ReceiveFromChild(child_id, frame_data->resource_list);
 
   ResourceProvider::ResourceIdSet referenced_resources;
   size_t reserve_size = frame_data->resource_list.size();
 #if defined(COMPILER_MSVC)
   referenced_resources.reserve(reserve_size);
 #elif defined(COMPILER_GCC)
   // Pre-standard hash-tables only implement resize, which behaves similarly
   // to reserve for these keys. Resizing to 0 may also be broken (particularly
   // on stlport).
   // TODO(jbauman): Replace with reserve when C++11 is supported everywhere.
   if (reserve_size)
     referenced_resources.resize(reserve_size);
 #endif
 
   bool invalid_frame = false;
-  DrawQuad::ResourceIteratorCallback remap =
-      base::Bind(&ValidateResourceHelper, &invalid_frame,
-                 base::ConstRef(provider_->GetChildToParentMap(child_id)),
-                 &referenced_resources);
+  const ResourceProvider::ResourceIdMap& child_to_parent_map =
+      provider_->GetChildToParentMap(child_id);
   for (const auto& render_pass : frame_data->render_pass_list) {
-    for (const auto& quad : render_pass->quad_list)
-      quad->IterateResources(remap);
+    for (const auto& quad : render_pass->quad_list) {
+      for (ResourceId resource_id : quad->resources) {
+        ResourceProvider::ResourceIdMap::const_iterator it =
+            child_to_parent_map.find(resource_id);
+        if (it == child_to_parent_map.end()) {
+          invalid_frame = true;
+          break;
+        }
+        referenced_resources.insert(resource_id);
+      }
+    }
   }
 
   if (!invalid_frame)
     provider_->DeclareUsedResourcesFromChild(child_id, referenced_resources);
 
   return invalid_frame;
 }
 
 gfx::Rect SurfaceAggregator::DamageRectForSurface(const Surface* surface,
                                                   const RenderPass& source,
@@ skipping 36 matching lines @@
   bool invalid_frame = ValidateResources(surface, frame_data);
   if (invalid_frame) {
     for (auto& request : copy_requests) {
       request.second->SendEmptyResult();
       delete request.second;
     }
     return;
   }
 
   SurfaceSet::iterator it = referenced_surfaces_.insert(surface_id).first;
-  DrawQuad::ResourceIteratorCallback remap;
-  if (provider_) {
-    int child_id = ChildIdForSurface(surface);
-    remap =
-        base::Bind(&ResourceRemapHelper,
-                   base::ConstRef(provider_->GetChildToParentMap(child_id)));
-  }
-
+  const ResourceProvider::ResourceIdMap& child_to_parent_map =
+      provider_ ? provider_->GetChildToParentMap(ChildIdForSurface(surface))

[danakj, 2015/05/28 23:00:15]

can you // TODO provider check is a hack for unit tests like we have in
VerifyResources() ?

[vmpstr, 2015/05/28 23:35:52]

Done.

+                : ResourceProvider::ResourceIdMap();
   bool merge_pass = surface_quad->opacity() == 1.f && copy_requests.empty();
 
   gfx::Rect surface_damage = DamageRectForSurface(
       surface, *render_pass_list.back(), surface_quad->visible_rect);
   const RenderPassList& referenced_passes = render_pass_list;
   size_t passes_to_copy =
       merge_pass ? referenced_passes.size() - 1 : referenced_passes.size();
   for (size_t j = 0; j < passes_to_copy; ++j) {
     const RenderPass& source = *referenced_passes[j];
 
@@ skipping 11 matching lines @@
 
     // Contributing passes aggregated in to the pass list need to take the
     // transform of the surface quad into account to update their transform to
     // the root surface.
     copy_pass->transform_to_root_target.ConcatTransform(
         surface_quad->quadTransform());
     copy_pass->transform_to_root_target.ConcatTransform(target_transform);
     copy_pass->transform_to_root_target.ConcatTransform(
         dest_pass->transform_to_root_target);
 
-    CopyQuadsToPass(source.quad_list, source.shared_quad_state_list, remap,
-                    gfx::Transform(), ClipData(), copy_pass.get(), surface_id);
+    CopyQuadsToPass(source.quad_list, source.shared_quad_state_list,
+                    child_to_parent_map, gfx::Transform(), ClipData(),
+                    copy_pass.get(), surface_id);
 
     if (j == referenced_passes.size() - 1)
       surface_damage = gfx::UnionRects(surface_damage, copy_pass->damage_rect);
 
     dest_pass_list_->push_back(copy_pass.Pass());
   }
 
   const RenderPass& last_pass = *render_pass_list.back();
   if (merge_pass) {
     // TODO(jamesr): Clean up last pass special casing.
     const QuadList& quads = last_pass.quad_list;
 
     gfx::Transform surface_transform = surface_quad->quadTransform();
     surface_transform.ConcatTransform(target_transform);
 
     // Intersect the transformed visible rect and the clip rect to create a
     // smaller cliprect for the quad.
     ClipData surface_quad_clip_rect(
         true, MathUtil::MapEnclosingClippedRect(surface_quad->quadTransform(),
                                                 surface_quad->visible_rect));
     if (surface_quad->isClipped())
       surface_quad_clip_rect.rect.Intersect(surface_quad->clipRect());
 
     ClipData quads_clip =
         CalculateClipRect(clip_rect, surface_quad_clip_rect, target_transform);
 
-    CopyQuadsToPass(quads, last_pass.shared_quad_state_list, remap,
-                    surface_transform, quads_clip, dest_pass, surface_id);
+    CopyQuadsToPass(quads, last_pass.shared_quad_state_list,
+                    child_to_parent_map, surface_transform, quads_clip,
+                    dest_pass, surface_id);
   } else {
     RenderPassId remapped_pass_id = RemapPassId(last_pass.id, surface_id);
 
     CopySharedQuadState(surface_quad->shared_quad_state, target_transform,
                         clip_rect, dest_pass);
 
     SharedQuadState* shared_quad_state =
         dest_pass->shared_quad_state_list.back();
     RenderPassDrawQuad* quad =
         dest_pass->CreateAndAppendDrawQuad<RenderPassDrawQuad>();
@@ skipping 36 matching lines @@
   ClipData new_clip_rect = CalculateClipRect(
       clip_rect, ClipData(source_sqs->is_clipped, source_sqs->clip_rect),
       target_transform);
   copy_shared_quad_state->is_clipped = new_clip_rect.is_clipped;
   copy_shared_quad_state->clip_rect = new_clip_rect.rect;
 }
 
 void SurfaceAggregator::CopyQuadsToPass(
     const QuadList& source_quad_list,
     const SharedQuadStateList& source_shared_quad_state_list,
-    const DrawQuad::ResourceIteratorCallback& remap,
+    const ResourceProvider::ResourceIdMap& child_to_parent_map,
     const gfx::Transform& target_transform,
     const ClipData& clip_rect,
     RenderPass* dest_pass,
     SurfaceId surface_id) {
   const SharedQuadState* last_copied_source_shared_quad_state = NULL;
 
   SharedQuadStateList::ConstIterator sqs_iter =
       source_shared_quad_state_list.begin();
   for (const auto& quad : source_quad_list) {
     while (quad->shared_quad_state != *sqs_iter) {
@@ skipping 31 matching lines @@
             pass_quad, dest_pass->shared_quad_state_list.back(),
             remapped_pass_id);
         dest_pass->damage_rect =
             gfx::UnionRects(dest_pass->damage_rect,
                             MathUtil::MapEnclosingClippedRect(
                                 dest_quad->quadTransform(), pass_damage));
       } else {
         dest_quad = dest_pass->CopyFromAndAppendDrawQuad(
             quad, dest_pass->shared_quad_state_list.back());
       }
-      if (!remap.is_null())
-        dest_quad->IterateResources(remap);
+      if (!child_to_parent_map.empty()) {
+        for (ResourceId& resource_id : dest_quad->resources) {
+          ResourceProvider::ResourceIdMap::const_iterator it =
+              child_to_parent_map.find(resource_id);
+          DCHECK(it != child_to_parent_map.end());
+
+          DCHECK_EQ(it->first, resource_id);
+          ResourceId remapped_id = it->second;
+          resource_id = remapped_id;
+        }
+      }
     }
   }
 }
 
 void SurfaceAggregator::CopyPasses(const DelegatedFrameData* frame_data,
                                    Surface* surface) {
   // The root surface is allowed to have copy output requests, so grab them
   // off its render passes.
   std::multimap<RenderPassId, CopyOutputRequest*> copy_requests;
   surface->TakeCopyOutputRequests(&copy_requests);
 
   const RenderPassList& source_pass_list = frame_data->render_pass_list;
   bool invalid_frame = ValidateResources(surface, frame_data);
   DCHECK(!invalid_frame);
   if (invalid_frame)
     return;
 
-  DrawQuad::ResourceIteratorCallback remap;
-  if (provider_) {
-    int child_id = ChildIdForSurface(surface);
-    remap =
-        base::Bind(&ResourceRemapHelper,
-                   base::ConstRef(provider_->GetChildToParentMap(child_id)));
-  }
-
+  const ResourceProvider::ResourceIdMap& child_to_parent_map =
+      provider_ ? provider_->GetChildToParentMap(ChildIdForSurface(surface))

[danakj, 2015/05/28 23:00:15]

todo-o

[vmpstr, 2015/05/28 23:35:52]

Done.

+                : ResourceProvider::ResourceIdMap();

[danakj, 2015/05/28 23:00:15]

I'm kinda surprised this doesn't cause the tests to DCHECK on l407 now. Nothing
would be in the map instead of just not checking anything. Maybe provider_ isn't
null in unit tests anymroe?

[vmpstr, 2015/05/28 23:35:52]

It's because of l403, we only run the loop when the map isn't empty. (which
before was run it when remap callback is not null)

   for (size_t i = 0; i < source_pass_list.size(); ++i) {
     const RenderPass& source = *source_pass_list[i];
 
     size_t sqs_size = source.shared_quad_state_list.size();
     size_t dq_size = source.quad_list.size();
     scoped_ptr<RenderPass> copy_pass(RenderPass::Create(sqs_size, dq_size));
 
     MoveMatchingRequests(source.id, &copy_requests, &copy_pass->copy_requests);
 
     RenderPassId remapped_pass_id =
         RemapPassId(source.id, surface->surface_id());
 
     gfx::Rect damage_rect =
         (i < source_pass_list.size() - 1)
             ? gfx::Rect()
             : DamageRectForSurface(surface, source, source.output_rect);
     copy_pass->SetAll(remapped_pass_id, source.output_rect, damage_rect,
                       source.transform_to_root_target,
                       source.has_transparent_background);
 
-    CopyQuadsToPass(source.quad_list, source.shared_quad_state_list, remap,
-                    gfx::Transform(), ClipData(), copy_pass.get(),
-                    surface->surface_id());
+    CopyQuadsToPass(source.quad_list, source.shared_quad_state_list,
+                    child_to_parent_map, gfx::Transform(), ClipData(),
+                    copy_pass.get(), surface->surface_id());
 
     dest_pass_list_->push_back(copy_pass.Pass());
   }
 }
 
 void SurfaceAggregator::RemoveUnreferencedChildren() {
   for (const auto& surface : previous_contained_surfaces_) {
     if (!contained_surfaces_.count(surface.first)) {
       SurfaceToResourceChildIdMap::iterator it =
           surface_id_to_resource_child_id_.find(surface.first);
@@ skipping 66 matching lines @@
 
 void SurfaceAggregator::SetFullDamageForSurface(SurfaceId surface_id) {
   auto it = previous_contained_surfaces_.find(surface_id);
   if (it == previous_contained_surfaces_.end())
     return;
   // Set the last drawn index as 0 to ensure full damage next time it's drawn.
   it->second = 0;
 }
 
 }  // namespace cc