Index: cc/tiles/picture_layer_tiling.cc |
diff --git a/cc/tiles/picture_layer_tiling.cc b/cc/tiles/picture_layer_tiling.cc |
index e52a8ab6af1bf1c860756e9681c0513883d06488..fe7e4c9a7a743289c208b99e846a45203ab03d20 100644 |
--- a/cc/tiles/picture_layer_tiling.cc |
+++ b/cc/tiles/picture_layer_tiling.cc |
@@ -30,6 +30,12 @@ |
namespace cc { |
+namespace { |
+float SnapToOne(float n) { |
+ return (std::abs(1.f - n) < std::numeric_limits<float>::epsilon()) ? 1.f : n; |
+} |
trchen
2017/04/14 22:07:33
I doubt this function works as intended...
By def
vmpstr
2017/04/14 23:36:26
The definition of epsilon is the difference betwee
trchen
2017/04/14 23:53:43
You're right. I forgot (1-epsilon) contains only 2
vmpstr
2017/04/15 00:31:24
It just happens that when coverage scale is 7352.3
trchen
2017/04/18 21:13:46
Does this helps with the bug you're fixing? I seem
|
+} |
+ |
PictureLayerTiling::PictureLayerTiling( |
WhichTree tree, |
const gfx::AxisTransform2d& raster_transform, |
@@ -380,9 +386,15 @@ PictureLayerTiling::CoverageIterator::CoverageIterator( |
const gfx::Rect& coverage_rect) |
: tiling_(tiling), |
coverage_rect_(coverage_rect), |
+ // Note that since this iterator will map possibly large numbers, it's |
+ // very sensitive to small differences in scale (even less than epsilon). |
+ // It is also very likely that this coverage iterator would be used with a |
+ // scale that matches the contents scale (since majority of the time, |
+ // we're rasterizing high res content. As a result, snap the scale to one |
+ // if it's within the epsilon. |
coverage_to_content_( |
- gfx::PreScaleAxisTransform2d(tiling->raster_transform(), |
- 1.f / coverage_scale)) { |
+ SnapToOne(tiling->raster_transform().scale() / coverage_scale), |
+ tiling->raster_transform().translation()) { |
DCHECK(tiling_); |
// In order to avoid artifacts in geometry_rect scaling and clamping to ints, |
// the |coverage_scale| should always be at least as big as the tiling's |
@@ -420,11 +432,35 @@ PictureLayerTiling::CoverageIterator::CoverageIterator( |
const TilingData& data = tiling_->tiling_data_; |
left_ = data.LastBorderTileXIndexFromSrcCoord(wanted_texels.x()); |
top_ = data.LastBorderTileYIndexFromSrcCoord(wanted_texels.y()); |
+ |
right_ = std::max( |
left_, data.FirstBorderTileXIndexFromSrcCoord(wanted_texels.right())); |
bottom_ = std::max( |
top_, data.FirstBorderTileYIndexFromSrcCoord(wanted_texels.bottom())); |
+ // Unfortunately due to floating point math in ToEnclosingRect, wanted_texels |
+ // could be lying about its dimensions. For example, consider the following: |
+ // |
+ // float a = 192.f; |
+ // float b = 2104670720.f; |
+ // output("%.0f\n", static_cast<double>(a + b) - b); |
+ // |
+ // This code prints out 256. So if that happens to be our coverage rect |
+ // position and dimension, then the texel extent would try to grab something |
+ // outside of it, resulting in a DCHECK later (or empty geometry rect with |
+ // DCHECks disabled), which is undesirable. The solution is to simply |
+ // backtrack the value until the content rect actually intersects the texel |
+ // extent for the values we generated. Note this only happens with |
+ // right/bottom values. |
+ while (!content_rect.Intersects(data.TexelExtent(right_, top_)) && |
enne (OOO)
2017/04/14 05:35:56
Hmmm, do you think that FirstBorderTileXIndexFromS
vmpstr
2017/04/14 18:17:42
The problem is that FirstBorderTileXIndexFromSrcCo
trchen
2017/04/14 22:07:33
Just double checked FirstBorderTileXIndexFromSrcCo
vmpstr
2017/04/14 23:36:26
Yes, I meant the access to right/bottom in enclosi
trchen
2017/04/18 21:13:46
Just sync'd up with enne@ offline. We agreed that
|
+ right_ > left_) { |
+ --right_; |
+ } |
+ while (!content_rect.Intersects(data.TexelExtent(left_, bottom_)) && |
+ bottom_ > top_) { |
+ --bottom_; |
+ } |
+ |
tile_i_ = left_ - 1; |
tile_j_ = top_; |
++(*this); |
@@ -451,6 +487,8 @@ PictureLayerTiling::CoverageIterator::operator++() { |
} |
} |
+ DCHECK_LT(tile_i_, tiling_->tiling_data_.num_tiles_x()); |
+ DCHECK_LT(tile_j_, tiling_->tiling_data_.num_tiles_y()); |
current_tile_ = tiling_->TileAt(tile_i_, tile_j_); |
// Calculate the current geometry rect. As we reserved overlap between tiles |
@@ -521,11 +559,13 @@ PictureLayerTiling::CoverageIterator::operator++() { |
int inset_top = std::max(0, min_top - current_geometry_rect_.y()); |
current_geometry_rect_.Inset(inset_left, inset_top, 0, 0); |
+#if DCHECK_IS_ON() |
enne (OOO)
2017/04/14 05:35:56
Do you need these extra #ifs?
vmpstr
2017/04/14 18:17:42
Not really, it's just everything is DCHECK in ther
|
if (!new_row) { |
DCHECK_EQ(last_geometry_rect.right(), current_geometry_rect_.x()); |
DCHECK_EQ(last_geometry_rect.bottom(), current_geometry_rect_.bottom()); |
DCHECK_EQ(last_geometry_rect.y(), current_geometry_rect_.y()); |
} |
+#endif |
return *this; |
} |
@@ -584,7 +624,7 @@ void PictureLayerTiling::ComputeTilePriorityRects( |
} |
const float content_to_screen_scale = |
- ideal_contents_scale / raster_transform_.scale(); |
+ SnapToOne(ideal_contents_scale / raster_transform_.scale()); |
const gfx::Rect* input_rects[] = { |
&visible_rect_in_layer_space, &skewport_in_layer_space, |