Fixes video to canvas drawing. The draw operationwas not updating the video

Fixes video to canvas drawing.  The draw operationwas not updating the video
past the first frame.  Added a call to notifyPixelsChanged on the bitmap to 
invalidate the associated texture chache entry, so that the frame will update
correctly.

BUG=57695
TEST=http://www.craftymind.com/factory/html5video/CanvasVideo.html

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=95535

[Justin Novosad, 2011-08-04 17:41:02]

PTAL

[Justin Novosad, 2011-08-04 17:57:35]

Changing reviewer to tony, scherkus is out-of-office.   This is an M14 release
blocker.

[tony, 2011-08-04 18:02:29]

I am not the right reviewer for this, but I am happy to forward on someone
else's review.  Looking at the recent commit log, maybe acolwell or hclam can
review?

[Ami GONE FROM CHROMIUM, 2011-08-04 19:33:56]

On 2011/08/04 18:02:29, tony wrote:
> I am not the right reviewer for this, but I am happy to forward on someone
> else's review.  Looking at the recent commit log, maybe acolwell or hclam can
> review?

acolwell is also ooto and hclam is in LON TZ.
vrk: you wanna look at this?

[Alpha Left Google, 2011-08-04 20:58:39]

Can you explain or add comments to describe how this can solve the problem?
notifyPixelsChanged() only update a sequence number, how is this related to the
texture cache entry and updates the canvas tag?

Also I'm not sure why we're going through the SlowPaint() routine, as the name
suggested it is really slow..

[Justin Novosad, 2011-08-04 21:34:02]

On 2011/08/04 20:58:39, Alpha wrote:
> Can you explain or add comments to describe how this can solve the problem?
> notifyPixelsChanged() only update a sequence number, how is this related to
the
> texture cache entry and updates the canvas tag?
> 
> Also I'm not sure why we're going through the SlowPaint() routine, as the name
> suggested it is really slow..

The purpose of the sequence number is to track versions of a bitmap.
The bitmap's generation ID is used as part of the cache key in skia's texture
cache.  See SkGpuDevice::lockCachedTexture in
src/third_party/skia/src/gpu/SkGpuDevice.cpp.  Therefore, calling
notifyPixelsChanged() will not evict anything from the texture cache, it will
simply guarantee a cache miss the next time we attempt to acquire a lock on the
texture.  The older generation of the bitmap will eventually get evicted when
GPU resource consumption reaches a threshold (based on LRU).

An alternate fix would be to call bitmap.setIsVolatile(true), which would signal
to skia to not use the texture cache and to upload via a texture from a pool of
scratch textures.  Using isVolatile is faster  in cases where we know that the
bitmap is always different every time we upload it to the GPU, because it avoids
the cost of allocating and releasing (i.e. evicting) textures.  On the other
hand, using notifyPixelsChanged means we get to skip the texture upload when the
cache is up to date.

As to why we are using the SlowPaint method, it looks like  CanUseFastPaint is
returning false because the canvas device is not flagged as being opaque.

[Alpha Left Google, 2011-08-04 21:37:02]

LGTM.

Is the canvas the final target? If there's another copy to get to the final copy
maybe we can set the canvas a opaque to save on memcpy.

[Justin Novosad, 2011-08-04 21:43:56]

On 2011/08/04 21:37:02, Alpha wrote:
> LGTM.
> 
> Is the canvas the final target? If there's another copy to get to the final
copy
> maybe we can set the canvas a opaque to save on memcpy.

Yeah, I was thinking the same thing. <Understatement> Unfortunately making that
determination is non trivial. </Understatement>

[tony, 2011-08-04 22:10:20]

On 2011/08/04 21:34:02, junov wrote:
> On 2011/08/04 20:58:39, Alpha wrote:
> > Can you explain or add comments to describe how this can solve the problem?
> > notifyPixelsChanged() only update a sequence number, how is this related to
> the
> > texture cache entry and updates the canvas tag?
> > 
> > Also I'm not sure why we're going through the SlowPaint() routine, as the
name
> > suggested it is really slow..
> 
> The purpose of the sequence number is to track versions of a bitmap.
> The bitmap's generation ID is used as part of the cache key in skia's texture
> cache.  See SkGpuDevice::lockCachedTexture in
> src/third_party/skia/src/gpu/SkGpuDevice.cpp.  Therefore, calling
> notifyPixelsChanged() will not evict anything from the texture cache, it will
> simply guarantee a cache miss the next time we attempt to acquire a lock on
the
> texture.  The older generation of the bitmap will eventually get evicted when
> GPU resource consumption reaches a threshold (based on LRU).

Can you put this in the change description?

Forwarding hclam's LGTM.  Maybe more people should be in the
webkit/glue/media/OWNERS file.

[commit-bot: I haz the power, 2011-08-04 23:35:34]

Change committed as 95535