Force use of and cache render surface.

components/viz/service/display/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 "components/viz/service/display/surface_aggregator.h"
 
 #include <stddef.h>
 
 #include <map>
 
@@ skipping 247 matching lines @@
     size_t sqs_size = source.shared_quad_state_list.size();
     size_t dq_size = source.quad_list.size();
     std::unique_ptr<cc::RenderPass> copy_pass(
         cc::RenderPass::Create(sqs_size, dq_size));
 
     cc::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.filters,
                       source.background_filters, blending_color_space_,
-                      source.has_transparent_background);
+                      source.has_transparent_background,
+                      source.cache_render_surface,
+                      source.has_damage_from_contributing_content);

[danakj, 2017/07/19 22:52:55]

Are you expecting CompositorFrames from clients to report
has_damage_from_contributing_content on renderpasses, or is it only meant to be
computed here? Which side is responsible for what parts of the computation?

[wutao, 2017/07/21 05:59:24]

There are 3 contributing_contents:
1. contributing render surface,
2. contributing layer,
3. surface qaud.

1 and 2 are handled by damage_tracker. Clients should report
has_damage_from_contributing_content of RenderPasses.

Here HandleSurfaceQuad is to calculate damage from surface qaud.

 
     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);
     copy_pass->transform_to_root_target.ConcatTransform(
         dest_pass->transform_to_root_target);
 
     CopyQuadsToPass(source.quad_list, source.shared_quad_state_list,
                     child_to_parent_map, gfx::Transform(), ClipData(),
                     copy_pass.get(), surface_id);
 
     if (!copy_request_passes_.count(remapped_pass_id) &&

[danakj, 2017/07/19 22:52:55]

Can you help me understand why in CopyQuadsToPass we check if there exists /any/
(copy requests|cached output) to ignore damage, but here we compute damage
anyway unless /this/ renderpass is (being copied|being cached).

[wutao, 2017/07/21 05:59:24]

Here we also ignore damage and do not recalculate damage if there is a copy
request. It is if (!copy_request...)
jbauman@ might be the best person to answer this. IIRC, non-root-renderpass'
damage_rect can be used in partial-swap. However if there is a copy request, the
copy_pass' damage_rect will be the output_rect. As indicated by the comment in
line 425, "If the current frame has copy requests, then aggregate the entire
thing, as otherwise parts of the copy requests may be ignored."

+        !cached_render_surface_passes_.count(remapped_pass_id) &&

[danakj, 2017/07/19 22:52:54]

How is this different than checking source.cache_render_surface ?

[wutao, 2017/07/21 05:59:24]

I think you are right. So I do not need to modify the function "PropogatePass"
to collecting this info. That would remove many changes in this cl.

         !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 =
             cc::MathUtil::ProjectEnclosingClippedRect(inverse_transform,
                                                       root_damage_rect_);
         copy_pass->damage_rect.Intersect(damage_rect_in_render_pass_space);
       }
     }
 
+    id_to_render_pass_map_.insert(
+        std::make_pair(copy_pass->id, copy_pass.get()));
     dest_pass_list_->push_back(std::move(copy_pass));
   }
 
   gfx::Transform surface_transform =
       surface_quad->shared_quad_state->quad_to_target_transform;
   surface_transform.ConcatTransform(target_transform);
 
   const auto& last_pass = *render_pass_list.back();
+  // Check if the surface_quad has damage.

[danakj, 2017/07/19 22:52:54]

Can this comment explain why this damage check is different from the above
computation of the copy_pass->damage_rect?

[wutao, 2017/07/21 05:59:24]

Will change the comment. The damage here is only from surface quads. The
copy_pass->damage_rect has all the damages on the top level surface including
all the damage from damage_tracker.

