Better state tracking and validation for bindBufferBase and bindBufferRange

Source/modules/webgl/WebGL2RenderingContextBase.cpp

Index: Source/modules/webgl/WebGL2RenderingContextBase.cpp
diff --git a/Source/modules/webgl/WebGL2RenderingContextBase.cpp b/Source/modules/webgl/WebGL2RenderingContextBase.cpp
index a839b1ce73f670ac09edba0b3d0937e1988734ec..3f05315cff3c99770284c4f59d838c60fc562541 100644
--- a/Source/modules/webgl/WebGL2RenderingContextBase.cpp
+++ b/Source/modules/webgl/WebGL2RenderingContextBase.cpp
@@ -84,6 +84,16 @@ void WebGL2RenderingContextBase::initializeNewContext()
     m_samplerUnits.clear();
     m_samplerUnits.resize(numCombinedTextureImageUnits);
 
+    GLint maxTransformFeedbackSeparateAttribs;

[Zhenyao Mo, 2015/09/04 18:10:44]

initialize it 0

+    webContext()->getIntegerv(GL_MAX_TRANSFORM_FEEDBACK_SEPARATE_ATTRIBS, &maxTransformFeedbackSeparateAttribs);
+    m_boundIndexedTransformFeedbackBuffers.clear();
+    m_boundIndexedTransformFeedbackBuffers.resize(maxTransformFeedbackSeparateAttribs);
+
+    GLint maxUniformBufferBindings = 0;
+    webContext()->getIntegerv(GL_MAX_UNIFORM_BUFFER_BINDINGS, &maxUniformBufferBindings);
+    m_boundIndexedUniformBuffers.clear();
+    m_boundIndexedUniformBuffers.resize(maxUniformBufferBindings);
+
     WebGLRenderingContextBase::initializeNewContext();
 }
 
@@ -1650,19 +1660,28 @@ void WebGL2RenderingContextBase::bindBufferBase(GLenum target, GLuint index, Web
         return;
     if (deleted)
         buffer = 0;
+    if (!validateAndUpdateBufferBindBaseTarget("bindBufferBase", target, index, buffer))
+        return;
     webContext()->bindBufferBase(target, index, objectOrZero(buffer));
 }
 
 void WebGL2RenderingContextBase::bindBufferRange(GLenum target, GLuint index, WebGLBuffer* buffer, long long offset, long long size)
 {
-    if (isContextLost() || !validateWebGLObject("bindBufferRange", buffer))
+    if (isContextLost())
         return;
-
+    bool deleted;
+    if (!checkObjectToBeBound("bindBufferRange", buffer, deleted))
+        return;
+    if (deleted)
+        buffer = 0;
     if (!validateValueFitNonNegInt32("bindBufferRange", "offset", offset)
         || !validateValueFitNonNegInt32("bindBufferRange", "size", size)) {
         return;
     }
 
+    if (!validateAndUpdateBufferBindBaseTarget("bindBufferRange", target, index, buffer))
+        return;
+
     webContext()->bindBufferRange(target, index, objectOrZero(buffer), static_cast<GLintptr>(offset), static_cast<GLsizeiptr>(size));
 }
 
