From fd332a0ae8917ccb19bf2a1dc2d5f72845936d1b Mon Sep 17 00:00:00 2001 From: Doug Sherk Date: Thu, 1 Sep 2011 15:28:34 -0400 Subject: [PATCH] Bug 681835: fixed a series of object deletion conformance issues r=bjacob The bindX() commands were erroring with INVALID_VALUE when they're instead supposed to simply fail silently when they're given a deleted object. Additionally, the getParameter() function was failing after its associated variable was deleted, sometimes returning values when it should return null. * * * Bug 681835: WebGL fix for previous patch which introduced a bug with deletion DeleteRenderbuffer and DeleteFramebuffer weren't checking if the deleted buffer was the currently bound buffer before deleting them. This patch implements this functionality. A separate test case patch was also submitted to Khronos: http://www.khronos.org/bugzilla/show_bug.cgi?id=518 --- content/canvas/src/WebGLContextGL.cpp | 47 ++++++++++++++----- .../canvas/test/webgl/failing_tests_linux.txt | 1 - .../canvas/test/webgl/failing_tests_mac.txt | 1 - .../test/webgl/failing_tests_windows.txt | 1 - 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/content/canvas/src/WebGLContextGL.cpp b/content/canvas/src/WebGLContextGL.cpp index 055d80bbab2..f50db07fe5d 100644 --- a/content/canvas/src/WebGLContextGL.cpp +++ b/content/canvas/src/WebGLContextGL.cpp @@ -201,8 +201,13 @@ WebGLContext::BindBuffer(WebGLenum target, nsIWebGLBuffer *bobj) { WebGLuint bufname; WebGLBuffer* buf; - PRBool isNull; - if (!GetConcreteObjectAndGLName("bindBuffer", bobj, &buf, &bufname, &isNull)) + PRBool isNull; // allow null objects + PRBool isDeleted; // allow deleted objects + if (!GetConcreteObjectAndGLName("bindBuffer", bobj, &buf, &bufname, &isNull, &isDeleted)) + return NS_OK; + + // silently ignore a deleted buffer + if (isDeleted) return NS_OK; if (target != LOCAL_GL_ARRAY_BUFFER && @@ -237,13 +242,18 @@ NS_IMETHODIMP WebGLContext::BindFramebuffer(WebGLenum target, nsIWebGLFramebuffer *fbobj) { WebGLuint framebuffername; - PRBool isNull; + PRBool isNull; // allow null objects + PRBool isDeleted; // allow deleted objects WebGLFramebuffer *wfb; if (target != LOCAL_GL_FRAMEBUFFER) return ErrorInvalidEnum("BindFramebuffer: target must be GL_FRAMEBUFFER"); - if (!GetConcreteObjectAndGLName("bindFramebuffer", fbobj, &wfb, &framebuffername, &isNull)) + if (!GetConcreteObjectAndGLName("bindFramebuffer", fbobj, &wfb, &framebuffername, &isNull, &isDeleted)) + return NS_OK; + + // silently ignore a deleted frame buffer + if (isDeleted) return NS_OK; MakeContextCurrent(); @@ -264,13 +274,18 @@ NS_IMETHODIMP WebGLContext::BindRenderbuffer(WebGLenum target, nsIWebGLRenderbuffer *rbobj) { WebGLuint renderbuffername; - PRBool isNull; + PRBool isNull; // allow null objects + PRBool isDeleted; // allow deleted objects WebGLRenderbuffer *wrb; if (target != LOCAL_GL_RENDERBUFFER) return ErrorInvalidEnumInfo("bindRenderbuffer: target", target); - if (!GetConcreteObjectAndGLName("bindRenderBuffer", rbobj, &wrb, &renderbuffername, &isNull)) + if (!GetConcreteObjectAndGLName("bindRenderBuffer", rbobj, &wrb, &renderbuffername, &isNull, &isDeleted)) + return NS_OK; + + // silently ignore a deleted buffer + if (isDeleted) return NS_OK; if (!isNull) @@ -290,8 +305,13 @@ WebGLContext::BindTexture(WebGLenum target, nsIWebGLTexture *tobj) { WebGLuint texturename; WebGLTexture *tex; - PRBool isNull; // allow null object - if (!GetConcreteObjectAndGLName("bindTexture", tobj, &tex, &texturename, &isNull)) + PRBool isNull; // allow null objects + PRBool isDeleted; // allow deleted objects + if (!GetConcreteObjectAndGLName("bindTexture", tobj, &tex, &texturename, &isNull, &isDeleted)) + return NS_OK; + + // silently ignore a deleted texture + if (isDeleted) return NS_OK; if (target == LOCAL_GL_TEXTURE_2D) { @@ -1063,6 +1083,9 @@ WebGLContext::DeleteFramebuffer(nsIWebGLFramebuffer *fbobj) fbuf->Delete(); mMapFramebuffers.Remove(fbufname); + if (mBoundFramebuffer && mBoundFramebuffer->GLName() == fbufname) + mBoundFramebuffer = NULL; + return NS_OK; } @@ -1093,10 +1116,12 @@ WebGLContext::DeleteRenderbuffer(nsIWebGLRenderbuffer *rbobj) */ gl->fDeleteRenderbuffers(1, &rbufname); - rbuf->Delete(); mMapRenderbuffers.Remove(rbufname); + if (mBoundRenderbuffer && mBoundRenderbuffer->GLName() == rbufname) + mBoundRenderbuffer = NULL; + return NS_OK; } @@ -2197,10 +2222,6 @@ WebGLContext::GetFramebufferAttachmentParameter(WebGLenum target, WebGLenum atta wrval->SetAsInt32(LOCAL_GL_NONE); break; - case LOCAL_GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME: - wrval->SetAsEmpty(); - break; - default: return ErrorInvalidEnumInfo("GetFramebufferAttachmentParameter: pname", pname); } diff --git a/content/canvas/test/webgl/failing_tests_linux.txt b/content/canvas/test/webgl/failing_tests_linux.txt index e7ee165a0ec..0b66451af59 100644 --- a/content/canvas/test/webgl/failing_tests_linux.txt +++ b/content/canvas/test/webgl/failing_tests_linux.txt @@ -12,7 +12,6 @@ conformance/gl-getshadersource.html conformance/gl-uniform-bool.html conformance/glsl-conformance.html conformance/glsl-long-variable-names.html -conformance/object-deletion-behaviour.html conformance/premultiplyalpha-test.html conformance/read-pixels-test.html conformance/uninitialized-test.html diff --git a/content/canvas/test/webgl/failing_tests_mac.txt b/content/canvas/test/webgl/failing_tests_mac.txt index b2285670ed2..b38e4fee694 100644 --- a/content/canvas/test/webgl/failing_tests_mac.txt +++ b/content/canvas/test/webgl/failing_tests_mac.txt @@ -6,7 +6,6 @@ conformance/gl-getshadersource.html conformance/gl-object-get-calls.html conformance/glsl-conformance.html conformance/glsl-long-variable-names.html -conformance/object-deletion-behaviour.html conformance/premultiplyalpha-test.html conformance/program-test.html conformance/read-pixels-test.html diff --git a/content/canvas/test/webgl/failing_tests_windows.txt b/content/canvas/test/webgl/failing_tests_windows.txt index fb8c2fd1a73..6916882fcb3 100644 --- a/content/canvas/test/webgl/failing_tests_windows.txt +++ b/content/canvas/test/webgl/failing_tests_windows.txt @@ -4,7 +4,6 @@ conformance/framebuffer-object-attachment.html conformance/gl-getshadersource.html conformance/glsl-conformance.html conformance/glsl-long-variable-names.html -conformance/object-deletion-behaviour.html conformance/premultiplyalpha-test.html conformance/read-pixels-test.html conformance/more/conformance/quickCheckAPI.html