Don't aggregate quads outside of damage rect when using partial swap.

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 23 matching lines @@
     DCHECK(it->second);
     output_requests->push_back(scoped_ptr<CopyOutputRequest>(it->second));
     it->second = nullptr;
   }
   copy_requests->erase(request_range.first, request_range.second);
 }
 
 }  // namespace
 
 SurfaceAggregator::SurfaceAggregator(SurfaceManager* manager,
-                                     ResourceProvider* provider)
-    : manager_(manager), provider_(provider), next_render_pass_id_(1) {
+                                     ResourceProvider* provider,
+                                     bool aggregate_only_damaged)
+    : manager_(manager),
+      provider_(provider),
+      next_render_pass_id_(1),
+      aggregate_only_damaged_(aggregate_only_damaged) {
   DCHECK(manager_);
 }
 
 SurfaceAggregator::~SurfaceAggregator() {}
 
 // Create a clip rect for an aggregated quad from the original clip rect and
 // the clip rect from the surface it's on.
 SurfaceAggregator::ClipData SurfaceAggregator::CalculateClipRect(
     const ClipData& surface_clip,
     const ClipData& quad_clip,
@@ skipping 91 matching lines @@
     const SurfaceDrawQuad* surface_quad,
     const gfx::Transform& target_transform,
     const ClipData& clip_rect,
     RenderPass* dest_pass) {
   SurfaceId surface_id = surface_quad->surface_id;
   // If this surface's id is already in our referenced set then it creates
   // a cycle in the graph and should be dropped.
   if (referenced_surfaces_.count(surface_id))
     return;
   Surface* surface = manager_->GetSurfaceForId(surface_id);
-  if (!surface) {
-    contained_surfaces_[surface_id] = 0;
+  if (!surface)
     return;
-  }
-  contained_surfaces_[surface_id] = surface->frame_index();
   const CompositorFrame* frame = surface->GetEligibleFrame();
   if (!frame)
     return;
   const DelegatedFrameData* frame_data = frame->delegated_frame_data.get();
   if (!frame_data)
     return;
 
   std::multimap<RenderPassId, CopyOutputRequest*> copy_requests;
   surface->TakeCopyOutputRequests(&copy_requests);
 
@@ skipping 72 matching lines @@
 
     ClipData quads_clip =
         CalculateClipRect(clip_rect, surface_quad_clip_rect, target_transform);
 
     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 =
+        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>();
     quad->SetNew(shared_quad_state,
                  surface_quad->rect,
                  surface_quad->visible_rect,
                  remapped_pass_id,
                  0,
                  gfx::Vector2dF(),
                  gfx::Size(),
                  FilterOperations(),
                  gfx::Vector2dF(),
                  FilterOperations());
   }
 
-
   referenced_surfaces_.erase(it);
 }
 
