[chromium] Convert WebPluginContainerImpl over to use WebExternalTextureLayer
Created attachment 137461 [details] Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Attachment 137461 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/Platform/ChangeLog', u'Source/Platf..." exit_code: 1 Source/WebKit/chromium/src/WebExternalTextureLayer.cpp:29: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 137469 [details] rebased
Stylebot is wrong about the #include order, it doesn't realize that: #include <public/WebExternalTextureLayer.h> in WebExternalTextureLayer.cpp is #including the header for that class. I'll try to get this up on github to make it easier to tell the code changes apart from the file moves, but it's currently giving me errors running remote hooks.
Attachment 137469 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/Platform/ChangeLog', u'Source/Platf..." exit_code: 1 Source/WebKit/chromium/src/WebExternalTextureLayer.cpp:29: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 137628 [details] patch 1 (for reference)
Created attachment 137629 [details] patch 2 (for reference)
Comment on attachment 137469 [details] rebased View in context: https://bugs.webkit.org/attachment.cgi?id=137469&action=review Thanks for splitting this up into two patches. That made it way easier to review. > Source/Platform/chromium/public/WebExternalTextureLayer.h:38 > class WebExternalTextureLayerImpl; Remove me. > Source/Platform/chromium/public/WebExternalTextureLayer.h:59 > // Sets the texture id that represents the layer, in the namespace of the > // compositor context. > WEBKIT_EXPORT void setTextureId(unsigned); > - WEBKIT_EXPORT unsigned textureId() const; > + > + // Sets the IO surface id that represents this layer. setTextureId() and setIOSurfaceProperties() are mutually > + // exclusive - a layer can be either backed by a texture or an IO surface, but not both. > + WEBKIT_EXPORT void setIOSurfaceProperties(const WebSize&, unsigned ioSurfaceId); A random design aside. It's always struck me as a little bit of a wart that we have this class that gets used in these two modes, and these modes infect its usage all the way down, even though one path (seemingly?) is only used by npapi plugins and the other only by ppapi. I kind of wish it wouldn't cost so much boilerplate to have them be two separate classes. It doesn't have to be fixed here, but do you have any thoughts on how to make this better?
(In reply to comment #9) > > Source/Platform/chromium/public/WebExternalTextureLayer.h:59 > > // Sets the texture id that represents the layer, in the namespace of the > > // compositor context. > > WEBKIT_EXPORT void setTextureId(unsigned); > > - WEBKIT_EXPORT unsigned textureId() const; > > + > > + // Sets the IO surface id that represents this layer. setTextureId() and setIOSurfaceProperties() are mutually > > + // exclusive - a layer can be either backed by a texture or an IO surface, but not both. > > + WEBKIT_EXPORT void setIOSurfaceProperties(const WebSize&, unsigned ioSurfaceId); > > A random design aside. It's always struck me as a little bit of a wart that we have this class that gets used in these two modes, and these modes infect its usage all the way down, even though one path (seemingly?) is only used by npapi plugins and the other only by ppapi. I kind of wish it wouldn't cost so much boilerplate to have them be two separate classes. It doesn't have to be fixed here, but do you have any thoughts on how to make this better? I think it's a problem. I think the first step to fixing things is to figure out how many of the features (meaning options like flipped, uvrect, premultiplied, etc) on this class are actually being used by the IOSurface path. If it's a small number, then I think having a dedicated type for IOSurface backed layers would make more sense than having them share the texture type. If IOSurface-backed layers can do pretty much everything that texture layers can do, then I'd like to continue to share as much code as possible. I haven't been able to do this investigation yet but would like to soon - of course if anyone else gets to it first that'd be great.
Committed r114604: <http://trac.webkit.org/changeset/114604>