D3D11: Fix 3D ReadPixels with PBOs.

Pass the FBO attachment to the Buffer packing method so we can
retrieve the layer of the attachment when doing an asynchronous
readback. Also take advantage of the TextureHelper11 class to
remove some redundant code.

BUG=angleproject:1290

Change-Id: I26bb21a03e0ff7a42aab4eee75f3c3d12915f398
Reviewed-on: https://chromium-review.googlesource.com/324021
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
diff --git a/src/libANGLE/renderer/d3d/d3d11/Buffer11.cpp b/src/libANGLE/renderer/d3d/d3d11/Buffer11.cpp
index 1a77db8..66e2d67 100644
--- a/src/libANGLE/renderer/d3d/d3d11/Buffer11.cpp
+++ b/src/libANGLE/renderer/d3d/d3d11/Buffer11.cpp
@@ -8,10 +8,13 @@
 
 #include "libANGLE/renderer/d3d/d3d11/Buffer11.h"
 
+#include <memory>
+
 #include "common/MemoryBuffer.h"
 #include "libANGLE/renderer/d3d/IndexDataManager.h"
 #include "libANGLE/renderer/d3d/VertexDataManager.h"
 #include "libANGLE/renderer/d3d/d3d11/Renderer11.h"
+#include "libANGLE/renderer/d3d/d3d11/RenderTarget11.h"
 #include "libANGLE/renderer/d3d/d3d11/renderer11_utils.h"
 #include "libANGLE/renderer/d3d/d3d11/formatutils11.h"
 
@@ -199,18 +202,15 @@
     uint8_t *map(size_t offset, size_t length, GLbitfield access) override;
     void unmap() override;
 
-    gl::Error packPixels(const TextureHelper11 &srcTexture,
-                         UINT srcSubresource,
+    gl::Error packPixels(const gl::FramebufferAttachment &readAttachment,
                          const PackPixelsParams &params);
 
   private:
     gl::Error flushQueuedPackCommand();
 
-    ID3D11Texture2D *mStagingTexture;
-    DXGI_FORMAT mTextureFormat;
-    gl::Extents mTextureSize;
+    TextureHelper11 mStagingTexture;
     MemoryBuffer mMemoryBuffer;
-    PackPixelsParams *mQueuedPackCommand;
+    std::unique_ptr<PackPixelsParams> mQueuedPackCommand;
     PackPixelsParams mPackParams;
     bool mDataModified;
 };
@@ -624,8 +624,7 @@
     return bufferSRV;
 }
 
