layers: Refactor DescriptorSet Update code

This migrates code for DescriptorSet in_use and flagging bound
command buffers as invalid from core_validation.cpp into DescriptorSet
class.

3 files changed, 91 insertions, 55 deletions

diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp
index f81a0c37..98c45c03 100644
--- a/layers/core_validation.cpp
+++ b/layers/core_validation.cpp
@@ -3381,61 +3381,67 @@ static bool validateIdleDescriptorSet(const layer_data *my_data, VkDescriptorSet
 }
 static void invalidateBoundCmdBuffers(layer_data *dev_data, const cvdescriptorset::DescriptorSet *pSet) {
     // Flag any CBs this set is bound to as INVALID
-    for (auto cb : pSet->GetBoundCmdBuffers()) {
-        auto cb_node = dev_data->commandBufferMap.find(cb);
-        if (cb_node != dev_data->commandBufferMap.end()) {
-            cb_node->second->state = CB_INVALID;
-        }
+    for (auto cb_node : pSet->GetBoundCmdBuffers()) {
+        cb_node->state = CB_INVALID;
     }
 }
 // update DS mappings based on write and copy update arrays
 static bool dsUpdate(layer_data *my_data, VkDevice device, uint32_t descriptorWriteCount, const VkWriteDescriptorSet *pWDS,
                      uint32_t descriptorCopyCount, const VkCopyDescriptorSet *pCDS) {
-    bool skipCall = false;
+    bool skip_call = false;
     // Validate Write updates
     uint32_t i = 0;
     for (i = 0; i < descriptorWriteCount; i++) {
-        VkDescriptorSet ds = pWDS[i].dstSet;
-        cvdescriptorset::DescriptorSet *pSet = my_data->setMap[ds];
-        // Set being updated cannot be in-flight
-        if ((skipCall = validateIdleDescriptorSet(my_data, ds, "VkUpdateDescriptorSets")) == true)
-            return skipCall;
-        // If set is bound to any cmdBuffers, mark them invalid
-        invalidateBoundCmdBuffers(my_data, pSet);
-        GENERIC_HEADER *pUpdate = (GENERIC_HEADER *)&pWDS[i];
-        // First verify valid update struct
-        if ((skipCall = validUpdateStruct(my_data, device, pUpdate)) == true) {
-            break;
-        }
-        string error_str;
-        if (!pSet->WriteUpdate(my_data->report_data, &pWDS[i], &error_str)) {
-            skipCall |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT,
-                                (uint64_t)(ds), __LINE__, DRAWSTATE_INVALID_UPDATE_INDEX, "DS",
-                                "vkUpdateDescriptorsSets() failed write update for Descriptor Set 0x%" PRIx64 " with error: %s",
-                                reinterpret_cast<uint64_t &>(ds), error_str.c_str());
+        auto dest_set = pWDS[i].dstSet;
+        auto set_pair = my_data->setMap.find(dest_set);
+        if (set_pair == my_data->setMap.end()) {
+            skip_call |=
+                log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT,
+                        reinterpret_cast<uint64_t &>(dest_set), __LINE__, DRAWSTATE_DOUBLE_DESTROY, "DS",
+                        "Cannot call vkUpdateDescriptorSets() on descriptor set 0x%" PRIxLEAST64 " that has not been allocated.",
+                        reinterpret_cast<uint64_t &>(dest_set));
+        } else {
+            string error_str;
+            if (!set_pair->second->WriteUpdate(my_data->report_data, &pWDS[i], &error_str)) {
+                skip_call |=
+                    log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT,
+                            reinterpret_cast<uint64_t &>(dest_set), __LINE__, DRAWSTATE_INVALID_UPDATE_INDEX, "DS",
+                            "vkUpdateDescriptorsSets() failed write update for Descriptor Set 0x%" PRIx64 " with error: %s",
+                            reinterpret_cast<uint64_t &>(dest_set), error_str.c_str());
+            }
         }
     }
     // Now validate copy updates
     for (i = 0; i < descriptorCopyCount; ++i) {
-        cvdescriptorset::DescriptorSet *pSrcSet = NULL, *pDstSet = NULL;
-        // For each copy make sure that update falls within given layout and that types match
-        pSrcSet = my_data->setMap[pCDS[i].srcSet];
-        pDstSet = my_data->setMap[pCDS[i].dstSet];
-        // Set being updated cannot be in-flight
-        if ((skipCall = validateIdleDescriptorSet(my_data, pDstSet->GetSet(), "VkUpdateDescriptorSets")) == true)
-            return skipCall;
-        invalidateBoundCmdBuffers(my_data, pDstSet);
-        std::string error_str;
-        if (!pDstSet->CopyUpdate(my_data->report_data, &pCDS[i], pSrcSet, &error_str)) {
-            skipCall |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT,
-                                reinterpret_cast<const uint64_t &>(pCDS[i].dstSet), __LINE__, DRAWSTATE_INVALID_UPDATE_INDEX, "DS",
-                                "vkUpdateDescriptorsSets() failed copy update from Descriptor Set 0x%" PRIx64
-                                " to Descriptor Set 0x%" PRIx64 " with error: %s",
-                                reinterpret_cast<const uint64_t &>(pCDS[i].srcSet),
-                                reinterpret_cast<const uint64_t &>(pCDS[i].dstSet), error_str.c_str());
+        auto dst_set = pCDS[i].dstSet;
+        auto src_set = pCDS[i].srcSet;
+        auto src_pair = my_data->setMap.find(src_set);
+        auto dst_pair = my_data->setMap.find(dst_set);
+        if (src_pair == my_data->setMap.end()) {
+            skip_call |= log_msg(
+                my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT,
+                reinterpret_cast<uint64_t &>(src_set), __LINE__, DRAWSTATE_DOUBLE_DESTROY, "DS",
+                "Cannot call vkUpdateDescriptorSets() to copy from descriptor set 0x%" PRIxLEAST64 " that has not been allocated.",
+                reinterpret_cast<uint64_t &>(src_set));
+        } else if (dst_pair == my_data->setMap.end()) {
+            skip_call |= log_msg(
+                my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT,
+                reinterpret_cast<uint64_t &>(dst_set), __LINE__, DRAWSTATE_DOUBLE_DESTROY, "DS",
+                "Cannot call vkUpdateDescriptorSets() to copy to descriptor set 0x%" PRIxLEAST64 " that has not been allocated.",
+                reinterpret_cast<uint64_t &>(dst_set));
+        } else {
+            std::string error_str;
+            if (!dst_pair->second->CopyUpdate(my_data->report_data, &pCDS[i], src_pair->second, &error_str)) {
+                skip_call |=
+                    log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT,
+                            reinterpret_cast<uint64_t &>(dst_set), __LINE__, DRAWSTATE_INVALID_UPDATE_INDEX, "DS",
+                            "vkUpdateDescriptorsSets() failed copy update from Descriptor Set 0x%" PRIx64
+                            " to Descriptor Set 0x%" PRIx64 " with error: %s",
+                            reinterpret_cast<uint64_t &>(src_set), reinterpret_cast<uint64_t &>(dst_set), error_str.c_str());
+            }
         }
     }
