cc: Move filters from RenderPassDrawQuad to RenderPass

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 <stddef.h>
 
 #include <map>
 
@@ skipping 207 matching lines @@
     const RenderPass& source = *referenced_passes[j];
 
     size_t sqs_size = source.shared_quad_state_list.size();
     size_t dq_size = source.quad_list.size();
     std::unique_ptr<RenderPass> copy_pass(
         RenderPass::Create(sqs_size, dq_size));
 
     RenderPassId remapped_pass_id = RemapPassId(source.id, surface_id);
 
     copy_pass->SetAll(remapped_pass_id, source.output_rect, source.output_rect,
-                      source.transform_to_root_target,
+                      source.transform_to_root_target, source.filters,
+                      source.background_filters,
                       source.has_transparent_background);
 
     MoveMatchingRequests(source.id, &copy_requests, &copy_pass->copy_requests);
 
     // 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->shared_quad_state->quad_to_target_transform);
     copy_pass->transform_to_root_target.ConcatTransform(target_transform);
@@ skipping 48 matching lines @@
     RenderPassId remapped_pass_id = RemapPassId(last_pass.id, surface_id);
 
     SharedQuadState* shared_quad_state =
         CopySharedQuadState(surface_quad->shared_quad_state, target_transform,
                             clip_rect, dest_pass);
 
     RenderPassDrawQuad* quad =
         dest_pass->CreateAndAppendDrawQuad<RenderPassDrawQuad>();
     quad->SetNew(shared_quad_state, surface_quad->rect,
                  surface_quad->visible_rect, remapped_pass_id, 0,
-                 gfx::Vector2dF(), gfx::Size(), FilterOperations(),
-                 gfx::Vector2dF(), gfx::PointF(), FilterOperations());
+                 gfx::Vector2dF(), gfx::Size(), gfx::Vector2dF(),
+                 gfx::PointF());
   }
 
   referenced_surfaces_.erase(it);
 }
 
 SharedQuadState* SurfaceAggregator::CopySharedQuadState(
     const SharedQuadState* source_sqs,
     const gfx::Transform& target_transform,
     const ClipData& clip_rect,
     RenderPass* dest_render_pass) {
@@ skipping 176 matching lines @@
     size_t dq_size = source.quad_list.size();
     std::unique_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());
 
     copy_pass->SetAll(remapped_pass_id, source.output_rect, source.output_rect,
-                      source.transform_to_root_target,
+                      source.transform_to_root_target, source.filters,
+                      source.background_filters,
                       source.has_transparent_background);
 
     CopyQuadsToPass(source.quad_list, source.shared_quad_state_list,
                     child_to_parent_map, gfx::Transform(), ClipData(),
                     copy_pass.get(), surface->surface_id());
     if (!copy_request_passes_.count(remapped_pass_id) &&
         !moved_pixel_passes_.count(remapped_pass_id)) {
       gfx::Transform inverse_transform(gfx::Transform::kSkipInitialization);
       if (copy_pass->transform_to_root_target.GetInverse(&inverse_transform)) {
         gfx::Rect damage_rect_in_render_pass_space =
@@ skipping 80 matching lines @@
   }
 
   struct SurfaceInfo {
     SurfaceId id;
     bool has_moved_pixels;
     RenderPassId parent_pass;
     gfx::Transform target_to_surface_transform;
   };
   std::vector<SurfaceInfo> child_surfaces;
 
+  std::unordered_set<RenderPassId, RenderPassIdHash> pixel_moving_filter_passes;
+  std::unordered_set<RenderPassId, RenderPassIdHash>
+      pixel_moving_background_filter_passes;
+  for (const auto& render_pass : frame.render_pass_list) {
+    if (render_pass->filters.HasFilterThatMovesPixels()) {
+      pixel_moving_filter_passes.insert(
+          RemapPassId(render_pass->id, surface_id));

[jbauman, 2016/12/07 01:15:47]

I don't think you need this - I think you can just check do 
"bool in_moved_pixel_pass = !!moved_pixel_passes_.count(remapped_pass_id) ||
render_pass->filters.HasFilterThatMovesPixels();" below.

[ajuma, 2016/12/13 15:08:11]

Done.

+    }
+    if (render_pass->background_filters.HasFilterThatMovesPixels()) {
+      pixel_moving_background_filter_passes.insert(
+          RemapPassId(render_pass->id, surface_id));
+    }
+  }
+
   for (const auto& render_pass : base::Reversed(frame.render_pass_list)) {
     RenderPassId remapped_pass_id = RemapPassId(render_pass->id, surface_id);
     bool in_moved_pixel_pass = !!moved_pixel_passes_.count(remapped_pass_id);

[danakj, 2016/12/01 01:50:46]

This variable shadows the outer one with same name which is very confusing..

[ajuma, 2016/12/13 15:08:11]

Renamed the outer one to |in_moved_pixel_surface|.

     for (auto* quad : render_pass->quad_list) {
       if (quad->material == DrawQuad::SURFACE_CONTENT) {
         const SurfaceDrawQuad* surface_quad =
             SurfaceDrawQuad::MaterialCast(quad);
         gfx::Transform target_to_surface_transform(
             render_pass->transform_to_root_target,
             surface_quad->shared_quad_state->quad_to_target_transform);
         child_surfaces.push_back(
             SurfaceInfo{surface_quad->surface_id, in_moved_pixel_pass,

[danakj, 2016/12/01 01:50:46]

aside.. this should use () not {} for the constructor
https://www.chromium.org/developers/coding-style/cpp-dos-and-donts?pli=1#TOC-...
tho emplace_back might be better

[ajuma, 2016/12/13 15:08:11]

Changed to using emplace_back.

[danakj, 2016/12/14 23:23:57]

So uh, I guess that was ok cuz it was struct init not a constructor.. but this
is much more clear so thanks.

                         remapped_pass_id, target_to_surface_transform});
       } else if (quad->material == DrawQuad::RENDER_PASS) {
         const RenderPassDrawQuad* render_pass_quad =
             RenderPassDrawQuad::MaterialCast(quad);
         if (in_moved_pixel_pass ||
-            render_pass_quad->filters.HasFilterThatMovesPixels()) {
+            pixel_moving_filter_passes.count(

[danakj, 2016/12/01 01:50:46]

I think this bit doesn't need to be in per-quad code anymore. This is making us
add to moved_pixel_passes_ any RenderPass with a moving-pixel filter.

We can just do that at the top of the render_pass loop if it has a moving-pixel
filter by checking render_pass.filters. And then we know we're inside a
moved_pixel_pass already for the code inside the loop.

The shadowing in_moved_pixel_pass makes it so that any RenderPass or Surface
appearing after (ie below) a background filtered RenderPass will be added to
moved_pixel_passes_, that still needs to be per-quad, and I agree we need to
prewalk the passes to find them since we go in the opposite order here.
However.. is there a reason to go in reversed order here anymore? I'm not seeing
it. If not then we don't need to prewalk we can just add a render_pass as we go
to the "has background filters" set.

[ajuma, 2016/12/13 15:08:11]

Done.
We need to go in reversed order since for any pass that has a (foreground)
filter, we want contributing passes to also go into moved_pixel_passes_. So we
need to visit passes before visiting their contributing passes.

Expanded one of the surface aggregator unit tests for this.

+                render_pass_quad->render_pass_id)) {
           moved_pixel_passes_.insert(
               RemapPassId(render_pass_quad->render_pass_id, surface_id));
         }
-        if (render_pass_quad->background_filters.HasFilterThatMovesPixels()) {
+        if (pixel_moving_background_filter_passes.count(
+                render_pass_quad->render_pass_id)) {
           in_moved_pixel_pass = true;
         }
         render_pass_dependencies_[remapped_pass_id].insert(
             RemapPassId(render_pass_quad->render_pass_id, surface_id));
       }
 
       if (!provider_)
         continue;
       for (ResourceId resource_id : quad->resources) {
         if (!child_to_parent_map.count(resource_id)) {
@@ skipping 199 matching lines @@
 
 void SurfaceAggregator::SetFullDamageForSurface(const 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