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 81550c6b5c79f743e923a2ce6fb6608fa4b07017..eea188f0c6ab55b05617266a581ad515dfaf7fc7 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)

[Zhenyao Mo, 2015/06/04 22:08:16]

No need for target.  As I pointed out in DrawingBuffer, both commit and bind do
not need to take a target arg.

[yunchao, 2015/06/09 10:37:54]

it is not appropriate to remove target from bind(). see the comments in
DrawingBuffer

             : 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)
@@ -873,6 +875,11 @@ void WebGLRenderingContextBase::markContextChanged(ContentChangeType changeType)
     }
 }
 
+GLenum WebGLRenderingContextBase::getDrawFramebufferTarget()

[Zhenyao Mo, 2015/06/04 22:08:16]

I think this function is unnecessary.

See comments where it's called.

[yunchao, 2015/06/09 10:37:54]

Acknowledged.

+{
+    return GL_FRAMEBUFFER;
+}
+
 WebGLRenderingContextBase::HowToClear WebGLRenderingContextBase::clearIfComposited(GLbitfield mask)
 {
     if (isContextLost())
@@ -917,11 +924,13 @@ WebGLRenderingContextBase::HowToClear WebGLRenderingContextBase::clearIfComposit
         webContext()->stencilMaskSeparate(GL_FRONT, 0xFFFFFFFF);
     }
 
-    drawingBuffer()->clearFramebuffers(clearMask);
+    GLenum target = getDrawFramebufferTarget();
+    drawingBuffer()->clearFramebuffers(target, clearMask);

[Zhenyao Mo, 2015/06/04 22:08:16]

See DrawingBuffer, target isn't necessary.

[yunchao, 2015/06/09 10:37:54]

Acknowledged.

 
     restoreStateAfterClear();
-    if (m_framebufferBinding)
-        webContext()->bindFramebuffer(GL_FRAMEBUFFER, objectOrZero(m_framebufferBinding.get()));
+    if (m_framebufferBinding) {
+        webContext()->bindFramebuffer(target, objectOrZero(m_framebufferBinding.get()));

[Zhenyao Mo, 2015/06/04 22:08:16]

I think you should just call DrawingBuffer's restoreFramebufferBindings() to do
this, to avoid code duplication.

[yunchao, 2015/06/09 10:37:54]

Acknowledged.

+    }
     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()) {

[Zhenyao Mo, 2015/06/04 22:08:16]

Again, call DrawBuffer's restoreFramebufferBindings().

[yunchao, 2015/06/09 10:37:54]

Acknowledged.

+    // 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;
@@ -1407,7 +1428,7 @@ void WebGLRenderingContextBase::clear(GLbitfield mask)
         return;
     }
     const char* reason = "framebuffer incomplete";
-    if (m_framebufferBinding && !m_framebufferBinding->onAccess(webContext(), &reason)) {
+    if (m_framebufferBinding && !m_framebufferBinding->onAccess(webContext(), GL_FRAMEBUFFER, &reason)) {
         synthesizeGLError(GL_INVALID_FRAMEBUFFER_OPERATION, "clear", reason);
         return;
     }
@@ -1568,12 +1589,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(), GL_FRAMEBUFFER, &reason))
+        || (getFramebufferBinding(GL_READ_FRAMEBUFFER) && !getFramebufferBinding(GL_READ_FRAMEBUFFER)->onAccess(webContext(), GL_READ_FRAMEBUFFER, &reason))) {
         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);
     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);
@@ -1611,12 +1635,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(), GL_FRAMEBUFFER, &reason))
+        || (getFramebufferBinding(GL_READ_FRAMEBUFFER) && !getFramebufferBinding(GL_READ_FRAMEBUFFER)->onAccess(webContext(), GL_READ_FRAMEBUFFER, &reason))) {
         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);
     webContext()->copyTexSubImage2D(target, level, xoffset, yoffset, x, y, width, height);
 }
 
@@ -1739,8 +1766,8 @@ void WebGLRenderingContextBase::deleteFramebuffer(WebGLFramebuffer* framebuffer)
         return;
     if (framebuffer == m_framebufferBinding) {
         m_framebufferBinding = nullptr;
-        drawingBuffer()->setFramebufferBinding(0);
-        // Have to call bindFramebuffer here to bind back to internal fbo.
+        drawingBuffer()->setFramebufferBinding(GL_FRAMEBUFFER, 0);
+        // Have to call drawingBuffer()->bind() here to bind back to internal fbo.
         drawingBuffer()->bind();
     }
 }