+  if (dest_pass->cache_render_surface) {
+    dest_pass->has_damage_from_contributing_content |=
+        !DamageRectForSurface(surface, last_pass, last_pass.output_rect)

[danakj, 2017/07/19 22:52:54]

This is different from the use of this function in PrewalkTree, it also looks at
child surfaces. Is that intentional?

Why doesn't this use a damage result computed there so we don't have to do it
again?

[wutao, 2017/07/21 05:59:24]

This will check if all the surface_quads (including child surfaces) has
damage because HandleSurfaceQuad is a recursive call by calling
CopyQuadsToPass in it.

This is different than the damage_rect calculated in PrewalkTree, which also
includes all the damage in the top level surface, containing damage from
damage_tracker.

+             .IsEmpty();
+  }
+
   if (merge_pass) {
     // TODO(jamesr): Clean up last pass special casing.
     const cc::QuadList& quads = last_pass.quad_list;
 
     // Intersect the transformed visible rect and the clip rect to create a
     // smaller cliprect for the quad.
     ClipData surface_quad_clip_rect(
         true, cc::MathUtil::MapEnclosingClippedRect(
                   surface_quad->shared_quad_state->quad_to_target_transform,
                   surface_quad->visible_rect));
@@ skipping 89 matching lines @@
     const cc::QuadList& source_quad_list,
     const cc::SharedQuadStateList& source_shared_quad_state_list,
     const std::unordered_map<cc::ResourceId, cc::ResourceId>&
         child_to_parent_map,
     const gfx::Transform& target_transform,
     const ClipData& clip_rect,
     cc::RenderPass* dest_pass,
     const SurfaceId& surface_id) {
   const cc::SharedQuadState* last_copied_source_shared_quad_state = nullptr;
   const cc::SharedQuadState* dest_shared_quad_state = nullptr;
-  // If the current frame has copy requests then aggregate the entire
-  // thing, as otherwise parts of the copy requests may be ignored.
-  const bool ignore_undamaged = aggregate_only_damaged_ &&
-                                !has_copy_requests_ &&
-                                !moved_pixel_passes_.count(dest_pass->id);
+  // If the current frame has copy requests or cache render surface, then
+  // aggregate the entire thing, as otherwise parts of the copy requests or
+  // render surface may be ignored.

[danakj, 2017/07/19 22:52:55]

I think you mean to say that you want what's in the cache to be the full content
here?

[wutao, 2017/07/21 05:59:24]

Yes. Changed accordingly.

+  const bool ignore_undamaged =
+      aggregate_only_damaged_ && !has_copy_requests_ &&
+      !has_cached_render_surfaces_ && !moved_pixel_passes_.count(dest_pass->id);
   // Damage rect in the quad space of the current shared quad state.
   // TODO(jbauman): This rect may contain unnecessary area if
   // transform isn't axis-aligned.
   gfx::Rect damage_rect_in_quad_space;
   bool damage_rect_in_quad_space_valid = false;
 
 #if DCHECK_IS_ON()
   // If quads have come in with SharedQuadState out of order, or when quads have
   // invalid SharedQuadState pointer, it should DCHECK.
   auto sqs_iter = source_shared_quad_state_list.cbegin();
@@ skipping 20 matching lines @@
         continue;
 
       HandleSurfaceQuad(surface_quad, target_transform, clip_rect, dest_pass,
                         ignore_undamaged, &damage_rect_in_quad_space,
                         &damage_rect_in_quad_space_valid);
     } else {
       if (quad->shared_quad_state != last_copied_source_shared_quad_state) {
         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_) {
+        if (aggregate_only_damaged_ && !has_copy_requests_ &&

[danakj, 2017/07/19 22:52:54]

Can you help me understand why this doesn't compute damage_rect_in_quad_space
when there is any cached renderpass, but we do when there's moved pixel passes,
even tho in both cases we're going to aggregate all the quads anyway?

[wutao, 2017/07/21 05:59:24]

Looks like the final result is same, but we might do extra calculate if there is
moved pixel in the pass in the current code. When there's moved pixel in the
pass, the ignore_undamaged if false, then the damage_rect_in_quad_space_valid is
not used. If there is no moved pixel in the pass, then we can ignore the
out-side quad damage.

+            !has_cached_render_surfaces_) {
           damage_rect_in_quad_space_valid = CalculateQuadSpaceDamageRect(
               dest_shared_quad_state->quad_to_target_transform,
               dest_pass->transform_to_root_target, root_damage_rect_,
               &damage_rect_in_quad_space);
         }
       }
 
       if (ignore_undamaged) {
         if (damage_rect_in_quad_space_valid &&
             !damage_rect_in_quad_space.Intersects(quad->visible_rect))
           continue;
       }
 
       cc::DrawQuad* dest_quad;
       if (quad->material == cc::DrawQuad::RENDER_PASS) {
         const auto* pass_quad = cc::RenderPassDrawQuad::MaterialCast(quad);
         int original_pass_id = pass_quad->render_pass_id;
         int remapped_pass_id = RemapPassId(original_pass_id, surface_id);
-
+        if (dest_pass->cache_render_surface) {
+          auto iter = id_to_render_pass_map_.find(remapped_pass_id);
+          if (iter != id_to_render_pass_map_.end()) {
+            dest_pass->has_damage_from_contributing_content |=
+                iter->second->has_damage_from_contributing_content;

[danakj, 2017/07/19 22:52:54]

Is this just what HandleSurfaceDrawQuad() computed above on L463? Should it just
get that from there? Or is this including other information, from where?

[wutao, 2017/07/21 05:59:24]

They are in different code path. Here is to handle cc::DrawQuad::RENDER_PASS. I
should have already checked this damage either in HandleSurfaceDrawQuad or
render surface in damage_tracker. Removed related code.

+          }
+        }
         dest_quad = dest_pass->CopyFromAndAppendRenderPassDrawQuad(
             pass_quad, dest_shared_quad_state, remapped_pass_id);
       } else if (quad->material == cc::DrawQuad::TEXTURE_CONTENT) {
         const auto* texture_quad = cc::TextureDrawQuad::MaterialCast(quad);
         if (texture_quad->secure_output_only &&
             (!output_is_secure_ || copy_request_passes_.count(dest_pass->id))) {
           auto* solid_color_quad =
               dest_pass->CreateAndAppendDrawQuad<cc::SolidColorDrawQuad>();
           solid_color_quad->SetNew(dest_shared_quad_state, quad->rect,
                                    quad->visible_rect, SK_ColorBLACK, false);
@@ skipping 47 matching lines @@
     auto copy_pass = cc::RenderPass::Create(sqs_size, dq_size);
 
     MoveMatchingRequests(source.id, &copy_requests, &copy_pass->copy_requests);
 
     cc::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.filters,
                       source.background_filters, blending_color_space_,
-                      source.has_transparent_background);
+                      source.has_transparent_background,
+                      source.cache_render_surface,
+                      source.has_damage_from_contributing_content);

[danakj, 2017/07/19 22:52:54]

Same questions here as in HandleSurfaceDrawQuad

[wutao, 2017/07/21 05:59:24]

Please see the reply to the other comment.

 
     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) &&
