Fix OSX MakeCurrent and fix obj-del-behav test. draft
authorJeff Gilbert <jdashg@gmail.com>
Thu, 17 Dec 2015 16:16:54 -0800
changeset 316105 b0efabfe9df22655e40eac46cd6d31b325cfd3d5
parent 316104 4dcf3b290ac192c3354796e8213a812c2c38ebc0
child 316106 d56b16dd46e2198c26eb76c9b567497634e97db7
push id8514
push userjgilbert@mozilla.com
push dateFri, 18 Dec 2015 00:24:33 +0000
milestone45.0a1
Fix OSX MakeCurrent and fix obj-del-behav test.
dom/canvas/TexUnpackBlob.cpp
dom/canvas/test/webgl-conformance/conformance/misc/object-deletion-behaviour.html
dom/canvas/test/webgl-conformance/conformance/resources/webgl-test-utils.js
--- a/dom/canvas/TexUnpackBlob.cpp
+++ b/dom/canvas/TexUnpackBlob.cpp
@@ -268,16 +268,17 @@ TexUnpackSurface::OriginsForDOM(WebGLCon
 /*static*/ bool
 TexUnpackSurface::UploadDataSurface(bool isSubImage, WebGLContext* webgl,
                                     TexImageTarget target, GLint level,
                                     const webgl::DriverUnpackInfo* dui, GLint xOffset,
                                     GLint yOffset, GLint zOffset, GLsizei width,
                                     GLsizei height, gfx::DataSourceSurface* surf,
                                     bool isSurfAlphaPremult, GLenum* const out_glError)
 {
+    MOZ_ASSERT(webgl->gl->IsCurrent());
     *out_glError = 0;
 
     if (isSurfAlphaPremult != webgl->mPixelStore_PremultiplyAlpha)
         return false;
 
     gl::OriginPos srcOrigin, dstOrigin;
     OriginsForDOM(webgl, &srcOrigin, &dstOrigin);
     if (srcOrigin != dstOrigin)
@@ -346,18 +347,17 @@ TexUnpackSurface::UploadDataSurface(bool
     if (!GuessAlignment(map.GetData(), bytesPerRow, map.GetStride(), kMaxUnpackAlignment,
                         &unpackAlignment))
     {
         return false;
         // TODO: Consider using UNPACK_ settings to set the stride based on the too-large
         // alignment used for some SourceSurfaces. (D2D allegedy likes alignment=16)
     }
 
-    gl->MakeCurrent();
-
+    MOZ_ALWAYS_TRUE( webgl->gl->MakeCurrent() );
     ScopedUnpackReset scopedReset(webgl);
     gl->fPixelStorei(LOCAL_GL_UNPACK_ALIGNMENT, unpackAlignment);
 
     const GLsizei depth = 1;
     GLenum error = DoTexOrSubImage(isSubImage, gl, target.get(), level, chosenDUI,
                                    xOffset, yOffset, zOffset, width, height, depth,
                                    map.GetData());
     if (error) {
@@ -552,19 +552,18 @@ TexUnpackSurface::ConvertSurface(WebGLCo
 {
     *out_outOfMemory = false;
 
     const size_t width = surf->GetSize().width;
     const size_t height = surf->GetSize().height;
 
     // Source args:
 
-    MOZ_ASSERT(webgl->gl->IsCurrent());
+    // After we call this, on OSX, our GLContext will no longer be current.
     gfx::DataSourceSurface::ScopedMap srcMap(surf, gfx::DataSourceSurface::MapType::READ);
-    MOZ_ASSERT(webgl->gl->IsCurrent());
     if (!srcMap.IsMapped())
         return false;
 
     const void* const srcBegin = srcMap.GetData();
     const size_t srcStride = srcMap.GetStride();
 
     WebGLTexelFormat srcFormat;
     if (!GetFormatForSurf(surf, &srcFormat))
@@ -600,29 +599,27 @@ TexUnpackSurface::ConvertSurface(WebGLCo
     }
     void* const dstBegin = dstBuffer.get();
 
     gl::OriginPos srcOrigin, dstOrigin;
     OriginsForDOM(webgl, &srcOrigin, &dstOrigin);
 
     const bool dstPremultiplied = webgl->mPixelStore_PremultiplyAlpha;
 
-    MOZ_ASSERT(webgl->gl->IsCurrent());
     // And go!:
     if (!ConvertImage(width, height,
                       srcBegin, srcStride, srcOrigin, srcFormat, srcPremultiplied,
                       dstBegin, dstStride, dstOrigin, dstFormat, dstPremultiplied))
     {
         MOZ_ASSERT(false, "ConvertImage failed unexpectedly.");
         NS_ERROR("ConvertImage failed unexpectedly.");
         *out_outOfMemory = true;
         return false;
     }
 
-    MOZ_ASSERT(webgl->gl->IsCurrent());
     *out_convertedBuffer = Move(dstBuffer);
     *out_convertedAlignment = dstAlignment;
     return true;
 }
 
 
 ////////////////////
 
@@ -640,34 +637,29 @@ TexUnpackSurface::TexOrSubImage(bool isS
                                 GLint yOffset, GLint zOffset, GLenum* const out_glError)
 {
     *out_glError = 0;
 
     WebGLContext* webgl = tex->mContext;
 
     // TODO: Do blitting of the native SourceSurface.
 
-    MOZ_ASSERT(webgl->gl->IsCurrent());
+    // MakeCurrent is a big mess in here, because mapping (and presumably unmapping) on
+    // OSX can lose our MakeCurrent. Therefore it's easiest to MakeCurrent just before we
+    // call into GL, instead of trying to keep MakeCurrent-ed.
 
     RefPtr<gfx::DataSourceSurface> dataSurf = mSurf->GetDataSurface();
     MOZ_ASSERT(dataSurf);
 
-    webgl->gl->MakeCurrent(); // This seems to be necessary after GetDataSurface.
-
-    MOZ_ASSERT(webgl->gl->IsCurrent());
-
     GLenum error;
     if (UploadDataSurface(isSubImage, webgl, target, level, dui, xOffset, yOffset,
                           zOffset, mWidth, mHeight, dataSurf, mIsAlphaPremult, &error))
     {
         return;
     }
-
-    MOZ_ASSERT(webgl->gl->IsCurrent());
-
     if (error == LOCAL_GL_OUT_OF_MEMORY) {
         *out_glError = LOCAL_GL_OUT_OF_MEMORY;
         return;
     }
 
     // CPU conversion. (++numCopies)
 
     UniqueBuffer convertedBuffer;
@@ -681,18 +673,17 @@ TexUnpackSurface::TexOrSubImage(bool isS
             *out_glError = LOCAL_GL_OUT_OF_MEMORY;
         } else {
             NS_ERROR("Failed to convert surface.");
             *out_glError = LOCAL_GL_OUT_OF_MEMORY;
         }
         return;
     }
 
-    MOZ_ASSERT(webgl->gl->IsCurrent());
-
+    MOZ_ALWAYS_TRUE( webgl->gl->MakeCurrent() );
     ScopedUnpackReset scopedReset(webgl);
     webgl->gl->fPixelStorei(LOCAL_GL_UNPACK_ALIGNMENT, convertedAlignment);
 
     error = DoTexOrSubImage(isSubImage, webgl->gl, target.get(), level, dui, xOffset,
                             yOffset, zOffset, mWidth, mHeight, mDepth,
                             convertedBuffer.get());
     *out_glError = error;
 }
--- a/dom/canvas/test/webgl-conformance/conformance/misc/object-deletion-behaviour.html
+++ b/dom/canvas/test/webgl-conformance/conformance/misc/object-deletion-behaviour.html
@@ -71,17 +71,17 @@ shouldGenerateGLError(gl, gl.NO_ERROR, "
 shouldBe("gl.getParameter(gl.TEXTURE_BINDING_2D)", "tex");
 shouldGenerateGLError(gl, gl.NO_ERROR, "gl.framebufferTexture2D(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.TEXTURE_2D, tex, 0)");
 shouldBe("gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)", "tex");
 shouldBe("gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE)", "gl.TEXTURE");
 shouldGenerateGLError(gl, gl.NO_ERROR, "gl.deleteTexture(tex)");
 // Deleting a texture bound to the currently-bound fbo is the same as
 // detaching the textue from fbo first, then delete the texture.
 shouldBe("gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE)", "gl.NONE");
-shouldBe("gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)", "null");
+shouldGenerateGLError(gl, gl.INVALID_ENUM, "gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)", "null");
 shouldBeFalse("gl.isTexture(tex)");
 shouldBeNull("gl.getParameter(gl.TEXTURE_BINDING_2D)");
 shouldGenerateGLError(gl, gl.NO_ERROR, "gl.bindTexture(gl.TEXTURE_2D, tex)");
 shouldBeNull("gl.getParameter(gl.TEXTURE_BINDING_2D)");
 
 var texCubeMap = gl.createTexture();
 shouldBeNonNull("texCubeMap");
 shouldGenerateGLError(gl, gl.NO_ERROR, "gl.bindTexture(gl.TEXTURE_CUBE_MAP, texCubeMap)");
@@ -123,17 +123,17 @@ shouldBeNonNull("rbo3");
 shouldGenerateGLError(gl, gl.NO_ERROR, "gl.bindRenderbuffer(gl.RENDERBUFFER, rbo)");
 shouldBe("gl.getParameter(gl.RENDERBUFFER_BINDING)", "rbo");
 shouldGenerateGLError(gl, gl.NO_ERROR, "gl.framebufferRenderbuffer(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.RENDERBUFFER, rbo)");
 shouldBe("gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)", "rbo");
 shouldGenerateGLError(gl, gl.NO_ERROR, "gl.deleteRenderbuffer(rbo)");
 // Deleting a renderbuffer bound to the currently-bound fbo is the same as
 // detaching the renderbuffer from fbo first, then delete the renderbuffer.
 shouldBe("gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE)", "gl.NONE");
-shouldBe("gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)", "null");
+shouldGenerateGLError(gl, gl.INVALID_ENUM, "gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)", "null");
 shouldBeFalse("gl.isRenderbuffer(rbo)");
 shouldBeNull("gl.getParameter(gl.RENDERBUFFER_BINDING)");
 shouldGenerateGLError(gl, gl.NO_ERROR, "gl.bindRenderbuffer(gl.RENDERBUFFER, rbo)");
 shouldBeNull("gl.getParameter(gl.RENDERBUFFER_BINDING)");
 shouldGenerateGLError(gl, gl.NO_ERROR, "gl.bindRenderbuffer(gl.RENDERBUFFER, rbo2)");
 shouldBe("gl.getParameter(gl.RENDERBUFFER_BINDING)", "rbo2");
 shouldGenerateGLError(gl, gl.NO_ERROR, "gl.deleteRenderbuffer(rbo3)");
 shouldBe("gl.getParameter(gl.RENDERBUFFER_BINDING)", "rbo2");
@@ -257,17 +257,17 @@ if (gl.checkFramebufferStatus(gl.FRAMEBU
   shouldGenerateGLError(gl, gl.NO_ERROR, "gl.bindFramebuffer(gl.FRAMEBUFFER, fbo2)");
   shouldGenerateGLError(gl, gl.NO_ERROR, 'wtu.checkCanvasRect(gl, 0, 0, 16, 16, [0,255,0,255], "fbo should be green")');
   shouldGenerateGLError(gl, gl.NO_ERROR, "gl.clearColor(0,0,1,1)");
   shouldGenerateGLError(gl, gl.NO_ERROR, "gl.clear(gl.COLOR_BUFFER_BIT)");
   shouldGenerateGLError(gl, gl.NO_ERROR, 'wtu.checkCanvasRect(gl, 0, 0, 16, 16, [0,0,255,255], "fbo should be blue")');
   shouldBe("gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)", "rbo");
 
   shouldGenerateGLError(gl, gl.NO_ERROR, "gl.bindFramebuffer(gl.FRAMEBUFFER, fbo)");
-  shouldGenerateGLError(gl, gl.NO_ERROR, "gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)");
+  shouldGenerateGLError(gl, gl.INVALID_ENUM, "gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)");
   shouldGenerateGLError(gl, gl.NO_ERROR, "gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE)");
   shouldNotBe("gl.checkFramebufferStatus(gl.FRAMEBUFFER)", "gl.FRAMEBUFFER_COMPLETE");
   // Bind backbuffer.
   shouldGenerateGLError(gl, gl.NO_ERROR, "gl.bindFramebuffer(gl.FRAMEBUFFER, null)");
   shouldGenerateGLError(gl, gl.NO_ERROR, 'wtu.checkCanvasRect(gl, 0, 0, 16, 16, [255,0,0,255], "backbuffer should be red")');
 }
 
 debug("");
@@ -294,17 +294,17 @@ if (gl.checkFramebufferStatus(gl.FRAMEBU
   shouldBe("gl.checkFramebufferStatus(gl.FRAMEBUFFER)", "gl.FRAMEBUFFER_COMPLETE");
   shouldGenerateGLError(gl, gl.NO_ERROR, 'wtu.checkCanvasRect(gl, 0, 0, 1, 1, [0,255,0,255], "fbo should be green")');
   shouldGenerateGLError(gl, gl.NO_ERROR, "gl.clearColor(0,0,1,1)");
   shouldGenerateGLError(gl, gl.NO_ERROR, "gl.clear(gl.COLOR_BUFFER_BIT)");
   shouldGenerateGLError(gl, gl.NO_ERROR, 'wtu.checkCanvasRect(gl, 0, 0, 1, 1, [0,0,255,255], "fbo should be blue")');
   shouldBe("gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)", "tex");
 
   shouldGenerateGLError(gl, gl.NO_ERROR, "gl.bindFramebuffer(gl.FRAMEBUFFER, fbo)");
-  shouldGenerateGLError(gl, gl.NO_ERROR, "gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)");
+  shouldGenerateGLError(gl, gl.INVALID_ENUM, "gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)");
   shouldNotBe("gl.checkFramebufferStatus(gl.FRAMEBUFFER)", "gl.FRAMEBUFFER_COMPLETE");
   // Bind backbuffer.
   shouldGenerateGLError(gl, gl.NO_ERROR, "gl.bindFramebuffer(gl.FRAMEBUFFER, null)");
   shouldGenerateGLError(gl, gl.NO_ERROR, 'wtu.checkCanvasRect(gl, 0, 0, 16, 16, [255,0,0,255], "backbuffer should be red")');
 }
 
 debug("");
 debug("buffer deletion");
--- a/dom/canvas/test/webgl-conformance/conformance/resources/webgl-test-utils.js
+++ b/dom/canvas/test/webgl-conformance/conformance/resources/webgl-test-utils.js
@@ -684,16 +684,20 @@ var glErrorShouldBeIn = function(gl, exp
 
 /**
  * Tests that an evaluated expression generates a specific GL error.
  * @param {!WebGLContext} gl The WebGLContext to use.
  * @param {number} glError The expected gl error.
  * @param {string} evalSTr The string to evaluate.
  */
 var shouldGenerateGLError = function(gl, glError, evalStr) {
+  glErrorShouldBe(gl, 0,
+                  "Should not be pre-existing errors during call to"
+                  + " shouldGenerateGLError().");
+
   var exception;
   try {
     eval(evalStr);
   } catch (e) {
     exception = e;
   }
   if (exception) {
     testFailed(evalStr + " threw exception " + exception);
@@ -704,16 +708,20 @@ var shouldGenerateGLError = function(gl,
 
 /**
  * Tests that an evaluated expression generates a GL error from a list.
  * @param {!WebGLContext} gl The WebGLContext to use.
  * @param {!Array.<number>} expectedErrorList The list of expected gl errors.
  * @param {string} evalSTr The string to evaluate.
  */
 var shouldGenerateGLErrorIn = function(gl, expectedErrorList, evalStr) {
+  glErrorShouldBe(gl, 0,
+                  "Should not be pre-existing errors during call to"
+                  + " shouldGenerateGLErrorIn().");
+
   var exception;
   try {
     eval(evalStr);
   } catch (e) {
     exception = e;
   }
   if (exception) {
     testFailed(evalStr + " threw exception " + exception);