Check descriptor writes for validity before updating in PostVS fetch

* This can happen for push descriptors that are declared but never used.

renderdoc/driver/vulkan/vk_common.cpp

@@ -794,6 +794,41 @@ FrameRefType GetRefType(VkDescriptorType descType)
   return eFrameRef_Read;
 }
 
+bool IsValid(const VkWriteDescriptorSet &write, uint32_t arrayElement)
+{
+  // this makes assumptions that only hold within the context of Serialise_InitialState below,
+  // specifically that if pTexelBufferView/pBufferInfo is set then we are using them. In the general
+  // case they can be garbage and we must ignore them based on the descriptorType
+
+  if(write.pTexelBufferView)
+    return write.pTexelBufferView[arrayElement] != VK_NULL_HANDLE;
+
+  if(write.pBufferInfo)
+    return write.pBufferInfo[arrayElement].buffer != VK_NULL_HANDLE;
+
+  if(write.pImageInfo)
+  {
+    // only these two types need samplers
+    bool needSampler = (write.descriptorType == VK_DESCRIPTOR_TYPE_SAMPLER ||
+                        write.descriptorType == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER);
+
+    // but all types that aren't just a sampler need an image
+    bool needImage = (write.descriptorType != VK_DESCRIPTOR_TYPE_SAMPLER);
+
+    if(needSampler && write.pImageInfo[arrayElement].sampler == VK_NULL_HANDLE)
+      return false;
+
+    if(needImage && write.pImageInfo[arrayElement].imageView == VK_NULL_HANDLE)
+      return false;
+
+    return true;
+  }
+
+  RDCERR("Encountered VkWriteDescriptorSet with no data!");
+
+  return false;
+}
+
 void DescriptorSetSlot::RemoveBindRefs(VkResourceRecord *record)
 {
   if(texelBufferView != VK_NULL_HANDLE)

renderdoc/driver/vulkan/vk_common.h

@@ -379,6 +379,8 @@ struct DescriptorSetSlot
 
 DECLARE_REFLECTION_STRUCT(DescriptorSetSlot);
 
+bool IsValid(const VkWriteDescriptorSet &write, uint32_t arrayElement);
+
 #define NUM_VK_IMAGE_ASPECTS 4
 #define VK_ACCESS_ALL_READ_BITS                                                        \
   (VK_ACCESS_INDIRECT_COMMAND_READ_BIT | VK_ACCESS_INDEX_READ_BIT |                    \

renderdoc/driver/vulkan/vk_initstate.cpp

@@ -675,41 +675,6 @@ static const char *NameOfType(VkResourceType type)
   return "VkResource";
 }
 
-static bool IsValid(const VkWriteDescriptorSet &write, uint32_t arrayElement)
-{
-  // this makes assumptions that only hold within the context of Serialise_InitialState below,
-  // specifically that if pTexelBufferView/pBufferInfo is set then we are using them. In the general
-  // case they can be garbage and we must ignore them based on the descriptorType
-
-  if(write.pTexelBufferView)
-    return write.pTexelBufferView[arrayElement] != VK_NULL_HANDLE;
-
-  if(write.pBufferInfo)
-    return write.pBufferInfo[arrayElement].buffer != VK_NULL_HANDLE;
-
-  if(write.pImageInfo)
-  {
-    // only these two types need samplers
-    bool needSampler = (write.descriptorType == VK_DESCRIPTOR_TYPE_SAMPLER ||
-                        write.descriptorType == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER);
-
-    // but all types that aren't just a sampler need an image
-    bool needImage = (write.descriptorType != VK_DESCRIPTOR_TYPE_SAMPLER);
-
-    if(needSampler && write.pImageInfo[arrayElement].sampler == VK_NULL_HANDLE)
-      return false;
-
-    if(needImage && write.pImageInfo[arrayElement].imageView == VK_NULL_HANDLE)
-      return false;
-
-    return true;
-  }
-
-  RDCERR("Encountered VkWriteDescriptorSet with no data!");
-
-  return false;
-}
-
 // second parameter isn't used, as we might be serialising init state for a deleted resource
 template <typename SerialiserType>
 bool WrappedVulkan::Serialise_InitialState(SerialiserType &ser, ResourceId id, WrappedVkRes *)

renderdoc/driver/vulkan/vk_postvs.cpp

@@ -1351,6 +1351,9 @@ void VulkanReplay::FetchVSOut(uint32_t eventId)
   // into them
   {
     std::vector<VkWriteDescriptorSet> descWrites;
+    std::vector<VkDescriptorImageInfo *> allocImgWrites;
+    std::vector<VkDescriptorBufferInfo *> allocBufWrites;
+    std::vector<VkBufferView *> allocBufViewWrites;
 
     // one for each descriptor type. 1 of each to start with plus enough for our internal resources,
     // we then increment for each descriptor we need to allocate
@@ -1556,6 +1559,7 @@ void VulkanReplay::FetchVSOut(uint32_t eventId)
               for(uint32_t w = 0; w < write.descriptorCount; w++)
                 out[w] = slot[w].imageInfo;
               write.pImageInfo = out;
+              allocImgWrites.push_back(out);
               break;
             }
             case VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER:
@@ -1565,6 +1569,7 @@ void VulkanReplay::FetchVSOut(uint32_t eventId)
               for(uint32_t w = 0; w < write.descriptorCount; w++)
                 out[w] = slot[w].texelBufferView;
               write.pTexelBufferView = out;
+              allocBufViewWrites.push_back(out);
               break;
             }
             case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER:
