D3D11: Fix ReadPixels with layer 3D attachments.

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

@@ -269,21 +269,8 @@ gl::Error Framebuffer11::readPixelsImpl(const gl::Rectangle &area,
                                         const gl::PixelPackState &pack,
                                         uint8_t *pixels) const
 {
-    const gl::FramebufferAttachment *colorbuffer = mData.getReadAttachment();
-    ASSERT(colorbuffer);
-
-    RenderTarget11 *renderTarget = nullptr;
-    gl::Error error = colorbuffer->getRenderTarget(&renderTarget);
-    if (error.isError())
-    {
-        return error;
-    }
-
-    ID3D11Resource *renderTargetResource = renderTarget->getTexture();
-    ASSERT(renderTargetResource);
-
-    unsigned int subresourceIndex = renderTarget->getSubresourceIndex();
-    TextureHelper11 textureHelper = TextureHelper11::MakeAndReference(renderTargetResource);
+    const gl::FramebufferAttachment *readAttachment = mData.getReadAttachment();
+    ASSERT(readAttachment);
 
     gl::Buffer *packBuffer = pack.pixelBuffer.get();
     if (packBuffer != nullptr)
@@ -295,27 +282,28 @@ gl::Error Framebuffer11::readPixelsImpl(const gl::Rectangle &area,
                              "Unimplemented pixel store parameters in readPixelsImpl");
         }
 
