Vulkan: Refactor CopyTexImage to use readPixelsImpl.

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -13,7 +13,6 @@
 #include <array>
 
 #include "common/debug.h"
-#include "image_util/imageformats.h"
 #include "libANGLE/Context.h"
 #include "libANGLE/Display.h"
 #include "libANGLE/formatutils.h"
@@ -58,8 +57,6 @@ FramebufferVk *FramebufferVk::CreateDefaultFBO(const gl::FramebufferState &state
 FramebufferVk::FramebufferVk(const gl::FramebufferState &state)
     : FramebufferImpl(state),
       mBackbuffer(nullptr),
-      mRenderPassDesc(),
-      mFramebuffer(),
       mActiveColorComponents(0),
       mReadPixelsBuffer(VK_BUFFER_USAGE_TRANSFER_DST_BIT, kMinReadPixelsBufferSize)
 {
@@ -68,16 +65,12 @@ FramebufferVk::FramebufferVk(const gl::FramebufferState &state)
 FramebufferVk::FramebufferVk(const gl::FramebufferState &state, WindowSurfaceVk *backbuffer)
     : FramebufferImpl(state),
       mBackbuffer(backbuffer),
-      mRenderPassDesc(),
-      mFramebuffer(),
       mActiveColorComponents(0),
       mReadPixelsBuffer(VK_BUFFER_USAGE_TRANSFER_DST_BIT, kMinReadPixelsBufferSize)
 {
 }
 
-FramebufferVk::~FramebufferVk()
-{
-}
+FramebufferVk::~FramebufferVk() = default;
 
 void FramebufferVk::destroy(const gl::Context *context)
 {
@@ -317,20 +310,13 @@ gl::Error FramebufferVk::readPixels(const gl::Context *context,
     params.packBuffer  = glState.getTargetBuffer(gl::BufferBinding::PixelPack);
     params.pack        = glState.getPackState();
 
-    if (!mReadPixelsBuffer.valid())
-    {
-        mReadPixelsBuffer.init(1, renderer);
-        ASSERT(mReadPixelsBuffer.valid());
-    }
-
-    ANGLE_TRY(ReadPixelsFromRenderTarget(context, clippedArea, params, mReadPixelsBuffer,
-                                         getColorReadRenderTarget(), commandBuffer,
-                                         reinterpret_cast<uint8_t *>(pixels) + outputSkipBytes));
+    ANGLE_TRY(readPixelsImpl(context, clippedArea, params,
+                             reinterpret_cast<uint8_t *>(pixels) + outputSkipBytes));
     mReadPixelsBuffer.releaseRetainedBuffers(renderer);
     return gl::NoError();
 }
 
-RenderTargetVk *FramebufferVk::getColorReadRenderTarget()
+RenderTargetVk *FramebufferVk::getColorReadRenderTarget() const
 {
     RenderTargetVk *renderTarget = mRenderTargetCache.getColorRead(mState);
     ASSERT(renderTarget && renderTarget->image->valid());
@@ -392,7 +378,7 @@ gl::Error FramebufferVk::syncState(const gl::Context *context,
                 }
                 else
                 {
-                    updateActiveColorMasks(colorIndex, 0, 0, 0, 0);
+                    updateActiveColorMasks(colorIndex, false, false, false, false);
                 }
                 break;
             }
@@ -706,16 +692,12 @@ gl::Error FramebufferVk::getCommandGraphNodeForDraw(ContextVk *contextVk,
 
     std::vector<VkClearValue> attachmentClearValues;
 
-    vk::CommandBuffer *commandBuffer = nullptr;
     if (!(*nodeOut)->getOutsideRenderPassCommands()->valid())
     {
+        vk::CommandBuffer *commandBuffer = nullptr;
         ANGLE_TRY((*nodeOut)->beginOutsideRenderPassRecording(
             renderer->getDevice(), renderer->getCommandPool(), &commandBuffer));
     }
-    else
-    {
-        commandBuffer = (*nodeOut)->getOutsideRenderPassCommands();
-    }
 
     // Initialize RenderPass info.
     // TODO(jmadill): Support gaps in RenderTargets. http://anglebug.com/2394
@@ -753,15 +735,24 @@ void FramebufferVk::updateActiveColorMasks(size_t colorIndex, bool r, bool g, bo
     mActiveColorComponentMasks[3].set(colorIndex, a);
 }
 
-gl::Error ReadPixelsFromRenderTarget(const gl::Context *context,
-                                     const gl::Rectangle &area,
-                                     const PackPixelsParams &packPixelsParams,
-                                     vk::DynamicBuffer &dynamicBuffer,
-                                     RenderTargetVk *renderTarget,
-                                     vk::CommandBuffer *commandBuffer,
-                                     void *pixels)
+gl::Error FramebufferVk::readPixelsImpl(const gl::Context *context,
+                                        const gl::Rectangle &area,
+                                        const PackPixelsParams &packPixelsParams,
+                                        void *pixels)
 {
-    RendererVk *renderer = vk::GetImpl(context)->getRenderer();
+    ContextVk *contextVk = vk::GetImpl(context);
+    RendererVk *renderer = contextVk->getRenderer();
+
+    if (!mReadPixelsBuffer.valid())
+    {
+        mReadPixelsBuffer.init(1, renderer);
+        ASSERT(mReadPixelsBuffer.valid());
+    }
+
+    vk::CommandBuffer *commandBuffer = nullptr;
+    ANGLE_TRY(beginWriteResource(renderer, &commandBuffer));
+
+    RenderTargetVk *renderTarget = getColorReadRenderTarget();
 
     vk::ImageHelper *renderTargetImage = renderTarget->image;
     const angle::Format &angleFormat   = renderTargetImage->getFormat().textureFormat();
@@ -771,8 +762,8 @@ gl::Error ReadPixelsFromRenderTarget(const gl::Context *context,
     uint32_t stagingOffset             = 0;
     size_t allocationSize              = area.width * angleFormat.pixelBytes * area.height;
 
-    dynamicBuffer.allocate(renderer, allocationSize, &readPixelBuffer, &bufferHandle,
-                           &stagingOffset, &newBufferAllocated);
+    mReadPixelsBuffer.allocate(renderer, allocationSize, &readPixelBuffer, &bufferHandle,
+                               &stagingOffset, &newBufferAllocated);
 
     VkBufferImageCopy region;
     region.bufferImageHeight               = area.height;
@@ -803,11 +794,16 @@ gl::Error ReadPixelsFromRenderTarget(const gl::Context *context,
 
     // The buffer we copied to needs to be invalidated before we read from it because its not been
     // created with the host coherent bit.
-    ANGLE_TRY(dynamicBuffer.invalidate(renderer->getDevice()));
+    ANGLE_TRY(mReadPixelsBuffer.invalidate(renderer->getDevice()));
 
     PackPixels(packPixelsParams, angleFormat, area.width * angleFormat.pixelBytes, readPixelBuffer,
                reinterpret_cast<uint8_t *>(pixels));
 
     return vk::NoError();
 }
+
+const gl::Extents &FramebufferVk::getReadImageExtents() const
+{
+    return getColorReadRenderTarget()->image->getExtents();
+}
 }  // namespace rx

src/libANGLE/renderer/vulkan/FramebufferVk.h

@@ -72,7 +72,6 @@ class FramebufferVk : public FramebufferImpl, public vk::CommandGraphResource
                          GLenum format,
                          GLenum type,
                          void *pixels) override;
-    RenderTargetVk *getColorReadRenderTarget();
     gl::Error blit(const gl::Context *context,
                    const gl::Rectangle &sourceArea,
                    const gl::Rectangle &destArea,
@@ -91,6 +90,14 @@ class FramebufferVk : public FramebufferImpl, public vk::CommandGraphResource
     const vk::RenderPassDesc &getRenderPassDesc();
     gl::Error getCommandGraphNodeForDraw(ContextVk *contextVk, vk::CommandGraphNode **nodeOut);
 
+    // Internal helper function for readPixels operations.
+    gl::Error readPixelsImpl(const gl::Context *context,
+                             const gl::Rectangle &area,
+                             const PackPixelsParams &packPixelsParams,
+                             void *pixels);
+
+    const gl::Extents &getReadImageExtents() const;
+
   private:
     FramebufferVk(const gl::FramebufferState &state);
     FramebufferVk(const gl::FramebufferState &state, WindowSurfaceVk *backbuffer);
@@ -103,6 +110,7 @@ class FramebufferVk : public FramebufferImpl, public vk::CommandGraphResource
                                         bool clearStencil);
     gl::Error clearWithDraw(ContextVk *contextVk, VkColorComponentFlags colorMaskFlags);
     void updateActiveColorMasks(size_t colorIndex, bool r, bool g, bool b, bool a);
+    RenderTargetVk *getColorReadRenderTarget() const;
 
     WindowSurfaceVk *mBackbuffer;
 
@@ -118,15 +126,6 @@ class FramebufferVk : public FramebufferImpl, public vk::CommandGraphResource
 
     vk::DynamicBuffer mReadPixelsBuffer;
 };
-
-gl::Error ReadPixelsFromRenderTarget(const gl::Context *context,
-                                     const gl::Rectangle &area,
-                                     const PackPixelsParams &packPixelsParams,
-                                     vk::DynamicBuffer &dynamicBuffer,
-                                     RenderTargetVk *renderTarget,
-                                     vk::CommandBuffer *commandBuffer,
-                                     void *pixels);
-
 }  // namespace rx
 
 #endif  // LIBANGLE_RENDERER_VULKAN_FRAMEBUFFERVK_H_

src/libANGLE/renderer/vulkan/TextureVk.cpp

@@ -148,19 +148,18 @@ gl::Error PixelBuffer::stageSubresourceUpdate(ContextVk *contextVk,
     return gl::NoError();
 }
 
-gl::Error PixelBuffer::stageSubresourceUpdateFromRenderTarget(const gl::Context *context,
-                                                              const gl::ImageIndex &index,
-                                                              const gl::Rectangle &sourceArea,
-                                                              const gl::Offset &dstOffset,
-                                                              const gl::Extents &dstExtent,
-                                                              const gl::InternalFormat &formatInfo,
-                                                              vk::CommandBuffer *commandBuffer,
-                                                              RenderTargetVk *renderTarget)
+gl::Error PixelBuffer::stageSubresourceUpdateFromFramebuffer(const gl::Context *context,
+                                                             const gl::ImageIndex &index,
+                                                             const gl::Rectangle &sourceArea,
+                                                             const gl::Offset &dstOffset,
+                                                             const gl::Extents &dstExtent,
+                                                             const gl::InternalFormat &formatInfo,
+                                                             FramebufferVk *framebufferVk)
 {
     // If the extents and offset is outside the source image, we need to clip.
     gl::Rectangle clippedRectangle;
-    const gl::Extents imageExtents = renderTarget->image->getExtents();
-    if (!ClipRectangle(sourceArea, gl::Rectangle(0, 0, imageExtents.width, imageExtents.height),
+    const gl::Extents readExtents = framebufferVk->getReadImageExtents();
+    if (!ClipRectangle(sourceArea, gl::Rectangle(0, 0, readExtents.width, readExtents.height),
                        &clippedRectangle))
     {
         // Empty source area, nothing to do.
@@ -206,8 +205,7 @@ gl::Error PixelBuffer::stageSubresourceUpdateFromRenderTarget(const gl::Context
         ANGLE_TRY(context->getScratchBuffer(bufferSize, &memoryBuffer));
 
         // Read into the scratch buffer
-        ANGLE_TRY(ReadPixelsFromRenderTarget(context, sourceArea, params, mStagingBuffer,
-                                             renderTarget, commandBuffer, memoryBuffer->data()));
+        ANGLE_TRY(framebufferVk->readPixelsImpl(context, sourceArea, params, memoryBuffer->data()));
 
         // Load from scratch buffer to our pixel buffer
         loadFunction.loadFunction(sourceArea.width, sourceArea.height, 1, memoryBuffer->data(),
@@ -216,8 +214,7 @@ gl::Error PixelBuffer::stageSubresourceUpdateFromRenderTarget(const gl::Context
     else
     {
         // We read directly from the framebuffer into our pixel buffer.
-        ANGLE_TRY(ReadPixelsFromRenderTarget(context, sourceArea, params, mStagingBuffer,
-                                             renderTarget, commandBuffer, stagingPointer));
+        ANGLE_TRY(framebufferVk->readPixelsImpl(context, sourceArea, params, stagingPointer));
     }
 
     // 3- enqueue the destination image subresource update
@@ -450,19 +447,15 @@ gl::Error TextureVk::copySubImageImpl(const gl::Context *context,
     ContextVk *contextVk = vk::GetImpl(context);
 
     FramebufferVk *framebufferVk = vk::GetImpl(source);
-    RenderTargetVk *renderTarget = framebufferVk->getColorReadRenderTarget();
-
-    vk::CommandBuffer *commandBuffer = nullptr;
-    ANGLE_TRY(framebufferVk->beginWriteResource(contextVk->getRenderer(), &commandBuffer));
 
     // For now, favor conformance. We do a CPU readback that does the conversion, and then stage the
     // change to the pixel buffer.
     // Eventually we can improve this easily by implementing vkCmdBlitImage to do the conversion
     // when its supported.
-    ANGLE_TRY(mPixelBuffer.stageSubresourceUpdateFromRenderTarget(
+    ANGLE_TRY(mPixelBuffer.stageSubresourceUpdateFromFramebuffer(
         context, index, clippedSourceArea, modifiedDestOffset,
         gl::Extents(clippedSourceArea.width, clippedSourceArea.height, 1), internalFormat,
-        commandBuffer, renderTarget));
+        framebufferVk));
 
     vk::CommandGraphNode *writingNode = getNewWritingNode(contextVk->getRenderer());
     framebufferVk->onReadResource(writingNode, contextVk->getRenderer()->getCurrentQueueSerial());

src/libANGLE/renderer/vulkan/TextureVk.h

@@ -35,14 +35,13 @@ class PixelBuffer final : angle::NonCopyable
                                      GLenum type,
                                      const uint8_t *pixels);
 
-    gl::Error stageSubresourceUpdateFromRenderTarget(const gl::Context *context,
-                                                     const gl::ImageIndex &index,
-                                                     const gl::Rectangle &sourceArea,
-                                                     const gl::Offset &dstOffset,
-                                                     const gl::Extents &dstExtent,
-                                                     const gl::InternalFormat &formatInfo,
-                                                     vk::CommandBuffer *commandBuffer,
-                                                     RenderTargetVk *renderTarget);
+    gl::Error stageSubresourceUpdateFromFramebuffer(const gl::Context *context,
+                                                    const gl::ImageIndex &index,
+                                                    const gl::Rectangle &sourceArea,
+                                                    const gl::Offset &dstOffset,
+                                                    const gl::Extents &dstExtent,
+                                                    const gl::InternalFormat &formatInfo,
+                                                    FramebufferVk *framebufferVk);
 
     vk::Error flushUpdatesToImage(RendererVk *renderer,
                                   vk::ImageHelper *image,