b=571078; fix webgl attribute validation; r=bjacob
authorVladimir Vukicevic <vladimir@pobox.com>
Thu, 10 Jun 2010 10:45:00 -0700
changeset 43474 7b15545cf9aaf59f1c7148872f0fa1071b611010
parent 43473 0327e126ea245112c0aa7283fee154e084866fb5
child 43475 ffa8bc798d99557e82ec7862e9ccdf555e1dde91
push id13726
push uservladimir@mozilla.com
push dateThu, 10 Jun 2010 17:45:28 +0000
treeherdermozilla-central@7b15545cf9aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbjacob
bugs571078
milestone1.9.3a6pre
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
b=571078; fix webgl attribute validation; r=bjacob
content/canvas/src/WebGLContext.h
content/canvas/src/WebGLContextGL.cpp
content/canvas/src/WebGLContextValidate.cpp
--- a/content/canvas/src/WebGLContext.h
+++ b/content/canvas/src/WebGLContext.h
@@ -36,16 +36,17 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #ifndef WEBGLCONTEXT_H_
 #define WEBGLCONTEXT_H_
 
 #include <stdarg.h>
+#include <vector>
 
 #include "nsTArray.h"
 #include "nsDataHashtable.h"
 #include "nsRefPtrHashtable.h"
 #include "nsHashKeys.h"
 
 #include "nsIDocShell.h"
 
@@ -67,16 +68,17 @@ class WebGLTexture;
 class WebGLBuffer;
 class WebGLProgram;
 class WebGLShader;
 class WebGLFramebuffer;
 class WebGLRenderbuffer;
 class WebGLUniformLocation;
 
 class WebGLZeroingObject;
