Bug 1313541 - ReadPixels impl. - r=ethlin, a=gchang
authorJeff Gilbert (:jgilbert) <jgilbert@mozilla.com>
Mon, 10 Oct 2016 15:34:18 -0700
changeset 439973 a99e419681e5c1ea4cf371015785ce136c82a53f
parent 439972 b80438cb08254aa968c9406b9db6787b9a3f501b
child 439974 4031945696c24b704d132cddf96990ccd1eb8b15
push id36143
push userjkingston@mozilla.com
push dateWed, 16 Nov 2016 22:20:38 +0000
reviewersethlin, gchang
bugs1313541
milestone51.0a2
Bug 1313541 - ReadPixels impl. - r=ethlin, a=gchang MozReview-Commit-ID: HInjyzjTdEY
dom/canvas/WebGL2Context.h
dom/canvas/WebGLContext.h
dom/canvas/WebGLContextGL.cpp
--- a/dom/canvas/WebGL2Context.h
+++ b/dom/canvas/WebGL2Context.h
@@ -48,25 +48,16 @@ public:
 
 private:
     template<typename BufferT>
     void GetBufferSubDataT(GLenum target, GLintptr offset, const BufferT& data);
 
 public:
     void GetBufferSubData(GLenum target, GLintptr offset,
                           const dom::ArrayBufferView& dstData);
-    void ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
-                    GLenum type, WebGLsizeiptr offset, ErrorResult& out_error);
-
-    void ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
-                    GLenum type, const dom::ArrayBufferView& pixels,
-                    ErrorResult& out_error)
-    {
-        WebGLContext::ReadPixels(x, y, width, height, format, type, pixels, out_error);
-    }
 
     // -------------------------------------------------------------------------
     // Framebuffer objects - WebGL2ContextFramebuffers.cpp
 
     void BlitFramebuffer(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1,
                          GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1,
                          GLbitfield mask, GLenum filter);
     void FramebufferTextureLayer(GLenum target, GLenum attachment, WebGLTexture* texture, GLint level, GLint layer);
--- a/dom/canvas/WebGLContext.h
+++ b/dom/canvas/WebGLContext.h
@@ -580,21 +580,26 @@ public:
                     GLenum type, const dom::Nullable<dom::ArrayBufferView>& maybeView,
                     ErrorResult& rv)
     {
         const char funcName[] = "readPixels";
         if (maybeView.IsNull()) {
             ErrorInvalidValue("%s: `pixels` must not be null.", funcName);
             return;
         }
-        ReadPixels(x, y, width, height, format, type, maybeView.Value(), rv);
+        ReadPixels(x, y, width, height, format, type, maybeView.Value(), 0, rv);
     }
 
     void ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
-                    GLenum type, const dom::ArrayBufferView& pixels, ErrorResult& rv);
+                    GLenum type, WebGLsizeiptr offset, ErrorResult& out_error);
+
+    void ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
+                    GLenum type, const dom::ArrayBufferView& dstData, GLuint dstOffset,
+                    ErrorResult& out_error);
+
     void RenderbufferStorage(GLenum target, GLenum internalFormat,
                              GLsizei width, GLsizei height);
 protected:
     void RenderbufferStorage_base(const char* funcName, GLenum target,
                                   GLsizei samples, GLenum internalformat,
                                   GLsizei width, GLsizei height);
 public:
     void SampleCoverage(GLclampf value, WebGLboolean invert);
--- a/dom/canvas/WebGLContextGL.cpp
+++ b/dom/canvas/WebGLContextGL.cpp
@@ -1424,61 +1424,57 @@ WebGLContext::ValidatePackSize(const cha
 
     *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::ArrayBufferView& view,
-                         ErrorResult& out_error)
+                         GLenum type, const dom::ArrayBufferView& dstView,
+                         GLuint dstElemOffset, ErrorResult& out_error)
 {
+    const char funcName[] = "readPixels";
     if (!ReadPixels_SharedPrecheck(&out_error))
         return;
 
     if (mBoundPixelPackBuffer) {
-        ErrorInvalidOperation("readPixels: PIXEL_PACK_BUFFER must be null.");
+        ErrorInvalidOperation("%s: PIXEL_PACK_BUFFER must be null.", funcName);
         return;
     }
 
-    //////
+    ////
 
     js::Scalar::Type reqScalarType;
     if (!GetJSScalarFromGLType(type, &reqScalarType)) {
-        ErrorInvalidEnum("readPixels: Bad `type`.");
+        ErrorInvalidEnum("%s: Bad `type`.", funcName);
         return;
     }
 
-    const js::Scalar::Type dataScalarType = JS_GetArrayBufferViewType(view.Obj());
-    if (dataScalarType != reqScalarType) {
-        ErrorInvalidOperation("readPixels: `pixels` type does not match `type`.");
+    const auto& viewElemType = dstView.Type();
+    if (viewElemType != reqScalarType) {
+        ErrorInvalidOperation("%s: `pixels` type does not match `type`.", funcName);
         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();
-    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);
+    ////
+
+    uint8_t* bytes;
+    size_t byteLen;
+    if (!ValidateArrayBufferView(funcName, dstView, dstElemOffset, 0, &bytes, &byteLen))
         return;
-    }
-
-    ReadPixelsImpl(x, y, width, height, format, type, data, dataLen);
+
+    ////
+
+    ReadPixelsImpl(x, y, width, height, format, type, bytes, byteLen);
 }
 
 void
-WebGL2Context::ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
-                          GLenum type, WebGLsizeiptr offset, ErrorResult& out_error)
+WebGLContext::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;
     }