cc: Replace RasterBuffer::Acquire/ReleaseCanvas with Playback function.

cc: Replace RasterBuffer::Acquire/ReleaseCanvas with Playback function.

This reduces the complexity of RasterBuffer implementations
significantly as they don't have to keep track of state inbetween
Acquire/ReleaseCanvas calls.

BUG=418553
Committed: https://crrev.com/54232ec61a00db275babf58cd16137a4cf807309
Cr-Commit-Position: refs/heads/master@{#299635}

[reveman, 2014-10-14 18:56:30]

reveman@chromium.org changed reviewers:
+ vmpstr@chromium.org

[reveman, 2014-10-14 18:56:30]

This makes what I'm trying to do in the 1-copy case much easier and I think it's
a new cleanup in general. Please take a look.

[reveman, 2014-10-14 19:19:43]

reveman@chromium.org changed reviewers:
+ hendrikw@chromium.org

[reveman, 2014-10-14 19:19:43]

+hendrikw as this might make it easier to add distance field font support

[hendrikw, 2014-10-14 20:32:13]

On 2014/10/14 19:19:43, reveman wrote:
> +hendrikw as this might make it easier to add distance field font support

With this implementation, I could:
1. add a dff flag to picturepile (to be set by pictureLayerImpl)
2. In Playback, ask ResourceProvider to recreate the sksurface if the dff
setting on the resource does not match picture pile's setting.

[vmpstr, 2014-10-15 00:09:30]

lgtm

https://codereview.chromium.org/659563002/diff/1/cc/resources/gpu_raster_work...
File cc/resources/gpu_raster_worker_pool.cc (right):

https://codereview.chromium.org/659563002/diff/1/cc/resources/gpu_raster_work...
cc/resources/gpu_raster_worker_pool.cc:44: resource_->size().width(),
resource_->size().height()));
nit: Can you make size a variable above?

https://codereview.chromium.org/659563002/diff/1/cc/resources/raster_buffer.h
File cc/resources/raster_buffer.h (right):

https://codereview.chromium.org/659563002/diff/1/cc/resources/raster_buffer.h...
cc/resources/raster_buffer.h:20: virtual void Playback(const PicturePileImpl*
picture_pile,
I'm not really a fan of this leaking to RasterBuffer, since picture pile impl is
really only the current approach of doing this, but I guess that's fine since
for the time being we're fairly committed to having picture likes, rects, and
scales...

[reveman, 2014-10-15 01:24:06]

https://codereview.chromium.org/659563002/diff/1/cc/resources/gpu_raster_work...
File cc/resources/gpu_raster_worker_pool.cc (right):

https://codereview.chromium.org/659563002/diff/1/cc/resources/gpu_raster_work...
cc/resources/gpu_raster_worker_pool.cc:44: resource_->size().width(),
resource_->size().height()));
On 2014/10/15 00:09:30, vmpstr wrote:
> nit: Can you make size a variable above?

Done.

https://codereview.chromium.org/659563002/diff/1/cc/resources/raster_buffer.h
File cc/resources/raster_buffer.h (right):

https://codereview.chromium.org/659563002/diff/1/cc/resources/raster_buffer.h...
cc/resources/raster_buffer.h:20: virtual void Playback(const PicturePileImpl*
picture_pile,
On 2014/10/15 00:09:30, vmpstr wrote:
> I'm not really a fan of this leaking to RasterBuffer, since picture pile impl
is
> really only the current approach of doing this, but I guess that's fine since
> for the time being we're fairly committed to having picture likes, rects, and
> scales...

Yes, I'm a bit annoyed by this too but the cleanup we get from doing it makes me
feel that it's worth it.

We could always add multiple functions to this interface. For example:

virtual void PlaybackPicturePile(... );
virtual void ExecuteAwesomeDisplayList(... );

Or we can create some other interface that PicturePileImpl could implement and
RasterBuffer would use. That seems like an unnecessary abstraction at this point
though..

[reveman, 2014-10-15 01:24:10]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-10-15 01:24:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/659563002/20001

[reveman, 2014-10-15 01:27:18]

On 2014/10/14 20:32:13, Hendrik wrote:
> On 2014/10/14 19:19:43, reveman wrote:
> > +hendrikw as this might make it easier to add distance field font support
> 
> With this implementation, I could:
> 1. add a dff flag to picturepile (to be set by pictureLayerImpl)
> 2. In Playback, ask ResourceProvider to recreate the sksurface if the dff
> setting on the resource does not match picture pile's setting.

That sounds like a reasonable solution to me. We'll be able to keep all the dff
logic in GpuRasterWorkerPool and the ResourceProvider. I'd prefer if the
property on the picturepile is something less gpu raster specific though.
animating, will_change or something like that

[commit-bot: I haz the power, 2014-10-15 02:37:12]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-10-15 02:38:15]

Patchset 2 (id:??) landed as
https://crrev.com/54232ec61a00db275babf58cd16137a4cf807309
Cr-Commit-Position: refs/heads/master@{#299635}