From ff9844ee322dc1be6f09b7a965044a1100f569f4 Mon Sep 17 00:00:00 2001 From: Jeff Gilbert Date: Thu, 21 Jul 2016 23:05:52 -0700 Subject: [PATCH] Bug 1280499 - Allow SKIP_ROWS+height>IMAGE_HEIGHT and fix paranoid uploading. - r=jrmuizel Top-of-tree test is green now. MozReview-Commit-ID: IbCTHK62qGT --- dom/canvas/TexUnpackBlob.cpp | 43 +++++++++++++++++++++++-------- dom/canvas/WebGLTextureUpload.cpp | 39 ++++++++++++++-------------- 2 files changed, 51 insertions(+), 31 deletions(-) diff --git a/dom/canvas/TexUnpackBlob.cpp b/dom/canvas/TexUnpackBlob.cpp index ddeb44809095..02ca502a41bc 100644 --- a/dom/canvas/TexUnpackBlob.cpp +++ b/dom/canvas/TexUnpackBlob.cpp @@ -132,6 +132,12 @@ FormatForPackingInfo(const PackingInfo& pi) //////////////////// +static uint32_t +ZeroOn2D(TexImageTarget target, uint32_t val) +{ + return (IsTarget3D(target) ? val : 0); +} + static uint32_t FallbackOnZero(uint32_t val, uint32_t fallback) { @@ -143,11 +149,13 @@ TexUnpackBlob::TexUnpackBlob(const WebGLContext* webgl, TexImageTarget target, uint32_t depth, bool isSrcPremult) : mAlignment(webgl->mPixelStore_UnpackAlignment) , mRowLength(rowLength) - , mImageHeight(FallbackOnZero(webgl->mPixelStore_UnpackImageHeight, height)) + , mImageHeight(FallbackOnZero(ZeroOn2D(target, + webgl->mPixelStore_UnpackImageHeight), + height)) , mSkipPixels(webgl->mPixelStore_UnpackSkipPixels) , mSkipRows(webgl->mPixelStore_UnpackSkipRows) - , mSkipImages(IsTarget3D(target) ? webgl->mPixelStore_UnpackSkipImages : 0) + , mSkipImages(ZeroOn2D(target, webgl->mPixelStore_UnpackSkipImages)) , mWidth(width) , mHeight(height) @@ -393,41 +401,54 @@ TexUnpackBytes::TexOrSubImage(bool isSubImage, bool needsRespec, const char* fun zOffset, mWidth, mHeight, mDepth, nullptr); gl->fBindBuffer(LOCAL_GL_PIXEL_UNPACK_BUFFER, webgl->mBoundPixelUnpackBuffer->mGLName); + if (*out_error) + return false; } ////// + // Make our sometimes-implicit values explicit. Also this keeps them constant when we + // ask for height=mHeight-1 and such. + gl->fPixelStorei(LOCAL_GL_UNPACK_ROW_LENGTH, mRowLength); + gl->fPixelStorei(LOCAL_GL_UNPACK_IMAGE_HEIGHT, mImageHeight); + if (mDepth > 1) { *out_error = DoTexOrSubImage(true, gl, target, level, dui, xOffset, yOffset, zOffset, mWidth, mHeight, mDepth-1, uploadPtr); } + // Skip the images we uploaded. + gl->fPixelStorei(LOCAL_GL_UNPACK_SKIP_IMAGES, mSkipImages + mDepth - 1); + if (mHeight > 1) { *out_error = DoTexOrSubImage(true, gl, target, level, dui, xOffset, yOffset, zOffset+mDepth-1, mWidth, mHeight-1, 1, uploadPtr); } - const uint32_t imageStride = rowStride.value() * mImageHeight; + const auto totalSkipRows = CheckedUint32(mSkipImages) * mImageHeight + mSkipRows; + const auto totalFullRows = CheckedUint32(mDepth - 1) * mImageHeight + mHeight - 1; + const auto tailOffsetRows = totalSkipRows + totalFullRows; - const uint32_t usedImages = mSkipImages + mDepth; - const uint32_t usedRows = mSkipRows + mHeight; + const auto tailOffsetBytes = tailOffsetRows * rowStride; - uploadPtr += (usedImages - 1) * imageStride; - uploadPtr += (usedRows - 1) * rowStride.value(); + uploadPtr += tailOffsetBytes.value(); ////// - gl->fPixelStorei(LOCAL_GL_UNPACK_ALIGNMENT, 1); - gl->fPixelStorei(LOCAL_GL_UNPACK_IMAGE_HEIGHT, 0); - gl->fPixelStorei(LOCAL_GL_UNPACK_SKIP_IMAGES, 0); - gl->fPixelStorei(LOCAL_GL_UNPACK_SKIP_ROWS, 0); + gl->fPixelStorei(LOCAL_GL_UNPACK_ALIGNMENT, 1); // No stride padding. + gl->fPixelStorei(LOCAL_GL_UNPACK_ROW_LENGTH, 0); // No padding in general. + gl->fPixelStorei(LOCAL_GL_UNPACK_SKIP_IMAGES, 0); // Don't skip images, + gl->fPixelStorei(LOCAL_GL_UNPACK_SKIP_ROWS, 0); // or rows. + // Keep skipping pixels though! *out_error = DoTexOrSubImage(true, gl, target, level, dui, xOffset, yOffset+mHeight-1, zOffset+mDepth-1, mWidth, 1, 1, uploadPtr); + // Reset all our modified state. gl->fPixelStorei(LOCAL_GL_UNPACK_ALIGNMENT, webgl->mPixelStore_UnpackAlignment); gl->fPixelStorei(LOCAL_GL_UNPACK_IMAGE_HEIGHT, webgl->mPixelStore_UnpackImageHeight); + gl->fPixelStorei(LOCAL_GL_UNPACK_ROW_LENGTH, webgl->mPixelStore_UnpackRowLength); gl->fPixelStorei(LOCAL_GL_UNPACK_SKIP_IMAGES, webgl->mPixelStore_UnpackSkipImages); gl->fPixelStorei(LOCAL_GL_UNPACK_SKIP_ROWS, webgl->mPixelStore_UnpackSkipRows); diff --git a/dom/canvas/WebGLTextureUpload.cpp b/dom/canvas/WebGLTextureUpload.cpp index 7fb7f62ef43b..5082a5d852a8 100644 --- a/dom/canvas/WebGLTextureUpload.cpp +++ b/dom/canvas/WebGLTextureUpload.cpp @@ -129,34 +129,33 @@ bool WebGLContext::ValidateUnpackPixels(const char* funcName, uint32_t fullRows, uint32_t tailPixels, webgl::TexUnpackBlob* blob) { + auto skipPixels = CheckedUint32(blob->mSkipPixels); + skipPixels += CheckedUint32(blob->mSkipRows); + const auto usedPixelsPerRow = CheckedUint32(blob->mSkipPixels) + blob->mWidth; - const auto usedRowsPerImage = CheckedUint32(blob->mSkipRows) + blob->mHeight; - const auto usedImages = CheckedUint32(blob->mSkipImages) + blob->mDepth; - - if (!usedPixelsPerRow.isValid() || - !usedRowsPerImage.isValid() || - !usedImages.isValid()) - { - ErrorOutOfMemory("%s: Invalid calculation for e.g. UNPACK_SKIP_PIXELS + width.", - funcName); - return false; - } - - ////// - - if (usedPixelsPerRow.value() > blob->mRowLength || - usedRowsPerImage.value() > blob->mImageHeight) - { - ErrorInvalidOperation("%s: UNPACK_ROW_LENGTH or UNPACK_IMAGE_HEIGHT too small.", + if (!usedPixelsPerRow.isValid() || usedPixelsPerRow.value() > blob->mRowLength) { + ErrorInvalidOperation("%s: UNPACK_SKIP_PIXELS + height > UNPACK_ROW_LENGTH.", funcName); return false; } + if (blob->mHeight > blob->mImageHeight) { + ErrorInvalidOperation("%s: height > UNPACK_IMAGE_HEIGHT.", funcName); + return false; + } + ////// - auto fullRowsNeeded = (usedImages - 1) * blob->mImageHeight; - fullRowsNeeded += usedRowsPerImage - 1; + // The spec doesn't bound SKIP_ROWS + height <= IMAGE_HEIGHT, unfortunately. + auto skipFullRows = CheckedUint32(blob->mSkipImages) * blob->mImageHeight; + skipFullRows += blob->mSkipRows; + MOZ_ASSERT(blob->mDepth >= 1); + MOZ_ASSERT(blob->mHeight >= 1); + auto usedFullRows = CheckedUint32(blob->mDepth - 1) * blob->mImageHeight; + usedFullRows += blob->mHeight - 1; // Full rows in the final image, excluding the tail. + + const auto fullRowsNeeded = skipFullRows + usedFullRows; if (!fullRowsNeeded.isValid()) { ErrorOutOfMemory("%s: Invalid calculation for required row count.", funcName);