update getFramebufferAttachmentParameter for WebGL 2

Source/core/html/canvas/WebGLRenderingContextBase.cpp

 /*
  * Copyright (C) 2009 Apple 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:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 2278 matching lines @@
 ScriptValue WebGLRenderingContextBase::getFramebufferAttachmentParameter(ScriptState* scriptState, GLenum target, GLenum attachment, GLenum pname)
 {
     if (isContextLost() || !validateFramebufferFuncParameters("getFramebufferAttachmentParameter", target, attachment))
         return ScriptValue::createNull(scriptState);
 
     if (!m_framebufferBinding || !m_framebufferBinding->object()) {
         synthesizeGLError(GL_INVALID_OPERATION, "getFramebufferAttachmentParameter", "no framebuffer bound");
         return ScriptValue::createNull(scriptState);
     }
 
-    WebGLSharedObject* object = m_framebufferBinding->getAttachmentObject(attachment);
-    if (!object) {
+    WebGLSharedObject* attachmentObject = m_framebufferBinding->getAttachmentObject(attachment);
+    if (!attachmentObject) {
         if (pname == GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE)
             return WebGLAny(scriptState, GL_NONE);
         // OpenGL ES 2.0 specifies INVALID_ENUM in this case, while desktop GL
         // specifies INVALID_OPERATION.
         synthesizeGLError(GL_INVALID_ENUM, "getFramebufferAttachmentParameter", "invalid parameter name");
         return ScriptValue::createNull(scriptState);
     }
 
-    ASSERT(object->isTexture() || object->isRenderbuffer());
-    if (object->isTexture()) {
+    ASSERT(attachmentObject->isTexture() || attachmentObject->isRenderbuffer());
+    if (attachmentObject->isTexture()) {
         switch (pname) {
         case GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE:
             return WebGLAny(scriptState, GL_TEXTURE);
         case GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME:
-            return WebGLAny(scriptState, PassRefPtrWillBeRawPtr<WebGLObject>(object));
+            return WebGLAny(scriptState, PassRefPtrWillBeRawPtr<WebGLObject>(attachmentObject));
         case GL_FRAMEBUFFER_ATTACHMENT_TEXTURE_LEVEL:
         case GL_FRAMEBUFFER_ATTACHMENT_TEXTURE_CUBE_MAP_FACE:
             {
                 GLint value = 0;
                 webContext()->getFramebufferAttachmentParameteriv(target, attachment, pname, &value);
                 return WebGLAny(scriptState, value);
             }
         case GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING_EXT:
-            if (extensionEnabled(EXTsRGBName) || isWebGL2OrHigher()) {
+            if (extensionEnabled(EXTsRGBName)) {
                 GLint value = 0;
                 webContext()->getFramebufferAttachmentParameteriv(target, attachment, pname, &value);
-                return WebGLAny(scriptState, value);
+                return WebGLAny(scriptState, static_cast<unsigned>(value));
             }
             synthesizeGLError(GL_INVALID_ENUM, "getFramebufferAttachmentParameter", "invalid parameter name for renderbuffer attachment");
             return ScriptValue::createNull(scriptState);
         default:
             synthesizeGLError(GL_INVALID_ENUM, "getFramebufferAttachmentParameter", "invalid parameter name for texture attachment");
             return ScriptValue::createNull(scriptState);
         }
     } else {
         switch (pname) {
         case GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE:
             return WebGLAny(scriptState, GL_RENDERBUFFER);
         case GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME:
-            return WebGLAny(scriptState, PassRefPtrWillBeRawPtr<WebGLObject>(object));
+            return WebGLAny(scriptState, PassRefPtrWillBeRawPtr<WebGLObject>(attachmentObject));
         case GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING_EXT:
             if (extensionEnabled(EXTsRGBName) || isWebGL2OrHigher()) {
                 GLint value = 0;
                 webContext()->getFramebufferAttachmentParameteriv(target, attachment, pname, &value);
                 return WebGLAny(scriptState, value);
             }
             synthesizeGLError(GL_INVALID_ENUM, "getFramebufferAttachmentParameter", "invalid parameter name for renderbuffer attachment");
             return ScriptValue::createNull(scriptState);
         default:
             synthesizeGLError(GL_INVALID_ENUM, "getFramebufferAttachmentParameter", "invalid parameter name for renderbuffer attachment");
@@ skipping 3712 matching lines @@
     m_backDrawBuffer = buf;
 }
 
 void WebGLRenderingContextBase::setFramebuffer(GLenum target, WebGLFramebuffer* buffer)
 {
     if (buffer)
         buffer->setHasEverBeenBound();
 
     if (target == GL_FRAMEBUFFER || target == GL_DRAW_FRAMEBUFFER) {
         m_framebufferBinding = buffer;
-        drawingBuffer()->setFramebufferBinding(objectOrZero(m_framebufferBinding.get()));
+        applyStencilTest();
+    }
+    drawingBuffer()->setFramebufferBinding(objectOrZero(m_framebufferBinding.get()));

[Ken Russell (switch to Gerrit), 2015/04/30 02:28:26]

Looking more deeply at the DrawingBuffer code, it resets the GL_FRAMEBUFFER
binding target internally, which will destroy the GL_READ_FRAMEBUFFER binding if
it was set by the user.

The more I look at this code the less correct it seems. The exposure of the
separate DRAW_FRAMEBUFFER and READ_FRAMEBUFFER binding points in GLES3 changes
all of the code in WebGLRenderingContextBase that deals with the default drawing
buffer, and the DrawingBuffer class itself.

Do you see the same problems here that I do? If so, what is your plan for making
this code correct? User-level multisampled renderbuffers, and the exposure of
blitFramebuffer, is a major feature in WebGL 2.0, and if we do a half-baked job
on this code along the way then we are setting ourselves up for a huge debugging
effort later.

 
-        if (!m_framebufferBinding) {
-            // Instead of binding fb 0, bind the drawing buffer.
-            drawingBuffer()->bind();
-        } else {
-            webContext()->bindFramebuffer(target, objectOrZero(buffer));
-        }
-
-        applyStencilTest();
+    if (!buffer) {
+        // Instead of binding fb 0, bind the drawing buffer.
+        drawingBuffer()->bind(target);
     } else {
-        webContext()->bindFramebuffer(target, objectOrZero(buffer));
+        webContext()->bindFramebuffer(target, buffer->object());
     }
 }
 
 void WebGLRenderingContextBase::restoreCurrentFramebuffer()
 {
     bindFramebuffer(GL_FRAMEBUFFER, m_framebufferBinding.get());
 }
 
 void WebGLRenderingContextBase::restoreCurrentTexture2D()
 {
@@ skipping 90 matching lines @@
     return m_sharedWebGraphicsContext3D ? m_sharedWebGraphicsContext3D->drawingBuffer() : 0;
 }
 #else
 DrawingBuffer* WebGLRenderingContextBase::drawingBuffer() const
 {
     return m_drawingBuffer.get();
 }
 #endif
 
 } // namespace blink