+        !cached_render_surface_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 =
             cc::MathUtil::ProjectEnclosingClippedRect(inverse_transform,
                                                       root_damage_rect_);
         copy_pass->damage_rect.Intersect(damage_rect_in_render_pass_space);
       }
     }
 
+    id_to_render_pass_map_.insert(
+        std::make_pair(copy_pass->id, copy_pass.get()));
     dest_pass_list_->push_back(std::move(copy_pass));
   }

[danakj, 2017/07/19 22:52:54]

It doesn't compute the damage from contributing things here, is that
intentional? Do copied currently-offscreen surfaces not need to compute that?
Why?

I think the root doesn't need to because we can't cache that one, is that right?

[wutao, 2017/07/21 05:59:24]

All damage from contributing contents should be handled:
1. copy_pass->SetAll(...)
2. HandleSurfaceQuad. Are offscreen surfaces handled here?

 }
 
 void SurfaceAggregator::ProcessAddedAndRemovedSurfaces() {
   for (const auto& surface : previous_contained_surfaces_) {
     if (!contained_surfaces_.count(surface.first)) {
       // Release resources of removed surface.
       auto it = surface_id_to_resource_child_id_.find(surface.first);
       if (it != surface_id_to_resource_child_id_.end()) {
         provider_->DestroyChild(it->second);
         surface_id_to_resource_child_id_.erase(it);
@@ skipping 181 matching lines @@
   CHECK(debug_weak_this.get());
   // TODO(staraz): It shouldn't need to call the callback when the damage is
   // from |surface| and not from |child_surfaces|.
   if (!damage_rect.IsEmpty()) {
     surface->RunWillDrawCallback(damage_rect);
     manager_->SurfaceWillDraw(surface->surface_id());
   }
 
   CHECK(debug_weak_this.get());
   for (const auto& render_pass : frame.render_pass_list) {
-    if (!render_pass->copy_requests.empty()) {
-      cc::RenderPassId remapped_pass_id =
-          RemapPassId(render_pass->id, surface_id);
+    cc::RenderPassId remapped_pass_id =
+        RemapPassId(render_pass->id, surface_id);
+    if (!render_pass->copy_requests.empty())
       copy_request_passes_.insert(remapped_pass_id);
-    }
+    if (render_pass->cache_render_surface)
+      cached_render_surface_passes_.insert(remapped_pass_id);
   }
 
   referenced_surfaces_.erase(referenced_surfaces_.find(surface->surface_id()));
   if (!damage_rect.IsEmpty() && frame.metadata.may_contain_video)
     result->may_contain_video = true;
   return damage_rect;
 }
 
 void SurfaceAggregator::CopyUndrawnSurfaces(PrewalkResult* prewalk_result) {
   // undrawn_surfaces are Surfaces that were identified by prewalk as being
@@ skipping 32 matching lines @@
       }
     } else {
       referenced_surfaces_.insert(surface_id);
       CopyPasses(frame, surface);
       // CopyPasses may have mutated container, need to re-query to erase.
       referenced_surfaces_.erase(referenced_surfaces_.find(surface_id));
     }
   }
 }
 
