Bug 682172: fixed tex2DImage WebGL function, which was rejecting 0-size textures and doing an incorrect validation - r=bjacob

There was actually some surrounding logic breaking 0-size textures. It was because there was code that basically checked "did uint=negative_num*other_vars overflow". For incorrect validation, two copies of the same variable (one stored internally and one passed in) were available to the function, but the one stored internally was being validated, while the version passed in wasn't. The fix for this was simply checking the passed var instead.

content/canvas/src/WebGLContext.h

@@ -428,6 +428,16 @@ protected:
     void UndoFakeVertexAttrib0();
     void InvalidateFakeVertexAttrib0();
 
+    static CheckedUint32 GetImageSize(WebGLsizei height, 
+                                      WebGLsizei width, 
+                                      PRUint32 pixelSize,
+                                      PRUint32 alignment);
+
+    // Returns x rounded to the next highest multiple of y.
+    static CheckedUint32 RoundedToNextMultipleOf(CheckedUint32 x, CheckedUint32 y) {
+        return ((x + y - 1) / y) * y;
+    }
+
     nsCOMPtr<nsIDOMHTMLCanvasElement> mCanvasElement;
     nsHTMLCanvasElement *HTMLCanvasElement() {
         return static_cast<nsHTMLCanvasElement*>(mCanvasElement.get());

content/canvas/src/WebGLContextGL.cpp

@@ -766,16 +766,8 @@ WebGLContext::CopyTexSubImage2D_base(WebGLenum target,
         if (!ValidateTexFormatAndType(internalformat, LOCAL_GL_UNSIGNED_BYTE, -1, &texelSize, info))
             return NS_OK;
 
-        CheckedUint32 checked_plainRowSize = CheckedUint32(width) * texelSize;
-
-        PRUint32 unpackAlignment = mPixelStoreUnpackAlignment;
-
-        // alignedRowSize = row size rounded up to next multiple of packAlignment
-        CheckedUint32 checked_alignedRowSize
-            = ((checked_plainRowSize + unpackAlignment-1) / unpackAlignment) * unpackAlignment;
-
-        CheckedUint32 checked_neededByteLength
-            = (height-1) * checked_alignedRowSize + checked_plainRowSize;
+        CheckedUint32 checked_neededByteLength = 
+            GetImageSize(height, width, texelSize, mPixelStoreUnpackAlignment);
 
         if (!checked_neededByteLength.valid())
             return ErrorInvalidOperation("%s: integer overflow computing the needed buffer size", info);
@@ -3015,16 +3007,13 @@ WebGLContext::ReadPixels_base(WebGLint x, WebGLint y, WebGLsizei width, WebGLsiz
     if (badType)
         return ErrorInvalidEnumInfo("ReadPixels: type", type);
 
+    CheckedUint32 checked_neededByteLength =
+        GetImageSize(height, width, size, mPixelStorePackAlignment);
+
     CheckedUint32 checked_plainRowSize = CheckedUint32(width) * size;
 
-    PRUint32 packAlignment = mPixelStorePackAlignment;
-
-    // alignedRowSize = row size rounded up to next multiple of packAlignment
-    CheckedUint32 checked_alignedRowSize
-        = ((checked_plainRowSize + packAlignment-1) / packAlignment) * packAlignment;
-
-    CheckedUint32 checked_neededByteLength
-        = (height-1) * checked_alignedRowSize + checked_plainRowSize;
+    CheckedUint32 checked_alignedRowSize = 
+        RoundedToNextMultipleOf(checked_plainRowSize, mPixelStorePackAlignment);
 
     if (!checked_neededByteLength.valid())
         return ErrorInvalidOperation("ReadPixels: integer overflow computing the needed buffer size");
@@ -3085,8 +3074,9 @@ WebGLContext::ReadPixels_base(WebGLint x, WebGLint y, WebGLsizei width, WebGLsiz
         // 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_alignedRowSize = (subrect_plainRowSize + packAlignment-1) &
-            ~PRUint32(packAlignment-1);
+	// 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
@@ -4305,7 +4295,7 @@ WebGLContext::TexImage2D_base(WebGLenum target, WebGLint level, WebGLenum intern
             return ErrorInvalidEnumInfo("texImage2D: target", target);
     }
 
-    switch (internalformat) {
+    switch (format) {
         case LOCAL_GL_RGB:
         case LOCAL_GL_RGBA:
         case LOCAL_GL_ALPHA:
@@ -4346,16 +4336,13 @@ WebGLContext::TexImage2D_base(WebGLenum target, WebGLint level, WebGLenum intern
     if (!ValidateTexFormatAndType(format, type, jsArrayType, &texelSize, "texImage2D"))
         return NS_OK;
 
+    CheckedUint32 checked_neededByteLength = 
+        GetImageSize(height, width, texelSize, mPixelStoreUnpackAlignment); 
+
     CheckedUint32 checked_plainRowSize = CheckedUint32(width) * texelSize;
 
-    PRUint32 unpackAlignment = mPixelStoreUnpackAlignment;
-
-    // alignedRowSize = row size rounded up to next multiple of packAlignment
-    CheckedUint32 checked_alignedRowSize
-        = ((checked_plainRowSize + unpackAlignment-1) / unpackAlignment) * unpackAlignment;
-
-    CheckedUint32 checked_neededByteLength
-        = (height-1) * checked_alignedRowSize + checked_plainRowSize;
+    CheckedUint32 checked_alignedRowSize =
+        RoundedToNextMultipleOf(checked_plainRowSize.value(), mPixelStoreUnpackAlignment);
 
     if (!checked_neededByteLength.valid())
         return ErrorInvalidOperation("texImage2D: integer overflow computing the needed buffer size");
@@ -4546,16 +4533,13 @@ WebGLContext::TexSubImage2D_base(WebGLenum target, WebGLint level,
     if (width == 0 || height == 0)
         return NS_OK; // ES 2.0 says it has no effect, we better return right now
 
+    CheckedUint32 checked_neededByteLength = 
+        GetImageSize(height, width, texelSize, mPixelStoreUnpackAlignment);
+
     CheckedUint32 checked_plainRowSize = CheckedUint32(width) * texelSize;
 
-    PRUint32 unpackAlignment = mPixelStoreUnpackAlignment;
-
-    // alignedRowSize = row size rounded up to next multiple of packAlignment
-    CheckedUint32 checked_alignedRowSize
-        = ((checked_plainRowSize + unpackAlignment-1) / unpackAlignment) * unpackAlignment;
-
-    CheckedUint32 checked_neededByteLength
-        = (height-1) * checked_alignedRowSize + checked_plainRowSize;
+    CheckedUint32 checked_alignedRowSize = 
+        RoundedToNextMultipleOf(checked_plainRowSize.value(), mPixelStoreUnpackAlignment);
 
     if (!checked_neededByteLength.valid())
         return ErrorInvalidOperation("texSubImage2D: integer overflow computing the needed buffer size");
@@ -4586,7 +4570,8 @@ WebGLContext::TexSubImage2D_base(WebGLenum target, WebGLint level,
     size_t srcStride = srcStrideOrZero ? srcStrideOrZero : checked_alignedRowSize.value();
 
     size_t dstPlainRowSize = texelSize * width;
-    size_t dstStride = ((dstPlainRowSize + unpackAlignment-1) / unpackAlignment) * unpackAlignment;
+    // There are checks above to ensure that this won't overflow.
+    size_t dstStride = RoundedToNextMultipleOf(dstPlainRowSize, mPixelStoreUnpackAlignment).value();
 
     if (actualSrcFormat == dstFormat &&
         srcPremultiplied == mPixelStorePremultiplyAlpha &&

content/canvas/src/WebGLContextUtils.cpp

@@ -117,6 +117,24 @@ WebGLContext::LogMessageIfVerbose(const char *fmt, va_list ap)
     firstTime = PR_FALSE;
 }
 
+CheckedUint32
+WebGLContext::GetImageSize(WebGLsizei height, 
+                           WebGLsizei width, 
+                           PRUint32 pixelSize,
+                           PRUint32 packOrUnpackAlignment)
+{
+    CheckedUint32 checked_plainRowSize = CheckedUint32(width) * pixelSize;
+
+    // alignedRowSize = row size rounded up to next multiple of packAlignment
+    CheckedUint32 checked_alignedRowSize = RoundedToNextMultipleOf(checked_plainRowSize, packOrUnpackAlignment);
+
+    // if height is 0, we don't need any memory to store this; without this check, we'll get an overflow
+    CheckedUint32 checked_neededByteLength
+        = height <= 0 ? 0 : (height-1) * checked_alignedRowSize + checked_plainRowSize;
+
+    return checked_neededByteLength;
+}
+
 nsresult
 WebGLContext::SynthesizeGLError(WebGLenum err)
 {

content/canvas/test/webgl/failing_tests_linux.txt

@@ -19,5 +19,4 @@ conformance/more/conformance/quickCheckAPI.html
 conformance/more/functions/copyTexImage2D.html
 conformance/more/functions/copyTexSubImage2D.html
 conformance/more/functions/deleteBufferBadArgs.html
-conformance/more/functions/texImage2DBadArgs.html
 conformance/more/functions/uniformfArrayLen1.html

content/canvas/test/webgl/failing_tests_mac.txt

@@ -15,6 +15,5 @@ conformance/more/conformance/quickCheckAPI.html
 conformance/more/functions/copyTexImage2D.html
 conformance/more/functions/copyTexSubImage2D.html
 conformance/more/functions/deleteBufferBadArgs.html
-conformance/more/functions/texImage2DBadArgs.html
 conformance/more/functions/uniformfBadArgs.html
 conformance/more/functions/uniformiBadArgs.html

content/canvas/test/webgl/failing_tests_windows.txt

@@ -10,5 +10,4 @@ conformance/more/conformance/quickCheckAPI.html
 conformance/more/functions/copyTexImage2D.html
 conformance/more/functions/copyTexSubImage2D.html
 conformance/more/functions/deleteBufferBadArgs.html
-conformance/more/functions/texImage2DBadArgs.html
 conformance/more/functions/uniformfArrayLen1.html