From 62b6fdb8d49e80c053fc6dd38548b933ba668e83 Mon Sep 17 00:00:00 2001 From: "enne@chromium.org" Date: Wed, 19 Mar 2014 05:46:33 +0000 Subject: cc: Replace recorded region with direct map lookup If the viewport is extremely large, then keeping track of the recorded region in PicturePile with an actual Region becomes extremely slow due to a large number of rects being inserted into it. The recorded region behaves as a cache to the picture map; it's a simpler way to know the state of all of the recordings contained within. In practice, the recorded region is only used for two things: a "should this pile bother to create tilings" optimization and a "can a tile be rastered in this content rect" check aka CanRaster. The optimization for "should create tilings" is replaced by a has_any_recordings_ boolean, which could have a false positive in theory (resizing to a smaller but non-empty size), but which shouldn't happen in practice. Even if it did, this would only be a performance penalty for creating no-op tilings that can't create tiles (due to CanRaster). The CanRaster check is replaced by a viewport hint, as most tiles that the tiling creates will be inside of the very large expanded viewport during recording, turning an expensive Region.Contains check to a Rect.Contains one. In the edge cases where tiles are being created outside of that expanded viewport, it will check the picture map directly. This should only happen when the user has scrolled thousands of pixels without a commit. BUG=353346 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=256953 Review URL: https://codereview.chromium.org/196343005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@257852 0039d316-1c4b-4281-b951-d872f2087c98 --- cc/layers/picture_layer.cc | 1 - cc/layers/picture_layer_impl.cc | 10 +-- cc/layers/picture_layer_impl_unittest.cc | 27 ++++---- cc/layers/picture_layer_unittest.cc | 2 +- cc/resources/picture_pile.cc | 20 ++++-- cc/resources/picture_pile_base.cc | 65 ++++++++++++------- cc/resources/picture_pile_base.h | 20 +++--- cc/resources/picture_pile_unittest.cc | 91 +++++++++++++++++++++++++++ cc/resources/prioritized_tile_set_unittest.cc | 2 +- cc/resources/tile_manager_perftest.cc | 2 +- cc/resources/tile_manager_unittest.cc | 2 +- cc/test/fake_picture_layer_tiling_client.cc | 18 +----- cc/test/fake_picture_pile_impl.cc | 29 +++++---- cc/test/fake_picture_pile_impl.h | 12 ++-- 14 files changed, 204 insertions(+), 97 deletions(-) diff --git a/cc/layers/picture_layer.cc b/cc/layers/picture_layer.cc index 6f08b08..3a474d3 100644 --- a/cc/layers/picture_layer.cc +++ b/cc/layers/picture_layer.cc @@ -43,7 +43,6 @@ void PictureLayer::PushPropertiesTo(LayerImpl* base_layer) { // Using layer_impl because either bounds() or paint_properties().bounds // may disagree and either one could have been pushed to layer_impl. pile_->Resize(gfx::Size()); - pile_->UpdateRecordedRegion(); } else if (update_source_frame_number_ == layer_tree_host()->source_frame_number()) { // If update called, then pile size must match bounds pushed to impl layer. diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc index 8ada9aa..9a91524 100644 --- a/cc/layers/picture_layer_impl.cc +++ b/cc/layers/picture_layer_impl.cc @@ -867,8 +867,7 @@ PictureLayerTiling* PictureLayerImpl::AddTiling(float contents_scale) { PictureLayerTiling* tiling = tilings_->AddTiling(contents_scale); - const Region& recorded = pile_->recorded_region(); - DCHECK(!recorded.IsEmpty()); + DCHECK(pile_->HasRecordings()); if (twin_layer_ && twin_layer_->ShouldUseGpuRasterization() == ShouldUseGpuRasterization()) @@ -1180,7 +1179,7 @@ void PictureLayerImpl::ResetRasterScale() { bool PictureLayerImpl::CanHaveTilings() const { if (!DrawsContent()) return false; - if (pile_->recorded_region().IsEmpty()) + if (!pile_->HasRecordings()) return false; return true; } @@ -1224,11 +1223,6 @@ void PictureLayerImpl::AsValueInto(base::DictionaryValue* state) const { state->Set("pictures", pile_->AsValue().release()); state->Set("invalidation", invalidation_.AsValue().release()); - Region unrecorded_region(gfx::Rect(pile_->size())); - unrecorded_region.Subtract(pile_->recorded_region()); - if (!unrecorded_region.IsEmpty()) - state->Set("unrecorded_region", unrecorded_region.AsValue().release()); - scoped_ptr coverage_tiles(new base::ListValue); for (PictureLayerTilingSet::CoverageIterator iter(tilings_.get(), contents_scale_x(), diff --git a/cc/layers/picture_layer_impl_unittest.cc b/cc/layers/picture_layer_impl_unittest.cc index 59bcfd9..e27a0e6 100644 --- a/cc/layers/picture_layer_impl_unittest.cc +++ b/cc/layers/picture_layer_impl_unittest.cc @@ -489,12 +489,10 @@ TEST_F(PictureLayerImplTest, AddTilesFromNewRecording) { ++iter) { EXPECT_FALSE(iter.full_tile_geometry_rect().IsEmpty()); // Ensure there is a recording for this tile. - gfx::Rect layer_rect = gfx::ScaleToEnclosingRect( - iter.full_tile_geometry_rect(), 1.f / tiling->contents_scale()); - layer_rect.Intersect(gfx::Rect(layer_bounds)); - - bool in_pending = pending_pile->recorded_region().Contains(layer_rect); - bool in_active = active_pile->recorded_region().Contains(layer_rect); + bool in_pending = pending_pile->CanRaster(tiling->contents_scale(), + iter.full_tile_geometry_rect()); + bool in_active = active_pile->CanRaster(tiling->contents_scale(), + iter.full_tile_geometry_rect()); if (in_pending && !in_active) EXPECT_EQ(pending_pile, iter->picture_pile()); @@ -634,7 +632,7 @@ TEST_F(PictureLayerImplTest, CreateTilingsEvenIfTwinHasNone) { gfx::Size layer_bounds(1300, 1900); scoped_refptr empty_pile = - FakePicturePileImpl::CreateFilledPile(tile_size, gfx::Size(1000, 0)); + FakePicturePileImpl::CreateEmptyPile(tile_size, layer_bounds); scoped_refptr valid_pile = FakePicturePileImpl::CreateFilledPile(tile_size, layer_bounds); @@ -1321,13 +1319,13 @@ TEST_F(PictureLayerImplTest, NothingRequiredIfActiveMissingTiles) { gfx::Size tile_size(100, 100); scoped_refptr pending_pile = FakePicturePileImpl::CreateFilledPile(tile_size, layer_bounds); - // An arbitrary bogus outside the layer recording. Enough for the layer to - // think it can create tiles, but not in bounds so all tiles are null. - Region active_recorded_region; - active_recorded_region.Union(gfx::Rect(1000, 1000, 1, 1)); + // This pile will create tilings, but has no recordings so will not create any + // tiles. This is attempting to simulate scrolling past the end of recorded + // content on the active layer, where the recordings are so far away that + // no tiles are created. scoped_refptr active_pile = - FakePicturePileImpl::CreatePileWithRecordedRegion( - tile_size, layer_bounds, active_recorded_region); + FakePicturePileImpl::CreateEmptyPileThatThinksItHasRecordings( + tile_size, layer_bounds); SetupTrees(pending_pile, active_pile); pending_layer_->set_fixed_tile_size(tile_size); active_layer_->set_fixed_tile_size(tile_size); @@ -1340,8 +1338,7 @@ TEST_F(PictureLayerImplTest, NothingRequiredIfActiveMissingTiles) { EXPECT_EQ(active_layer_->HighResTiling()->AllTilesForTesting().size(), 0u); // Since the active layer has no tiles at all, the pending layer doesn't - // need content in order to activate. This is attempting to simulate - // scrolling past the end of recorded content on the active layer. + // need content in order to activate. pending_layer_->MarkVisibleResourcesAsRequired(); AssertNoTilesRequired(pending_layer_->HighResTiling()); AssertNoTilesRequired(pending_layer_->LowResTiling()); diff --git a/cc/layers/picture_layer_unittest.cc b/cc/layers/picture_layer_unittest.cc index 1353016..71973b2 100644 --- a/cc/layers/picture_layer_unittest.cc +++ b/cc/layers/picture_layer_unittest.cc @@ -58,7 +58,7 @@ TEST(PictureLayerTest, NoTilesIfEmptyBounds) { EXPECT_FALSE(layer_impl->CanHaveTilings()); EXPECT_TRUE(layer_impl->bounds() == gfx::Size(0, 0)); EXPECT_TRUE(layer_impl->pile()->size() == gfx::Size(0, 0)); - EXPECT_TRUE(layer_impl->pile()->recorded_region().IsEmpty()); + EXPECT_FALSE(layer_impl->pile()->HasRecordings()); } } diff --git a/cc/resources/picture_pile.cc b/cc/resources/picture_pile.cc index 35d026f..e82a23e 100644 --- a/cc/resources/picture_pile.cc +++ b/cc/resources/picture_pile.cc @@ -164,6 +164,8 @@ bool PicturePile::Update( -kPixelDistanceToRecord, -kPixelDistanceToRecord, -kPixelDistanceToRecord); + recorded_viewport_ = interest_rect; + recorded_viewport_.Intersect(gfx::Rect(size())); bool invalidated = false; for (Region::Iterator i(invalidation); i.has_rect(); i.next()) { @@ -201,17 +203,20 @@ bool PicturePile::Update( if (info.NeedsRecording(frame_number, distance_to_visible)) { gfx::Rect tile = tiling_.TileBounds(key.first, key.second); invalid_tiles.push_back(tile); + } else if (!info.GetPicture() && recorded_viewport_.Intersects(rect)) { + // Recorded viewport is just an optimization for a fully recorded + // interest rect. In this case, a tile in that rect has declined + // to be recorded (probably due to frequent invalidations). + // TODO(enne): Shrink the recorded_viewport_ rather than clearing. + recorded_viewport_ = gfx::Rect(); } } std::vector record_rects; ClusterTiles(invalid_tiles, &record_rects); - if (record_rects.empty()) { - if (invalidated) - UpdateRecordedRegion(); + if (record_rects.empty()) return invalidated; - } for (std::vector::iterator it = record_rects.begin(); it != record_rects.end(); @@ -247,6 +252,8 @@ bool PicturePile::Update( stats_instrumentation->AddRecord(best_duration, recorded_pixel_count); } + bool found_tile_for_recorded_picture = false; + bool include_borders = true; for (TilingData::Iterator it(&tiling_, record_rect, include_borders); it; ++it) { @@ -255,11 +262,14 @@ bool PicturePile::Update( if (record_rect.Contains(tile)) { PictureInfo& info = picture_map_[key]; info.SetPicture(picture); + found_tile_for_recorded_picture = true; } } + DCHECK(found_tile_for_recorded_picture); } - UpdateRecordedRegion(); + has_any_recordings_ = true; + DCHECK(CanRasterSlowTileCheck(recorded_viewport_)); return true; } diff --git a/cc/resources/picture_pile_base.cc b/cc/resources/picture_pile_base.cc index 73f2cd7..ee2f519 100644 --- a/cc/resources/picture_pile_base.cc +++ b/cc/resources/picture_pile_base.cc @@ -44,7 +44,8 @@ PicturePileBase::PicturePileBase() slow_down_raster_scale_factor_for_debug_(0), contents_opaque_(false), show_debug_picture_borders_(false), - clear_canvas_with_debug_color_(kDefaultClearCanvasSetting) { + clear_canvas_with_debug_color_(kDefaultClearCanvasSetting), + has_any_recordings_(false) { tiling_.SetMaxTextureSize(gfx::Size(kBasePictureSize, kBasePictureSize)); tile_grid_info_.fTileInterval.setEmpty(); tile_grid_info_.fMargin.setEmpty(); @@ -54,7 +55,7 @@ PicturePileBase::PicturePileBase() PicturePileBase::PicturePileBase(const PicturePileBase* other) : picture_map_(other->picture_map_), tiling_(other->tiling_), - recorded_region_(other->recorded_region_), + recorded_viewport_(other->recorded_viewport_), min_contents_scale_(other->min_contents_scale_), tile_grid_info_(other->tile_grid_info_), background_color_(other->background_color_), @@ -62,13 +63,13 @@ PicturePileBase::PicturePileBase(const PicturePileBase* other) other->slow_down_raster_scale_factor_for_debug_), contents_opaque_(other->contents_opaque_), show_debug_picture_borders_(other->show_debug_picture_borders_), - clear_canvas_with_debug_color_(other->clear_canvas_with_debug_color_) { -} + clear_canvas_with_debug_color_(other->clear_canvas_with_debug_color_), + has_any_recordings_(other->has_any_recordings_) {} -PicturePileBase::PicturePileBase( - const PicturePileBase* other, unsigned thread_index) +PicturePileBase::PicturePileBase(const PicturePileBase* other, + unsigned thread_index) : tiling_(other->tiling_), - recorded_region_(other->recorded_region_), + recorded_viewport_(other->recorded_viewport_), min_contents_scale_(other->min_contents_scale_), tile_grid_info_(other->tile_grid_info_), background_color_(other->background_color_), @@ -76,7 +77,8 @@ PicturePileBase::PicturePileBase( other->slow_down_raster_scale_factor_for_debug_), contents_opaque_(other->contents_opaque_), show_debug_picture_borders_(other->show_debug_picture_borders_), - clear_canvas_with_debug_color_(other->clear_canvas_with_debug_color_) { + clear_canvas_with_debug_color_(other->clear_canvas_with_debug_color_), + has_any_recordings_(other->has_any_recordings_) { for (PictureMap::const_iterator it = other->picture_map_.begin(); it != other->picture_map_.end(); ++it) { @@ -94,6 +96,8 @@ void PicturePileBase::Resize(const gfx::Size& new_size) { gfx::Size old_size = size(); tiling_.SetTotalSize(new_size); + has_any_recordings_ = false; + // Find all tiles that contain any pixels outside the new size. std::vector to_erase; int min_toss_x = tiling_.FirstBorderTileXIndexFromSrcCoord( @@ -104,13 +108,18 @@ void PicturePileBase::Resize(const gfx::Size& new_size) { it != picture_map_.end(); ++it) { const PictureMapKey& key = it->first; - if (key.first < min_toss_x && key.second < min_toss_y) + if (key.first < min_toss_x && key.second < min_toss_y) { + has_any_recordings_ |= !!it->second.GetPicture(); continue; + } to_erase.push_back(key); } for (size_t i = 0; i < to_erase.size(); ++i) picture_map_.erase(to_erase[i]); + + // Don't waste time in Resize figuring out what these hints should be. + recorded_viewport_ = gfx::Rect(); } void PicturePileBase::SetMinContentsScale(float min_contents_scale) { @@ -164,18 +173,7 @@ void PicturePileBase::SetBufferPixels(int new_buffer_pixels) { void PicturePileBase::Clear() { picture_map_.clear(); -} - -void PicturePileBase::UpdateRecordedRegion() { - recorded_region_.Clear(); - for (PictureMap::const_iterator it = picture_map_.begin(); - it != picture_map_.end(); - ++it) { - if (it->second.GetPicture()) { - const PictureMapKey& key = it->first; - recorded_region_.Union(tile_bounds(key.first, key.second)); - } - } + recorded_viewport_ = gfx::Rect(); } bool PicturePileBase::HasRecordingAt(int x, int y) { @@ -192,7 +190,30 @@ bool PicturePileBase::CanRaster(float contents_scale, gfx::Rect layer_rect = gfx::ScaleToEnclosingRect( content_rect, 1.f / contents_scale); layer_rect.Intersect(gfx::Rect(tiling_.total_size())); - return recorded_region_.Contains(layer_rect); + + // Common case inside of viewport to avoid the slower map lookups. + if (recorded_viewport_.Contains(layer_rect)) { + // Sanity check that there are no false positives in recorded_viewport_. + DCHECK(CanRasterSlowTileCheck(layer_rect)); + return true; + } + + return CanRasterSlowTileCheck(layer_rect); +} + +bool PicturePileBase::CanRasterSlowTileCheck( + const gfx::Rect& layer_rect) const { + bool include_borders = false; + for (TilingData::Iterator tile_iter(&tiling_, layer_rect, include_borders); + tile_iter; + ++tile_iter) { + PictureMap::const_iterator map_iter = picture_map_.find(tile_iter.index()); + if (map_iter == picture_map_.end()) + return false; + if (!map_iter->second.GetPicture()) + return false; + } + return true; } gfx::Rect PicturePileBase::PaddedRect(const PictureMapKey& key) { diff --git a/cc/resources/picture_pile_base.h b/cc/resources/picture_pile_base.h index 26f6bdb..afdf1d9 100644 --- a/cc/resources/picture_pile_base.h +++ b/cc/resources/picture_pile_base.h @@ -33,15 +33,15 @@ class CC_EXPORT PicturePileBase : public base::RefCounted { gfx::Size size() const { return tiling_.total_size(); } void SetMinContentsScale(float min_contents_scale); - void UpdateRecordedRegion(); - const Region& recorded_region() const { return recorded_region_; } - int num_tiles_x() const { return tiling_.num_tiles_x(); } int num_tiles_y() const { return tiling_.num_tiles_y(); } gfx::Rect tile_bounds(int x, int y) const { return tiling_.TileBounds(x, y); } bool HasRecordingAt(int x, int y); bool CanRaster(float contents_scale, const gfx::Rect& content_rect); + // If this pile contains any valid recordings. May have false positives. + bool HasRecordings() const { return has_any_recordings_; } + static void ComputeTileGridInfo(const gfx::Size& tile_grid_size, SkTileGridPicture::TileGridInfo* info); @@ -84,21 +84,22 @@ class CC_EXPORT PicturePileBase : public base::RefCounted { virtual ~PicturePileBase(); - void SetRecordedRegionForTesting(const Region& recorded_region) { - recorded_region_ = recorded_region; - } - int buffer_pixels() const { return tiling_.border_texels(); } void Clear(); gfx::Rect PaddedRect(const PictureMapKey& key); gfx::Rect PadRect(const gfx::Rect& rect); + // An internal CanRaster check that goes to the picture_map rather than + // using the recorded_viewport hint. + bool CanRasterSlowTileCheck(const gfx::Rect& layer_rect) const; + // A picture pile is a tiled set of pictures. The picture map is a map of tile // indices to picture infos. PictureMap picture_map_; TilingData tiling_; - Region recorded_region_; + // If non-empty, all pictures tiles inside this rect are recorded. + gfx::Rect recorded_viewport_; float min_contents_scale_; SkTileGridPicture::TileGridInfo tile_grid_info_; SkColor background_color_; @@ -106,6 +107,9 @@ class CC_EXPORT PicturePileBase : public base::RefCounted { bool contents_opaque_; bool show_debug_picture_borders_; bool clear_canvas_with_debug_color_; + // A hint about whether there are any recordings. This may be a false + // positive. + bool has_any_recordings_; private: void SetBufferPixels(int buffer_pixels); diff --git a/cc/resources/picture_pile_unittest.cc b/cc/resources/picture_pile_unittest.cc index d87f928..5c1a57ad 100644 --- a/cc/resources/picture_pile_unittest.cc +++ b/cc/resources/picture_pile_unittest.cc @@ -18,8 +18,15 @@ namespace { class TestPicturePile : public PicturePile { public: using PicturePile::buffer_pixels; + using PicturePile::CanRasterSlowTileCheck; + using PicturePile::Clear; PictureMap& picture_map() { return picture_map_; } + const gfx::Rect& recorded_viewport() const { return recorded_viewport_; } + + bool CanRasterLayerRect(const gfx::Rect& layer_rect) { + return CanRaster(1.f, layer_rect); + } typedef PicturePile::PictureInfo PictureInfo; typedef PicturePile::PictureMapKey PictureMapKey; @@ -229,5 +236,89 @@ TEST_F(PicturePileTest, StopRecordingOffscreenInvalidations) { } } +TEST_F(PicturePileTest, ClearingInvalidatesRecordedRect) { + UpdateWholeLayer(); + + gfx::Rect rect(0, 0, 5, 5); + EXPECT_TRUE(pile_->CanRasterLayerRect(rect)); + EXPECT_TRUE(pile_->CanRasterSlowTileCheck(rect)); + + pile_->Clear(); + + // Make sure both the cache-aware check (using recorded region) and the normal + // check are both false after clearing. + EXPECT_FALSE(pile_->CanRasterLayerRect(rect)); + EXPECT_FALSE(pile_->CanRasterSlowTileCheck(rect)); +} + +TEST_F(PicturePileTest, FrequentInvalidationCanRaster) { + // This test makes sure that if part of the page is frequently invalidated + // and doesn't get re-recorded, then CanRaster is not true for any + // tiles touching it, but is true for adjacent tiles, even if it + // overlaps on borders (edge case). + gfx::Size layer_size = gfx::ToFlooredSize(gfx::ScaleSize(pile_->size(), 4.f)); + pile_->Resize(layer_size); + + gfx::Rect tile01_borders = pile_->tiling().TileBoundsWithBorder(0, 1); + gfx::Rect tile02_borders = pile_->tiling().TileBoundsWithBorder(0, 2); + gfx::Rect tile01_noborders = pile_->tiling().TileBounds(0, 1); + gfx::Rect tile02_noborders = pile_->tiling().TileBounds(0, 2); + + // Sanity check these two tiles are overlapping with borders, since this is + // what the test is trying to repro. + EXPECT_TRUE(tile01_borders.Intersects(tile02_borders)); + EXPECT_FALSE(tile01_noborders.Intersects(tile02_noborders)); + UpdateWholeLayer(); + EXPECT_TRUE(pile_->CanRasterLayerRect(tile01_noborders)); + EXPECT_TRUE(pile_->CanRasterSlowTileCheck(tile01_noborders)); + EXPECT_TRUE(pile_->CanRasterLayerRect(tile02_noborders)); + EXPECT_TRUE(pile_->CanRasterSlowTileCheck(tile02_noborders)); + // Sanity check that an initial paint goes down the fast path of having + // a valid recorded viewport. + EXPECT_TRUE(!pile_->recorded_viewport().IsEmpty()); + + // Update the whole layer until the invalidation frequency is high. + for (int frame = 0; frame < 33; ++frame) { + UpdateWholeLayer(); + } + + // Update once more with a small viewport. + gfx::Rect viewport(0, 0, layer_size.width(), 1); + Update(layer_rect(), viewport); + + // Sanity check some pictures exist and others don't. + EXPECT_TRUE(pile_->picture_map() + .find(TestPicturePile::PictureMapKey(0, 1)) + ->second.GetPicture()); + EXPECT_FALSE(pile_->picture_map() + .find(TestPicturePile::PictureMapKey(0, 2)) + ->second.GetPicture()); + + EXPECT_TRUE(pile_->CanRasterLayerRect(tile01_noborders)); + EXPECT_TRUE(pile_->CanRasterSlowTileCheck(tile01_noborders)); + EXPECT_FALSE(pile_->CanRasterLayerRect(tile02_noborders)); + EXPECT_FALSE(pile_->CanRasterSlowTileCheck(tile02_noborders)); +} + +TEST_F(PicturePileTest, NoInvalidationValidViewport) { + // This test validates that the recorded_viewport cache of full tiles + // is still valid for some use cases. If it's not, it's a performance + // issue because CanRaster checks will go down the slow path. + UpdateWholeLayer(); + EXPECT_TRUE(!pile_->recorded_viewport().IsEmpty()); + + // No invalidation, same viewport. + Update(gfx::Rect(), layer_rect()); + EXPECT_TRUE(!pile_->recorded_viewport().IsEmpty()); + + // Partial invalidation, same viewport. + Update(gfx::Rect(gfx::Rect(0, 0, 1, 1)), layer_rect()); + EXPECT_TRUE(!pile_->recorded_viewport().IsEmpty()); + + // No invalidation, changing viewport. + Update(gfx::Rect(), gfx::Rect(5, 5, 5, 5)); + EXPECT_TRUE(!pile_->recorded_viewport().IsEmpty()); +} + } // namespace } // namespace cc diff --git a/cc/resources/prioritized_tile_set_unittest.cc b/cc/resources/prioritized_tile_set_unittest.cc index a74cc6d..3f20dbe 100644 --- a/cc/resources/prioritized_tile_set_unittest.cc +++ b/cc/resources/prioritized_tile_set_unittest.cc @@ -61,7 +61,7 @@ class PrioritizedTileSetTest : public testing::Test { 1).Pass(); tile_manager_.reset( new FakeTileManager(&tile_manager_client_, resource_provider_.get())); - picture_pile_ = FakePicturePileImpl::CreatePile(); + picture_pile_ = FakePicturePileImpl::CreateInfiniteFilledPile(); } scoped_refptr CreateTile() { diff --git a/cc/resources/tile_manager_perftest.cc b/cc/resources/tile_manager_perftest.cc index 4a85b6a..d2a8ccd 100644 --- a/cc/resources/tile_manager_perftest.cc +++ b/cc/resources/tile_manager_perftest.cc @@ -46,7 +46,7 @@ class TileManagerPerfTest : public testing::Test { make_scoped_ptr(new FakeTileManager(&tile_manager_client_, resource_provider_.get(), raster_task_limit_bytes)); - picture_pile_ = FakePicturePileImpl::CreatePile(); + picture_pile_ = FakePicturePileImpl::CreateInfiniteFilledPile(); } GlobalStateThatImpactsTilePriority GlobalStateForTest() { diff --git a/cc/resources/tile_manager_unittest.cc b/cc/resources/tile_manager_unittest.cc index 70260e3..2493312 100644 --- a/cc/resources/tile_manager_unittest.cc +++ b/cc/resources/tile_manager_unittest.cc @@ -38,7 +38,7 @@ class TileManagerTest : public testing::TestWithParam, memory_limit_policy_ = memory_limit_policy; max_tiles_ = max_tiles; - picture_pile_ = FakePicturePileImpl::CreatePile(); + picture_pile_ = FakePicturePileImpl::CreateInfiniteFilledPile(); SetTreePriority(tree_priority); } diff --git a/cc/test/fake_picture_layer_tiling_client.cc b/cc/test/fake_picture_layer_tiling_client.cc index 36d1eee..4289b9d4 100644 --- a/cc/test/fake_picture_layer_tiling_client.cc +++ b/cc/test/fake_picture_layer_tiling_client.cc @@ -6,26 +6,14 @@ #include +#include "cc/test/fake_picture_pile_impl.h" #include "cc/test/fake_tile_manager.h" namespace cc { -class FakeInfinitePicturePileImpl : public PicturePileImpl { - public: - FakeInfinitePicturePileImpl() { - gfx::Size size(std::numeric_limits::max(), - std::numeric_limits::max()); - Resize(size); - recorded_region_ = Region(gfx::Rect(size)); - } - - protected: - virtual ~FakeInfinitePicturePileImpl() {} -}; - FakePictureLayerTilingClient::FakePictureLayerTilingClient() : tile_manager_(new FakeTileManager(&tile_manager_client_)), - pile_(new FakeInfinitePicturePileImpl()), + pile_(FakePicturePileImpl::CreateInfiniteFilledPile()), twin_tiling_(NULL), allow_create_tile_(true), max_tiles_for_interest_area_(10000), @@ -36,7 +24,7 @@ FakePictureLayerTilingClient::FakePictureLayerTilingClient( ResourceProvider* resource_provider) : tile_manager_( new FakeTileManager(&tile_manager_client_, resource_provider)), - pile_(new FakeInfinitePicturePileImpl()), + pile_(FakePicturePileImpl::CreateInfiniteFilledPile()), twin_tiling_(NULL), allow_create_tile_(true), max_tiles_for_interest_area_(10000), diff --git a/cc/test/fake_picture_pile_impl.cc b/cc/test/fake_picture_pile_impl.cc index bb60034..4bcf4b0 100644 --- a/cc/test/fake_picture_pile_impl.cc +++ b/cc/test/fake_picture_pile_impl.cc @@ -23,11 +23,12 @@ scoped_refptr FakePicturePileImpl::CreateFilledPile( pile->tiling().SetTotalSize(layer_bounds); pile->tiling().SetMaxTextureSize(tile_size); pile->SetTileGridSize(ImplSidePaintingSettings().default_tile_size); + pile->recorded_viewport_ = gfx::Rect(layer_bounds); + pile->has_any_recordings_ = true; for (int x = 0; x < pile->tiling().num_tiles_x(); ++x) { for (int y = 0; y < pile->tiling().num_tiles_y(); ++y) pile->AddRecordingAt(x, y); } - pile->UpdateRecordedRegion(); return pile; } @@ -38,29 +39,37 @@ scoped_refptr FakePicturePileImpl::CreateEmptyPile( pile->tiling().SetTotalSize(layer_bounds); pile->tiling().SetMaxTextureSize(tile_size); pile->SetTileGridSize(ImplSidePaintingSettings().default_tile_size); - pile->UpdateRecordedRegion(); + pile->recorded_viewport_ = gfx::Rect(); + pile->has_any_recordings_ = false; return pile; } scoped_refptr -FakePicturePileImpl::CreatePileWithRecordedRegion( +FakePicturePileImpl::CreateEmptyPileThatThinksItHasRecordings( const gfx::Size& tile_size, - const gfx::Size& layer_bounds, - const Region& recorded_region) { + const gfx::Size& layer_bounds) { scoped_refptr pile(new FakePicturePileImpl()); pile->tiling().SetTotalSize(layer_bounds); pile->tiling().SetMaxTextureSize(tile_size); pile->SetTileGridSize(ImplSidePaintingSettings().default_tile_size); - pile->SetRecordedRegionForTesting(recorded_region); + // This simulates a false positive for this flag. + pile->recorded_viewport_ = gfx::Rect(); + pile->has_any_recordings_ = true; return pile; } -scoped_refptr FakePicturePileImpl::CreatePile() { +scoped_refptr +FakePicturePileImpl::CreateInfiniteFilledPile() { scoped_refptr pile(new FakePicturePileImpl()); gfx::Size size(std::numeric_limits::max(), std::numeric_limits::max()); pile->Resize(size); - pile->recorded_region_ = Region(gfx::Rect(size)); + pile->tiling().SetTotalSize(size); + pile->tiling().SetMaxTextureSize(size); + pile->SetTileGridSize(size); + pile->recorded_viewport_ = gfx::Rect(size); + pile->has_any_recordings_ = true; + pile->AddRecordingAt(0, 0); return pile; } @@ -80,7 +89,7 @@ void FakePicturePileImpl::AddRecordingAt(int x, int y) { picture_map_[std::pair(x, y)].SetPicture(picture); EXPECT_TRUE(HasRecordingAt(x, y)); - UpdateRecordedRegion(); + has_any_recordings_ = true; } void FakePicturePileImpl::RemoveRecordingAt(int x, int y) { @@ -93,8 +102,6 @@ void FakePicturePileImpl::RemoveRecordingAt(int x, int y) { return; picture_map_.erase(std::pair(x, y)); EXPECT_FALSE(HasRecordingAt(x, y)); - - UpdateRecordedRegion(); } void FakePicturePileImpl::RerecordPile() { diff --git a/cc/test/fake_picture_pile_impl.h b/cc/test/fake_picture_pile_impl.h index d4ab8e0..6bf1b88 100644 --- a/cc/test/fake_picture_pile_impl.h +++ b/cc/test/fake_picture_pile_impl.h @@ -16,17 +16,13 @@ class FakePicturePileImpl : public PicturePileImpl { static scoped_refptr CreateFilledPile( const gfx::Size& tile_size, const gfx::Size& layer_bounds); - static scoped_refptr CreateEmptyPile( const gfx::Size& tile_size, const gfx::Size& layer_bounds); - - static scoped_refptr CreatePileWithRecordedRegion( - const gfx::Size& tile_size, - const gfx::Size& layer_bounds, - const Region& recorded_region); - - static scoped_refptr CreatePile(); + static scoped_refptr + CreateEmptyPileThatThinksItHasRecordings(const gfx::Size& tile_size, + const gfx::Size& layer_bounds); + static scoped_refptr CreateInfiniteFilledPile(); TilingData& tiling() { return tiling_; } -- cgit v1.1