D3D11: Fix 3D ReadPixels with PBOs.

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 @@ class Buffer11::PackStorage : public Buffer11::BufferStorage
     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 @@ ID3D11ShaderResourceView *Buffer11::getSRV(DXGI_FORMAT srvFormat)
     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 @@ gl::Error Buffer11::packPixels(const TextureHelper11 &srcTexture,
 
     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 @@ void Buffer11::EmulatedIndexedStorage::unmap()
 }
 
 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 @@ void Buffer11::PackStorage::unmap()
     // 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)
+    gl::Error error = flushQueuedPackCommand();
+    if (error.isError())
     {
-        // TODO(jmadill): Implement for 3D textures.
-        return gl::Error(GL_OUT_OF_MEMORY,
-                         "Buffer11::PackStorage::packPixels called with 3D texture.");
+        return error;
     }
 
-    gl::Error error = flushQueuedPackCommand();
+    RenderTarget11 *renderTarget = nullptr;
+    error = readAttachment.getRenderTarget(&renderTarget);
     if (error.isError())
     {
         return error;
     }
 
-    mQueuedPackCommand = new PackPixelsParams(params);
+    ID3D11Resource *renderTargetResource = renderTarget->getTexture();
+    ASSERT(renderTargetResource);
 
-    if (mStagingTexture != nullptr &&
-        (mTextureFormat != srcTexture.getFormat() || mTextureSize.width != params.area.width ||
-         mTextureSize.height != params.area.height))
-    {
-        SafeRelease(mStagingTexture);
-        mTextureSize.width  = 0;
-        mTextureSize.height = 0;
-        mTextureFormat      = DXGI_FORMAT_UNKNOWN;
-    }
+    unsigned int srcSubresource = renderTarget->getSubresourceIndex();
+    TextureHelper11 srcTexture  = TextureHelper11::MakeAndReference(renderTargetResource);
 
-    if (mStagingTexture == nullptr)
+    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 @@ gl::Error Buffer11::PackStorage::packPixels(const TextureHelper11 &srcTexture,
     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 @@ gl::Error Buffer11::PackStorage::flushQueuedPackCommand()
 
     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;

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 @@ class Buffer11 : public BufferD3D
     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;
 

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

@@ -302,24 +302,11 @@ gl::Error Framebuffer11::readPixelsImpl(const gl::Rectangle &area,
                              "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,

src/tests/gl_tests/ReadPixelsTest.cpp

@@ -439,17 +439,42 @@ class ReadPixelsTextureTest : public ANGLETest
         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 @@ class ReadPixelsTextureTest : public ANGLETest
         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 @@ class ReadPixelsTextureTest : public ANGLETest
     angle::RNG mRNG;
     GLuint mFBO;
     GLuint mTexture;
+    GLuint mBuffer;
 };
 
 // Test 3D attachment readback.
@@ -539,7 +580,53 @@ TEST_P(ReadPixelsTextureTest, MipLayerAttachment2DArray)
     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