Backed out changeset 0528322db042 (bug 1330022) for failing test_conformance__more__functions__vertexAttribPointerBadArgs.htm. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Fri, 10 Feb 2017 23:32:32 +0100
changeset 376452 8675b60fad97dd1c1c8e999b3dd5f7c7aa9582dd
parent 376451 f2fa758c8d307b7ecf204439b4a0bf08b53927aa
child 376486 447c280cbd29c2184ee5818c9a678f5b21a5e1bf
push id36
push userfmarier@mozilla.com
push dateSat, 18 Feb 2017 19:38:57 +0000
reviewersbackout
bugs1330022
milestone54.0a1
backs out0528322db042d01a79c70427546f7ba6ceac7a8e
Backed out changeset 0528322db042 (bug 1330022) for failing test_conformance__more__functions__vertexAttribPointerBadArgs.htm. r=backout
dom/canvas/WebGL1Context.h
dom/canvas/WebGL1ContextUniforms.cpp
dom/canvas/WebGL2Context.h
dom/canvas/WebGL2ContextVertices.cpp
dom/canvas/WebGLContext.h
dom/canvas/WebGLContextBuffers.cpp
dom/canvas/WebGLContextValidate.cpp
dom/canvas/WebGLContextVertices.cpp
dom/canvas/moz.build
--- a/dom/canvas/WebGL1Context.h
+++ b/dom/canvas/WebGL1Context.h
@@ -28,14 +28,17 @@ public:
     virtual bool IsWebGL2() const override {
         return false;
     }
 
     // nsWrapperCache
     virtual JSObject* WrapObject(JSContext* cx, JS::Handle<JSObject*> givenProto) override;
 
 private:
+    virtual bool ValidateAttribPointerType(bool integerMode, GLenum type,
+                                           uint32_t* alignment,
+                                           const char* info) override;
     virtual bool ValidateUniformMatrixTranspose(bool transpose, const char* info) override;
 };
 
 } // namespace mozilla
 
 #endif // WEBGL_1_CONTEXT_H_
--- a/dom/canvas/WebGL1ContextUniforms.cpp
+++ b/dom/canvas/WebGL1ContextUniforms.cpp
@@ -3,16 +3,44 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "WebGL1Context.h"
 
 namespace mozilla {
 
 bool
+WebGL1Context::ValidateAttribPointerType(bool /*integerMode*/, GLenum type,
+                                         uint32_t* out_alignment, const char* info)
+{
+    MOZ_ASSERT(out_alignment);
+    if (!out_alignment)
+        return false;
+
+    switch (type) {
+    case LOCAL_GL_BYTE:
+    case LOCAL_GL_UNSIGNED_BYTE:
+        *out_alignment = 1;
+        return true;
+
+    case LOCAL_GL_SHORT:
+    case LOCAL_GL_UNSIGNED_SHORT:
+        *out_alignment = 2;
+        return true;
+        // XXX case LOCAL_GL_FIXED:
+    case LOCAL_GL_FLOAT:
+        *out_alignment = 4;
+        return true;
+    }
+
+    ErrorInvalidEnumInfo(info, type);
+    return false;
+}
+
+bool
 WebGL1Context::ValidateUniformMatrixTranspose(bool transpose, const char* info)
 {
     if (transpose) {
         ErrorInvalidValue("%s: transpose must be FALSE as per the "
                           "OpenGL ES 2.0 spec", info);
         return false;
     }
 
--- a/dom/canvas/WebGL2Context.h
+++ b/dom/canvas/WebGL2Context.h
@@ -233,26 +233,17 @@ protected:
 public:
     // -------------------------------------------------------------------------
     // Programs and shaders - WebGL2ContextPrograms.cpp
     GLint GetFragDataLocation(const WebGLProgram& program, const nsAString& name);
 
 
     // -------------------------------------------------------------------------
     // Uniforms and attributes - WebGL2ContextUniforms.cpp
-
-    void VertexAttribIPointer(GLuint index, GLint size, GLenum type, GLsizei stride,
-                              WebGLintptr byteOffset)
-    {
-        const char funcName[] = "vertexAttribIPointer";
-        const bool isFuncInt = true;
-        const bool normalized = false;
-        VertexAttribAnyPointer(funcName, isFuncInt, index, size, type, normalized, stride,
-                               byteOffset);
-    }
+    void VertexAttribIPointer(GLuint index, GLint size, GLenum type, GLsizei stride, GLintptr offset);
 
     ////////////////
 
     // GL 3.0 & ES 3.0
     void VertexAttribI4i(GLuint index, GLint x, GLint y, GLint z, GLint w,
                          const char* funcName = nullptr);
     void VertexAttribI4ui(GLuint index, GLuint x, GLuint y, GLuint z, GLuint w,
                           const char* funcName = nullptr);
@@ -426,14 +417,17 @@ private:
     CreateFormatUsage(gl::GLContext* gl) const override;
 
     virtual bool IsTexParamValid(GLenum pname) const override;
 
     void UpdateBoundQuery(GLenum target, WebGLQuery* query);
 
     // CreateVertexArrayImpl is assumed to be infallible.
     virtual WebGLVertexArray* CreateVertexArrayImpl() override;
+    virtual bool ValidateAttribPointerType(bool integerMode, GLenum type,
+                                           uint32_t* alignment,
+                                           const char* info) override;
     virtual bool ValidateUniformMatrixTranspose(bool transpose, const char* info) override;
 };
 
 } // namespace mozilla
 
 #endif
