Bug 682496 - fixed program-test.html test failures - r=bjacob
authorDoug Sherk <dsherk@mozilla.com>
Thu, 03 Nov 2011 10:50:40 -0400
changeset 80328 7da669483434e13e0cf1c8557cd7de6d8803287b
parent 80327 35ecb3d6a36b7581382fad8ce91a652bcf52e8de
child 80329 ce1330a5c9cb4a3d590dc81b378e5236279aa76f
push id506
push userclegnitto@mozilla.com
push dateWed, 09 Nov 2011 02:03:18 +0000
treeherdermozilla-aurora@63587fc7bb93 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbjacob
bugs682496
milestone10.0a1
Bug 682496 - fixed program-test.html test failures - r=bjacob Fixed programs and shaders so that when they're marked for deletion and then detached completely, are deleted.
content/canvas/src/WebGLContext.h
content/canvas/src/WebGLContextGL.cpp
content/canvas/test/webgl/failing_tests_mac.txt
--- a/content/canvas/src/WebGLContext.h
+++ b/content/canvas/src/WebGLContext.h
@@ -728,16 +728,17 @@ public:
     // console logging helpers
     static void LogMessage(const char *fmt, ...);
     static void LogMessage(const char *fmt, va_list ap);
     void LogMessageIfVerbose(const char *fmt, ...);
     void LogMessageIfVerbose(const char *fmt, va_list ap);
 
     friend class WebGLTexture;
     friend class WebGLFramebuffer;
+    friend class WebGLProgram;
 };
 
 // this class is a mixin for the named type wrappers, and is used
 // by WebGLObjectRefPtr to tell the object who holds references, so that
 // we can zero them out appropriately when the object is deleted, because
 // it will be unbound in the GL.
 //
 // PreallocatedOwnersCapacity is the preallocated capacity for the array of refptrs to owners.
