diff options
author | danakj <danakj@chromium.org> | 2015-01-30 09:34:28 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-01-30 17:35:37 +0000 |
commit | d6abedbc881cd740d76f80f1c89bd1e737b07e7a (patch) | |
tree | b7be1d09de17b34b2b28587e1ec86ebe511dceed /cc | |
parent | e6904ac0e7eddc08252f22cc26aae6919e90685f (diff) | |
download | chromium_src-d6abedbc881cd740d76f80f1c89bd1e737b07e7a.zip chromium_src-d6abedbc881cd740d76f80f1c89bd1e737b07e7a.tar.gz chromium_src-d6abedbc881cd740d76f80f1c89bd1e737b07e7a.tar.bz2 |
Revert of cc: Stop pushing properties every activation for picture layers. (patchset #3 id:40001 of https://codereview.chromium.org/874613003/)
Reason for revert:
flaky tests https://code.google.com/p/chromium/issues/detail?id=452965
Original issue's description:
> cc: Stop pushing properties every activation for picture layers.
>
> Since we don't swap tilings anymore, we only need to push properties
> when a property, or a tiling, or the raster source (invalidation) has
> changed.
>
> R=vmpstr
> BUG=303943
>
> Committed: https://crrev.com/dcaa923e704847eb6ba7003602ff70579aaaf47e
> Cr-Commit-Position: refs/heads/master@{#313329}
TBR=vmpstr@chromium.org,enne@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=303943
Review URL: https://codereview.chromium.org/890963002
Cr-Commit-Position: refs/heads/master@{#313935}
Diffstat (limited to 'cc')
-rw-r--r-- | cc/debug/rasterize_and_record_benchmark_impl.cc | 2 | ||||
-rw-r--r-- | cc/layers/picture_layer_impl.cc | 15 | ||||
-rw-r--r-- | cc/layers/picture_layer_impl.h | 1 | ||||
-rw-r--r-- | cc/layers/picture_layer_impl_unittest.cc | 165 | ||||
-rw-r--r-- | cc/resources/picture_layer_tiling.cc | 1 | ||||
-rw-r--r-- | cc/resources/picture_layer_tiling.h | 4 | ||||
-rw-r--r-- | cc/resources/picture_layer_tiling_set.cc | 1 | ||||
-rw-r--r-- | cc/test/fake_picture_layer_tiling_client.h | 1 |
8 files changed, 10 insertions, 180 deletions
diff --git a/cc/debug/rasterize_and_record_benchmark_impl.cc b/cc/debug/rasterize_and_record_benchmark_impl.cc index 187bb68..5a9577b 100644 --- a/cc/debug/rasterize_and_record_benchmark_impl.cc +++ b/cc/debug/rasterize_and_record_benchmark_impl.cc @@ -105,8 +105,6 @@ class FixedInvalidationPictureLayerTilingClient return base_client_->RequiresHighResToDraw(); } - void TilingLiveRectChanged() override {} - private: PictureLayerTilingClient* base_client_; Region invalidation_; diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc index 5d64cede..36c63c4 100644 --- a/cc/layers/picture_layer_impl.cc +++ b/cc/layers/picture_layer_impl.cc @@ -142,6 +142,11 @@ void PictureLayerImpl::PushPropertiesTo(LayerImpl* base_layer) { layer_impl->low_res_raster_contents_scale_ = low_res_raster_contents_scale_; layer_impl->SanityCheckTilingState(); + + // We always need to push properties. + // See http://crbug.com/303943 + // TODO(danakj): Stop always pushing properties since we don't swap tilings. + needs_push_properties_ = true; } void PictureLayerImpl::AppendQuads(RenderPass* render_pass, @@ -566,10 +571,6 @@ void PictureLayerImpl::UpdateRasterSource( if (could_have_tilings != can_have_tilings) layer_tree_impl()->set_needs_update_draw_properties(); - // SetNeedsPushProperties when changing the raster source. This shouldn't - // imply damage but need to be synced. - SetNeedsPushProperties(); - if (!can_have_tilings) { RemoveAllTilings(); return; @@ -663,12 +664,6 @@ bool PictureLayerImpl::RequiresHighResToDraw() const { return layer_tree_impl()->RequiresHighResToDraw(); } -void PictureLayerImpl::TilingLiveRectChanged() { - // SetNeedsPushProperties when the live rect changes, as there are new tiles - // on the tree. This shouldn't imply damage but need to be synced. - SetNeedsPushProperties(); -} - gfx::Size PictureLayerImpl::CalculateTileSize( const gfx::Size& content_bounds) const { int max_texture_size = diff --git a/cc/layers/picture_layer_impl.h b/cc/layers/picture_layer_impl.h index 1163445..4b4816a 100644 --- a/cc/layers/picture_layer_impl.h +++ b/cc/layers/picture_layer_impl.h @@ -73,7 +73,6 @@ class CC_EXPORT PictureLayerImpl TilePriority::PriorityBin GetMaxTilePriorityBin() const override; WhichTree GetTree() const override; bool RequiresHighResToDraw() const override; - void TilingLiveRectChanged() override; void UpdateRasterSource(scoped_refptr<RasterSource> raster_source, Region* new_invalidation, diff --git a/cc/layers/picture_layer_impl_unittest.cc b/cc/layers/picture_layer_impl_unittest.cc index 75470a6..0a95f31 100644 --- a/cc/layers/picture_layer_impl_unittest.cc +++ b/cc/layers/picture_layer_impl_unittest.cc @@ -73,8 +73,7 @@ class PictureLayerImplTest : public testing::Test { : proxy_(base::MessageLoopProxy::current()), host_impl_(LowResTilingsSettings(), &proxy_, &shared_bitmap_manager_), root_id_(6), - parent_id_(7), - id_(8), + id_(7), pending_layer_(nullptr), old_pending_layer_(nullptr), active_layer_(nullptr) { @@ -85,8 +84,7 @@ class PictureLayerImplTest : public testing::Test { : proxy_(base::MessageLoopProxy::current()), host_impl_(settings, &proxy_, &shared_bitmap_manager_), root_id_(6), - parent_id_(7), - id_(8) { + id_(7) { host_impl_.SetViewportSize(gfx::Size(10000, 10000)); } @@ -198,21 +196,17 @@ class PictureLayerImplTest : public testing::Test { // Steal from the recycled tree if possible. scoped_ptr<LayerImpl> pending_root = pending_tree->DetachLayerTree(); - scoped_ptr<LayerImpl> pending_parent; scoped_ptr<FakePictureLayerImpl> pending_layer; DCHECK_IMPLIES(pending_root, pending_root->id() == root_id_); if (!pending_root) { pending_root = LayerImpl::Create(pending_tree, root_id_); - pending_parent = LayerImpl::Create(pending_tree, parent_id_); pending_layer = FakePictureLayerImpl::Create(pending_tree, id_); if (!tile_size.IsEmpty()) pending_layer->set_fixed_tile_size(tile_size); pending_layer->SetDrawsContent(true); } else { - pending_parent = pending_root->RemoveChild(pending_root->children()[0]); pending_layer.reset(static_cast<FakePictureLayerImpl*>( - pending_parent->RemoveChild(pending_parent->children()[0]) - .release())); + pending_root->RemoveChild(pending_root->children()[0]).release())); if (!tile_size.IsEmpty()) pending_layer->set_fixed_tile_size(tile_size); } @@ -222,8 +216,7 @@ class PictureLayerImplTest : public testing::Test { pending_layer->SetContentBounds(raster_source->GetSize()); pending_layer->SetRasterSourceOnPending(raster_source, invalidation); - pending_parent->AddChild(pending_layer.Pass()); - pending_root->AddChild(pending_parent.Pass()); + pending_root->AddChild(pending_layer.Pass()); pending_tree->SetRootLayer(pending_root.Pass()); pending_layer_ = static_cast<FakePictureLayerImpl*>( @@ -313,7 +306,6 @@ class PictureLayerImplTest : public testing::Test { TestSharedBitmapManager shared_bitmap_manager_; FakeLayerTreeHostImpl host_impl_; int root_id_; - int parent_id_; int id_; FakePictureLayerImpl* pending_layer_; FakePictureLayerImpl* old_pending_layer_; @@ -395,155 +387,6 @@ TEST_F(PictureLayerImplTest, CloneNoInvalidation) { VerifyAllTilesExistAndHavePile(tilings->tiling_at(i), pending_pile.get()); } -TEST_F(PictureLayerImplTest, PushPropertiesForNewRasterSource) { - gfx::Size tile_size(100, 100); - gfx::Size layer_bounds(400, 400); - - scoped_refptr<FakePicturePileImpl> filled_pile = - FakePicturePileImpl::CreateFilledPile(tile_size, layer_bounds); - scoped_refptr<FakePicturePileImpl> empty_pile = - FakePicturePileImpl::CreateEmptyPile(tile_size, layer_bounds); - - // A new layer needs to push properties. - SetupPendingTree(filled_pile); - EXPECT_TRUE(pending_layer_->needs_push_properties()); - - host_impl_.ActivateSyncTree(); - - // By default a layer has nothing to push. - host_impl_.CreatePendingTree(); - EXPECT_FALSE(pending_layer_->needs_push_properties()); - - // Setting a new raster source will require pushing. - pending_layer_->SetRasterSourceOnPending(filled_pile, Region()); - EXPECT_TRUE(pending_layer_->needs_push_properties()); - - host_impl_.ActivateSyncTree(); - host_impl_.CreatePendingTree(); - EXPECT_FALSE(pending_layer_->needs_push_properties()); - - // A new source that changes CanHaveTilings also requires pushing. - EXPECT_TRUE(pending_layer_->CanHaveTilings()); - pending_layer_->SetRasterSourceOnPending(empty_pile, Region()); - EXPECT_FALSE(pending_layer_->CanHaveTilings()); - EXPECT_TRUE(pending_layer_->needs_push_properties()); -} - -TEST_F(PictureLayerImplTest, PushPropertiesForNewTiling) { - base::TimeTicks time_ticks; - time_ticks += base::TimeDelta::FromMilliseconds(1); - host_impl_.SetCurrentBeginFrameArgs( - CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, time_ticks)); - - gfx::Size tile_size(100, 100); - gfx::Size layer_bounds(400, 400); - - scoped_refptr<FakePicturePileImpl> filled_pile = - FakePicturePileImpl::CreateFilledPile(tile_size, layer_bounds); - scoped_refptr<FakePicturePileImpl> empty_pile = - FakePicturePileImpl::CreateEmptyPile(tile_size, layer_bounds); - - // A new layer needs to push properties. - SetupPendingTree(filled_pile); - EXPECT_TRUE(pending_layer_->needs_push_properties()); - - host_impl_.ActivateSyncTree(); - - // By default a layer has nothing to push. - host_impl_.CreatePendingTree(); - host_impl_.pending_tree()->PushPageScaleFromMainThread(1.f, 0.25f, 100.f); - EXPECT_FALSE(pending_layer_->needs_push_properties()); - - // Update tiles without changing the scale, shouldn't need to push properties. - time_ticks += base::TimeDelta::FromMilliseconds(200); - host_impl_.SetCurrentBeginFrameArgs( - CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, time_ticks)); - SetupDrawPropertiesAndUpdateTiles(pending_layer_, 1.f, 1.f, 1.f, 1.f, false); - - EXPECT_EQ(1.f, pending_layer_->HighResTiling()->contents_scale()); - EXPECT_FALSE(pending_layer_->needs_push_properties()); - - // Change the scale on the layer, which should make a new high res tiling. - time_ticks += base::TimeDelta::FromMilliseconds(200); - host_impl_.SetCurrentBeginFrameArgs( - CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, time_ticks)); - SetupDrawPropertiesAndUpdateTiles(pending_layer_, 6.f, 1.f, 6.f, 1.f, false); - - EXPECT_EQ(6.f, pending_layer_->HighResTiling()->contents_scale()); - EXPECT_TRUE(pending_layer_->needs_push_properties()); -} - -TEST_F(PictureLayerImplTest, PushPropertiesForNewTiles) { - base::TimeTicks time_ticks; - time_ticks += base::TimeDelta::FromMilliseconds(1); - host_impl_.SetCurrentBeginFrameArgs( - CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, time_ticks)); - - gfx::Size tile_size(100, 100); - gfx::Size layer_bounds(400, 100000); - - host_impl_.SetViewportSize(gfx::Size(100, 100)); - - scoped_refptr<FakePicturePileImpl> filled_pile = - FakePicturePileImpl::CreateFilledPile(tile_size, layer_bounds); - scoped_refptr<FakePicturePileImpl> empty_pile = - FakePicturePileImpl::CreateEmptyPile(tile_size, layer_bounds); - - // A new layer needs to push properties. - SetupPendingTree(filled_pile); - EXPECT_TRUE(pending_layer_->needs_push_properties()); - - host_impl_.ActivateSyncTree(); - - // By default a layer has nothing to push. - host_impl_.CreatePendingTree(); - EXPECT_FALSE(pending_layer_->needs_push_properties()); - - host_impl_.pending_tree()->PushPageScaleFromMainThread(1.f, 0.25f, 100.f); - - int num_tiles_y = - pending_layer_->HighResTiling()->TilingDataForTesting().num_tiles_y(); - - // Verify a bit about current pending tree's current tiles. - EXPECT_EQ(1.f, pending_layer_->HighResTiling()->contents_scale()); - EXPECT_TRUE(pending_layer_->HighResTiling()->TileAt(0, 0)); - EXPECT_FALSE(pending_layer_->HighResTiling()->TileAt(0, num_tiles_y - 1)); - - // Update tiles without changing the viewport, nothing new to push. - time_ticks += base::TimeDelta::FromMilliseconds(1); - host_impl_.SetCurrentBeginFrameArgs( - CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, time_ticks)); - host_impl_.pending_tree()->UpdateDrawProperties(); - - // Same tiles on the pending tree. - EXPECT_TRUE(pending_layer_->HighResTiling()->TileAt(0, 0)); - EXPECT_FALSE(pending_layer_->HighResTiling()->TileAt(0, num_tiles_y - 1)); - // Means nothing new to push. - EXPECT_FALSE(pending_layer_->needs_push_properties()); - - host_impl_.ActivateSyncTree(); - host_impl_.CreatePendingTree(); - EXPECT_FALSE(pending_layer_->needs_push_properties()); - - host_impl_.pending_tree()->PushPageScaleFromMainThread(1.f, 0.25f, 100.f); - - // Change what part of the layer is visible in the viewport and update draw - // properties and tile priorities. This should create new tiles on the layer. - pending_layer_->parent()->SetPosition( - gfx::PointF(0.f, -layer_bounds.height() + 100.f)); - - time_ticks += base::TimeDelta::FromMilliseconds(1); - host_impl_.SetCurrentBeginFrameArgs( - CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, time_ticks)); - host_impl_.pending_tree()->UpdateDrawProperties(); - - // There are new tiles on the pending tree now. - EXPECT_FALSE(pending_layer_->HighResTiling()->TileAt(0, 0)); - EXPECT_TRUE(pending_layer_->HighResTiling()->TileAt(0, num_tiles_y - 1)); - // So the layer needs to push properties to sync the new tiles. - EXPECT_TRUE(pending_layer_->needs_push_properties()); -} - TEST_F(PictureLayerImplTest, ExternalViewportRectForPrioritizingTiles) { base::TimeTicks time_ticks; time_ticks += base::TimeDelta::FromMilliseconds(1); diff --git a/cc/resources/picture_layer_tiling.cc b/cc/resources/picture_layer_tiling.cc index 3ed71d6..033f9f6 100644 --- a/cc/resources/picture_layer_tiling.cc +++ b/cc/resources/picture_layer_tiling.cc @@ -680,7 +680,6 @@ void PictureLayerTiling::SetLiveTilesRect( recycled_twin->live_tiles_rect_ = live_tiles_rect_; recycled_twin->VerifyLiveTilesRect(true); } - client_->TilingLiveRectChanged(); } void PictureLayerTiling::VerifyLiveTilesRect(bool is_on_recycle_tree) const { diff --git a/cc/resources/picture_layer_tiling.h b/cc/resources/picture_layer_tiling.h index 4717573..15fdb43 100644 --- a/cc/resources/picture_layer_tiling.h +++ b/cc/resources/picture_layer_tiling.h @@ -50,10 +50,6 @@ class CC_EXPORT PictureLayerTilingClient { virtual WhichTree GetTree() const = 0; virtual bool RequiresHighResToDraw() const = 0; - // Callback to notify the client that the live rect (set of tiles) changed for - // a tiling. - virtual void TilingLiveRectChanged() = 0; - protected: virtual ~PictureLayerTilingClient() {} }; diff --git a/cc/resources/picture_layer_tiling_set.cc b/cc/resources/picture_layer_tiling_set.cc index 5095863..779e2c0 100644 --- a/cc/resources/picture_layer_tiling_set.cc +++ b/cc/resources/picture_layer_tiling_set.cc @@ -208,6 +208,7 @@ PictureLayerTiling* PictureLayerTilingSet::AddTiling( skewport_target_time_in_seconds_, skewport_extrapolation_limit_in_content_pixels_)); PictureLayerTiling* appended = tilings_.back(); + tilings_.sort(LargestToSmallestScaleFunctor()); return appended; } diff --git a/cc/test/fake_picture_layer_tiling_client.h b/cc/test/fake_picture_layer_tiling_client.h index 762c4d9..e217a78 100644 --- a/cc/test/fake_picture_layer_tiling_client.h +++ b/cc/test/fake_picture_layer_tiling_client.h @@ -36,7 +36,6 @@ class FakePictureLayerTilingClient : public PictureLayerTilingClient { const PictureLayerTiling* tiling) override; bool RequiresHighResToDraw() const override; WhichTree GetTree() const override; - void TilingLiveRectChanged() override {} void set_twin_tiling_set(PictureLayerTilingSet* set) { twin_set_ = set; |