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 = 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)
+{
+    ASSERT(buffer);
+
+    if (m_boundTransformFeedbackBuffer == buffer)
+        return true;
+
+    for (size_t i = 0; i < m_boundIndexedTransformFeedbackBuffers.size(); ++i) {

[Ken Russell (switch to Gerrit), 2015/09/04 20:51:31]

Any idea how large this array is in practice? Concerned about the overhead that
will be introduced for each binding operation. Have you run some WebGL samples
that draw lots of geometry to make sure there's no visible performance
regression introduced? (We should add more of these to the tough_webgl_cases
benchmark.)

+        if (m_boundIndexedTransformFeedbackBuffers[i] == buffer) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+bool WebGL2RenderingContextBase::isBufferBoundToNonTransformFeedback(WebGLBuffer* buffer)
+{
+    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) {

[Ken Russell (switch to Gerrit), 2015/09/04 20:51:31]

Same concern here.

+        if (m_boundIndexedUniformBuffers[i] == buffer) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+bool WebGL2RenderingContextBase::validateBufferTargetCompatiblity(const char* functionName, GLenum target, WebGLBuffer* buffer)
+{
+    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) {
+        if (isBufferBoundToNonTransformFeedback(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 (isBufferBoundToTransformFeedback(buffer)) {
+        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