[webkit-reviews] review granted: [Bug 70084] Add WebAcceleratedContentLayer backed by a texture to support accelerated content hosting : [Attachment 111099] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has granted Antoine Labour
<piman at chromium.org>'s request for review:
Bug 70084: Add WebAcceleratedContentLayer backed by a texture to support
accelerated content hosting
https://bugs.webkit.org/show_bug.cgi?id=70084

Attachment 111099: Patch
https://bugs.webkit.org/attachment.cgi?id=111099&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=111099&action=review


I think you should explicitly document how the compositor will use the
passed-in texture.  I believe that with our current implementation it would be
something like:

In single threaded mode, the texture will only be used during an invocation of
WebLayerTreeView::composite()
In multi threaded mode, the texture may be used starting at some undetermined
time after the texture ID is set (specifically at the first commit) and then
used at arbitrary points in time up until an undetermined time after the
texture id or layer is destroyed, or until the WebLayerTreeView is destroyed.

I'm not sure if the second is workable - we might pick up weird intermediate
states if the compositor tries to draw from a texture id that is bound to an
FBO as a color attachment in another context. So far in the WebCore compositor
we've been ignoring this problem as other issues are currently higher priority,
but if this is exposed as API that's a little less workable.

R=me on the implementation stuff.

> Source/WebKit/chromium/src/WebExternalTextureLayerImpl.cpp:58
> +void WebExternalTextureLayerImpl::paintContents(GraphicsContext& gc, const
IntRect& clip)

nit: webkit style when an implementation doesn't use a parameter is to omit the
name of the parameter. this avoids unused variable warnings on some compilers