From 3e8e513a777f4b74d4fc60770529eeb3c0714c89 Mon Sep 17 00:00:00 2001 From: Tobin Ehlis Date: Thu, 17 Nov 2016 10:50:52 -0700 Subject: [PATCH] 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. --- layers/descriptor_sets.cpp | 16 +++++++++++++--- layers/descriptor_sets.h | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/layers/descriptor_sets.cpp b/layers/descriptor_sets.cpp index 6c8be9c..d231543 100644 --- 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 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 &bindings, const std::vector &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::mapGetGlobalEndIndexFromBinding(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::mapcreateInfo.size; auto range = static_cast(descriptors_[i].get())->GetRange(); auto desc_offset = static_cast(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 --- 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 binding_to_index_map_; std::unordered_map binding_to_global_start_index_map_; std::unordered_map binding_to_global_end_index_map_; + // For a given binding map to associated index in the dynamic offset array + std::unordered_map binding_to_dynamic_array_idx_map_; // VkDescriptorSetLayoutCreateFlags flags_; uint32_t binding_count_; // # of bindings in this layout std::vector 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? -- 2.7.4