WebGL: remove validation code for vertexAttribPointer and vertexAttribIPointer

third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp

 /*
  * Copyright (C) 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 1394 matching lines @@
 int WebGLRenderingContextBase::drawingBufferWidth() const
 {
     return isContextLost() ? 0 : drawingBuffer()->size().width();
 }
 
 int WebGLRenderingContextBase::drawingBufferHeight() const
 {
     return isContextLost() ? 0 : drawingBuffer()->size().height();
 }
 
-unsigned WebGLRenderingContextBase::sizeInBytes(GLenum type) const
-{
-    switch (type) {
-    case GL_BYTE:
-        return sizeof(GLbyte);
-    case GL_UNSIGNED_BYTE:
-        return sizeof(GLubyte);
-    case GL_SHORT:
-        return sizeof(GLshort);
-    case GL_UNSIGNED_SHORT:
-        return sizeof(GLushort);
-    case GL_INT:
-        return sizeof(GLint);
-    case GL_UNSIGNED_INT:
-        return sizeof(GLuint);
-    case GL_FLOAT:
-        return sizeof(GLfloat);
-    case GL_HALF_FLOAT:
-        return sizeof(GLushort);
-    case GL_INT_2_10_10_10_REV:
-        return sizeof(GLint);
-    case GL_UNSIGNED_INT_2_10_10_10_REV:
-        return sizeof(GLuint);
-    }
-    ASSERT_NOT_REACHED();
-    return 0;
-}
-
 void WebGLRenderingContextBase::activeTexture(GLenum texture)
 {
     if (isContextLost())
         return;
     if (texture - GL_TEXTURE0 >= m_textureUnits.size()) {
         synthesizeGLError(GL_INVALID_ENUM, "activeTexture", "texture unit out of range");
         return;
     }
     m_activeTextureUnit = texture - GL_TEXTURE0;
     webContext()->activeTexture(texture);
@@ skipping 818 matching lines @@
 
 void WebGLRenderingContextBase::disableVertexAttribArray(GLuint index)
 {
     if (isContextLost())
         return;
     if (index >= m_maxVertexAttribs) {
         synthesizeGLError(GL_INVALID_VALUE, "disableVertexAttribArray", "index out of range");
         return;
     }
 
-    WebGLVertexArrayObjectBase::VertexAttribState* state = m_boundVertexArrayObject->getVertexAttribState(index);
-    state->enabled = false;
-
     webContext()->disableVertexAttribArray(index);
 }
 
 bool WebGLRenderingContextBase::validateRenderingState(const char* functionName)
 {
     if (!m_currentProgram) {
         synthesizeGLError(GL_INVALID_OPERATION, functionName, "no valid shader program in use");
         return false;
     }
 
@@ skipping 89 matching lines @@
 
 void WebGLRenderingContextBase::enableVertexAttribArray(GLuint index)
 {
     if (isContextLost())
         return;
     if (index >= m_maxVertexAttribs) {
         synthesizeGLError(GL_INVALID_VALUE, "enableVertexAttribArray", "index out of range");
         return;
     }
 
-    WebGLVertexArrayObjectBase::VertexAttribState* state = m_boundVertexArrayObject->getVertexAttribState(index);
-    state->enabled = true;
-
     webContext()->enableVertexAttribArray(index);
 }
 
 void WebGLRenderingContextBase::finish()
 {
     if (isContextLost())
         return;
     webContext()->flush(); // Intentionally a flush, not a finish.
 }
 
@@ skipping 1085 matching lines @@
 }
 
 ScriptValue WebGLRenderingContextBase::getVertexAttrib(ScriptState* scriptState, GLuint index, GLenum pname)
 {
     if (isContextLost())
         return ScriptValue::createNull(scriptState);
     if (index >= m_maxVertexAttribs) {
         synthesizeGLError(GL_INVALID_VALUE, "getVertexAttrib", "index out of range");
         return ScriptValue::createNull(scriptState);
     }
-    const WebGLVertexArrayObjectBase::VertexAttribState* state = m_boundVertexArrayObject->getVertexAttribState(index);
 
     if ((extensionEnabled(ANGLEInstancedArraysName) || isWebGL2OrHigher())
         && pname == GL_VERTEX_ATTRIB_ARRAY_DIVISOR_ANGLE)
-        return WebGLAny(scriptState, state->divisor);
+    {
+        GLint value = 0;
+        webContext()->getVertexAttribiv(index, pname, &value);
+        return WebGLAny(scriptState, value);
+    }
 
     switch (pname) {
     case GL_VERTEX_ATTRIB_ARRAY_BUFFER_BINDING:
-        if (!state->bufferBinding || !state->bufferBinding->object())
-            return ScriptValue::createNull(scriptState);
-        return WebGLAny(scriptState, state->bufferBinding.get());
+        {
+            WeakMember<WebGLBuffer> buffer = m_boundVertexArrayObject->getArrayBufferForAttrib(index);
+            if (!buffer || !buffer->object())
+                return ScriptValue::createNull(scriptState);
+            return WebGLAny(scriptState, buffer.get());

[yunchao, 2016/01/06 17:31:42]

The simplest way is to use "WebGLBuffer* buffer" to get ArrayBuffer for attrib.
However, this line would treat "buffer" as "boolean" when overloading, but it
treated m_boundArrayBuffer.get() as "WebGLBuffer *" when
overloading,(interesting). It is not correct to return a bool to JS. 

I tried many methods to fix this bug today, like 1) PassRefPtr<WebGLBuffer> (It
doesn't work), 
2)adding a function variable in WebGLRenderingContextBase.h, say
m_arrayBufferForAttrib, and trace it by the visitor(seems to be ugly). 

Now I found "WeakMember<WebGLBuffer>" . Seems that it is OK. 

[Zhenyao Mo, 2016/01/06 18:04:08]

Can't you just return WebGLAny(scriptState,
m_boundVertexArrayObject->getArrayBufferForAttrb(index)?

in getParameter(), we do the same for GL_ELEMENT_ARRAY_BUFFER_BINDING.  I really
can't imagine why it works there but not here.

[Ken Russell (switch to Gerrit), 2016/01/06 20:53:54]

It should definitely be possible to do this. There's no reason the code should
need to be different. The ScriptValue::createNull() code should be able to be
deleted now. Using a WeakMember here is incorrect.

[yunchao, 2016/01/07 00:24:08]

Ah... I have made a detour, and it turned out to be a wrong way.

+        }
     case GL_VERTEX_ATTRIB_ARRAY_ENABLED:
-        return WebGLAny(scriptState, state->enabled);
     case GL_VERTEX_ATTRIB_ARRAY_NORMALIZED:
-        return WebGLAny(scriptState, state->normalized);
+        {
+            GLint value = 0;
+            webContext()->getVertexAttribiv(index, pname, &value);
+            return WebGLAny(scriptState, static_cast<bool>(value));
+        }
     case GL_VERTEX_ATTRIB_ARRAY_SIZE:
-        return WebGLAny(scriptState, state->size);
     case GL_VERTEX_ATTRIB_ARRAY_STRIDE:
-        return WebGLAny(scriptState, state->originalStride);
+        {
+            GLint value = 0;
+            webContext()->getVertexAttribiv(index, pname, &value);
+            return WebGLAny(scriptState, value);
+        }
     case GL_VERTEX_ATTRIB_ARRAY_TYPE:
-        return WebGLAny(scriptState, state->type);
+        {
+            GLint value = 0;
+            webContext()->getVertexAttribiv(index, pname, &value);
+            return WebGLAny(scriptState, static_cast<GLenum>(value));
+        }
     case GL_CURRENT_VERTEX_ATTRIB:
         {
             VertexAttribValue& attribValue = m_vertexAttribValue[index];
             switch (attribValue.type) {
             case Float32ArrayType:
                 return WebGLAny(scriptState, DOMFloat32Array::create(attribValue.value.floatValue, 4));
             case Int32ArrayType:
                 return WebGLAny(scriptState, DOMInt32Array::create(attribValue.value.intValue, 4));
             case Uint32ArrayType:
                 return WebGLAny(scriptState, DOMUint32Array::create(attribValue.value.uintValue, 4));
@@ skipping 1605 matching lines @@
 void WebGLRenderingContextBase::vertexAttrib4fv(GLuint index, const DOMFloat32Array* v)
 {
     vertexAttribfvImpl("vertexAttrib4fv", index, v, 4);
 }
 
 void WebGLRenderingContextBase::vertexAttrib4fv(GLuint index, const Vector<GLfloat>& v)
 {
     vertexAttribfvImpl("vertexAttrib4fv", index, v.data(), v.size(), 4);
 }
 
-bool WebGLRenderingContextBase::validateVertexAttribPointerTypeAndSize(GLenum type, GLint size)
-{
-    switch (type) {
-    case GL_BYTE:
-    case GL_UNSIGNED_BYTE:
-    case GL_SHORT:
-    case GL_UNSIGNED_SHORT:
-    case GL_FLOAT:
-        if (size < 1 || size > 4) {
-            synthesizeGLError(GL_INVALID_VALUE, "vertexAttribPointer", "bad size");
-            return false;
-        }
-        return true;
-    default:
-        synthesizeGLError(GL_INVALID_ENUM, "vertexAttribPointer", "invalid type");
-        return false;
-    }
-}
-
 void WebGLRenderingContextBase::vertexAttribPointer(ScriptState* scriptState, GLuint index, GLint size, GLenum type, GLboolean normalized, GLsizei stride, long long offset)
 {
-    if (isContextLost() || !validateVertexAttribPointerTypeAndSize(type, size))
+    if (isContextLost())
         return;
     if (index >= m_maxVertexAttribs) {
         synthesizeGLError(GL_INVALID_VALUE, "vertexAttribPointer", "index out of range");
         return;
     }
-    if (stride < 0 || stride > 255) {
-        synthesizeGLError(GL_INVALID_VALUE, "vertexAttribPointer", "bad stride");
-        return;
-    }
     if (!validateValueFitNonNegInt32("vertexAttribPointer", "offset", offset))
         return;
     if (!m_boundArrayBuffer) {
         synthesizeGLError(GL_INVALID_OPERATION, "vertexAttribPointer", "no bound ARRAY_BUFFER");
         return;
     }
-    unsigned typeSize = sizeInBytes(type);
-    ASSERT((typeSize & (typeSize - 1)) == 0); // Ensure that the value is POT.
-    if ((stride & (typeSize - 1)) || (static_cast<GLintptr>(offset) & (typeSize - 1))) {
-        synthesizeGLError(GL_INVALID_OPERATION, "vertexAttribPointer", "stride or offset not valid for type");
-        return;
-    }
-    GLsizei bytesPerElement = size * typeSize;
 
-    m_boundVertexArrayObject->setVertexAttribState(index, bytesPerElement, size, type, normalized, stride, static_cast<GLintptr>(offset), m_boundArrayBuffer);
+    m_boundVertexArrayObject->setArrayBufferForAttrib(index, m_boundArrayBuffer.get());
     webContext()->vertexAttribPointer(index, size, type, normalized, stride, static_cast<GLintptr>(offset));
     maybePreserveDefaultVAOObjectWrapper(scriptState);
     preserveObjectWrapper(scriptState, m_boundVertexArrayObject, "arraybuffer", index, m_boundArrayBuffer);
 }
 
 void WebGLRenderingContextBase::vertexAttribDivisorANGLE(GLuint index, GLuint divisor)
 {
     if (isContextLost())
         return;
 
     if (index >= m_maxVertexAttribs) {
         synthesizeGLError(GL_INVALID_VALUE, "vertexAttribDivisorANGLE", "index out of range");
         return;
     }
 
-    m_boundVertexArrayObject->setVertexAttribDivisor(index, divisor);
     webContext()->vertexAttribDivisorANGLE(index, divisor);
 }
 
 void WebGLRenderingContextBase::viewport(GLint x, GLint y, GLsizei width, GLsizei height)
 {
     if (isContextLost())
         return;
     if (!validateSize("viewport", width, height))
         return;
     webContext()->viewport(x, y, width, height);
@@ skipping 1852 matching lines @@
         webContext()->pixelStorei(GL_UNPACK_ALIGNMENT, 1);
 }
 
 void WebGLRenderingContextBase::restoreUnpackParameters()
 {
     if (m_unpackAlignment != 1)
         webContext()->pixelStorei(GL_UNPACK_ALIGNMENT, m_unpackAlignment);
 }
 
 } // namespace blink