Revert "viz: Re-enable ignore_undamaged on platforms using GL buffer queue"

This reverts commit a79f5b3b.

Reason for revert: Enabled the optimization on the wrong platforms and it causes flickering (crbug.com/1082947)

Original change's description:
> viz: Re-enable ignore_undamaged on platforms using GL buffer queue
> 
> SurfaceAggregator has an option to output only the quads that
> intersect the damage rect computed by SA itself.
> 
> This optimization was disabled on devices with HW overlays or where
> the renderer might expand the damage, since SA would not know the
> final damage rect.
> 
> This CL re-enables the optimization on devices where we can estimate
> a bounding rect of the renderer expanded damage.
> 
> This is achieved by asking to the renderer for a bounding rect that
> represents the maximum area that can be expanded.
> SurfaceAggregator will aggregate the boundingrect union the damage.
> 
> The damage on the root render pass will be unchanged, so that HW
> overlays damage computation won't be affected.
> 
> 
> Bug: 1077210
> Test: viz_unittest (new test and Display tests)
> Change-Id: Ie94d1f09923959e2e025845f89a9b3eae759a64a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2179763
> Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
> Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
> Reviewed-by: kylechar <kylechar@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#768572}

TBR=dcastagna@chromium.org,sunnyps@chromium.org,kylechar@chromium.org

Change-Id: Id7453435c8b50981e5b7752855e3fd2190076676
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1077210
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2203219Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769005}

components/viz/service/display/direct_renderer.cc

@@ -465,15 +465,6 @@ void DirectRenderer::DrawFrame(
   current_frame_valid_ = false;
 }
 
