Don't create new descriptor set for vulkan uniforms every draw

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1872553005

Review URL: https://codereview.chromium.org/1872553005

diff --git a/src/gpu/vk/GrVkBuffer.cpp b/src/gpu/vk/GrVkBuffer.cpp
index 6791328..9b54dee 100644 (file)
--- a/src/gpu/vk/GrVkBuffer.cpp
+++ b/src/gpu/vk/GrVkBuffer.cpp
@@ -163,7 +163,8 @@ bool GrVkBuffer::vkIsMapped() const {
     return SkToBool(fMapPtr);
 }
 
-bool GrVkBuffer::vkUpdateData(const GrVkGpu* gpu, const void* src, size_t srcSizeInBytes) {
+bool GrVkBuffer::vkUpdateData(const GrVkGpu* gpu, const void* src, size_t srcSizeInBytes,
+                              bool* createdNewBuffer) {
     SkASSERT(!this->vkIsMapped());
     VALIDATE();
     if (srcSizeInBytes > fDesc.fSizeInBytes) {
@@ -174,6 +175,9 @@ bool GrVkBuffer::vkUpdateData(const GrVkGpu* gpu, const void* src, size_t srcSiz
         // in use by the command buffer, so we need to create a new one
         fResource->unref(gpu);
         fResource = Create(gpu, fDesc);
+        if (createdNewBuffer) {
+            *createdNewBuffer = true;
+        }
     }
 
     void* mapPtr;

diff --git a/src/gpu/vk/GrVkBuffer.h b/src/gpu/vk/GrVkBuffer.h
index 289ef30..141b2a3 100644 (file)
--- a/src/gpu/vk/GrVkBuffer.h
+++ b/src/gpu/vk/GrVkBuffer.h
@@ -73,7 +73,10 @@ protected:
 
     void* vkMap(const GrVkGpu* gpu);
     void vkUnmap(const GrVkGpu* gpu);
-    bool vkUpdateData(const GrVkGpu* gpu, const void* src, size_t srcSizeInBytes);
+    // If the caller passes in a non null createdNewBuffer, this function will set the bool to true
+    // if it creates a new VkBuffer to upload the data to.
+    bool vkUpdateData(const GrVkGpu* gpu, const void* src, size_t srcSizeInBytes,
+                      bool* createdNewBuffer = nullptr);
 
     void vkAbandon();
     void vkRelease(const GrVkGpu* gpu);

diff --git a/src/gpu/vk/GrVkPipelineState.cpp b/src/gpu/vk/GrVkPipelineState.cpp
index 3e442fb..e464848 100644 (file)
--- a/src/gpu/vk/GrVkPipelineState.cpp
+++ b/src/gpu/vk/GrVkPipelineState.cpp
@@ -200,17 +200,17 @@ void GrVkPipelineState::setData(GrVkGpu* gpu,
         this->writeSamplers(gpu, textureBindings);
     }
 
-
     if (fVertexUniformBuffer.get() || fFragmentUniformBuffer.get()) {
-        fUniformPoolManager.getNewDescriptorSet(gpu,
+        if (fDataManager.uploadUniformBuffers(gpu, fVertexUniformBuffer, fFragmentUniformBuffer) ||
+            VK_NULL_HANDLE == fDescriptorSets[GrVkUniformHandler::kUniformBufferDescSet]) {
+            fUniformPoolManager.getNewDescriptorSet(gpu,
                                        &fDescriptorSets[GrVkUniformHandler::kUniformBufferDescSet]);
-        this->writeUniformBuffers(gpu);
+            this->writeUniformBuffers(gpu);
+        }
     }
 }
 
 void GrVkPipelineState::writeUniformBuffers(const GrVkGpu* gpu) {
-    fDataManager.uploadUniformBuffers(gpu, fVertexUniformBuffer, fFragmentUniformBuffer);
-
     VkWriteDescriptorSet descriptorWrites[2];
     memset(descriptorWrites, 0, 2 * sizeof(VkWriteDescriptorSet));
 

diff --git a/src/gpu/vk/GrVkPipelineStateDataManager.cpp b/src/gpu/vk/GrVkPipelineStateDataManager.cpp
index 86066ab..60d7d7f 100644 (file)
--- a/src/gpu/vk/GrVkPipelineStateDataManager.cpp
+++ b/src/gpu/vk/GrVkPipelineStateDataManager.cpp
@@ -253,9 +253,10 @@ template<> struct set_uniform_matrix<4> {
     }
 };
 
-void GrVkPipelineStateDataManager::uploadUniformBuffers(const GrVkGpu* gpu,
+bool GrVkPipelineStateDataManager::uploadUniformBuffers(const GrVkGpu* gpu,
                                                         GrVkUniformBuffer* vertexBuffer,
                                                         GrVkUniformBuffer* fragmentBuffer) const {
+    bool updatedBuffer = false;
     if (vertexBuffer && fVertexUniformsDirty) {
         vertexBuffer->addMemoryBarrier(gpu,
                                        VK_ACCESS_UNIFORM_READ_BIT,
@@ -263,7 +264,8 @@ void GrVkPipelineStateDataManager::uploadUniformBuffers(const GrVkGpu* gpu,
                                        VK_PIPELINE_STAGE_VERTEX_SHADER_BIT,
                                        VK_PIPELINE_STAGE_HOST_BIT,
                                        false);
-        SkAssertResult(vertexBuffer->updateData(gpu, fVertexUniformData.get(), fVertexUniformSize));
+        SkAssertResult(vertexBuffer->updateData(gpu, fVertexUniformData.get(), fVertexUniformSize,
+                                                &updatedBuffer));
         fVertexUniformsDirty = false;
     }
 
@@ -275,7 +277,8 @@ void GrVkPipelineStateDataManager::uploadUniformBuffers(const GrVkGpu* gpu,
                                          VK_PIPELINE_STAGE_HOST_BIT,
                                          false);
         SkAssertResult(fragmentBuffer->updateData(gpu, fFragmentUniformData.get(),
-                                                  fFragmentUniformSize));
+                                                  fFragmentUniformSize, &updatedBuffer));
         fFragmentUniformsDirty = false;
     }
+    return updatedBuffer;
 }

diff --git a/src/gpu/vk/GrVkPipelineStateDataManager.h b/src/gpu/vk/GrVkPipelineStateDataManager.h
index b75d65c..089d170 100644 (file)
--- a/src/gpu/vk/GrVkPipelineStateDataManager.h
+++ b/src/gpu/vk/GrVkPipelineStateDataManager.h
@@ -46,7 +46,10 @@ public:
         SkFAIL("Only supported in NVPR, which is not in vulkan");
     }
 
-    void uploadUniformBuffers(const GrVkGpu* gpu,
+    // Returns true if either the vertex or fragment buffer needed to generate a new underlying
+    // VkBuffer object in order upload data. If true is returned, this is a signal to the caller
+    // that they will need to update the descriptor set that is using these buffers.
+    bool uploadUniformBuffers(const GrVkGpu* gpu,
                               GrVkUniformBuffer* vertexBuffer,
                               GrVkUniformBuffer* fragmentBuffer) const;
 private:

diff --git a/src/gpu/vk/GrVkUniformBuffer.h b/src/gpu/vk/GrVkUniformBuffer.h
index a894aec..37ede72 100644 (file)
--- a/src/gpu/vk/GrVkUniformBuffer.h
+++ b/src/gpu/vk/GrVkUniformBuffer.h
@@ -23,8 +23,11 @@ public:
     void unmap(const GrVkGpu* gpu) {
         this->vkUnmap(gpu);
     }
-    bool updateData(const GrVkGpu* gpu, const void* src, size_t srcSizeInBytes) {
-        return this->vkUpdateData(gpu, src, srcSizeInBytes);
+    // The output variable createdNewBuffer must be set to true if a new VkBuffer is created in
+    // order to upload the data
+    bool updateData(const GrVkGpu* gpu, const void* src, size_t srcSizeInBytes,
+                    bool* createdNewBuffer) {
+        return this->vkUpdateData(gpu, src, srcSizeInBytes, createdNewBuffer);
     }
     void release(const GrVkGpu* gpu) {
         this->vkRelease(gpu);