From: Tobin Ehlis Date: Wed, 18 May 2016 19:43:26 +0000 (-0600) Subject: layers: Refactor DescriptorSet update interface X-Git-Tag: upstream/1.1.92~3040^2~177 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=8ac68a5c55f9662b74db3826f97aa2f28d21503f;p=platform%2Fupstream%2FVulkan-Tools.git layers: Refactor DescriptorSet update interface Mainly refactor and moving code in order to provide an interface to DescriptorSet class that matches top-level vkUpdateDescriptorSets() function. Split the validation of an update as a separate task from performing the update. This allows validation prior to calling down the chain and then only update the state if validation is clean. Hoisted all of the update validation into the DescriptorSet class which prevents having to copy all of the maps into the individual Descriptor classes. This simplifies both their creation and updating their contents. Updated the top-level core_validation UpdateDescriptorSets() code to match Vulkan Validation Layer Authoring Guidelines. As this is an initial POC for the architecture, I kept the Pre* & Post* functions in the core_validation.cpp file, but they should eventually be spun out. --- diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index 6e05d48..cd9388a 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -3407,65 +3407,6 @@ static bool validateIdleDescriptorSet(const layer_data *my_data, VkDescriptorSet } return skip_call; } -// 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 skip_call = false; - // Validate Write updates - uint32_t i = 0; - for (i = 0; i < descriptorWriteCount; i++) { - 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(dest_set), __LINE__, DRAWSTATE_DOUBLE_DESTROY, "DS", - "Cannot call vkUpdateDescriptorSets() on descriptor set 0x%" PRIxLEAST64 " that has not been allocated.", - reinterpret_cast(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(dest_set), __LINE__, DRAWSTATE_INVALID_UPDATE_INDEX, "DS", - "vkUpdateDescriptorsSets() failed write update for Descriptor Set 0x%" PRIx64 " with error: %s", - reinterpret_cast(dest_set), error_str.c_str()); - } - } - } - // Now validate copy updates - for (i = 0; i < descriptorCopyCount; ++i) { - 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(src_set), __LINE__, DRAWSTATE_DOUBLE_DESTROY, "DS", - "Cannot call vkUpdateDescriptorSets() to copy from descriptor set 0x%" PRIxLEAST64 " that has not been allocated.", - reinterpret_cast(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(dst_set), __LINE__, DRAWSTATE_DOUBLE_DESTROY, "DS", - "Cannot call vkUpdateDescriptorSets() to copy to descriptor set 0x%" PRIxLEAST64 " that has not been allocated.", - reinterpret_cast(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(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(src_set), reinterpret_cast(dst_set), error_str.c_str()); - } - } - } - return skip_call; -} - // Verify that given pool has descriptors that are being requested for allocation. // NOTE : Calls to this function should be wrapped in mutex static bool validate_descriptor_availability_in_pool(layer_data *dev_data, DESCRIPTOR_POOL_NODE *pPoolNode, uint32_t count, @@ -6070,18 +6011,47 @@ FreeDescriptorSets(VkDevice device, VkDescriptorPool descriptorPool, uint32_t co // TODO : Any other clean-up or book-keeping to do here? return result; } +// TODO : This is a Proof-of-concept for core validation architecture +// Really we'll want to break out these functions to separate files but +// keeping it all together here to prove out design +// PreCallValidate* handles validating all of the state prior to calling down chain to UpdateDescriptorSets() +static bool PreCallValidateUpdateDescriptorSets(layer_data *dev_data, uint32_t descriptorWriteCount, + const VkWriteDescriptorSet *pDescriptorWrites, uint32_t descriptorCopyCount, + const VkCopyDescriptorSet *pDescriptorCopies) { + // First thing to do is perform map look-ups. + // NOTE : UpdateDescriptorSets is somewhat unique in that it's operating on a number of DescriptorSets + // so we can't just do a single map look-up up-front, but do them individually in functions below + + // Now make call(s) that validate state, but don't perform state updates in this function + // Note, here DescriptorSets is unique in that we don't yet have an instance. Using a helper function in the + // namespace which will parse params and make calls into specific class instances + return cvdescriptorset::ValidateUpdateDescriptorSets(dev_data->report_data, dev_data->setMap, descriptorWriteCount, + pDescriptorWrites, descriptorCopyCount, pDescriptorCopies); +} +// PostCallRecord* handles recording state updates following call down chain to UpdateDescriptorSets() +static void PostCallRecordUpdateDescriptorSets(layer_data *dev_data, uint32_t descriptorWriteCount, + const VkWriteDescriptorSet *pDescriptorWrites, uint32_t descriptorCopyCount, + const VkCopyDescriptorSet *pDescriptorCopies) { + cvdescriptorset::PerformUpdateDescriptorSets(dev_data->setMap, descriptorWriteCount, pDescriptorWrites, descriptorCopyCount, + pDescriptorCopies); +} VKAPI_ATTR void VKAPI_CALL UpdateDescriptorSets(VkDevice device, uint32_t descriptorWriteCount, const VkWriteDescriptorSet *pDescriptorWrites, uint32_t descriptorCopyCount, const VkCopyDescriptorSet *pDescriptorCopies) { - // dsUpdate will return true only if a bailout error occurs, so we want to call down tree when update returns false + // Only map look-up at top level is for device-level layer_data layer_data *dev_data = get_my_data_ptr(get_dispatch_key(device), layer_data_map); std::unique_lock lock(global_lock); - bool rtn = dsUpdate(dev_data, device, descriptorWriteCount, pDescriptorWrites, descriptorCopyCount, pDescriptorCopies); + bool skip_call = PreCallValidateUpdateDescriptorSets(dev_data, descriptorWriteCount, pDescriptorWrites, descriptorCopyCount, + pDescriptorCopies); lock.unlock(); - if (!rtn) { + if (!skip_call) { dev_data->device_dispatch_table->UpdateDescriptorSets(device, descriptorWriteCount, pDescriptorWrites, descriptorCopyCount, pDescriptorCopies); + lock.lock(); + // Since UpdateDescriptorSets() is void, nothing to check prior to updating state + PostCallRecordUpdateDescriptorSets(dev_data, descriptorWriteCount, pDescriptorWrites, descriptorCopyCount, + pDescriptorCopies); } } diff --git a/layers/descriptor_sets.cpp b/layers/descriptor_sets.cpp index 1d9eed6..2955a6a 100644 --- a/layers/descriptor_sets.cpp +++ b/layers/descriptor_sets.cpp @@ -280,9 +280,9 @@ cvdescriptorset::DescriptorSet::DescriptorSet(const VkDescriptorSet set, const D auto immut_sampler = p_layout_->GetImmutableSamplerPtrFromIndex(i); for (uint32_t di = 0; di < p_layout_->GetDescriptorCountFromIndex(i); ++di) { if (immut_sampler) - descriptors_.emplace_back(std::unique_ptr(new SamplerDescriptor(immut_sampler + di, sampler_map_))); + descriptors_.emplace_back(std::unique_ptr(new SamplerDescriptor(immut_sampler + di))); else - descriptors_.emplace_back(std::unique_ptr(new SamplerDescriptor(sampler_map_))); + descriptors_.emplace_back(std::unique_ptr(new SamplerDescriptor())); } break; } @@ -290,11 +290,9 @@ cvdescriptorset::DescriptorSet::DescriptorSet(const VkDescriptorSet set, const D auto immut = p_layout_->GetImmutableSamplerPtrFromIndex(i); for (uint32_t di = 0; di < p_layout_->GetDescriptorCountFromIndex(i); ++di) { if (immut) - descriptors_.emplace_back(std::unique_ptr(new ImageSamplerDescriptor( - immut + di, image_view_map_, image_map_, image_to_swapchain_map_, swapchain_map_, sampler_map_))); + descriptors_.emplace_back(std::unique_ptr(new ImageSamplerDescriptor(immut + di))); else - descriptors_.emplace_back(std::unique_ptr(new ImageSamplerDescriptor( - image_view_map_, image_map_, image_to_swapchain_map_, swapchain_map_, sampler_map_))); + descriptors_.emplace_back(std::unique_ptr(new ImageSamplerDescriptor())); } break; } @@ -303,20 +301,19 @@ cvdescriptorset::DescriptorSet::DescriptorSet(const VkDescriptorSet set, const D case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT: case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: for (uint32_t di = 0; di < p_layout_->GetDescriptorCountFromIndex(i); ++di) - descriptors_.emplace_back(std::unique_ptr( - new ImageDescriptor(type, image_view_map_, image_map_, image_to_swapchain_map_, swapchain_map_))); + descriptors_.emplace_back(std::unique_ptr(new ImageDescriptor(type))); break; case VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER: case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER: for (uint32_t di = 0; di < p_layout_->GetDescriptorCountFromIndex(i); ++di) - descriptors_.emplace_back(std::unique_ptr(new TexelDescriptor(type, buffer_view_map_))); + descriptors_.emplace_back(std::unique_ptr(new TexelDescriptor(type))); break; case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER: case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC: case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER: case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC: for (uint32_t di = 0; di < p_layout_->GetDescriptorCountFromIndex(i); ++di) - descriptors_.emplace_back(std::unique_ptr(new BufferDescriptor(type, buffer_map_))); + descriptors_.emplace_back(std::unique_ptr(new BufferDescriptor(type))); break; default: assert(0); // Bad descriptor type specified @@ -468,74 +465,22 @@ void cvdescriptorset::DescriptorSet::InvalidateBoundCmdBuffers() { } } // Perform write update in given update struct -// If an error occurs, return false and fill in details in error_msg string -bool cvdescriptorset::DescriptorSet::WriteUpdate(debug_report_data *report_data, const VkWriteDescriptorSet *update, - std::string *error_msg) { +void cvdescriptorset::DescriptorSet::PerformWriteUpdate(const VkWriteDescriptorSet *update) { 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; - error_str << "DescriptorSet " << set_ << " does not have binding " << update->dstBinding << "."; - *error_msg = error_str.str(); - return false; - } else { - // We know that binding is valid, verify update and do update on each descriptor - auto start_idx = p_layout_->GetGlobalStartIndexFromBinding(update->dstBinding) + update->dstArrayElement; - auto type = p_layout_->GetTypeFromBinding(update->dstBinding); - if (type != update->descriptorType) { - std::stringstream error_str; - error_str << "Attempting write update to descriptor set " << set_ << " binding #" << update->dstBinding << " with type " - << string_VkDescriptorType(type) << " but update type is " << string_VkDescriptorType(update->descriptorType); - *error_msg = error_str.str(); - return false; - } - if ((start_idx + update->descriptorCount) > p_layout_->GetTotalDescriptorCount()) { - 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."; - *error_msg = error_str.str(); - return false; - } - // Verify consecutive bindings match (if needed) - if (!p_layout_->VerifyUpdateConsistency(update->dstBinding, update->dstArrayElement, update->descriptorCount, - "write update to", set_, error_msg)) - return false; - // Update is within bounds and consistent so perform update - for (uint32_t di = 0; di < update->descriptorCount; ++di) { - // TODO : Can we break this into a set-level "Validate" followed by Descriptor updating itself - // if the validate passes? That saves us all the map ptrs in each descriptor instance - if (!descriptors_[start_idx + di]->WriteUpdate(update, di, error_msg)) { - std::stringstream error_str; - error_str << "Write update to descriptor at global index " << start_idx + di << " within set " << set_ - << " binding #" << update->dstBinding << " failed with error message: " << error_msg->c_str(); - *error_msg = error_str.str(); - return false; - } - ++num_updates; - } + 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); + ++num_updates; } if (num_updates != 0) { some_update_ = true; } InvalidateBoundCmdBuffers(); - 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; +// Validate Copy update +bool cvdescriptorset::DescriptorSet::ValidateCopyUpdate(const debug_report_data *report_data, const VkCopyDescriptorSet *update, + const DescriptorSet *src_set, std::string *error) { // Verify idle ds if (in_use.load()) { std::stringstream error_str; @@ -598,28 +543,34 @@ bool cvdescriptorset::DescriptorSet::CopyUpdate(debug_report_data *report_data, set_, error))) { return false; } + // Update parameters all look good so verify update contents + if (!VerifyCopyUpdateContents(update, src_set, src_start_idx, error)) + return false; + + // All checks passed so update is good + return true; +} +// Perform Copy update +void cvdescriptorset::DescriptorSet::PerformCopyUpdate(const VkCopyDescriptorSet *update, const DescriptorSet *src_set) { + auto num_updates = 0; + auto src_start_idx = src_set->GetGlobalStartIndexFromBinding(update->srcBinding) + update->srcArrayElement; + auto dst_start_idx = p_layout_->GetGlobalStartIndexFromBinding(update->dstBinding) + update->dstArrayElement; // Update parameters all look good so perform update for (uint32_t di = 0; di < update->descriptorCount; ++di) { - if (!descriptors_[dst_start_idx]->CopyUpdate(src_set->descriptors_[src_start_idx].get(), error)) - return false; + descriptors_[dst_start_idx + di]->CopyUpdate(src_set->descriptors_[src_start_idx + di].get()); ++num_updates; } if (num_updates != 0) { some_update_ = true; } InvalidateBoundCmdBuffers(); - return true; } -cvdescriptorset::SamplerDescriptor::SamplerDescriptor( - const std::unordered_map> *sampler_map) - : sampler_(VK_NULL_HANDLE), immutable_(false), sampler_map_(sampler_map) { +cvdescriptorset::SamplerDescriptor::SamplerDescriptor() : sampler_(VK_NULL_HANDLE), immutable_(false) { updated = false; descriptor_class = PlainSampler; }; -cvdescriptorset::SamplerDescriptor::SamplerDescriptor( - const VkSampler *immut, const std::unordered_map> *sampler_map) - : sampler_(VK_NULL_HANDLE), immutable_(false), sampler_map_(sampler_map) { +cvdescriptorset::SamplerDescriptor::SamplerDescriptor(const VkSampler *immut) : sampler_(VK_NULL_HANDLE), immutable_(false) { updated = false; descriptor_class = PlainSampler; if (immut) { @@ -726,75 +677,26 @@ bool cvdescriptorset::ValidateImageUpdate(const VkImageView image_view, const Vk } return true; } -bool cvdescriptorset::SamplerDescriptor::WriteUpdate(const VkWriteDescriptorSet *update, const uint32_t index, std::string *error) { - if (!immutable_) { - if (!ValidateSampler(update->pImageInfo[index].sampler, sampler_map_)) { - std::stringstream error_str; - error_str << "Attempted write update to sampler descriptor with invalid sampler: " << update->pImageInfo[index].sampler - << "."; - *error = error_str.str(); - return false; - } - sampler_ = update->pImageInfo[index].sampler; - } else { - if (!ValidateSampler(sampler_, sampler_map_)) { - std::stringstream error_str; - error_str << "Attempted write update to immutable sampler descriptor which has invalid immutable sampler: " << sampler_ - << "."; - *error = error_str.str(); - return false; - } - // TODO : Do we want a way to warn here in case of updating immutable sampler (which can't be updated)? - } +void cvdescriptorset::SamplerDescriptor::WriteUpdate(const VkWriteDescriptorSet *update, const uint32_t index) { + sampler_ = update->pImageInfo[index].sampler; updated = true; - return true; } -bool cvdescriptorset::SamplerDescriptor::CopyUpdate(const Descriptor *src, std::string *error) { +void cvdescriptorset::SamplerDescriptor::CopyUpdate(const Descriptor *src) { if (!immutable_) { auto update_sampler = static_cast(src)->sampler_; - if (!ValidateSampler(update_sampler, sampler_map_)) { - std::stringstream error_str; - error_str << "Attempted copy update to sampler descriptor with invalid sampler: " << update_sampler << "."; - *error = error_str.str(); - return false; - } sampler_ = update_sampler; - } else { - if (!ValidateSampler(sampler_, sampler_map_)) { - std::stringstream error_str; - error_str << "Attempted copy update to immutable sampler descriptor which has invalid immutable sampler: " << sampler_ - << "."; - *error = error_str.str(); - return false; - } - // TODO : Do we want a way to warn here in case of updating immutable sampler (which can't be updated)? } updated = true; - return true; } -cvdescriptorset::ImageSamplerDescriptor::ImageSamplerDescriptor( - const std::unordered_map *image_view_map, - const std::unordered_map *image_map, - const std::unordered_map *image_to_swapchain_map, - const std::unordered_map *swapchain_map, - const std::unordered_map> *sampler_map) - : sampler_(VK_NULL_HANDLE), immutable_(false), image_view_(VK_NULL_HANDLE), image_layout_(VK_IMAGE_LAYOUT_UNDEFINED), - 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) { +cvdescriptorset::ImageSamplerDescriptor::ImageSamplerDescriptor() + : sampler_(VK_NULL_HANDLE), immutable_(false), image_view_(VK_NULL_HANDLE), image_layout_(VK_IMAGE_LAYOUT_UNDEFINED) { updated = false; descriptor_class = ImageSampler; } -cvdescriptorset::ImageSamplerDescriptor::ImageSamplerDescriptor( - const VkSampler *immut, const std::unordered_map *image_view_map, - const std::unordered_map *image_map, - const std::unordered_map *image_to_swapchain_map, - const std::unordered_map *swapchain_map, - const std::unordered_map> *sampler_map) - : sampler_(VK_NULL_HANDLE), immutable_(true), image_view_(VK_NULL_HANDLE), image_layout_(VK_IMAGE_LAYOUT_UNDEFINED), - 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) { +cvdescriptorset::ImageSamplerDescriptor::ImageSamplerDescriptor(const VkSampler *immut) + : sampler_(VK_NULL_HANDLE), immutable_(true), image_view_(VK_NULL_HANDLE), image_layout_(VK_IMAGE_LAYOUT_UNDEFINED) { updated = false; descriptor_class = ImageSampler; if (immut) { @@ -803,134 +705,51 @@ cvdescriptorset::ImageSamplerDescriptor::ImageSamplerDescriptor( updated = true; } } -bool cvdescriptorset::ImageSamplerDescriptor::WriteUpdate(const VkWriteDescriptorSet *update, const uint32_t index, - std::string *error) { - // First check sampler - if (!immutable_) { - if (!ValidateSampler(update->pImageInfo[index].sampler, sampler_map_)) { - std::stringstream error_str; - error_str << "Attempted write update to combined image sampler descriptor with invalid sampler: " - << update->pImageInfo[index].sampler << "."; - *error = error_str.str(); - return false; - } - sampler_ = update->pImageInfo[index].sampler; - } else { - if (!ValidateSampler(sampler_, sampler_map_)) { - std::stringstream error_str; - error_str << "Attempted write update to combined image sampler descriptor with immutable sampler which has invalid " - "immutable sampler: " - << sampler_ << "."; - *error = error_str.str(); - return false; - } - // TODO : Do we want a way to warn here in case of updating immutable sampler (which can't be updated)? - } - // Now validate images - auto image_view = update->pImageInfo[index].imageView; - auto image_layout = update->pImageInfo[index].imageLayout; - if (!ValidateImageUpdate(image_view, image_layout, image_view_map_, image_map_, image_to_swapchain_map_, swapchain_map_, - error)) { - std::stringstream error_str; - error_str << "Attempted write update to combined image sampler descriptor failed due to: " << error->c_str(); - *error = error_str.str(); - return false; - } +void cvdescriptorset::ImageSamplerDescriptor::WriteUpdate(const VkWriteDescriptorSet *update, const uint32_t index) { updated = true; - image_view_ = image_view; - image_layout_ = image_layout; - return true; + auto image_info = update->pImageInfo[index]; + sampler_ = image_info.sampler; + image_view_ = image_info.imageView; + image_layout_ = image_info.imageLayout; } -bool cvdescriptorset::ImageSamplerDescriptor::CopyUpdate(const Descriptor *src, std::string *error) { +void cvdescriptorset::ImageSamplerDescriptor::CopyUpdate(const Descriptor *src) { if (!immutable_) { auto update_sampler = static_cast(src)->sampler_; - if (!ValidateSampler(update_sampler, sampler_map_)) { - std::stringstream error_str; - error_str << "Attempted copy update to combined image sampler descriptor with invalid sampler: " << update_sampler - << "."; - *error = error_str.str(); - return false; - } sampler_ = update_sampler; - } else { - if (!ValidateSampler(sampler_, sampler_map_)) { - std::stringstream error_str; - error_str << "Attempted copy update to combined image sampler descriptor with immutable sampler that has invalid " - "immutable sampler: " - << sampler_ << "."; - *error = error_str.str(); - return false; - } - // TODO : Do we want a way to warn here in case of updating immutable sampler (which can't be updated)? } - // Now validate images auto image_view = static_cast(src)->image_view_; auto image_layout = static_cast(src)->image_layout_; - if (!ValidateImageUpdate(image_view, image_layout, image_view_map_, image_map_, image_to_swapchain_map_, swapchain_map_, - error)) { - std::stringstream error_str; - error_str << "Attempted copy update to combined image sampler descriptor failed due to: " << error->c_str(); - *error = error_str.str(); - return false; - } updated = true; image_view_ = image_view; image_layout_ = image_layout; - return true; } -cvdescriptorset::ImageDescriptor::ImageDescriptor(const VkDescriptorType type, - const std::unordered_map *image_view_map, - const std::unordered_map *image_map, - const std::unordered_map *image_to_swapchain_map, - const std::unordered_map *swapchain_map) - : storage_(false), image_view_(VK_NULL_HANDLE), image_layout_(VK_IMAGE_LAYOUT_UNDEFINED), image_view_map_(image_view_map), - image_map_(image_map), image_to_swapchain_map_(image_to_swapchain_map), swapchain_map_(swapchain_map) { +cvdescriptorset::ImageDescriptor::ImageDescriptor(const VkDescriptorType type) + : storage_(false), image_view_(VK_NULL_HANDLE), image_layout_(VK_IMAGE_LAYOUT_UNDEFINED) { updated = false; descriptor_class = Image; if (VK_DESCRIPTOR_TYPE_STORAGE_IMAGE == type) storage_ = true; }; -bool cvdescriptorset::ImageDescriptor::WriteUpdate(const VkWriteDescriptorSet *update, const uint32_t index, std::string *error) { - // First validate that the write update is valid - auto image_view = update->pImageInfo[index].imageView; - auto image_layout = update->pImageInfo[index].imageLayout; - if (!ValidateImageUpdate(image_view, image_layout, image_view_map_, image_map_, image_to_swapchain_map_, swapchain_map_, - error)) { - std::stringstream error_str; - error_str << "Attempted write update to image descriptor failed due to: " << error->c_str(); - *error = error_str.str(); - return false; - } - // Update is clean so process it +void cvdescriptorset::ImageDescriptor::WriteUpdate(const VkWriteDescriptorSet *update, const uint32_t index) { updated = true; - image_view_ = image_view; - image_layout_ = image_layout; - return true; + auto image_info = update->pImageInfo[index]; + image_view_ = image_info.imageView; + image_layout_ = image_info.imageLayout; } -bool cvdescriptorset::ImageDescriptor::CopyUpdate(const Descriptor *src, std::string *error) { - // First validate update +void cvdescriptorset::ImageDescriptor::CopyUpdate(const Descriptor *src) { auto image_view = static_cast(src)->image_view_; auto image_layout = static_cast(src)->image_layout_; - if (!ValidateImageUpdate(image_view, image_layout, image_view_map_, image_map_, image_to_swapchain_map_, swapchain_map_, - error)) { - std::stringstream error_str; - error_str << "Attempted copy update to image descriptor failed due to: " << error->c_str(); - *error = error_str.str(); - return false; - } updated = true; image_view_ = image_view; image_layout_ = image_layout; - return true; } -cvdescriptorset::BufferDescriptor::BufferDescriptor(const VkDescriptorType type, - const std::unordered_map *buffer_map) - : storage_(false), dynamic_(false), buffer_(VK_NULL_HANDLE), offset_(0), range_(0), buffer_map_(buffer_map) { +cvdescriptorset::BufferDescriptor::BufferDescriptor(const VkDescriptorType type) + : storage_(false), dynamic_(false), buffer_(VK_NULL_HANDLE), offset_(0), range_(0) { updated = false; descriptor_class = GeneralBuffer; if (VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC == type) { @@ -942,70 +761,361 @@ cvdescriptorset::BufferDescriptor::BufferDescriptor(const VkDescriptorType type, storage_ = true; } } -bool cvdescriptorset::BufferDescriptor::WriteUpdate(const VkWriteDescriptorSet *update, const uint32_t index, std::string *error) { - // First validate bufferinfo - auto buffer = update->pBufferInfo[index].buffer; - if (!buffer_map_->count(buffer)) { - std::stringstream error_str; - error_str << "Attempted write update to buffer descriptor with invalid buffer: " << buffer; - *error = error_str.str(); - return false; - } +void cvdescriptorset::BufferDescriptor::WriteUpdate(const VkWriteDescriptorSet *update, const uint32_t index) { updated = true; - buffer_ = buffer; - offset_ = update->pBufferInfo[index].offset; - range_ = update->pBufferInfo[index].range; - return true; + auto buffer_info = update->pBufferInfo[index]; + buffer_ = buffer_info.buffer; + offset_ = buffer_info.offset; + range_ = buffer_info.range; } -bool cvdescriptorset::BufferDescriptor::CopyUpdate(const Descriptor *src, std::string *error) { - // First validate bufferinfo - auto buffer = static_cast(src)->buffer_; - if (!buffer_map_->count(buffer)) { - std::stringstream error_str; - error_str << "Attempted copy update to buffer descriptor with invalid buffer: " << buffer; - *error = error_str.str(); - return false; - } +void cvdescriptorset::BufferDescriptor::CopyUpdate(const Descriptor *src) { + auto buff_desc = static_cast(src); updated = true; - buffer_ = buffer; - offset_ = static_cast(src)->offset_; - range_ = static_cast(src)->range_; - return true; + buffer_ = buff_desc->buffer_; + offset_ = buff_desc->offset_; + range_ = buff_desc->range_; } -cvdescriptorset::TexelDescriptor::TexelDescriptor(const VkDescriptorType type, - const std::unordered_map *buffer_view_map) - : buffer_view_(VK_NULL_HANDLE), storage_(false), buffer_view_map_(buffer_view_map) { +cvdescriptorset::TexelDescriptor::TexelDescriptor(const VkDescriptorType type) : buffer_view_(VK_NULL_HANDLE), storage_(false) { updated = false; descriptor_class = TexelBuffer; if (VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER == type) storage_ = true; }; -bool cvdescriptorset::TexelDescriptor::WriteUpdate(const VkWriteDescriptorSet *update, const uint32_t index, std::string *error) { - // First validate buffer view - auto buffer_view = update->pTexelBufferView[index]; - if (!buffer_view_map_->count(buffer_view)) { - std::stringstream error_str; - error_str << "Attempted write update to texel buffer descriptor with invalid buffer view: " << buffer_view; - *error = error_str.str(); - return false; - } +void cvdescriptorset::TexelDescriptor::WriteUpdate(const VkWriteDescriptorSet *update, const uint32_t index) { updated = true; - buffer_view_ = buffer_view; - return true; + buffer_view_ = update->pTexelBufferView[index]; } -bool cvdescriptorset::TexelDescriptor::CopyUpdate(const Descriptor *src, std::string *error) { - // First validate buffer view - auto buffer_view = static_cast(src)->buffer_view_; - if (!buffer_view_map_->count(buffer_view)) { +void cvdescriptorset::TexelDescriptor::CopyUpdate(const Descriptor *src) { + updated = true; + buffer_view_ = static_cast(src)->buffer_view_; +} +// This is a helper function that iterates over a set of Write and Copy updates, pulls the DescriptorSet* for updated +// sets, and then calls their respective Validate[Write|Copy]Update functions. +// If the update hits an issue for which the callback returns "true", meaning that the call down the chain should +// be skipped, then true is returned. +// If there is no issue with the update, then false is returned. +bool cvdescriptorset::ValidateUpdateDescriptorSets( + const debug_report_data *report_data, const std::unordered_map &set_map, + const uint32_t write_count, const VkWriteDescriptorSet *p_wds, const uint32_t copy_count, const VkCopyDescriptorSet *p_cds) { + bool skip_call = false; + // Validate Write updates + uint32_t i = 0; + for (i = 0; i < write_count; i++) { + auto dest_set = p_wds[i].dstSet; + auto set_pair = set_map.find(dest_set); + if (set_pair == set_map.end()) { + skip_call |= + log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT, + reinterpret_cast(dest_set), __LINE__, DRAWSTATE_DOUBLE_DESTROY, "DS", + "Cannot call vkUpdateDescriptorSets() on descriptor set 0x%" PRIxLEAST64 " that has not been allocated.", + reinterpret_cast(dest_set)); + } else { + std::string error_str; + if (!set_pair->second->ValidateWriteUpdate(report_data, &p_wds[i], &error_str)) { + skip_call |= log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT, + reinterpret_cast(dest_set), __LINE__, DRAWSTATE_INVALID_UPDATE_INDEX, "DS", + "vkUpdateDescriptorsSets() failed write update validation for Descriptor Set 0x%" PRIx64 + " with error: %s", + reinterpret_cast(dest_set), error_str.c_str()); + } + } + } + // Now validate copy updates + for (i = 0; i < copy_count; ++i) { + auto dst_set = p_cds[i].dstSet; + auto src_set = p_cds[i].srcSet; + auto src_pair = set_map.find(src_set); + auto dst_pair = set_map.find(dst_set); + if (src_pair == set_map.end()) { + skip_call |= log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT, + reinterpret_cast(src_set), __LINE__, DRAWSTATE_DOUBLE_DESTROY, "DS", + "Cannot call vkUpdateDescriptorSets() to copy from descriptor set 0x%" PRIxLEAST64 + " that has not been allocated.", + reinterpret_cast(src_set)); + } else if (dst_pair == set_map.end()) { + skip_call |= log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT, + reinterpret_cast(dst_set), __LINE__, DRAWSTATE_DOUBLE_DESTROY, "DS", + "Cannot call vkUpdateDescriptorSets() to copy to descriptor set 0x%" PRIxLEAST64 + " that has not been allocated.", + reinterpret_cast(dst_set)); + } else { + std::string error_str; + if (!dst_pair->second->ValidateCopyUpdate(report_data, &p_cds[i], src_pair->second, &error_str)) { + skip_call |= + log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT, + reinterpret_cast(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(src_set), reinterpret_cast(dst_set), error_str.c_str()); + } + } + } + return skip_call; +} +// This is a helper function that iterates over a set of Write and Copy updates, pulls the DescriptorSet* for updated +// sets, and then calls their respective Perform[Write|Copy]Update functions. +// Prerequisite : ValidateUpdateDescriptorSets() should be called and return "false" prior to calling PerformUpdateDescriptorSets() +// with the same set of updates. +// This is split from the validate code to allow validation prior to calling down the chain, and then update after +// calling down the chain. +void cvdescriptorset::PerformUpdateDescriptorSets( + const std::unordered_map &set_map, const uint32_t write_count, + const VkWriteDescriptorSet *p_wds, const uint32_t copy_count, const VkCopyDescriptorSet *p_cds) { + // Write updates first + uint32_t i = 0; + for (i = 0; i < write_count; ++i) { + auto dest_set = p_wds[i].dstSet; + auto set_pair = set_map.find(dest_set); + if (set_pair != set_map.end()) { + set_pair->second->PerformWriteUpdate(&p_wds[i]); + } + } + // Now copy updates + for (i = 0; i < copy_count; ++i) { + auto dst_set = p_cds[i].dstSet; + auto src_set = p_cds[i].srcSet; + auto src_pair = set_map.find(src_set); + auto dst_pair = set_map.find(dst_set); + if (src_pair != set_map.end() && dst_pair != set_map.end()) { + dst_pair->second->PerformCopyUpdate(&p_cds[i], src_pair->second); + } + } +} +// Validate the state for a given write update but don't actually perform the update +// If an error would occur for this update, return false and fill in details in error_msg string +bool cvdescriptorset::DescriptorSet::ValidateWriteUpdate(const debug_report_data *report_data, const VkWriteDescriptorSet *update, + std::string *error_msg) { + // Verify idle ds + if (in_use.load()) { std::stringstream error_str; - error_str << "Attempted write update to texel buffer descriptor with invalid buffer view: " << buffer_view; - *error = error_str.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; } - updated = true; - buffer_view_ = buffer_view; + // Verify dst binding exists + if (!p_layout_->HasBinding(update->dstBinding)) { + std::stringstream error_str; + error_str << "DescriptorSet " << set_ << " does not have binding " << update->dstBinding << "."; + *error_msg = error_str.str(); + return false; + } else { + // We know that binding is valid, verify update and do update on each descriptor + auto start_idx = p_layout_->GetGlobalStartIndexFromBinding(update->dstBinding) + update->dstArrayElement; + auto type = p_layout_->GetTypeFromBinding(update->dstBinding); + if (type != update->descriptorType) { + std::stringstream error_str; + error_str << "Attempting write update to descriptor set " << set_ << " binding #" << update->dstBinding << " with type " + << string_VkDescriptorType(type) << " but update type is " << string_VkDescriptorType(update->descriptorType); + *error_msg = error_str.str(); + return false; + } + if ((start_idx + update->descriptorCount) > p_layout_->GetTotalDescriptorCount()) { + 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."; + *error_msg = error_str.str(); + return false; + } + // Verify consecutive bindings match (if needed) + if (!p_layout_->VerifyUpdateConsistency(update->dstBinding, update->dstArrayElement, update->descriptorCount, + "write update to", set_, error_msg)) + return false; + // Update is within bounds and consistent so last step is to validate update contents + if (!VerifyWriteUpdateContents(update, start_idx, error_msg)) { + std::stringstream error_str; + error_str << "Write update to descriptor in set " << set_ << " binding #" << update->dstBinding + << " failed with error message: " << error_msg->c_str(); + *error_msg = error_str.str(); + return false; + } + } + // All checks passed, update is clean return true; } +// Verify that the contents of the update are ok, but don't perform actual update +bool cvdescriptorset::DescriptorSet::VerifyWriteUpdateContents(const VkWriteDescriptorSet *update, const uint32_t index, + std::string *error) const { + switch (update->descriptorType) { + case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: { + for (uint32_t di = 0; di < update->descriptorCount; ++di) { + // Validate image + auto image_view = update->pImageInfo[di].imageView; + auto image_layout = update->pImageInfo[di].imageLayout; + if (!ValidateImageUpdate(image_view, image_layout, image_view_map_, image_map_, image_to_swapchain_map_, swapchain_map_, + error)) { + std::stringstream error_str; + error_str << "Attempted write update to combined image sampler descriptor failed due to: " << error->c_str(); + *error = error_str.str(); + return false; + } + } + // Intentional fall-through to validate sampler + } + case VK_DESCRIPTOR_TYPE_SAMPLER: { + for (uint32_t di = 0; di < update->descriptorCount; ++di) { + if (!descriptors_[index + di].get()->IsImmutableSampler()) { + if (!ValidateSampler(update->pImageInfo[di].sampler, sampler_map_)) { + std::stringstream error_str; + error_str << "Attempted write update to sampler descriptor with invalid sampler: " + << update->pImageInfo[di].sampler << "."; + *error = error_str.str(); + return false; + } + } else { + // TODO : Warn here + } + } + break; + } + case VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE: + case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT: + case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: { + for (uint32_t di = 0; di < update->descriptorCount; ++di) { + auto image_view = update->pImageInfo[di].imageView; + auto image_layout = update->pImageInfo[di].imageLayout; + if (!ValidateImageUpdate(image_view, image_layout, image_view_map_, image_map_, image_to_swapchain_map_, swapchain_map_, + error)) { + std::stringstream error_str; + error_str << "Attempted write update to image descriptor failed due to: " << error->c_str(); + *error = error_str.str(); + return false; + } + } + break; + } + case VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER: + case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER: { + for (uint32_t di = 0; di < update->descriptorCount; ++di) { + auto buffer_view = update->pTexelBufferView[di]; + if (!buffer_view_map_->count(buffer_view)) { + std::stringstream error_str; + error_str << "Attempted write update to texel buffer descriptor with invalid buffer view: " << buffer_view; + *error = error_str.str(); + return false; + } + } + break; + } + case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER: + case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC: + case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER: + case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC: { + for (uint32_t di = 0; di < update->descriptorCount; ++di) { + auto buffer = update->pBufferInfo[di].buffer; + if (!buffer_map_->count(buffer)) { + std::stringstream error_str; + error_str << "Attempted write update to buffer descriptor with invalid buffer: " << buffer; + *error = error_str.str(); + return false; + } + } + break; + } + default: + assert(0); // We've already verified update type so should never get here + break; + } + // All checks passed so update contents are good + return true; +} +// Verify that the contents of the update are ok, but don't perform actual update +bool cvdescriptorset::DescriptorSet::VerifyCopyUpdateContents(const VkCopyDescriptorSet *update, const DescriptorSet *src_set, + const uint32_t index, std::string *error) const { + switch (src_set->descriptors_[index]->descriptor_class) { + case PlainSampler: { + for (uint32_t di = 0; di < update->descriptorCount; ++di) { + if (!src_set->descriptors_[index + di]->IsImmutableSampler()) { + auto update_sampler = static_cast(src_set->descriptors_[index + di].get())->GetSampler(); + if (!ValidateSampler(update_sampler, sampler_map_)) { + std::stringstream error_str; + error_str << "Attempted copy update to sampler descriptor with invalid sampler: " << update_sampler << "."; + *error = error_str.str(); + return false; + } + } else { + // TODO : Warn here + } + } + break; + } + case ImageSampler: { + for (uint32_t di = 0; di < update->descriptorCount; ++di) { + auto img_samp_desc = static_cast(src_set->descriptors_[index + di].get()); + // First validate sampler + if (!img_samp_desc->IsImmutableSampler()) { + auto update_sampler = img_samp_desc->GetSampler(); + if (!ValidateSampler(update_sampler, sampler_map_)) { + std::stringstream error_str; + error_str << "Attempted copy update to sampler descriptor with invalid sampler: " << update_sampler << "."; + *error = error_str.str(); + return false; + } + } else { + // TODO : Warn here + } + // Validate image + auto image_view = img_samp_desc->GetImageView(); + auto image_layout = img_samp_desc->GetImageLayout(); + if (!ValidateImageUpdate(image_view, image_layout, image_view_map_, image_map_, image_to_swapchain_map_, swapchain_map_, + error)) { + std::stringstream error_str; + error_str << "Attempted write update to combined image sampler descriptor failed due to: " << error->c_str(); + *error = error_str.str(); + return false; + } + } + } + case Image: { + for (uint32_t di = 0; di < update->descriptorCount; ++di) { + auto img_desc = static_cast(src_set->descriptors_[index + di].get()); + auto image_view = img_desc->GetImageView(); + auto image_layout = img_desc->GetImageLayout(); + if (!ValidateImageUpdate(image_view, image_layout, image_view_map_, image_map_, image_to_swapchain_map_, swapchain_map_, + error)) { + std::stringstream error_str; + error_str << "Attempted write update to image descriptor failed due to: " << error->c_str(); + *error = error_str.str(); + return false; + } + } + break; + } + case TexelBuffer: { + for (uint32_t di = 0; di < update->descriptorCount; ++di) { + auto buffer_view = static_cast(src_set->descriptors_[index + di].get())->GetBufferView(); + if (!buffer_view_map_->count(buffer_view)) { + std::stringstream error_str; + error_str << "Attempted write update to texel buffer descriptor with invalid buffer view: " << buffer_view; + *error = error_str.str(); + return false; + } + } + break; + } + case GeneralBuffer: { + for (uint32_t di = 0; di < update->descriptorCount; ++di) { + auto buffer = static_cast(src_set->descriptors_[index + di].get())->GetBuffer(); + if (!buffer_map_->count(buffer)) { + std::stringstream error_str; + error_str << "Attempted write update to buffer descriptor with invalid buffer: " << buffer; + *error = error_str.str(); + return false; + } + } + break; + } + default: + assert(0); // We've already verified update type so should never get here + break; + } + // All checks passed so update contents are good + return true; +} \ No newline at end of file diff --git a/layers/descriptor_sets.h b/layers/descriptor_sets.h index 5abe45f..655d572 100644 --- a/layers/descriptor_sets.h +++ b/layers/descriptor_sets.h @@ -146,8 +146,8 @@ typedef enum _DescriptorClass { PlainSampler, ImageSampler, Image, TexelBuffer, class Descriptor { public: virtual ~Descriptor(){}; - virtual bool WriteUpdate(const VkWriteDescriptorSet *, const uint32_t, std::string *) = 0; - virtual bool CopyUpdate(const Descriptor *, std::string *) = 0; + virtual void WriteUpdate(const VkWriteDescriptorSet *, const uint32_t) = 0; + virtual void CopyUpdate(const Descriptor *) = 0; virtual DescriptorClass GetClass() const { return descriptor_class; }; // Special fast-path check for SamplerDescriptors that are immutable virtual bool IsImmutableSampler() const { return false; }; @@ -167,51 +167,41 @@ bool ValidateImageUpdate(const VkImageView, const VkImageLayout, const std::unor class SamplerDescriptor : public Descriptor { public: - SamplerDescriptor(const std::unordered_map> *); - SamplerDescriptor(const VkSampler *, const std::unordered_map> *); - bool WriteUpdate(const VkWriteDescriptorSet *, const uint32_t, std::string *) override; - bool CopyUpdate(const Descriptor *, std::string *) override; + SamplerDescriptor(); + SamplerDescriptor(const VkSampler *); + void WriteUpdate(const VkWriteDescriptorSet *, const uint32_t) override; + void CopyUpdate(const Descriptor *) override; virtual bool IsImmutableSampler() const override { return immutable_; }; + VkSampler GetSampler() const { return sampler_; } private: // bool ValidateSampler(const VkSampler) const; VkSampler sampler_; bool immutable_; - const std::unordered_map> *sampler_map_; }; class ImageSamplerDescriptor : public Descriptor { public: - ImageSamplerDescriptor(const std::unordered_map *, - const std::unordered_map *, const std::unordered_map *, - const std::unordered_map *, - const std::unordered_map> *); - ImageSamplerDescriptor(const VkSampler *, const std::unordered_map *, - const std::unordered_map *, const std::unordered_map *, - const std::unordered_map *, - const std::unordered_map> *); - bool WriteUpdate(const VkWriteDescriptorSet *, const uint32_t, std::string *) override; - bool CopyUpdate(const Descriptor *, std::string *) override; + ImageSamplerDescriptor(); + ImageSamplerDescriptor(const VkSampler *); + void WriteUpdate(const VkWriteDescriptorSet *, const uint32_t) override; + void CopyUpdate(const Descriptor *) override; + VkSampler GetSampler() const { return sampler_; } + VkImageView GetImageView() const { return image_view_; } + VkImageLayout GetImageLayout() const { return image_layout_; } private: VkSampler sampler_; bool immutable_; VkImageView image_view_; VkImageLayout image_layout_; - const std::unordered_map> *sampler_map_; - const std::unordered_map *image_view_map_; - const std::unordered_map *image_map_; - const std::unordered_map *image_to_swapchain_map_; - const std::unordered_map *swapchain_map_; }; class ImageDescriptor : public Descriptor { public: - ImageDescriptor(const VkDescriptorType, const std::unordered_map *, - const std::unordered_map *, const std::unordered_map *, - const std::unordered_map *); - bool WriteUpdate(const VkWriteDescriptorSet *, const uint32_t, std::string *) override; - bool CopyUpdate(const Descriptor *, std::string *) override; + ImageDescriptor(const VkDescriptorType); + void WriteUpdate(const VkWriteDescriptorSet *, const uint32_t) override; + void CopyUpdate(const Descriptor *) override; virtual bool IsStorage() const override { return storage_; } VkImageView GetImageView() const { return image_view_; } VkImageLayout GetImageLayout() const { return image_layout_; } @@ -220,31 +210,26 @@ class ImageDescriptor : public Descriptor { bool storage_; VkImageView image_view_; VkImageLayout image_layout_; - const std::unordered_map *image_view_map_; - const std::unordered_map *image_map_; - const std::unordered_map *image_to_swapchain_map_; - const std::unordered_map *swapchain_map_; }; class TexelDescriptor : public Descriptor { public: - TexelDescriptor(const VkDescriptorType, const std::unordered_map *); - bool WriteUpdate(const VkWriteDescriptorSet *, const uint32_t, std::string *) override; - bool CopyUpdate(const Descriptor *, std::string *) override; + TexelDescriptor(const VkDescriptorType); + void WriteUpdate(const VkWriteDescriptorSet *, const uint32_t) override; + void CopyUpdate(const Descriptor *) override; virtual bool IsStorage() const override { return storage_; } VkBufferView GetBufferView() const { return buffer_view_; } private: VkBufferView buffer_view_; bool storage_; - const std::unordered_map *buffer_view_map_; }; class BufferDescriptor : public Descriptor { public: - BufferDescriptor(const VkDescriptorType, const std::unordered_map *); - bool WriteUpdate(const VkWriteDescriptorSet *, const uint32_t, std::string *) override; - bool CopyUpdate(const Descriptor *, std::string *) override; + BufferDescriptor(const VkDescriptorType); + void WriteUpdate(const VkWriteDescriptorSet *, const uint32_t) override; + void CopyUpdate(const Descriptor *) override; virtual bool IsDynamic() const override { return dynamic_; } virtual bool IsStorage() const override { return storage_; } VkBuffer GetBuffer() const { return buffer_; } @@ -257,11 +242,15 @@ class BufferDescriptor : public Descriptor { VkBuffer buffer_; VkDeviceSize offset_; VkDeviceSize range_; - const std::unordered_map *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); +// Helper functions for Updating descriptor sets since it crosses multiple sets +// Validate will make sure an update is ok without actually performing it +bool ValidateUpdateDescriptorSets(const debug_report_data *, + const std::unordered_map &, const uint32_t, + const VkWriteDescriptorSet *, const uint32_t, const VkCopyDescriptorSet *); +// Perform does the update with the assumption that ValidateUpdateDescriptorSets() has passed for the given update +void PerformUpdateDescriptorSets(const std::unordered_map &, const uint32_t, + const VkWriteDescriptorSet *, const uint32_t, const VkCopyDescriptorSet *); /* * DescriptorSet class * @@ -323,10 +312,16 @@ class DescriptorSet : public BASE_NODE { // For all descriptors in a set, add any buffers and images that may be updated to their respective unordered_sets & return // number of objects inserted uint32_t GetAllStorageUpdates(std::unordered_set *, std::unordered_set *) const; - // Perform write update based on update struct - bool WriteUpdate(debug_report_data *, const VkWriteDescriptorSet *, std::string *); - // Perform copy update, using 'this' set as the dest and the passed-in DescriptorSet as the src - bool CopyUpdate(debug_report_data *, const VkCopyDescriptorSet *, const DescriptorSet *, std::string *); + + // Descriptor Update functions. These functions validate state and perform update separately + // Validate contents of a WriteUpdate + bool ValidateWriteUpdate(const debug_report_data *, const VkWriteDescriptorSet *, std::string *); + // Perform a WriteUpdate whose contents were just validated using ValidateWriteUpdate + void PerformWriteUpdate(const VkWriteDescriptorSet *); + // Validate contents of a CopyUpdate + bool ValidateCopyUpdate(const debug_report_data *, const VkCopyDescriptorSet *, const DescriptorSet *, std::string *); + // Perform a CopyUpdate whose contents were just validated using ValidateCopyUpdate + void PerformCopyUpdate(const VkCopyDescriptorSet *, const DescriptorSet *); const DescriptorSetLayout *GetLayout() const { return p_layout_; }; VkDescriptorSet GetSet() const { return set_; }; @@ -350,7 +345,8 @@ class DescriptorSet : public BASE_NODE { bool IsUpdated() const { return some_update_; }; private: - bool ValidateUpdate(const VkWriteDescriptorSet *, const uint32_t, std::string *) const; + bool VerifyWriteUpdateContents(const VkWriteDescriptorSet *, const uint32_t, std::string *) const; + bool VerifyCopyUpdateContents(const VkCopyDescriptorSet *, const DescriptorSet *, const uint32_t, std::string *) const; // Private helper to set all bound cmd buffers to INVALID state void InvalidateBoundCmdBuffers(); bool some_update_; // has any part of the set ever been updated?