Bug 697560 - Generate WebGL errors in readPixels according to spec - r=bjacob
authorJeff Gilbert <jgilbert@mozilla.com>
Mon, 31 Oct 2011 16:55:01 -0700
changeset 79485 022e7d3042cb249edd30777d233878aefd4a2237
parent 79484 2cd25d4a839b2ebff011abbada4dddff3f71672b
child 79486 c4f4dc3d3b204a91ce86a66cf13f61648348f596
push id21408
push userkhuey@mozilla.com
push dateTue, 01 Nov 2011 14:32:20 +0000
treeherdermozilla-central@cd9add22f090 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbjacob
bugs697560
milestone10.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 697560 - Generate WebGL errors in readPixels according to spec - r=bjacob * * * Bug 697560 - Disallow conformance failures on read-pixels-test - r=bjacob
content/canvas/src/WebGLContext.h
content/canvas/src/WebGLContextGL.cpp
content/canvas/test/webgl/failing_tests_linux.txt
content/canvas/test/webgl/failing_tests_mac.txt
content/canvas/test/webgl/failing_tests_windows.txt
--- a/content/canvas/src/WebGLContext.h
+++ b/content/canvas/src/WebGLContext.h
@@ -575,17 +575,17 @@ protected:
     nsresult TexSubImage2D_base(WebGLenum target, WebGLint level,
                                 WebGLint xoffset, WebGLint yoffset,
                                 WebGLsizei width, WebGLsizei height, WebGLsizei srcStrideOrZero,
                                 WebGLenum format, WebGLenum type,
                                 void *pixels, PRUint32 byteLength,
                                 int jsArrayType,
                                 int srcFormat, bool srcPremultiplied);
     nsresult ReadPixels_base(WebGLint x, WebGLint y, WebGLsizei width, WebGLsizei height,
-                             WebGLenum format, WebGLenum type, void *data, PRUint32 byteLength);
+                             WebGLenum format, WebGLenum type, JSObject* pixels);
     nsresult TexParameter_base(WebGLenum target, WebGLenum pname,
                                WebGLint *intParamPtr, WebGLfloat *floatParamPtr);
 
     void ConvertImage(size_t width, size_t height, size_t srcStride, size_t dstStride,
                       const PRUint8*src, PRUint8 *dst,
                       int srcFormat, bool srcPremultiplied,
                       int dstFormat, bool dstPremultiplied,
                       size_t dstTexelSize);
