Bug 66434 - [chromium] Update VideoLayerChromium to not access GC3D on the main thread
Summary: [chromium] Update VideoLayerChromium to not access GC3D on the main thread
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adrienne Walker
URL:
Keywords:
Depends on:
Blocks: 66430
  Show dependency treegraph
 
Reported: 2011-08-17 16:55 PDT by Adrienne Walker
Modified: 2011-08-25 10:24 PDT (History)
6 users (show)

See Also:


Attachments
Patch (21.87 KB, patch)
2011-08-24 18:04 PDT, Adrienne Walker
jamesr: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrienne Walker 2011-08-17 16:55:53 PDT
VideoLayerChromium needs to be refactored to not access layerRenderer() or layerRendererContext().  The only GC3D that should be accessed is during updateCompositorResources.

This also means that resource deletion (via the destructor, for example) needs to be deferred until the next commit as the GC3D is not immediately available.  I'm not sure of the best way to do this.  These texture IDs need to be put on a queue either in the TextureManager or perhaps via some out-of-band message to the compositor thread.
Comment 1 Adrienne Walker 2011-08-24 18:04:28 PDT
Created attachment 105111 [details]
Patch
Comment 2 James Robinson 2011-08-24 18:14:19 PDT
Comment on attachment 105111 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=105111&action=review

R=me.

The TextureManager right now doesn't know how much memory different texture formats take up (see memoryUseBytes in TextureManager.cpp).  That might be worth fixing now that we're managing non-RGBA textures, could you file a bug for that? The current code is a bit pessimistic but not obviously harmful.

> Source/WebCore/platform/graphics/chromium/cc/CCVideoLayerImpl.cpp:72
> +void CCVideoLayerImpl::setTexture(size_t idx, Platform3DObject textureId, const IntSize& size, const IntSize& visibleSize)

we don't abbreviate in WebKit, although we do use 'i' so either 'i' or 'index' are good but not 'idx'
Comment 3 Adrienne Walker 2011-08-24 18:25:21 PDT
(In reply to comment #2)
> (From update of attachment 105111 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=105111&action=review
> 
> R=me.
> 
> The TextureManager right now doesn't know how much memory different texture formats take up (see memoryUseBytes in TextureManager.cpp).  That might be worth fixing now that we're managing non-RGBA textures, could you file a bug for that? The current code is a bit pessimistic but not obviously harmful.

https://bugs.webkit.org/show_bug.cgi?id=66917

> > Source/WebCore/platform/graphics/chromium/cc/CCVideoLayerImpl.cpp:72
> > +void CCVideoLayerImpl::setTexture(size_t idx, Platform3DObject textureId, const IntSize& size, const IntSize& visibleSize)
> 
> we don't abbreviate in WebKit, although we do use 'i' so either 'i' or 'index' are good but not 'idx'

Ah, quite right.  I personally don't like 'i' as a variable except in a small loop, so I'll just change it to 'index'.
Comment 4 Adrienne Walker 2011-08-25 10:24:20 PDT
Committed r93796: <http://trac.webkit.org/changeset/93796>