Remove caching avoidance when there is a non-empty scissor.

See https://chromium-review.googlesource.com/c/chromium/src/+/760558#message-83efb689457770dfabb6d7371847f06f8091dfd7
and https://bugs.chromium.org/p/chromium/issues/detail?id=783087#c24
for explanations, but tl;dr this check would break caching, and
shouldn't be there. Before it was used to skip render passes
regardless of caching, but incorrectly, and the unittest in
https://chromium-review.googlesource.com/c/chromium/src/+/760558
demonstrates that, and fails with that check.

This removes the check from the caching block.

R=enne@chromium.org, wutao@chromium.org

Bug: 782044, 738190, 782042, 783087
Change-Id: Idc5baea6da5e6d663da9c1aa23ce81a55e61168d
Reviewed-on: https://chromium-review.googlesource.com/763710
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515757}

components/viz/service/display/direct_renderer.cc

@@ -606,31 +606,29 @@ bool DirectRenderer::CanSkipRenderPass(const RenderPass* render_pass) const {
   if (render_pass == current_frame()->root_render_pass)
     return false;
 
+  // TODO(crbug.com/783275): It's possible to skip a child RenderPass if damage
+  // does not overlap it, since that means nothing has changed:
+  //   ComputeScissorRectForRenderPass(render_pass).IsEmpty()
+  // However that caused crashes where the RenderPass' texture was not present
+  // (never seen the RenderPass before, or the texture was deleted when not used
+  // for a frame). It could avoid skipping if there is no texture present, which
+  // is what was done for a while, but this seems to papering over a missing
+  // damage problem, or we're failing to understand the system wholey.
+  // If attempted again this should probably CHECK() that the texture exists,
+  // and attempt to figure out where the new RenderPass texture without damage
+  // is coming from.
+
   // If the RenderPass wants to be cached, then we only draw it if we need to.
   // When damage is present, then we can't skip the RenderPass. Or if the
   // texture does not exist (first frame, or was deleted) then we can't skip
-  // the RenderPass, but that is checked above already.
+  // the RenderPass.
   if (render_pass->cache_render_pass) {
     if (render_pass->has_damage_from_contributing_content)
       return false;
     auto it = render_pass_textures_.find(render_pass->id);
     DCHECK(it != render_pass_textures_.end());
     DCHECK(it->second);
-    if (it->second->id() == 0)
-      return false;
-    // Normally the scissor rect shouldn't affect caching, as that is global
-    // damage and not local to the RenderPass. It can be part of the scissor
-    // without the contents of the RenderPass being dirty - which is why there
-    // is a check for |has_damage_from_contributing_content| above. However the
-    // damage in the RenderPass can be modified by SurfaceAggregator, so we
-    // check to verify if that's the case.
-    // TODO(danakj) This check was done to be sure to have the needed content
-    // available when partial swapping. But for a cached RenderPass we have all
-    // the content already, so this is redundant and prevents cacheing from
-    // working correctly. Just remove this?
-    if (!ComputeScissorRectForRenderPass(render_pass).IsEmpty())
-      return false;
-    return true;
+    return it->second->id() != 0;
   }
 
   return false;

components/viz/service/display/gl_renderer_unittest.cc

@@ -2804,6 +2804,10 @@ class FramebufferWatchingGLRenderer : public FakeRendererGL {
     return bind_child_framebuffer_calls_;
   }
 
+  void ResetBindCalls() {
+    bind_root_framebuffer_calls_ = bind_child_framebuffer_calls_ = 0;
+  }
+
  private:
   int bind_root_framebuffer_calls_ = 0;
   int bind_child_framebuffer_calls_ = 0;
@@ -2837,18 +2841,47 @@ TEST_F(GLRendererTest, UndamagedRenderPassStillDrawnWhenNoPartialSwap) {
     gfx::Size viewport_size(100, 100);
     gfx::Rect child_rect(10, 10);
 
-    // Child RenderPass has no damage in it.
+    // First frame, the child and root RenderPass each have damage.
     RenderPass* child_pass =
         cc::AddRenderPass(&render_passes_in_draw_order_, 2, child_rect,
                           gfx::Transform(), cc::FilterOperations());
     cc::AddQuad(child_pass, child_rect, SK_ColorGREEN);
-    child_pass->damage_rect = gfx::Rect();
+    child_pass->damage_rect = child_rect;
 
-    // Root RenderPass has some damage that doesn't intersect the child.
     RenderPass* root_pass = cc::AddRenderPass(
         &render_passes_in_draw_order_, 1, gfx::Rect(viewport_size),
         gfx::Transform(), cc::FilterOperations());
     cc::AddQuad(root_pass, gfx::Rect(viewport_size), SK_ColorRED);
+    cc::AddRenderPassQuad(root_pass, child_pass, 0, gfx::Transform(),
+                          SkBlendMode::kSrcOver);
+    root_pass->damage_rect = gfx::Rect(viewport_size);
+
+    EXPECT_EQ(0, renderer.bind_root_framebuffer_calls());
+    EXPECT_EQ(0, renderer.bind_child_framebuffer_calls());
+
+    renderer.DecideRenderPassAllocationsForFrame(render_passes_in_draw_order_);
+    DrawFrame(&renderer, viewport_size);
+
+    // We had to draw the root, and the child.
+    EXPECT_EQ(1, renderer.bind_child_framebuffer_calls());
+    // When the RenderPassDrawQuad in the root is drawn, we may re-bind the root
+    // framebuffer. So it can be bound more than once.
+    EXPECT_GE(renderer.bind_root_framebuffer_calls(), 1);
+
+    // Reset counting.
+    renderer.ResetBindCalls();
+
+    // Second frame, the child RenderPass has no damage in it.
+    child_pass = cc::AddRenderPass(&render_passes_in_draw_order_, 2, child_rect,
+                                   gfx::Transform(), cc::FilterOperations());
+    cc::AddQuad(child_pass, child_rect, SK_ColorGREEN);
+    child_pass->damage_rect = gfx::Rect();
+
+    // Root RenderPass has some damage that doesn't intersect the child.
+    root_pass = cc::AddRenderPass(&render_passes_in_draw_order_, 1,
+                                  gfx::Rect(viewport_size), gfx::Transform(),
+                                  cc::FilterOperations());
+    cc::AddQuad(root_pass, gfx::Rect(viewport_size), SK_ColorRED);
     cc::AddRenderPassQuad(root_pass, child_pass, 0, gfx::Transform(),
                           SkBlendMode::kSrcOver);
     root_pass->damage_rect = gfx::Rect(child_rect.right(), 0, 10, 10);