@@ -1673,9 +1692,17 @@ ScriptValue WebGL2RenderingContextBase::getIndexedParameter(ScriptState* scriptS
 
     switch (target) {
     case GL_TRANSFORM_FEEDBACK_BUFFER_BINDING:
+        if (index >= m_boundIndexedTransformFeedbackBuffers.size()) {
+            synthesizeGLError(GL_INVALID_VALUE, "getIndexedParameter", "index out of range");
+            return ScriptValue::createNull(scriptState);
+        }
+        return WebGLAny(scriptState, m_boundIndexedTransformFeedbackBuffers[index].get());
     case GL_UNIFORM_BUFFER_BINDING:
-        notImplemented();
-        return ScriptValue::createNull(scriptState);
+        if (index >= m_boundIndexedUniformBuffers.size()) {
+            synthesizeGLError(GL_INVALID_VALUE, "getIndexedParameter", "index out of range");
+            return ScriptValue::createNull(scriptState);
+        }
+        return WebGLAny(scriptState, m_boundIndexedUniformBuffers[index].get());
     case GL_TRANSFORM_FEEDBACK_BUFFER_SIZE:
     case GL_TRANSFORM_FEEDBACK_BUFFER_START:
     case GL_UNIFORM_BUFFER_SIZE:
@@ -2055,6 +2082,90 @@ bool WebGL2RenderingContextBase::validateCapability(const char* functionName, GL
     }
 }
 
+bool WebGL2RenderingContextBase::isBufferBoundToTransformFeedback(WebGLBuffer* buffer)
+{

[Zhenyao Mo, 2015/09/04 18:10:44]

The comparison only makes sense if buffer != nullptr, so please add an ASSERT
here.

+    if (m_boundTransformFeedbackBuffer == buffer)
+        return true;
+
+    for (size_t i = 0; i < m_boundIndexedTransformFeedbackBuffers.size(); ++i) {
+        if (m_boundIndexedTransformFeedbackBuffers[i] == buffer) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+bool WebGL2RenderingContextBase::isBufferBoundToNonTransformFeedback(WebGLBuffer* buffer)
+{

[Zhenyao Mo, 2015/09/04 18:10:44]

ASSERT(buffer)

+    if (m_boundArrayBuffer == buffer
+        || m_boundVertexArrayObject->boundElementArrayBuffer() == buffer
+        || m_boundCopyReadBuffer == buffer
+        || m_boundCopyWriteBuffer == buffer
+        || m_boundPixelPackBuffer == buffer
+        || m_boundPixelUnpackBuffer == buffer
+        || m_boundUniformBuffer == buffer) {
+        return true;
+    }
+
+    for (size_t i = 0; i < m_boundIndexedUniformBuffers.size(); ++i) {
+        if (m_boundIndexedUniformBuffers[i] == buffer) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+bool WebGL2RenderingContextBase::validateBufferTargetCompatiblity(const char* functionName, GLenum target, WebGLBuffer* buffer)
+{

[Zhenyao Mo, 2015/09/04 18:10:44]

ASSERT(buffer)

+    switch (buffer->getInitialTarget()) {
+    case GL_ELEMENT_ARRAY_BUFFER:
+        switch (target) {
+        case GL_ARRAY_BUFFER:
+        case GL_PIXEL_PACK_BUFFER:
+        case GL_PIXEL_UNPACK_BUFFER:
+        case GL_TRANSFORM_FEEDBACK_BUFFER:
+        case GL_UNIFORM_BUFFER:
+            synthesizeGLError(GL_INVALID_OPERATION, functionName,
+                "element array buffers can not be bound to a different target");
+
+            return false;
+        default:
+            break;
+        }
+        break;
+    case GL_ARRAY_BUFFER:
+    case GL_COPY_READ_BUFFER:
+    case GL_COPY_WRITE_BUFFER:
+    case GL_PIXEL_PACK_BUFFER:
+    case GL_PIXEL_UNPACK_BUFFER:
+    case GL_UNIFORM_BUFFER:
+    case GL_TRANSFORM_FEEDBACK_BUFFER:
+        if (target == GL_ELEMENT_ARRAY_BUFFER) {
+            synthesizeGLError(GL_INVALID_OPERATION, functionName,
+                "buffers bound to non ELEMENT_ARRAY_BUFFER targets can not be bound to ELEMENT_ARRAY_BUFFER target");
+            return false;
+        }
+        break;
+    default:
+        break;
+    }
+
+    if (target == GL_TRANSFORM_FEEDBACK_BUFFER && isBufferBoundToNonTransformFeedback(buffer)) {
+        synthesizeGLError(GL_INVALID_OPERATION, functionName,
+            "a buffer bound to TRANSFORM_FEEDBACK_BUFFER can not be bound to any other targets");
+        return false;
+    }
+    if (isBufferBoundToTransformFeedback(buffer)) {

[Zhenyao Mo, 2015/09/04 18:10:44]

you can combine this with the previous if clause with ||

+        synthesizeGLError(GL_INVALID_OPERATION, functionName,
+            "a buffer bound to TRANSFORM_FEEDBACK_BUFFER can not be bound to any other targets");
+        return false;
+    }
+
+    return true;
+}
+
 bool WebGL2RenderingContextBase::validateBufferTarget(const char* functionName, GLenum target)
 {
     switch (target) {
@@ -2078,57 +2189,8 @@ bool WebGL2RenderingContextBase::validateAndUpdateBufferBindTarget(const char* f
     if (!validateBufferTarget(functionName, target))
         return false;
 
-    if (buffer) {
-        switch (buffer->getInitialTarget()) {
-        case GL_ELEMENT_ARRAY_BUFFER:
-            switch (target) {
-            case GL_ARRAY_BUFFER:
-            case GL_PIXEL_PACK_BUFFER:
-            case GL_PIXEL_UNPACK_BUFFER:
-            case GL_TRANSFORM_FEEDBACK_BUFFER:
-            case GL_UNIFORM_BUFFER:
-                synthesizeGLError(GL_INVALID_OPERATION, functionName,
-                    "element array buffers can not be bound to a different target");
-
-                return false;
-            default:
-                break;
-            }
-            break;
-        case GL_ARRAY_BUFFER:
-        case GL_COPY_READ_BUFFER:
-        case GL_COPY_WRITE_BUFFER:
-        case GL_PIXEL_PACK_BUFFER:
-        case GL_PIXEL_UNPACK_BUFFER:
-        case GL_UNIFORM_BUFFER:
-        case GL_TRANSFORM_FEEDBACK_BUFFER:
-            if (target == GL_ELEMENT_ARRAY_BUFFER) {
-                synthesizeGLError(GL_INVALID_OPERATION, functionName,
-                    "buffers bound to non ELEMENT_ARRAY_BUFFER targets can not be bound to ELEMENT_ARRAY_BUFFER target");
-                return false;
-            }
-            break;
-        default:
-            break;
-        }
-        if (target == GL_TRANSFORM_FEEDBACK_BUFFER) {
-            if (m_boundArrayBuffer == buffer
-                || m_boundVertexArrayObject->boundElementArrayBuffer() == buffer
-                || m_boundCopyReadBuffer == buffer
-                || m_boundCopyWriteBuffer == buffer
-                || m_boundPixelPackBuffer == buffer
-                || m_boundPixelUnpackBuffer == buffer
-                || m_boundUniformBuffer == buffer) {
-                synthesizeGLError(GL_INVALID_OPERATION, functionName,
-                    "a buffer bound to TRANSFORM_FEEDBACK_BUFFER can not be bound to any other targets");
-                return false;
-            }
-        } else if (m_boundTransformFeedbackBuffer == buffer) {
-            synthesizeGLError(GL_INVALID_OPERATION, functionName,
-                "a buffer bound to TRANSFORM_FEEDBACK_BUFFER can not be bound to any other targets");
-            return false;
-        }
-    }
+    if (buffer && !validateBufferTargetCompatiblity(functionName, target, buffer))
+        return false;
 
     switch (target) {
     case GL_ARRAY_BUFFER:
@@ -2165,6 +2227,53 @@ bool WebGL2RenderingContextBase::validateAndUpdateBufferBindTarget(const char* f
     return true;
 }
 
+bool WebGL2RenderingContextBase::validateBufferBaseTarget(const char* functionName, GLenum target)
+{
+    switch (target) {
+    case GL_TRANSFORM_FEEDBACK_BUFFER:
+    case GL_UNIFORM_BUFFER:
+        return true;
+    default:
+        synthesizeGLError(GL_INVALID_ENUM, functionName, "invalid target");
+        return false;
+    }
+}
+
+bool WebGL2RenderingContextBase::validateAndUpdateBufferBindBaseTarget(const char* functionName, GLenum target, GLuint index, WebGLBuffer* buffer)
+{
+    if (!validateBufferBaseTarget(functionName, target))
+        return false;
+
+    if (buffer && !validateBufferTargetCompatiblity(functionName, target, buffer))
+        return false;
+
+    switch (target) {
+    case GL_TRANSFORM_FEEDBACK_BUFFER:
+        if (index >= m_boundIndexedTransformFeedbackBuffers.size()) {
+            synthesizeGLError(GL_INVALID_OPERATION, functionName, "index out of range");
+            return false;
+        }
+        m_boundIndexedTransformFeedbackBuffers[index] = buffer;
+        m_boundTransformFeedbackBuffer = buffer;
+        break;
+    case GL_UNIFORM_BUFFER:
+        if (index >= m_boundIndexedUniformBuffers.size()) {
+            synthesizeGLError(GL_INVALID_OPERATION, functionName, "index out of range");
+            return false;
+        }
+        m_boundIndexedUniformBuffers[index] = buffer;
+        m_boundUniformBuffer = buffer;
+        break;
+    default:
+        ASSERT_NOT_REACHED();
+        break;
+    }
+
+    if (buffer && !buffer->getInitialTarget())
+        buffer->setInitialTarget(target);
+    return true;
+}
+
 bool WebGL2RenderingContextBase::validateFramebufferTarget(GLenum target)
 {
     switch (target) {