new file mode 100644
--- /dev/null
+++ b/dom/canvas/WebGL2ContextVertices.cpp
@@ -0,0 +1,92 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "WebGL2Context.h"
+
+#include "GLContext.h"
+#include "WebGLVertexArray.h"
+#include "WebGLVertexAttribData.h"
+
+#include "mozilla/Casting.h"
+
+namespace mozilla {
+
+bool
+WebGL2Context::ValidateAttribPointerType(bool integerMode, GLenum type,
+                                         uint32_t* out_alignment, const char* info)
+{
+  MOZ_ASSERT(out_alignment);
+
+  switch (type) {
+  case LOCAL_GL_BYTE:
+  case LOCAL_GL_UNSIGNED_BYTE:
+    *out_alignment = 1;
+    return true;
+
+  case LOCAL_GL_SHORT:
+  case LOCAL_GL_UNSIGNED_SHORT:
+    *out_alignment = 2;
+    return true;
+
+  case LOCAL_GL_INT:
+  case LOCAL_GL_UNSIGNED_INT:
+    *out_alignment = 4;
+    return true;
+  }
+
+  if (!integerMode) {
+    switch (type) {
+    case LOCAL_GL_HALF_FLOAT:
+      *out_alignment = 2;
+      return true;
+
+    case LOCAL_GL_FLOAT:
+    case LOCAL_GL_FIXED:
+    case LOCAL_GL_INT_2_10_10_10_REV:
+    case LOCAL_GL_UNSIGNED_INT_2_10_10_10_REV:
+      *out_alignment = 4;
+      return true;
+    }
+  }
+
+  ErrorInvalidEnum("%s: invalid enum value 0x%x", info, type);
+  return false;
+}
+
+// -------------------------------------------------------------------------
+// Vertex Attributes
+
+void
+WebGL2Context::VertexAttribIPointer(GLuint index, GLint size, GLenum type, GLsizei stride,
+                                    GLintptr offset)
+{
+  if (IsContextLost())
+    return;
+
+  if (!ValidateAttribIndex(index, "vertexAttribIPointer"))
+    return;
+
+  if (!ValidateAttribPointer(true, index, size, type, LOCAL_GL_FALSE, stride, offset,
+                             "vertexAttribIPointer"))
+  {
+    return;
+  }
+
+  MOZ_ASSERT(mBoundVertexArray);
+
+  InvalidateBufferFetching();
+
+  MakeContextCurrent();
+  gl->fVertexAttribIPointer(index, size, type, stride, reinterpret_cast<void*>(offset));
+
+  WebGLVertexAttribData& vd = mBoundVertexArray->mAttribs[index];
+  const bool integerFunc = true;
+  const bool normalized = false;
+  vd.VertexAttribPointer(integerFunc, mBoundArrayBuffer, size, type, normalized, stride,
+                         offset);
+}
+
+} // namespace mozilla
--- a/dom/canvas/WebGLContext.h
+++ b/dom/canvas/WebGLContext.h
@@ -1347,32 +1347,19 @@ public:
             return;
 
         VertexAttrib4f(index, arr.elemBytes[0], arr.elemBytes[1], arr.elemBytes[2],
                        arr.elemBytes[3], funcName);
     }
 
     ////
 