-void SurfaceAggregator::CopySharedQuadState(
+SharedQuadState* SurfaceAggregator::CopySharedQuadState(
     const SharedQuadState* source_sqs,
     const gfx::Transform& target_transform,
     const ClipData& clip_rect,
     RenderPass* dest_render_pass) {
   SharedQuadState* copy_shared_quad_state =
       dest_render_pass->CreateAndAppendSharedQuadState();
   copy_shared_quad_state->CopyFrom(source_sqs);
   // target_transform contains any transformation that may exist
   // between the context that these quads are being copied from (i.e. the
   // surface's draw transform when aggregated from within a surface) to the
   // target space of the pass. This will be identity except when copying the
   // root draw pass from a surface into a pass when the surface draw quad's
   // transform is not identity.
   copy_shared_quad_state->quad_to_target_transform.ConcatTransform(
       target_transform);
 
   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;
+  return copy_shared_quad_state;
+}
+
+static bool CalculateQuadSpaceDamageRect(
+    const gfx::Transform& quad_to_target_transform,
+    const gfx::Transform& target_to_root_transform,
+    const gfx::Rect& root_damage_rect,
+    gfx::Rect* quad_damage_rect) {
+  gfx::Transform quad_to_root_transform(quad_to_target_transform,
+                                        target_to_root_transform);
+  gfx::Transform inverse_transform(gfx::Transform::kSkipInitialization);
+  if (!quad_to_root_transform.GetInverse(&inverse_transform))

[danakj, 2015/07/07 20:53:53]

i don't think compositor is supposed to produce non-invertible transforms in the
frame. so you could just DCHECK the result was true here

+    return false;
+
+  *quad_damage_rect = MathUtil::ProjectEnclosingClippedRect(inverse_transform,
+                                                            root_damage_rect);
+  return true;
 }
 
 void SurfaceAggregator::CopyQuadsToPass(
     const QuadList& source_quad_list,
     const SharedQuadStateList& source_shared_quad_state_list,
     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;
+  const SharedQuadState* dest_shared_quad_state = nullptr;
+  // Damage rect in the quad space of the current shared quad state, if
+  // the transform from that is invertible.
+  // TODO(jbauman): This rect may contain unnecessary area if
+  // transform isn't axis-aligned.
+  gfx::Rect damage_rect_in_quad_space;
+  bool valid_damage_rect = false;
 
   SharedQuadStateList::ConstIterator sqs_iter =
       source_shared_quad_state_list.begin();
   for (const auto& quad : source_quad_list) {
     while (quad->shared_quad_state != *sqs_iter) {
       ++sqs_iter;
       DCHECK(sqs_iter != source_shared_quad_state_list.end());
     }
     DCHECK_EQ(quad->shared_quad_state, *sqs_iter);
 
     if (quad->material == DrawQuad::SURFACE_CONTENT) {
       const SurfaceDrawQuad* surface_quad = SurfaceDrawQuad::MaterialCast(quad);
+
+      if (aggregate_only_damaged_ && !has_copy_requests_) {

[danakj, 2015/07/07 20:53:53]

can you comment why you're checking has copy requests?

+        gfx::Transform quad_to_target_transform(
+            quad->shared_quad_state->quad_to_target_transform,
+            target_transform);
+        valid_damage_rect = CalculateQuadSpaceDamageRect(
+            quad_to_target_transform, dest_pass->transform_to_root_target,
+            root_damage_rect_, &damage_rect_in_quad_space);

[danakj, 2015/07/07 20:53:53]

you're overwriting the damage_rect_in_quad_space, but the
last_copied_source_shared_quad_state isn't changed, this looks like it would
cause us to use the wrong damage_rect_in_quad_space after a surface quad?

+        if (valid_damage_rect &&
+            !damage_rect_in_quad_space.Intersects(quad->visible_rect))
+          continue;
+      }
+
       HandleSurfaceQuad(surface_quad, target_transform, clip_rect, dest_pass);
     } else {
       if (quad->shared_quad_state != last_copied_source_shared_quad_state) {
-        CopySharedQuadState(quad->shared_quad_state, target_transform,
-                            clip_rect, dest_pass);
+        dest_shared_quad_state = CopySharedQuadState(
+            quad->shared_quad_state, target_transform, clip_rect, dest_pass);
         last_copied_source_shared_quad_state = quad->shared_quad_state;
+        if (aggregate_only_damaged_ && !has_copy_requests_) {
+          valid_damage_rect = CalculateQuadSpaceDamageRect(
+              dest_shared_quad_state->quad_to_target_transform,
+              dest_pass->transform_to_root_target, root_damage_rect_,
+              &damage_rect_in_quad_space);
+        }
       }
+
+      if (aggregate_only_damaged_ && !has_copy_requests_) {
+        if (valid_damage_rect &&
+            !damage_rect_in_quad_space.Intersects(quad->visible_rect))
+          continue;
+      }
+
       DrawQuad* dest_quad;
       if (quad->material == DrawQuad::RENDER_PASS) {
         const RenderPassDrawQuad* pass_quad =
             RenderPassDrawQuad::MaterialCast(quad);
         RenderPassId original_pass_id = pass_quad->render_pass_id;
         RenderPassId remapped_pass_id =
             RemapPassId(original_pass_id, surface_id);
 
         dest_quad = dest_pass->CopyFromAndAppendRenderPassDrawQuad(
-            pass_quad, dest_pass->shared_quad_state_list.back(),
-            remapped_pass_id);
+            pass_quad, dest_shared_quad_state, remapped_pass_id);
       } else {
-        dest_quad = dest_pass->CopyFromAndAppendDrawQuad(
-            quad, dest_pass->shared_quad_state_list.back());
+        dest_quad =
+            dest_pass->CopyFromAndAppendDrawQuad(quad, dest_shared_quad_state);
       }
       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;
@@ skipping 56 matching lines @@
       }
 
       Surface* surface_ptr = manager_->GetSurfaceForId(surface.first);
       if (surface_ptr)
         surface_ptr->RunDrawCallbacks(SurfaceDrawStatus::DRAW_SKIPPED);
     }
   }
 }
 
 // Validate the resources of the current surface and its descendants, and
 // calculate their combined damage rect.

