From 8e6c4c87cb5c1d8bb0d9f71660bebc4bda6b780d Mon Sep 17 00:00:00 2001 From: Doug Sherk Date: Thu, 3 Nov 2011 10:50:40 -0400 Subject: [PATCH] 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 | 63 ++++++++++++++++--- content/canvas/src/WebGLContextGL.cpp | 28 ++++----- .../canvas/test/webgl/failing_tests_mac.txt | 1 - 3 files changed, 69 insertions(+), 23 deletions(-) diff --git a/content/canvas/src/WebGLContext.h b/content/canvas/src/WebGLContext.h index 6ae016cd1f00..f905218f6074 100644 --- a/content/canvas/src/WebGLContext.h +++ b/content/canvas/src/WebGLContext.h @@ -733,6 +733,7 @@ public: friend class WebGLTexture; friend class WebGLFramebuffer; + friend class WebGLProgram; }; // this class is a mixin for the named type wrappers, and is used @@ -1421,21 +1422,43 @@ public: 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--; } @@ -1469,7 +1492,8 @@ protected: nsString mSource; nsCString mTranslationLog; bool mNeedsTranslation; - PRUint32 mAttachCount; + PRInt32 mAttachCount; + bool mDeletePending; }; NS_DEFINE_STATIC_IID_ACCESSOR(WebGLShader, WEBGLSHADER_PRIVATE_IID) @@ -1496,22 +1520,45 @@ public: 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; } @@ -1538,7 +1585,7 @@ public: // 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; diff --git a/content/canvas/src/WebGLContextGL.cpp b/content/canvas/src/WebGLContextGL.cpp index 8b120e791fcd..c33dbb6550c2 100644 --- a/content/canvas/src/WebGLContextGL.cpp +++ b/content/canvas/src/WebGLContextGL.cpp @@ -1265,13 +1265,7 @@ WebGLContext::DeleteProgram(nsIWebGLProgram *pobj) gl->fDeleteProgram(progname); - if (prog == mCurrentProgram) { - prog->SetDeletePending(); - } else { - prog->DetachShaders(); - } - - prog->Delete(); + prog->DeleteWhenNotCurrent(); mMapPrograms.Remove(progname); return NS_OK; @@ -1295,7 +1289,7 @@ WebGLContext::DeleteShader(nsIWebGLShader *sobj) MakeContextCurrent(); gl->fDeleteShader(shadername); - shader->Delete(); + shader->DeleteWhenNotAttached(); mMapShaders.Remove(shadername); return NS_OK; @@ -1324,6 +1318,8 @@ WebGLContext::DetachShader(nsIWebGLProgram *pobj, nsIWebGLShader *shobj) gl->fDetachShader(progname, shadername); + shader->DetachedFromProgram(); + return NS_OK; } @@ -2636,6 +2632,12 @@ WebGLContext::GetProgramParameter(nsIWebGLProgram *pobj, PRUint32 pname, nsIVari 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; @@ -4329,13 +4331,12 @@ WebGLContext::UseProgram(nsIWebGLProgram *pobj) gl->fUseProgram(progname); - if (mCurrentProgram && mCurrentProgram->HasDeletePending()) { - mCurrentProgram->DetachShaders(); - mCurrentProgram->ClearDeletePending(); - } - + WebGLProgram* previous = mCurrentProgram; mCurrentProgram = prog; + if (previous) + previous->NoLongerCurrent(); + return NS_OK; } @@ -4571,7 +4572,6 @@ WebGLContext::GetShaderParameter(nsIWebGLShader *sobj, WebGLenum pname, nsIVaria wrval->SetAsBool(bool(i)); } break; - default: return NS_ERROR_NOT_IMPLEMENTED; } diff --git a/content/canvas/test/webgl/failing_tests_mac.txt b/content/canvas/test/webgl/failing_tests_mac.txt index 0631e227eefb..921f938c72b8 100644 --- a/content/canvas/test/webgl/failing_tests_mac.txt +++ b/content/canvas/test/webgl/failing_tests_mac.txt @@ -3,7 +3,6 @@ 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