Delay removing damage rect by one frame for underlays.

When we detect that an underlay is unoccluded, we would previously
consider removing its damage rect if it hadn't moved.  However, this
can cause problems if the underlay was occluded on the previous
frame, since we still need to erase whatever disappeared above it
from the framebuffer.

This CL requires that an underlay is unoccluded for one frame before
removing its damage rect.

Bug: 875879
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I00394c6d6f61c4381a97a6c753c8840485163845
Reviewed-on: https://chromium-review.googlesource.com/1211202Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589969}

components/viz/service/display/overlay_processor.cc

@@ -134,9 +134,12 @@ void OverlayProcessor::ProcessForOverlays(
 #endif
 
   // Reset |previous_frame_underlay_rect_| in case UpdateDamageRect() not being
-  // invoked.
+  // invoked.  Also reset |previous_frame_underlay_was_unoccluded_|.
   const gfx::Rect previous_frame_underlay_rect = previous_frame_underlay_rect_;
   previous_frame_underlay_rect_ = gfx::Rect();
+  bool previous_frame_underlay_was_unoccluded =
+      previous_frame_underlay_was_unoccluded_;
+  previous_frame_underlay_was_unoccluded_ = false;
 
   RenderPass* render_pass = render_passes->back().get();
 
@@ -170,7 +173,8 @@ void OverlayProcessor::ProcessForOverlays(
       continue;
     }
     successful_strategy = strategy.get();
-    UpdateDamageRect(candidates, previous_frame_underlay_rect, damage_rect);
+    UpdateDamageRect(candidates, previous_frame_underlay_rect,
+                     previous_frame_underlay_was_unoccluded, damage_rect);
     break;
   }
 
@@ -193,6 +197,7 @@ void OverlayProcessor::ProcessForOverlays(
 void OverlayProcessor::UpdateDamageRect(
     OverlayCandidateList* candidates,
     const gfx::Rect& previous_frame_underlay_rect,
+    bool previous_frame_underlay_was_unoccluded,
     gfx::Rect* damage_rect) {
   gfx::Rect output_surface_overlay_damage_rect;
   gfx::Rect this_frame_underlay_rect;
@@ -217,11 +222,18 @@ void OverlayProcessor::UpdateDamageRect(
       // has been chosen and the previous underlay rect should be damaged
       // because it has changed planes from the underlay plane to the
       // main plane.
+      //
+      // We also insist that the underlay is unoccluded for at leat one frame,
+      // else when content above the overlay transitions from not fully
+      // transparent to fully transparent, we still need to erase it from the
+      // framebuffer.  Otherwise, the last non-transparent frame will remain.
+      // https://crbug.com/875879
       this_frame_underlay_rect = ToEnclosedRect(overlay.display_rect);
       if ((this_frame_underlay_rect == previous_frame_underlay_rect) &&
-          overlay.is_unoccluded) {
+          overlay.is_unoccluded && previous_frame_underlay_was_unoccluded) {
         damage_rect->Subtract(this_frame_underlay_rect);
       }
+      previous_frame_underlay_was_unoccluded_ = overlay.is_unoccluded;
     }
   }
 

components/viz/service/display/overlay_processor.h

@@ -84,6 +84,7 @@ class VIZ_SERVICE_EXPORT OverlayProcessor {
   OutputSurface* surface_;
   gfx::Rect overlay_damage_rect_;
   gfx::Rect previous_frame_underlay_rect_;
+  bool previous_frame_underlay_was_unoccluded_ = false;
 
  private:
   bool ProcessForCALayers(
@@ -105,6 +106,7 @@ class VIZ_SERVICE_EXPORT OverlayProcessor {
   // Update |damage_rect| by removing damage casued by |candidates|.
   void UpdateDamageRect(OverlayCandidateList* candidates,
                         const gfx::Rect& previous_frame_underlay_rect,
+                        bool previous_frame_underlay_was_unoccluded,
                         gfx::Rect* damage_rect);
 
   DCLayerOverlayProcessor dc_processor_;

components/viz/service/display/overlay_unittest.cc

@@ -1805,6 +1805,43 @@ TEST_F(UnderlayTest, DamageNotSubtractedForNonConsecutiveIdenticalUnderlays) {
   EXPECT_EQ(kOverlayRect, damage_rect_);
 }
 
+// An identical overlay that is occluded should not have damage subtracted until
+// it has been unoccluded for more than one frame.
+TEST_F(UnderlayTest, DamageSubtractedForOneFrameAfterBecomingUnoccluded) {
+  for (int i = 0; i < 3; ++i) {
+    std::unique_ptr<RenderPass> pass = CreateRenderPass();
+    if (i == 0) {
+      // Add an overlapping quad above the candidate for the first frame.
+      CreateFullscreenOpaqueQuad(resource_provider_.get(),
+                                 pass->shared_quad_state_list.back(),
+                                 pass.get());
+    }
+    CreateFullscreenCandidateQuad(
+        resource_provider_.get(), child_resource_provider_.get(),
+        child_provider_.get(), pass->shared_quad_state_list.back(), pass.get());
+
+    damage_rect_ = kOverlayRect;
+
+    OverlayCandidateList candidate_list;
+    OverlayProcessor::FilterOperationsMap render_pass_filters;
+    OverlayProcessor::FilterOperationsMap render_pass_background_filters;
+    RenderPassList pass_list;
+    pass_list.push_back(std::move(pass));
+    overlay_processor_->ProcessForOverlays(
+        resource_provider_.get(), &pass_list, GetIdentityColorMatrix(),
+        render_pass_filters, render_pass_background_filters, &candidate_list,
+        nullptr, nullptr, &damage_rect_, &content_bounds_);
+
+    // The damage rect should not be subtracted if the underlay is occluded
+    // (i==0) or it is unoccluded for the first time (i==1).
+    if (i < 2)
+      EXPECT_FALSE(damage_rect_.IsEmpty());
+  }
+  // The second time the same overlay rect is scheduled it should be subtracted
+  // from the damage rect.
+  EXPECT_TRUE(damage_rect_.IsEmpty());
+}
+
 TEST_F(UnderlayTest, DamageNotSubtractedWhenQuadsAboveOverlap) {
   for (int i = 0; i < 2; ++i) {
     std::unique_ptr<RenderPass> pass = CreateRenderPass();