[danakj, 2015/07/07 20:53:52]

and determine if they have copy requests?

at some point this function needs a more general name :)

 gfx::Rect SurfaceAggregator::ValidateAndCalculateDamageRect(
     SurfaceId surface_id) {
   if (referenced_surfaces_.count(surface_id))
     return gfx::Rect();
   Surface* surface = manager_->GetSurfaceForId(surface_id);
-  if (!surface)
+  if (!surface) {
+    contained_surfaces_[surface_id] = 0;
     return gfx::Rect();
+  }
+  contained_surfaces_[surface_id] = surface->frame_index();
   const CompositorFrame* surface_frame = surface->GetEligibleFrame();
   if (!surface_frame)
     return gfx::Rect();
   const DelegatedFrameData* frame_data =
       surface_frame->delegated_frame_data.get();
   if (!frame_data)
     return gfx::Rect();
   int child_id = 0;
   // TODO(jbauman): hack for unit tests that don't set up rp
   if (provider_) {
@@ skipping 48 matching lines @@
     }
   }
 
   if (invalid_frame)
     return gfx::Rect();
   valid_surfaces_.insert(surface->surface_id());
 
   if (provider_)
     provider_->DeclareUsedResourcesFromChild(child_id, referenced_resources);
 
+  for (const auto& render_pass : frame_data->render_pass_list)
+    has_copy_requests_ |= !render_pass->copy_requests.empty();
+
   gfx::Rect damage_rect;
   if (!frame_data->render_pass_list.empty()) {
+    RenderPass* last_pass = frame_data->render_pass_list.back();
     damage_rect =
-        DamageRectForSurface(surface, *frame_data->render_pass_list.back(),
-                             frame_data->render_pass_list.back()->output_rect);
+        DamageRectForSurface(surface, *last_pass, last_pass->output_rect);
   }
 
   // Avoid infinite recursion by adding current surface to
   // referenced_surfaces_.
   SurfaceSet::iterator it =
       referenced_surfaces_.insert(surface->surface_id()).first;
   for (const auto& surface_info : child_surfaces) {
     gfx::Rect surface_damage =
         ValidateAndCalculateDamageRect(surface_info.first);
     damage_rect.Union(
@@ skipping 14 matching lines @@
 
   scoped_ptr<CompositorFrame> frame(new CompositorFrame);
   frame->delegated_frame_data = make_scoped_ptr(new DelegatedFrameData);
 
   DCHECK(root_surface_frame->delegated_frame_data);
 
   dest_resource_list_ = &frame->delegated_frame_data->resource_list;
   dest_pass_list_ = &frame->delegated_frame_data->render_pass_list;
 
   valid_surfaces_.clear();
-  gfx::Rect damage_rect = ValidateAndCalculateDamageRect(surface_id);
+  has_copy_requests_ = false;

[danakj, 2015/07/07 20:53:53]

Do these need to be member variables? Can they be kept on the stack instead?
Maybe a struct passed through the recursion?

+  root_damage_rect_ = ValidateAndCalculateDamageRect(surface_id);
 
   SurfaceSet::iterator it = referenced_surfaces_.insert(surface_id).first;
   CopyPasses(root_surface_frame->delegated_frame_data.get(), surface);
   referenced_surfaces_.erase(it);
 
   DCHECK(referenced_surfaces_.empty());
 
   if (dest_pass_list_->empty())
     return nullptr;
-  dest_pass_list_->back()->damage_rect = damage_rect;
+  dest_pass_list_->back()->damage_rect = root_damage_rect_;
 
   dest_pass_list_ = NULL;
   RemoveUnreferencedChildren();
   contained_surfaces_.swap(previous_contained_surfaces_);
   contained_surfaces_.clear();
 
   for (SurfaceIndexMap::iterator it = previous_contained_surfaces_.begin();
        it != previous_contained_surfaces_.end();
        ++it) {
     Surface* surface = manager_->GetSurfaceForId(it->first);
@@ skipping 18 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