Skip to content

Commit

Permalink
cc: Remove PicturePileBase as a base class of PicturePileImpl.
Browse files Browse the repository at this point in the history
This patch removes PicturePileBase from being a base class of
PicturePileImpl. This is meant to serve as a first step in a cleanup
that eliminates PicturePileBase and instead provides an explicit
passing of values from PicturePile to PicturePileImpl. This, in turn,
would hopefully expose some opportunities for cleanups, since it feels
that PicturePileBase has become a bit of a catch-all for passing
settings from PictureLayer to PictureLayerImpl/RasterWorkerPool.

R=danakj
BUG=430260

Review URL: https://codereview.chromium.org/689423003

Cr-Commit-Position: refs/heads/master@{#303158}
  • Loading branch information
vmpstr authored and Commit bot committed Nov 7, 2014
1 parent 5a8f0ab commit acd5661
Show file tree
Hide file tree
Showing 12 changed files with 294 additions and 202 deletions.
2 changes: 1 addition & 1 deletion cc/layers/picture_layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ skia::RefPtr<SkPicture> PictureLayerImpl::GetPicture() {
scoped_refptr<Tile> PictureLayerImpl::CreateTile(PictureLayerTiling* tiling,
const gfx::Rect& content_rect) {
DCHECK(!pile_->is_solid_color());
if (!pile_->CanRaster(tiling->contents_scale(), content_rect))
if (!pile_->CoversRect(content_rect, tiling->contents_scale()))
return scoped_refptr<Tile>();

int flags = 0;
Expand Down
31 changes: 19 additions & 12 deletions cc/layers/picture_layer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -658,10 +658,10 @@ TEST_F(PictureLayerImplTest, AddTilesFromNewRecording) {
++iter) {
EXPECT_FALSE(iter.full_tile_geometry_rect().IsEmpty());
// Ensure there is a recording for this tile.
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());
bool in_pending = pending_pile->CoversRect(iter.full_tile_geometry_rect(),
tiling->contents_scale());
bool in_active = active_pile->CoversRect(iter.full_tile_geometry_rect(),
tiling->contents_scale());

if (in_pending && !in_active)
EXPECT_EQ(pending_pile.get(), iter->raster_source());
Expand Down Expand Up @@ -1132,8 +1132,15 @@ TEST_F(PictureLayerImplTest, DontAddLowResDuringAnimation) {
}

TEST_F(PictureLayerImplTest, DontAddLowResForSmallLayers) {
gfx::Size tile_size(host_impl_.settings().default_tile_size);
SetupDefaultTrees(tile_size);
gfx::Size layer_bounds(host_impl_.settings().default_tile_size);
gfx::Size tile_size(100, 100);

scoped_refptr<FakePicturePileImpl> pending_pile =
FakePicturePileImpl::CreateFilledPile(tile_size, layer_bounds);
scoped_refptr<FakePicturePileImpl> active_pile =
FakePicturePileImpl::CreateFilledPile(tile_size, layer_bounds);

SetupTrees(pending_pile, active_pile);

float low_res_factor = host_impl_.settings().low_res_contents_scale_factor;
float device_scale = 1.f;
Expand Down Expand Up @@ -1181,8 +1188,8 @@ TEST_F(PictureLayerImplTest, DontAddLowResForSmallLayers) {
ResetTilingsAndRasterScales();

// Mask layers dont create low res since they always fit on one tile.
pending_layer_->pile()->set_is_mask(true);
active_layer_->pile()->set_is_mask(true);
pending_pile->SetIsMask(true);
active_pile->SetIsMask(true);
SetContentsScaleOnBothLayers(contents_scale,
device_scale,
page_scale,
Expand All @@ -1197,7 +1204,7 @@ TEST_F(PictureLayerImplTest, HugeMasksDontGetTiles) {

scoped_refptr<FakePicturePileImpl> valid_pile =
FakePicturePileImpl::CreateFilledPile(tile_size, gfx::Size(1000, 1000));
valid_pile->set_is_mask(true);
valid_pile->SetIsMask(true);
SetupPendingTree(valid_pile);

SetupDrawPropertiesAndUpdateTiles(pending_layer_, 1.f, 1.f, 1.f, 1.f, false);
Expand All @@ -1224,7 +1231,7 @@ TEST_F(PictureLayerImplTest, HugeMasksDontGetTiles) {
scoped_refptr<FakePicturePileImpl> huge_pile =
FakePicturePileImpl::CreateFilledPile(
tile_size, gfx::Size(max_texture_size + 1, 10));
huge_pile->set_is_mask(true);
huge_pile->SetIsMask(true);
SetupPendingTree(huge_pile);

SetupDrawPropertiesAndUpdateTiles(pending_layer_, 1.f, 1.f, 1.f, 1.f, false);
Expand All @@ -1249,7 +1256,7 @@ TEST_F(PictureLayerImplTest, ScaledMaskLayer) {

scoped_refptr<FakePicturePileImpl> valid_pile =
FakePicturePileImpl::CreateFilledPile(tile_size, gfx::Size(1000, 1000));
valid_pile->set_is_mask(true);
valid_pile->SetIsMask(true);
SetupPendingTree(valid_pile);

float ideal_contents_scale = 1.3f;
Expand Down Expand Up @@ -3708,7 +3715,7 @@ TEST_F(PictureLayerImplTest, UpdateTilesForMasksWithNoVisibleContent) {

scoped_refptr<FakePicturePileImpl> pending_pile =
FakePicturePileImpl::CreateFilledPile(tile_size, bounds);
pending_pile->set_is_mask(true);
pending_pile->SetIsMask(true);
scoped_ptr<FakePictureLayerImpl> mask = FakePictureLayerImpl::CreateWithPile(
host_impl_.pending_tree(), 3, pending_pile);

Expand Down
118 changes: 30 additions & 88 deletions cc/output/renderer_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1759,14 +1759,9 @@ TYPED_TEST(RendererPixelTest, PictureDrawQuadIdentityScale) {

blue_quad->SetNew(blue_shared_state,
viewport, // Intentionally bigger than clip.
gfx::Rect(),
viewport,
gfx::RectF(viewport),
viewport.size(),
texture_format,
viewport,
1.f,
PicturePileImpl::CreateFromOther(blue_pile.get()));
gfx::Rect(), viewport, gfx::RectF(viewport),
viewport.size(), texture_format, viewport, 1.f,
blue_pile.get());

// One viewport-filling green quad.
scoped_refptr<FakePicturePileImpl> green_pile =
Expand All @@ -1782,16 +1777,9 @@ TYPED_TEST(RendererPixelTest, PictureDrawQuadIdentityScale) {

PictureDrawQuad* green_quad =
pass->CreateAndAppendDrawQuad<PictureDrawQuad>();
green_quad->SetNew(green_shared_state,
viewport,
gfx::Rect(),
viewport,
gfx::RectF(0.f, 0.f, 1.f, 1.f),
viewport.size(),
texture_format,
viewport,
1.f,
PicturePileImpl::CreateFromOther(green_pile.get()));
green_quad->SetNew(green_shared_state, viewport, gfx::Rect(), viewport,
gfx::RectF(0.f, 0.f, 1.f, 1.f), viewport.size(),
texture_format, viewport, 1.f, green_pile.get());

RenderPassList pass_list;
pass_list.push_back(pass.Pass());
Expand Down Expand Up @@ -1828,16 +1816,9 @@ TYPED_TEST(RendererPixelTest, PictureDrawQuadOpacity) {

PictureDrawQuad* green_quad =
pass->CreateAndAppendDrawQuad<PictureDrawQuad>();
green_quad->SetNew(green_shared_state,
viewport,
gfx::Rect(),
viewport,
gfx::RectF(0, 0, 1, 1),
viewport.size(),
texture_format,
viewport,
1.f,
PicturePileImpl::CreateFromOther(green_pile.get()));
green_quad->SetNew(green_shared_state, viewport, gfx::Rect(), viewport,
gfx::RectF(0, 0, 1, 1), viewport.size(), texture_format,
viewport, 1.f, green_pile.get());

// One viewport-filling white quad.
scoped_refptr<FakePicturePileImpl> white_pile =
Expand All @@ -1853,16 +1834,9 @@ TYPED_TEST(RendererPixelTest, PictureDrawQuadOpacity) {

PictureDrawQuad* white_quad =
pass->CreateAndAppendDrawQuad<PictureDrawQuad>();
white_quad->SetNew(white_shared_state,
viewport,
gfx::Rect(),
viewport,
gfx::RectF(0, 0, 1, 1),
viewport.size(),
texture_format,
viewport,
1.f,
PicturePileImpl::CreateFromOther(white_pile.get()));
white_quad->SetNew(white_shared_state, viewport, gfx::Rect(), viewport,
gfx::RectF(0, 0, 1, 1), viewport.size(), texture_format,
viewport, 1.f, white_pile.get());

RenderPassList pass_list;
pass_list.push_back(pass.Pass());
Expand Down Expand Up @@ -1927,16 +1901,9 @@ TYPED_TEST(RendererPixelTest, PictureDrawQuadDisableImageFiltering) {
content_to_target_transform, viewport, pass.get());

PictureDrawQuad* quad = pass->CreateAndAppendDrawQuad<PictureDrawQuad>();
quad->SetNew(shared_state,
viewport,
gfx::Rect(),
viewport,
gfx::RectF(0, 0, 2, 2),
viewport.size(),
texture_format,
viewport,
1.f,
PicturePileImpl::CreateFromOther(pile.get()));
quad->SetNew(shared_state, viewport, gfx::Rect(), viewport,
gfx::RectF(0, 0, 2, 2), viewport.size(), texture_format,
viewport, 1.f, pile.get());

RenderPassList pass_list;
pass_list.push_back(pass.Pass());
Expand Down Expand Up @@ -1984,29 +1951,17 @@ TYPED_TEST(RendererPixelTest, PictureDrawQuadNonIdentityScale) {

PictureDrawQuad* green_quad1 =
pass->CreateAndAppendDrawQuad<PictureDrawQuad>();
green_quad1->SetNew(top_right_green_shared_quad_state,
green_rect1,
gfx::Rect(),
green_rect1,
gfx::RectF(green_rect1.size()),
green_rect1.size(),
texture_format,
green_rect1,
1.f,
PicturePileImpl::CreateFromOther(green_pile.get()));
green_quad1->SetNew(top_right_green_shared_quad_state, green_rect1,
gfx::Rect(), green_rect1, gfx::RectF(green_rect1.size()),
green_rect1.size(), texture_format, green_rect1, 1.f,
green_pile.get());

PictureDrawQuad* green_quad2 =
pass->CreateAndAppendDrawQuad<PictureDrawQuad>();
green_quad2->SetNew(top_right_green_shared_quad_state,
green_rect2,
gfx::Rect(),
green_rect2,
gfx::RectF(green_rect2.size()),
green_rect2.size(),
texture_format,
green_rect2,
1.f,
PicturePileImpl::CreateFromOther(green_pile.get()));
green_quad2->SetNew(top_right_green_shared_quad_state, green_rect2,
gfx::Rect(), green_rect2, gfx::RectF(green_rect2.size()),
green_rect2.size(), texture_format, green_rect2, 1.f,
green_pile.get());

// Add a green clipped checkerboard in the bottom right to help test
// interleaving picture quad content and solid color content.
Expand Down Expand Up @@ -2072,16 +2027,10 @@ TYPED_TEST(RendererPixelTest, PictureDrawQuadNonIdentityScale) {
content_to_target_transform, quad_content_rect, pass.get());

PictureDrawQuad* blue_quad = pass->CreateAndAppendDrawQuad<PictureDrawQuad>();
blue_quad->SetNew(blue_shared_state,
quad_content_rect,
gfx::Rect(),
quad_content_rect,
gfx::RectF(quad_content_rect),
content_union_rect.size(),
texture_format,
content_union_rect,
contents_scale,
PicturePileImpl::CreateFromOther(pile.get()));
blue_quad->SetNew(blue_shared_state, quad_content_rect, gfx::Rect(),
quad_content_rect, gfx::RectF(quad_content_rect),
content_union_rect.size(), texture_format,
content_union_rect, contents_scale, pile.get());

// Fill left half of viewport with green.
gfx::Transform half_green_content_to_target_transform;
Expand Down Expand Up @@ -2284,16 +2233,9 @@ TEST_F(GLRendererPixelTest, PictureDrawQuadTexture4444) {
blue_content_to_target_transform, viewport, pass.get());

PictureDrawQuad* blue_quad = pass->CreateAndAppendDrawQuad<PictureDrawQuad>();
blue_quad->SetNew(blue_shared_state,
viewport,
gfx::Rect(),
viewport,
gfx::RectF(0.f, 0.f, 1.f, 1.f),
viewport.size(),
texture_format,
viewport,
1.f,
PicturePileImpl::CreateFromOther(blue_pile.get()));
blue_quad->SetNew(blue_shared_state, viewport, gfx::Rect(), viewport,
gfx::RectF(0.f, 0.f, 1.f, 1.f), viewport.size(),
texture_format, viewport, 1.f, blue_pile.get());

RenderPassList pass_list;
pass_list.push_back(pass.Pass());
Expand Down
13 changes: 13 additions & 0 deletions cc/resources/picture_pile.cc
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,19 @@ void PicturePile::SetEmptyBounds() {
Clear();
}

bool PicturePile::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;
}

void PicturePile::DetermineIfSolidColor() {
is_solid_color_ = false;
solid_color_ = SK_ColorTRANSPARENT;
Expand Down
5 changes: 5 additions & 0 deletions cc/resources/picture_pile.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ class CC_EXPORT PicturePile : public PicturePileBase {

void SetPixelRecordDistanceForTesting(int d) { pixel_record_distance_ = d; }

protected:
// 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;

private:
friend class PicturePileImpl;

Expand Down
52 changes: 0 additions & 52 deletions cc/resources/picture_pile_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,39 +141,6 @@ bool PicturePileBase::HasRecordingAt(int x, int y) {
return !!found->second.GetPicture();
}

bool PicturePileBase::CanRaster(float contents_scale,
const gfx::Rect& content_rect) const {
if (tiling_.tiling_size().IsEmpty())
return false;
gfx::Rect layer_rect = gfx::ScaleToEnclosingRect(
content_rect, 1.f / contents_scale);
layer_rect.Intersect(gfx::Rect(tiling_.tiling_size()));

// 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) const {
gfx::Rect tile = tiling_.TileBounds(key.first, key.second);
return PadRect(tile);
Expand All @@ -186,25 +153,6 @@ gfx::Rect PicturePileBase::PadRect(const gfx::Rect& rect) const {
return padded_rect;
}

void PicturePileBase::AsValueInto(base::debug::TracedValue* pictures) const {
gfx::Rect tiling_rect(tiling_.tiling_size());
std::set<const void*> appended_pictures;
bool include_borders = true;
for (TilingData::Iterator tile_iter(&tiling_, tiling_rect, include_borders);
tile_iter;
++tile_iter) {
PictureMap::const_iterator map_iter = picture_map_.find(tile_iter.index());
if (map_iter == picture_map_.end())
continue;

const Picture* picture = map_iter->second.GetPicture();
if (picture && (appended_pictures.count(picture) == 0)) {
appended_pictures.insert(picture);
TracedValue::AppendIDRef(picture, pictures);
}
}
}

PicturePileBase::PictureInfo::PictureInfo() : last_frame_number_(0) {}

PicturePileBase::PictureInfo::~PictureInfo() {}
Expand Down
Loading

0 comments on commit acd5661

Please sign in to comment.