layers:Fix descriptor dynamic offset handling

Fixes #1162

The dynamic offsets passed in at descriptor bind time are ordered based
on the binding order and any consecutive array indicies within those
bindings. When we validate the dynamic offsets at draw time, we were
using the entire dynamic offset array, but only the active bindings.

If we had an inactive dynamic binding before the end of the bindings
we would use the wrong offsets.

This change fixes the issue by creating a mapping between bindings and
dynamic offset array indicies at descriptor layout creation time. At
draw time the mapping is then used to lookup the correct dynamic offset
array index for a given binding.

diff --git a/layers/descriptor_sets.cpp b/layers/descriptor_sets.cpp
index 6c8be9c..d231543 100644 (file)
--- a/layers/descriptor_sets.cpp
+++ b/layers/descriptor_sets.cpp
@@ -28,6 +28,9 @@ cvdescriptorset::DescriptorSetLayout::DescriptorSetLayout(const VkDescriptorSetL
                                                           const VkDescriptorSetLayout layout)
     : layout_(layout), binding_count_(p_create_info->bindingCount), descriptor_count_(0), dynamic_descriptor_count_(0) {
     uint32_t global_index = 0;
+    // Dyn array indicies are ordered by binding # and array index of any array within the binding
+    //  so we store up bindings w/ count in ordered map in order to create dyn array mappings below
+    std::map<uint32_t, uint32_t> binding_to_dyn_count;
     for (uint32_t i = 0; i < binding_count_; ++i) {
         descriptor_count_ += p_create_info->pBindings[i].descriptorCount;
         binding_to_index_map_[p_create_info->pBindings[i].binding] = i;
@@ -44,9 +47,16 @@ cvdescriptorset::DescriptorSetLayout::DescriptorSetLayout(const VkDescriptorSetL
         }
         if (p_create_info->pBindings[i].descriptorType == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC ||
             p_create_info->pBindings[i].descriptorType == VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC) {
+            binding_to_dyn_count[p_create_info->pBindings[i].binding] = p_create_info->pBindings[i].descriptorCount;
             dynamic_descriptor_count_ += p_create_info->pBindings[i].descriptorCount;
         }
     }
+    // Now create dyn offset array mapping for any dynamic descriptors
+    uint32_t dyn_array_idx = 0;
+    for (const auto &bc_pair : binding_to_dyn_count) {
+        binding_to_dynamic_array_idx_map_[bc_pair.first] = dyn_array_idx;
+        dyn_array_idx += bc_pair.second;
+    }
 }
 
 // Validate descriptor set layout create info
@@ -361,7 +371,6 @@ bool cvdescriptorset::DescriptorSet::IsCompatible(const DescriptorSetLayout *lay
 // Return true if state is acceptable, or false and write an error message into error string
 bool cvdescriptorset::DescriptorSet::ValidateDrawState(const std::map<uint32_t, descriptor_req> &bindings,
                                                        const std::vector<uint32_t> &dynamic_offsets, std::string *error) const {
-    auto dyn_offset_index = 0;
     for (auto binding_pair : bindings) {
         auto binding = binding_pair.first;
         if (!p_layout_->HasBinding(binding)) {
@@ -376,7 +385,8 @@ bool cvdescriptorset::DescriptorSet::ValidateDrawState(const std::map<uint32_t,
             // Nothing to do for strictly immutable sampler
         } else {
             auto end_idx = p_layout_->GetGlobalEndIndexFromBinding(binding);
-            for (uint32_t i = start_idx; i <= end_idx; ++i) {
+            auto array_idx = 0; // Track array idx if we're dealing with array descriptors
+            for (uint32_t i = start_idx; i <= end_idx; ++i, ++array_idx) {
                 if (!descriptors_[i]->updated) {
                     std::stringstream error_str;
                     error_str << "Descriptor in binding #" << binding << " at global descriptor index " << i
@@ -411,7 +421,7 @@ bool cvdescriptorset::DescriptorSet::ValidateDrawState(const std::map<uint32_t,
                             auto buffer_size = buffer_node->createInfo.size;
                             auto range = static_cast<BufferDescriptor *>(descriptors_[i].get())->GetRange();
                             auto desc_offset = static_cast<BufferDescriptor *>(descriptors_[i].get())->GetOffset();
-                            auto dyn_offset = dynamic_offsets[dyn_offset_index++];
+                            auto dyn_offset = dynamic_offsets[GetDynamicOffsetIndexFromBinding(binding) + array_idx];
                             if (VK_WHOLE_SIZE == range) {
                                 if ((dyn_offset + desc_offset) > buffer_size) {
                                     std::stringstream error_str;

diff --git a/layers/descriptor_sets.h b/layers/descriptor_sets.h
index 2cf45e7..b9103ec 100644 (file)
--- a/layers/descriptor_sets.h
+++ b/layers/descriptor_sets.h
@@ -119,6 +119,15 @@ class DescriptorSetLayout {
     VkShaderStageFlags GetStageFlagsFromBinding(const uint32_t) const;
     VkSampler const *GetImmutableSamplerPtrFromBinding(const uint32_t) const;
     VkSampler const *GetImmutableSamplerPtrFromIndex(const uint32_t) const;
+    // For a given binding and array index, return the corresponding index into the dynamic offset array
+    int32_t GetDynamicOffsetIndexFromBinding(uint32_t binding) const {
+        auto dyn_off = binding_to_dynamic_array_idx_map_.find(binding);
+        if (dyn_off == binding_to_dynamic_array_idx_map_.end()) {
+            assert(0); // Requesting dyn offset for invalid binding/array idx pair
+            return -1;
+        }
+        return dyn_off->second;
+    }
     // For a particular binding, get the global index
     //  These calls should be guarded by a call to "HasBinding(binding)" to verify that the given binding exists
     uint32_t GetGlobalStartIndexFromBinding(const uint32_t) const;
@@ -132,6 +141,8 @@ class DescriptorSetLayout {
     std::unordered_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
+    std::unordered_map<uint32_t, uint32_t> binding_to_dynamic_array_idx_map_;
     // VkDescriptorSetLayoutCreateFlags flags_;
     uint32_t binding_count_; // # of bindings in this layout
     std::vector<safe_VkDescriptorSetLayoutBinding> bindings_;
@@ -319,6 +330,10 @@ class DescriptorSet : public BASE_NODE {
     uint32_t GetDescriptorCountFromBinding(const uint32_t binding) const {
         return p_layout_ ? p_layout_->GetDescriptorCountFromBinding(binding) : 0;
     };
+    // Return index into dynamic offset array for given binding
+    int32_t GetDynamicOffsetIndexFromBinding(uint32_t binding) const {
+        return p_layout_ ? p_layout_->GetDynamicOffsetIndexFromBinding(binding) : -1;
+    }
     // Return true if given binding is present in this set
     bool HasBinding(const uint32_t binding) const { return p_layout_->HasBinding(binding); };
     // Is this set compatible with the given layout?