Bug 1250710 - Add PACK PBO support. - r=jrmuizel
authorJeff Gilbert <jgilbert@mozilla.com>
Mon, 04 Jul 2016 20:35:20 -0700
changeset 330409 385d885ca02fbc48c66f1ce0941b176e4d0e28bb
parent 330408 c2ee198a1c719a61ed7666a20ab8883d6f7ddaa9
child 330410 057ec3ca368097a2594d34cef36e7151353297c9
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1250710
milestone50.0a1
Bug 1250710 - Add PACK PBO support. - r=jrmuizel MozReview-Commit-ID: DK7FgtE9ymm
dom/canvas/WebGL2ContextBuffers.cpp
dom/canvas/WebGLContext.h
dom/canvas/WebGLContextGL.cpp
dom/canvas/WebGLFormats.cpp
dom/canvas/WebGLFormats.h
gfx/gl/GLContext.h
--- a/dom/canvas/WebGL2ContextBuffers.cpp
+++ b/dom/canvas/WebGL2ContextBuffers.cpp
@@ -257,20 +257,9 @@ WebGL2Context::GetBufferSubData(GLenum t
 
 void
 WebGL2Context::GetBufferSubData(GLenum target, GLintptr offset,
                                 const dom::SharedArrayBuffer& data)
 {
     GetBufferSubDataT(target, offset, data);
 }
 
-void
-WebGL2Context::ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
-                          GLenum type, GLintptr offset)
-{
-    const char funcName[] = "readPixels";
-    if (IsContextLost())
-        return;
-
-    ErrorInvalidOperation("%s: Not yet implemented.", funcName);
-}
-
 } // namespace mozilla
--- a/dom/canvas/WebGLContext.h
+++ b/dom/canvas/WebGLContext.h
@@ -534,19 +534,22 @@ public:
     bool IsRenderbuffer(WebGLRenderbuffer* rb);
     bool IsShader(WebGLShader* shader);
     bool IsVertexArray(WebGLVertexArray* vao);
     void LineWidth(GLfloat width);
     void LinkProgram(WebGLProgram* prog);
     void PixelStorei(GLenum pname, GLint param);
     void PolygonOffset(GLfloat factor, GLfloat units);
 protected:
-    bool DoReadPixelsAndConvert(GLint x, GLint y, GLsizei width, GLsizei height,
-                                GLenum destFormat, GLenum destType, void* destBytes,
-                                GLenum auxReadFormat, GLenum auxReadType);
+    bool ReadPixels_SharedPrecheck(ErrorResult* const out_error);
+    void ReadPixelsImpl(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
+                        GLenum type, void* data, uint32_t dataLen);
+    bool DoReadPixelsAndConvert(const webgl::FormatInfo* srcFormat, GLint x, GLint y,
+                                GLsizei width, GLsizei height, GLenum format,
+                                GLenum destType, void* dest);
 public:
     void ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height,
                     GLenum format, GLenum type,
                     const dom::Nullable<dom::ArrayBufferView>& pixels,
                     ErrorResult& rv);
     void RenderbufferStorage(GLenum target, GLenum internalFormat,
                              GLsizei width, GLsizei height);
 protected:
--- a/dom/canvas/WebGLContextGL.cpp
+++ b/dom/canvas/WebGLContextGL.cpp
@@ -1,14 +1,15 @@
 /* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "WebGLContext.h"
+#include "WebGL2Context.h"
 
 #include "WebGLActiveInfo.h"
 #include "WebGLContextUtils.h"
 #include "WebGLBuffer.h"
 #include "WebGLVertexAttribData.h"
 #include "WebGLShader.h"
 #include "WebGLProgram.h"
 #include "WebGLUniformLocation.h"
@@ -1209,38 +1210,47 @@ WebGLContext::PixelStorei(GLenum pname, 
 
     default:
         break;
     }
 
     ErrorInvalidEnumInfo("pixelStorei: parameter", pname);
 }
 
+static bool
+IsNeedsANGLEWorkAround(const webgl::FormatInfo* format)
+{
+    switch (format->effectiveFormat) {
+    case webgl::EffectiveFormat::RGB16F:
+    case webgl::EffectiveFormat::RGBA16F:
+        return true;
+
+    default:
+        return false;
+    }
+}
+
 bool
-WebGLContext::DoReadPixelsAndConvert(GLint x, GLint y, GLsizei width, GLsizei height,
-                                     GLenum destFormat, GLenum destType, void* destBytes,
-                                     GLenum auxReadFormat, GLenum auxReadType)
+WebGLContext::DoReadPixelsAndConvert(const webgl::FormatInfo* srcFormat, GLint x, GLint y,
+                                     GLsizei width, GLsizei height, GLenum format,
+                                     GLenum destType, void* dest)
 {
-    GLenum readFormat = destFormat;
-    GLenum readType = destType;
-
     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)
+        IsNeedsANGLEWorkAround(srcFormat))
     {
         MOZ_RELEASE_ASSERT(!IsWebGL2()); // No SKIP_PIXELS, etc.
-
-        readType = auxReadType;
+        MOZ_ASSERT(!mBoundPixelPackBuffer); // Let's be real clear.
+
+        const GLenum readType = LOCAL_GL_HALF_FLOAT_OES;
 
         const char funcName[] = "readPixels";
-        const auto readBytesPerPixel = webgl::BytesPerPixel({readFormat, readType});
-        const auto destBytesPerPixel = webgl::BytesPerPixel({destFormat, destType});
+        const auto readBytesPerPixel = webgl::BytesPerPixel({format, readType});
+        const auto destBytesPerPixel = webgl::BytesPerPixel({format, destType});
 
         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,
@@ -1253,34 +1263,34 @@ WebGLContext::DoReadPixelsAndConvert(GLi
         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());
+        gl->fReadPixels(x, y, width, height, format, readType, readBuffer.get());
 
         const GLenum error = errorScope.GetError();
         if (error == LOCAL_GL_OUT_OF_MEMORY) {
             ErrorOutOfMemory("readPixels: Driver ran out of memory.");
             return false;
         }
 
         if (error) {
             MOZ_RELEASE_ASSERT(false, "GFX: Unexpected driver error.");
             return false;
         }
 
         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;
+        uint8_t* dstRow = (uint8_t*)dest;
 
         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);
@@ -1290,17 +1300,17 @@ WebGLContext::DoReadPixelsAndConvert(GLi
 
             srcRow += readStride;
             dstRow += destStride;
         }
 
         return true;
     }
 
-    gl->fReadPixels(x, y, width, height, destFormat, destType, destBytes);
+    gl->fReadPixels(x, y, width, height, format, destType, dest);
     return true;
 }
 
 static bool
 IsFormatAndTypeUnpackable(GLenum format, GLenum type, bool isWebGL2)
 {
     switch (type) {
     case LOCAL_GL_UNSIGNED_BYTE:
@@ -1388,16 +1398,78 @@ IsIntegerFormatAndTypeUnpackable(GLenum 
     case LOCAL_GL_UNSIGNED_INT_5_9_9_9_REV:
         return format == LOCAL_GL_RGB;
 
     default:
         return false;
     }
 }
 
+static bool
+GetJSScalarFromGLType(GLenum type, js::Scalar::Type* const out_scalarType)
+{
+    switch (type) {
+    case LOCAL_GL_BYTE:
+        *out_scalarType = js::Scalar::Int8;
+        return true;
+
+    case LOCAL_GL_UNSIGNED_BYTE:
+        *out_scalarType = js::Scalar::Uint8;
+        return true;
+
+    case LOCAL_GL_SHORT:
+        *out_scalarType = js::Scalar::Int16;
+        return true;
+
+    case LOCAL_GL_HALF_FLOAT:
+    case LOCAL_GL_HALF_FLOAT_OES:
+    case LOCAL_GL_UNSIGNED_SHORT:
+    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:
+        *out_scalarType = js::Scalar::Uint16;
+        return true;
+
+    case LOCAL_GL_UNSIGNED_INT:
+    case LOCAL_GL_UNSIGNED_INT_2_10_10_10_REV:
+    case LOCAL_GL_UNSIGNED_INT_5_9_9_9_REV:
+    case LOCAL_GL_UNSIGNED_INT_10F_11F_11F_REV:
+    case LOCAL_GL_UNSIGNED_INT_24_8:
+        *out_scalarType = js::Scalar::Uint32;
+        return true;
+    case LOCAL_GL_INT:
+        *out_scalarType = js::Scalar::Int32;
+        return true;
+
+    case LOCAL_GL_FLOAT:
+        *out_scalarType = js::Scalar::Float32;
+        return true;
+
+    default:
+        return false;
+    }
+}
+
+bool
+WebGLContext::ReadPixels_SharedPrecheck(ErrorResult* const out_error)
+{
+    if (IsContextLost())
+        return false;
+
+    if (mCanvasElement &&
+        mCanvasElement->IsWriteOnly() &&
+        !nsContentUtils::IsCallerChrome())
+    {
+        GenerateWarning("readPixels: Not allowed");
+        out_error->Throw(NS_ERROR_DOM_SECURITY_ERR);
+        return false;
+    }
+
+    return true;
+}
 
 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_rowStride = 0;
@@ -1438,237 +1510,266 @@ WebGLContext::ValidatePackSize(const cha
 }
 
 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");
-        out_error.Throw(NS_ERROR_DOM_SECURITY_ERR);
+    if (!ReadPixels_SharedPrecheck(&out_error))
         return;
-    }
-
-    if (width < 0 || height < 0)
-        return ErrorInvalidValue("readPixels: negative size passed");
-
-    if (pixels.IsNull())
-        return ErrorInvalidValue("readPixels: null destination buffer");
-
-    if (!(IsWebGL2() && IsIntegerFormatAndTypeUnpackable(format, type)) &&
-        !IsFormatAndTypeUnpackable(format, type, IsWebGL2())) {
-        return ErrorInvalidEnum("readPixels: Bad format or type.");
-    }
-
-    int channels = 0;
-
-    // Check the format param
-    switch (format) {
-    case LOCAL_GL_ALPHA:
-    case LOCAL_GL_LUMINANCE:
-    case LOCAL_GL_RED:
-    case LOCAL_GL_RED_INTEGER:
-        channels = 1;
-        break;
-    case LOCAL_GL_LUMINANCE_ALPHA:
-    case LOCAL_GL_RG:
-    case LOCAL_GL_RG_INTEGER:
-        channels = 2;
-        break;
-    case LOCAL_GL_RGB:
-    case LOCAL_GL_RGB_INTEGER:
-        channels = 3;
-        break;
-    case LOCAL_GL_RGBA:
-    case LOCAL_GL_RGBA_INTEGER:
-        channels = 4;
-        break;
-    default:
-        MOZ_CRASH("GFX: bad `format`");
-    }
-
-
-    // Check the type param
-    int bytesPerPixel;
-    int requiredDataType;
-    switch (type) {
-    case LOCAL_GL_BYTE:
-        bytesPerPixel = 1*channels;
-        requiredDataType = js::Scalar::Int8;
-        break;
-
-    case LOCAL_GL_UNSIGNED_BYTE:
-        bytesPerPixel = 1*channels;
-        requiredDataType = js::Scalar::Uint8;
-        break;
-
-    case LOCAL_GL_SHORT:
-        bytesPerPixel = 2*channels;
-        requiredDataType = js::Scalar::Int16;
-        break;
-
-    case LOCAL_GL_UNSIGNED_SHORT:
-    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::Scalar::Uint16;
-        break;
-
-    case LOCAL_GL_UNSIGNED_INT_2_10_10_10_REV:
-    case LOCAL_GL_UNSIGNED_INT_5_9_9_9_REV:
-    case LOCAL_GL_UNSIGNED_INT_10F_11F_11F_REV:
-    case LOCAL_GL_UNSIGNED_INT_24_8:
-        bytesPerPixel = 4;
-        requiredDataType = js::Scalar::Uint32;
-        break;
-
-    case LOCAL_GL_UNSIGNED_INT:
-        bytesPerPixel = 4*channels;
-        requiredDataType = js::Scalar::Uint32;
-        break;
-
-    case LOCAL_GL_INT:
-        bytesPerPixel = 4*channels;
-        requiredDataType = js::Scalar::Int32;
-        break;
-
-    case LOCAL_GL_FLOAT:
-        bytesPerPixel = 4*channels;
-        requiredDataType = js::Scalar::Float32;
-        break;
-
-    case LOCAL_GL_HALF_FLOAT:
-    case LOCAL_GL_HALF_FLOAT_OES:
-        bytesPerPixel = 2*channels;
-        requiredDataType = js::Scalar::Uint16;
-        break;
-
-    default:
-        MOZ_CRASH("GFX: bad `type`");
+
+    if (mBoundPixelPackBuffer) {
+        ErrorInvalidOperation("readPixels: PIXEL_PACK_BUFFER must be null.");
+        return;
     }
 
     //////
 
+    if (pixels.IsNull()) {
+        ErrorInvalidValue("readPixels: null destination buffer");
+        return;
+    }
+
     const auto& view = pixels.Value();
 
+    //////
+
+    js::Scalar::Type reqScalarType;
+    if (!GetJSScalarFromGLType(type, &reqScalarType)) {
+        ErrorInvalidEnum("readPixels: Bad `type`.");
+        return;
+    }
+
+    const js::Scalar::Type dataScalarType = JS_GetArrayBufferViewType(view.Obj());
+    if (dataScalarType != reqScalarType) {
+        ErrorInvalidOperation("readPixels: `pixels` type does not match `type`.");
+        return;
+    }
+
+    //////
+
     // 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");
+    const auto dataLen = view.LengthAllowShared();
 
     if (!data) {
         ErrorOutOfMemory("readPixels: buffer storage is null. Did we run out of memory?");
         out_error.Throw(NS_ERROR_OUT_OF_MEMORY);
         return;
     }
 
+    ReadPixelsImpl(x, y, width, height, format, type, data, dataLen);
+}
+
+void
+WebGL2Context::ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
+                          GLenum type, WebGLsizeiptr offset, ErrorResult& out_error)
+{
+    if (!ReadPixels_SharedPrecheck(&out_error))
+        return;
+
+    if (!mBoundPixelPackBuffer) {
+        ErrorInvalidOperation("readPixels: PIXEL_PACK_BUFFER must not be null.");
+        return;
+    }
+
     //////
 
-    uint32_t rowStride;
-    uint32_t bytesNeeded;
-    if (!ValidatePackSize(funcName, width, height, bytesPerPixel, &rowStride,
-                          &bytesNeeded))
-    {
+    if (offset < 0) {
+        ErrorInvalidValue("readPixels: offset must not be negative.");
         return;
     }
 
-    if (bytesNeeded > bytesAvailable) {
-        ErrorInvalidOperation("readPixels: buffer too small");
+    {
+        const auto bytesPerType = webgl::BytesPerPixel({LOCAL_GL_RED, type});
+
+        if (offset % bytesPerType != 0) {
+            ErrorInvalidOperation("readPixels: `offset` must be divisible by the size"
+                                  " a `type` in bytes.");
+            return;
+        }
+    }
+
+    //////
+
+    const auto bytesAvailable = mBoundPixelPackBuffer->ByteLength();
+    const auto checkedBytesAfterOffset = CheckedUint32(bytesAvailable) - offset;
+
+    uint32_t bytesAfterOffset = 0;
+    if (checkedBytesAfterOffset.isValid()) {
+        bytesAfterOffset = checkedBytesAfterOffset.value();
+    }
+
+    ReadPixelsImpl(x, y, width, height, format, type, (void*)offset, bytesAfterOffset);
+}
+
+static bool
+ValidateReadPixelsFormatAndType(const webgl::FormatInfo* srcFormat,
+                                const webgl::PackingInfo& pi, gl::GLContext* gl,
+                                WebGLContext* webgl)
+{
+    // Check the format and type params to assure they are an acceptable pair (as per spec)
+    GLenum mainFormat;
+    GLenum mainType;
+
+    switch (srcFormat->componentType) {
+    case webgl::ComponentType::Float:
+        mainFormat = LOCAL_GL_RGBA;
+        mainType = LOCAL_GL_FLOAT;
+        break;
+
+    case webgl::ComponentType::UInt:
+        mainFormat = LOCAL_GL_RGBA_INTEGER;
+        mainType = LOCAL_GL_UNSIGNED_INT;
+        break;
+
+    case webgl::ComponentType::Int:
+        mainFormat = LOCAL_GL_RGBA_INTEGER;
+        mainType = LOCAL_GL_INT;
+        break;
+
+    case webgl::ComponentType::NormInt:
+    case webgl::ComponentType::NormUInt:
+        mainFormat = LOCAL_GL_RGBA;
+        mainType = LOCAL_GL_UNSIGNED_BYTE;
+        break;
+
+    default:
+        gfxCriticalNote << "Unhandled srcFormat->componentType: "
+                        << (uint32_t)srcFormat->componentType;
+        webgl->ErrorInvalidOperation("readPixels: Unhandled srcFormat->componentType."
+                                     " Please file a bug!");
+        return false;
+    }
+
+    if (pi.format == mainFormat && pi.type == mainType)
+        return true;
+
+    //////
+    // OpenGL ES 3.0.4 p194 - When the internal format of the rendering surface is
+    // RGB10_A2, a third combination of format RGBA and type UNSIGNED_INT_2_10_10_10_REV
+    // is accepted.
+
+    if (webgl->IsWebGL2() &&
+        srcFormat->effectiveFormat == webgl::EffectiveFormat::RGB10_A2 &&
+        pi.format == LOCAL_GL_RGBA &&
+        pi.type == LOCAL_GL_UNSIGNED_INT_2_10_10_10_REV)
+    {
+        return true;
+    }
+
+    //////
+    // OpenGL ES 2.0 $4.3.1 - IMPLEMENTATION_COLOR_READ_{TYPE/FORMAT} is a valid
+    // combination for glReadPixels()...
+
+    // So yeah, we are actually checking that these are valid as /unpack/ formats, instead
+    // of /pack/ formats here, but it should cover the INVALID_ENUM cases.
+    if (!webgl->mFormatUsage->AreUnpackEnumsValid(pi.format, pi.type)) {
+        webgl->ErrorInvalidEnum("readPixels: Bad format and/or type.");
+        return false;
+    }
+
+    // Only valid when pulled from:
+    // * GLES 2.0.25 p105:
+    //   "table 3.4, excluding formats LUMINANCE and LUMINANCE_ALPHA."
+    // * GLES 3.0.4 p193:
+    //   "table 3.2, excluding formats DEPTH_COMPONENT and DEPTH_STENCIL."
+    switch (pi.format) {
+    case LOCAL_GL_LUMINANCE:
+    case LOCAL_GL_LUMINANCE_ALPHA:
+    case LOCAL_GL_DEPTH_COMPONENT:
+    case LOCAL_GL_DEPTH_STENCIL:
+        webgl->ErrorInvalidEnum("readPixels: Invalid format: 0x%04x", pi.format);
+        return false;
+    }
+
+    MOZ_ASSERT(gl->IsCurrent());
+    if (gl->IsSupported(gl::GLFeature::ES2_compatibility)) {
+        const auto auxFormat = gl->GetIntAs<GLenum>(LOCAL_GL_IMPLEMENTATION_COLOR_READ_FORMAT);
+        const auto auxType = gl->GetIntAs<GLenum>(LOCAL_GL_IMPLEMENTATION_COLOR_READ_TYPE);
+
+        if (auxFormat && auxType &&
+            pi.format == auxFormat && pi.type == auxType)
+        {
+            return true;
+        }
+    }
+
+    //////
+
+    webgl->ErrorInvalidOperation("readPixels: Invalid format or type.");
+    return false;
+}
+
+void
+WebGLContext::ReadPixelsImpl(GLint x, GLint y, GLsizei rawWidth, GLsizei rawHeight,
+                             GLenum packFormat, GLenum packType, void* dest,
+                             uint32_t dataLen)
+{
+    if (rawWidth < 0 || rawHeight < 0) {
+        ErrorInvalidValue("readPixels: negative size passed");
         return;
     }
 
+    const uint32_t width(rawWidth);
+    const uint32_t height(rawHeight);
+
     //////
 
     MakeContextCurrent();
 
     const webgl::FormatUsageInfo* srcFormat;
     uint32_t srcWidth;
     uint32_t srcHeight;
     if (!ValidateCurFBForRead("readPixels", &srcFormat, &srcWidth, &srcHeight))
         return;
 
-    // Check the format and type params to assure they are an acceptable pair (as per spec)
-    auto srcType = srcFormat->format->componentType;
-    GLenum mainReadFormat;
-    GLenum mainReadType;
-    switch (srcType) {
-        case webgl::ComponentType::Float:
-            mainReadFormat = LOCAL_GL_RGBA;
-            mainReadType = LOCAL_GL_FLOAT;
-            break;
-        case webgl::ComponentType::UInt:
-            mainReadFormat = LOCAL_GL_RGBA_INTEGER;
-            mainReadType = LOCAL_GL_UNSIGNED_INT;
-            break;
-        case webgl::ComponentType::Int:
-            mainReadFormat = LOCAL_GL_RGBA_INTEGER;
-            mainReadType = LOCAL_GL_INT;
-            break;
-        default:
-            mainReadFormat = LOCAL_GL_RGBA;
-            mainReadType = LOCAL_GL_UNSIGNED_BYTE;
-            break;
+    //////
+
+    const webgl::PackingInfo pi = {packFormat, packType};
+    if (!ValidateReadPixelsFormatAndType(srcFormat->format, pi, gl, this))
+        return;
+
+    uint8_t bytesPerPixel;
+    if (!webgl::GetBytesPerPixel(pi, &bytesPerPixel)) {
+        ErrorInvalidOperation("readPixels: Unsupported format and type.");
+        return;
     }
 
-    GLenum auxReadFormat = mainReadFormat;
-    GLenum auxReadType = mainReadType;
-
-    // OpenGL ES 2.0 $4.3.1 - IMPLEMENTATION_COLOR_READ_{TYPE/FORMAT} is a valid
-    // combination for glReadPixels().
-    if (gl->IsSupported(gl::GLFeature::ES2_compatibility)) {
-        gl->fGetIntegerv(LOCAL_GL_IMPLEMENTATION_COLOR_READ_FORMAT,
-                         reinterpret_cast<GLint*>(&auxReadFormat));
-        gl->fGetIntegerv(LOCAL_GL_IMPLEMENTATION_COLOR_READ_TYPE,
-                         reinterpret_cast<GLint*>(&auxReadType));
+    //////
+
+    uint32_t rowStride;
+    uint32_t bytesNeeded;
+    if (!ValidatePackSize("readPixels", width, height, bytesPerPixel, &rowStride,
+                          &bytesNeeded))
+    {
+        return;
     }
 
-    const bool mainMatches = (format == mainReadFormat && type == mainReadType);
-    const bool auxMatches = (format == auxReadFormat && type == auxReadType);
-    bool isValid = mainMatches || auxMatches;
-
-    // OpenGL ES 3.0.4 p194 - When the internal format of the rendering surface is
-    // RGB10_A2, a third combination of format RGBA and type UNSIGNED_INT_2_10_10_10_REV
-    // is accepted.
-    if (srcFormat->format->effectiveFormat == webgl::EffectiveFormat::RGB10_A2 &&
-        format == LOCAL_GL_RGBA &&
-        type == LOCAL_GL_UNSIGNED_INT_2_10_10_10_REV)
-    {
-        isValid = true;
+    if (bytesNeeded > dataLen) {
+        ErrorInvalidOperation("readPixels: buffer too small");
+        return;
     }
 
-    if (!isValid)
-        return ErrorInvalidOperation("readPixels: Invalid format/type pair");
-
+    ////////////////
     // Now that the errors are out of the way, on to actually reading!
 
     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)) {
-        DoReadPixelsAndConvert(x, y, width, height, format, type, data, auxReadFormat,
-                               auxReadType);
+        DoReadPixelsAndConvert(srcFormat->format, x, y, width, height, packFormat,
+                               packType, dest);
         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."
@@ -1683,35 +1784,36 @@ WebGLContext::ReadPixels(GLint x, GLint 
     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, mPixelStore_PackSkipPixels + 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);
+        DoReadPixelsAndConvert(srcFormat->format, readX, readY, rwWidth, rwHeight,
+                               packFormat, packType, dest);
 
         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 + writeX * bytesPerPixel;
+        uint8_t* row = (uint8_t*)dest + writeX * bytesPerPixel;
         row += writeY * rowStride;
         for (uint32_t j = 0; j < rwHeight; j++) {
-            DoReadPixelsAndConvert(readX, readY+j, rwWidth, 1, format, type, row,
-                                   auxReadFormat, auxReadType);
+            DoReadPixelsAndConvert(srcFormat->format, readX, readY+j, rwWidth, 1,
+                                   packFormat, packType, row);
             row += rowStride;
         }
     }
 }
 
 void
 WebGLContext::RenderbufferStorage_base(const char* funcName, GLenum target,
                                        GLsizei samples, GLenum internalFormat,
--- a/dom/canvas/WebGLFormats.cpp
+++ b/dom/canvas/WebGLFormats.cpp
@@ -435,34 +435,38 @@ GetFormat(EffectiveFormat format)
 //////////////////////////////////////////////////////////////////////////////////////////
 
 const FormatInfo*
 FormatInfo::GetCopyDecayFormat(UnsizedFormat uf) const
 {
     return FindOrNull(this->copyDecayFormats, uf);
 }
 
-uint8_t
-BytesPerPixel(const PackingInfo& packing)
+bool
+GetBytesPerPixel(const PackingInfo& packing, uint8_t* const out_bytes)
 {
     uint8_t bytesPerChannel;
+
     switch (packing.type) {
     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:
-        return 2;
+        *out_bytes = 2;
+        return true;
 
     case LOCAL_GL_UNSIGNED_INT_10F_11F_11F_REV:
     case LOCAL_GL_UNSIGNED_INT_2_10_10_10_REV:
     case LOCAL_GL_UNSIGNED_INT_24_8:
     case LOCAL_GL_UNSIGNED_INT_5_9_9_9_REV:
-        return 4;
+        *out_bytes = 4;
+        return true;
 
     case LOCAL_GL_FLOAT_32_UNSIGNED_INT_24_8_REV:
-        return 8;
+        *out_bytes = 8;
+        return true;
 
     // Alright, that's all the fixed-size unpackTypes.
 
     case LOCAL_GL_BYTE:
     case LOCAL_GL_UNSIGNED_BYTE:
         bytesPerChannel = 1;
         break;
 
@@ -475,46 +479,68 @@ BytesPerPixel(const PackingInfo& packing
 
     case LOCAL_GL_INT:
     case LOCAL_GL_UNSIGNED_INT:
     case LOCAL_GL_FLOAT:
         bytesPerChannel = 4;
         break;
 
     default:
-        MOZ_CRASH("GFX: invalid PackingInfo");
+        return false;
     }
 
     uint8_t channels;
+
     switch (packing.format) {
+    case LOCAL_GL_RED:
+    case LOCAL_GL_RED_INTEGER:
+    case LOCAL_GL_LUMINANCE:
+    case LOCAL_GL_ALPHA:
+    case LOCAL_GL_DEPTH_COMPONENT:
+        channels = 1;
+        break;
+
     case LOCAL_GL_RG:
     case LOCAL_GL_RG_INTEGER:
     case LOCAL_GL_LUMINANCE_ALPHA:
         channels = 2;
         break;
 
     case LOCAL_GL_RGB:
     case LOCAL_GL_RGB_INTEGER:
+    case LOCAL_GL_SRGB:
         channels = 3;
         break;
 
+    case LOCAL_GL_BGRA:
     case LOCAL_GL_RGBA:
     case LOCAL_GL_RGBA_INTEGER:
+    case LOCAL_GL_SRGB_ALPHA:
         channels = 4;
         break;
 
     default:
-        channels = 1;
-        break;
+        return false;
     }
 
-    return bytesPerChannel * channels;
+    *out_bytes = bytesPerChannel * channels;
+    return true;
 }
 
+uint8_t
+BytesPerPixel(const PackingInfo& packing)
+{
+    uint8_t ret;
+    if (MOZ_LIKELY(GetBytesPerPixel(packing, &ret)))
+        return ret;
 
+    gfxCriticalError() << "Bad `packing`: " << gfx::hexa(packing.format) << ", "
+                       << gfx::hexa(packing.type);
+    MOZ_CRASH("Bad `packing`.");
+}
 
 //////////////////////////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////////////////////////
--- a/dom/canvas/WebGLFormats.h
+++ b/dom/canvas/WebGLFormats.h
@@ -249,16 +249,17 @@ struct DriverUnpackInfo
         return {unpackFormat, unpackType};
     }
 };
 
 //////////////////////////////////////////////////////////////////////////////////////////
 
 const FormatInfo* GetFormat(EffectiveFormat format);
 uint8_t BytesPerPixel(const PackingInfo& packing);
+bool GetBytesPerPixel(const PackingInfo& packing, uint8_t* const out_bytes);
 /*
 GLint ComponentSize(const FormatInfo* format, GLenum component);
 GLenum ComponentType(const FormatInfo* format);
 */
 ////////////////////////////////////////
 
 struct FormatUsageInfo
 {
--- a/gfx/gl/GLContext.h
+++ b/gfx/gl/GLContext.h
@@ -1223,16 +1223,24 @@ private:
 public:
 
     void fGetIntegerv(GLenum pname, GLint* params);
 
     void GetUIntegerv(GLenum pname, GLuint* params) {
         fGetIntegerv(pname, reinterpret_cast<GLint*>(params));
     }
 
+    template<typename T>
+    T GetIntAs(GLenum pname) {
+        static_assert(sizeof(T) == sizeof(GLint), "Invalid T.");
+        T ret = 0;
+        fGetIntegerv(pname, (GLint*)&ret);
+        return ret;
+    }
+
     void fGetFloatv(GLenum pname, GLfloat* params) {
         BEFORE_GL_CALL;
         mSymbols.fGetFloatv(pname, params);
         AFTER_GL_CALL;
     }
 
     void fGetBooleanv(GLenum pname, realGLboolean* params) {
         BEFORE_GL_CALL;