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 177893cd0e25848484b4d2a88363ac1d16799430..2a4276b7e9a77be4a036768a0c5f125f4e2b872a 100644
--- a/Source/core/html/canvas/WebGLRenderingContextBase.cpp
+++ b/Source/core/html/canvas/WebGLRenderingContextBase.cpp
@@ -221,28 +221,33 @@ void WebGLRenderingContextBase::willDestroyContext(WebGLRenderingContextBase* co
 
 namespace {
 
+    // ScopedDrawingBufferBinder is used for ReadPixels/CopyTexImage2D/CopySubImage2D to read from
+    // a multisampled DrawingBuffer. In this situation, we need to blit to a single sampled buffer
+    // for reading, during which the bindings could be changed and need to be recovered.
     class ScopedDrawingBufferBinder {
         STACK_ALLOCATED();
     public:
-        ScopedDrawingBufferBinder(DrawingBuffer* drawingBuffer, WebGLFramebuffer* framebufferBinding)
+        ScopedDrawingBufferBinder(DrawingBuffer* drawingBuffer, WebGLFramebuffer* framebufferBinding, GLenum target)
             : m_drawingBuffer(drawingBuffer)
-            , m_framebufferBinding(framebufferBinding)
+            , m_readFramebufferBinding(framebufferBinding)
+            , m_target(target)
         {
             // Commit DrawingBuffer if needed (e.g., for multisampling)
-            if (!m_framebufferBinding && m_drawingBuffer)
-                m_drawingBuffer->commit();
+            if (!m_readFramebufferBinding && m_drawingBuffer)
+                m_drawingBuffer->commit(m_target);
         }
 
         ~ScopedDrawingBufferBinder()
         {
             // Restore DrawingBuffer if needed
-            if (!m_framebufferBinding && m_drawingBuffer)
-                m_drawingBuffer->bind();
+            if (!m_readFramebufferBinding && m_drawingBuffer)
+                m_drawingBuffer->bind(m_target);

[Zhenyao Mo, 2015/06/10 19:35:41]

Instead of calling m_drawingBuffer->bind(m_target), you should call
m_drawingBuffer->restoreFramebufferBindings(). This is because in commit, not
just the read binding target, but the draw binding target could be disturbed in
WebGL 2, and you didn't recover the draw binding target.

So this way, you actually don't need the target parameter.

(In theory you also don't need the readFramebufferBinding parameter because
drawingBuffer should have it, but it's OK to be redundant here.)

[yunchao, 2015/06/11 09:50:13]

Yes, you are right. I removed the m_target variable. In DrawingBuffer::commit,
just binding m_fbo to GL_FRAMEBUFFER binding point is OK. we will restore
read/draw framebuffer after reading. 

         }
 
     private:
         DrawingBuffer* m_drawingBuffer;
-        RawPtrWillBeMember<WebGLFramebuffer> m_framebufferBinding;
+        RawPtrWillBeMember<WebGLFramebuffer> m_readFramebufferBinding;
+        GLenum m_target;
     };
 
     GLint clamp(GLint value, GLint min, GLint max)
@@ -651,7 +656,7 @@ WebGLRenderingContextBase::WebGLRenderingContextBase(HTMLCanvasElement* passedCa
 
     m_drawingBuffer = buffer.release();
 
-    drawingBuffer()->bind();
+    drawingBuffer()->bind(GL_FRAMEBUFFER);
     setupFlags();
 }
 
@@ -912,8 +917,7 @@ WebGLRenderingContextBase::HowToClear WebGLRenderingContextBase::clearIfComposit
     drawingBuffer()->clearFramebuffers(clearMask);
 
     restoreStateAfterClear();
-    if (m_framebufferBinding)
-        webContext()->bindFramebuffer(GL_FRAMEBUFFER, objectOrZero(m_framebufferBinding.get()));
+    drawingBuffer()->restoreFramebufferBindings();
     drawingBuffer()->setBufferClearNeeded(false);
 
     return combinedClear ? CombinedClear : JustClear;
@@ -964,7 +968,7 @@ bool WebGLRenderingContextBase::paintRenderingResultsToCanvas(SourceDrawingBuffe
     ScopedTexture2DRestorer restorer(this);
     ScopedFramebufferRestorer fboRestorer(this);
 
-    drawingBuffer()->commit();
+    drawingBuffer()->commit(GL_FRAMEBUFFER);
     if (!canvas()->buffer()->copyRenderingResultsFromDrawingBuffer(drawingBuffer(), sourceBuffer)) {
         canvas()->ensureUnacceleratedImageBuffer();
         if (canvas()->hasImageBuffer())
@@ -982,7 +986,7 @@ ImageData* WebGLRenderingContextBase::paintRenderingResultsToImageData(SourceDra
         return nullptr;
 
     clearIfComposited();
-    drawingBuffer()->commit();
+    drawingBuffer()->commit(GL_FRAMEBUFFER);
     ScopedFramebufferRestorer restorer(this);
     int width, height;
     WTF::ArrayBufferContents contents;
@@ -1018,8 +1022,7 @@ 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)
-        webContext()->bindFramebuffer(GL_FRAMEBUFFER, objectOrZero(m_framebufferBinding.get()));
+    drawingBuffer()->restoreFramebufferBindings();
 }
 
 int WebGLRenderingContextBase::drawingBufferWidth() const
@@ -1370,7 +1373,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)
@@ -1381,10 +1386,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);
     if (result != GL_FRAMEBUFFER_COMPLETE) {
         emitGLWarning("checkFramebufferStatus", reason);
         return result;
@@ -1402,7 +1408,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;
     }
@@ -1563,12 +1569,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());
+    GLenum framebufferTarget = isWebGL2OrHigher() ? GL_READ_FRAMEBUFFER : GL_FRAMEBUFFER;
+    WebGLFramebuffer* readFramebufferBinding = getFramebufferBinding(framebufferTarget);
+    ScopedDrawingBufferBinder binder(drawingBuffer(), readFramebufferBinding, framebufferTarget);
     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);
