Better state tracking and validation for bindBufferBase and bindBufferRange

Source/modules/webgl/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 "modules/webgl/WebGL2RenderingContextBase.h"
 
 #include "bindings/modules/v8/WebGLAny.h"
 #include "core/html/HTMLCanvasElement.h"
 #include "core/html/HTMLImageElement.h"
@@ skipping 66 matching lines @@
 
     m_max3DTextureSize = 0;
     webContext()->getIntegerv(GL_MAX_3D_TEXTURE_SIZE, &m_max3DTextureSize);
     m_max3DTextureLevel = WebGLTexture::computeLevelCount(m_max3DTextureSize, m_max3DTextureSize, m_max3DTextureSize);
 
     GLint numCombinedTextureImageUnits = 0;
     webContext()->getIntegerv(GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, &numCombinedTextureImageUnits);
     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();
 }
 
 void WebGL2RenderingContextBase::copyBufferSubData(GLenum readTarget, GLenum writeTarget, long long readOffset, long long writeOffset, long long size)
 {
     if (isContextLost())
         return;
 
     if (!validateValueFitNonNegInt32("copyBufferSubData", "readOffset", readOffset)
         || !validateValueFitNonNegInt32("copyBufferSubData", "writeOffset", writeOffset)
@@ skipping 1546 matching lines @@
 
 void WebGL2RenderingContextBase::bindBufferBase(GLenum target, GLuint index, WebGLBuffer* buffer)
 {
     if (isContextLost())
         return;
     bool deleted;
     if (!checkObjectToBeBound("bindBufferBase", buffer, deleted))
         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));
 }
 
 ScriptValue WebGL2RenderingContextBase::getIndexedParameter(ScriptState* scriptState, GLenum target, GLuint index)
 {
     if (isContextLost())
         return ScriptValue::createNull(scriptState);
 
     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:
     case GL_UNIFORM_BUFFER_START:
         {
             GLint64 value = -1;
             webContext()->getInteger64i_v(target, index, &value);
             return WebGLAny(scriptState, value);
         }
     default:
@@ skipping 359 matching lines @@
 bool WebGL2RenderingContextBase::validateCapability(const char* functionName, GLenum cap)
 {
     switch (cap) {
     case GL_RASTERIZER_DISCARD:
         return true;
     default:
         return WebGLRenderingContextBase::validateCapability(functionName, cap);
     }
 }
 
+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) {
     case GL_ARRAY_BUFFER:
     case GL_COPY_READ_BUFFER:
     case GL_COPY_WRITE_BUFFER:
     case GL_ELEMENT_ARRAY_BUFFER:
     case GL_PIXEL_PACK_BUFFER:
     case GL_PIXEL_UNPACK_BUFFER:
     case GL_TRANSFORM_FEEDBACK_BUFFER:
     case GL_UNIFORM_BUFFER:
         return true;
     default:
         synthesizeGLError(GL_INVALID_ENUM, functionName, "invalid target");
         return false;
     }
 }
 
 bool WebGL2RenderingContextBase::validateAndUpdateBufferBindTarget(const char* functionName, GLenum target, WebGLBuffer* buffer)
 {
     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:
         m_boundArrayBuffer = buffer;
         break;
     case GL_COPY_READ_BUFFER:
         m_boundCopyReadBuffer = buffer;
         break;
     case GL_COPY_WRITE_BUFFER:
         m_boundCopyWriteBuffer = buffer;
@@ skipping 16 matching lines @@
     default:
         ASSERT_NOT_REACHED();
         break;
     }
 
     if (buffer && !buffer->getInitialTarget())
         buffer->setInitialTarget(target);
     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) {
     case GL_FRAMEBUFFER:
     case GL_READ_FRAMEBUFFER:
     case GL_DRAW_FRAMEBUFFER:
         return true;
     default:
         return false;
     }
@@ skipping 367 matching lines @@
 GLenum WebGL2RenderingContextBase::boundFramebufferColorFormat()
 {
     if (m_readFramebufferBinding && m_readFramebufferBinding->object())
         return m_readFramebufferBinding->colorBufferFormat();
     if (m_requestedAttributes.alpha())
         return GL_RGBA;
     return GL_RGB;
 }
 
 } // namespace blink