-gl::Error Buffer11::packPixels(const TextureHelper11 &srcTexture,
-                               UINT srcSubresource,
+gl::Error Buffer11::packPixels(const gl::FramebufferAttachment &readAttachment,
                                const PackPixelsParams &params)
 {
     PackStorage *packStorage     = getPackStorage();
@@ -633,7 +632,7 @@
 
     if (packStorage)
     {
-        gl::Error error = packStorage->packPixels(srcTexture, srcSubresource, params);
+        gl::Error error = packStorage->packPixels(readAttachment, params);
         if (error.isError())
         {
             return error;
@@ -1245,18 +1244,12 @@
 }
 
 Buffer11::PackStorage::PackStorage(Renderer11 *renderer)
-    : BufferStorage(renderer, BUFFER_USAGE_PIXEL_PACK),
-      mStagingTexture(nullptr),
-      mTextureFormat(DXGI_FORMAT_UNKNOWN),
-      mQueuedPackCommand(nullptr),
-      mDataModified(false)
+    : BufferStorage(renderer, BUFFER_USAGE_PIXEL_PACK), mStagingTexture(), mDataModified(false)
 {
 }
 
 Buffer11::PackStorage::~PackStorage()
 {
-    SafeRelease(mStagingTexture);
-    SafeDelete(mQueuedPackCommand);
 }
 
 bool Buffer11::PackStorage::copyFromStorage(BufferStorage *source,
@@ -1308,64 +1301,42 @@
     // No-op
 }
 
-gl::Error Buffer11::PackStorage::packPixels(const TextureHelper11 &srcTexture,
-                                            UINT srcSubresource,
+gl::Error Buffer11::PackStorage::packPixels(const gl::FramebufferAttachment &readAttachment,
                                             const PackPixelsParams &params)
 {
-    ID3D11Texture2D *tex2D = srcTexture.getTexture2D();
-    if (tex2D == nullptr)
-    {
-        // TODO(jmadill): Implement for 3D textures.
-        return gl::Error(GL_OUT_OF_MEMORY,
-                         "Buffer11::PackStorage::packPixels called with 3D texture.");
-    }
-
     gl::Error error = flushQueuedPackCommand();
     if (error.isError())
     {
         return error;
     }
 
-    mQueuedPackCommand = new PackPixelsParams(params);
-
-    if (mStagingTexture != nullptr &&
-        (mTextureFormat != srcTexture.getFormat() || mTextureSize.width != params.area.width ||
-         mTextureSize.height != params.area.height))
+    RenderTarget11 *renderTarget = nullptr;
+    error = readAttachment.getRenderTarget(&renderTarget);
+    if (error.isError())
     {
-        SafeRelease(mStagingTexture);
-        mTextureSize.width  = 0;
-        mTextureSize.height = 0;
-        mTextureFormat      = DXGI_FORMAT_UNKNOWN;
+        return error;
     }
 
-    if (mStagingTexture == nullptr)
+    ID3D11Resource *renderTargetResource = renderTarget->getTexture();
+    ASSERT(renderTargetResource);
+
+    unsigned int srcSubresource = renderTarget->getSubresourceIndex();
+    TextureHelper11 srcTexture  = TextureHelper11::MakeAndReference(renderTargetResource);
+
+    mQueuedPackCommand.reset(new PackPixelsParams(params));
+
+    gl::Extents srcTextureSize(params.area.width, params.area.height, 1);
+    if (!mStagingTexture.getResource() || mStagingTexture.getFormat() != srcTexture.getFormat() ||
+        mStagingTexture.getExtents() != srcTextureSize)
     {
-        ID3D11Device *device = mRenderer->getDevice();
-        HRESULT hr;
-
-        mTextureSize.width  = params.area.width;
-        mTextureSize.height = params.area.height;
-        mTextureFormat      = srcTexture.getFormat();
-
-        D3D11_TEXTURE2D_DESC stagingDesc;
-        stagingDesc.Width              = params.area.width;
-        stagingDesc.Height             = params.area.height;
-        stagingDesc.MipLevels          = 1;
-        stagingDesc.ArraySize          = 1;
-        stagingDesc.Format             = mTextureFormat;
-        stagingDesc.SampleDesc.Count   = 1;
-        stagingDesc.SampleDesc.Quality = 0;
-        stagingDesc.Usage              = D3D11_USAGE_STAGING;
-        stagingDesc.BindFlags          = 0;
-        stagingDesc.CPUAccessFlags     = D3D11_CPU_ACCESS_READ;
-        stagingDesc.MiscFlags          = 0;
-
-        hr = device->CreateTexture2D(&stagingDesc, nullptr, &mStagingTexture);
-        if (FAILED(hr))
+        auto textureOrError =
+            CreateStagingTexture(srcTexture.getTextureType(), srcTexture.getFormat(),
+                                 srcTextureSize, mRenderer->getDevice());
+        if (textureOrError.isError())
         {
-            ASSERT(hr == E_OUTOFMEMORY);
-            return gl::Error(GL_OUT_OF_MEMORY, "Failed to allocate internal staging texture.");
+            return textureOrError.getError();
         }
+        mStagingTexture = std::move(textureOrError.getResult());
     }
 
     // ReadPixels from multisampled FBOs isn't supported in current GL
@@ -1377,12 +1348,18 @@
     srcBox.right  = params.area.x + params.area.width;
     srcBox.top    = params.area.y;
     srcBox.bottom = params.area.y + params.area.height;
-    srcBox.front  = 0;
-    srcBox.back   = 1;
+
+    // Select the correct layer from a 3D attachment
+    srcBox.front = 0;
+    if (mStagingTexture.getTextureType() == GL_TEXTURE_3D)
+    {
+        srcBox.front = static_cast<UINT>(readAttachment.layer());
+    }
+    srcBox.back = srcBox.front + 1;
 
     // Asynchronous copy
-    immediateContext->CopySubresourceRegion(mStagingTexture, 0, 0, 0, 0, tex2D, srcSubresource,
-                                            &srcBox);
+    immediateContext->CopySubresourceRegion(mStagingTexture.getResource(), 0, 0, 0, 0,
+                                            srcTexture.getResource(), srcSubresource, &srcBox);
 
     return gl::Error(GL_NO_ERROR);
 }
@@ -1393,11 +1370,9 @@
 
     if (mQueuedPackCommand)
     {
-        TextureHelper11 stagingHelper = TextureHelper11::MakeAndReference(mStagingTexture);
-
         gl::Error error =
-            mRenderer->packPixels(stagingHelper, *mQueuedPackCommand, mMemoryBuffer.data());
-        SafeDelete(mQueuedPackCommand);
+            mRenderer->packPixels(mStagingTexture, *mQueuedPackCommand, mMemoryBuffer.data());
+        mQueuedPackCommand.reset(nullptr);
         if (error.isError())
         {
             return error;
diff --git a/src/libANGLE/renderer/d3d/d3d11/Buffer11.h b/src/libANGLE/renderer/d3d/d3d11/Buffer11.h
index 07dcc1c..a748db5 100644
--- a/src/libANGLE/renderer/d3d/d3d11/Buffer11.h
+++ b/src/libANGLE/renderer/d3d/d3d11/Buffer11.h
@@ -14,11 +14,15 @@
 #include "libANGLE/angletypes.h"
 #include "libANGLE/renderer/d3d/BufferD3D.h"
 
+namespace gl
+{
+class FramebufferAttachment;
+}
+
 namespace rx
 {
 class Renderer11;
 struct SourceIndexData;
-class TextureHelper11;
 struct TranslatedAttribute;
 
 enum BufferUsage
@@ -63,8 +67,7 @@
     ID3D11Buffer *getConstantBufferRange(GLintptr offset, GLsizeiptr size);
     ID3D11ShaderResourceView *getSRV(DXGI_FORMAT srvFormat);
     bool isMapped() const { return mMappedStorage != NULL; }
-    gl::Error packPixels(const TextureHelper11 &srcTexture,
-                         UINT srcSubresource,
+    gl::Error packPixels(const gl::FramebufferAttachment &readAttachment,
                          const PackPixelsParams &params);
     size_t getTotalCPUBufferMemoryBytes() const;
 
diff --git a/src/libANGLE/renderer/d3d/d3d11/Framebuffer11.cpp b/src/libANGLE/renderer/d3d/d3d11/Framebuffer11.cpp
index 97ca9b9..beffa30 100644
--- a/src/libANGLE/renderer/d3d/d3d11/Framebuffer11.cpp
+++ b/src/libANGLE/renderer/d3d/d3d11/Framebuffer11.cpp
@@ -302,24 +302,11 @@
                              "Unimplemented pixel store parameters in readPixelsImpl");
         }
 
-        RenderTarget11 *renderTarget = nullptr;
-        gl::Error error = readAttachment->getRenderTarget(&renderTarget);
-        if (error.isError())
-        {
-            return error;
-        }
-
-        ID3D11Resource *renderTargetResource = renderTarget->getTexture();
-        ASSERT(renderTargetResource);
-
-        unsigned int subresourceIndex = renderTarget->getSubresourceIndex();
-        TextureHelper11 textureHelper = TextureHelper11::MakeAndReference(renderTargetResource);
-
         Buffer11 *packBufferStorage = GetImplAs<Buffer11>(packBuffer);
         PackPixelsParams packParams(area, format, type, static_cast<GLuint>(outputPitch), pack,
                                     reinterpret_cast<ptrdiff_t>(pixels));
 
-        return packBufferStorage->packPixels(textureHelper, subresourceIndex, packParams);
+        return packBufferStorage->packPixels(*readAttachment, packParams);
     }
 
     return mRenderer->readFromAttachment(*readAttachment, area, format, type,
diff --git a/src/tests/gl_tests/ReadPixelsTest.cpp b/src/tests/gl_tests/ReadPixelsTest.cpp
index c2853c9..8d5c077 100644
--- a/src/tests/gl_tests/ReadPixelsTest.cpp
+++ b/src/tests/gl_tests/ReadPixelsTest.cpp
@@ -439,17 +439,42 @@
         ANGLETest::TearDown();
     }
 
-    void testRead(GLenum textureTarget, GLint levels, GLint attachmentLevel, GLint attachmentLayer)
+    void initTexture(GLenum textureTarget,
+                     GLint levels,
+                     GLint attachmentLevel,
+                     GLint attachmentLayer)
     {
         glBindTexture(textureTarget, mTexture);
         glTexStorage3D(textureTarget, levels, GL_RGBA8, 4, 4, 4);
         glFramebufferTextureLayer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, mTexture, attachmentLevel,
                                   attachmentLayer);
-
         initializeTextureData(textureTarget, levels);
+    }
+
+    void testRead(GLenum textureTarget, GLint levels, GLint attachmentLevel, GLint attachmentLayer)
+    {
+        initTexture(textureTarget, levels, attachmentLevel, attachmentLayer);
         verifyColor(attachmentLevel, attachmentLayer);
     }
 
+    void initPBO()
+    {
+        glGenBuffers(1, &mBuffer);
+        glBindBuffer(GL_PIXEL_PACK_BUFFER, mBuffer);
+        glBufferData(GL_PIXEL_PACK_BUFFER, sizeof(angle::GLColor), nullptr, GL_STREAM_COPY);
+        ASSERT_GL_NO_ERROR();
+    }
+
+    void testPBORead(GLenum textureTarget,
+                     GLint levels,
+                     GLint attachmentLevel,
+                     GLint attachmentLayer)
+    {
+        initPBO();
+        initTexture(textureTarget, levels, attachmentLevel, attachmentLayer);
+        verifyPBO(attachmentLevel, attachmentLayer);
+    }
+
     // Give each {level,layer} pair a (probably) unique color via random.
     GLuint getColorValue(GLint level, GLint layer)
     {
@@ -463,6 +488,21 @@
         EXPECT_PIXEL_COLOR_EQ(0, 0, colorValue);
     }
 
+    void verifyPBO(GLint level, GLint layer)
+    {
+        glReadPixels(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
+
+        angle::GLColor expectedColor(getColorValue(level, layer));
+        void *mapPointer =
+            glMapBufferRange(GL_PIXEL_PACK_BUFFER, 0, sizeof(angle::GLColor), GL_MAP_READ_BIT);
+        ASSERT_NE(nullptr, mapPointer);
+        angle::GLColor actualColor;
+        memcpy(&actualColor, mapPointer, sizeof(angle::GLColor));
+        glUnmapBuffer(GL_PIXEL_PACK_BUFFER);
+        ASSERT_GL_NO_ERROR();
+        EXPECT_EQ(expectedColor, actualColor);
+    }
+
     void initializeTextureData(GLenum textureTarget, GLint levels)
     {
         for (GLint level = 0; level < levels; ++level)
@@ -489,6 +529,7 @@
     angle::RNG mRNG;
     GLuint mFBO;
     GLuint mTexture;
+    GLuint mBuffer;
 };
 
 // Test 3D attachment readback.
@@ -539,7 +580,53 @@
     testRead(GL_TEXTURE_2D_ARRAY, 2, 1, 1);
 }
 
-// TODO(jmadill): Tests for PBOs with 3D and layer attachments.
+// Test 3D attachment PBO readback.
+TEST_P(ReadPixelsTextureTest, BasicAttachment3DPBO)
+{
+    testPBORead(GL_TEXTURE_3D, 1, 0, 0);
+}
+
+// Test 3D attachment readback, non-zero mip.
+TEST_P(ReadPixelsTextureTest, MipAttachment3DPBO)
+{
+    testPBORead(GL_TEXTURE_3D, 2, 1, 0);
+}
+
+// Test 3D attachment readback, non-zero layer.
+TEST_P(ReadPixelsTextureTest, LayerAttachment3DPBO)
+{
+    testPBORead(GL_TEXTURE_3D, 1, 0, 1);
+}
+
+// Test 3D attachment readback, non-zero mip and layer.
+TEST_P(ReadPixelsTextureTest, MipLayerAttachment3DPBO)
+{
+    testPBORead(GL_TEXTURE_3D, 2, 1, 1);
+}
+
+// Test 2D array attachment readback.
+TEST_P(ReadPixelsTextureTest, BasicAttachment2DArrayPBO)
+{
+    testPBORead(GL_TEXTURE_2D_ARRAY, 1, 0, 0);
+}
+
+// Test 3D attachment readback, non-zero mip.
+TEST_P(ReadPixelsTextureTest, MipAttachment2DArrayPBO)
+{
+    testPBORead(GL_TEXTURE_2D_ARRAY, 2, 1, 0);
+}
+
+// Test 3D attachment readback, non-zero layer.
+TEST_P(ReadPixelsTextureTest, LayerAttachment2DArrayPBO)
+{
+    testPBORead(GL_TEXTURE_2D_ARRAY, 1, 0, 1);
+}
+
+// Test 3D attachment readback, non-zero mip and layer.
+TEST_P(ReadPixelsTextureTest, MipLayerAttachment2DArrayPBO)
+{
+    testPBORead(GL_TEXTURE_2D_ARRAY, 2, 1, 1);
+}
 
 }  // anonymous namespace