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

Source/platform/graphics/gpu/DrawingBuffer.cpp

 /*
  * Copyright (c) 2010, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 105 matching lines @@
     PassOwnPtr<Extensions3DUtil> extensionsUtil,
     bool multisampleExtensionSupported,
     bool packedDepthStencilExtensionSupported,
     bool discardFramebufferSupported,
     PreserveDrawingBuffer preserve,
     WebGraphicsContext3D::Attributes requestedAttributes)
     : m_preserveDrawingBuffer(preserve)
     , m_scissorEnabled(false)
     , m_texture2DBinding(0)
     , m_framebufferBinding(0)
+    , m_readFramebufferBinding(0)
     , m_activeTextureUnit(GL_TEXTURE0)
     , m_context(context)
     , m_extensionsUtil(extensionsUtil)
     , m_size(-1, -1)
     , m_requestedAttributes(requestedAttributes)
     , m_multisampleExtensionSupported(multisampleExtensionSupported)
     , m_packedDepthStencilExtensionSupported(packedDepthStencilExtensionSupported)
     , m_discardFramebufferSupported(discardFramebufferSupported)
     , m_fbo(0)
     , m_depthStencilBuffer(0)
@@ skipping 562 matching lines @@
             else
                 m_context->renderbufferStorage(GL_RENDERBUFFER, GL_STENCIL_INDEX8, size.width(), size.height());
             m_context->framebufferRenderbuffer(GL_FRAMEBUFFER, GL_STENCIL_ATTACHMENT, GL_RENDERBUFFER, m_stencilBuffer);
         }
     }
     m_context->bindRenderbuffer(GL_RENDERBUFFER, 0);
 }
 
 
 
-void DrawingBuffer::clearFramebuffers(GLbitfield clearMask)
+void DrawingBuffer::clearFramebuffers(GLenum target, GLbitfield clearMask)

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

No need for the target, the original code is correct.  It's always correct to
bind to GL_FRAMEBUFFER for clearing.  We restore later both READ and DRAW
binding points.

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

Acknowledged.

 {
+    ASSERT(target == GL_FRAMEBUFFER || target == GL_DRAW_FRAMEBUFFER);
     // We will clear the multisample FBO, but we also need to clear the non-multisampled buffer.
     if (m_multisampleFBO) {
-        m_context->bindFramebuffer(GL_FRAMEBUFFER, m_fbo);
+        m_context->bindFramebuffer(target, m_fbo);
         m_context->clear(GL_COLOR_BUFFER_BIT);
     }
 
-    m_context->bindFramebuffer(GL_FRAMEBUFFER, m_multisampleFBO ? m_multisampleFBO : m_fbo);
+    m_context->bindFramebuffer(target, m_multisampleFBO ? m_multisampleFBO : m_fbo);
     m_context->clear(clearMask);
 }
 
 void DrawingBuffer::setSize(const IntSize& size)
 {
     if (m_size == size)
         return;
 
     m_size = size;
 }
@@ skipping 44 matching lines @@
         m_context->clearDepth(1.0f);
         clearMask |= GL_DEPTH_BUFFER_BIT;
         m_context->depthMask(true);
     }
     if (m_actualAttributes.stencil) {
         m_context->clearStencil(0);
         clearMask |= GL_STENCIL_BUFFER_BIT;
         m_context->stencilMaskSeparate(GL_FRONT, 0xFFFFFFFF);
     }
 
-    clearFramebuffers(clearMask);
+    clearFramebuffers(GL_FRAMEBUFFER, clearMask);
     return true;
 }
 
-void DrawingBuffer::commit()
+void DrawingBuffer::commit(GLenum target)

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

|target| is not needed.

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

Acknowledged.

 {
     if (m_multisampleFBO && !m_contentsChangeCommitted) {
         m_context->bindFramebuffer(GL_READ_FRAMEBUFFER_ANGLE, m_multisampleFBO);
         m_context->bindFramebuffer(GL_DRAW_FRAMEBUFFER_ANGLE, m_fbo);
 
         if (m_scissorEnabled)
             m_context->disable(GL_SCISSOR_TEST);
 
         int width = m_size.width();
         int height = m_size.height();
         // Use NEAREST, because there is no scale performed during the blit.
         m_context->blitFramebufferCHROMIUM(0, 0, width, height, 0, 0, width, height, GL_COLOR_BUFFER_BIT, GL_NEAREST);
 
         if (m_scissorEnabled)
             m_context->enable(GL_SCISSOR_TEST);
     }
 
-    m_context->bindFramebuffer(GL_FRAMEBUFFER, m_fbo);
+    m_context->bindFramebuffer(target, m_fbo);

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

Instead of calling this directly, you should call restoreFramebufferBindings().

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

Acknowledged.

     m_contentsChangeCommitted = true;
 }
 
-void DrawingBuffer::restoreFramebufferBinding()
+void DrawingBuffer::restoreFramebufferBinding(GLenum target)

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

It should not have a target, but restore both READ and DRAW bindings always.  So
maybe rename this to restoreFramebufferBindings().

If m_readFramebufferBinding and m_drawFramebufferBinding is the same, it could
be just one call with GL_FRAMEBUFFER (always the case in ES2), or it needs to be
restore separately sometimes in ES3 when the two bindings differ.

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

Acknowledged.

 {
-    if (!m_framebufferBinding) {
-        bind();
+    if (((target == GL_FRAMEBUFFER || target == GL_DRAW_FRAMEBUFFER) && !m_framebufferBinding)
+        || (target == GL_READ_FRAMEBUFFER && !m_readFramebufferBinding)) {
+        bind(target);
         return;
     }
-    m_context->bindFramebuffer(GL_FRAMEBUFFER, m_framebufferBinding);
+    m_context->bindFramebuffer(target, target == GL_READ_FRAMEBUFFER ? m_readFramebufferBinding : m_framebufferBinding);
 }
 
 bool DrawingBuffer::multisample() const
 {
     return m_multisampleMode != None;
 }
 
 void DrawingBuffer::bind(GLenum target)
 {
     m_context->bindFramebuffer(target, m_multisampleFBO ? m_multisampleFBO : m_fbo);

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

I think we should get rid of the target in this function.  If m_multisampleFBO
isn't null, we should always bind m_multisampleFBO to DRAW_FRAMEBUFFER, and
m_fbo to READ_FRAMEBUFFER; otherwise, we should always bind m_fbo to just
FRAMEBUFFER.

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

In WebGL 1.0, there may have both m_multisampleFBO and m_fbo when
render_to_texture is enabled for tiled gpu. But there is no DRAW or READ
framebuffer binding point separately, framebuffer in WebGL 1.0 just have
GL_FRAMEBUFFER binding point.

So I prefer to keep target parameter. 

On the other hand, I think that it is a good idea to bind draw framebuffer to
m_multisampleFBO if it exists, and bind read framebuffer to m_fbo against WebGL
2.0.

 }
 
 void DrawingBuffer::setPackAlignment(GLint param)
 {
     m_packAlignment = param;
 }
 
 void DrawingBuffer::paintRenderingResultsToCanvas(ImageBuffer* imageBuffer)
 {
     paintFramebufferToCanvas(framebuffer(), size().width(), size().height(), !m_actualAttributes.premultipliedAlpha, imageBuffer);
@@ skipping 147 matching lines @@
 void DrawingBuffer::deleteChromiumImageForTexture(TextureInfo* info)
 {
     if (info->imageId) {
         m_context->releaseTexImage2DCHROMIUM(GL_TEXTURE_2D, info->imageId);
         m_context->destroyImageCHROMIUM(info->imageId);
         info->imageId = 0;
     }
 }
 
 } // namespace blink