Bug 1284346 - Fix PACK_ params. - r=jrmuizel draft
authorJeff Gilbert <jgilbert@mozilla.com>
Mon, 04 Jul 2016 14:40:38 -0700
changeset 383703 9b9e1c637b20544ecc0805011a45fd4ba6ad3490
parent 383702 8bebe53b354879671c3036501efc8ee261a8859e
child 524541 81076c7a8b3d071f3068ca4a3c158031431bc19c
push id22090
push userbmo:jgilbert@mozilla.com
push dateMon, 04 Jul 2016 22:44:06 +0000
reviewersjrmuizel
bugs1284346
milestone50.0a1
Bug 1284346 - Fix PACK_ params. - r=jrmuizel MozReview-Commit-ID: 4cEwl5ekuoh
dom/canvas/WebGLContext.h
dom/canvas/WebGLContextGL.cpp
--- a/dom/canvas/WebGLContext.h
+++ b/dom/canvas/WebGLContext.h
@@ -1432,19 +1432,19 @@ protected:
     uint32_t mPixelStore_PackRowLength;
     uint32_t mPixelStore_PackSkipRows;
     uint32_t mPixelStore_PackSkipPixels;
     uint32_t mPixelStore_PackAlignment;
 
     CheckedUint32 GetUnpackSize(bool isFunc3D, uint32_t width, uint32_t height,
                                 uint32_t depth, uint8_t bytesPerPixel);
 
