WebGL 2: add read/write framebuffer binding points to related APIs

Source/core/html/canvas/WebGLRenderingContextBase.cpp

Index: Source/core/html/canvas/WebGLRenderingContextBase.cpp
diff --git a/Source/core/html/canvas/WebGLRenderingContextBase.cpp b/Source/core/html/canvas/WebGLRenderingContextBase.cpp
index 9f5cf4d01bdbff880da0c97198a57d165a87d40a..47f96c12bbbfb17d485107d255f3b126428b1319 100644
--- a/Source/core/html/canvas/WebGLRenderingContextBase.cpp
+++ b/Source/core/html/canvas/WebGLRenderingContextBase.cpp
@@ -223,25 +223,27 @@ namespace {
     class ScopedDrawingBufferBinder {
         STACK_ALLOCATED();
     public:
-        ScopedDrawingBufferBinder(DrawingBuffer* drawingBuffer, WebGLFramebuffer* framebufferBinding)
+        ScopedDrawingBufferBinder(DrawingBuffer* drawingBuffer, WebGLFramebuffer* framebufferBinding, GLenum target)
             : m_drawingBuffer(drawingBuffer)
             , m_framebufferBinding(framebufferBinding)
+            , m_target(target)
         {
             // Commit DrawingBuffer if needed (e.g., for multisampling)
             if (!m_framebufferBinding && m_drawingBuffer)
-                m_drawingBuffer->commit();
+                m_drawingBuffer->commit(m_target);
         }
 
         ~ScopedDrawingBufferBinder()
         {
             // Restore DrawingBuffer if needed
             if (!m_framebufferBinding && m_drawingBuffer)
-                m_drawingBuffer->bind();
+                m_drawingBuffer->bind(m_target);
         }
 
     private:
         DrawingBuffer* m_drawingBuffer;
         RawPtrWillBeMember<WebGLFramebuffer> m_framebufferBinding;
+        GLenum m_target;
     };
 
     GLint clamp(GLint value, GLint min, GLint max)
@@ -917,11 +919,18 @@ WebGLRenderingContextBase::HowToClear WebGLRenderingContextBase::clearIfComposit
         webContext()->stencilMaskSeparate(GL_FRONT, 0xFFFFFFFF);
     }
 
-    drawingBuffer()->clearFramebuffers(clearMask);
+    if (!isWebGL2OrHigher())
+        drawingBuffer()->clearFramebuffers(clearMask);
+    else
+        drawingBuffer()->clearFramebuffers(clearMask, GL_DRAW_FRAMEBUFFER);

[Ken Russell (switch to Gerrit), 2015/05/06 21:17:33]

Please add a new virtual function that returns the enum GL_FRAMEBUFFER in
WebGLRenderingContextBase or GL_DRAW_FRAMEBUFFER in WebGL2RenderingContextBase.
This would eliminate the multiple if-tests here.

[yunchao, 2015/05/22 09:54:35]

Done.

 
     restoreStateAfterClear();
-    if (m_framebufferBinding)
-        webContext()->bindFramebuffer(GL_FRAMEBUFFER, objectOrZero(m_framebufferBinding.get()));
+    if (m_framebufferBinding) {
+        if (!isWebGL2OrHigher())
+            webContext()->bindFramebuffer(GL_FRAMEBUFFER, objectOrZero(m_framebufferBinding.get()));
+        else
+            webContext()->bindFramebuffer(GL_DRAW_FRAMEBUFFER, objectOrZero(m_framebufferBinding.get()));
+    }
     drawingBuffer()->setBufferClearNeeded(false);
 
     return combinedClear ? CombinedClear : JustClear;
@@ -1026,8 +1035,17 @@ void WebGLRenderingContextBase::reshape(int width, int height)
 
     webContext()->bindTexture(GL_TEXTURE_2D, objectOrZero(m_textureUnits[m_activeTextureUnit].m_texture2DBinding.get()));
     webContext()->bindRenderbuffer(GL_RENDERBUFFER, objectOrZero(m_renderbufferBinding.get()));
