From 6f433a8b2f8bb5b5446054a46e3399896b215c04 Mon Sep 17 00:00:00 2001 From: John Zulauf Date: Tue, 6 Mar 2018 16:44:43 -0700 Subject: layers: Cleanup naming for layout compatibiity Type and variable names have been clarified for the pipeline layout array of descriptor set layouts based on reviewer feedback. Change-Id: I6b191b2121db87285f7fd50810991340c02475c4 --- layers/core_validation.cpp | 34 ++++++++++++++++++++-------------- layers/core_validation_types.h | 15 +++++++-------- layers/descriptor_sets.cpp | 2 ++ layers/descriptor_sets.h | 7 ++++--- 4 files changed, 33 insertions(+), 25 deletions(-) diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index f6b0bb34..e1feeada 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -95,9 +95,9 @@ size_t PipelineLayoutCompatDef::hash() const { hash_util::HashCombiner hc; // The set number is integral to the CompatDef's distinctiveness hc << set << push_constant_ranges.get(); - const auto &layout_layout = *layout_id.get(); + const auto &descriptor_set_layouts = *set_layouts_id.get(); for (uint32_t i = 0; i <= set; i++) { - hc << layout_layout[i].get(); + hc << descriptor_set_layouts[i].get(); } return hc.Value(); } @@ -107,12 +107,18 @@ bool PipelineLayoutCompatDef::operator==(const PipelineLayoutCompatDef &other) c return false; } - const auto &layout_layout = *layout_id.get(); - assert(set < layout_layout.size()); - const auto &other_ll = *other.layout_id.get(); - assert(set < other_ll.size()); + if (set_layouts_id == other.set_layouts_id) { + // if it's the same set_layouts_id, then *any* subset will match + return true; + } + + // They aren't exactly the same PipelineLayoutSetLayouts, so we need to check if the required subsets match + const auto &descriptor_set_layouts = *set_layouts_id.get(); + assert(set < descriptor_set_layouts.size()); + const auto &other_ds_layouts = *other.set_layouts_id.get(); + assert(set < other_ds_layouts.size()); for (uint32_t i = 0; i <= set; i++) { - if (layout_layout[i] != other_ll[i]) { + if (descriptor_set_layouts[i] != other_ds_layouts[i]) { return false; } } @@ -5446,14 +5452,14 @@ PushConstantRangesId get_canonical_id(const VkPipelineLayoutCreateInfo *info) { } // Dictionary of canoncial form of the pipeline set layout of descriptor set layouts -static DescriptorSetLayoutLayoutDict layout_layout_dict; +static PipelineLayoutSetLayoutsDict pipeline_layout_set_layouts_dict; // Dictionary of canonical form of the "compatible for set" records static PipelineLayoutCompatDict pipeline_layout_compat_dict; static PipelineLayoutCompatId get_canonical_id(const uint32_t set_index, const PushConstantRangesId pcr_id, - const DescriptorSetLayoutLayoutId ll_id) { - return pipeline_layout_compat_dict.look_up(PipelineLayoutCompatDef(set_index, pcr_id, ll_id)); + const PipelineLayoutSetLayoutsId set_layouts_id) { + return pipeline_layout_compat_dict.look_up(PipelineLayoutCompatDef(set_index, pcr_id, set_layouts_id)); } static void PostCallRecordCreatePipelineLayout(layer_data *dev_data, const VkPipelineLayoutCreateInfo *pCreateInfo, @@ -5463,20 +5469,20 @@ static void PostCallRecordCreatePipelineLayout(layer_data *dev_data, const VkPip PIPELINE_LAYOUT_NODE &plNode = dev_data->pipelineLayoutMap[*pPipelineLayout]; plNode.layout = *pPipelineLayout; plNode.set_layouts.resize(pCreateInfo->setLayoutCount); - DescriptorSetLayoutLayoutDef layout_layout(pCreateInfo->setLayoutCount); + PipelineLayoutSetLayoutsDef set_layouts(pCreateInfo->setLayoutCount); for (uint32_t i = 0; i < pCreateInfo->setLayoutCount; ++i) { plNode.set_layouts[i] = GetDescriptorSetLayout(dev_data, pCreateInfo->pSetLayouts[i]); - layout_layout[i] = plNode.set_layouts[i]->get_layout_id(); + set_layouts[i] = plNode.set_layouts[i]->get_layout_id(); } // Get canonical form IDs for the "compatible for set" contents plNode.push_constant_ranges = get_canonical_id(pCreateInfo); - auto ll_id = layout_layout_dict.look_up(layout_layout); + auto set_layouts_id = pipeline_layout_set_layouts_dict.look_up(set_layouts); plNode.compat_for_set.reserve(pCreateInfo->setLayoutCount); // Create table of "compatible for set N" cannonical forms for trivial accept validation for (uint32_t i = 0; i < pCreateInfo->setLayoutCount; ++i) { - plNode.compat_for_set.emplace_back(get_canonical_id(i, plNode.push_constant_ranges, ll_id)); + plNode.compat_for_set.emplace_back(get_canonical_id(i, plNode.push_constant_ranges, set_layouts_id)); } // Implicit unlock diff --git a/layers/core_validation_types.h b/layers/core_validation_types.h index 1db4e4e4..3a4b0cb9 100644 --- a/layers/core_validation_types.h +++ b/layers/core_validation_types.h @@ -551,10 +551,10 @@ using PushConstantRangesId = PushConstantRangesDict::Id; // Canonical dictionary for the pipeline layout's layout of descriptorsetlayouts using DescriptorSetLayoutDef = cvdescriptorset::DescriptorSetLayoutDef; using DescriptorSetLayoutId = std::shared_ptr; -using DescriptorSetLayoutLayoutDef = std::vector; -using DescriptorSetLayoutLayoutDict = - hash_util::Dictionary>; -using DescriptorSetLayoutLayoutId = DescriptorSetLayoutLayoutDict::Id; +using PipelineLayoutSetLayoutsDef = std::vector; +using PipelineLayoutSetLayoutsDict = + hash_util::Dictionary>; +using PipelineLayoutSetLayoutsId = PipelineLayoutSetLayoutsDict::Id; // Defines/stores a compatibility defintion for set N // The "layout layout" must store at least set+1 entries, but only the first set+1 are considered for hash and equality testing @@ -563,10 +563,9 @@ using DescriptorSetLayoutLayoutId = DescriptorSetLayoutLayoutDict::Id; struct PipelineLayoutCompatDef { uint32_t set; PushConstantRangesId push_constant_ranges; - using LayoutLayout = DescriptorSetLayoutLayoutId; - LayoutLayout layout_id; - PipelineLayoutCompatDef(const uint32_t set_index, const PushConstantRangesId pcr_id, const LayoutLayout ll_id) - : set(set_index), push_constant_ranges(pcr_id), layout_id(ll_id) {} + PipelineLayoutSetLayoutsId set_layouts_id; + PipelineLayoutCompatDef(const uint32_t set_index, const PushConstantRangesId pcr_id, const PipelineLayoutSetLayoutsId sl_id) + : set(set_index), push_constant_ranges(pcr_id), set_layouts_id(sl_id) {} size_t hash() const; bool operator==(const PipelineLayoutCompatDef &other) const; }; diff --git a/layers/descriptor_sets.cpp b/layers/descriptor_sets.cpp index 928e4095..66a8a48e 100644 --- a/layers/descriptor_sets.cpp +++ b/layers/descriptor_sets.cpp @@ -217,6 +217,8 @@ bool cvdescriptorset::DescriptorSetLayout::IsCompatible(DescriptorSetLayout cons return detailed_compat_check; } +// Do a detailed compatibility check of this def (referenced by ds_layout), vs. the rhs (layout and def) +// Should only be called if trivial accept has failed, and in that context should return false. bool cvdescriptorset::DescriptorSetLayoutDef::IsCompatible(VkDescriptorSetLayout ds_layout, VkDescriptorSetLayout rh_ds_layout, DescriptorSetLayoutDef const *const rh_ds_layout_def, std::string *error_msg) const { diff --git a/layers/descriptor_sets.h b/layers/descriptor_sets.h index 866de8c9..133657a7 100644 --- a/layers/descriptor_sets.h +++ b/layers/descriptor_sets.h @@ -103,8 +103,8 @@ class DescriptorSetLayoutDef { const std::set &GetSortedBindingSet() const { return non_empty_bindings_; } // Return true if given binding is present in this layout bool HasBinding(const uint32_t binding) const { return binding_to_index_map_.count(binding) > 0; }; - // Return true if this layout is compatible with passed in layout from a pipelineLayout, - // else return false and update error_msg with description of incompatibility + // Return true if this DSL Def (referenced by the 1st layout) is compatible with another DSL Def (ref'd from the 2nd layout) + // else return false and update error_msg with description of incompatibility bool IsCompatible(VkDescriptorSetLayout, VkDescriptorSetLayout, DescriptorSetLayoutDef const *const, std::string *) const; // Return true if binding 1 beyond given exists and has same type, stageFlags & immutable sampler use bool IsNextBindingConsistent(const uint32_t) const; @@ -161,7 +161,8 @@ class DescriptorSetLayoutDef { const BindingTypeStats &GetBindingTypeStats() const { return binding_type_stats_; } private: - // Only the first two are needed for hash and equality checks, the other fields are derivative them uniquely + // Only the first two data members are used for hash and equality checks, the other members are derived from them, and are used + // to speed up the various lookups/queries/validations VkDescriptorSetLayoutCreateFlags flags_; std::vector bindings_; -- cgit v1.2.3