-protected:
-    void VertexAttribAnyPointer(const char* funcName, bool isFuncInt, GLuint index,
-                                GLint size, GLenum type, bool normalized, GLsizei stride,
-                                WebGLintptr byteOffset);
-
-public:
     void VertexAttribPointer(GLuint index, GLint size, GLenum type,
                              WebGLboolean normalized, GLsizei stride,
-                             WebGLintptr byteOffset)
-    {
-        const char funcName[] = "vertexAttribPointer";
-        const bool isFuncInt = false;
-        VertexAttribAnyPointer(funcName, isFuncInt, index, size, type, normalized, stride,
-                               byteOffset);
-    }
-
+                             WebGLintptr byteOffset);
     void VertexAttribDivisor(GLuint index, GLuint divisor);
 
 private:
     // Cache the max number of vertices and instances that can be read from
     // bound VBOs (result of ValidateBuffers).
     bool mBufferFetchingIsVerified;
     bool mBufferFetchingHasPerVertex;
     uint32_t mMaxFetchedVertices;
@@ -1798,16 +1785,17 @@ public:
 
     ////
 
 private:
     // -------------------------------------------------------------------------
     // Context customization points
     virtual WebGLVertexArray* CreateVertexArrayImpl();
 
+    virtual bool ValidateAttribPointerType(bool integerMode, GLenum type, uint32_t* alignment, const char* info) = 0;
     virtual bool ValidateUniformMatrixTranspose(bool transpose, const char* info) = 0;
 
 public:
     void ForceLoseContext(bool simulateLoss = false);
 
 protected:
     void ForceRestoreContext();
 
--- a/dom/canvas/WebGLContextBuffers.cpp
+++ b/dom/canvas/WebGLContextBuffers.cpp
@@ -2,17 +2,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "WebGLContext.h"
 
 #include "GLContext.h"
 #include "WebGLBuffer.h"
-#include "WebGLTransformFeedback.h"
 #include "WebGLVertexArray.h"
 
 namespace mozilla {
 
 WebGLRefPtr<WebGLBuffer>*
 WebGLContext::ValidateBufferSlot(const char* funcName, GLenum target)
 {
     WebGLRefPtr<WebGLBuffer>* slot = nullptr;
--- a/dom/canvas/WebGLContextValidate.cpp
+++ b/dom/canvas/WebGLContextValidate.cpp
@@ -334,16 +334,76 @@ WebGLContext::ValidateAttribIndex(GLuint
                               " MAX_VERTEX_ATTRIBS.", info);
         }
     }
 
     return valid;
 }
 
 bool