-    if (m_framebufferBinding)
+    if (isWebGL2OrHigher()) {
+    // In GLES 3.0, bindFramebuffer(GL_FRAMEBUFFER, buffer1) = bindBuffer(GL_READ_FRAMEBUFFER, buffer1) +
+    // bindFramebuffer(GL_DRAW_FRAMEBUFFER, buffer1)
+        if (m_framebufferBinding)
+            webContext()->bindFramebuffer(GL_DRAW_FRAMEBUFFER, objectOrZero(m_framebufferBinding.get()));
+        WebGLFramebuffer* readFramebufferBinding = getFramebufferBinding(GL_READ_FRAMEBUFFER);
+        if (readFramebufferBinding)
+            webContext()->bindFramebuffer(GL_READ_FRAMEBUFFER, objectOrZero(readFramebufferBinding));
+    } else if (m_framebufferBinding) {
         webContext()->bindFramebuffer(GL_FRAMEBUFFER, objectOrZero(m_framebufferBinding.get()));
+    }
 }
 
 int WebGLRenderingContextBase::drawingBufferWidth() const
@@ -1375,7 +1393,9 @@ bool WebGLRenderingContextBase::validateFramebufferTarget(GLenum target)
 
 WebGLFramebuffer* WebGLRenderingContextBase::getFramebufferBinding(GLenum target)
 {
-    return m_framebufferBinding.get();
+    if (target == GL_FRAMEBUFFER)
+        return m_framebufferBinding.get();
+    return nullptr;
 }
 
 GLenum WebGLRenderingContextBase::checkFramebufferStatus(GLenum target)
@@ -1386,10 +1406,11 @@ GLenum WebGLRenderingContextBase::checkFramebufferStatus(GLenum target)
         synthesizeGLError(GL_INVALID_ENUM, "checkFramebufferStatus", "invalid target");
         return 0;
     }
-    if (!getFramebufferBinding(target) || !getFramebufferBinding(target)->object())
+    WebGLFramebuffer* framebufferBinding = getFramebufferBinding(target);
+    if (!framebufferBinding || !framebufferBinding->object())
         return GL_FRAMEBUFFER_COMPLETE;
     const char* reason = "framebuffer incomplete";