@@ -1759,7 +1786,9 @@ void WebGLRenderingContextBase::deleteRenderbuffer(WebGLRenderbuffer* renderbuff
     if (renderbuffer == m_renderbufferBinding)
         m_renderbufferBinding = nullptr;
     if (m_framebufferBinding)
-        m_framebufferBinding->removeAttachmentFromBoundFramebuffer(renderbuffer);
+        m_framebufferBinding->removeAttachmentFromBoundFramebuffer(GL_FRAMEBUFFER, renderbuffer);
+    if (getFramebufferBinding(GL_READ_FRAMEBUFFER))
+        getFramebufferBinding(GL_READ_FRAMEBUFFER)->removeAttachmentFromBoundFramebuffer(GL_READ_FRAMEBUFFER, renderbuffer);
 }
 
 void WebGLRenderingContextBase::deleteShader(WebGLShader* shader)
@@ -1796,7 +1825,9 @@ void WebGLRenderingContextBase::deleteTexture(WebGLTexture* texture)
         }
     }
     if (m_framebufferBinding)
-        m_framebufferBinding->removeAttachmentFromBoundFramebuffer(texture);
+        m_framebufferBinding->removeAttachmentFromBoundFramebuffer(GL_FRAMEBUFFER, texture);
+    if (getFramebufferBinding(GL_READ_FRAMEBUFFER))
+        getFramebufferBinding(GL_READ_FRAMEBUFFER)->removeAttachmentFromBoundFramebuffer(GL_READ_FRAMEBUFFER, texture);
 
     // 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)) {
@@ -2016,7 +2047,8 @@ 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;
     }
@@ -2039,7 +2071,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();
 }
 
@@ -2058,7 +2090,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;
     }
@@ -2068,16 +2101,10 @@ 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);
+    framebufferBinding->setAttachmentForBoundFramebuffer(target, attachment, textarget, texture, level);
     applyStencilTest();
 }
 
@@ -3331,7 +3358,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(), target, &reason)) {
         synthesizeGLError(GL_INVALID_FRAMEBUFFER_OPERATION, "readPixels", reason);
         return;
     }
@@ -3352,14 +3381,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) {
@@ -4627,7 +4656,7 @@ void WebGLRenderingContextBase::loseContextImpl(WebGLRenderingContextBase::LostC
 
     // Make absolutely sure we do not refer to an already-deleted texture or framebuffer.
     drawingBuffer()->setTexture2DBinding(0);
-    drawingBuffer()->setFramebufferBinding(0);
+    drawingBuffer()->setFramebufferBinding(GL_FRAMEBUFFER, 0);
 
     detachAndRemoveAllObjects();
 
@@ -5722,7 +5751,7 @@ bool WebGLRenderingContextBase::validateDrawArrays(const char* functionName, GLe
     }
 
     const char* reason = "framebuffer incomplete";
-    if (m_framebufferBinding && !m_framebufferBinding->onAccess(webContext(), &reason)) {
+    if (m_framebufferBinding && !m_framebufferBinding->onAccess(webContext(), GL_FRAMEBUFFER, &reason)) {
         synthesizeGLError(GL_INVALID_FRAMEBUFFER_OPERATION, functionName, reason);
         return false;
     }
@@ -5774,7 +5803,7 @@ bool WebGLRenderingContextBase::validateDrawElements(const char* functionName, G
     }
 
     const char* reason = "framebuffer incomplete";
-    if (m_framebufferBinding && !m_framebufferBinding->onAccess(webContext(), &reason)) {
+    if (m_framebufferBinding && !m_framebufferBinding->onAccess(webContext(), GL_FRAMEBUFFER, &reason)) {
         synthesizeGLError(GL_INVALID_FRAMEBUFFER_OPERATION, functionName, reason);
         return false;
     }
@@ -6114,7 +6143,7 @@ void WebGLRenderingContextBase::setFramebuffer(GLenum target, WebGLFramebuffer*
         m_framebufferBinding = buffer;
         applyStencilTest();
     }
-    drawingBuffer()->setFramebufferBinding(objectOrZero(m_framebufferBinding.get()));
+    drawingBuffer()->setFramebufferBinding(target, objectOrZero(getFramebufferBinding(target)));
 
     if (!buffer) {
         // Instead of binding fb 0, bind the drawing buffer.