Move ReadPixels to the Framebuffer object and Impl.

src/libANGLE/Context.cpp

@@ -845,17 +845,6 @@ void Context::getIntegerv(GLenum pname, GLint *params)
       case GL_MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS:    *params = mCaps.maxTransformFeedbackSeparateComponents;    break;
       case GL_NUM_COMPRESSED_TEXTURE_FORMATS:           *params = mCaps.compressedTextureFormats.size();                break;
       case GL_MAX_SAMPLES_ANGLE:                        *params = mExtensions.maxSamples;                               break;
-      case GL_IMPLEMENTATION_COLOR_READ_TYPE:
-      case GL_IMPLEMENTATION_COLOR_READ_FORMAT:
-        {
-            GLenum internalFormat, format, type;
-            getCurrentReadFormatType(&internalFormat, &format, &type);
-            if (pname == GL_IMPLEMENTATION_COLOR_READ_FORMAT)
-                *params = format;
-            else
-                *params = type;
-        }
-        break;
       case GL_MAX_VIEWPORT_DIMS:
         {
             params[0] = mCaps.maxViewportWidth;
@@ -1231,12 +1220,6 @@ bool Context::getIndexedQueryParameterInfo(GLenum target, GLenum *type, unsigned
     return false;
 }
 
-Error Context::readPixels(GLint x, GLint y, GLsizei width, GLsizei height,
-                          GLenum format, GLenum type, GLsizei *bufSize, void* pixels)
-{
-    return mRenderer->readPixels(getData(), x, y, width, height, format, type, bufSize, pixels);
-}
-
 Error Context::drawArrays(GLenum mode, GLint first, GLsizei count, GLsizei instances)
 {
     return mRenderer->drawArrays(getData(), mode, first, count, instances);
@@ -1332,22 +1315,6 @@ const Extensions &Context::getExtensions() const
     return mExtensions;
 }
 
-void Context::getCurrentReadFormatType(GLenum *internalFormat, GLenum *format, GLenum *type)
-{
-    Framebuffer *framebuffer = mState.getReadFramebuffer();
-    ASSERT(framebuffer && framebuffer->checkStatus(getData()) == GL_FRAMEBUFFER_COMPLETE);
-
-    FramebufferAttachment *attachment = framebuffer->getReadColorbuffer();
-    ASSERT(attachment);
-
-    GLenum actualFormat = attachment->getActualFormat();
-    const InternalFormat &actualFormatInfo = GetInternalFormatInfo(actualFormat);
-
-    *internalFormat = actualFormat;
-    *format = actualFormatInfo.format;
-    *type = actualFormatInfo.type;
-}
-
 void Context::detachTexture(GLuint texture)
 {
     // Simple pass-through to State's detachTexture method, as textures do not require

src/libANGLE/Context.h

@@ -175,7 +175,6 @@ class Context
     bool getQueryParameterInfo(GLenum pname, GLenum *type, unsigned int *numParams);
     bool getIndexedQueryParameterInfo(GLenum target, GLenum *type, unsigned int *numParams);
 
-    Error readPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, GLsizei *bufSize, void* pixels);
     Error drawArrays(GLenum mode, GLint first, GLsizei count, GLsizei instances);
     Error drawElements(GLenum mode, GLsizei count, GLenum type,
                        const GLvoid *indices, GLsizei instances,
@@ -200,8 +199,6 @@ class Context
     const std::string &getExtensionString(size_t idx) const;
     size_t getExtensionStringCount() const;
 
-    void getCurrentReadFormatType(GLenum *internalFormat, GLenum *format, GLenum *type);
-
     Error blitFramebuffer(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1,
                           GLbitfield mask, GLenum filter);
 

src/libANGLE/Framebuffer.cpp

@@ -488,6 +488,21 @@ Error Framebuffer::clearBufferfi(const State &state, GLenum buffer, GLint drawbu
     return mImpl->clearBufferfi(state, buffer, drawbuffer, depth, stencil);
 }
 
+GLenum Framebuffer::getImplementationColorReadFormat() const
+{
+    return mImpl->getImplementationColorReadFormat();
+}
+
+GLenum Framebuffer::getImplementationColorReadType() const
+{
+    return mImpl->getImplementationColorReadType();
+}
+
+Error Framebuffer::readPixels(const gl::State &state, const gl::Rectangle &area, GLenum format, GLenum type, GLvoid *pixels) const
+{
+    return mImpl->readPixels(state, area, format, type, pixels);
+}
+
 int Framebuffer::getSamples(const gl::Data &data) const
 {
     if (checkStatus(data) == GL_FRAMEBUFFER_COMPLETE)

src/libANGLE/Framebuffer.h

@@ -94,6 +94,10 @@ class Framebuffer
     Error clearBufferiv(const State &state, GLenum buffer, GLint drawbuffer, const GLint *values);
     Error clearBufferfi(const State &state, GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil);
 
+    GLenum getImplementationColorReadFormat() const;
+    GLenum getImplementationColorReadType() const;
+    Error readPixels(const gl::State &state, const gl::Rectangle &area, GLenum format, GLenum type, GLvoid *pixels) const;
+
     // Use this method to retrieve the color buffer map when doing rendering.
     // It will apply a workaround for poor shader performance on some systems
     // by compacting the list to skip NULL values.

src/libANGLE/State.cpp

@@ -1247,6 +1247,8 @@ void State::getIntegerv(const gl::Data &data, GLenum pname, GLint *params)
       case GL_STENCIL_WRITEMASK:                        *params = clampToInt(mDepthStencil.stencilWritemask);     break;
       case GL_STENCIL_BACK_WRITEMASK:                   *params = clampToInt(mDepthStencil.stencilBackWritemask); break;
       case GL_STENCIL_CLEAR_VALUE:                      *params = mStencilClearValue;                             break;
+      case GL_IMPLEMENTATION_COLOR_READ_TYPE:           *params = mReadFramebuffer->getImplementationColorReadType();   break;
+      case GL_IMPLEMENTATION_COLOR_READ_FORMAT:         *params = mReadFramebuffer->getImplementationColorReadFormat(); break;
       case GL_SAMPLE_BUFFERS:
       case GL_SAMPLES:
         {

src/libANGLE/renderer/FramebufferImpl.h

@@ -57,6 +57,10 @@ class FramebufferImpl
     virtual gl::Error clearBufferiv(const gl::State &state, GLenum buffer, GLint drawbuffer, const GLint *values) = 0;
     virtual gl::Error clearBufferfi(const gl::State &state, GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil) = 0;
 
+    virtual GLenum getImplementationColorReadFormat() const = 0;
+    virtual GLenum getImplementationColorReadType() const = 0;
+    virtual gl::Error readPixels(const gl::State &state, const gl::Rectangle &area, GLenum format, GLenum type, GLvoid *pixels) const = 0;
+
     virtual GLenum checkStatus() const = 0;
 };
 

src/libANGLE/renderer/Renderer.h

@@ -84,9 +84,6 @@ class Renderer
                                    const GLvoid *indices, GLsizei instances,
                                    const RangeUI &indexRange) = 0;
 
-    virtual gl::Error readPixels(const gl::Data &data, GLint x, GLint y, GLsizei width, GLsizei height,
-                                 GLenum format, GLenum type, GLsizei *bufSize, void* pixels) = 0;
-
     virtual gl::Error blitFramebuffer(const gl::Data &data,
                                       GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1,
                                       GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1,

src/libANGLE/renderer/d3d/FramebufferD3D.cpp

@@ -11,6 +11,7 @@
 #include "libANGLE/renderer/d3d/RendererD3D.h"
 #include "libANGLE/renderer/d3d/RenderbufferD3D.h"
 #include "libANGLE/renderer/RenderTarget.h"
+#include "libANGLE/formatutils.h"
 #include "libANGLE/FramebufferAttachment.h"
 
 namespace rx
@@ -69,7 +70,8 @@ FramebufferD3D::FramebufferD3D(RendererD3D *renderer)
       mColorBuffers(renderer->getRendererCaps().maxColorAttachments),
       mDepthbuffer(nullptr),
       mStencilbuffer(nullptr),
-      mDrawBuffers(renderer->getRendererCaps().maxDrawBuffers)
+      mDrawBuffers(renderer->getRendererCaps().maxDrawBuffers),
+      mReadBuffer(GL_COLOR_ATTACHMENT0)
 {
     ASSERT(mRenderer != nullptr);
 
@@ -114,6 +116,7 @@ void FramebufferD3D::setDrawBuffers(size_t count, const GLenum *buffers)
 
 void FramebufferD3D::setReadBuffer(GLenum buffer)
 {
+    mReadBuffer = buffer;
 }
 
 gl::Error FramebufferD3D::invalidate(size_t, const GLenum *)
@@ -208,6 +211,47 @@ gl::Error FramebufferD3D::clearBufferfi(const gl::State &state, GLenum buffer, G
     return clear(state, clearParams);
 }
 
+GLenum FramebufferD3D::getImplementationColorReadFormat() const
+{
+    // Will require more logic if glReadBuffers is supported
+    ASSERT(mReadBuffer == GL_COLOR_ATTACHMENT0 || mReadBuffer == GL_BACK);
+
+    if (mColorBuffers[0] == nullptr)
+    {
+        return GL_NONE;
+    }
+
+    GLenum actualFormat = mColorBuffers[0]->getActualFormat();
+    const gl::InternalFormat &actualFormatInfo = gl::GetInternalFormatInfo(actualFormat);
+
+    return actualFormatInfo.format;
+}
+
+GLenum FramebufferD3D::getImplementationColorReadType() const
+{
+    // Will require more logic if glReadBuffers is supported
+    ASSERT(mReadBuffer == GL_COLOR_ATTACHMENT0 || mReadBuffer == GL_BACK);
+
+    if (mColorBuffers[0] == nullptr)
+    {
+        return GL_NONE;
+    }
+
+    GLenum actualFormat = mColorBuffers[0]->getActualFormat();
+    const gl::InternalFormat &actualFormatInfo = gl::GetInternalFormatInfo(actualFormat);
+
+    return actualFormatInfo.type;
+}
+
+gl::Error FramebufferD3D::readPixels(const gl::State &state, const gl::Rectangle &area, GLenum format, GLenum type, GLvoid *pixels) const
+{
+    GLenum sizedInternalFormat = gl::GetSizedInternalFormat(format, type);
+    const gl::InternalFormat &sizedFormatInfo = gl::GetInternalFormatInfo(sizedInternalFormat);
+    GLuint outputPitch = sizedFormatInfo.computeRowPitch(type, area.width, state.getPackAlignment());
+
+    return readPixels(area, format, type, outputPitch, state.getPackState(), reinterpret_cast<uint8_t*>(pixels));
+}
+
 GLenum FramebufferD3D::checkStatus() const
 {
     // D3D11 does not allow for overlapping RenderTargetViews, so ensure uniqueness

src/libANGLE/renderer/d3d/FramebufferD3D.h

@@ -12,11 +12,13 @@
 #include "libANGLE/renderer/FramebufferImpl.h"
 
 #include <vector>
+#include <cstdint>
 
 namespace gl
 {
 struct ClearParameters;
 class FramebufferAttachment;
+struct PixelPackState;
 }
 
 namespace rx
@@ -67,6 +69,10 @@ class FramebufferD3D : public FramebufferImpl
     gl::Error clearBufferiv(const gl::State &state, GLenum buffer, GLint drawbuffer, const GLint *values) override;
     gl::Error clearBufferfi(const gl::State &state, GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil) override;
 
+    GLenum getImplementationColorReadFormat() const override;
+    GLenum getImplementationColorReadType() const override;
+    gl::Error readPixels(const gl::State &state, const gl::Rectangle &area, GLenum format, GLenum type, GLvoid *pixels) const override;
+
     GLenum checkStatus() const override;
 
   protected:
@@ -75,12 +81,15 @@ class FramebufferD3D : public FramebufferImpl
     const gl::FramebufferAttachment *mStencilbuffer;
 
     std::vector<GLenum> mDrawBuffers;
+    GLenum mReadBuffer;
 
   private:
     RendererD3D *const mRenderer;
 
     virtual gl::Error clear(const gl::State &state, const gl::ClearParameters &clearParams) = 0;
 
+    virtual gl::Error readPixels(const gl::Rectangle &area, GLenum format, GLenum type, size_t outputPitch,
+                                 const gl::PixelPackState &pack, uint8_t *pixels) const = 0;
 };
 
 gl::Error GetAttachmentRenderTarget(const gl::FramebufferAttachment *attachment, RenderTarget **outRT);

src/libANGLE/renderer/d3d/RendererD3D.cpp

@@ -676,19 +676,6 @@ gl::Error RendererD3D::blitFramebuffer(const gl::Data &data,
     return gl::Error(GL_NO_ERROR);
 }
 
-gl::Error RendererD3D::readPixels(const gl::Data &data, GLint x, GLint y, GLsizei width, GLsizei height,
-                                  GLenum format, GLenum type, GLsizei *bufSize, void* pixels)
-{
-    const gl::Framebuffer *framebuffer = data.state->getReadFramebuffer();
-
-    GLenum sizedInternalFormat = gl::GetSizedInternalFormat(format, type);
-    const gl::InternalFormat &sizedFormatInfo = gl::GetInternalFormatInfo(sizedInternalFormat);
-    GLuint outputPitch = sizedFormatInfo.computeRowPitch(type, width, data.state->getPackAlignment());
-
-    return readPixels(framebuffer, x, y, width, height, format, type, outputPitch, data.state->getPackState(),
-                      reinterpret_cast<uint8_t*>(pixels));
-}
-
 bool RendererD3D::isDeviceLost() const
 {
     return mDeviceLost;

src/libANGLE/renderer/d3d/RendererD3D.h

@@ -59,9 +59,6 @@ class RendererD3D : public Renderer
                            const GLvoid *indices, GLsizei instances,
                            const RangeUI &indexRange) override;
 
-    gl::Error readPixels(const gl::Data &data, GLint x, GLint y, GLsizei width, GLsizei height,
-                         GLenum format, GLenum type, GLsizei *bufSize, void* pixels) override;
-
     gl::Error blitFramebuffer(const gl::Data &data,
                               GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1,
                               GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1,
@@ -124,9 +121,6 @@ class RendererD3D : public Renderer
     virtual gl::Error copyImage2DArray(gl::Framebuffer *framebuffer, const gl::Rectangle &sourceRect, GLenum destFormat,
                                        GLint xoffset, GLint yoffset, GLint zOffset, TextureStorage *storage, GLint level) = 0;
 
-    virtual gl::Error readPixels(const gl::Framebuffer *framebuffer, GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
-                                 GLenum type, GLuint outputPitch, const gl::PixelPackState &pack, uint8_t *pixels) = 0;
-
     // RenderTarget creation
     virtual gl::Error createRenderTarget(int width, int height, GLenum format, GLsizei samples, RenderTarget **outRT) = 0;
 

src/libANGLE/renderer/d3d/d3d11/Framebuffer11.cpp

@@ -7,9 +7,12 @@
 // Framebuffer11.cpp: Implements the Framebuffer11 class.
 
 #include "libANGLE/renderer/d3d/d3d11/Framebuffer11.h"
+#include "libANGLE/renderer/d3d/d3d11/Buffer11.h"
 #include "libANGLE/renderer/d3d/d3d11/Clear11.h"
 #include "libANGLE/renderer/d3d/d3d11/TextureStorage11.h"
 #include "libANGLE/renderer/d3d/d3d11/Renderer11.h"
+#include "libANGLE/renderer/d3d/d3d11/renderer11_utils.h"
+#include "libANGLE/renderer/d3d/d3d11/RenderTarget11.h"
 #include "libANGLE/renderer/d3d/TextureD3D.h"
 #include "libANGLE/Framebuffer.h"
 #include "libANGLE/FramebufferAttachment.h"
@@ -73,4 +76,74 @@ gl::Error Framebuffer11::clear(const gl::State &state, const gl::ClearParameters
     return gl::Error(GL_NO_ERROR);
 }
 
+static gl::Error getRenderTargetResource(const gl::FramebufferAttachment *colorbuffer, unsigned int *subresourceIndexOut,
+                                         ID3D11Texture2D **texture2DOut)
+{
+    ASSERT(colorbuffer);
+
+    RenderTarget11 *renderTarget = NULL;
+    gl::Error error = d3d11::GetAttachmentRenderTarget(colorbuffer, &renderTarget);
+    if (error.isError())
+    {
+        return error;
+    }
+
+    ID3D11Resource *renderTargetResource = renderTarget->getTexture();
+    ASSERT(renderTargetResource);
+
+    *subresourceIndexOut = renderTarget->getSubresourceIndex();
+    *texture2DOut = d3d11::DynamicCastComObject<ID3D11Texture2D>(renderTargetResource);
+
+    if (!(*texture2DOut))
+    {
+        return gl::Error(GL_OUT_OF_MEMORY, "Failed to query the ID3D11Texture2D from a RenderTarget");
+    }
+
+    return gl::Error(GL_NO_ERROR);
+}
+
+gl::Error Framebuffer11::readPixels(const gl::Rectangle &area, GLenum format, GLenum type, size_t outputPitch, const gl::PixelPackState &pack, uint8_t *pixels) const
+{
+    ID3D11Texture2D *colorBufferTexture = NULL;
+    unsigned int subresourceIndex = 0;
+
+    const gl::FramebufferAttachment *colorbuffer = mColorBuffers[0];
+    ASSERT(colorbuffer);
+
+    gl::Error error = getRenderTargetResource(colorbuffer, &subresourceIndex, &colorBufferTexture);
+    if (error.isError())
+    {
+        return error;
+    }
+
+    gl::Buffer *packBuffer = pack.pixelBuffer.get();
+    if (packBuffer != NULL)
+    {
+        Buffer11 *packBufferStorage = Buffer11::makeBuffer11(packBuffer->getImplementation());
+        PackPixelsParams packParams(area, format, type, outputPitch, pack, reinterpret_cast<ptrdiff_t>(pixels));
+
+        error = packBufferStorage->packPixels(colorBufferTexture, subresourceIndex, packParams);
+        if (error.isError())
+        {
+            SafeRelease(colorBufferTexture);
+            return error;
+        }
+
+        packBuffer->getIndexRangeCache()->clear();
+    }
+    else
+    {
+        error = mRenderer->readTextureData(colorBufferTexture, subresourceIndex, area, format, type, outputPitch, pack, pixels);
+        if (error.isError())
+        {
+            SafeRelease(colorBufferTexture);
+            return error;
+        }
+    }
+
+    SafeRelease(colorBufferTexture);
+
+    return gl::Error(GL_NO_ERROR);
+}
+
 }

src/libANGLE/renderer/d3d/d3d11/Framebuffer11.h

@@ -27,6 +27,9 @@ class Framebuffer11 : public FramebufferD3D
   private:
     gl::Error clear(const gl::State &state, const gl::ClearParameters &clearParams) override;
 
+    gl::Error readPixels(const gl::Rectangle &area, GLenum format, GLenum type, size_t outputPitch,
+                         const gl::PixelPackState &pack, uint8_t *pixels) const override;
+
     Renderer11 *const mRenderer;
 };
 

src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp

@@ -2736,32 +2736,6 @@ gl::Error Renderer11::fastCopyBufferToTexture(const gl::PixelUnpackState &unpack
     return mPixelTransfer->copyBufferToTexture(unpack, offset, destRenderTarget, destinationFormat, sourcePixelsType, destArea);
 }
 
-gl::Error Renderer11::getRenderTargetResource(gl::FramebufferAttachment *colorbuffer, unsigned int *subresourceIndexOut, ID3D11Texture2D **texture2DOut)
-
-{
-    ASSERT(colorbuffer);
-
-    RenderTarget11 *renderTarget = NULL;
-    gl::Error error = d3d11::GetAttachmentRenderTarget(colorbuffer, &renderTarget);
-    if (error.isError())
-    {
-        return error;
-    }
-
-    ID3D11Resource *renderTargetResource = renderTarget->getTexture();
-    ASSERT(renderTargetResource);
-
-    *subresourceIndexOut = renderTarget->getSubresourceIndex();
-    *texture2DOut = d3d11::DynamicCastComObject<ID3D11Texture2D>(renderTargetResource);
-
-    if (!(*texture2DOut))
-    {
-        return gl::Error(GL_OUT_OF_MEMORY, "Failed to query the ID3D11Texture2D from a RenderTarget");
-    }
-
-    return gl::Error(GL_NO_ERROR);
-}
-
 gl::Error Renderer11::blitRect(const gl::Framebuffer *readTarget, const gl::Rectangle &readRect,
                                const gl::Framebuffer *drawTarget, const gl::Rectangle &drawRect,
                                const gl::Rectangle *scissor, bool blitRenderTarget,
@@ -2842,57 +2816,6 @@ gl::Error Renderer11::blitRect(const gl::Framebuffer *readTarget, const gl::Rect
     return gl::Error(GL_NO_ERROR);
 }
 
-gl::Error Renderer11::readPixels(const gl::Framebuffer *framebuffer, GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
-                                 GLenum type, GLuint outputPitch, const gl::PixelPackState &pack, uint8_t *pixels)
-{
-    ID3D11Texture2D *colorBufferTexture = NULL;
-    unsigned int subresourceIndex = 0;
-
-    gl::FramebufferAttachment *colorbuffer = framebuffer->getReadColorbuffer();
-    ASSERT(colorbuffer);
-
-    gl::Error error = getRenderTargetResource(colorbuffer, &subresourceIndex, &colorBufferTexture);
-    if (error.isError())
-    {
-        return error;
-    }
-
-    gl::Rectangle area;
-    area.x = x;
-    area.y = y;
-    area.width = width;
-    area.height = height;
-
-    gl::Buffer *packBuffer = pack.pixelBuffer.get();
-    if (packBuffer != NULL)
-    {
-        Buffer11 *packBufferStorage = Buffer11::makeBuffer11(packBuffer->getImplementation());
-        PackPixelsParams packParams(area, format, type, outputPitch, pack, reinterpret_cast<ptrdiff_t>(pixels));
-
-        error = packBufferStorage->packPixels(colorBufferTexture, subresourceIndex, packParams);
-        if (error.isError())
-        {
-            SafeRelease(colorBufferTexture);
-            return error;
-        }
-
-        packBuffer->getIndexRangeCache()->clear();
-    }
-    else
-    {
-        error = readTextureData(colorBufferTexture, subresourceIndex, area, format, type, outputPitch, pack, pixels);
-        if (error.isError())
-        {
-            SafeRelease(colorBufferTexture);
-            return error;
-        }
-    }
-
-    SafeRelease(colorBufferTexture);
-
-    return gl::Error(GL_NO_ERROR);
-}
-
 Image *Renderer11::createImage()
 {
     return new Image11();

src/libANGLE/renderer/d3d/d3d11/Renderer11.h

@@ -130,9 +130,6 @@ class Renderer11 : public RendererD3D
     gl::Error blitRect(const gl::Framebuffer *readTarget, const gl::Rectangle &readRect, const gl::Framebuffer *drawTarget, const gl::Rectangle &drawRect,
                        const gl::Rectangle *scissor, bool blitRenderTarget, bool blitDepth, bool blitStencil, GLenum filter) override;
 
-    virtual gl::Error readPixels(const gl::Framebuffer *framebuffer, GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
-                                 GLenum type, GLuint outputPitch, const gl::PixelPackState &pack, uint8_t *pixels);
-
     // RenderTarget creation
     virtual gl::Error createRenderTarget(int width, int height, GLenum format, GLsizei samples, RenderTarget **outRT);
 
@@ -200,8 +197,6 @@ class Renderer11 : public RendererD3D
     virtual gl::Error fastCopyBufferToTexture(const gl::PixelUnpackState &unpack, unsigned int offset, RenderTarget *destRenderTarget,
                                               GLenum destinationFormat, GLenum sourcePixelsType, const gl::Box &destArea);
 
-    gl::Error getRenderTargetResource(gl::FramebufferAttachment *colorbuffer, unsigned int *subresourceIndexOut, ID3D11Texture2D **texture2DOut);
-
     void unapplyRenderTargets();
     void setOneTimeRenderTarget(ID3D11RenderTargetView *renderTargetView);
     gl::Error packPixels(ID3D11Texture2D *readTexture, const PackPixelsParams &params, uint8_t *pixelsOut);

src/libANGLE/renderer/d3d/d3d9/Framebuffer9.cpp

@@ -52,4 +52,179 @@ gl::Error Framebuffer9::clear(const gl::State &state, const gl::ClearParameters
     return mRenderer->clear(clearParams, mColorBuffers[0], depthStencilBuffer);
 }
 
+gl::Error Framebuffer9::readPixels(const gl::Rectangle &area, GLenum format, GLenum type, size_t outputPitch, const gl::PixelPackState &pack, uint8_t *pixels) const
+{
+    ASSERT(pack.pixelBuffer.get() == NULL);
+
+    const gl::FramebufferAttachment *colorbuffer = mColorBuffers[0];
+    ASSERT(colorbuffer);
+
+    RenderTarget9 *renderTarget = NULL;
+    gl::Error error = d3d9::GetAttachmentRenderTarget(colorbuffer, &renderTarget);
+    if (error.isError())
+    {
+        return error;
+    }
+    ASSERT(renderTarget);
+
+    IDirect3DSurface9 *surface = renderTarget->getSurface();
+    ASSERT(surface);
+
+    D3DSURFACE_DESC desc;
+    surface->GetDesc(&desc);
+
+    if (desc.MultiSampleType != D3DMULTISAMPLE_NONE)
+    {
+        UNIMPLEMENTED();   // FIXME: Requires resolve using StretchRect into non-multisampled render target
+        SafeRelease(surface);
+        return gl::Error(GL_OUT_OF_MEMORY, "ReadPixels is unimplemented for multisampled framebuffer attachments.");
+    }
+
+    IDirect3DDevice9 *device = mRenderer->getDevice();
+    ASSERT(device);
+
+    HRESULT result;
+    IDirect3DSurface9 *systemSurface = NULL;
+    bool directToPixels = !pack.reverseRowOrder && pack.alignment <= 4 && mRenderer->getShareHandleSupport() &&
+                          area.x == 0 && area.y == 0 &&
+                          static_cast<UINT>(area.width) == desc.Width && static_cast<UINT>(area.height) == desc.Height &&
+                          desc.Format == D3DFMT_A8R8G8B8 && format == GL_BGRA_EXT && type == GL_UNSIGNED_BYTE;
+    if (directToPixels)
+    {
+        // Use the pixels ptr as a shared handle to write directly into client's memory
+        result = device->CreateOffscreenPlainSurface(desc.Width, desc.Height, desc.Format,
+                                                     D3DPOOL_SYSTEMMEM, &systemSurface, reinterpret_cast<void**>(&pixels));
+        if (FAILED(result))
+        {
+            // Try again without the shared handle
+            directToPixels = false;
+        }
+    }
+
+    if (!directToPixels)
+    {
+        result = device->CreateOffscreenPlainSurface(desc.Width, desc.Height, desc.Format,
+                                                     D3DPOOL_SYSTEMMEM, &systemSurface, NULL);
+        if (FAILED(result))
+        {
+            ASSERT(result == D3DERR_OUTOFVIDEOMEMORY || result == E_OUTOFMEMORY);
+            SafeRelease(surface);
+            return gl::Error(GL_OUT_OF_MEMORY, "Failed to allocate internal texture for ReadPixels.");
+        }
+    }
+
+    result = device->GetRenderTargetData(surface, systemSurface);
+    SafeRelease(surface);
+
+    if (FAILED(result))
+    {
+        SafeRelease(systemSurface);
+
+        // It turns out that D3D will sometimes produce more error
+        // codes than those documented.
+        if (d3d9::isDeviceLostError(result))
+        {
+            mRenderer->notifyDeviceLost();
+        }
+        else
+        {
+            UNREACHABLE();
+        }
+
+        return gl::Error(GL_OUT_OF_MEMORY, "Failed to read internal render target data.");
+    }
+
+    if (directToPixels)
+    {
+        SafeRelease(systemSurface);
+        return gl::Error(GL_NO_ERROR);
+    }
+
+    RECT rect;
+    rect.left = gl::clamp(area.x, 0L, static_cast<LONG>(desc.Width));
+    rect.top = gl::clamp(area.y, 0L, static_cast<LONG>(desc.Height));
+    rect.right = gl::clamp(area.x + area.width, 0L, static_cast<LONG>(desc.Width));
+    rect.bottom = gl::clamp(area.y + area.height, 0L, static_cast<LONG>(desc.Height));
+
+    D3DLOCKED_RECT lock;
+    result = systemSurface->LockRect(&lock, &rect, D3DLOCK_READONLY);
+
+    if (FAILED(result))
+    {
+        UNREACHABLE();
+        SafeRelease(systemSurface);
+
+        return gl::Error(GL_OUT_OF_MEMORY, "Failed to lock internal render target.");
+    }
+
+    uint8_t *source;
+    int inputPitch;
+    if (pack.reverseRowOrder)
+    {
+        source = reinterpret_cast<uint8_t*>(lock.pBits) + lock.Pitch * (rect.bottom - rect.top - 1);
+        inputPitch = -lock.Pitch;
+    }
+    else
+    {
+        source = reinterpret_cast<uint8_t*>(lock.pBits);
+        inputPitch = lock.Pitch;
+    }
+
+    const d3d9::D3DFormat &d3dFormatInfo = d3d9::GetD3DFormatInfo(desc.Format);
+    const gl::InternalFormat &sourceFormatInfo = gl::GetInternalFormatInfo(d3dFormatInfo.internalFormat);
+    if (sourceFormatInfo.format == format && sourceFormatInfo.type == type)
+    {
+        // Direct copy possible
+        for (int y = 0; y < rect.bottom - rect.top; y++)
+        {
+            memcpy(pixels + y * outputPitch, source + y * inputPitch, (rect.right - rect.left) * sourceFormatInfo.pixelBytes);
+        }
+    }
+    else
+    {
+        const d3d9::D3DFormat &sourceD3DFormatInfo = d3d9::GetD3DFormatInfo(desc.Format);
+        ColorCopyFunction fastCopyFunc = sourceD3DFormatInfo.getFastCopyFunction(format, type);
+
+        const gl::FormatType &destFormatTypeInfo = gl::GetFormatTypeInfo(format, type);
+        const gl::InternalFormat &destFormatInfo = gl::GetInternalFormatInfo(destFormatTypeInfo.internalFormat);
+
+        if (fastCopyFunc)
+        {
+            // Fast copy is possible through some special function
+            for (int y = 0; y < rect.bottom - rect.top; y++)
+            {
+                for (int x = 0; x < rect.right - rect.left; x++)
+                {
+                    uint8_t *dest = pixels + y * outputPitch + x * destFormatInfo.pixelBytes;
+                    const uint8_t *src = source + y * inputPitch + x * sourceFormatInfo.pixelBytes;
+
+                    fastCopyFunc(src, dest);
+                }
+            }
+        }
+        else
+        {
+            uint8_t temp[sizeof(gl::ColorF)];
+            for (int y = 0; y < rect.bottom - rect.top; y++)
+            {
+                for (int x = 0; x < rect.right - rect.left; x++)
+                {
+                    uint8_t *dest = pixels + y * outputPitch + x * destFormatInfo.pixelBytes;
+                    const uint8_t *src = source + y * inputPitch + x * sourceFormatInfo.pixelBytes;
+
+                    // readFunc and writeFunc will be using the same type of color, CopyTexImage
+                    // will not allow the copy otherwise.
+                    sourceD3DFormatInfo.colorReadFunction(src, temp);
+                    destFormatTypeInfo.colorWriteFunction(temp, dest);
+                }
+            }
+        }
+    }
+
+    systemSurface->UnlockRect();
+    SafeRelease(systemSurface);
+
+    return gl::Error(GL_NO_ERROR);
+}
+
 }

src/libANGLE/renderer/d3d/d3d9/Framebuffer9.h

@@ -24,6 +24,9 @@ class Framebuffer9 : public FramebufferD3D
   private:
     gl::Error clear(const gl::State &state, const gl::ClearParameters &clearParams) override;
 
+    gl::Error readPixels(const gl::Rectangle &area, GLenum format, GLenum type, size_t outputPitch,
+                         const gl::PixelPackState &pack, uint8_t *pixels) const override;
+
     Renderer9 *const mRenderer;
 };
 

src/libANGLE/renderer/d3d/d3d9/Renderer9.cpp

@@ -2633,178 +2633,6 @@ gl::Error Renderer9::blitRect(const gl::Framebuffer *readFramebuffer, const gl::
     return gl::Error(GL_NO_ERROR);
 }
 
-gl::Error Renderer9::readPixels(const gl::Framebuffer *framebuffer, GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
-                                GLenum type, GLuint outputPitch, const gl::PixelPackState &pack, uint8_t *pixels)
-{
-    ASSERT(pack.pixelBuffer.get() == NULL);
-
-    gl::FramebufferAttachment *colorbuffer = framebuffer->getColorbuffer(0);
-    ASSERT(colorbuffer);
-
-    RenderTarget9 *renderTarget = NULL;
-    gl::Error error = d3d9::GetAttachmentRenderTarget(colorbuffer, &renderTarget);
-    if (error.isError())
-    {
-        return error;
-    }
-    ASSERT(renderTarget);
-
-    IDirect3DSurface9 *surface = renderTarget->getSurface();
-    ASSERT(surface);
-
-    D3DSURFACE_DESC desc;
-    surface->GetDesc(&desc);
-
-    if (desc.MultiSampleType != D3DMULTISAMPLE_NONE)
-    {
-        UNIMPLEMENTED();   // FIXME: Requires resolve using StretchRect into non-multisampled render target
-        SafeRelease(surface);
-        return gl::Error(GL_OUT_OF_MEMORY, "ReadPixels is unimplemented for multisampled framebuffer attachments.");
-    }
-
-    HRESULT result;
-    IDirect3DSurface9 *systemSurface = NULL;
-    bool directToPixels = !pack.reverseRowOrder && pack.alignment <= 4 && getShareHandleSupport() &&
-                          x == 0 && y == 0 && UINT(width) == desc.Width && UINT(height) == desc.Height &&
-                          desc.Format == D3DFMT_A8R8G8B8 && format == GL_BGRA_EXT && type == GL_UNSIGNED_BYTE;
-    if (directToPixels)
-    {
-        // Use the pixels ptr as a shared handle to write directly into client's memory
-        result = mDevice->CreateOffscreenPlainSurface(desc.Width, desc.Height, desc.Format,
-                                                      D3DPOOL_SYSTEMMEM, &systemSurface, reinterpret_cast<void**>(&pixels));
-        if (FAILED(result))
-        {
-            // Try again without the shared handle
-            directToPixels = false;
-        }
-    }
-
-    if (!directToPixels)
-    {
-        result = mDevice->CreateOffscreenPlainSurface(desc.Width, desc.Height, desc.Format,
-                                                      D3DPOOL_SYSTEMMEM, &systemSurface, NULL);
-        if (FAILED(result))
-        {
-            ASSERT(result == D3DERR_OUTOFVIDEOMEMORY || result == E_OUTOFMEMORY);
-            SafeRelease(surface);
-            return gl::Error(GL_OUT_OF_MEMORY, "Failed to allocate internal texture for ReadPixels.");
-        }
-    }
-
-    result = mDevice->GetRenderTargetData(surface, systemSurface);
-    SafeRelease(surface);
-
-    if (FAILED(result))
-    {
-        SafeRelease(systemSurface);
-
-        // It turns out that D3D will sometimes produce more error
-        // codes than those documented.
-        if (d3d9::isDeviceLostError(result))
-        {
-            notifyDeviceLost();
-        }
-        else
-        {
-            UNREACHABLE();
-        }
-
-        return gl::Error(GL_OUT_OF_MEMORY, "Failed to read internal render target data.");
-    }
-
-    if (directToPixels)
-    {
-        SafeRelease(systemSurface);
-        return gl::Error(GL_NO_ERROR);
-    }
-
-    RECT rect;
-    rect.left = gl::clamp(x, 0L, static_cast<LONG>(desc.Width));
-    rect.top = gl::clamp(y, 0L, static_cast<LONG>(desc.Height));
-    rect.right = gl::clamp(x + width, 0L, static_cast<LONG>(desc.Width));
-    rect.bottom = gl::clamp(y + height, 0L, static_cast<LONG>(desc.Height));
-
-    D3DLOCKED_RECT lock;
-    result = systemSurface->LockRect(&lock, &rect, D3DLOCK_READONLY);
-
-    if (FAILED(result))
-    {
-        UNREACHABLE();
-        SafeRelease(systemSurface);
-
-        return gl::Error(GL_OUT_OF_MEMORY, "Failed to lock internal render target.");
-    }
-
-    uint8_t *source;
-    int inputPitch;
-    if (pack.reverseRowOrder)
-    {
-        source = reinterpret_cast<uint8_t*>(lock.pBits) + lock.Pitch * (rect.bottom - rect.top - 1);
-        inputPitch = -lock.Pitch;
-    }
-    else
-    {
-        source = reinterpret_cast<uint8_t*>(lock.pBits);
-        inputPitch = lock.Pitch;
-    }
-
-    const d3d9::D3DFormat &d3dFormatInfo = d3d9::GetD3DFormatInfo(desc.Format);
-    const gl::InternalFormat &sourceFormatInfo = gl::GetInternalFormatInfo(d3dFormatInfo.internalFormat);
-    if (sourceFormatInfo.format == format && sourceFormatInfo.type == type)
-    {
-        // Direct copy possible
-        for (int y = 0; y < rect.bottom - rect.top; y++)
-        {
-            memcpy(pixels + y * outputPitch, source + y * inputPitch, (rect.right - rect.left) * sourceFormatInfo.pixelBytes);
-        }
-    }
-    else
-    {
-        const d3d9::D3DFormat &sourceD3DFormatInfo = d3d9::GetD3DFormatInfo(desc.Format);
-        ColorCopyFunction fastCopyFunc = sourceD3DFormatInfo.getFastCopyFunction(format, type);
-
-        const gl::FormatType &destFormatTypeInfo = gl::GetFormatTypeInfo(format, type);
-        const gl::InternalFormat &destFormatInfo = gl::GetInternalFormatInfo(destFormatTypeInfo.internalFormat);
-
-        if (fastCopyFunc)
-        {
-            // Fast copy is possible through some special function
-            for (int y = 0; y < rect.bottom - rect.top; y++)
-            {
-                for (int x = 0; x < rect.right - rect.left; x++)
-                {
-                    uint8_t *dest = pixels + y * outputPitch + x * destFormatInfo.pixelBytes;
-                    const uint8_t *src = source + y * inputPitch + x * sourceFormatInfo.pixelBytes;
-
-                    fastCopyFunc(src, dest);
-                }
-            }
-        }
-        else
-        {
-            uint8_t temp[sizeof(gl::ColorF)];
-            for (int y = 0; y < rect.bottom - rect.top; y++)
-            {
-                for (int x = 0; x < rect.right - rect.left; x++)
-                {
-                    uint8_t *dest = pixels + y * outputPitch + x * destFormatInfo.pixelBytes;
-                    const uint8_t *src = source + y * inputPitch + x * sourceFormatInfo.pixelBytes;
-
-                    // readFunc and writeFunc will be using the same type of color, CopyTexImage
-                    // will not allow the copy otherwise.
-                    sourceD3DFormatInfo.colorReadFunction(src, temp);
-                    destFormatTypeInfo.colorWriteFunction(temp, dest);
-                }
-            }
-        }
-    }
-
-    systemSurface->UnlockRect();
-    SafeRelease(systemSurface);
-
-    return gl::Error(GL_NO_ERROR);
-}
-
 gl::Error Renderer9::createRenderTarget(int width, int height, GLenum format, GLsizei samples, RenderTarget **outRT)
 {
     const d3d9::TextureFormat &d3d9FormatInfo = d3d9::GetTextureFormatInfo(format);

src/libANGLE/renderer/d3d/d3d9/Renderer9.h

@@ -141,9 +141,6 @@ class Renderer9 : public RendererD3D
                        const gl::Rectangle *scissor, bool blitRenderTarget,
                        bool blitDepth, bool blitStencil, GLenum filter) override;
 
-    virtual gl::Error readPixels(const gl::Framebuffer *framebuffer, GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
-                                 GLenum type, GLuint outputPitch, const gl::PixelPackState &pack, uint8_t *pixels);
-
     // RenderTarget creation
     virtual gl::Error createRenderTarget(int width, int height, GLenum format, GLsizei samples, RenderTarget **outRT);
 

src/libANGLE/validationES.cpp

@@ -906,17 +906,18 @@ bool ValidateReadPixelsParameters(gl::Context *context, GLint x, GLint y, GLsize
         return false;
     }
 
-    if (!framebuffer->getReadColorbuffer())
+    const FramebufferAttachment *readBuffer = framebuffer->getReadColorbuffer();
+    if (!readBuffer)
     {
         context->recordError(Error(GL_INVALID_OPERATION));
         return false;
     }
 
-    GLenum currentInternalFormat, currentFormat, currentType;
+    GLenum currentFormat = framebuffer->getImplementationColorReadFormat();
+    GLenum currentType = framebuffer->getImplementationColorReadType();
+    GLenum currentInternalFormat = readBuffer->getActualFormat();
     GLuint clientVersion = context->getClientVersion();
 
-    context->getCurrentReadFormatType(&currentInternalFormat, &currentFormat, &currentType);
-
     bool validReadFormat = (clientVersion < 3) ? ValidES2ReadFormatType(context, format, type) :
                                                  ValidES3ReadFormatType(context, currentInternalFormat, format, type);
 

src/libGLESv2/entry_points_gles_2_0.cpp

@@ -3317,7 +3317,11 @@ void GL_APIENTRY ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height,
             return;
         }
 
-        Error error = context->readPixels(x, y, width, height, format, type, NULL, pixels);
+        Framebuffer *framebufferObject = context->getState().getReadFramebuffer();
+        ASSERT(framebufferObject);
+
+        Rectangle area(x, y, width, height);
+        Error error = framebufferObject->readPixels(context->getState(), area, format, type, pixels);
         if (error.isError())
         {
             context->recordError(error);

src/libGLESv2/entry_points_gles_2_0_ext.cpp

@@ -490,7 +490,11 @@ void GL_APIENTRY ReadnPixelsEXT(GLint x, GLint y, GLsizei width, GLsizei height,
             return;
         }
 
-        Error error = context->readPixels(x, y, width, height, format, type, &bufSize, data);
+        Framebuffer *framebufferObject = context->getState().getReadFramebuffer();
+        ASSERT(framebufferObject);
+
+        Rectangle area(x, y, width, height);
+        Error error = framebufferObject->readPixels(context->getState(), area, format, type, data);
         if (error.isError())
         {
             context->recordError(error);