Per recent changes to the WebGL specification (Framebuffer Object Attachments, section 6.3), checks about the validity of the framebuffer configuration need to occur upon draw call rather than at the point a renderbuffer is attached to a framebuffer. In particular: - Attachments to more than one attachment point (DEPTH, STENCIL, DEPTH_STENCIL) are illegal. - The attachment's format must match that of the attachment point. The reason for this is to avoid implicitly supporting separate depth+stencil (or packed depth+stencil) on some platforms and not others.
Created attachment 72252 [details] patch I will commit the test change to khronos once reviewed.
Comment on attachment 72252 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=72252&action=review Looks good overall. Couple of minor comments. > WebCore/html/canvas/WebGLFramebuffer.cpp:38 > +static unsigned long getInternalFormat(WebGLObject* buffer) You could use an anonymous namespace instead of static methods here. > WebCore/html/canvas/WebGLFramebuffer.cpp:141 > + return true; This test seems redundant. > WebCore/html/canvas/WebGLFramebuffer.h:49 > + // readPixels, copyTexImage2D, copyTexSubImage2D if this frame bufferis bufferis -> buffer is
Comment on attachment 72252 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=72252&action=review Sorry, I noticed an issue after r+'ing the patch. Hopefully you get this email before landing. > WebCore/html/canvas/WebGLRenderingContext.cpp:458 > + return GraphicsContext3D::FRAMEBUFFER_INCOMPLETE_ATTACHMENT; According to the spec this is supposed to be FRAMEBUFFER_UNSUPPORTED. Is there a reason to diverge? If not, please update the code and the test.
Created attachment 72754 [details] revised patch: responding to kbr's review
Comment on attachment 72754 [details] revised patch: responding to kbr's review Looks good. Thanks for taking care of this. Don't forget to sync the test with the Khronos repository.
Committed r71190: <http://trac.webkit.org/changeset/71190>