WebGL 2: add readPixels API to read pixels into pixel pack buffer

Source/modules/webgl/WebGLRenderingContextBase.cpp

Index: Source/modules/webgl/WebGLRenderingContextBase.cpp
diff --git a/Source/modules/webgl/WebGLRenderingContextBase.cpp b/Source/modules/webgl/WebGLRenderingContextBase.cpp
index 53aaea0151620316a369e2c80916623f0b6df6ab..c04cf510b7030195219e387d5673d41dff9deb29 100644
--- a/Source/modules/webgl/WebGLRenderingContextBase.cpp
+++ b/Source/modules/webgl/WebGLRenderingContextBase.cpp
@@ -222,33 +222,6 @@ void WebGLRenderingContextBase::willDestroyContext(WebGLRenderingContextBase* co
 
 namespace {
 
-// ScopedDrawingBufferBinder is used for ReadPixels/CopyTexImage2D/CopySubImage2D to read from
-// a multisampled DrawingBuffer. In this situation, we need to blit to a single sampled buffer
-// for reading, during which the bindings could be changed and need to be recovered.
-class ScopedDrawingBufferBinder {
-    STACK_ALLOCATED();
-public:
-    ScopedDrawingBufferBinder(DrawingBuffer* drawingBuffer, WebGLFramebuffer* framebufferBinding)
-        : m_drawingBuffer(drawingBuffer)
-        , m_readFramebufferBinding(framebufferBinding)
-    {
-        // Commit DrawingBuffer if needed (e.g., for multisampling)
-        if (!m_readFramebufferBinding && m_drawingBuffer)
-            m_drawingBuffer->commit();
-    }
-
-    ~ScopedDrawingBufferBinder()
-    {
-        // Restore DrawingBuffer if needed
-        if (!m_readFramebufferBinding && m_drawingBuffer)
-            m_drawingBuffer->restoreFramebufferBindings();
-    }
-
-private:
-    DrawingBuffer* m_drawingBuffer;
-    Member<WebGLFramebuffer> m_readFramebufferBinding;
-};
-
 GLint clamp(GLint value, GLint min, GLint max)
 {
     if (value < min)
@@ -1608,6 +1581,8 @@ void WebGLRenderingContextBase::bufferDataImpl(GLenum target, long long size, co
     if (!validateValueFitNonNegInt32("bufferData", "size", size))
         return;
 
+    buffer->setSize(size);
+
     webContext()->bufferData(target, static_cast<GLsizeiptr>(size), data, usage);
 }
 
@@ -3729,33 +3704,16 @@ DOMArrayBufferView::ViewType WebGLRenderingContextBase::readPixelsExpectedArrayB
     }
 }
 
-void WebGLRenderingContextBase::readPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, DOMArrayBufferView* pixels)
+bool WebGLRenderingContextBase::validateReadPixelsFuncParameters(GLsizei width, GLsizei height, GLenum format, GLenum type, long long bufferSize)
 {
-    if (isContextLost())
-        return;
-    // Due to WebGL's same-origin restrictions, it is not possible to
-    // taint the origin using the WebGL API.
-    ASSERT(canvas()->originClean());
-    // Validate input parameters.
-    if (!pixels) {
-        synthesizeGLError(GL_INVALID_VALUE, "readPixels", "no destination ArrayBufferView");
-        return;
-    }
     if (!validateReadPixelsFormatAndType(format, type))
-        return;
-    GLenum readBufferInternalFormat = 0, readBufferType = 0;
+        return false;
     WebGLFramebuffer* readFramebufferBinding = nullptr;
+    GLenum readBufferInternalFormat = 0, readBufferType = 0;
     if (!validateReadBufferAndGetInfo("readPixels", readFramebufferBinding, &readBufferInternalFormat, &readBufferType))
-        return;
+        return false;
     if (!validateReadPixelsFormatTypeCombination(format, type, readBufferInternalFormat, readBufferType))
-        return;
-
-    DOMArrayBufferView::ViewType expectedViewType = readPixelsExpectedArrayBufferViewType(type);
-    // Validate array type against pixel type.
-    if (pixels->type() != expectedViewType) {
-        synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "ArrayBufferView was the wrong type for the pixel format");
-        return;
-    }
+        return false;
 
     // Calculate array size, taking into consideration of PACK_ALIGNMENT.
     unsigned totalBytesRequired = 0;
@@ -3763,22 +3721,54 @@ void WebGLRenderingContextBase::readPixels(GLint x, GLint y, GLsizei width, GLsi
     GLenum error = WebGLImageConversion::computeImageSizeInBytes(format, type, width, height, m_packAlignment, &totalBytesRequired, &padding);
     if (error != GL_NO_ERROR) {
         synthesizeGLError(error, "readPixels", "invalid dimensions");
-        return;
+        return false;
     }
-    if (pixels->byteLength() < totalBytesRequired) {
-        synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "ArrayBufferView not large enough for dimensions");
-        return;
+    if (bufferSize < static_cast<long long>(totalBytesRequired)) {
+        synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "buffer is not large enough for dimensions");
+        return false;
     }
+    return true;
+}
+
+void WebGLRenderingContextBase::readPixelsImpl(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, void* data, long long bufferSize)
+{
+    // Due to WebGL's same-origin restrictions, it is not possible to
+    // taint the origin using the WebGL API.
+    ASSERT(canvas()->originClean());
+
+    if (!validateReadPixelsFuncParameters(width, height, format, type, bufferSize))
+        return;
 
     clearIfComposited();
-    void* data = pixels->baseAddress();
 
+    GLenum target = isWebGL2OrHigher() ? GL_READ_FRAMEBUFFER : GL_FRAMEBUFFER;
+    WebGLFramebuffer* readFramebufferBinding = getFramebufferBinding(target);
     {
         ScopedDrawingBufferBinder binder(drawingBuffer(), readFramebufferBinding);
         webContext()->readPixels(x, y, width, height, format, type, data);
     }
 }
 
+void WebGLRenderingContextBase::readPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, DOMArrayBufferView* pixels)
+{
+    if (isContextLost())
+        return;
+    // Validate input parameters.
+    if (!pixels) {
+        synthesizeGLError(GL_INVALID_VALUE, "readPixels", "no destination ArrayBufferView");
+        return;
+    }
+
+    DOMArrayBufferView::ViewType expectedViewType = readPixelsExpectedArrayBufferViewType(type);

[yunchao, 2015/09/01 02:49:57]

readPixelsExpectedArrayBufferViewType(type) supposes that the parameter 'type'
is valid. So putting it before readPixelsImpl (who call
validateReadPixelsFuncParameters to validate format / type) is not appropriate.
It leads to failures in conformance test.

So I removed the function readPixelsImpl() and did a code refactoring in the
latest patch set.

+    // Validate array type against pixel type.
+    if (pixels->type() != expectedViewType) {
+        synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "ArrayBufferView was the wrong type for the pixel format");
+        return;
+    }
+
+    readPixelsImpl(x, y, width, height, format, type, pixels->baseAddress(), static_cast<long long>(pixels->byteLength()));
+}
+
 void WebGLRenderingContextBase::renderbufferStorageImpl(
     GLenum target, GLsizei samples, GLenum internalformat, GLsizei width, GLsizei height,
     const char* functionName)