layers:Handle consecutive descriptor updates
Fixes #1165
According to spec descriptor updates should roll over to the next
binding number. If bindings were out of order, this was broken in
validation.
This fix corrects validation and state update for out-of-order
descriptor bindings by checking consecutive updates based on correct
binding count for consecutive bindings and by performing updates
on a per-binding basis, making sure to roll update over to the
correct next binding.
Also update the error message, related negative test, and database
file.
diff --git a/layers/descriptor_sets.cpp b/layers/descriptor_sets.cpp
index cf2229a..142f1ef 100644
--- a/layers/descriptor_sets.cpp
+++ b/layers/descriptor_sets.cpp
@@ -18,6 +18,9 @@
* Author: Tobin Ehlis <tobine@google.com>
*/
+// Allow use of STL min and max functions in Windows
+#define NOMINMAX
+
#include "descriptor_sets.h"
#include "vk_enum_string_helper.h"
#include "vk_safe_struct.h"
@@ -74,6 +77,18 @@
return skip;
}
+// Return the number of descriptors for the given binding and all successive bindings
+uint32_t cvdescriptorset::DescriptorSetLayout::GetConsecutiveDescriptorCountFromBinding(uint32_t binding) const {
+ // If binding is invalid we'll return 0
+ uint32_t binding_count = 0;
+ auto bi_itr = binding_to_index_map_.find(binding);
+ while (bi_itr != binding_to_index_map_.end()) {
+ binding_count += bindings_[bi_itr->second].descriptorCount;
+ bi_itr++;
+ }
+ return binding_count;
+}
+
// put all bindings into the given set
void cvdescriptorset::DescriptorSetLayout::FillBindingSet(std::unordered_set<uint32_t> *binding_set) const {
for (auto binding_index_pair : binding_to_index_map_)
@@ -546,10 +561,21 @@
}
// Perform write update in given update struct
void cvdescriptorset::DescriptorSet::PerformWriteUpdate(const VkWriteDescriptorSet *update) {
- auto start_idx = p_layout_->GetGlobalStartIndexFromBinding(update->dstBinding) + update->dstArrayElement;
- // perform update
- for (uint32_t di = 0; di < update->descriptorCount; ++di) {
- descriptors_[start_idx + di]->WriteUpdate(update, di);
+ // Perform update on a per-binding basis as consecutive updates roll over to next binding
+ auto descriptors_remaining = update->descriptorCount;
+ auto binding_being_updated = update->dstBinding;
+ auto offset = update->dstArrayElement;
+ while (descriptors_remaining) {
+ uint32_t update_count = std::min(descriptors_remaining, GetDescriptorCountFromBinding(binding_being_updated));
+ auto global_idx = p_layout_->GetGlobalStartIndexFromBinding(binding_being_updated) + offset;
+ // Loop over the updates for a single binding at a time
+ for (uint32_t di = 0; di < update_count; ++di) {
+ descriptors_[global_idx + di]->WriteUpdate(update, di);
+ }
+ // Roll over to next binding in case of consecutive update
+ descriptors_remaining -= update_count;
+ offset = 0;
+ binding_being_updated++;
}
if (update->descriptorCount)
some_update_ = true;
@@ -1169,14 +1195,15 @@
*error_msg = error_str.str();
return false;
}
- if ((start_idx + update->descriptorCount) > p_layout_->GetTotalDescriptorCount()) {
+ if (update->descriptorCount >
+ (p_layout_->GetConsecutiveDescriptorCountFromBinding(update->dstBinding) - update->dstArrayElement)) {
*error_code = VALIDATION_ERROR_00938;
std::stringstream error_str;
error_str << "Attempting write update to descriptor set " << set_ << " binding #" << update->dstBinding << " with "
- << p_layout_->GetTotalDescriptorCount() << " total descriptors but update of " << update->descriptorCount
- << " descriptors starting at binding offset of " << p_layout_->GetGlobalStartIndexFromBinding(update->dstBinding)
- << " combined with update array element offset of " << update->dstArrayElement
- << " oversteps the size of this descriptor set";
+ << p_layout_->GetConsecutiveDescriptorCountFromBinding(update->dstBinding)
+ << " descriptors in that binding and all successive bindings of the set, but update of "
+ << update->descriptorCount << " descriptors combined with update array element offset of "
+ << update->dstArrayElement << " oversteps the available number of consecutive descriptors";
*error_msg = error_str.str();
return false;
}
diff --git a/layers/descriptor_sets.h b/layers/descriptor_sets.h
index 2fdb1c4..b455b35 100644
--- a/layers/descriptor_sets.h
+++ b/layers/descriptor_sets.h
@@ -96,6 +96,8 @@
VkDescriptorSetLayout GetDescriptorSetLayout() const { return layout_; };
uint32_t GetTotalDescriptorCount() const { return descriptor_count_; };
uint32_t GetDynamicDescriptorCount() const { return dynamic_descriptor_count_; };
+ // For a given binding, return the number of descriptors in that binding and all successive bindings
+ uint32_t GetConsecutiveDescriptorCountFromBinding(uint32_t) const;
uint32_t GetBindingCount() const { return binding_count_; };
// Fill passed-in set with bindings
void FillBindingSet(std::unordered_set<uint32_t> *) const;
@@ -138,7 +140,7 @@
private:
VkDescriptorSetLayout layout_;
- std::unordered_map<uint32_t, uint32_t> binding_to_index_map_;
+ std::map<uint32_t, uint32_t> binding_to_index_map_;
std::unordered_map<uint32_t, uint32_t> binding_to_global_start_index_map_;
std::unordered_map<uint32_t, uint32_t> binding_to_global_end_index_map_;
// For a given binding map to associated index in the dynamic offset array
diff --git a/layers/vk_validation_error_database.txt b/layers/vk_validation_error_database.txt
index 9ce55fd..c91ad0a 100644
--- a/layers/vk_validation_error_database.txt
+++ b/layers/vk_validation_error_database.txt
@@ -927,7 +927,7 @@
VALIDATION_ERROR_00935~^~U~^~Unknown~^~vkUpdateDescriptorSets~^~For more information refer to Vulkan Spec Section '13.2.4. Descriptor Set Updates' which states 'If descriptorCopyCount is not 0, pDescriptorCopies must be a pointer to an array of descriptorCopyCount valid VkCopyDescriptorSet structures' (https://www.khronos.org/registry/vulkan/specs/1.0-extensions/xhtml/vkspec.html#vkUpdateDescriptorSets)~^~
VALIDATION_ERROR_00936~^~Y~^~InvalidDSUpdateIndex~^~vkUpdateDescriptorSets~^~For more information refer to Vulkan Spec Section '13.2.4. Descriptor Set Updates' which states 'dstBinding must be less than or equal to the maximum value of binding of all VkDescriptorSetLayoutBinding structures specified when dstSets descriptor set layout was created' (https://www.khronos.org/registry/vulkan/specs/1.0-extensions/xhtml/vkspec.html#descriptorsets-updates-consecutive)~^~
VALIDATION_ERROR_00937~^~Y~^~Unknown~^~vkUpdateDescriptorSets~^~For more information refer to Vulkan Spec Section '13.2.4. Descriptor Set Updates' which states 'descriptorType must match the type of dstBinding within dstSet' (https://www.khronos.org/registry/vulkan/specs/1.0-extensions/xhtml/vkspec.html#descriptorsets-updates-consecutive)~^~
-VALIDATION_ERROR_00938~^~Y~^~WriteDescriptorSetIntegrityCheck~^~vkUpdateDescriptorSets~^~For more information refer to Vulkan Spec Section '13.2.4. Descriptor Set Updates' which states 'The sum of dstArrayElement and descriptorCount must be less than or equal to the number of array elements in the descriptor set binding specified by dstBinding, and all applicable consecutive bindings, as described by consecutive binding updates' (https://www.khronos.org/registry/vulkan/specs/1.0-extensions/xhtml/vkspec.html#descriptorsets-updates-consecutive)~^~
+VALIDATION_ERROR_00938~^~Y~^~WriteDescriptorSetIntegrityCheck,DSUpdateOutOfBounds~^~vkUpdateDescriptorSets~^~For more information refer to Vulkan Spec Section '13.2.4. Descriptor Set Updates' which states 'The sum of dstArrayElement and descriptorCount must be less than or equal to the number of array elements in the descriptor set binding specified by dstBinding, and all applicable consecutive bindings, as described by consecutive binding updates' (https://www.khronos.org/registry/vulkan/specs/1.0-extensions/xhtml/vkspec.html#descriptorsets-updates-consecutive)~^~
VALIDATION_ERROR_00939~^~U~^~Unknown~^~vkUpdateDescriptorSets~^~For more information refer to Vulkan Spec Section '13.2.4. Descriptor Set Updates' which states 'If descriptorType is VK_DESCRIPTOR_TYPE_SAMPLER, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE, VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, or VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT, pImageInfo must be a pointer to an array of descriptorCount valid VkDescriptorImageInfo structures' (https://www.khronos.org/registry/vulkan/specs/1.0-extensions/xhtml/vkspec.html#descriptorsets-updates-consecutive)~^~
VALIDATION_ERROR_00940~^~Y~^~InvalidBufferViewObject~^~vkUpdateDescriptorSets~^~For more information refer to Vulkan Spec Section '13.2.4. Descriptor Set Updates' which states 'If descriptorType is VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER or VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER, pTexelBufferView must be a pointer to an array of descriptorCount valid VkBufferView handles' (https://www.khronos.org/registry/vulkan/specs/1.0-extensions/xhtml/vkspec.html#descriptorsets-updates-consecutive)~^~
VALIDATION_ERROR_00941~^~U~^~Unknown~^~vkUpdateDescriptorSets~^~For more information refer to Vulkan Spec Section '13.2.4. Descriptor Set Updates' which states 'If descriptorType is VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC, or VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC, pBufferInfo must be a pointer to an array of descriptorCount valid VkDescriptorBufferInfo structures' (https://www.khronos.org/registry/vulkan/specs/1.0-extensions/xhtml/vkspec.html#descriptorsets-updates-consecutive)~^~