@@ -1606,12 +1615,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());
+    GLenum framebufferTarget = isWebGL2OrHigher() ? GL_READ_FRAMEBUFFER : GL_FRAMEBUFFER;
+    WebGLFramebuffer* readFramebufferBinding = getFramebufferBinding(framebufferTarget);
+    ScopedDrawingBufferBinder binder(drawingBuffer(), readFramebufferBinding, framebufferTarget);
     webContext()->copyTexSubImage2D(target, level, xoffset, yoffset, x, y, width, height);
 }
 
@@ -1731,9 +1743,9 @@ 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()->bind();
+        drawingBuffer()->setFramebufferBinding(GL_FRAMEBUFFER, 0);
+        // Have to call drawingBuffer()->bind() here to bind back to internal fbo.
+        drawingBuffer()->bind(GL_FRAMEBUFFER);
     }
 }
 
@@ -1751,7 +1763,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)
@@ -1788,7 +1802,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)) {
@@ -2008,7 +2024,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;
     }
@@ -2031,7 +2048,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();
 }
 
@@ -2050,7 +2067,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;
     }
@@ -2060,16 +2078,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:

[Zhenyao Mo, 2015/06/10 19:35:41]

You haven't answered my question: why removing these two?

[yunchao, 2015/06/11 09:50:13]

Sorry... seems that this code snippet is redundant: depth / stencil attachment
don't need particular operation. They are exactly the same with the code in
default branch, just like color attachment, who will go to the default branch.

I have added these two again.

-        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();
 }
 
@@ -3323,7 +3335,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;
     }
@@ -3344,14 +3358,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) {
@@ -4629,7 +4643,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();
 
@@ -5732,7 +5746,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;
     }
@@ -5784,7 +5798,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;
     }
@@ -5942,7 +5956,7 @@ void WebGLRenderingContextBase::maybeRestoreContext(Timer<WebGLRenderingContextB
 
     m_drawingBuffer = buffer.release();
 
-    drawingBuffer()->bind();
+    drawingBuffer()->bind(GL_FRAMEBUFFER);
     m_lostContextErrors.clear();
     m_contextLostMode = NotLostContext;
     m_autoRecoveryMethod = Manual;
@@ -6113,7 +6127,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.