[Webkit-unassigned] [Bug 93010] [chromium] Add WebExternalTextureProvider; an alternate means for providing textures to a TextureLayerChromium

[bugzilla-daemon at webkit.org]

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


James Robinson <jamesr at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #168708|review?                     |review-
               Flag|                            |




--- Comment #17 from James Robinson <jamesr at chromium.org>  2012-10-15 16:36:10 PST ---
(From update of attachment 168708)
View in context: https://bugs.webkit.org/attachment.cgi?id=168708&action=review

Needs a bit more polish.

> Source/Platform/chromium/public/WebExternalTextureLayer.h:83
> +    // TODO: make pure virtual once the chromium impl is changed and deps roll

WebKit spells this FIXME, not TODO.

I would put this next to setTextureId() and document how this interacts with setTextureId() and WebExternalTextureLayerClient.  Needing 3 different ways to provide a texture is kind of unfortunate :/

Do we need a way to set this after the fact or could this be another ::create() override? The advantage of having this be ::create() is that you don't have to worry about the provider changing after creation

> Source/Platform/chromium/public/WebExternalTextureProvider.h:33
> +class WebGraphicsContext3D;

this appears to be unused

> Source/Platform/chromium/public/WebExternalTextureProvider.h:52
> +        virtual void didUpdatePremultipliedAlpha(bool) = 0;
> +        virtual void didUpdateFlipped(bool) = 0;
> +        virtual void didUpdateUVRect(const WebFloatRect&) = 0;

Do these mean that the various state bits on the WebExternalTextureLayer are not used?  Do you really need this level of granularity on properties for your use case?  Please document

> Source/Platform/chromium/public/WebExternalTextureProvider.h:58
> +    // This is called on impl thread and happens during pushProperties

The "happens during pushProperties" part is not really useful to people reading this header since it's not part of the public API.  Much more useful would be knowing how often this is called (once) and what the lifetime of the parameter is.

nit: end comments with a period

> Source/WebKit/chromium/src/WebPluginContainerImpl.h:106
> +    // This is mutually exclusive with setBackingTextureId and setBackingIOSurfaceId

This is not the most useful place to document this since it's the implementation, not the public API.

Comments are supposed to be sentences and thus end with a period.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.