Skip to content

Commit

Permalink
cc: Band-aid coverage iterator for large layers
Browse files Browse the repository at this point in the history
gfx::ScaleToEnclosingRect converts to floats, which can lead to
precision errors at large integral values.  Ironically,
ScaleToEnclosingRect can create a rect that is smaller than the input in
some cases.  This leads to DCHECKs in debug, and quads that don't quite
fill the entire content rect in release.

One solution to punt this a little bit would be to use doubles for this
conversion (or in several places).

This solution just special cases scale=1, which is the common case for
the vast majority of tilings.  The only time that tilings are not at
scale=1 (relative to the drawing scale, which is the max of all the
contents scales in the tiling set) are for low res tilings or for
intermediate tilings during pinch zoom.

This is not an amazing solution, but it is *a* solution that should
make many cases work better.

R=weiliangc@chromium.org,danakj@chromium.org
BUG=458524
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

Cr-Commit-Position: refs/heads/master@{#341256}
  • Loading branch information
quisquous authored and Commit bot committed Jul 31, 2015
1 parent d23a86c commit 292d7a9
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 3 deletions.
5 changes: 2 additions & 3 deletions cc/tiles/picture_layer_tiling.cc
Original file line number Diff line number Diff line change
Expand Up @@ -456,11 +456,10 @@ PictureLayerTiling::CoverageIterator::operator++() {
gfx::Rect content_rect = tiling_->tiling_data_.TileBounds(tile_i_, tile_j_);

current_geometry_rect_ =
gfx::ScaleToEnclosingRect(content_rect,
1 / dest_to_content_scale_,
1 / dest_to_content_scale_);
gfx::ScaleToEnclosingRect(content_rect, 1 / dest_to_content_scale_);

current_geometry_rect_.Intersect(dest_rect_);
DCHECK(!current_geometry_rect_.IsEmpty());

if (first_time)
return *this;
Expand Down
17 changes: 17 additions & 0 deletions cc/tiles/picture_layer_tiling_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1772,5 +1772,22 @@ TEST_F(PictureLayerTilingIteratorTest, ResizeTilesAndUpdateToCurrent) {
EXPECT_EQ(0u, tiling_->AllTilesForTesting().size());
}

// This test runs into floating point issues because of big numbers.
TEST_F(PictureLayerTilingIteratorTest, GiantRect) {
gfx::Size tile_size(256, 256);
gfx::Size layer_size(33554432, 33554432);
bool filled = false;
float contents_scale = 1.f;

client_.SetTileSize(tile_size);
scoped_refptr<FakePicturePileImpl> pile =
FakePicturePileImpl::CreatePile(tile_size, layer_size, filled);
tiling_ = TestablePictureLayerTiling::Create(
PENDING_TREE, contents_scale, pile, &client_, LayerTreeSettings());

gfx::Rect content_rect(25554432, 25554432, 950, 860);
VerifyTilesExactlyCoverRect(contents_scale, content_rect);
}

} // namespace
} // namespace cc
4 changes: 4 additions & 0 deletions ui/gfx/geometry/rect.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ GFX_EXPORT Rect BoundingRect(const Point& p1, const Point& p2);
inline Rect ScaleToEnclosingRect(const Rect& rect,
float x_scale,
float y_scale) {
if (x_scale == 1.f && y_scale == 1.f)
return rect;
// These next functions cast instead of using e.g. ToFlooredInt() because we
// haven't checked to ensure that the clamping behavior of the helper
// functions doesn't degrade performance, and callers shouldn't be passing
Expand Down Expand Up @@ -248,6 +250,8 @@ inline Rect ScaleToEnclosingRect(const Rect& rect, float scale) {
inline Rect ScaleToEnclosedRect(const Rect& rect,
float x_scale,
float y_scale) {
if (x_scale == 1.f && y_scale == 1.f)
return rect;
DCHECK(base::IsValueInRangeForNumericType<int>(
std::ceil(rect.x() * x_scale)));
DCHECK(base::IsValueInRangeForNumericType<int>(
Expand Down

0 comments on commit 292d7a9

Please sign in to comment.