-void SurfaceAggregator::PropagateCopyRequestPasses() {
-  std::vector<cc::RenderPassId> copy_requests_to_iterate(
-      copy_request_passes_.begin(), copy_request_passes_.end());
-  while (!copy_requests_to_iterate.empty()) {
-    int first = copy_requests_to_iterate.back();
-    copy_requests_to_iterate.pop_back();
+void SurfaceAggregator::PropagatePasses(
+    base::flat_set<cc::RenderPassId>* passes) {
+  std::vector<cc::RenderPassId> passes_to_iterate(passes->begin(),
+                                                  passes->end());
+  while (!passes_to_iterate.empty()) {
+    cc::RenderPassId first = passes_to_iterate.back();
+    passes_to_iterate.pop_back();
     auto it = render_pass_dependencies_.find(first);
     if (it == render_pass_dependencies_.end())
       continue;
     for (auto pass : it->second) {
-      if (copy_request_passes_.insert(pass).second) {
-        copy_requests_to_iterate.push_back(pass);
+      if (passes->insert(pass).second) {
+        passes_to_iterate.push_back(pass);
       }
     }
   }
 }
 
 cc::CompositorFrame SurfaceAggregator::Aggregate(const SurfaceId& surface_id) {
   uma_stats_.Reset();
 
   cc::Surface* surface = manager_->GetSurfaceForId(surface_id);
   DCHECK(surface);
   contained_surfaces_[surface_id] = surface->frame_index();
 
   if (!surface->HasActiveFrame())
     return {};
 
   const cc::CompositorFrame& root_surface_frame = surface->GetActiveFrame();
   TRACE_EVENT0("viz", "SurfaceAggregator::Aggregate");
 
   cc::CompositorFrame frame;
 
   dest_pass_list_ = &frame.render_pass_list;
 
   valid_surfaces_.clear();
   PrewalkResult prewalk_result;
   root_damage_rect_ = PrewalkTree(surface_id, false, 0, &prewalk_result);
-  PropagateCopyRequestPasses();
+  PropagatePasses(&copy_request_passes_);

[danakj, 2017/07/19 22:52:55]

granted the name comes from what was there before but what is this propogating?
I don't think it describes what it's doing at all.. it's more like collecting or
something. Can you improve this?

[wutao, 2017/07/21 05:59:24]

1. I do not need to modify this function.
2. I think the name could be fine. It is propagating all the dependent passes to
the /copy_request_passes_/.

+  PropagatePasses(&cached_render_surface_passes_);
   has_copy_requests_ = !copy_request_passes_.empty();
+  has_cached_render_surfaces_ = !cached_render_surface_passes_.empty();
   frame.metadata.may_contain_video = prewalk_result.may_contain_video;
 
   CopyUndrawnSurfaces(&prewalk_result);
   referenced_surfaces_.insert(surface_id);
   CopyPasses(root_surface_frame, surface);
   // CopyPasses may have mutated container, need to re-query to erase.
   referenced_surfaces_.erase(referenced_surfaces_.find(surface_id));
   AddColorConversionPass();
 
   moved_pixel_passes_.clear();
   copy_request_passes_.clear();
+  cached_render_surface_passes_.clear();
   render_pass_dependencies_.clear();
 
   // Remove all render pass mappings that weren't used in the current frame.
   for (auto it = render_pass_allocator_map_.begin();
        it != render_pass_allocator_map_.end();) {
     if (it->second.in_use) {
       it->second.in_use = false;
       it++;
     } else {
       it = render_pass_allocator_map_.erase(it);
     }
   }
 
   DCHECK(referenced_surfaces_.empty());
 
   if (dest_pass_list_->empty())
     return {};
 
   dest_pass_list_ = NULL;
+  id_to_render_pass_map_.clear();
   ProcessAddedAndRemovedSurfaces();
   contained_surfaces_.swap(previous_contained_surfaces_);
   contained_surfaces_.clear();
 
   for (auto it : previous_contained_surfaces_) {
     cc::Surface* surface = manager_->GetSurfaceForId(it.first);
     if (surface)
       surface->TakeLatencyInfo(&frame.metadata.latency_info);
   }
 
@@ skipping 33 matching lines @@
     const gfx::ColorSpace& output_color_space) {
   blending_color_space_ = blending_color_space.IsValid()
                               ? blending_color_space
                               : gfx::ColorSpace::CreateSRGB();
   output_color_space_ = output_color_space.IsValid()
                             ? output_color_space
                             : gfx::ColorSpace::CreateSRGB();
 }
 
 }  // namespace viz