update getFramebufferAttachmentParameter for WebGL 2

Source/core/html/canvas/WebGL2RenderingContextBase.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "core/html/canvas/WebGL2RenderingContextBase.h"
 
 #include "bindings/core/v8/WebGLAny.h"
 #include "core/html/HTMLCanvasElement.h"
 #include "core/html/HTMLImageElement.h"
@@ skipping 1178 matching lines @@
         // FIXME: Some of these ES3 buffer targets may require additional state tracking.
         break;
     default:
         synthesizeGLError(GL_INVALID_ENUM, functionName, "invalid target");
         return false;
     }
 
     return true;
 }
 
+bool WebGL2RenderingContextBase::validateGetFramebufferAttachmentParameters(const char* functionName, GLenum target, GLenum attachment)

[Zhenyao Mo, 2015/03/06 18:35:37]

I think you should fold this function into getFramebufferAttachmentParameter()
because you are not just checking for enums, but also other stuff, for example,
of the depth_stencil attached objects are valid.

[yunchao, 2015/03/09 07:16:57]

Agree. Considering that this validation is standalone, what about rename the
function to validateGetFramebufferAttachmentParameterFunc? If you still think
that it is better to fold this code snippet into
getFramebufferAttachmentParameter, I will follow that.

+{
+    if (target != GL_FRAMEBUFFER && target != GL_READ_FRAMEBUFFER) {

[Zhenyao Mo, 2015/03/06 18:35:37]

Add a comment GL_DRAW_FRAMEBUFFER == GL_FRAMEBUFFER>

[yunchao, 2015/03/09 07:16:57]

A little tricky here. GL_DRAW_FRAMEBUFFER is not equal to GL_FRAMEBUFFER in GLES
3.0 spec and WebGL Spec. This differs from openGL spec. I added
GL_DRAW_FRAMEBUFFER in the latest patch set. 

[yunchao, 2015/03/09 10:00:34]

GLenum GL_DRAW_FRAMEBUFFER(0x8CA9) is not equal to GL_FRAMEBUFFER(0x8D40). In
addtion, bindFramebuffer with GL_FRAMEBUFFER will bind to both read and draw
buffers, although for framebufferRenderbuffer, framebufferTexture2D,
checkFramebufferStatus as well as framebufferAttachmentParameter, target set to
GL_FRAMEBUFFER has the same effect as target set to GL_DRAW_FRAMEBUFFER. 

This feature is the same in OpenGL and GLES. My statement above that the feature
in GLES differs from that in OpenGL is wrong.

+        synthesizeGLError(GL_INVALID_ENUM, functionName, "invalid target");
+        return false;
+    }
+    ASSERT(!m_framebufferBinding && !drawingBuffer());

[Zhenyao Mo, 2015/03/06 18:35:37]

This ASSERTION is incorrect.  It should be ||, not &&.

[yunchao, 2015/03/09 07:16:57]

If binding to default FB, m_framebufferBinding will be 0. Query the default FB
is allowed in WebGL 2.0 (but it is not allowed in WebGL 1.0). In this case,
Chromium doesn't really bind to window-system's default FB, but the
drawingBuffer for WebGL. So, if binding to default FB, drawingBuffer should be
existed. (seems that drawingBuffer should be always existed).

+    if (!m_framebufferBinding) {

[Zhenyao Mo, 2015/03/06 18:35:37]

You also need to check  || !m_framebufferBinding->object().

Maybe we don't need to do it, but it's better to be consistent with the existing
code.

[yunchao, 2015/03/09 07:16:57]

Personally, I think it is not necessary to check m_framebufferBinding->object(),
which refer to an FBO.

+    // for the default framebuffer
+        switch (attachment) {
+        case GL_BACK:
+        case GL_DEPTH:
+        case GL_STENCIL:
+            break;
+        default:
+            synthesizeGLError(GL_INVALID_OPERATION, functionName, "invalid attachment");
+            return false;
+        }
+    } else {
+    // for the FBO
+        switch (attachment) {
+        case GL_COLOR_ATTACHMENT0:
+        case GL_DEPTH_ATTACHMENT:
+        case GL_STENCIL_ATTACHMENT:
+            break;
+        case GL_DEPTH_STENCIL_ATTACHMENT:
+            if (m_framebufferBinding->getAttachmentObject(GL_DEPTH_STENCIL_ATTACHMENT) || (m_framebufferBinding->getAttachmentObject(GL_DEPTH_ATTACHMENT) == m_framebufferBinding->getAttachmentObject(GL_STENCIL_ATTACHMENT)))
+                break;
+        default:
+            if (attachment > GL_COLOR_ATTACHMENT0
+                && attachment < static_cast<GLenum>(GL_COLOR_ATTACHMENT0 + maxColorAttachments()))
+                break;
+            synthesizeGLError(GL_INVALID_OPERATION, functionName, "invalid attachment");
+            return false;
+        }
+    }
+    return true;
+}
+
+ScriptValue WebGL2RenderingContextBase::getFramebufferAttachmentParameter(ScriptState* scriptState, GLenum target, GLenum attachment, GLenum pname)
+{
+    if (isContextLost() || !validateGetFramebufferAttachmentParameters("getFramebufferAttachmentParameter", target, attachment))
+        return ScriptValue::createNull(scriptState);
+
+    if (m_framebufferBinding && !m_framebufferBinding->object()) {

[Zhenyao Mo, 2015/03/06 18:35:37]

This is incorrect, in ES3 we allow querying the back buffer.  In the above
function you actually handled that.

[yunchao, 2015/03/09 07:16:57]

Agree, but this is different from the original code in WebGLRenderingContext,
whose condition for if-clause is !m_framebufferBinding ||
!m_framebufferBinding->object(). 

I add an assert here in the latest patch set: ASSERT(m_framebufferBinding ||
!m_framebufferBinding->object()); 

+        synthesizeGLError(GL_INVALID_OPERATION, "getFramebufferAttachmentParameter", "no framebuffer bound");
+        return ScriptValue::createNull(scriptState);
+    }
+
+    WebGLSharedObject* object = m_framebufferBinding ? m_framebufferBinding->getAttachmentObject(attachment) : 0;
+    if (m_framebufferBinding && !object) {
+        if (pname == GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE)
+            return WebGLAny(scriptState, GL_NONE);
+        if (pname == GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)
+            return WebGLAny(scriptState, 0);
+        synthesizeGLError(GL_INVALID_OPERATION, "getFramebufferAttachmentParameter", "invalid parameter name");
+        return ScriptValue::createNull(scriptState);
+    }
+
+    switch (pname) {
+    case GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE:
+        if (!object)
+            return WebGLAny(scriptState, GL_FRAMEBUFFER_DEFAULT);
+        if (object->isTexture())
+            return WebGLAny(scriptState, GL_TEXTURE);
+        if (object->isRenderbuffer())
+            return WebGLAny(scriptState, GL_RENDERBUFFER);
+        break;
+    case GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME:
+        if (!object)
+            break;
+        return WebGLAny(scriptState, PassRefPtrWillBeRawPtr<WebGLObject>(object));
+    case GL_FRAMEBUFFER_ATTACHMENT_TEXTURE_CUBE_MAP_FACE:
+    case GL_FRAMEBUFFER_ATTACHMENT_TEXTURE_LAYER:
+    case GL_FRAMEBUFFER_ATTACHMENT_TEXTURE_LEVEL:
+        if (!object || !object->isTexture())
+            break;
+    case GL_FRAMEBUFFER_ATTACHMENT_RED_SIZE:
+    case GL_FRAMEBUFFER_ATTACHMENT_GREEN_SIZE:
+    case GL_FRAMEBUFFER_ATTACHMENT_BLUE_SIZE:
+    case GL_FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE:
+    case GL_FRAMEBUFFER_ATTACHMENT_DEPTH_SIZE:
+    case GL_FRAMEBUFFER_ATTACHMENT_STENCIL_SIZE:
+        {
+            GLint value = 0;
+            webContext()->getFramebufferAttachmentParameteriv(target, attachment, pname, &value);
+            return WebGLAny(scriptState, value);
+        }
+    case GL_FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE:
+    case GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING:
+        {
+            GLint value = 0;
+            webContext()->getFramebufferAttachmentParameteriv(target, attachment, pname, &value);
+            return WebGLAny(scriptState, static_cast<unsigned>(value));
+        }
+    default:
+        break;
+    }
+    synthesizeGLError(GL_INVALID_ENUM, "getFramebufferAttachmentParameter", "invalid parameter name");
+    return ScriptValue::createNull(scriptState);
+}
+
 DEFINE_TRACE(WebGL2RenderingContextBase)
 {
     WebGLRenderingContextBase::trace(visitor);
 }
 
 } // namespace blink