@@ -1416,31 +1417,53 @@ class WebGLShader :
     public WebGLContextBoundObject
 {
 public:
     NS_DECLARE_STATIC_IID_ACCESSOR(WEBGLSHADER_PRIVATE_IID)
 
     WebGLShader(WebGLContext *context, WebGLuint name, WebGLenum stype) :
         WebGLContextBoundObject(context),
         mName(name), mDeleted(false), mType(stype),
-        mNeedsTranslation(true), mAttachCount(0)
+        mNeedsTranslation(true), mAttachCount(0),
+        mDeletePending(false)
     { }
 
+    void DetachedFromProgram() {
+        DecrementAttachCount();
+        if (mDeletePending && AttachCount() <= 0) {
+            DeleteWhenNotAttached();
+        }
+    }
+
+    void DeleteWhenNotAttached() {
+        if (mDeleted)
+            return;
+
+        if (AttachCount() > 0) {
+            mDeletePending = true;
+            return;
+        }
+
+        Delete();
+    }
+
     void Delete() {
         if (mDeleted)
             return;
+
         ZeroOwners();
         mDeleted = true;
+        mDeletePending = false;
     }
 
-    bool Deleted() { return mDeleted && mAttachCount == 0; }
+    bool Deleted() { return mDeleted; }
     WebGLuint GLName() { return mName; }
     WebGLenum ShaderType() { return mType; }
 
-    PRUint32 AttachCount() { return mAttachCount; }
+    PRInt32 AttachCount() { return mAttachCount; }
     void IncrementAttachCount() { mAttachCount++; }
     void DecrementAttachCount() { mAttachCount--; }
 
     void SetSource(const nsAString& src) {
         // XXX do some quick gzip here maybe -- getting this will be very rare
         mSource.Assign(src);
     }
 
@@ -1464,17 +1487,18 @@ public:
     NS_DECL_NSIWEBGLSHADER
 protected:
     WebGLuint mName;
     bool mDeleted;
     WebGLenum mType;
     nsString mSource;
     nsCString mTranslationLog;
     bool mNeedsTranslation;
-    PRUint32 mAttachCount;
+    PRInt32 mAttachCount;
+    bool mDeletePending;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(WebGLShader, WEBGLSHADER_PRIVATE_IID)
 
 #define WEBGLPROGRAM_PRIVATE_IID \
     {0xb3084a5b, 0xa5b4, 0x4ee0, {0xa0, 0xf0, 0xfb, 0xdd, 0x64, 0xaf, 0x8e, 0x82}}
 class WebGLProgram :
     public nsIWebGLProgram,
@@ -1491,32 +1515,55 @@ public:
         mName(name), mDeleted(false), mDeletePending(false),
         mLinkStatus(false), mGeneration(0),
         mUniformMaxNameLength(0), mAttribMaxNameLength(0),
         mUniformCount(0), mAttribCount(0)
     {
         mMapUniformLocations.Init();
     }
 
+    void DeleteWhenNotCurrent() {
+        if (mDeleted)
+            return;
+
+        if (mContext->mCurrentProgram == this) {
+            mDeletePending = true;
+            return;
+        }
+
+        Delete();
+    }
+
     void Delete() {
         if (mDeleted)
             return;
+
+        DetachShaders();
         ZeroOwners();
         mDeleted = true;
+        mDeletePending = false;
     }
 
     void DetachShaders() {
         for (PRUint32 i = 0; i < mAttachedShaders.Length(); ++i) {
-            if (mAttachedShaders[i])
-                mAttachedShaders[i]->DecrementAttachCount();
+            WebGLShader* shader = mAttachedShaders[i];
+            if (shader)
+                shader->DetachedFromProgram();
         }
         mAttachedShaders.Clear();
     }
 
-    bool Deleted() { return mDeleted && !mDeletePending; }
+    void NoLongerCurrent() {
+        if (mDeletePending) {
+            DetachShaders();
+            DeleteWhenNotCurrent();
+        }
+    }
+
+    bool Deleted() { return mDeleted; }
     void SetDeletePending() { mDeletePending = true; }
     void ClearDeletePending() { mDeletePending = false; }
     bool HasDeletePending() { return mDeletePending; }
 
     WebGLuint GLName() { return mName; }
     const nsTArray<nsRefPtr<WebGLShader> >& AttachedShaders() const { return mAttachedShaders; }
     bool LinkStatus() { return mLinkStatus; }
     PRUint32 Generation() const { return mGeneration.value(); }
@@ -1533,17 +1580,17 @@ public:
         mAttachedShaders.AppendElement(shader);
         shader->IncrementAttachCount();
         return true;
     }
 
     // return true if the shader was found and removed
     bool DetachShader(WebGLShader *shader) {
         if (mAttachedShaders.RemoveElement(shader)) {
-            shader->DecrementAttachCount();
+            shader->DetachedFromProgram();
             return true;
         }
         return false;
     }
 
     bool HasAttachedShaderOfType(GLenum shaderType) {
         for (PRUint32 i = 0; i < mAttachedShaders.Length(); ++i) {
             if (mAttachedShaders[i] && mAttachedShaders[i]->ShaderType() == shaderType) {
--- a/content/canvas/src/WebGLContextGL.cpp
+++ b/content/canvas/src/WebGLContextGL.cpp
@@ -1260,23 +1260,17 @@ WebGLContext::DeleteProgram(nsIWebGLProg
 
     if (isNull || isDeleted)
         return NS_OK;
 
     MakeContextCurrent();
 
     gl->fDeleteProgram(progname);
 
-    if (prog == mCurrentProgram) {
-        prog->SetDeletePending();
-    } else {
-        prog->DetachShaders();
-    }
-
-    prog->Delete();
+    prog->DeleteWhenNotCurrent();
     mMapPrograms.Remove(progname);
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
 WebGLContext::DeleteShader(nsIWebGLShader *sobj)
 {
@@ -1290,17 +1284,17 @@ WebGLContext::DeleteShader(nsIWebGLShade
         return NS_OK;
 
     if (isNull || isDeleted)
         return NS_OK;
 
     MakeContextCurrent();
 
     gl->fDeleteShader(shadername);
-    shader->Delete();
+    shader->DeleteWhenNotAttached();
     mMapShaders.Remove(shadername);
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
 WebGLContext::DetachShader(nsIWebGLProgram *pobj, nsIWebGLShader *shobj)
 {
@@ -1319,16 +1313,18 @@ WebGLContext::DetachShader(nsIWebGLProgr
     // deleted shader, since it's still a shader
     if (!program->DetachShader(shader))
         return ErrorInvalidOperation("DetachShader: shader is not attached");
 
     MakeContextCurrent();
 
     gl->fDetachShader(progname, shadername);
 
+    shader->DetachedFromProgram();
+
     return NS_OK;
 }
 
 NS_IMETHODIMP
 WebGLContext::DepthFunc(WebGLenum func)
 {
     if (mContextLost)
         return NS_OK;
@@ -2631,16 +2627,22 @@ WebGLContext::GetProgramParameter(nsIWeb
         {
             GLint i = 0;
             gl->fGetProgramiv(progname, pname, &i);
             wrval->SetAsInt32(i);
         }
             break;
         case LOCAL_GL_DELETE_STATUS:
         case LOCAL_GL_LINK_STATUS:
+        {
+            GLint i = 0;
+            gl->fGetProgramiv(progname, pname, &i);
+            wrval->SetAsBool(bool(i));
+        }
+            break;
         case LOCAL_GL_VALIDATE_STATUS:
         {
             GLint i = 0;
 #ifdef XP_MACOSX
             // See comment in ValidateProgram below.
             i = 1;
 #else
             gl->fGetProgramiv(progname, pname, &i);
@@ -4324,23 +4326,22 @@ WebGLContext::UseProgram(nsIWebGLProgram
 
     MakeContextCurrent();
 
     if (prog && !prog->LinkStatus())
         return ErrorInvalidOperation("UseProgram: program was not linked successfully");
 
     gl->fUseProgram(progname);
 
-    if (mCurrentProgram && mCurrentProgram->HasDeletePending()) {
-        mCurrentProgram->DetachShaders();
-        mCurrentProgram->ClearDeletePending();
-    }
-
+    WebGLProgram* previous = mCurrentProgram;
     mCurrentProgram = prog;
 
+    if (previous)
+        previous->NoLongerCurrent();
+
     return NS_OK;
 }
 
 NS_IMETHODIMP
 WebGLContext::ValidateProgram(nsIWebGLProgram *pobj)
 {
     if (mContextLost)
         return NS_OK;
@@ -4566,17 +4567,16 @@ WebGLContext::GetShaderParameter(nsIWebG
         case LOCAL_GL_DELETE_STATUS:
         case LOCAL_GL_COMPILE_STATUS:
         {
             GLint i = 0;
             gl->fGetShaderiv(shadername, pname, &i);
             wrval->SetAsBool(bool(i));
         }
             break;
-
         default:
             return NS_ERROR_NOT_IMPLEMENTED;
     }
 
     *retval = wrval.forget().get();
 
     return NS_OK;
 }
--- a/content/canvas/test/webgl/failing_tests_mac.txt
+++ b/content/canvas/test/webgl/failing_tests_mac.txt
@@ -1,14 +1,13 @@
 conformance/context/premultiplyalpha-test.html
 conformance/glsl/misc/glsl-function-nodes.html
 conformance/glsl/misc/glsl-long-variable-names.html
 conformance/glsl/misc/shader-with-256-character-identifier.frag.html
 conformance/glsl/misc/shader-with-long-line.html
-conformance/programs/program-test.html
 conformance/textures/texture-mips.html
 conformance/textures/texture-npot.html
 conformance/more/conformance/quickCheckAPI-S_V.html
 conformance/more/functions/uniformfBadArgs.html
 conformance/more/functions/uniformiBadArgs.html
 conformance/glsl/misc/attrib-location-length-limits.html
 conformance/glsl/misc/uniform-location-length-limits.html
 conformance/renderbuffers/framebuffer-object-attachment.html