-gfx::Rect DirectRenderer::GetTargetDamageBoundingRect() const {
-  gfx::Rect bounding_rect = output_surface_->GetCurrentFramebufferDamage();
-  if (overlay_processor_) {
-    bounding_rect.Union(
-        overlay_processor_->GetPreviousFrameOverlaysBoundingRect());
-  }
-  return bounding_rect;
-}
-
 gfx::Rect DirectRenderer::DeviceViewportRectInDrawSpace() const {
   gfx::Rect device_viewport_rect(current_frame()->device_viewport_size);
   device_viewport_rect -= current_viewport_rect_.OffsetFromOrigin();

components/viz/service/display/direct_renderer.h

@@ -69,11 +69,6 @@ class VIZ_SERVICE_EXPORT DirectRenderer {
                  const gfx::Size& device_viewport_size,
                  const gfx::DisplayColorSpaces& display_color_spaces);
 
-  // The renderer might expand the damage (e.g: HW overlays were used,
-  // invalidation rects on previous buffers). This function returns a
-  // bounding rect of the area that might need to be recomposited.
-  gfx::Rect GetTargetDamageBoundingRect() const;
-
   // Public interface implemented by subclasses.
   struct SwapFrameData {
     SwapFrameData();

components/viz/service/display/display.cc

@@ -520,9 +520,9 @@ void Display::InitializeRenderer(bool enable_shared_images) {
   // Outputting a partial list of quads might not work in cases where contents
   // outside the damage rect might be needed by the renderer.
   bool output_partial_list =
+      output_surface_->capabilities().only_invalidates_damage_rect &&
       renderer_->use_partial_swap() &&
-      (!overlay_processor_->IsOverlaySupported() ||
-       output_surface_->capabilities().supports_target_damage);
+      !overlay_processor_->IsOverlaySupported();
 
   aggregator_ = std::make_unique<SurfaceAggregator>(
       surface_manager_, resource_provider_.get(), output_partial_list,
@@ -609,13 +609,9 @@ bool Display::DrawAndSwap(base::TimeTicks expected_display_time) {
   {
     FrameRateDecider::ScopedAggregate scoped_aggregate(
         frame_rate_decider_.get());
-    gfx::Rect target_damage_bounding_rect;
-    if (output_surface_->capabilities().supports_target_damage)
-      target_damage_bounding_rect = renderer_->GetTargetDamageBoundingRect();
-
-    frame = aggregator_->Aggregate(
-        current_surface_id_, expected_display_time, current_display_transform,
-        target_damage_bounding_rect, ++swapped_trace_id_);
+    frame =
+        aggregator_->Aggregate(current_surface_id_, expected_display_time,
+                               current_display_transform, ++swapped_trace_id_);
   }
 
 #if defined(OS_ANDROID)

components/viz/service/display/output_surface.h

@@ -84,9 +84,10 @@ class VIZ_SERVICE_EXPORT OutputSurface {
     // the unified display on Chrome OS. All drawing is handled by the physical
     // displays so the unified display should skip that work.
     bool skips_draw = false;
-    // Whether OutputSurface::GetTargetDamageBoundingRect is implemented and
-    // will return a bounding rectangle of the target buffer invalidated area.
-    bool supports_target_damage = false;
+    // Indicates whether this surface will invalidate only the damage rect.
+    // When this is false contents outside the damaged area might need to be
+    // recomposited to the surface.
+    bool only_invalidates_damage_rect = true;
     // Whether the gpu supports surfaceless surface (equivalent of using buffer
     // queue).
     bool supports_surfaceless = false;

components/viz/service/display/overlay_processor_interface.h

@@ -107,10 +107,6 @@ class VIZ_SERVICE_EXPORT OverlayProcessorInterface {
   virtual ~OverlayProcessorInterface() {}
 
   virtual bool IsOverlaySupported() const = 0;
-  // Returns a bounding rectangle of the last set of overlay planes scheduled.
-  // It's expected to be called after ProcessForOverlays at frame N-1 has been
-  // called and before GetAndResetOverlayDamage at frame N.
-  virtual gfx::Rect GetPreviousFrameOverlaysBoundingRect() const = 0;
   virtual gfx::Rect GetAndResetOverlayDamage() = 0;
 
   // Returns true if the platform supports hw overlays and surface occluding

components/viz/service/display/overlay_processor_mac.cc

@@ -37,12 +37,6 @@ bool OverlayProcessorMac::IsOverlaySupported() const {
   return could_overlay_;
 }
 
-gfx::Rect OverlayProcessorMac::GetPreviousFrameOverlaysBoundingRect() const {
-  // TODO(dcastagna): Implement me.
-  NOTIMPLEMENTED();
-  return gfx::Rect();
-}
-
 gfx::Rect OverlayProcessorMac::GetAndResetOverlayDamage() {
   gfx::Rect result = ca_overlay_damage_rect_;
   ca_overlay_damage_rect_ = gfx::Rect();

components/viz/service/display/overlay_processor_mac.h

@@ -35,7 +35,6 @@ class VIZ_SERVICE_EXPORT OverlayProcessorMac
   bool DisableSplittingQuads() const override;
 
   bool IsOverlaySupported() const override;
-  gfx::Rect GetPreviousFrameOverlaysBoundingRect() const override;
   gfx::Rect GetAndResetOverlayDamage() override;
 
   // Returns true if the platform supports hw overlays and surface occluding

components/viz/service/display/overlay_processor_stub.cc

@@ -11,10 +11,6 @@ bool OverlayProcessorStub::IsOverlaySupported() const {
 gfx::Rect OverlayProcessorStub::GetAndResetOverlayDamage() {
   return gfx::Rect();
 }
-gfx::Rect OverlayProcessorStub::GetPreviousFrameOverlaysBoundingRect() const {
-  return gfx::Rect();
-}
-
 bool OverlayProcessorStub::NeedsSurfaceOccludingDamageRect() const {
   return false;
 }

components/viz/service/display/overlay_processor_stub.h

@@ -18,7 +18,6 @@ class VIZ_SERVICE_EXPORT OverlayProcessorStub
 
   // Overrides OverlayProcessorInterface's pure virtual functions.
   bool IsOverlaySupported() const final;
-  gfx::Rect GetPreviousFrameOverlaysBoundingRect() const final;
   gfx::Rect GetAndResetOverlayDamage() final;
   bool NeedsSurfaceOccludingDamageRect() const final;
   void ProcessForOverlays(

components/viz/service/display/overlay_processor_using_strategy.cc

@@ -35,13 +35,6 @@ OverlayProcessorUsingStrategy::OverlayProcessorUsingStrategy()
 
 OverlayProcessorUsingStrategy::~OverlayProcessorUsingStrategy() = default;
 
-gfx::Rect OverlayProcessorUsingStrategy::GetPreviousFrameOverlaysBoundingRect()
-    const {
-  gfx::Rect result = overlay_damage_rect_;
-  result.Union(previous_frame_underlay_rect_);
-  return result;
-}
-
 gfx::Rect OverlayProcessorUsingStrategy::GetAndResetOverlayDamage() {
   gfx::Rect result = overlay_damage_rect_;
   overlay_damage_rect_ = gfx::Rect();

components/viz/service/display/overlay_processor_using_strategy.h

@@ -66,7 +66,6 @@ class VIZ_SERVICE_EXPORT OverlayProcessorUsingStrategy
 
   ~OverlayProcessorUsingStrategy() override;
 
-  gfx::Rect GetPreviousFrameOverlaysBoundingRect() const final;
   gfx::Rect GetAndResetOverlayDamage() final;
 
   // Override OverlayProcessor.

components/viz/service/display/overlay_processor_win.cc

@@ -27,12 +27,6 @@ bool OverlayProcessorWin::IsOverlaySupported() const {
   return enable_dc_overlay_;
 }
 
-gfx::Rect OverlayProcessorWin::GetPreviousFrameOverlaysBoundingRect() const {
-  // TODO(dcastagna): Implement me.
-  NOTIMPLEMENTED();
-  return gfx::Rect();
-}
-
 gfx::Rect OverlayProcessorWin::GetAndResetOverlayDamage() {
   return gfx::Rect();
 }

components/viz/service/display/overlay_processor_win.h

@@ -34,7 +34,6 @@ class VIZ_SERVICE_EXPORT OverlayProcessorWin
   ~OverlayProcessorWin() override;
 
   bool IsOverlaySupported() const override;
-  gfx::Rect GetPreviousFrameOverlaysBoundingRect() const override;
   gfx::Rect GetAndResetOverlayDamage() override;
 
   // Returns true if the platform supports hw overlays and surface occluding

components/viz/service/display/surface_aggregator.cc

@@ -1763,7 +1763,6 @@ CompositorFrame SurfaceAggregator::Aggregate(
     const SurfaceId& surface_id,
     base::TimeTicks expected_display_time,
     gfx::OverlayTransform display_transform,
-    const gfx::Rect& target_damage,
     int64_t display_trace_id) {
   DCHECK(!expected_display_time.is_null());
 
@@ -1811,13 +1810,8 @@ CompositorFrame SurfaceAggregator::Aggregate(
   new_surfaces_.clear();
   DCHECK(referenced_surfaces_.empty());
   PrewalkResult prewalk_result;
-  gfx::Rect surfaces_damage_rect =
+  root_damage_rect_ =
       PrewalkTree(surface, false, 0, true /* will_draw */, &prewalk_result);
-
-  root_damage_rect_ = surfaces_damage_rect;
-  // |root_damage_rect_| is used to restrict aggregating quads only if they
-  // intersect this area.
-  root_damage_rect_.Union(target_damage);
   root_content_color_usage_ = prewalk_result.content_color_usage;
 
   if (prewalk_result.frame_sinks_changed)
@@ -1833,15 +1827,6 @@ CompositorFrame SurfaceAggregator::Aggregate(
   CopyPasses(root_surface_frame, surface);
   referenced_surfaces_.erase(surface_id);
 
-  // The root render pass damage might have been expanded by target_damage (the
-  // area that might need to be recomposited on the target surface). We restrict
-  // the damage_rect of the root render pass to the one caused by the source
-  // surfaces.
-  // The damage on the root render pass should not include the expanded area
-  // since Renderer and OverlayProcessor expect the non expanded damage.
-  if (!RenderPassNeedsFullDamage(dest_pass_list_->back().get()))
-    dest_pass_list_->back()->damage_rect.Intersect(surfaces_damage_rect);
-
   // Now that we've handled our main surface aggregation, apply de-jelly effect
   // if enabled.
   if (de_jelly_enabled_)

components/viz/service/display/surface_aggregator.h

@@ -50,14 +50,9 @@ class VIZ_SERVICE_EXPORT SurfaceAggregator {
                     bool needs_surface_occluding_damage_rect);
   ~SurfaceAggregator();
 
-  // |target_damage| represents an area on the output surface that might have
-  // been invalidated. It can be used in cases where we still want to support
-  // partial damage but the target surface might need contents outside the
-  // damage rect of the root surface.
   CompositorFrame Aggregate(const SurfaceId& surface_id,
                             base::TimeTicks expected_display_time,
                             gfx::OverlayTransform display_transform,
-                            const gfx::Rect& target_damage = gfx::Rect(),
                             int64_t display_trace_id = -1);
   void ReleaseResources(const SurfaceId& surface_id);
   const SurfaceIndexMap& previous_contained_surfaces() const {

components/viz/service/display/surface_aggregator_unittest.cc

@@ -132,11 +132,10 @@ class SurfaceAggregatorTest : public testing::Test, public DisplayTimeSource {
     testing::Test::TearDown();
   }
 
-  CompositorFrame AggregateFrame(const SurfaceId& surface_id,
-                                 gfx::Rect target_damage = gfx::Rect()) {
+  CompositorFrame AggregateFrame(const SurfaceId& surface_id) {
     return aggregator_.Aggregate(
         surface_id, GetNextDisplayTimeAndIncrement(),
-        /*display_transform=*/gfx::OVERLAY_TRANSFORM_NONE, target_damage);
+        gfx::OVERLAY_TRANSFORM_NONE /* display_transform */);
   }
 
   struct Quad {
@@ -4400,133 +4399,6 @@ TEST_F(SurfaceAggregatorPartialSwapTest, IgnoreOutside) {
   }
 }
 
-TEST_F(SurfaceAggregatorPartialSwapTest, ExpandByTargetDamage) {
-  ParentLocalSurfaceIdAllocator allocator;
-  allocator.GenerateId();
-  LocalSurfaceId child_local_surface_id =
-      allocator.GetCurrentLocalSurfaceIdAllocation().local_surface_id();
-  SurfaceId child_surface_id(child_sink_->frame_sink_id(),
-                             child_local_surface_id);
-  constexpr float device_scale_factor = 1.0f;
-
-  // The child surface has one quad.
-  {
-    int child_pass_id = 1;
-    std::vector<Quad> child_quads1 = {
-        Quad::SolidColorQuad(SK_ColorGREEN, gfx::Rect(5, 5))};
-    std::vector<Pass> child_passes = {
-        Pass(child_quads1, child_pass_id, gfx::Rect(5, 5))};
-
-    RenderPassList child_pass_list;
-    std::vector<SurfaceRange> referenced_surfaces;
-    AddPasses(&child_pass_list, child_passes, &referenced_surfaces);
-
-    SubmitPassListAsFrame(child_sink_.get(), child_local_surface_id,
-                          &child_pass_list, std::move(referenced_surfaces),
-                          device_scale_factor);
-  }
-
-  {
-    std::vector<Quad> root_quads = {Quad::SurfaceQuad(
-        SurfaceRange(base::nullopt, child_surface_id), SK_ColorWHITE,
-        gfx::Rect(SurfaceSize()), /*stretch_content_to_fill_bounds=*/false)};
-
-    std::vector<Pass> root_passes = {Pass(root_quads, SurfaceSize())};
-
-    RenderPassList root_pass_list;
-    std::vector<SurfaceRange> referenced_surfaces;
-    AddPasses(&root_pass_list, root_passes, &referenced_surfaces);
-    // No damage, this is the first frame submitted, so all quads should be
-    // produced.
-    SubmitPassListAsFrame(root_sink_.get(), root_local_surface_id_,
-                          &root_pass_list, std::move(referenced_surfaces),
-                          device_scale_factor);
-  }
-
-  SurfaceId root_surface_id(root_sink_->frame_sink_id(),
-                            root_local_surface_id_);
-  CompositorFrame aggregated_frame = AggregateFrame(root_surface_id);
-
-  const auto& aggregated_pass_list = aggregated_frame.render_pass_list;
-
-  ASSERT_EQ(1u, aggregated_pass_list.size());
-
-  // Damage rect for first aggregation should contain entire root surface.
-  EXPECT_EQ(gfx::Rect(SurfaceSize()), aggregated_pass_list.back()->damage_rect);
-  EXPECT_EQ(1u, aggregated_pass_list[0]->quad_list.size());
-
-  // Create a root surface with a smaller damage rect.
-  // This time the damage should be smaller.
-  {
-    std::vector<Quad> root_quads = {Quad::SurfaceQuad(
-        SurfaceRange(base::nullopt, child_surface_id), SK_ColorWHITE,
-        gfx::Rect(SurfaceSize()), /*stretch_content_to_fill_bounds=*/false)};
-
-    std::vector<Pass> root_passes = {Pass(root_quads, SurfaceSize())};
-
-    RenderPassList root_pass_list;
-    std::vector<SurfaceRange> referenced_surfaces;
-    AddPasses(&root_pass_list, root_passes, &referenced_surfaces);
-
-    auto* root_pass = root_pass_list[0].get();
-    root_pass->damage_rect = gfx::Rect(10, 10, 2, 2);
-    SubmitPassListAsFrame(root_sink_.get(), root_local_surface_id_,
-                          &root_pass_list, std::move(referenced_surfaces),
-                          device_scale_factor);
-  }
-
-  {
-    CompositorFrame aggregated_frame = AggregateFrame(root_surface_id);
-
-    const auto& aggregated_pass_list = aggregated_frame.render_pass_list;
-
-    ASSERT_EQ(1u, aggregated_pass_list.size());
-
-    // No quads inside the damage
-    EXPECT_EQ(gfx::Rect(10, 10, 2, 2),
-              aggregated_pass_list.back()->damage_rect);
-    EXPECT_EQ(0u, aggregated_pass_list.back()->quad_list.size());
-  }
-
-  // This pass has damage that does not intersect the quad in the child
-  // surface.
-  {
-    std::vector<Quad> root_quads = {
-        Quad::SurfaceQuad(SurfaceRange(base::nullopt, child_surface_id),
-                          SK_ColorWHITE, gfx::Rect(SurfaceSize()), false)};
-
-    std::vector<Pass> root_passes = {Pass(root_quads, SurfaceSize())};
-
-    RenderPassList root_pass_list;
-    std::vector<SurfaceRange> referenced_surfaces;
-    AddPasses(&root_pass_list, root_passes, &referenced_surfaces);
-
-    auto* root_pass = root_pass_list[0].get();
-    root_pass->damage_rect = gfx::Rect(10, 10, 2, 2);
-    SubmitPassListAsFrame(root_sink_.get(), root_local_surface_id_,
-                          &root_pass_list, std::move(referenced_surfaces),
-                          device_scale_factor);
-  }
-
-  // The target surface invalidates one pixel in the top left, the quad in the
-  // child surface should be added even if it's not causing damage nor in the
-  // root render pass damage.
-  {
-    gfx::Rect target_damage(0, 0, 1, 1);
-    CompositorFrame aggregated_frame =
-        AggregateFrame(root_surface_id, target_damage);
-
-    const auto& aggregated_pass_list = aggregated_frame.render_pass_list;
-    ASSERT_EQ(1u, aggregated_pass_list.size());
-
-    // The damage rect of the root render pass should not be changed.
-    EXPECT_EQ(gfx::Rect(10, 10, 2, 2),
-              aggregated_pass_list.back()->damage_rect);
-    // We expect one quad
-    ASSERT_EQ(1u, aggregated_pass_list[0]->quad_list.size());
-  }
-}
-
 class SurfaceAggregatorWithResourcesTest : public testing::Test,
                                            public DisplayTimeSource {
  public:

components/viz/service/display_embedder/buffer_queue.cc

@@ -52,18 +52,8 @@ void BufferQueue::UpdateBufferDamage(const gfx::Rect& damage) {
 }
 
 gfx::Rect BufferQueue::CurrentBufferDamage() const {
-  if (current_surface_)
-    return current_surface_->damage;
-
-  // In case there is no current_surface_, we get the damage from the surface
-  // that will be set as current_surface_ by the next call to GetNextSurface.
-  if (!available_surfaces_.empty()) {
-    return available_surfaces_.back()->damage;
-  }
-
-  // If we can't determine which surface will be the next current_surface_, we
-  // conservatively invalidate the whole buffer.
-  return gfx::Rect(size_);
+  DCHECK(current_surface_);
+  return current_surface_->damage;
 }
 
 void BufferQueue::SwapBuffers(const gfx::Rect& damage) {

components/viz/service/display_embedder/gl_output_surface_buffer_queue.cc

@@ -8,7 +8,6 @@
 
 #include "base/bind.h"
 #include "base/command_line.h"
-#include "build/build_config.h"
 #include "components/viz/common/frame_sinks/begin_frame_source.h"
 #include "components/viz/common/gpu/context_provider.h"
 #include "components/viz/common/switches.h"
@@ -30,6 +29,7 @@ GLOutputSurfaceBufferQueue::GLOutputSurfaceBufferQueue(
     std::unique_ptr<BufferQueue> buffer_queue)
     : GLOutputSurface(context_provider, surface_handle),
       buffer_queue_(std::move(buffer_queue)) {
+  capabilities_.only_invalidates_damage_rect = false;
   capabilities_.uses_default_gl_framebuffer = false;
   capabilities_.output_surface_origin = gfx::SurfaceOrigin::kTopLeft;
   // Set |max_frames_pending| to 2 for buffer_queue, which aligns scheduling
@@ -40,13 +40,7 @@ GLOutputSurfaceBufferQueue::GLOutputSurfaceBufferQueue(
   // shifts the start of the new frame forward relative to the old
   // implementation.
   capabilities_.max_frames_pending = 2;
-  // GetCurrentFramebufferDamage will return an upper bound of the part of the
-  // buffer that needs to be recomposited.
-#if defined(OS_MACOSX)
-  capabilities_.supports_target_damage = false;
-#else
-  capabilities_.supports_target_damage = true;
-#endif
+
   // Force the number of max pending frames to one when the switch
   // "double-buffer-compositing" is passed.
   // This will keep compositing in double buffered mode assuming |buffer_queue_|

components/viz/service/display_embedder/skia_output_device_buffer_queue.cc

@@ -291,6 +291,7 @@ SkiaOutputDeviceBufferQueue::SkiaOutputDeviceBufferQueue(
   if (command_line->HasSwitch(switches::kDoubleBufferCompositing))
     capabilities_.max_frames_pending = 1;
 
+  capabilities_.only_invalidates_damage_rect = false;
   // Set supports_surfaceless to enable overlays.
   capabilities_.supports_surfaceless = true;
   capabilities_.preserve_buffer_content = true;

components/viz/service/display_embedder/skia_output_surface_impl.cc

@@ -691,6 +691,7 @@ bool SkiaOutputSurfaceImpl::Initialize() {
 
   if (capabilities_.preserve_buffer_content &&
       capabilities_.supports_post_sub_buffer) {
+    capabilities_.only_invalidates_damage_rect = false;
     damage_of_buffers_.resize(capabilities_.max_frames_pending + 1);
   }