+WebGLContext::ValidateAttribPointer(bool integerMode, GLuint index, GLint size, GLenum type,
+                                    WebGLboolean normalized, GLsizei stride,
+                                    WebGLintptr byteOffset, const char* info)
+{
+    WebGLBuffer* buffer = mBoundArrayBuffer;
+    if (!buffer) {
+        ErrorInvalidOperation("%s: must have valid GL_ARRAY_BUFFER binding", info);
+        return false;
+    }
+
+    uint32_t requiredAlignment = 0;
+    if (!ValidateAttribPointerType(integerMode, type, &requiredAlignment, info))
+        return false;
+
+    // requiredAlignment should always be a power of two
+    MOZ_ASSERT(IsPowerOfTwo(requiredAlignment));
+    GLsizei requiredAlignmentMask = requiredAlignment - 1;
+
+    if (size < 1 || size > 4) {
+        ErrorInvalidValue("%s: invalid element size", info);
+        return false;
+    }
+
+    switch (type) {
+    case LOCAL_GL_INT_2_10_10_10_REV:
+    case LOCAL_GL_UNSIGNED_INT_2_10_10_10_REV:
+        if (size != 4) {
+            ErrorInvalidOperation("%s: size must be 4 for this type.", info);
+            return false;
+        }
+        break;
+    }
+
+    // see WebGL spec section 6.6 "Vertex Attribute Data Stride"
+    if (stride < 0 || stride > 255) {
+        ErrorInvalidValue("%s: negative or too large stride", info);
+        return false;
+    }
+
+    if (byteOffset < 0) {
+        ErrorInvalidValue("%s: negative offset", info);
+        return false;
+    }
+
+    if (stride & requiredAlignmentMask) {
+        ErrorInvalidOperation("%s: stride doesn't satisfy the alignment "
+                              "requirement of given type", info);
+        return false;
+    }
+
+    if (byteOffset & requiredAlignmentMask) {
+        ErrorInvalidOperation("%s: byteOffset doesn't satisfy the alignment "
+                              "requirement of given type", info);
+        return false;
+    }
+
+    return true;
+}
+
+bool
 WebGLContext::ValidateStencilParamsForDrawCall()
 {
     const char msg[] = "%s set different front and back stencil %s. Drawing in"
                        " this configuration is not allowed.";
 
     if (mStencilRefFront != mStencilRefBack) {
         ErrorInvalidOperation(msg, "stencilFuncSeparate", "reference values");
         return false;
--- a/dom/canvas/WebGLContextVertices.cpp
+++ b/dom/canvas/WebGLContextVertices.cpp
@@ -282,158 +282,49 @@ WebGLContext::GetVertexAttribOffset(GLui
         ErrorInvalidEnum("getVertexAttribOffset: bad parameter");
         return 0;
     }
 
     MOZ_ASSERT(mBoundVertexArray);
     return mBoundVertexArray->mAttribs[index].ByteOffset();
 }
 
-////////////////////////////////////////
-
 void
-WebGLContext::VertexAttribAnyPointer(const char* funcName, bool isFuncInt, GLuint index,
-                                     GLint size, GLenum type, bool normalized,
-                                     GLsizei stride, WebGLintptr byteOffset)
+WebGLContext::VertexAttribPointer(GLuint index, GLint size, GLenum type,
+                                  WebGLboolean normalized, GLsizei stride,
+                                  WebGLintptr byteOffset)
 {
     if (IsContextLost())
         return;
 
-    if (!ValidateAttribIndex(index, funcName))
+    if (!ValidateAttribIndex(index, "vertexAttribPointer"))
+        return;
+
+    if (!ValidateAttribPointer(false, index, size, type, normalized, stride, byteOffset, "vertexAttribPointer"))
         return;
 
-    ////
-
-    if (size < 1 || size > 4) {
-        ErrorInvalidValue("%s: invalid element size", funcName);
-        return;
-    }
-
-    // see WebGL spec section 6.6 "Vertex Attribute Data Stride"
-    if (stride < 0 || stride > 255) {
-        ErrorInvalidValue("%s: negative or too large stride", funcName);
-        return;
-    }
-
-    if (byteOffset < 0) {
-        ErrorInvalidValue("%s: negative offset", funcName);
-        return;
-    }
-
-    ////
+    MOZ_ASSERT(mBoundVertexArray);
 
-    bool isTypeValid = true;
-    uint8_t typeAlignment;
-    switch (type) {
-    // WebGL 1:
-    case LOCAL_GL_BYTE:
-    case LOCAL_GL_UNSIGNED_BYTE:
-        typeAlignment = 1;
-        break;
-
-    case LOCAL_GL_SHORT:
-    case LOCAL_GL_UNSIGNED_SHORT:
-        typeAlignment = 2;
-        break;
-
-    case LOCAL_GL_FLOAT:
-        if (isFuncInt) {
-            isTypeValid = false;
-        }
-        typeAlignment = 4;
-        break;
-
-    // WebGL 2:
-    case LOCAL_GL_INT:
-    case LOCAL_GL_UNSIGNED_INT:
-        if (!IsWebGL2()) {
-            isTypeValid = false;
-        }
-        typeAlignment = 4;
-        break;
-
-    case LOCAL_GL_HALF_FLOAT:
-        if (isFuncInt || !IsWebGL2()) {
-            isTypeValid = false;
-        }
-        typeAlignment = 2;
-        break;
+    InvalidateBufferFetching();
 
-    case LOCAL_GL_FIXED:
-        if (isFuncInt || !IsWebGL2()) {
-            isTypeValid = false;
-        }
-        typeAlignment = 4;
-        break;
-
-    case LOCAL_GL_INT_2_10_10_10_REV:
-    case LOCAL_GL_UNSIGNED_INT_2_10_10_10_REV:
-        if (isFuncInt || !IsWebGL2()) {
-            isTypeValid = false;
-            break;
-        }
-        if (size != 4) {
-            ErrorInvalidOperation("%s: size must be 4 for this type.", funcName);
-            return;
-        }
-        typeAlignment = 4;
-        break;
-
-    default:
-        isTypeValid = false;
-        break;
-    }
-    if (!isTypeValid) {
-        ErrorInvalidEnumArg(funcName, "type", type);
-        return;
-    }
-
-    ////
+    /* XXX make work with bufferSubData & heterogeneous types
+     if (type != mBoundArrayBuffer->GLType())
+     return ErrorInvalidOperation("vertexAttribPointer: type must match bound VBO type: %d != %d", type, mBoundArrayBuffer->GLType());
+     */
 
-    // `alignment` should always be a power of two.
-    MOZ_ASSERT(IsPowerOfTwo(typeAlignment));
-    const GLsizei typeAlignmentMask = typeAlignment - 1;
-
-    if (stride & typeAlignmentMask ||
-        byteOffset & typeAlignmentMask)
-    {
-        ErrorInvalidOperation("%s: `stride` and `byteOffset` must satisfy the alignment"
-                              " requirement of `type`.",
-                              funcName);
-        return;
-    }
-
-    ////
-
-    const auto& buffer = mBoundArrayBuffer;
-    if (!buffer && byteOffset) {
-        ErrorInvalidOperation("%s: If ARRAY_BUFFER is null, byteOffset must be zero.",
-                              funcName);
-        return;
-    }
-
-    ////
-
-    gl->MakeCurrent();
-    if (isFuncInt) {
-        gl->fVertexAttribIPointer(index, size, type, stride,
-                                  reinterpret_cast<void*>(byteOffset));
-    } else {
-        gl->fVertexAttribPointer(index, size, type, normalized, stride,
-                                 reinterpret_cast<void*>(byteOffset));
-    }
+    MakeContextCurrent();
+    gl->fVertexAttribPointer(index, size, type, normalized, stride,
+                             reinterpret_cast<void*>(byteOffset));
 
     WebGLVertexAttribData& vd = mBoundVertexArray->mAttribs[index];
-    vd.VertexAttribPointer(isFuncInt, buffer, size, type, normalized, stride, byteOffset);
-
-    InvalidateBufferFetching();
+    const bool integerFunc = false;
+    vd.VertexAttribPointer(integerFunc, mBoundArrayBuffer, size, type, normalized, stride,
+                           byteOffset);
 }
 
-////////////////////////////////////////
-
 void
 WebGLContext::VertexAttribDivisor(GLuint index, GLuint divisor)
 {
     if (IsContextLost())
         return;
 
     if (!ValidateAttribIndex(index, "vertexAttribDivisor"))
         return;
--- a/dom/canvas/moz.build
+++ b/dom/canvas/moz.build
@@ -105,16 +105,17 @@ UNIFIED_SOURCES += [
     'WebGL2ContextRenderbuffers.cpp',
     'WebGL2ContextSamplers.cpp',
     'WebGL2ContextState.cpp',
     'WebGL2ContextSync.cpp',
     'WebGL2ContextTextures.cpp',
     'WebGL2ContextTransformFeedback.cpp',
     'WebGL2ContextUniforms.cpp',
     'WebGL2ContextVAOs.cpp',
+    'WebGL2ContextVertices.cpp',
     'WebGLActiveInfo.cpp',
     'WebGLBuffer.cpp',
     'WebGLContext.cpp',
     'WebGLContextBuffers.cpp',
     'WebGLContextDraw.cpp',
     'WebGLContextExtensions.cpp',
     'WebGLContextFramebufferOperations.cpp',
     'WebGLContextGL.cpp',