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 391349 8675b60fad97dd1c1c8e999b3dd5f7c7aa9582dd
parent 391348 f2fa758c8d307b7ecf204439b4a0bf08b53927aa
child 391350 447c280cbd29c2184ee5818c9a678f5b21a5e1bf
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1330022
milestone54.0a1
backs out0528322db042d01a79c70427546f7ba6ceac7a8e
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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',