-    return skipCall;
+    return skip_call;
 }
 
 // Verify that given pool has descriptors that are being requested for allocation.
@@ -3701,7 +3707,7 @@ static void resetCB(layer_data *dev_data, const VkCommandBuffer cb) {
             for (auto set : pCB->lastBound[i].uniqueBoundSets) {
                 auto set_node = dev_data->setMap.find(set);
                 if (set_node != dev_data->setMap.end()) {
-                    set_node->second->RemoveBoundCommandBuffer(pCB->commandBuffer);
+                    set_node->second->RemoveBoundCommandBuffer(pCB);
                 }
             }
             pCB->lastBound[i].reset();
@@ -5995,8 +6001,8 @@ AllocateDescriptorSets(VkDevice device, const VkDescriptorSetAllocateInfo *pAllo
                 }
                 // Create new DescriptorSet instance and add to the pool's unordered_set of DescriptorSets
                 cvdescriptorset::DescriptorSet *pNewNode = new cvdescriptorset::DescriptorSet(
-                    pDescriptorSets[i], layout_pair->second, &dev_data->bufferMap, &dev_data->memObjMap, &dev_data->bufferViewMap,
-                    &dev_data->samplerMap, &dev_data->imageViewMap, &dev_data->imageMap,
+                    pDescriptorSets[i], layout_pair->second, dev_data->report_data, &dev_data->bufferMap, &dev_data->memObjMap,
+                    &dev_data->bufferViewMap, &dev_data->samplerMap, &dev_data->imageViewMap, &dev_data->imageMap,
                     &dev_data->device_extensions.imageToSwapchainMap, &dev_data->device_extensions.swapchainMap);
                 if (NULL == pNewNode) {
                     if (log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
@@ -6509,7 +6515,7 @@ CmdBindDescriptorSets(VkCommandBuffer commandBuffer, VkPipelineBindPoint pipelin
                 cvdescriptorset::DescriptorSet *pSet = getSetNode(dev_data, pDescriptorSets[i]);
                 if (pSet) {
                     pCB->lastBound[pipelineBindPoint].uniqueBoundSets.insert(pDescriptorSets[i]);
-                    pSet->BindCommandBuffer(commandBuffer);
+                    pSet->BindCommandBuffer(pCB);
                     pCB->lastBound[pipelineBindPoint].pipelineLayout = layout;
                     pCB->lastBound[pipelineBindPoint].boundDescriptorSets[i + firstSet] = pDescriptorSets[i];
                     skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_INFORMATION_BIT_EXT,
diff --git a/layers/descriptor_sets.cpp b/layers/descriptor_sets.cpp
index 89c44c68..437dcdc3 100644
--- a/layers/descriptor_sets.cpp
+++ b/layers/descriptor_sets.cpp
@@ -260,6 +260,7 @@ bool cvdescriptorset::DescriptorSetLayout::VerifyUpdateConsistency(uint32_t curr
 }
 
 cvdescriptorset::DescriptorSet::DescriptorSet(const VkDescriptorSet set, const DescriptorSetLayout *layout,
+                                              const debug_report_data *debug_report_data,
                                               const std::unordered_map<VkBuffer, BUFFER_NODE> *buffer_map,
                                               const std::unordered_map<VkDeviceMemory, DEVICE_MEM_INFO> *memory_map,
                                               const std::unordered_map<VkBufferView, VkBufferViewCreateInfo> *buffer_view_map,
@@ -268,9 +269,9 @@ cvdescriptorset::DescriptorSet::DescriptorSet(const VkDescriptorSet set, const D
                                               const std::unordered_map<VkImage, IMAGE_NODE> *image_map,
                                               const std::unordered_map<VkImage, VkSwapchainKHR> *image_to_swapchain_map,
                                               const std::unordered_map<VkSwapchainKHR, SWAPCHAIN_NODE *> *swapchain_map)
-    : some_update_(false), set_(set), p_layout_(layout), buffer_map_(buffer_map), memory_map_(memory_map),
-      buffer_view_map_(buffer_view_map), sampler_map_(sampler_map), image_view_map_(image_view_map), image_map_(image_map),
-      image_to_swapchain_map_(image_to_swapchain_map), swapchain_map_(swapchain_map) {
+    : some_update_(false), set_(set), p_layout_(layout), report_data_(debug_report_data), buffer_map_(buffer_map),
+      memory_map_(memory_map), buffer_view_map_(buffer_view_map), sampler_map_(sampler_map), image_view_map_(image_view_map),
+      image_map_(image_map), image_to_swapchain_map_(image_to_swapchain_map), swapchain_map_(swapchain_map) {
     // Foreach binding, create default descriptors of given type
     for (uint32_t i = 0; i < p_layout_->GetBindingCount(); ++i) {
         auto type = p_layout_->GetTypeFromIndex(i);
@@ -456,6 +457,14 @@ uint32_t cvdescriptorset::DescriptorSet::GetAllStorageUpdates(std::unordered_set
 bool cvdescriptorset::DescriptorSet::WriteUpdate(debug_report_data *report_data, const VkWriteDescriptorSet *update,
                                                  std::string *error_msg) {
     auto num_updates = 0;
+    // Verify idle ds
+    if (in_use.load()) {
+        std::stringstream error_str;
+        error_str << "Cannot call vkUpdateDescriptorSets() to perform write update on descriptor set " << set_
+                  << " that is in use by a command buffer.";
+        *error_msg = error_str.str();
+        return false;
+    }
     // Verify dst binding exists
     if (!p_layout_->HasBinding(update->dstBinding)) {
         std::stringstream error_str;
@@ -505,12 +514,24 @@ bool cvdescriptorset::DescriptorSet::WriteUpdate(debug_report_data *report_data,
     if (num_updates != 0) {
         some_update_ = true;
     }
+    // Invalidate any bound command buffers
+    for (auto cb_node : bound_cmd_buffers_) {
+        cb_node->state = CB_INVALID;
+    }
     return true;
 }
 // Copy update
 bool cvdescriptorset::DescriptorSet::CopyUpdate(debug_report_data *report_data, const VkCopyDescriptorSet *update,
                                                 const DescriptorSet *src_set, std::string *error) {
     auto num_updates = 0;
+    // Verify idle ds
+    if (in_use.load()) {
+        std::stringstream error_str;
+        error_str << "Cannot call vkUpdateDescriptorSets() to perform copy update on descriptor set " << set_
+                  << " that is in use by a command buffer.";
+        *error = error_str.str();
+        return false;
+    }
     if (!p_layout_->HasBinding(update->dstBinding)) {
         std::stringstream error_str;
         error_str << "DescriptorSet " << set_ << " does not have copy update dest binding of " << update->dstBinding << ".";
@@ -574,6 +595,10 @@ bool cvdescriptorset::DescriptorSet::CopyUpdate(debug_report_data *report_data,
     if (num_updates != 0) {
         some_update_ = true;
     }
+    // Invalidate any bound command buffers
+    for (auto cb_node : bound_cmd_buffers_) {
+        cb_node->state = CB_INVALID;
+    }
     return true;
 }
 cvdescriptorset::SamplerDescriptor::SamplerDescriptor(
@@ -974,4 +999,4 @@ bool cvdescriptorset::TexelDescriptor::CopyUpdate(const Descriptor *src, std::st
     updated = true;
     buffer_view_ = buffer_view;
     return true;
-}
\ No newline at end of file
+}
diff --git a/layers/descriptor_sets.h b/layers/descriptor_sets.h
index 92fda958..5b358d63 100644
--- a/layers/descriptor_sets.h
+++ b/layers/descriptor_sets.h
@@ -259,6 +259,9 @@ class BufferDescriptor : public Descriptor {
     VkDeviceSize range_;
     const std::unordered_map<VkBuffer, BUFFER_NODE> *buffer_map_;
 };
+// Helper function for Updating descriptor sets since it crosses multiple sets
+void UpdateDescriptorSets(VkDevice device, uint32_t descriptorWriteCount, const VkWriteDescriptorSet *pDescriptorWrites,
+                          uint32_t descriptorCopyCount, const VkCopyDescriptorSet *pDescriptorCopies);
 /*
  * DescriptorSet class
  *
@@ -279,8 +282,9 @@ class BufferDescriptor : public Descriptor {
  */
 class DescriptorSet : public BASE_NODE {
   public:
-    DescriptorSet(const VkDescriptorSet, const DescriptorSetLayout *, const std::unordered_map<VkBuffer, BUFFER_NODE> *,
-                  const std::unordered_map<VkDeviceMemory, DEVICE_MEM_INFO> *,
+    using BASE_NODE::in_use;
+    DescriptorSet(const VkDescriptorSet, const DescriptorSetLayout *, const debug_report_data *,
+                  const std::unordered_map<VkBuffer, BUFFER_NODE> *, const std::unordered_map<VkDeviceMemory, DEVICE_MEM_INFO> *,
                   const std::unordered_map<VkBufferView, VkBufferViewCreateInfo> *,
                   const std::unordered_map<VkSampler, std::unique_ptr<SAMPLER_NODE>> *,
                   const std::unordered_map<VkImageView, VkImageViewCreateInfo> *, const std::unordered_map<VkImage, IMAGE_NODE> *,
@@ -327,11 +331,11 @@ class DescriptorSet : public BASE_NODE {
     const DescriptorSetLayout *GetLayout() const { return p_layout_; };
     VkDescriptorSet GetSet() const { return set_; };
     // Return unordered_set of all command buffers that this set is bound to
-    std::unordered_set<VkCommandBuffer> GetBoundCmdBuffers() const { return bound_cmd_buffers_; }
+    std::unordered_set<GLOBAL_CB_NODE *> GetBoundCmdBuffers() const { return bound_cmd_buffers_; }
     // Bind given cmd_buffer to this descriptor set
-    void BindCommandBuffer(const VkCommandBuffer cmd_buffer) { bound_cmd_buffers_.insert(cmd_buffer); }
+    void BindCommandBuffer(GLOBAL_CB_NODE *cb_node) { bound_cmd_buffers_.insert(cb_node); }
     // If given cmd_buffer is in the bound_cmd_buffers_ set, remove it
-    void RemoveBoundCommandBuffer(const VkCommandBuffer cmd_buffer) { bound_cmd_buffers_.erase(cmd_buffer); }
+    void RemoveBoundCommandBuffer(GLOBAL_CB_NODE *cb_node) { bound_cmd_buffers_.erase(cb_node); }
     VkSampler const *GetImmutableSamplerPtrFromBinding(const uint32_t index) const {
         return p_layout_->GetImmutableSamplerPtrFromBinding(index);
     };
@@ -351,8 +355,9 @@ class DescriptorSet : public BASE_NODE {
     VkDescriptorSet set_;
     uint32_t descriptor_count_; // Count of all descriptors in this set
     const DescriptorSetLayout *p_layout_;
-    std::unordered_set<VkCommandBuffer> bound_cmd_buffers_;
+    std::unordered_set<GLOBAL_CB_NODE *> bound_cmd_buffers_;
     std::vector<std::unique_ptr<Descriptor>> descriptors_;
+    const debug_report_data *report_data_;
     // Ptrs to object containers to verify bound data
     const std::unordered_map<VkBuffer, BUFFER_NODE> *buffer_map_;
     const std::unordered_map<VkDeviceMemory, DEVICE_MEM_INFO> *memory_map_;