+class WebGLContextBoundObject;
 
 class WebGLObjectBaseRefPtr
 {
 protected:
     friend class WebGLZeroingObject;
 
     WebGLObjectBaseRefPtr()
         : mRawPtr(0)
@@ -645,17 +647,19 @@ class WebGLProgram :
     public WebGLZeroingObject,
     public WebGLContextBoundObject
 {
 public:
     NS_DECLARE_STATIC_IID_ACCESSOR(WEBGLPROGRAM_PRIVATE_IID)
 
     WebGLProgram(WebGLContext *context, WebGLuint name) :
         WebGLContextBoundObject(context),
-        mName(name), mDeleted(PR_FALSE), mLinkStatus(PR_FALSE), mGeneration(0)
+        mName(name), mDeleted(PR_FALSE), mLinkStatus(PR_FALSE), mGeneration(0),
+        mUniformMaxNameLength(0), mAttribMaxNameLength(0),
+        mUniformCount(0), mAttribCount(0)
     {
         mMapUniformLocations.Init();
     }
 
     void Delete() {
         if (mDeleted)
             return;
         ZeroOwners();
@@ -707,25 +711,40 @@ public:
             return PR_FALSE; // must exit without changing mGeneration
         mGeneration = nextGeneration;
         mMapUniformLocations.Clear();
         return PR_TRUE;
     }
 
     already_AddRefed<WebGLUniformLocation> GetUniformLocationObject(GLint glLocation);
 
+    /* Called only after LinkProgram */
+    PRBool UpdateInfo(gl::GLContext *gl);
+
+    /* Getters for cached program info */
+    WebGLint UniformMaxNameLength() const { return mUniformMaxNameLength; }
+    WebGLint AttribMaxNameLength() const { return mAttribMaxNameLength; }
+    WebGLint UniformCount() const { return mUniformCount; }
+    WebGLint AttribCount() const { return mAttribCount; }
+    bool IsAttribInUse(unsigned i) const { return mAttribsInUse[i]; }
+
     NS_DECL_ISUPPORTS
     NS_DECL_NSIWEBGLPROGRAM
 protected:
     WebGLuint mName;
     PRPackedBool mDeleted;
     PRPackedBool mLinkStatus;
     nsTArray<WebGLShader*> mAttachedShaders;
     nsRefPtrHashtable<nsUint32HashKey, WebGLUniformLocation> mMapUniformLocations;
     GLuint mGeneration;
+    GLint mUniformMaxNameLength;
+    GLint mAttribMaxNameLength;
+    GLint mUniformCount;
+    GLint mAttribCount;
+    std::vector<bool> mAttribsInUse;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(WebGLProgram, WEBGLPROGRAM_PRIVATE_IID)
 
 #define WEBGLFRAMEBUFFER_PRIVATE_IID \
     {0x0052a16f, 0x4bc9, 0x4a55, {0x9d, 0xa3, 0x54, 0x95, 0xaa, 0x4e, 0x80, 0xb9}}
 class WebGLFramebuffer :
     public nsIWebGLFramebuffer,
--- a/content/canvas/src/WebGLContextGL.cpp
+++ b/content/canvas/src/WebGLContextGL.cpp
@@ -870,17 +870,17 @@ WebGLContext::DrawElements(WebGLenum mod
     // If there is no current program, this is silently ignored.
     // Any checks below this depend on a program being available.
     if (!mCurrentProgram)
         return NS_OK;
 
     if (!mBoundElementArrayBuffer)
         return ErrorInvalidOperation("DrawElements: must have element array buffer binding");
 
-    if (byteOffset+byteCount < byteOffset || byteOffset+byteCount < byteCount)
+    if (byteOffset+byteCount < WebGLuint(byteOffset) || byteOffset+byteCount < byteCount)
         return ErrorInvalidOperation("DrawElements: overflow in byteOffset+byteCount");
 
     if (byteOffset + byteCount > mBoundElementArrayBuffer->ByteLength())
         return ErrorInvalidOperation("DrawElements: bound element array buffer is too small for given count and offset");
 
     WebGLuint maxIndex = 0;
     if (type == LOCAL_GL_UNSIGNED_SHORT) {
         maxIndex = mBoundElementArrayBuffer->FindMaximum<GLushort>(count, byteOffset);
@@ -1037,26 +1037,25 @@ WebGLContext::GetActiveAttrib(nsIWebGLPr
     if (NS_FAILED(js.error))
         return js.error;
 
     MakeContextCurrent();
 
     GLint len = 0;
     gl->fGetProgramiv(progname, LOCAL_GL_ACTIVE_ATTRIBUTE_MAX_LENGTH, &len);
     if (len == 0) {
-        // is this an error?  can you have a program with no attributes?
         *retval = nsnull;
         return NS_OK;
     }
 
-    nsAutoArrayPtr<char> name(new char[len+1]);
+    nsAutoArrayPtr<char> name(new char[len]);
     PRInt32 attrsize = 0;
     PRUint32 attrtype = 0;
 
-    gl->fGetActiveAttrib(progname, index, len+1, &len, (GLint*) &attrsize, (WebGLuint*) &attrtype, name);
+    gl->fGetActiveAttrib(progname, index, len, &len, (GLint*) &attrsize, (WebGLuint*) &attrtype, name);
     if (attrsize == 0 || attrtype == 0) {
         *retval = nsnull;
         return NS_OK;
     }
 
     JSObjectHelper retobj(&js);
     retobj.DefineProperty("size", attrsize);
     retobj.DefineProperty("type", attrtype);
@@ -1077,26 +1076,30 @@ WebGLContext::GetActiveUniform(nsIWebGLP
     NativeJSContext js;
     if (NS_FAILED(js.error))
         return js.error;
 
     MakeContextCurrent();
 
     GLint len = 0;
     gl->fGetProgramiv(progname, LOCAL_GL_ACTIVE_UNIFORM_MAX_LENGTH, &len);
-    if (len == 0)
-        return NS_ERROR_FAILURE; // XXX GL error?  This really shouldn't happen.
-
-    nsAutoArrayPtr<char> name(new char[len+1]);
+    if (len == 0) {
+        *retval = nsnull;
+        return NS_OK;
+    }
+
+    nsAutoArrayPtr<char> name(new char[len]);
     PRInt32 attrsize = 0;
     PRUint32 attrtype = 0;
 
-    gl->fGetActiveUniform(progname, index, len+1, &len, (GLint*) &attrsize, (WebGLenum*) &attrtype, name);
-    if (attrsize == 0 || attrtype == 0)
-        return NS_ERROR_FAILURE; // XXX GL error?  This really shouldn't happen.
+    gl->fGetActiveUniform(progname, index, len, &len, (GLint*) &attrsize, (WebGLenum*) &attrtype, name);
+    if (len == 0 || attrsize == 0 || attrtype == 0) {
+        *retval = nsnull;
+        return NS_OK;
+    }
 
     JSObjectHelper retobj(&js);
     retobj.DefineProperty("size", attrsize);
     retobj.DefineProperty("type", attrtype);
     retobj.DefineProperty("name", name, len);
 
     js.SetRetVal(retobj.Object());
 
@@ -1765,17 +1768,16 @@ WebGLContext::GetTexParameter(WebGLenum 
 
         default:
             return ErrorInvalidEnum("GetTexParameter: invalid parameter");
     }
 
     return NS_OK;
 }
 
-/* XXX fix */
 /* any getUniform(in WebGLProgram program, in WebGLUniformLocation location) raises(DOMException); */
 NS_IMETHODIMP
 WebGLContext::GetUniform(nsIWebGLProgram *pobj, nsIWebGLUniformLocation *ploc)
 {
     WebGLuint progname;
     WebGLProgram *prog;
     if (!GetConcreteObjectAndGLName(pobj, &prog, &progname))
         return ErrorInvalidOperation("GetUniform: invalid program");
@@ -1792,25 +1794,25 @@ WebGLContext::GetUniform(nsIWebGLProgram
 
     NativeJSContext js;
     if (NS_FAILED(js.error))
         return js.error;
 
     MakeContextCurrent();
 
     GLint uniforms = 0;
+    GLint uniformNameMaxLength = 0;
     gl->fGetProgramiv(progname, LOCAL_GL_ACTIVE_UNIFORMS, &uniforms);
+    gl->fGetProgramiv(progname, LOCAL_GL_ACTIVE_UNIFORM_MAX_LENGTH, &uniformNameMaxLength);
 
     // we now need the type info to switch between fGetUniformfv and fGetUniformiv
     // the only way to get that is to iterate through all active uniforms by index until
     // one matches the given uniform location.
     GLenum uniformType = 0;
-    GLint uniformNameMaxLength = 0;
-    gl->fGetProgramiv(progname, LOCAL_GL_ACTIVE_UNIFORM_MAX_LENGTH, &uniformNameMaxLength);
-    nsAutoArrayPtr<GLchar> uniformName(new GLchar[uniformNameMaxLength+1]);
+    nsAutoArrayPtr<GLchar> uniformName(new GLchar[uniformNameMaxLength]);
     GLint index;
     for (index = 0; index < uniforms; ++index) {
         GLsizei dummyLength;
         GLint dummySize;
         gl->fGetActiveUniform(progname, index, uniformNameMaxLength, &dummyLength,
                               &dummySize, &uniformType, uniformName);
         if (gl->fGetUniformLocation(progname, uniformName) == location->Location())
             break;
@@ -2011,17 +2013,22 @@ WebGLContext::LinkProgram(nsIWebGLProgra
     MakeContextCurrent();
 
     gl->fLinkProgram(progname);
     if (!program->NextGeneration())
         return NS_ERROR_FAILURE;
 
     GLint ok;
     gl->fGetProgramiv(progname, LOCAL_GL_LINK_STATUS, &ok);
-    program->SetLinkStatus(ok ? PR_TRUE : PR_FALSE);
+    if (ok) {
+        program->SetLinkStatus(PR_TRUE);
+        program->UpdateInfo(gl);
+    } else {
+        program->SetLinkStatus(PR_FALSE);
+    }
 
     return NS_OK;
 }
 
 // XXX #if 0
 NS_IMETHODIMP
 WebGLContext::PixelStorei(WebGLenum pname, WebGLint param)
 {
@@ -2652,70 +2659,16 @@ WebGLContext::VertexAttribPointer(WebGLu
 
     gl->fVertexAttribPointer(index, size, type, normalized,
                              stride,
                              (void*) (byteOffset));
 
     return NS_OK;
 }
 
-PRBool
-WebGLContext::ValidateGL()
-{
-    // make sure that the opengl stuff that we need is supported
-    GLint val = 0;
-
-    // XXX this exposes some strange latent bug; what's going on?
-    //MakeContextCurrent();
-
-    gl->fGetIntegerv(LOCAL_GL_MAX_VERTEX_ATTRIBS, &val);
-    if (val == 0) {
-        LogMessage("GL_MAX_VERTEX_ATTRIBS is 0!");
-        return PR_FALSE;
-    }
-
-    mAttribBuffers.SetLength(val);
-
-    //fprintf(stderr, "GL_MAX_VERTEX_ATTRIBS: %d\n", val);
-
-    // Note: GL_MAX_TEXTURE_UNITS is fixed at 4 for most desktop hardware,
-    // even though the hardware supports much more.  The
-    // GL_MAX_{COMBINED_}TEXTURE_IMAGE_UNITS value is the accurate
-    // value.  For GLES2, GL_MAX_TEXTURE_UNITS is still correct.
-    gl->fGetIntegerv(LOCAL_GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, &val);
-    if (val == 0) {
-        LogMessage("GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS is 0!");
-        return PR_FALSE;
-    }
-
-    mBound2DTextures.SetLength(val);
-    mBoundCubeMapTextures.SetLength(val);
-
-    //fprintf(stderr, "GL_MAX_TEXTURE_UNITS: %d\n", val);
-
-    gl->fGetIntegerv(LOCAL_GL_MAX_COLOR_ATTACHMENTS, &val);
-    mFramebufferColorAttachments.SetLength(val);
-
-#if defined(DEBUG_vladimir) && defined(USE_GLES2)
-    gl->fGetIntegerv(LOCAL_GL_IMPLEMENTATION_COLOR_READ_FORMAT, &val);
-    fprintf(stderr, "GL_IMPLEMENTATION_COLOR_READ_FORMAT: 0x%04x\n", val);
-
-    gl->fGetIntegerv(LOCAL_GL_IMPLEMENTATION_COLOR_READ_TYPE, &val);
-    fprintf(stderr, "GL_IMPLEMENTATION_COLOR_READ_TYPE: 0x%04x\n", val);
-#endif
-
-#ifndef USE_GLES2
-    // gl_PointSize is always available in ES2 GLSL, but has to be
-    // specifically enabled on desktop GLSL.
-    gl->fEnable(LOCAL_GL_VERTEX_PROGRAM_POINT_SIZE);
-#endif
-
-    return PR_TRUE;
-}
-
 NS_IMETHODIMP
 WebGLContext::TexImage2D(PRInt32 dummy)
 {
     return NS_ERROR_FAILURE;
 }
 
 nsresult
 WebGLContext::TexImage2D_base(WebGLenum target, WebGLint level, WebGLenum internalformat,
--- a/content/canvas/src/WebGLContextValidate.cpp
+++ b/content/canvas/src/WebGLContextValidate.cpp
@@ -37,77 +37,94 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "WebGLContext.h"
 
 using namespace mozilla;
 
 /*
+ * Pull all the data out of the program that will be used by validate later on
+ */
+PRBool
+WebGLProgram::UpdateInfo(gl::GLContext *gl)
+{
+    gl->fGetProgramiv(mName, LOCAL_GL_ACTIVE_ATTRIBUTE_MAX_LENGTH, &mAttribMaxNameLength);
+    gl->fGetProgramiv(mName, LOCAL_GL_ACTIVE_UNIFORM_MAX_LENGTH, &mUniformMaxNameLength);
+    gl->fGetProgramiv(mName, LOCAL_GL_ACTIVE_UNIFORMS, &mUniformCount);
+    gl->fGetProgramiv(mName, LOCAL_GL_ACTIVE_ATTRIBUTES, &mAttribCount);
+
+    GLint numVertexAttribs;
+    gl->fGetIntegerv(LOCAL_GL_MAX_VERTEX_ATTRIBS, &numVertexAttribs);
+    mAttribsInUse.clear();
+    mAttribsInUse.resize(numVertexAttribs);
+
+    nsAutoArrayPtr<char> nameBuf(new char[mAttribMaxNameLength]);
+
+    for (int i = 0; i < mAttribCount; ++i) {
+        GLint attrnamelen;
+        GLint attrsize;
+        GLenum attrtype;
+        gl->fGetActiveAttrib(mName, i, mAttribMaxNameLength, &attrnamelen, &attrsize, &attrtype, nameBuf);
+        if (attrnamelen > 0) {
+            GLint loc = gl->fGetAttribLocation(mName, nameBuf);
+            mAttribsInUse[loc] = true;
+        }
+    }
+
+    return PR_TRUE;
+}
+
+/*
  * Verify that we can read count consecutive elements from each bound VBO.
  */
 
 PRBool
 WebGLContext::ValidateBuffers(PRUint32 count)
 {
-    GLint currentProgram = -1;
-    GLint numAttributes = -1;
-
     NS_ENSURE_TRUE(count > 0, PR_TRUE);
 
+#ifdef DEBUG
+    GLuint currentProgram = 0;
     MakeContextCurrent();
-
-    // XXX cache this per program
-    gl->fGetIntegerv(LOCAL_GL_CURRENT_PROGRAM, &currentProgram);
-    if (currentProgram == -1) {
-        // what?
-        LogMessage("glGetIntegerv GL_CURRENT_PROGRAM failed: 0x%08x", (uint) gl->fGetError());
+    gl->fGetIntegerv(LOCAL_GL_CURRENT_PROGRAM, (GLint*) &currentProgram);
+    NS_ASSERTION(currentProgram == mCurrentProgram->GLName(),
+                 "WebGL: current program doesn't agree with GL state");
+    if (currentProgram != mCurrentProgram->GLName())
         return PR_FALSE;
-    }
+#endif
 
-    if (WebGLuint(currentProgram) != mCurrentProgram->GLName()) {
-        LogMessage("WebGL internal error: current program (%u) doesn't agree with GL current program (%d)", mCurrentProgram->GLName(), currentProgram);
-        return PR_FALSE;
-    }
+    PRUint32 attribs = mAttribBuffers.Length();
+    for (PRUint32 i = 0; i < attribs; ++i) {
+        const WebGLVertexAttribData& vd = mAttribBuffers[i];
 
-    gl->fGetProgramiv(currentProgram, LOCAL_GL_ACTIVE_ATTRIBUTES, &numAttributes);
-    if (numAttributes == -1) {
-        // what?
-        LogMessage("glGetProgramiv GL_ACTIVE_ATTRIBUTES failed: 0x%08x", (uint) gl->fGetError());
-        return PR_FALSE;
-    }
+        // If the attrib array isn't enabled, there's nothing to check;
+        // it's a static value.
+        if (!vd.enabled)
+            continue;
 
-    // is this valid?
-    if (numAttributes > (GLint) mAttribBuffers.Length()) {
-        // what?
-        LogMessage("GL_ACTIVE_ATTRIBUTES > GL_MAX_VERTEX_ATTRIBS");
-        return PR_FALSE;
-    }
-    PRUint32 maxAttribs = numAttributes;
+        if (vd.buf == nsnull) {
+            LogMessage("No VBO bound to enabled attrib index %d!", i);
+            return PR_FALSE;
+        }
 
-    for (PRUint32 i = 0; i < maxAttribs; ++i) {
-      WebGLVertexAttribData& vd = mAttribBuffers[i];
+        // If the attrib is not in use, then we don't have to validate
+        // it, just need to make sure that the binding is non-null.
+        if (!mCurrentProgram->IsAttribInUse(i))
+            continue;
 
-      // is this a problem?
-      if (!vd.enabled)
-          continue;
-
-      if (vd.buf == nsnull) {
-          LogMessage("No VBO bound to index %d (or it's been deleted)!", i);
-          return PR_FALSE;
-      }
+        // compute the number of bytes we actually need
+        WebGLuint needed = vd.byteOffset +     // the base offset
+            vd.actualStride() * (count-1) +    // to stride to the start of the last element group
+            vd.componentSize() * vd.size;      // and the number of bytes needed for these components
 
-      WebGLuint needed = vd.byteOffset +     // the base offset
-          vd.actualStride() * (count-1) +    // to stride to the start of the last element group
-          vd.componentSize() * vd.size;      // and the number of bytes needed for these components
-
-      if (vd.buf->ByteLength() < needed) {
-          LogMessage("VBO too small for bound attrib index %d: need at least %d bytes, but have only %d", i, needed, vd.buf->ByteLength());
-          return PR_FALSE;
-      }
+        if (vd.buf->ByteLength() < needed) {
+            LogMessage("VBO too small for bound attrib index %d: need at least %d bytes, but have only %d", i, needed, vd.buf->ByteLength());
+            return PR_FALSE;
+        }
     }
 
     return PR_TRUE;
 }
 
 PRBool WebGLContext::ValidateCapabilityEnum(WebGLenum cap)
 {
     switch (cap) {
@@ -120,8 +137,62 @@ PRBool WebGLContext::ValidateCapabilityE
         case LOCAL_GL_SAMPLE_COVERAGE:
         case LOCAL_GL_SCISSOR_TEST:
         case LOCAL_GL_STENCIL_TEST:
             return PR_TRUE;
         default:
             return PR_FALSE;
     }
 }
+
+PRBool
+WebGLContext::ValidateGL()
+{
+    // make sure that the opengl stuff that we need is supported
+    GLint val = 0;
+
+    // XXX this exposes some strange latent bug; what's going on?
+    //MakeContextCurrent();
+
+    gl->fGetIntegerv(LOCAL_GL_MAX_VERTEX_ATTRIBS, &val);
+    if (val == 0) {
+        LogMessage("GL_MAX_VERTEX_ATTRIBS is 0!");
+        return PR_FALSE;
+    }
+
+    mAttribBuffers.SetLength(val);
+
+    //fprintf(stderr, "GL_MAX_VERTEX_ATTRIBS: %d\n", val);
+
+    // Note: GL_MAX_TEXTURE_UNITS is fixed at 4 for most desktop hardware,
+    // even though the hardware supports much more.  The
+    // GL_MAX_{COMBINED_}TEXTURE_IMAGE_UNITS value is the accurate
+    // value.  For GLES2, GL_MAX_TEXTURE_UNITS is still correct.
+    gl->fGetIntegerv(LOCAL_GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, &val);
+    if (val == 0) {
+        LogMessage("GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS is 0!");
+        return PR_FALSE;
+    }
+
+    mBound2DTextures.SetLength(val);
+    mBoundCubeMapTextures.SetLength(val);
+
+    //fprintf(stderr, "GL_MAX_TEXTURE_UNITS: %d\n", val);
+
+    gl->fGetIntegerv(LOCAL_GL_MAX_COLOR_ATTACHMENTS, &val);
+    mFramebufferColorAttachments.SetLength(val);
+
+#if defined(DEBUG_vladimir) && defined(USE_GLES2)
+    gl->fGetIntegerv(LOCAL_GL_IMPLEMENTATION_COLOR_READ_FORMAT, &val);
+    fprintf(stderr, "GL_IMPLEMENTATION_COLOR_READ_FORMAT: 0x%04x\n", val);
+
+    gl->fGetIntegerv(LOCAL_GL_IMPLEMENTATION_COLOR_READ_TYPE, &val);
+    fprintf(stderr, "GL_IMPLEMENTATION_COLOR_READ_TYPE: 0x%04x\n", val);
+#endif
+
+#ifndef USE_GLES2
+    // gl_PointSize is always available in ES2 GLSL, but has to be
+    // specifically enabled on desktop GLSL.
+    gl->fEnable(LOCAL_GL_VERTEX_PROGRAM_POINT_SIZE);
+#endif
+
+    return PR_TRUE;
+}