Add API to GraphicsSurface to allow reading in a texture directly from an FBO. Further we should have a possibility to export a texture ID, so that the texture can directly be rebound in the GPU. These changes should make the transferring of the texture from the UIProcess to the WebProcess much faster. This is in particular necessary for WebGL.
Created attachment 144508 [details] patch for review.
Up for review?
(In reply to comment #2) > Up for review? absolutely... if you have time. :)
Comment on attachment 144508 [details] patch for review. View in context: https://bugs.webkit.org/attachment.cgi?id=144508&action=review > Source/WebCore/ChangeLog:9 > + This allows binding/blitting the texture directly in the GPU. on* ? > Source/WebCore/platform/graphics/surfaces/GraphicsSurface.h:67 > + void copyFromFramebuffer(uint32_t fbo, const IntRect& sourceRect); FrameBuffer ? what is used elsewhere? > Source/WebCore/platform/graphics/surfaces/GraphicsSurface.h:69 > uint32_t exportToken(); > + uint32_t exportTexture(); What does the exportToken do? Isn't it also like a texture id ? > Source/WebCore/platform/graphics/surfaces/mac/GraphicsSurfaceMac.cpp:94 > +void GraphicsSurface::platformCopyFromFramebuffer(uint32_t originFbo, const IntRect& sourceRect) Maybe FrameBufferObject? fbo? > Source/WebCore/platform/graphics/surfaces/mac/GraphicsSurfaceMac.cpp:101 > + glPushAttrib(GL_ALL_ATTRIB_BITS); > + if (!m_texture) > + m_texture = createTexture(m_platformSurface); > + if (!m_fbo) > + glGenFramebuffers(1, &m_fbo); > + GLint oldFBO; I like newlines before and after if clauses
Comment on attachment 144508 [details] patch for review. View in context: https://bugs.webkit.org/attachment.cgi?id=144508&action=review Good direction, let's fix the nitpicks :) > Source/WebCore/platform/graphics/surfaces/GraphicsSurface.cpp:49 > +uint32_t GraphicsSurface::exportTexture() Let's call it getTextureID(). export is for cross-process sharing. >> Source/WebCore/platform/graphics/surfaces/GraphicsSurface.h:69 >> + uint32_t exportTexture(); > > What does the exportToken do? Isn't it also like a texture id ? exportToken is a cross-process number that can mean anything.
Comment on attachment 144508 [details] patch for review. View in context: https://bugs.webkit.org/attachment.cgi?id=144508&action=review >> Source/WebCore/ChangeLog:9 >> + This allows binding/blitting the texture directly in the GPU. > > on* ? fixed. >> Source/WebCore/platform/graphics/surfaces/GraphicsSurface.cpp:49 >> +uint32_t GraphicsSurface::exportTexture() > > Let's call it getTextureID(). export is for cross-process sharing. fixed. >> Source/WebCore/platform/graphics/surfaces/GraphicsSurface.h:67 >> + void copyFromFramebuffer(uint32_t fbo, const IntRect& sourceRect); > > FrameBuffer ? what is used elsewhere? The abbreviation fbo is used quite frequently in GraphicsContext3DQt/GraphicsContext3DOpenGLCommon. I think FBO is quite well known in this "context". >>> Source/WebCore/platform/graphics/surfaces/GraphicsSurface.h:69 >>> + uint32_t exportTexture(); >> >> What does the exportToken do? Isn't it also like a texture id ? > > exportToken is a cross-process number that can mean anything. In this specific case, the token identifies a GraphicsSurface, where the texture is a texture id that can be used directly with GL on the GPU. I renamed "exportTexture" to "getTextureID" as proposed by Noam. >> Source/WebCore/platform/graphics/surfaces/mac/GraphicsSurfaceMac.cpp:94 >> +void GraphicsSurface::platformCopyFromFramebuffer(uint32_t originFbo, const IntRect& sourceRect) > > Maybe FrameBufferObject? fbo? dito. >> Source/WebCore/platform/graphics/surfaces/mac/GraphicsSurfaceMac.cpp:101 >> + GLint oldFBO; > > I like newlines before and after if clauses I don't, for single lines in a small function. :-$
Created attachment 144552 [details] patch for review. - fixed issues as commented before.
This patch is changing GraphicsSurfaceMac.cpp, but how is it used? I don't think Mac even uses GraphicsSurface* anywhere.
(In reply to comment #8) > This patch is changing GraphicsSurfaceMac.cpp, but how is it used? I don't think Mac even uses GraphicsSurface* anywhere. As far as i know GraphicsSurfaceMac is currently only used by the Qt port. (when compiling Qt/WebKit for Mac)
(In reply to comment #9) > (In reply to comment #8) > > This patch is changing GraphicsSurfaceMac.cpp, but how is it used? I don't think Mac even uses GraphicsSurface* anywhere. > As far as i know GraphicsSurfaceMac is currently only used by the Qt port. (when compiling Qt/WebKit for Mac) It's a question I have. GraphicsSurfaceMac is Mac-specific code, but we only use it in Qt. There's nothing in the code that's Qt specific, but it's not used by "the mac port". What would be the way to go about files like that? I want to do the right thing but I don't have a strong opinion other way.
Comment on attachment 144552 [details] patch for review. - fixed issues as commented before. View in context: https://bugs.webkit.org/attachment.cgi?id=144552&action=review > Source/WebCore/platform/graphics/surfaces/mac/GraphicsSurfaceMac.cpp:22 > +#include "GraphicsContext3D.h" What is this for?
Comment on attachment 144552 [details] patch for review. - fixed issues as commented before. View in context: https://bugs.webkit.org/attachment.cgi?id=144552&action=review >> Source/WebCore/platform/graphics/surfaces/mac/GraphicsSurfaceMac.cpp:22 >> +#include "GraphicsContext3D.h" > > What is this for? I'll remove it. Thanks for reviewing. :-)
Committed r118987: <http://trac.webkit.org/changeset/118987>