-    GLenum result = m_framebufferBinding->checkStatus(&reason);
+    GLenum result = framebufferBinding->checkStatus(&reason, target);
     if (result != GL_FRAMEBUFFER_COMPLETE) {
         emitGLWarning("checkFramebufferStatus", reason);
         return result;
@@ -1559,12 +1580,15 @@ void WebGLRenderingContextBase::copyTexImage2D(GLenum target, GLint level, GLenu
         return;
     }
     const char* reason = "framebuffer incomplete";
-    if (m_framebufferBinding && !m_framebufferBinding->onAccess(webContext(), &reason)) {
+    if ((m_framebufferBinding && !m_framebufferBinding->onAccess(webContext(), &reason))
+        || (getFramebufferBinding(GL_READ_FRAMEBUFFER) && !getFramebufferBinding(GL_READ_FRAMEBUFFER)->onAccess(webContext(), &reason, GL_READ_FRAMEBUFFER))) {
         synthesizeGLError(GL_INVALID_FRAMEBUFFER_OPERATION, "copyTexImage2D", reason);
         return;
     }
     clearIfComposited();
-    ScopedDrawingBufferBinder binder(drawingBuffer(), m_framebufferBinding.get());
+    ScopedDrawingBufferBinder binder(drawingBuffer(), m_framebufferBinding.get(), GL_FRAMEBUFFER);
+    if (isWebGL2OrHigher())
+        ScopedDrawingBufferBinder readBinder(drawingBuffer(), getFramebufferBinding(GL_READ_FRAMEBUFFER), GL_READ_FRAMEBUFFER);

[Ken Russell (switch to Gerrit), 2015/05/06 21:17:33]

This won't work. The scoping of readBinder ends outside the if-clause. Please
rethink this.

[yunchao, 2015/05/22 09:54:35]

I am very sorry, Ken. I didn't got what you said. What is the problem that the
readBinder ends outside the if-clause? when the function come to the end, the
destructor of readBinder will be executed for WebGL2RenderingContext. It seems
to be correct. Could you kindly explain the potential error? Many thanks!

     webContext()->copyTexImage2D(target, level, internalformat, x, y, width, height, border);
     // FIXME: if the framebuffer is not complete, none of the below should be executed.
     tex->setLevelInfo(target, level, internalformat, width, height, 1, GL_UNSIGNED_BYTE);
@@ -1602,12 +1626,15 @@ void WebGLRenderingContextBase::copyTexSubImage2D(GLenum target, GLint level, GL
         return;
     }
     const char* reason = "framebuffer incomplete";
-    if (m_framebufferBinding && !m_framebufferBinding->onAccess(webContext(), &reason)) {
+    if ((m_framebufferBinding && !m_framebufferBinding->onAccess(webContext(), &reason))
+        || (getFramebufferBinding(GL_READ_FRAMEBUFFER) && !getFramebufferBinding(GL_READ_FRAMEBUFFER)->onAccess(webContext(), &reason, GL_READ_FRAMEBUFFER))) {
         synthesizeGLError(GL_INVALID_FRAMEBUFFER_OPERATION, "copyTexSubImage2D", reason);
         return;
     }
     clearIfComposited();
-    ScopedDrawingBufferBinder binder(drawingBuffer(), m_framebufferBinding.get());
+    ScopedDrawingBufferBinder binder(drawingBuffer(), m_framebufferBinding.get(), GL_FRAMEBUFFER);
+    if (isWebGL2OrHigher())
+        ScopedDrawingBufferBinder readBinder(drawingBuffer(), getFramebufferBinding(GL_READ_FRAMEBUFFER), GL_READ_FRAMEBUFFER);

[Ken Russell (switch to Gerrit), 2015/05/06 21:17:33]

Same problem here.

     webContext()->copyTexSubImage2D(target, level, xoffset, yoffset, x, y, width, height);
 }
 
@@ -1751,6 +1778,8 @@ void WebGLRenderingContextBase::deleteRenderbuffer(WebGLRenderbuffer* renderbuff
         m_renderbufferBinding = nullptr;
     if (m_framebufferBinding)
         m_framebufferBinding->removeAttachmentFromBoundFramebuffer(renderbuffer);
+    if (getFramebufferBinding(GL_READ_FRAMEBUFFER))
+        getFramebufferBinding(GL_READ_FRAMEBUFFER)->removeAttachmentFromBoundFramebuffer(renderbuffer, GL_READ_FRAMEBUFFER);
 }
 
 void WebGLRenderingContextBase::deleteShader(WebGLShader* shader)
@@ -1788,6 +1817,8 @@ void WebGLRenderingContextBase::deleteTexture(WebGLTexture* texture)
     }
     if (m_framebufferBinding)
         m_framebufferBinding->removeAttachmentFromBoundFramebuffer(texture);
+    if (getFramebufferBinding(GL_READ_FRAMEBUFFER))
+        getFramebufferBinding(GL_READ_FRAMEBUFFER)->removeAttachmentFromBoundFramebuffer(texture, GL_READ_FRAMEBUFFER);
 
     // If the deleted was bound to the the current maximum index, trace backwards to find the new max texture index
     if (m_onePlusMaxNonDefaultTextureUnit == static_cast<unsigned long>(maxBoundTextureIndex + 1)) {
@@ -2007,13 +2038,15 @@ void WebGLRenderingContextBase::framebufferRenderbuffer(GLenum target, GLenum at
     // Don't allow the default framebuffer to be mutated; all current
     // implementations use an FBO internally in place of the default
     // FBO.
-    if (!m_framebufferBinding || !m_framebufferBinding->object()) {
+    WebGLFramebuffer* framebufferBinding = getFramebufferBinding(target);
+    if (!framebufferBinding || !framebufferBinding->object()) {
         synthesizeGLError(GL_INVALID_OPERATION, "framebufferRenderbuffer", "no framebuffer bound");
         return;
     }
     Platform3DObject bufferObject = objectOrZero(buffer);
     switch (attachment) {
     case GC3D_DEPTH_STENCIL_ATTACHMENT_WEBGL:
+        // TODO. are there any devices can support depth_stencil buffer directly, not use oes_depth_stencil extension?
         if (isDepthStencilSupported() || !buffer) {
             webContext()->framebufferRenderbuffer(target, GL_DEPTH_ATTACHMENT, renderbuffertarget, bufferObject);
             webContext()->framebufferRenderbuffer(target, GL_STENCIL_ATTACHMENT, renderbuffertarget, bufferObject);
@@ -2030,7 +2063,7 @@ void WebGLRenderingContextBase::framebufferRenderbuffer(GLenum target, GLenum at
     default:
         webContext()->framebufferRenderbuffer(target, attachment, renderbuffertarget, bufferObject);
     }
-    m_framebufferBinding->setAttachmentForBoundFramebuffer(attachment, buffer);
+    framebufferBinding->setAttachmentForBoundFramebuffer(target, attachment, buffer);
     applyStencilTest();
 }
 
@@ -2049,7 +2082,8 @@ void WebGLRenderingContextBase::framebufferTexture2D(GLenum target, GLenum attac
     // Don't allow the default framebuffer to be mutated; all current
     // implementations use an FBO internally in place of the default
     // FBO.
-    if (!m_framebufferBinding || !m_framebufferBinding->object()) {
+    WebGLFramebuffer* framebufferBinding = getFramebufferBinding(target);
+    if (!framebufferBinding || !framebufferBinding->object()) {
         synthesizeGLError(GL_INVALID_OPERATION, "framebufferTexture2D", "no framebuffer bound");
         return;
     }
@@ -2059,16 +2093,11 @@ void WebGLRenderingContextBase::framebufferTexture2D(GLenum target, GLenum attac
         webContext()->framebufferTexture2D(target, GL_DEPTH_ATTACHMENT, textarget, textureObject, level);
         webContext()->framebufferTexture2D(target, GL_STENCIL_ATTACHMENT, textarget, textureObject, level);
         break;
-    case GL_DEPTH_ATTACHMENT:
-        webContext()->framebufferTexture2D(target, attachment, textarget, textureObject, level);
-        break;
-    case GL_STENCIL_ATTACHMENT:
-        webContext()->framebufferTexture2D(target, attachment, textarget, textureObject, level);
-        break;
     default:
         webContext()->framebufferTexture2D(target, attachment, textarget, textureObject, level);
     }
-    m_framebufferBinding->setAttachmentForBoundFramebuffer(attachment, textarget, texture, level);
+    // TODO: Check whether this is correct

[bajones, 2015/05/06 17:47:54]

Let's figure this out before landing the patch.

[yunchao, 2015/05/22 09:54:35]

I think this is correct. So remove this TODO.

+    framebufferBinding->setAttachmentForBoundFramebuffer(target, attachment, textarget, texture, level);
     applyStencilTest();
 }
 
@@ -3322,7 +3351,9 @@ void WebGLRenderingContextBase::readPixels(GLint x, GLint y, GLsizei width, GLsi
         return;
     }
     const char* reason = "framebuffer incomplete";
-    if (m_framebufferBinding && !m_framebufferBinding->onAccess(webContext(), &reason)) {
+    GLenum target = isWebGL2OrHigher() ? GL_READ_FRAMEBUFFER : GL_FRAMEBUFFER;
+    WebGLFramebuffer* readFramebufferBinding = getFramebufferBinding(target);
+    if (readFramebufferBinding && !readFramebufferBinding->onAccess(webContext(), &reason, target)) {
         synthesizeGLError(GL_INVALID_FRAMEBUFFER_OPERATION, "readPixels", reason);
         return;
     }
@@ -3343,14 +3374,14 @@ void WebGLRenderingContextBase::readPixels(GLint x, GLint y, GLsizei width, GLsi
     void* data = pixels->baseAddress();
 
     {
-        ScopedDrawingBufferBinder binder(drawingBuffer(), m_framebufferBinding.get());
+        ScopedDrawingBufferBinder binder(drawingBuffer(), readFramebufferBinding, target);
         webContext()->readPixels(x, y, width, height, format, type, data);
     }
 
 #if OS(MACOSX)
     // FIXME: remove this section when GL driver bug on Mac is fixed, i.e.,
     // when alpha is off, readPixels should set alpha to 255 instead of 0.
-    if (!m_framebufferBinding && !drawingBuffer()->getActualAttributes().alpha) {
+    if (!readFramebufferBinding && !drawingBuffer()->getActualAttributes().alpha) {
         unsigned char* pixels = reinterpret_cast<unsigned char*>(data);
         for (GLsizei iy = 0; iy < height; ++iy) {
             for (GLsizei ix = 0; ix < width; ++ix) {
@@ -6081,7 +6112,7 @@ void WebGLRenderingContextBase::setFramebuffer(GLenum target, WebGLFramebuffer*
         m_framebufferBinding = buffer;
         applyStencilTest();
     }
-    drawingBuffer()->setFramebufferBinding(objectOrZero(m_framebufferBinding.get()));
+    drawingBuffer()->setFramebufferBinding(objectOrZero(getFramebufferBinding(target)), target);
 
     if (!buffer) {
         // Instead of binding fb 0, bind the drawing buffer.