-    CheckedUint32 GetPackSize(uint32_t width, uint32_t height, uint8_t bytesPerPixel,
-                              CheckedUint32* const out_startOffset,
-                              CheckedUint32* const out_rowStride);
+    bool ValidatePackSize(const char* funcName, uint32_t width, uint32_t height,
+                          uint8_t bytesPerPixel, uint32_t* const out_rowStride,
+                          uint32_t* const out_endOffset);
 
     GLenum mPixelStore_ColorspaceConversion;
     bool mPixelStore_FlipY;
     bool mPixelStore_PremultiplyAlpha;
 
     ////////////////////////////////////
     class FakeBlackTexture {
     public:
--- a/dom/canvas/WebGLContextGL.cpp
+++ b/dom/canvas/WebGLContextGL.cpp
@@ -1224,38 +1224,38 @@ WebGLContext::DoReadPixelsAndConvert(GLi
 
     if (gl->WorkAroundDriverBugs() &&
         gl->IsANGLE() &&
         gl->Version() < 300 && // ANGLE ES2 doesn't support HALF_FLOAT reads properly.
         readType == LOCAL_GL_FLOAT &&
         auxReadFormat == destFormat &&
         auxReadType == LOCAL_GL_HALF_FLOAT)
     {
-        MOZ_RELEASE_ASSERT(!IsWebGL2());
+        MOZ_RELEASE_ASSERT(!IsWebGL2()); // No SKIP_PIXELS, etc.
 
         readType = auxReadType;
 
+        const char funcName[] = "readPixels";
         const auto readBytesPerPixel = webgl::BytesPerPixel({readFormat, readType});
         const auto destBytesPerPixel = webgl::BytesPerPixel({destFormat, destType});
 
-        CheckedUint32 readOffset;
-        CheckedUint32 readStride;
-        const CheckedUint32 readSize = GetPackSize(width, height, readBytesPerPixel,
-                                                   &readOffset, &readStride);
-
-        CheckedUint32 destOffset;
-        CheckedUint32 destStride;
-        const CheckedUint32 destSize = GetPackSize(width, height, destBytesPerPixel,
-                                                   &destOffset, &destStride);
-        if (!readSize.isValid() || !destSize.isValid()) {
+        uint32_t readStride;
+        uint32_t readByteCount;
+        uint32_t destStride;
+        uint32_t destByteCount;
+        if (!ValidatePackSize(funcName, width, height, readBytesPerPixel, &readStride,
+                              &readByteCount) ||
+            !ValidatePackSize(funcName, width, height, destBytesPerPixel, &destStride,
+                              &destByteCount))
+        {
             ErrorOutOfMemory("readPixels: Overflow calculating sizes for conversion.");
             return false;
         }
 
-        UniqueBuffer readBuffer = malloc(readSize.value());
+        UniqueBuffer readBuffer = malloc(readByteCount);
         if (!readBuffer) {
             ErrorOutOfMemory("readPixels: Failed to alloc temp buffer for conversion.");
             return false;
         }
 
         gl::GLContext::LocalErrorScope errorScope(*gl);
 
         gl->fReadPixels(x, y, width, height, readFormat, readType, readBuffer.get());
@@ -1266,35 +1266,35 @@ WebGLContext::DoReadPixelsAndConvert(GLi
             return false;
         }
 
         if (error) {
             MOZ_RELEASE_ASSERT(false, "GFX: Unexpected driver error.");
             return false;
         }
 
-        size_t channelsPerRow = std::min(readStride.value() / sizeof(uint16_t),
-                                         destStride.value() / sizeof(float));
-
-        const uint8_t* srcRow = (uint8_t*)(readBuffer.get()) + readOffset.value();
-        uint8_t* dstRow = (uint8_t*)(destBytes) + destOffset.value();
+        size_t channelsPerRow = std::min(readStride / sizeof(uint16_t),
+                                         destStride / sizeof(float));
+
+        const uint8_t* srcRow = (uint8_t*)readBuffer.get();
+        uint8_t* dstRow = (uint8_t*)destBytes;
 
         for (size_t j = 0; j < (size_t)height; j++) {
             auto src = (const uint16_t*)srcRow;
             auto dst = (float*)dstRow;
 
             const auto srcEnd = src + channelsPerRow;
             while (src != srcEnd) {
                 *dst = unpackFromFloat16(*src);
                 ++src;
                 ++dst;
             }
 
-            srcRow += readStride.value();
-            dstRow += destStride.value();
+            srcRow += readStride;
+            dstRow += destStride;
         }
 
         return true;
     }
 
     gl->fReadPixels(x, y, width, height, destFormat, destType, destBytes);
     return true;
 }
@@ -1389,53 +1389,66 @@ IsIntegerFormatAndTypeUnpackable(GLenum 
         return format == LOCAL_GL_RGB;
 
     default:
         return false;
     }
 }
 
 
-CheckedUint32
-WebGLContext::GetPackSize(uint32_t width, uint32_t height, uint8_t bytesPerPixel,
-                          CheckedUint32* const out_startOffset,
-                          CheckedUint32* const out_rowStride)
+bool
+WebGLContext::ValidatePackSize(const char* funcName, uint32_t width, uint32_t height,
+                               uint8_t bytesPerPixel, uint32_t* const out_rowStride,
+                               uint32_t* const out_endOffset)
 {
     if (!width || !height) {
-        *out_startOffset = 0;
         *out_rowStride = 0;
-        return 0;
+        *out_endOffset = 0;
+        return true;
     }
 
-    const CheckedUint32 pixelsPerRow = (mPixelStore_PackRowLength ? mPixelStore_PackRowLength
-                                                                  : width);
-    const CheckedUint32 skipPixels = mPixelStore_PackSkipPixels;
-    const CheckedUint32 skipRows = mPixelStore_PackSkipRows;
-    const CheckedUint32 alignment = mPixelStore_PackAlignment;
-
     // GLES 3.0.4, p116 (PACK_ functions like UNPACK_)
-    const auto totalBytesPerRow = bytesPerPixel * pixelsPerRow;
-    const auto rowStride = RoundUpToMultipleOf(totalBytesPerRow, alignment);
-
-    const auto startOffset = rowStride * skipRows + bytesPerPixel * skipPixels;
-    const auto usedBytesPerRow = bytesPerPixel * width;
-
-    const auto bytesNeeded = startOffset + rowStride * (height - 1) + usedBytesPerRow;
-
-    *out_startOffset = startOffset;
-    *out_rowStride = rowStride;
-    return bytesNeeded;
+
+    const auto rowLength = (mPixelStore_PackRowLength ? mPixelStore_PackRowLength
+                                                      : width);
+    const auto skipPixels = mPixelStore_PackSkipPixels;
+    const auto skipRows = mPixelStore_PackSkipRows;
+    const auto alignment = mPixelStore_PackAlignment;
+
+    const auto usedPixelsPerRow = CheckedUint32(skipPixels) + width;
+    const auto usedRowsPerImage = CheckedUint32(skipRows) + height;
+
+    if (!usedPixelsPerRow.isValid() || usedPixelsPerRow.value() > rowLength) {
+        ErrorInvalidOperation("%s: SKIP_PIXELS + width > ROW_LENGTH.", funcName);
+        return false;
+    }
+
+    const auto rowLengthBytes = CheckedUint32(rowLength) * bytesPerPixel;
+    const auto rowStride = RoundUpToMultipleOf(rowLengthBytes, alignment);
+
+    const auto usedBytesPerRow = usedPixelsPerRow * bytesPerPixel;
+    const auto usedBytesPerImage = (usedRowsPerImage - 1) * rowStride + usedBytesPerRow;
+
+    if (!rowStride.isValid() || !usedBytesPerImage.isValid()) {
+        ErrorInvalidOperation("%s: Invalid UNPACK_ params.", funcName);
+        return false;
+    }
+
+    *out_rowStride = rowStride.value();
+    *out_endOffset = usedBytesPerImage.value();
+    return true;
 }
 
 void
 WebGLContext::ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
                          GLenum type,
                          const dom::Nullable<dom::ArrayBufferView>& pixels,
                          ErrorResult& out_error)
 {
+    const char funcName[] = "readPixels";
     if (IsContextLost())
         return;
 
     if (mCanvasElement &&
         mCanvasElement->IsWriteOnly() &&
         !nsContentUtils::IsCallerChrome())
     {
         GenerateWarning("readPixels: Not allowed");
@@ -1537,50 +1550,54 @@ WebGLContext::ReadPixels(GLint x, GLint 
         bytesPerPixel = 2*channels;
         requiredDataType = js::Scalar::Uint16;
         break;
 
     default:
         MOZ_CRASH("GFX: bad `type`");
     }
 
+    //////
+
     const auto& view = pixels.Value();
 
     // Compute length and data.  Don't reenter after this point, lest the
     // precomputed go out of sync with the instant length/data.
     view.ComputeLengthAndData();
     void* data = view.DataAllowShared();
     size_t bytesAvailable = view.LengthAllowShared();
     js::Scalar::Type dataType = JS_GetArrayBufferViewType(view.Obj());
 
     // Check the pixels param type
     if (dataType != requiredDataType)
         return ErrorInvalidOperation("readPixels: Mismatched type/pixels types");
 
-    CheckedUint32 startOffset;
-    CheckedUint32 rowStride;
-    const auto bytesNeeded = GetPackSize(width, height, bytesPerPixel, &startOffset,
-                                         &rowStride);
-    if (!bytesNeeded.isValid()) {
-        ErrorInvalidOperation("readPixels: Integer overflow computing the needed buffer"
-                              " size.");
-        return;
-    }
-
-    if (bytesNeeded.value() > bytesAvailable) {
-        ErrorInvalidOperation("readPixels: buffer too small");
-        return;
-    }
-
     if (!data) {
         ErrorOutOfMemory("readPixels: buffer storage is null. Did we run out of memory?");
         out_error.Throw(NS_ERROR_OUT_OF_MEMORY);
         return;
     }
 
+    //////
+
+    uint32_t rowStride;
+    uint32_t bytesNeeded;
+    if (!ValidatePackSize(funcName, width, height, bytesPerPixel, &rowStride,
+                          &bytesNeeded))
+    {
+        return;
+    }
+
+    if (bytesNeeded > bytesAvailable) {
+        ErrorInvalidOperation("readPixels: buffer too small");
+        return;
+    }
+
+    //////
+
     MakeContextCurrent();
 
     const webgl::FormatUsageInfo* srcFormat;
     uint32_t srcWidth;
     uint32_t srcHeight;
     GLenum srcMode;
     if (!ValidateCurFBForRead("readPixels", &srcFormat, &srcWidth, &srcHeight, &srcMode))
         return;
@@ -1641,91 +1658,62 @@ WebGLContext::ReadPixels(GLint x, GLint 
 
     uint32_t readX, readY;
     uint32_t writeX, writeY;
     uint32_t rwWidth, rwHeight;
     Intersect(srcWidth, x, width, &readX, &writeX, &rwWidth);
     Intersect(srcHeight, y, height, &readY, &writeY, &rwHeight);
 
     if (rwWidth == uint32_t(width) && rwHeight == uint32_t(height)) {
-        // Warning: Possibly shared memory.  See bug 1225033.
         DoReadPixelsAndConvert(x, y, width, height, format, type, data, auxReadFormat,
                                auxReadType);
         return;
     }
 
     // Read request contains out-of-bounds pixels. Unfortunately:
     // GLES 3.0.4 p194 "Obtaining Pixels from the Framebuffer":
     // "If any of these pixels lies outside of the window allocated to the current GL
     //  context, or outside of the image attached to the currently bound framebuffer
     //  object, then the values obtained for those pixels are undefined."
 
     // This is a slow-path, so warn people away!
     GenerateWarning("readPixels: Out-of-bounds reads with readPixels are deprecated, and"
                     " may be slow.");
 
-    // Currently, the spec dictates that we need to zero the out-of-bounds pixels.
-
-    // Ideally we could just ReadPixels into the buffer, then zero the undefined parts.
-    // However, we can't do this for *shared* ArrayBuffers, as they can have racey
-    // accesses from Workers.
-
-    // We can use a couple tricks to do this faster, but we shouldn't encourage this
-    // anyway. Why not just do it the really safe, dead-simple way, even if it is
-    // hilariously slow?
-
-    ////////////////////////////////////
-    // Clear the targetted pixels to zero.
-
-    if (mPixelStore_PackRowLength ||
-        mPixelStore_PackSkipPixels ||
-        mPixelStore_PackSkipRows)
-    {
-        // Targetted bytes might not be contiguous, so do it row-by-row.
-        uint8_t* row = (uint8_t*)data + startOffset.value();
-        const auto bytesPerRow = bytesPerPixel * width;
-        for (uint32_t j = 0; j < uint32_t(height); j++) {
-            std::memset(row, 0, bytesPerRow);
-            row += rowStride.value();
-        }
-    } else {
-        std::memset(data, 0, bytesNeeded.value());
-    }
-
     ////////////////////////////////////
     // Read only the in-bounds pixels.
 
     if (!rwWidth || !rwHeight) {
         // There aren't any, so we're 'done'.
         DummyReadFramebufferOperation("readPixels");
         return;
     }
 
     if (IsWebGL2()) {
         if (!mPixelStore_PackRowLength) {
-            gl->fPixelStorei(LOCAL_GL_PACK_ROW_LENGTH, width);
+            gl->fPixelStorei(LOCAL_GL_PACK_ROW_LENGTH, mPixelStore_PackSkipPixels + width);
         }
         gl->fPixelStorei(LOCAL_GL_PACK_SKIP_PIXELS, mPixelStore_PackSkipPixels + writeX);
         gl->fPixelStorei(LOCAL_GL_PACK_SKIP_ROWS, mPixelStore_PackSkipRows + writeY);
 
         DoReadPixelsAndConvert(readX, readY, rwWidth, rwHeight, format, type, data,
                                auxReadFormat, auxReadType);
 
         gl->fPixelStorei(LOCAL_GL_PACK_ROW_LENGTH, mPixelStore_PackRowLength);
         gl->fPixelStorei(LOCAL_GL_PACK_SKIP_PIXELS, mPixelStore_PackSkipPixels);
         gl->fPixelStorei(LOCAL_GL_PACK_SKIP_ROWS, mPixelStore_PackSkipRows);
     } else {
         // I *did* say "hilariously slow".
 
-        uint8_t* row = (uint8_t*)data + startOffset.value() + writeX * bytesPerPixel;
-        row += writeY * rowStride.value();
+        uint8_t* row = (uint8_t*)data + writeX * bytesPerPixel;
+        row += writeY * rowStride;
         for (uint32_t j = 0; j < rwHeight; j++) {
             DoReadPixelsAndConvert(readX, readY+j, rwWidth, 1, format, type, row,
                                    auxReadFormat, auxReadType);
-            row += rowStride.value();
+            row += rowStride;
         }
     }
 }
 
 void
 WebGLContext::RenderbufferStorage_base(const char* funcName, GLenum target,
                                        GLsizei samples, GLenum internalFormat,
                                        GLsizei width, GLsizei height)