From 97f22e2838460e7d1ec1df299da7498669543050 Mon Sep 17 00:00:00 2001 From: Jeff Gilbert Date: Wed, 19 Oct 2011 15:09:57 -0400 Subject: [PATCH] Bug 681791 - Fixes GLContext::ResizeOffscreen leaving a mess on failure * * * try: -b do -p all -u all -t all --- content/canvas/src/WebGLContext.cpp | 14 +- .../canvas/test/webgl/failing_tests_linux.txt | 2 - .../canvas/test/webgl/failing_tests_mac.txt | 2 - .../test/webgl/failing_tests_windows.txt | 2 - gfx/thebes/GLContext.cpp | 160 +++++++++++------- gfx/thebes/GLContextProviderCGL.mm | 3 + gfx/thebes/GLContextProviderEGL.cpp | 3 + gfx/thebes/GLContextProviderWGL.cpp | 37 ++++ 8 files changed, 145 insertions(+), 78 deletions(-) diff --git a/content/canvas/src/WebGLContext.cpp b/content/canvas/src/WebGLContext.cpp index b381eb181df4..249d644ff179 100644 --- a/content/canvas/src/WebGLContext.cpp +++ b/content/canvas/src/WebGLContext.cpp @@ -527,14 +527,14 @@ WebGLContext::SetDimensions(PRInt32 width, PRInt32 height) height = 1; } - // If we already have a gl context, then we just need to resize - // FB0. - if (gl && - gl->ResizeOffscreen(gfxIntSize(width, height))) - { + // If we already have a gl context, then we just need to resize it + if (gl) { + gl->ResizeOffscreen(gfxIntSize(width, height)); // Doesn't matter if it succeeds (soft-fail) + // It's unlikely that we'll get a proper-sized context if we recreate if we didn't on resize + // everything's good, we're done here - mWidth = width; - mHeight = height; + mWidth = gl->OffscreenActualSize().width; + mHeight = gl->OffscreenActualSize().height; mResetLayer = true; return NS_OK; } diff --git a/content/canvas/test/webgl/failing_tests_linux.txt b/content/canvas/test/webgl/failing_tests_linux.txt index 0e3a11e44b02..4594b3760de5 100644 --- a/content/canvas/test/webgl/failing_tests_linux.txt +++ b/content/canvas/test/webgl/failing_tests_linux.txt @@ -1,5 +1,3 @@ -conformance/canvas/drawingbuffer-static-canvas-test.html -conformance/canvas/drawingbuffer-test.html conformance/context/premultiplyalpha-test.html conformance/glsl/misc/glsl-long-variable-names.html conformance/glsl/misc/shader-with-256-character-identifier.frag.html diff --git a/content/canvas/test/webgl/failing_tests_mac.txt b/content/canvas/test/webgl/failing_tests_mac.txt index 83fbd3f0ba21..b8d84f636952 100644 --- a/content/canvas/test/webgl/failing_tests_mac.txt +++ b/content/canvas/test/webgl/failing_tests_mac.txt @@ -1,6 +1,4 @@ conformance/canvas/buffer-preserve-test.html -conformance/canvas/drawingbuffer-static-canvas-test.html -conformance/canvas/drawingbuffer-test.html conformance/context/context-attributes-alpha-depth-stencil-antialias.html conformance/context/premultiplyalpha-test.html conformance/glsl/misc/glsl-function-nodes.html diff --git a/content/canvas/test/webgl/failing_tests_windows.txt b/content/canvas/test/webgl/failing_tests_windows.txt index 81d0e98729ff..863b9fa2e49e 100644 --- a/content/canvas/test/webgl/failing_tests_windows.txt +++ b/content/canvas/test/webgl/failing_tests_windows.txt @@ -1,6 +1,4 @@ conformance/canvas/buffer-preserve-test.html -conformance/canvas/drawingbuffer-static-canvas-test.html -conformance/canvas/drawingbuffer-test.html conformance/context/premultiplyalpha-test.html conformance/glsl/functions/glsl-function-atan.html conformance/glsl/functions/glsl-function-atan-xy.html diff --git a/gfx/thebes/GLContext.cpp b/gfx/thebes/GLContext.cpp index fa6e67090ca6..c6112ab45eb8 100644 --- a/gfx/thebes/GLContext.cpp +++ b/gfx/thebes/GLContext.cpp @@ -993,11 +993,11 @@ GLContext::ResizeOffscreenFBO(const gfxIntSize& aSize) MakeCurrent(); - bool alpha = mCreationFormat.alpha > 0; - int depth = mCreationFormat.depth; - int stencil = mCreationFormat.stencil; + const bool alpha = mCreationFormat.alpha > 0; + const int depth = mCreationFormat.depth; + const int stencil = mCreationFormat.stencil; - bool firstTime = (mOffscreenFBO == 0); + const bool firstTime = (mOffscreenFBO == 0); GLuint curBoundTexture = 0; GLuint curBoundRenderbuffer = 0; @@ -1005,44 +1005,44 @@ GLContext::ResizeOffscreenFBO(const gfxIntSize& aSize) GLint viewport[4]; - bool useDepthStencil = - !mIsGLES2 || IsExtensionSupported(OES_packed_depth_stencil); + const bool useDepthStencil = + !mIsGLES2 || IsExtensionSupported(OES_packed_depth_stencil); // save a few things for later restoring - fGetIntegerv(LOCAL_GL_TEXTURE_BINDING_2D, (GLint*) &curBoundTexture); fGetIntegerv(LOCAL_GL_FRAMEBUFFER_BINDING, (GLint*) &curBoundFramebuffer); + fGetIntegerv(LOCAL_GL_TEXTURE_BINDING_2D, (GLint*) &curBoundTexture); fGetIntegerv(LOCAL_GL_RENDERBUFFER_BINDING, (GLint*) &curBoundRenderbuffer); fGetIntegerv(LOCAL_GL_VIEWPORT, viewport); - // the context format of what we're defining; - // for some reason, UpdateActualFormat isn't working with a bound FBO. + // the context format of what we're defining + // This becomes mActualFormat on success ContextFormat cf; - // If this is the first time we're going through this, we need - // to create the objects we'll use. Otherwise, just bind them. - if (firstTime) { - fGenTextures(1, &mOffscreenTexture); - fBindTexture(LOCAL_GL_TEXTURE_2D, mOffscreenTexture); - fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_MIN_FILTER, LOCAL_GL_LINEAR); - fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_MAG_FILTER, LOCAL_GL_LINEAR); + // Create everything we need for the resize, so if it fails, we haven't broken anything + // If successful, these new resized objects will replace their associated member vars in GLContext + GLuint newOffscreenFBO = 0; + GLuint newOffscreenTexture = 0; + GLuint newOffscreenDepthRB = 0; + GLuint newOffscreenStencilRB = 0; - fGenFramebuffers(1, &mOffscreenFBO); - fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, mOffscreenFBO); + fGenFramebuffers(1, &newOffscreenFBO); + fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, newOffscreenFBO); - if (depth && stencil && useDepthStencil) { - fGenRenderbuffers(1, &mOffscreenDepthRB); - } else { - if (depth) { - fGenRenderbuffers(1, &mOffscreenDepthRB); - } + fGenTextures(1, &newOffscreenTexture); + fBindTexture(LOCAL_GL_TEXTURE_2D, newOffscreenTexture); + fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_MIN_FILTER, LOCAL_GL_LINEAR); + fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_MAG_FILTER, LOCAL_GL_LINEAR); - if (stencil) { - fGenRenderbuffers(1, &mOffscreenStencilRB); - } - } + if (depth && stencil && useDepthStencil) { + fGenRenderbuffers(1, &newOffscreenDepthRB); } else { - fBindTexture(LOCAL_GL_TEXTURE_2D, mOffscreenTexture); - fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, mOffscreenFBO); + if (depth) { + fGenRenderbuffers(1, &newOffscreenDepthRB); + } + + if (stencil) { + fGenRenderbuffers(1, &newOffscreenStencilRB); + } } // resize the FBO components @@ -1083,7 +1083,7 @@ GLContext::ResizeOffscreenFBO(const gfxIntSize& aSize) } if (depth && stencil && useDepthStencil) { - fBindRenderbuffer(LOCAL_GL_RENDERBUFFER, mOffscreenDepthRB); + fBindRenderbuffer(LOCAL_GL_RENDERBUFFER, newOffscreenDepthRB); fRenderbufferStorage(LOCAL_GL_RENDERBUFFER, LOCAL_GL_DEPTH24_STENCIL8, aSize.width, aSize.height); @@ -1108,13 +1108,13 @@ GLContext::ResizeOffscreenFBO(const gfxIntSize& aSize) cf.depth = 24; } - fBindRenderbuffer(LOCAL_GL_RENDERBUFFER, mOffscreenDepthRB); + fBindRenderbuffer(LOCAL_GL_RENDERBUFFER, newOffscreenDepthRB); fRenderbufferStorage(LOCAL_GL_RENDERBUFFER, depthType, aSize.width, aSize.height); } if (stencil) { - fBindRenderbuffer(LOCAL_GL_RENDERBUFFER, mOffscreenStencilRB); + fBindRenderbuffer(LOCAL_GL_RENDERBUFFER, newOffscreenStencilRB); fRenderbufferStorage(LOCAL_GL_RENDERBUFFER, LOCAL_GL_STENCIL_INDEX8, aSize.width, aSize.height); @@ -1122,38 +1122,35 @@ GLContext::ResizeOffscreenFBO(const gfxIntSize& aSize) } } - // Now assemble the FBO, if we're creating one - // for the first time. - if (firstTime) { - fFramebufferTexture2D(LOCAL_GL_FRAMEBUFFER, - LOCAL_GL_COLOR_ATTACHMENT0, - LOCAL_GL_TEXTURE_2D, - mOffscreenTexture, - 0); + // Now assemble the FBO + fFramebufferTexture2D(LOCAL_GL_FRAMEBUFFER, + LOCAL_GL_COLOR_ATTACHMENT0, + LOCAL_GL_TEXTURE_2D, + newOffscreenTexture, + 0); - if (depth && stencil && useDepthStencil) { + if (depth && stencil && useDepthStencil) { + fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, + LOCAL_GL_DEPTH_ATTACHMENT, + LOCAL_GL_RENDERBUFFER, + newOffscreenDepthRB); + fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, + LOCAL_GL_STENCIL_ATTACHMENT, + LOCAL_GL_RENDERBUFFER, + newOffscreenDepthRB); + } else { + if (depth) { fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, LOCAL_GL_DEPTH_ATTACHMENT, LOCAL_GL_RENDERBUFFER, - mOffscreenDepthRB); + newOffscreenDepthRB); + } + + if (stencil) { fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, LOCAL_GL_STENCIL_ATTACHMENT, LOCAL_GL_RENDERBUFFER, - mOffscreenDepthRB); - } else { - if (depth) { - fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, - LOCAL_GL_DEPTH_ATTACHMENT, - LOCAL_GL_RENDERBUFFER, - mOffscreenDepthRB); - } - - if (stencil) { - fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, - LOCAL_GL_STENCIL_ATTACHMENT, - LOCAL_GL_RENDERBUFFER, - mOffscreenStencilRB); - } + newOffscreenStencilRB); } } @@ -1161,24 +1158,56 @@ GLContext::ResizeOffscreenFBO(const gfxIntSize& aSize) GLenum status = fCheckFramebufferStatus(LOCAL_GL_FRAMEBUFFER); if (status != LOCAL_GL_FRAMEBUFFER_COMPLETE) { NS_WARNING("Error resizing offscreen framebuffer -- framebuffer not complete"); + + // Clean up the mess + fDeleteFramebuffers(1, &newOffscreenFBO); + fDeleteTextures(1, &newOffscreenTexture); + fDeleteRenderbuffers(1, &newOffscreenDepthRB); + fDeleteRenderbuffers(1, &newOffscreenStencilRB); + + fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, curBoundFramebuffer); + fBindTexture(LOCAL_GL_TEXTURE_2D, curBoundTexture); + fBindRenderbuffer(LOCAL_GL_RENDERBUFFER, curBoundRenderbuffer); + fViewport(viewport[0], viewport[1], viewport[2], viewport[3]); + return false; } + // Success, so delete the old and busted + fDeleteFramebuffers(1, &mOffscreenFBO); + fDeleteTextures(1, &mOffscreenTexture); + fDeleteRenderbuffers(1, &mOffscreenDepthRB); + fDeleteRenderbuffers(1, &mOffscreenStencilRB); + + // Update currently bound references if we're changing what they were point to + // This way we don't rebind to old buffers when we're done here + if (curBoundFramebuffer == mOffscreenFBO) + curBoundFramebuffer = newOffscreenFBO; + if (curBoundTexture == mOffscreenTexture) + curBoundTexture = newOffscreenTexture; + if (curBoundRenderbuffer == mOffscreenDepthRB) + curBoundRenderbuffer = newOffscreenDepthRB; + else if (curBoundRenderbuffer == mOffscreenStencilRB) + curBoundRenderbuffer = newOffscreenStencilRB; + + // Replace with the new hotness + mOffscreenFBO = newOffscreenFBO; + mOffscreenTexture = newOffscreenTexture; + mOffscreenDepthRB = newOffscreenDepthRB; + mOffscreenStencilRB = newOffscreenStencilRB; + mOffscreenSize = aSize; mOffscreenActualSize = aSize; - if (firstTime) { - // UpdateActualFormat() doesn't work for some reason, with a - // FBO bound, even though it should. - //UpdateActualFormat(); - mActualFormat = cf; + mActualFormat = cf; #ifdef DEBUG + if (mDebugMode) { printf_stderr("Created offscreen FBO: r: %d g: %d b: %d a: %d depth: %d stencil: %d\n", mActualFormat.red, mActualFormat.green, mActualFormat.blue, mActualFormat.alpha, mActualFormat.depth, mActualFormat.stencil); -#endif } +#endif // We're good, and the framebuffer is already attached, so let's // clear out our new framebuffer; otherwise we'll end up displaying @@ -1187,12 +1216,13 @@ GLContext::ResizeOffscreenFBO(const gfxIntSize& aSize) fViewport(0, 0, aSize.width, aSize.height); // Clear the new framebuffer with the full viewport + fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, mOffscreenFBO); ClearSafely(); // Ok, now restore the GL state back to what it was before the resize took place. + fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, curBoundFramebuffer); fBindTexture(LOCAL_GL_TEXTURE_2D, curBoundTexture); fBindRenderbuffer(LOCAL_GL_RENDERBUFFER, curBoundRenderbuffer); - fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, curBoundFramebuffer); // -don't- restore the viewport the first time through this, since // the previous one isn't valid. diff --git a/gfx/thebes/GLContextProviderCGL.mm b/gfx/thebes/GLContextProviderCGL.mm index 61ae7c143d88..00526d4a6fdd 100644 --- a/gfx/thebes/GLContextProviderCGL.mm +++ b/gfx/thebes/GLContextProviderCGL.mm @@ -264,6 +264,9 @@ GLContextCGL::UnbindTex2DOffscreen(GLContext *aOffscreen) bool GLContextCGL::ResizeOffscreen(const gfxIntSize& aNewSize) { + if (!IsOffscreenSizeAllowed(aNewSize)) + return false; + if (mPBuffer) { NSOpenGLPixelBuffer *pb = [[NSOpenGLPixelBuffer alloc] initWithTextureTarget:LOCAL_GL_TEXTURE_2D diff --git a/gfx/thebes/GLContextProviderEGL.cpp b/gfx/thebes/GLContextProviderEGL.cpp index 7d9a095db7be..06a5f32c1711 100644 --- a/gfx/thebes/GLContextProviderEGL.cpp +++ b/gfx/thebes/GLContextProviderEGL.cpp @@ -1054,6 +1054,9 @@ GLContextEGL::UnbindTex2DOffscreen(GLContext *aOffscreen) bool GLContextEGL::ResizeOffscreen(const gfxIntSize& aNewSize) { + if (!IsOffscreenSizeAllowed(aNewSize)) + return false; + if (mIsPBuffer) { gfxIntSize pbsize(aNewSize); diff --git a/gfx/thebes/GLContextProviderWGL.cpp b/gfx/thebes/GLContextProviderWGL.cpp index 768cbd0df672..c156676e2596 100644 --- a/gfx/thebes/GLContextProviderWGL.cpp +++ b/gfx/thebes/GLContextProviderWGL.cpp @@ -400,10 +400,44 @@ GLContextWGL::UnbindTex2DOffscreen(GLContext *aOffscreen) } } + +static bool +GetMaxSize(HDC hDC, int format, gfxIntSize& size) +{ + int query[] = {LOCAL_WGL_MAX_PBUFFER_WIDTH_ARB, LOCAL_WGL_MAX_PBUFFER_HEIGHT_ARB}; + int result[2]; + + // (HDC hdc, int iPixelFormat, int iLayerPlane, UINT nAttributes, int* piAttributes, int *piValues) + if (!sWGLLibrary.fGetPixelFormatAttribiv(hDC, format, 0, 2, query, result)) + return false; + + size.width = result[0]; + size.height = result[1]; + return true; +} + +static bool +IsValidSizeForFormat(HDC hDC, int format, const gfxIntSize& requested) +{ + gfxIntSize max; + if (!GetMaxSize(hDC, format, max)) + return true; + + if (requested.width > max.width) + return false; + if (requested.height > max.height) + return false; + + return true; +} + bool GLContextWGL::ResizeOffscreen(const gfxIntSize& aNewSize) { if (mPBuffer) { + if (!IsValidSizeForFormat(gSharedWindowDC, mPixelFormat, aNewSize)) + return false; + int pbattrs[] = { LOCAL_WGL_TEXTURE_FORMAT_ARB, mCreationFormat.alpha > 0 ? LOCAL_WGL_TEXTURE_RGBA_ARB @@ -545,6 +579,9 @@ CreatePBufferOffscreenContext(const gfxIntSize& aSize, // XXX add back the priority choosing code here int chosenFormat = formats[0]; + if (!IsValidSizeForFormat(gSharedWindowDC, chosenFormat, aSize)) + return nsnull; + HANDLE pbuffer = sWGLLibrary.fCreatePbuffer(gSharedWindowDC, chosenFormat, aSize.width, aSize.height, pbattrs.Elements());