Bug 1250710 - Add PACK PBO support. - r=jrmuizel
authorJeff Gilbert <jgilbert@mozilla.com>
Mon, 04 Jul 2016 20:35:20 -0700
changeset 305267 385d885ca02fbc48c66f1ce0941b176e4d0e28bb
parent 305266 c2ee198a1c719a61ed7666a20ab8883d6f7ddaa9
child 305268 057ec3ca368097a2594d34cef36e7151353297c9
push id79535
push userjgilbert@mozilla.com
push dateMon, 18 Jul 2016 04:46:14 +0000
treeherdermozilla-inbound@b658098634b8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1250710
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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;