cc: Don't consider padding for analysis.

cc: Don't consider padding for analysis.

Currently we analyze exactly what we raster. However, there
are cases like edge tiles on a layer that are padded, so that
the shader samples non-garbage values when it's interpolating
the texture values.

However, for analysis we don't need this padding, we can analyze
just the integer canvas, which allows us to properly detect solid
layer-edge tiles.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=199903

[vmpstr, 2013-05-10 22:54:10]

Please take a look. This change is enough to make the solid colors work on edge
tiles, and I think this reflects the discussion we had about how to fix this.

[reveman, 2013-05-13 15:48:44]

https://codereview.chromium.org/14690020/diff/1/cc/resources/picture_pile_imp...
File cc/resources/picture_pile_impl.cc (right):

https://codereview.chromium.org/14690020/diff/1/cc/resources/picture_pile_imp...
cc/resources/picture_pile_impl.cc:249:
content_rect.Intersect(gfx::Rect(gfx::ToFlooredSize(
Can we really round down here? I think we can avoid rounding up but I guess that
doesn't help you much in this case.

[enne (OOO), 2013-05-13 16:10:56]

I'm not sure that this CL is right either, so I think I can now understand why
you were unsure about this approach previously.  I think I had been thinking
that maybe we could do the analysis in layer space and clamp to integer layer
coordinates, but I'm not totally sure about that either.

[reveman, 2013-05-13 16:16:46]

I think the case we can handle this way is when the bottom-right tile is aligned
with the pixel grid but needs padding to filter properly. The analysis can skip
the padding in this case.

[vmpstr, 2013-05-13 16:18:00]

My reasoning for this was that the layer extends past a pixel, but does not
quite cover the next pixel. We rounded up to give it the padding. If we round
down, we eliminate the padding but keep the region which is completely covered
by the layer.

Yeah, that's the reason I was a bit hesitant about doing the analysis on a
different thing, as compared to raster. Layer space would work, I'm just weary
if that would make it slower. It shouldn't since nothing is being raster, but
I'd like to test that first.

[vmpstr, 2013-05-13 16:19:25]

On 2013/05/13 16:16:46, David Reveman wrote:
> I think the case we can handle this way is when the bottom-right tile is
aligned
> with the pixel grid but needs padding to filter properly. The analysis can
skip
> the padding in this case.

Isn't that already done by analysis? That is, we make the canvas as big as the
ceiled scaled layer, not bigger

[reveman, 2013-05-13 16:58:44]

Yes, maybe this already works correctly. Fyi, I'm currently seeing a raster task
on http://goo.gl/iHtwD but would expect all tiles to be detected as solid.

Doing the analysis in layer space sgtm.

[vmpstr, 2013-05-13 17:02:42]

On 2013/05/13 16:58:44, David Reveman wrote:
> Yes, maybe this already works correctly. Fyi, I'm currently seeing a raster
task
> on http://goo.gl/iHtwD but would expect all tiles to be detected as solid.
> 
> Doing the analysis in layer space sgtm.

This patch makes the page you provided to be correctly detected as solid (both
low res and high res tiles). I'll make the change for layer space though

[vmpstr, 2013-05-13 17:37:03]

PTAL. This works perfectly if 1.0 / contents_scale ends up being an integer. For
the sake of being conservative, I'm using the enclosing rect, instead of
enclosed or floored, so there's a chance that it might not detect something as
solid. This should be ok, since we intersect with the tiling size to trim it
right up to the layer, and tiles not on layer boundary might check an extra
layer space pixel around them (which is ok as well).

[enne (OOO), 2013-05-13 17:48:52]

https://codereview.chromium.org/14690020/diff/8001/cc/resources/picture_pile_...
File cc/resources/picture_pile_impl.cc (right):

https://codereview.chromium.org/14690020/diff/8001/cc/resources/picture_pile_...
cc/resources/picture_pile_impl.cc:249: gfx::Rect layer_rect =
gfx::ToEnclosingRect(
layer_rect is tiling_.total_size().  No need for scaling.

[enne (OOO), 2013-05-13 18:14:57]

Oops, I totally misread that.   lgtm.

[reveman, 2013-05-13 20:20:13]

lg, do we need adjust the prediction benchmarks code too?

[vmpstr, 2013-05-13 22:29:58]

On 2013/05/13 20:20:13, David Reveman wrote:
> lg, do we need adjust the prediction benchmarks code too?

I don't think so. We still want to check the actually rasterized pixels that
come from the RunRasterTask run. The only thing the prediction benchmark would
be using from this is the result of the analysis (which should be the same or
better by design)

[vmpstr, 2013-05-13 23:04:03]

Changed the scale in prediction benchmarking. Also filed crbug.com/240500 to fix
the fact that benchmarking is checking all pixels, even if they are outside of
layer boundary.

[reveman, 2013-05-13 23:17:26]

lgtm, please consider moving all scale logic into TileManager::RunAnalyzeTask in
a follow up

[commit-bot: I haz the power, 2013-05-13 23:22:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vmpstr@chromium.org/14690020/15001

[commit-bot: I haz the power, 2013-05-14 04:58:03]

Change committed as 199903