--- a/content/canvas/src/WebGLContextGL.cpp
+++ b/content/canvas/src/WebGLContextGL.cpp
@@ -3335,98 +3335,134 @@ WebGLContext::ReadPixels(PRInt32)
     if (mContextLost)
         return NS_OK;
 
     return NS_ERROR_FAILURE;
 }
 
 nsresult
 WebGLContext::ReadPixels_base(WebGLint x, WebGLint y, WebGLsizei width, WebGLsizei height,
-                              WebGLenum format, WebGLenum type, void *data, PRUint32 byteLength)
+                              WebGLenum format, WebGLenum type, JSObject* pixels)
 {
     if (HTMLCanvasElement()->IsWriteOnly() && !nsContentUtils::IsCallerTrustedForRead()) {
         LogMessageIfVerbose("ReadPixels: Not allowed");
         return NS_ERROR_DOM_SECURITY_ERR;
     }
 
     if (width < 0 || height < 0)
         return ErrorInvalidValue("ReadPixels: negative size passed");
 
-    // there's nothing to do in this case, since we won't read any pixels
-    if (width == 0 || height == 0)
-        return NS_OK;
+    if (!pixels)
+        return ErrorInvalidValue("ReadPixels: null array passed");
 
     WebGLsizei boundWidth = mBoundFramebuffer ? mBoundFramebuffer->width() : mWidth;
     WebGLsizei boundHeight = mBoundFramebuffer ? mBoundFramebuffer->height() : mHeight;
 
-    PRUint32 size = 0;
-    bool badFormat = false, badType = false;
+    void* data = JS_GetTypedArrayData(pixels);
+    PRUint32 dataByteLen = JS_GetTypedArrayByteLength(pixels);
+    int dataType = JS_GetTypedArrayType(pixels);
+
+    PRUint32 channels = 0;
+
+    // Check the format param
     switch (format) {
-    case LOCAL_GL_RGBA:
-        size = 4;
-        break;
-    default:
-        badFormat = true;
-        break;
+        case LOCAL_GL_ALPHA:
+            channels = 1;
+            break;
+        case LOCAL_GL_RGB:
+            channels = 3;
+            break;
+        case LOCAL_GL_RGBA:
+            channels = 4;
+            break;
+        default:
+            return ErrorInvalidEnum("readPixels: Bad format");
     }
 
+    PRUint32 bytesPerPixel = 0;
+    int requiredDataType = 0;
+
+    // Check the type param
     switch (type) {
-    case LOCAL_GL_UNSIGNED_BYTE:
-        break;
-    default:
-        badType = true;
-        break;
+        case LOCAL_GL_UNSIGNED_BYTE:
+            bytesPerPixel = 1 * channels;
+            requiredDataType = js::TypedArray::TYPE_UINT8;
+            break;
+        case LOCAL_GL_UNSIGNED_SHORT_4_4_4_4:
+        case LOCAL_GL_UNSIGNED_SHORT_5_5_5_1:
+        case LOCAL_GL_UNSIGNED_SHORT_5_6_5:
+            bytesPerPixel = 2;
+            requiredDataType = js::TypedArray::TYPE_UINT16;
+            break;
+        default:
+            return ErrorInvalidEnum("readPixels: Bad type");
     }
 
-    if (badFormat && badType)
-        return ErrorInvalidOperation("readPixels: bad format and type");
-    if (badFormat)
-        return ErrorInvalidEnumInfo("readPixels: format", format);
-    if (badType)
-        return ErrorInvalidEnumInfo("ReadPixels: type", type);
-
+    // Check the pixels param type
+    if (dataType != requiredDataType)
+        return ErrorInvalidOperation("readPixels: Mismatched type/pixels types");
+
+    // Check the pixels param size
     CheckedUint32 checked_neededByteLength =
-        GetImageSize(height, width, size, mPixelStorePackAlignment);
-
-    CheckedUint32 checked_plainRowSize = CheckedUint32(width) * size;
-
-    CheckedUint32 checked_alignedRowSize = 
+        GetImageSize(height, width, bytesPerPixel, mPixelStorePackAlignment);
+
+    CheckedUint32 checked_plainRowSize = CheckedUint32(width) * bytesPerPixel;
+
+    CheckedUint32 checked_alignedRowSize =
         RoundedToNextMultipleOf(checked_plainRowSize, mPixelStorePackAlignment);
 
     if (!checked_neededByteLength.valid())
         return ErrorInvalidOperation("ReadPixels: integer overflow computing the needed buffer size");
 
-    if (checked_neededByteLength.value() > byteLength)
+    if (checked_neededByteLength.value() > dataByteLen)
         return ErrorInvalidOperation("ReadPixels: buffer too small");
 
+    // Check the format and type params to assure they are an acceptable pair (as per spec)
+    switch (format) {
+        case LOCAL_GL_RGBA: {
+            switch (type) {
+                case LOCAL_GL_UNSIGNED_BYTE:
+                    break;
+                default:
+                    return ErrorInvalidOperation("readPixels: Invalid format/type pair");
+            }
+            break;
+        }
+        default:
+            return ErrorInvalidOperation("readPixels: Invalid format/type pair");
+    }
+
+    // there's nothing to do in this case, since we won't read any pixels
+    if (width == 0 || height == 0)
+        return NS_OK;
+
     MakeContextCurrent();
 
     if (mBoundFramebuffer) {
         // prevent readback of arbitrary video memory through uninitialized renderbuffers!
         if (!mBoundFramebuffer->CheckAndInitializeRenderbuffers())
             return NS_OK;
     } else {
         EnsureBackbufferClearedAsNeeded();
     }
 
-
     if (CanvasUtils::CheckSaneSubrectSize(x, y, width, height, boundWidth, boundHeight)) {
         // the easy case: we're not reading out-of-range pixels
         gl->fReadPixels(x, y, width, height, format, type, data);
     } else {
         // the rectangle doesn't fit entirely in the bound buffer. We then have to set to zero the part
         // of the buffer that correspond to out-of-range pixels. We don't want to rely on system OpenGL
         // to do that for us, because passing out of range parameters to a buggy OpenGL implementation
         // could conceivably allow to read memory we shouldn't be allowed to read. So we manually initialize
         // the buffer to zero and compute the parameters to pass to OpenGL. We have to use an intermediate buffer
         // to accomodate the potentially different strides (widths).
 
         // zero the whole destination buffer. Too bad for the part that's going to be overwritten, we're not
         // 100% efficient here, but in practice this is a quite rare case anyway.
-        memset(data, 0, byteLength);
+        memset(data, 0, dataByteLen);
 
         if (   x >= boundWidth
             || x+width <= 0
             || y >= boundHeight
             || y+height <= 0)
         {
             // we are completely outside of range, can exit now with buffer filled with zeros
             return NS_OK;
@@ -3444,33 +3480,33 @@ WebGLContext::ReadPixels_base(WebGLint x
         if (subrect_width < 0 || subrect_height < 0 ||
             subrect_width > width || subrect_height > height)
             return ErrorInvalidOperation("ReadPixels: integer overflow computing clipped rect size");
 
         // now we know that subrect_width is in the [0..width] interval, and same for heights.
 
         // now, same computation as above to find the size of the intermediate buffer to allocate for the subrect
         // no need to check again for integer overflow here, since we already know the sizes aren't greater than before
-        PRUint32 subrect_plainRowSize = subrect_width * size;
+        PRUint32 subrect_plainRowSize = subrect_width * bytesPerPixel;
 	// There are checks above to ensure that this doesn't overflow.
         PRUint32 subrect_alignedRowSize = 
             RoundedToNextMultipleOf(subrect_plainRowSize, mPixelStorePackAlignment).value();
         PRUint32 subrect_byteLength = (subrect_height-1)*subrect_alignedRowSize + subrect_plainRowSize;
 
         // create subrect buffer, call glReadPixels, copy pixels into destination buffer, delete subrect buffer
         GLubyte *subrect_data = new GLubyte[subrect_byteLength];
         gl->fReadPixels(subrect_x, subrect_y, subrect_width, subrect_height, format, type, subrect_data);
 
         // notice that this for loop terminates because we already checked that subrect_height is at most height
         for (GLint y_inside_subrect = 0; y_inside_subrect < subrect_height; ++y_inside_subrect) {
             GLint subrect_x_in_dest_buffer = subrect_x - x;
             GLint subrect_y_in_dest_buffer = subrect_y - y;
             memcpy(static_cast<GLubyte*>(data)
                      + checked_alignedRowSize.value() * (subrect_y_in_dest_buffer + y_inside_subrect)
-                     + size * subrect_x_in_dest_buffer, // destination
+                     + bytesPerPixel * subrect_x_in_dest_buffer, // destination
                    subrect_data + subrect_alignedRowSize * y_inside_subrect, // source
                    subrect_plainRowSize); // size
         }
         delete [] subrect_data;
     }
 
     // if we're reading alpha, we may need to do fixup.  Note that we don't allow
     // GL_ALPHA to readpixels currently, but we had the code written for it already.
@@ -3522,19 +3558,17 @@ WebGLContext::ReadPixels_base(WebGLint x
 
 NS_IMETHODIMP
 WebGLContext::ReadPixels_array(WebGLint x, WebGLint y, WebGLsizei width, WebGLsizei height,
                                WebGLenum format, WebGLenum type, JSObject *pixels)
 {
     if (mContextLost)
         return NS_OK;
 
-    return ReadPixels_base(x, y, width, height, format, type,
-                           pixels ? JS_GetTypedArrayData(pixels) : 0,
-                           pixels ? JS_GetTypedArrayByteLength(pixels) : 0);
+    return ReadPixels_base(x, y, width, height, format, type, pixels);
 }
 
 NS_IMETHODIMP
 WebGLContext::RenderbufferStorage(WebGLenum target, WebGLenum internalformat, WebGLsizei width, WebGLsizei height)
 {
     if (mContextLost)
         return NS_OK;
 
--- a/content/canvas/test/webgl/failing_tests_linux.txt
+++ b/content/canvas/test/webgl/failing_tests_linux.txt
@@ -1,14 +1,13 @@
 conformance/context/premultiplyalpha-test.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/misc/uninitialized-test.html
 conformance/programs/gl-get-active-attribute.html
-conformance/reading/read-pixels-test.html
 conformance/renderbuffers/framebuffer-object-attachment.html
 conformance/textures/texture-mips.html
 conformance/uniforms/gl-uniform-bool.html
 conformance/more/conformance/quickCheckAPI-S_V.html
 conformance/more/functions/copyTexImage2D.html
 conformance/more/functions/copyTexSubImage2D.html
 conformance/more/functions/uniformfArrayLen1.html
--- a/content/canvas/test/webgl/failing_tests_mac.txt
+++ b/content/canvas/test/webgl/failing_tests_mac.txt
@@ -1,16 +1,15 @@
 conformance/context/context-attributes-alpha-depth-stencil-antialias.html
 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/reading/read-pixels-test.html
 conformance/renderbuffers/framebuffer-object-attachment.html
 conformance/state/gl-object-get-calls.html
 conformance/textures/tex-input-validation.html
 conformance/textures/texture-mips.html
 conformance/textures/texture-npot.html
 conformance/more/conformance/quickCheckAPI-S_V.html
 conformance/more/functions/copyTexImage2D.html
 conformance/more/functions/copyTexSubImage2D.html
--- a/content/canvas/test/webgl/failing_tests_windows.txt
+++ b/content/canvas/test/webgl/failing_tests_windows.txt
@@ -1,13 +1,12 @@
 conformance/context/premultiplyalpha-test.html
 conformance/glsl/functions/glsl-function-atan.html
 conformance/glsl/functions/glsl-function-atan-xy.html
 conformance/glsl/functions/glsl-function-mod-gentype.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/reading/read-pixels-test.html
 conformance/renderbuffers/framebuffer-object-attachment.html
 conformance/more/conformance/quickCheckAPI-S_V.html
 conformance/more/functions/copyTexImage2D.html
 conformance/more/functions/copyTexSubImage2D.html
 conformance/more/functions/uniformfArrayLen1.html