Bug 104553

Created attachment 178564 [details] patch

Created attachment 178574 [details] patch

Attachment 178574 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:150: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 178574 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=178574&action=review > Source/WebCore/ChangeLog:9 > + > + With this patch we use the same FBO for canvas and backbuffer of offscreen surface. > + Also, removes some unused code. You should explain why that is a good things etc. What it really means > Source/WebCore/ChangeLog:23 > + * platform/graphics/efl/GraphicsContext3DEfl.cpp: > + (WebCore::GraphicsContext3D::createGraphicsSurfaces): > + * platform/graphics/efl/GraphicsContext3DPrivate.cpp: > + (GraphicsContext3DPrivate::canvasResized): > + (GraphicsContext3DPrivate::copyToGraphicsSurface): > + * platform/graphics/efl/GraphicsContext3DPrivate.h: > + (GraphicsContext3DPrivate): > + * platform/graphics/opengl/GLDefs.h: > + * platform/graphics/opengl/GLPlatformContext.h: > + * platform/graphics/opengl/GLPlatformSurface.h: > + * platform/graphics/surfaces/glx/GLXSurface.h: Please add more info about your changes here. Especially when you refactor > Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:236 > + m_private->canvasResized(); didResizeCanvas() seems more consistent with other webcore code > Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:146 > + // We should copy the full buffer, and not respect the > + // current scissor bounds. keep on one line > Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:147 > + // FIXME::It would be more efficient to track the state of the scissor test. not two ::, just "FIXME: " >> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:150 >> + // m_context->disable(GraphicsContext3D::SCISSOR_TEST); > > Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Dont ever commit outcommented code! > Source/WebCore/platform/graphics/opengl/GLPlatformSurface.h:56 > + // Creates FBO used by the surface. > + // Buffers can be bound to this FBO. one line! > Source/WebCore/platform/graphics/opengl/GLPlatformSurface.h:57 > + void initialize(GLuint *frameBufferId); * placement! > Source/WebCore/platform/graphics/opengl/GLPlatformSurface.h:70 > - // a)Blits texture contents to back buffer. > + // a)Blits back buffer contents to front buffer. > // b)Calls Swap Buffers. > - // c)Sets current FBO as bindFboId and binds texture to bindTexture. > - virtual void updateContents(const uint32_t texture, const IntRect& sourceRect, const GLuint bindFboId, const uint32_t bindTexture); > + // c)Sets current FBO as bindFboId. space after )

Created attachment 178581 [details] proposedpatch patch for review

Comment on attachment 178581 [details] proposedpatch r- given my other comments

Created attachment 178628 [details] patchv3

(In reply to comment #4) > (From update of attachment 178574 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=178574&action=review > > > Source/WebCore/ChangeLog:9 > > + > > + With this patch we use the same FBO for canvas and backbuffer of offscreen surface. > > + Also, removes some unused code. > > You should explain why that is a good things etc. What it really means > > > Source/WebCore/ChangeLog:23 > > + * platform/graphics/efl/GraphicsContext3DEfl.cpp: > > + (WebCore::GraphicsContext3D::createGraphicsSurfaces): > > + * platform/graphics/efl/GraphicsContext3DPrivate.cpp: > > + (GraphicsContext3DPrivate::canvasResized): > > + (GraphicsContext3DPrivate::copyToGraphicsSurface): > > + * platform/graphics/efl/GraphicsContext3DPrivate.h: > > + (GraphicsContext3DPrivate): > > + * platform/graphics/opengl/GLDefs.h: > > + * platform/graphics/opengl/GLPlatformContext.h: > > + * platform/graphics/opengl/GLPlatformSurface.h: > > + * platform/graphics/surfaces/glx/GLXSurface.h: > > Please add more info about your changes here. Especially when you refactor > > > Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:236 > > + m_private->canvasResized(); > > didResizeCanvas() seems more consistent with other webcore code > > > Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:146 > > + // We should copy the full buffer, and not respect the > > + // current scissor bounds. > > keep on one line > > > Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:147 > > + // FIXME::It would be more efficient to track the state of the scissor test. > > not two ::, just "FIXME: " > > >> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:150 > >> + // m_context->disable(GraphicsContext3D::SCISSOR_TEST); > > > > Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] > > Dont ever commit outcommented code! > > > Source/WebCore/platform/graphics/opengl/GLPlatformSurface.h:56 > > + // Creates FBO used by the surface. > > + // Buffers can be bound to this FBO. > > one line! > > > Source/WebCore/platform/graphics/opengl/GLPlatformSurface.h:57 > > + void initialize(GLuint *frameBufferId); > > * placement! > > > Source/WebCore/platform/graphics/opengl/GLPlatformSurface.h:70 > > - // a)Blits texture contents to back buffer. > > + // a)Blits back buffer contents to front buffer. > > // b)Calls Swap Buffers. > > - // c)Sets current FBO as bindFboId and binds texture to bindTexture. > > - virtual void updateContents(const uint32_t texture, const IntRect& sourceRect, const GLuint bindFboId, const uint32_t bindTexture); > > + // c)Sets current FBO as bindFboId. > > space after ) done.

Comment on attachment 178628 [details] patchv3 Clearing flags on attachment: 178628 Committed r137211: <http://trac.webkit.org/changeset/137211>

All reviewed patches have been landed. Closing bug.

This seems to break all of webgl for me!