@@ -1576,12 +1581,47 @@ void VulkanReplay::FetchVSOut(uint32_t eventId)
               for(uint32_t w = 0; w < write.descriptorCount; w++)
                 out[w] = slot[w].bufferInfo;
               write.pBufferInfo = out;
+              allocBufWrites.push_back(out);
               break;
             }
             default: RDCERR("Unexpected descriptor type %d", write.descriptorType);
           }
 
-          descWrites.push_back(write);
+          // start with no descriptors
+          write.descriptorCount = 0;
+
+          for(uint32_t w = 0; w < bind.descriptorCount; w++)
+          {
+            // if this write is valid, we increment the descriptor count and continue
+            if(IsValid(write, w))
+            {
+              write.descriptorCount++;
+            }
+            else
+            {
+              // if this write isn't valid, then we first check to see if we had any previous
+              // pending writes in the array we were going to batch together, if so we add them.
+              if(write.descriptorCount > 0)
+                descWrites.push_back(write);
+
+              // skip past any previous descriptors we just wrote, as well as the current invalid
+              // one
+              if(write.pBufferInfo)
+                write.pBufferInfo += write.descriptorCount + 1;
+              if(write.pImageInfo)
+                write.pImageInfo += write.descriptorCount + 1;
+              if(write.pTexelBufferView)
+                write.pTexelBufferView += write.descriptorCount + 1;
+
+              // now start again from 0 descriptors, at the next array element
+              write.dstArrayElement += write.descriptorCount + 1;
+              write.descriptorCount = 0;
+            }
+          }
+
+          // if there are any left, add them here
+          if(write.descriptorCount > 0)
+            descWrites.push_back(write);
 
           // don't leak the arrays and cause double deletes, NULL them after each time
           write.pImageInfo = NULL;
@@ -1594,12 +1634,12 @@ void VulkanReplay::FetchVSOut(uint32_t eventId)
     m_pDriver->vkUpdateDescriptorSets(dev, (uint32_t)descWrites.size(), descWrites.data(), 0, NULL);
 
     // delete allocated arrays for descriptor writes
-    for(const VkWriteDescriptorSet &write : descWrites)
-    {
-      delete[] write.pBufferInfo;
-      delete[] write.pImageInfo;
-      delete[] write.pTexelBufferView;
-    }
+    for(VkDescriptorBufferInfo *a : allocBufWrites)
+      delete[] a;
+    for(VkDescriptorImageInfo *a : allocImgWrites)
+      delete[] a;
+    for(VkBufferView *a : allocBufViewWrites)
+      delete[] a;
   }
 
   // create pipeline layout with new descriptor set layouts