From bb94fefa3f5d9c9e9b2477642958f056e96ed667 Mon Sep 17 00:00:00 2001 From: Doug Sherk Date: Thu, 1 Sep 2011 15:28:34 -0400 Subject: [PATCH] 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 | 10 +++ content/canvas/src/WebGLContextGL.cpp | 61 +++++++------------ content/canvas/src/WebGLContextUtils.cpp | 18 ++++++ .../canvas/test/webgl/failing_tests_linux.txt | 1 - .../canvas/test/webgl/failing_tests_mac.txt | 1 - .../test/webgl/failing_tests_windows.txt | 1 - 6 files changed, 51 insertions(+), 41 deletions(-) diff --git a/content/canvas/src/WebGLContext.h b/content/canvas/src/WebGLContext.h index 97d69855651..30e668240a8 100644 --- a/content/canvas/src/WebGLContext.h +++ b/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 mCanvasElement; nsHTMLCanvasElement *HTMLCanvasElement() { return static_cast(mCanvasElement.get()); diff --git a/content/canvas/src/WebGLContextGL.cpp b/content/canvas/src/WebGLContextGL.cpp index f50db07fe5d..c8c7998dcd4 100644 --- a/content/canvas/src/WebGLContextGL.cpp +++ b/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 && diff --git a/content/canvas/src/WebGLContextUtils.cpp b/content/canvas/src/WebGLContextUtils.cpp index 74de373b0d6..e2128168992 100644 --- a/content/canvas/src/WebGLContextUtils.cpp +++ b/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) { diff --git a/content/canvas/test/webgl/failing_tests_linux.txt b/content/canvas/test/webgl/failing_tests_linux.txt index 0b66451af59..7a34f85d6b1 100644 --- a/content/canvas/test/webgl/failing_tests_linux.txt +++ b/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 diff --git a/content/canvas/test/webgl/failing_tests_mac.txt b/content/canvas/test/webgl/failing_tests_mac.txt index b38e4fee694..14bf23092a9 100644 --- a/content/canvas/test/webgl/failing_tests_mac.txt +++ b/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 diff --git a/content/canvas/test/webgl/failing_tests_windows.txt b/content/canvas/test/webgl/failing_tests_windows.txt index 6916882fcb3..1ef04b4ad48 100644 --- a/content/canvas/test/webgl/failing_tests_windows.txt +++ b/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