Use transfer buffer for BatchAtlas texture copies

src/gpu/vk/GrVkGpu.cpp

Index: src/gpu/vk/GrVkGpu.cpp
diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index 28dce344be37659f0e18b4fbd768dc0b7164461f..88b9542a032c51753083d7c2b935e0959b30df5e 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -183,10 +183,12 @@ GrBuffer* GrVkGpu::onCreateBuffer(size_t size, GrBufferType type, GrAccessPatter
                      kStatic_GrAccessPattern == accessPattern);
             return GrVkIndexBuffer::Create(this, size, kDynamic_GrAccessPattern == accessPattern);
         case kXferCpuToGpu_GrBufferType:
-            SkASSERT(kStream_GrAccessPattern == accessPattern);
+        SkASSERT(kDynamic_GrAccessPattern == accessPattern ||
+                 kStream_GrAccessPattern == accessPattern);
             return GrVkTransferBuffer::Create(this, size, GrVkBuffer::kCopyRead_Type);
         case kXferGpuToCpu_GrBufferType:
-            SkASSERT(kStream_GrAccessPattern == accessPattern);
+        SkASSERT(kDynamic_GrAccessPattern == accessPattern ||
+                 kStream_GrAccessPattern == accessPattern);
             return GrVkTransferBuffer::Create(this, size, GrVkBuffer::kCopyWrite_Type);
         default:
             SkFAIL("Unknown buffer type.");
@@ -274,6 +276,100 @@ bool GrVkGpu::onWritePixels(GrSurface* surface,
     return false;
 }
 
+
+bool GrVkGpu::onTransferPixels(GrSurface* surface,
+                               int left, int top, int width, int height,
+                               GrPixelConfig config, GrBuffer* transferBuffer,
+                               size_t bufferOffset, size_t rowBytes) {
+    GrVkTexture* vkTex = static_cast<GrVkTexture*>(surface->asTexture());
+    if (!vkTex) {

[egdaniel, 2016/04/18 16:46:34]

why do you require that this is a texture for vulkan? seems like this could be
done on any generic VkImage. However you may need then check for multisampling
on render targets

[jvanverth1, 2016/04/19 17:14:27]

The expected use case for this is to transfer CPU-rendered data to textures.
Adding support for RTs adds a lot of conditionals -- I'd rather not complicate
the code unless we're sure we'll use it.

Also, it means that transferPixels works on different kinds of data between
OpenGL and Vulkan, which I think would confuse things at a higher level.

[egdaniel, 2016/04/19 17:22:18]

Well if we don't want to OpenGL and Vulkan be different, can we change the
function signature to take a GrTexture instead? If we leave it as GrSurface I
feel it should support RT if able.

[bsalomon, 2016/04/19 18:19:26]

If we are only planning to use it with textures and supporting RTs is
complicated on either GL or VK then it makes sense to me to make it take a
texture. 

+        return false;
+    }
+    GrVkTransferBuffer* vkBuffer = static_cast<GrVkTransferBuffer*>(transferBuffer);
+    if (!vkBuffer) {
+        return false;
+    }
+
+    // We assume Vulkan doesn't do sRGB <-> linear conversions when reading and writing pixels.
+    if (GrPixelConfigIsSRGB(surface->config()) != GrPixelConfigIsSRGB(config)) {
+        return false;
+    }
+
+    // TODO: Not clear how to handle y axis flip

[egdaniel, 2016/04/18 16:46:34]

well I guess there are two options here. First would be to create a second
buffer that we copy all the data row by row into but reversed. The second would
be to first do the copyBufferToImage into a temp surface with origin TopLeft,
and then do a blit to the dst surface to perform the flip

[jvanverth1, 2016/04/19 17:14:27]

The temp image seems like the best bet -- I'm going to wait until I get the
image allocator working for that. And again, the textures we're using for
atlases and path uploads have top left origins.

+    if (kBottomLeft_GrSurfaceOrigin == vkTex->origin()) {
+        return false;
+    }
+
+    bool success = false;
+    if (GrPixelConfigIsCompressed(vkTex->desc().fConfig)) {
+        // We check that config == desc.fConfig in GrGpu::getWritePixelsInfo()
+        SkASSERT(config == vkTex->desc().fConfig);
+        // TODO: add compressed texture support
+        // delete the following two lines and uncomment the two after that when ready
+        vkTex->unref();
+        return false;
+        //success = this->uploadCompressedTexData(vkTex->desc(), buffer, false, left, top, width,
+        //                                       height);
+    } else {
+        // make sure the unmap has finished
+        vkBuffer->addMemoryBarrier(this,
+                                   VK_ACCESS_HOST_WRITE_BIT,
+                                   VK_ACCESS_TRANSFER_READ_BIT,
+                                   VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT,
+                                   VK_PIPELINE_STAGE_TRANSFER_BIT,
+                                   false);
+
+        // Set up copy region
+        VkOffset3D offset = {

[egdaniel, 2016/04/18 16:46:34]

why not just inline this like subresource or extent? In writePixels we pulled it
out cause we did some work on settings its values for flipping y.

[jvanverth1, 2016/04/19 17:14:27]

Done.

+            left,
+            top,
+            0
+        };
+        size_t bpp = GrBytesPerPixel(config);
+
+        VkBufferImageCopy region;
+        memset(&region, 0, sizeof(VkBufferImageCopy));
+        region.bufferOffset = bufferOffset;
+        region.bufferRowLength = (uint32_t)(rowBytes/bpp);
+        region.bufferImageHeight = 0;
+        region.imageSubresource = { VK_IMAGE_ASPECT_COLOR_BIT, 0, 0, 1 };
+        region.imageOffset = offset;
+        region.imageExtent = { (uint32_t)width, (uint32_t)height, 1 };
+
+        // Change layout of our target so it can be copied to
+        VkImageLayout layout = vkTex->currentLayout();
+        VkPipelineStageFlags srcStageMask = GrVkMemory::LayoutToPipelineStageFlags(layout);
+        VkPipelineStageFlags dstStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT;
+        VkAccessFlags srcAccessMask = GrVkMemory::LayoutToSrcAccessMask(layout);
+        VkAccessFlags dstAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT;
+        vkTex->setImageLayout(this,
+                              VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
+                              srcAccessMask,
+                              dstAccessMask,
+                              srcStageMask,
+                              dstStageMask,
+                              false);
+
+        // Copy the buffer to the image
+        fCurrentCmdBuffer->copyBufferToImage(this,

[egdaniel, 2016/04/18 16:46:34]

if the surface is linearly tiled should we not just map and copy it directly?

[jvanverth1, 2016/04/19 17:14:27]

That would copy the data from the GPU to the CPU, and back to the GPU.

[egdaniel, 2016/04/19 17:22:18]

unless it is cached :), but we can punt on this for now until we add some
tracking to know if the gpu memory is cached on cpu or not.

+                                             vkBuffer,
+                                             vkTex,
+                                             VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
+                                             1,
+                                             &region);
+
+        // Submit the current command buffer to the Queue
+        this->submitCommandBuffer(kSkip_SyncQueue);
+    }
+
+    if (success) {
+        vkTex->texturePriv().dirtyMipMaps(true);
+        return true;
+    }
+
+    return false;
+}
+
 bool GrVkGpu::uploadTexData(GrVkTexture* tex,
                             int left, int top, int width, int height,
                             GrPixelConfig dataConfig,