-        Buffer11 *packBufferStorage = GetImplAs<Buffer11>(packBuffer);
-        PackPixelsParams packParams(area, format, type, static_cast<GLuint>(outputPitch), pack,
-                                    reinterpret_cast<ptrdiff_t>(pixels));
-
-        error = packBufferStorage->packPixels(textureHelper, subresourceIndex, packParams);
-        if (error.isError())
-        {
-            return error;
-        }
-    }
-    else
-    {
-        error = mRenderer->readTextureData(textureHelper, subresourceIndex, area, format, type,
-                                           static_cast<GLuint>(outputPitch), pack, pixels);
+        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 gl::Error(GL_NO_ERROR);
+    return mRenderer->readFromAttachment(*readAttachment, area, format, type,
+                                         static_cast<GLuint>(outputPitch), pack, pixels);
 }
 
 gl::Error Framebuffer11::blit(const gl::Rectangle &sourceArea, const gl::Rectangle &destArea, const gl::Rectangle *scissor,

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

@@ -338,21 +338,24 @@ gl::Error Image11::copyFromFramebuffer(const gl::Offset &destOffset,
     const gl::FramebufferAttachment *srcAttachment = sourceFBO->getReadColorbuffer();
     ASSERT(srcAttachment);
 
-    RenderTargetD3D *renderTarget = nullptr;
-    gl::Error error = srcAttachment->getRenderTarget(&renderTarget);
-    if (error.isError())
+    const auto &d3d11Format = d3d11::GetTextureFormatInfo(srcAttachment->getInternalFormat(),
+                                                          mRenderer->getRenderer11DeviceCaps());
+
+    if (d3d11Format.texFormat == mDXGIFormat)
     {
-        return error;
-    }
+        RenderTargetD3D *renderTarget = nullptr;
+        gl::Error error = srcAttachment->getRenderTarget(&renderTarget);
+        if (error.isError())
+        {
+            return error;
+        }
 
-    RenderTarget11 *rt11 = GetAs<RenderTarget11>(renderTarget);
-    ASSERT(rt11->getTexture());
+        RenderTarget11 *rt11 = GetAs<RenderTarget11>(renderTarget);
+        ASSERT(rt11->getTexture());
 
-    TextureHelper11 textureHelper  = TextureHelper11::MakeAndReference(rt11->getTexture());
-    unsigned int sourceSubResource = rt11->getSubresourceIndex();
+        TextureHelper11 textureHelper = TextureHelper11::MakeAndReference(rt11->getTexture());
+        unsigned int sourceSubResource = rt11->getSubresourceIndex();
 
-    if (textureHelper.getFormat() == mDXGIFormat)
-    {
         gl::Box sourceBox(sourceArea.x, sourceArea.y, 0, sourceArea.width, sourceArea.height, 1);
         return copyWithoutConversion(destOffset, sourceBox, textureHelper, sourceSubResource);
     }
@@ -360,7 +363,7 @@ gl::Error Image11::copyFromFramebuffer(const gl::Offset &destOffset,
     // This format requires conversion, so we must copy the texture to staging and manually convert
     // via readPixels
     D3D11_MAPPED_SUBRESOURCE mappedImage;
-    error = map(D3D11_MAP_WRITE, &mappedImage);
+    gl::Error error = map(D3D11_MAP_WRITE, &mappedImage);
     if (error.isError())
     {
         return error;
@@ -376,24 +379,14 @@ gl::Error Image11::copyFromFramebuffer(const gl::Offset &destOffset,
 
     const gl::InternalFormat &formatInfo = gl::GetInternalFormatInfo(mInternalFormat);
 
-    // Currently in ANGLE, the source data may only need to be converted if the source is the
-    // current framebuffer and OpenGL ES framebuffers must be 2D textures therefore we should not
-    // need to convert 3D textures between different formats.
-    ASSERT(textureHelper.getTextureType() == GL_TEXTURE_2D);
-    error = mRenderer->readTextureData(textureHelper, sourceSubResource, sourceArea,
-                                       formatInfo.format, formatInfo.type, mappedImage.RowPitch,
-                                       gl::PixelPackState(), dataOffset);
+    error = mRenderer->readFromAttachment(*srcAttachment, sourceArea, formatInfo.format,
+                                          formatInfo.type, mappedImage.RowPitch,
+                                          gl::PixelPackState(), dataOffset);
 
     unmap();
-
-    if (error.isError())
-    {
-        return error;
-    }
-
     mDirty = true;
 
-    return gl::Error(GL_NO_ERROR);
+    return error;
 }
 
 gl::Error Image11::copyWithoutConversion(const gl::Offset &destOffset,

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

@@ -3602,27 +3602,41 @@ RenderbufferImpl *Renderer11::createRenderbuffer()
     return renderbuffer;
 }
 
-gl::Error Renderer11::readTextureData(const TextureHelper11 &textureHelper,
-                                      unsigned int subResource,
-                                      const gl::Rectangle &area,
-                                      GLenum format,
-                                      GLenum type,
-                                      GLuint outputPitch,
-                                      const gl::PixelPackState &pack,
-                                      uint8_t *pixels)
+gl::Error Renderer11::readFromAttachment(const gl::FramebufferAttachment &srcAttachment,
+                                         const gl::Rectangle &sourceArea,
+                                         GLenum format,
+                                         GLenum type,
+                                         GLuint outputPitch,
+                                         const gl::PixelPackState &pack,
+                                         uint8_t *pixelsOut)
 {
-    ASSERT(area.width >= 0);
-    ASSERT(area.height >= 0);
+    ASSERT(sourceArea.width >= 0);
+    ASSERT(sourceArea.height >= 0);
+
+    RenderTargetD3D *renderTarget = nullptr;
+    gl::Error error = srcAttachment.getRenderTarget(&renderTarget);
+    if (error.isError())
+    {
+        return error;
+    }
+
+    RenderTarget11 *rt11 = GetAs<RenderTarget11>(renderTarget);
+    ASSERT(rt11->getTexture());
+
+    TextureHelper11 textureHelper = TextureHelper11::MakeAndReference(rt11->getTexture());
+    unsigned int sourceSubResource = rt11->getSubresourceIndex();
 
     const gl::Extents &texSize = textureHelper.getExtents();
 
     // Clamp read region to the defined texture boundaries, preventing out of bounds reads
     // and reads of uninitialized data.
     gl::Rectangle safeArea;
-    safeArea.x      = gl::clamp(area.x, 0, texSize.width);
-    safeArea.y      = gl::clamp(area.y, 0, texSize.height);
-    safeArea.width  = gl::clamp(area.width + std::min(area.x, 0), 0, texSize.width - safeArea.x);
-    safeArea.height = gl::clamp(area.height + std::min(area.y, 0), 0, texSize.height - safeArea.y);
+    safeArea.x = gl::clamp(sourceArea.x, 0, texSize.width);
+    safeArea.y = gl::clamp(sourceArea.y, 0, texSize.height);
+    safeArea.width =
+        gl::clamp(sourceArea.width + std::min(sourceArea.x, 0), 0, texSize.width - safeArea.x);
+    safeArea.height =
+        gl::clamp(sourceArea.height + std::min(sourceArea.y, 0), 0, texSize.height - safeArea.y);
 
     ASSERT(safeArea.x >= 0 && safeArea.y >= 0);
     ASSERT(safeArea.x + safeArea.width <= texSize.width);
@@ -3675,11 +3689,11 @@ gl::Error Renderer11::readTextureData(const TextureHelper11 &textureHelper,
         }
 
         mDeviceContext->ResolveSubresource(resolveTex2D, 0, textureHelper.getTexture2D(),
-                                           subResource, textureHelper.getFormat());
+                                           sourceSubResource, textureHelper.getFormat());
         resolvedTextureHelper = TextureHelper11::MakeAndReference(resolveTex2D);
 
-        subResource = 0;
-        srcTexture  = &resolvedTextureHelper;
+        sourceSubResource = 0;
+        srcTexture        = &resolvedTextureHelper;
     }
 
     D3D11_BOX srcBox;
@@ -3687,14 +3701,20 @@ gl::Error Renderer11::readTextureData(const TextureHelper11 &textureHelper,
     srcBox.right  = static_cast<UINT>(safeArea.x + safeArea.width);
     srcBox.top    = static_cast<UINT>(safeArea.y);
     srcBox.bottom = static_cast<UINT>(safeArea.y + safeArea.height);
-    srcBox.front  = 0;
-    srcBox.back   = 1;
+
+    // Select the correct layer from a 3D attachment
+    srcBox.front = 0;
+    if (textureHelper.getTextureType() == GL_TEXTURE_3D)
+    {
+        srcBox.front = static_cast<UINT>(srcAttachment.layer());
+    }
+    srcBox.back = srcBox.front + 1;
 
     mDeviceContext->CopySubresourceRegion(stagingHelper.getResource(), 0, 0, 0, 0,
-                                          srcTexture->getResource(), subResource, &srcBox);
+                                          srcTexture->getResource(), sourceSubResource, &srcBox);
 
     PackPixelsParams packParams(safeArea, format, type, outputPitch, pack, 0);
-    return packPixels(stagingHelper, packParams, pixels);
+    return packPixels(stagingHelper, packParams, pixelsOut);
 }
 
 gl::Error Renderer11::packPixels(const TextureHelper11 &textureHelper,

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

@@ -264,14 +264,13 @@ class Renderer11 : public RendererD3D
     VertexConversionType getVertexConversionType(gl::VertexFormatType vertexFormatType) const override;
     GLenum getVertexComponentType(gl::VertexFormatType vertexFormatType) const override;
 
-    gl::Error readTextureData(const TextureHelper11 &textureHelper,
-                              unsigned int subResource,
-                              const gl::Rectangle &area,
-                              GLenum format,
-                              GLenum type,
-                              GLuint outputPitch,
-                              const gl::PixelPackState &pack,
-                              uint8_t *pixels);
+    gl::Error readFromAttachment(const gl::FramebufferAttachment &srcAttachment,
+                                 const gl::Rectangle &sourceArea,
+                                 GLenum format,
+                                 GLenum type,
+                                 GLuint outputPitch,
+                                 const gl::PixelPackState &pack,
+                                 uint8_t *pixels);
 
     void setShaderResource(gl::SamplerType shaderType, UINT resourceSlot, ID3D11ShaderResourceView *srv);
 

src/tests/gl_tests/ReadPixelsTest.cpp

@@ -506,26 +506,12 @@ TEST_P(ReadPixelsTextureTest, MipAttachment3D)
 // Test 3D attachment readback, non-zero layer.
 TEST_P(ReadPixelsTextureTest, LayerAttachment3D)
 {
-    if (isD3D11())
-    {
-        // TODO(jmadill): Fix ReadPixels from non-zero layer attachments.
-        std::cout << "Test disabled on D3D11." << std::endl;
-        return;
-    }
-
     testRead(GL_TEXTURE_3D, 1, 0, 1);
 }
 
 // Test 3D attachment readback, non-zero mip and layer.
 TEST_P(ReadPixelsTextureTest, MipLayerAttachment3D)
 {
-    if (isD3D11())
-    {
-        // TODO(jmadill): Fix ReadPixels from non-zero layer attachments.
-        std::cout << "Test disabled on D3D11." << std::endl;
-        return;
-    }
-
     testRead(GL_TEXTURE_3D, 2, 1, 1);
 }