diff options
author | danakj <danakj@chromium.org> | 2014-11-06 14:05:49 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-11-06 22:06:12 +0000 |
commit | 45377978a7fbf2054502875aec48a90cb27f62e0 (patch) | |
tree | 74239444a956779f1f9f8f386bfd9f416f44e5b8 /cc | |
parent | d411b9c43fffaa7ccd4929ea15a1ec94e3b37884 (diff) | |
download | chromium_src-45377978a7fbf2054502875aec48a90cb27f62e0.zip chromium_src-45377978a7fbf2054502875aec48a90cb27f62e0.tar.gz chromium_src-45377978a7fbf2054502875aec48a90cb27f62e0.tar.bz2 |
cc: DCHECK that pile size == layer size during draw if pile isn't empty.
Ensure that when the pile and layer size don't match during push
properties that the pile is invalidated and given an empty size, since
it means Update was skipped (layer isn't part of the frame) and the
pile is not longer up to date.
In AppendQuads only dcheck the layer bounds() == pile's size when the
pile is not empty. A layer may animate or scroll into the viewport
while it was not part of the main thread frame. We will checkerboard in
that case.
Adds a test to ensure we don't consider the pile to be solid color once
we make it empty to avoid drawing a solid color if that isn't correct
in this scenario. An empty pile shouldn't be used to draw with.
R=enne, vmpstr
BUG=430894
Review URL: https://codereview.chromium.org/711503002
Cr-Commit-Position: refs/heads/master@{#303097}
Diffstat (limited to 'cc')
-rw-r--r-- | cc/layers/picture_layer.cc | 24 | ||||
-rw-r--r-- | cc/layers/picture_layer_impl.cc | 13 | ||||
-rw-r--r-- | cc/layers/picture_layer_unittest.cc | 4 | ||||
-rw-r--r-- | cc/resources/picture_pile.cc | 4 | ||||
-rw-r--r-- | cc/resources/picture_pile_base.cc | 2 | ||||
-rw-r--r-- | cc/resources/picture_pile_unittest.cc | 12 |
6 files changed, 39 insertions, 20 deletions
diff --git a/cc/layers/picture_layer.cc b/cc/layers/picture_layer.cc index f608ba7..c56ff8b 100644 --- a/cc/layers/picture_layer.cc +++ b/cc/layers/picture_layer.cc @@ -35,16 +35,22 @@ void PictureLayer::PushPropertiesTo(LayerImpl* base_layer) { Layer::PushPropertiesTo(base_layer); PictureLayerImpl* layer_impl = static_cast<PictureLayerImpl*>(base_layer); - if (layer_impl->bounds().IsEmpty()) { - // Update may not get called for an empty layer, so resize here instead. - // Using layer_impl because either bounds() or paint_properties().bounds - // may disagree and either one could have been pushed to layer_impl. + int source_frame_number = layer_tree_host()->source_frame_number(); + gfx::Size impl_bounds = layer_impl->bounds(); + gfx::Size pile_bounds = pile_.tiling_size(); + + // If update called, then pile size must match bounds pushed to impl layer. + DCHECK_IMPLIES(update_source_frame_number_ == source_frame_number, + impl_bounds == pile_bounds) + << " bounds " << impl_bounds.ToString() << " pile " + << pile_bounds.ToString(); + + if (update_source_frame_number_ != source_frame_number && + pile_bounds != impl_bounds) { + // Update may not get called for the layer (if it's not in the viewport + // for example, even though it has resized making the pile no longer + // valid. In this case just destroy the pile. pile_.SetEmptyBounds(); - } else { - // If update called, then pile size must match bounds pushed to impl layer. - DCHECK_IMPLIES( - update_source_frame_number_ == layer_tree_host()->source_frame_number(), - layer_impl->bounds().ToString() == pile_.tiling_size().ToString()); } // Unlike other properties, invalidation must always be set on layer_impl. diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc index 3559c45..f9678cc 100644 --- a/cc/layers/picture_layer_impl.cc +++ b/cc/layers/picture_layer_impl.cc @@ -173,14 +173,11 @@ void PictureLayerImpl::AppendQuads(RenderPass* render_pass, AppendQuadsData* append_quads_data) { DCHECK(!needs_post_commit_initialization_); // The bounds and the pile size may differ if the pile wasn't updated (ie. - // PictureLayer::Update didn't happen). But that should never be the case if - // the layer is part of the visible frame, which is why we're appending quads - // in the first place - // TODO(danakj): This DCHECK is causing problems for WebRTC tests and we hit - // this 100% when testing with apprtc (e.g. go/looprtc). - // DCHECK_EQ(bounds().ToString(), pile_->tiling_size().ToString()); - DLOG_IF(ERROR, bounds().ToString() != pile_->tiling_size().ToString()) - << "bounds not equal to tiling size"; + // PictureLayer::Update didn't happen). In that case the pile will be empty. + DCHECK_IMPLIES(!pile_->tiling_size().IsEmpty(), + bounds() == pile_->tiling_size()) + << " bounds " << bounds().ToString() << " pile " + << pile_->tiling_size().ToString(); SharedQuadState* shared_quad_state = render_pass->CreateAndAppendSharedQuadState(); diff --git a/cc/layers/picture_layer_unittest.cc b/cc/layers/picture_layer_unittest.cc index 78182d9..ca622aa 100644 --- a/cc/layers/picture_layer_unittest.cc +++ b/cc/layers/picture_layer_unittest.cc @@ -43,6 +43,10 @@ TEST(PictureLayerTest, NoTilesIfEmptyBounds) { scoped_ptr<ResourceUpdateQueue> queue(new ResourceUpdateQueue); layer->Update(queue.get(), &occlusion); + EXPECT_EQ(0, host->source_frame_number()); + host->CommitComplete(); + EXPECT_EQ(1, host->source_frame_number()); + layer->SetBounds(gfx::Size(0, 0)); layer->SavePaintProperties(); // Intentionally skipping Update since it would normally be skipped on diff --git a/cc/resources/picture_pile.cc b/cc/resources/picture_pile.cc index e00ed54..1ad673b 100644 --- a/cc/resources/picture_pile.cc +++ b/cc/resources/picture_pile.cc @@ -538,9 +538,7 @@ bool PicturePile::UpdateAndExpandInvalidation( void PicturePile::SetEmptyBounds() { tiling_.SetTilingSize(gfx::Size()); - picture_map_.clear(); - has_any_recordings_ = false; - recorded_viewport_ = gfx::Rect(); + Clear(); } void PicturePile::DetermineIfSolidColor() { diff --git a/cc/resources/picture_pile_base.cc b/cc/resources/picture_pile_base.cc index e7888f7..a145b6c 100644 --- a/cc/resources/picture_pile_base.cc +++ b/cc/resources/picture_pile_base.cc @@ -130,6 +130,8 @@ void PicturePileBase::SetBufferPixels(int new_buffer_pixels) { void PicturePileBase::Clear() { picture_map_.clear(); recorded_viewport_ = gfx::Rect(); + has_any_recordings_ = false; + is_solid_color_ = false; } bool PicturePileBase::HasRecordingAt(int x, int y) { diff --git a/cc/resources/picture_pile_unittest.cc b/cc/resources/picture_pile_unittest.cc index 98415ca3..24bed3e 100644 --- a/cc/resources/picture_pile_unittest.cc +++ b/cc/resources/picture_pile_unittest.cc @@ -1456,5 +1456,17 @@ TEST_F(PicturePileTest, NonSolidRectangleOnOffsettedLayerIsNonSolid) { EXPECT_FALSE(pile_.is_solid_color()); } +TEST_F(PicturePileTest, SetEmptyBounds) { + EXPECT_TRUE(pile_.is_solid_color()); + EXPECT_FALSE(pile_.tiling_size().IsEmpty()); + EXPECT_FALSE(pile_.picture_map().empty()); + EXPECT_TRUE(pile_.HasRecordings()); + pile_.SetEmptyBounds(); + EXPECT_FALSE(pile_.is_solid_color()); + EXPECT_TRUE(pile_.tiling_size().IsEmpty()); + EXPECT_TRUE(pile_.picture_map().empty()); + EXPECT_FALSE(pile_.HasRecordings()); +} + } // namespace } // namespace cc |