From 1e3ddda564efbaa41286a6fbb0b0b089c20a5a9b Mon Sep 17 00:00:00 2001 From: jdonnelly Date: Thu, 23 Apr 2015 13:55:35 -0700 Subject: [PATCH] Revert of ui: Clean up damaged rects and clear them after painting. (patchset #2 id:20001 of https://codereview.chromium.org/1080633009/) Reason for revert: ash_unittests are timing out on Win8 Aura and Linux ChromiumOS Tests (dbg)(1) bot and this CL is suspected to be the cause of the problem. Original issue's description: > ui: Clean up damaged rects and clear them after painting. > > This cleans up some of the damage rects code by converting the SkRegion > to a cc::Region, allowing use of gfx::Rects. It moves the recursion > over the Layer tree out to Compositor instead of on Layer. > > And we keep the damaged_region_ valid until the layer is painted. This > will allow us to pass that damaged_region_ to the painting code with > impl-side slimming paint, because with impl-side painting, the paint > clip rect can be larger than the invalidations (as large as the whole > layer). > > R=piman@chromium.org, sky > BUG=466426 > > Committed: https://crrev.com/a5e585868e16ce53c990f764d4943592f11749aa > Cr-Commit-Position: refs/heads/master@{#326547} TBR=piman@chromium.org,sky@chromium.org,danakj@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=466426 Review URL: https://codereview.chromium.org/1101103004 Cr-Commit-Position: refs/heads/master@{#326637} --- .../compositor/reflector_impl_unittest.cc | 13 ++--- ui/compositor/compositor.cc | 11 +--- ui/compositor/layer.cc | 51 ++++++++++--------- ui/compositor/layer.h | 10 ++-- 4 files changed, 41 insertions(+), 44 deletions(-) diff --git a/content/browser/compositor/reflector_impl_unittest.cc b/content/browser/compositor/reflector_impl_unittest.cc index 867a9de10a2249..2f0c299fcf1fd4 100644 --- a/content/browser/compositor/reflector_impl_unittest.cc +++ b/content/browser/compositor/reflector_impl_unittest.cc @@ -91,7 +91,8 @@ class TestOutputSurface : public BrowserCompositorOutputSurface { gfx::Size SurfaceSize() const override { return gfx::Size(256, 256); } }; -const gfx::Rect kSubRect(0, 0, 64, 64); +const gfx::Rect kSubRect = gfx::Rect(0, 0, 64, 64); +const SkIRect kSkSubRect = SkIRect::MakeXYWH(0, 0, 64, 64); } // namespace @@ -160,10 +161,10 @@ TEST_F(ReflectorImplTest, CheckNormalOutputSurface) { SetUpReflector(); UpdateTexture(); EXPECT_TRUE(mirroring_layer_->TextureFlipped()); - gfx::Rect expected_rect = - kSubRect + gfx::Vector2d(0, output_surface_->SurfaceSize().height()) - - gfx::Vector2d(0, kSubRect.height()); - EXPECT_EQ(expected_rect, mirroring_layer_->damaged_region()); + EXPECT_EQ(SkRegion(SkIRect::MakeXYWH( + 0, output_surface_->SurfaceSize().height() - kSubRect.height(), + kSubRect.width(), kSubRect.height())), + mirroring_layer_->damaged_region()); } TEST_F(ReflectorImplTest, CheckInvertedOutputSurface) { @@ -171,7 +172,7 @@ TEST_F(ReflectorImplTest, CheckInvertedOutputSurface) { SetUpReflector(); UpdateTexture(); EXPECT_FALSE(mirroring_layer_->TextureFlipped()); - EXPECT_EQ(kSubRect, mirroring_layer_->damaged_region()); + EXPECT_EQ(SkRegion(kSkSubRect), mirroring_layer_->damaged_region()); } #if defined(USE_OZONE) diff --git a/ui/compositor/compositor.cc b/ui/compositor/compositor.cc index c50f9efdddb532..43dd4605a11ab6 100644 --- a/ui/compositor/compositor.cc +++ b/ui/compositor/compositor.cc @@ -326,16 +326,9 @@ void Compositor::BeginMainFrame(const cc::BeginFrameArgs& args) { void Compositor::BeginMainFrameNotExpectedSoon() { } -static void SendDamagedRectsRecursive(ui::Layer* layer) { - layer->SendDamagedRects(); - for (auto* child : layer->children()) - SendDamagedRectsRecursive(child); -} - void Compositor::Layout() { - if (!root_layer()) - return; - SendDamagedRectsRecursive(root_layer()); + if (root_layer_) + root_layer_->SendDamagedRects(); } void Compositor::RequestNewOutputSurface() { diff --git a/ui/compositor/layer.cc b/ui/compositor/layer.cc index 914e000feedff6..3e61d40b95a2c0 100644 --- a/ui/compositor/layer.cc +++ b/ui/compositor/layer.cc @@ -657,7 +657,11 @@ bool Layer::SchedulePaint(const gfx::Rect& invalid_rect) { type_ == LAYER_NINE_PATCH || (!delegate_ && !mailbox_.IsValid())) return false; - damaged_region_.Union(invalid_rect); + damaged_region_.op(invalid_rect.x(), + invalid_rect.y(), + invalid_rect.right(), + invalid_rect.bottom(), + SkRegion::kUnion_Op); ScheduleDraw(); return true; } @@ -669,17 +673,20 @@ void Layer::ScheduleDraw() { } void Layer::SendDamagedRects() { - if (damaged_region_.IsEmpty()) - return; - if (!delegate_ && !mailbox_.IsValid()) - return; - - for (cc::Region::Iterator iter(damaged_region_); iter.has_rect(); iter.next()) - cc_layer_->SetNeedsDisplayRect(iter.rect()); -} - -void Layer::ClearDamagedRects() { - damaged_region_.Clear(); + if ((delegate_ || mailbox_.IsValid()) && !damaged_region_.isEmpty()) { + for (SkRegion::Iterator iter(damaged_region_); !iter.done(); iter.next()) { + const SkIRect& sk_damaged = iter.rect(); + gfx::Rect damaged( + sk_damaged.x(), + sk_damaged.y(), + sk_damaged.width(), + sk_damaged.height()); + cc_layer_->SetNeedsDisplayRect(damaged); + } + damaged_region_.setEmpty(); + } + for (size_t i = 0; i < children_.size(); ++i) + children_[i]->SendDamagedRects(); } void Layer::CompleteAllAnimations() { @@ -738,7 +745,6 @@ void Layer::PaintContents( const gfx::Rect& clip, ContentLayerClient::PaintingControlSetting painting_control) { TRACE_EVENT1("ui", "Layer::PaintContents", "name", name_); - ClearDamagedRects(); scoped_ptr canvas(gfx::Canvas::CreateCanvasWithoutScaling( sk_canvas, device_scale_factor_)); if (delegate_) @@ -750,16 +756,15 @@ void Layer::PaintContentsToDisplayList( const gfx::Rect& clip, ContentLayerClient::PaintingControlSetting painting_control) { TRACE_EVENT1("ui", "Layer::PaintContentsToDisplayList", "name", name_); - ClearDamagedRects(); - if (!delegate_) - return; - // TODO(danakj): Save the invalidation on the layer and pass that down - // instead of the |clip| here. That will break everything until View - // early-outs emit cached display items instead of nothing. - gfx::Rect invalidation = clip; - DCHECK(clip.Contains(invalidation)); - delegate_->OnPaintLayer( - PaintContext(display_list, device_scale_factor_, clip, invalidation)); + if (delegate_) { + // TODO(danakj): Save the invalidation on the layer and pass that down + // instead of the |clip| here. That will break everything until View + // early-outs emit cached display items instead of nothing. + gfx::Rect invalidation = clip; + DCHECK(clip.Contains(invalidation)); + delegate_->OnPaintLayer( + PaintContext(display_list, device_scale_factor_, clip, invalidation)); + } } bool Layer::FillsBoundsCompletely() const { return fills_bounds_completely_; } diff --git a/ui/compositor/layer.h b/ui/compositor/layer.h index bb8b1af6de89c7..381e9829b3250a 100644 --- a/ui/compositor/layer.h +++ b/ui/compositor/layer.h @@ -14,7 +14,6 @@ #include "base/message_loop/message_loop.h" #include "cc/animation/animation_events.h" #include "cc/animation/layer_animation_event_observer.h" -#include "cc/base/region.h" #include "cc/base/scoped_ptr_vector.h" #include "cc/layers/content_layer_client.h" #include "cc/layers/layer_client.h" @@ -320,9 +319,8 @@ class COMPOSITOR_EXPORT Layer // Uses damaged rectangles recorded in |damaged_region_| to invalidate the // |cc_layer_|. void SendDamagedRects(); - void ClearDamagedRects(); - const cc::Region& damaged_region() const { return damaged_region_; } + const SkRegion& damaged_region() const { return damaged_region_; } void CompleteAllAnimations(); @@ -461,9 +459,9 @@ class COMPOSITOR_EXPORT Layer bool fills_bounds_opaquely_; bool fills_bounds_completely_; - // Union of damaged rects, in layer space, to be used when compositor is ready - // to paint the content. - cc::Region damaged_region_; + // Union of damaged rects, in pixel coordinates, to be used when + // compositor is ready to paint the content. + SkRegion damaged_region_; int background_blur_radius_;