update getFramebufferAttachmentParameter for WebGL 2

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 bfd90ea3a27b545759bf0e24f38a4a06cba908b0..003d3743da0f9bd94f749dfcbdec5fe708808efb 100644
--- a/Source/core/html/canvas/WebGLRenderingContextBase.cpp
+++ b/Source/core/html/canvas/WebGLRenderingContextBase.cpp
@@ -2296,8 +2296,8 @@ ScriptValue WebGLRenderingContextBase::getFramebufferAttachmentParameter(ScriptS
         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
@@ -2306,13 +2306,13 @@ ScriptValue WebGLRenderingContextBase::getFramebufferAttachmentParameter(ScriptS
         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:
             {
@@ -2321,10 +2321,10 @@ ScriptValue WebGLRenderingContextBase::getFramebufferAttachmentParameter(ScriptS
                 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);
@@ -2337,7 +2337,7 @@ ScriptValue WebGLRenderingContextBase::getFramebufferAttachmentParameter(ScriptS
         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;
@@ -6070,18 +6070,15 @@ void WebGLRenderingContextBase::setFramebuffer(GLenum target, WebGLFramebuffer*
 
     if (target == GL_FRAMEBUFFER || target == GL_DRAW_FRAMEBUFFER) {
         m_framebufferBinding = buffer;
-        drawingBuffer()->setFramebufferBinding(objectOrZero(m_framebufferBinding.get()));
-
-        if (!m_framebufferBinding) {
-            // Instead of binding fb 0, bind the drawing buffer.
-            drawingBuffer()->bind();
-        } else {
-            webContext()->bindFramebuffer(target, objectOrZero(buffer));
-        }
-
         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 (!buffer) {
+        // Instead of binding fb 0, bind the drawing buffer.
+        drawingBuffer()->bind(target);
     } else {
-        webContext()->bindFramebuffer(target, objectOrZero(buffer));
+        webContext()->bindFramebuffer(target, buffer->object());
     }
 }