Refactor vulkan buffer mapping and unmapping

src/gpu/vk/GrVkBuffer.cpp

Index: src/gpu/vk/GrVkBuffer.cpp
diff --git a/src/gpu/vk/GrVkBuffer.cpp b/src/gpu/vk/GrVkBuffer.cpp
index a03ea0cc41ef43ba0c4f71b7b4eaa9054409c27f..8a8af9a88876571c42c7ef9f9a1c9c9ad3c603bf 100644
--- a/src/gpu/vk/GrVkBuffer.cpp
+++ b/src/gpu/vk/GrVkBuffer.cpp
@@ -121,43 +121,87 @@ void GrVkBuffer::vkAbandon() {
     VALIDATE();
 }
 
-void* GrVkBuffer::vkMap(const GrVkGpu* gpu) {
+void* GrVkBuffer::vkMap(GrVkGpu* gpu) {
+    this->innerMap(gpu, fDesc.fSizeInBytes);
+    return fMapPtr;
+}
+
+VkAccessFlags buffer_type_to_access_flags(GrVkBuffer::Type type) {
+    switch (type) {
+        case GrVkBuffer::kIndex_Type:
+            return VK_ACCESS_INDEX_READ_BIT;
+        case GrVkBuffer::kVertex_Type:
+            return VK_ACCESS_VERTEX_ATTRIBUTE_READ_BIT;
+        default:
+            // This helper is only called for static buffers so we should only ever see index or
+            // vertex buffers types
+            SkASSERT(false);
+            return 0;
+    }
+}
+
+void GrVkBuffer::innerMap(GrVkGpu* gpu, size_t size, bool* createdNewBuffer) {
     VALIDATE();
     SkASSERT(!this->vkIsMapped());
+
     if (!fResource->unique()) {
-        // in use by the command buffer, so we need to create a new one
-        fResource->unref(gpu);
-        fResource = Create(gpu, fDesc);
+        if (fDesc.fDynamic) {
+            // in use by the command buffer, so we need to create a new one
+            fResource->recycle(gpu);
+            fResource = this->createResource(gpu, fDesc);
+            if (createdNewBuffer) {
+                *createdNewBuffer = true;
+            }
+        } else {
+            SkASSERT(fMapPtr);

[egdaniel, 2016/09/16 20:00:28]

we now never delete the resources for static buffers. Before that was true in
updateData but not map.

+            this->addMemoryBarrier(gpu,
+                                   buffer_type_to_access_flags(fDesc.fType),
+                                   VK_ACCESS_TRANSFER_WRITE_BIT,
+                                   VK_PIPELINE_STAGE_VERTEX_INPUT_BIT,
+                                   VK_PIPELINE_STAGE_TRANSFER_BIT,
+                                   false);
+            delete [] (unsigned char*)fMapPtr;

[egdaniel, 2016/09/16 20:43:12]

We actually should not need to ever delete and realloc the cpu data since it is
copied to the cmdbuffer when updateBuffer is called.

+            fMapPtr = nullptr;
+        }
     }
 
     if (fDesc.fDynamic) {
         const GrVkAlloc& alloc = this->alloc();
         VkResult err = VK_CALL(gpu, MapMemory(gpu->device(), alloc.fMemory,
                                               alloc.fOffset + fOffset,
-                                              fDesc.fSizeInBytes, 0, &fMapPtr));
+                                              size, 0, &fMapPtr));
         if (err) {
             fMapPtr = nullptr;
         }
     } else {
-        fMapPtr = new unsigned char[this->size()];
+        if (!fMapPtr) {
+            fMapPtr = new unsigned char[this->size()];
+        }
     }
 
     VALIDATE();
-    return fMapPtr;
 }
 
 void GrVkBuffer::vkUnmap(GrVkGpu* gpu) {
+    this->innerUnmap(gpu, this->size());
+}
+
+void GrVkBuffer::innerUnmap(GrVkGpu* gpu, size_t size) {
     VALIDATE();
     SkASSERT(this->vkIsMapped());
 
     if (fDesc.fDynamic) {
         VK_CALL(gpu, UnmapMemory(gpu->device(), this->alloc().fMemory));
+        fMapPtr = nullptr;
     } else {
-        gpu->updateBuffer(this, fMapPtr, this->offset(), this->size());
-        delete [] (unsigned char*)fMapPtr;
+        gpu->updateBuffer(this, fMapPtr, this->offset(), size);
+        this->addMemoryBarrier(gpu,
+                               VK_ACCESS_TRANSFER_WRITE_BIT,
+                               buffer_type_to_access_flags(fDesc.fType),
+                               VK_PIPELINE_STAGE_TRANSFER_BIT,
+                               VK_PIPELINE_STAGE_VERTEX_INPUT_BIT,
+                               false);
     }
-
-    fMapPtr = nullptr;
 }
 
 bool GrVkBuffer::vkIsMapped() const {
@@ -167,39 +211,18 @@ bool GrVkBuffer::vkIsMapped() const {
 
 bool GrVkBuffer::vkUpdateData(GrVkGpu* gpu, const void* src, size_t srcSizeInBytes,
                               bool* createdNewBuffer) {
-    SkASSERT(!this->vkIsMapped());
-    VALIDATE();
     if (srcSizeInBytes > fDesc.fSizeInBytes) {
         return false;
     }
 
-    // TODO: update data based on buffer offset
-    if (!fDesc.fDynamic) {
-        return gpu->updateBuffer(this, src, fOffset, srcSizeInBytes);
-    }
-
-    if (!fResource->unique()) {
-        // in use by the command buffer, so we need to create a new one
-        fResource->recycle(gpu);
-        fResource = this->createResource(gpu, fDesc);
-        if (createdNewBuffer) {
-            *createdNewBuffer = true;
-        }
-    }
-
-    void* mapPtr;
-    const GrVkAlloc& alloc = this->alloc();
-    VkResult err = VK_CALL(gpu, MapMemory(gpu->device(), alloc.fMemory,
-                                          alloc.fOffset + fOffset,
-                                          srcSizeInBytes, 0, &mapPtr));
-
-    if (VK_SUCCESS != err) {
+    this->innerMap(gpu, srcSizeInBytes, createdNewBuffer);
+    if (!fMapPtr) {
         return false;
     }
 
-    memcpy(mapPtr, src, srcSizeInBytes);
+    memcpy(fMapPtr, src, srcSizeInBytes);
 
-    VK_CALL(gpu, UnmapMemory(gpu->device(), alloc.fMemory));
+    this->innerUnmap(gpu, srcSizeInBytes);
 
     return true;
 }