layers: Fix Layout Set Compatible check

KhronosGroup · Oct 3, 2024 · 16c0528 · 16c0528
1 parent a0b10c0
commit 16c0528
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 13 deletions.
diff --git a/layers/core_checks/cc_descriptor.cpp b/layers/core_checks/cc_descriptor.cpp
@@ -3660,16 +3660,17 @@ bool CoreChecks::ValidateCmdPushDescriptorSetWithTemplate(VkCommandBuffer comman
                              FormatHandle(descriptorUpdateTemplate).c_str(), template_ci.set, set);
         }
         auto template_layout = Get<vvl::PipelineLayout>(template_ci.pipelineLayout);
-        if (!IsPipelineLayoutSetCompat(set, layout_data.get(), template_layout.get())) {
+        if (!IsPipelineLayoutSetCompatible(set, layout_data.get(), template_layout.get())) {
             const LogObjectList objlist(commandBuffer, descriptorUpdateTemplate, template_ci.pipelineLayout, layout);
             const char *vuid = is_2 ? "VUID-VkPushDescriptorSetWithTemplateInfoKHR-layout-07993"
                                     : "VUID-vkCmdPushDescriptorSetWithTemplateKHR-layout-07993";
             skip |= LogError(vuid, objlist, loc.dot(Field::descriptorUpdateTemplate),
                              "%s created with %s is incompatible "
                              "with command parameter "
-                             "%s for set %" PRIu32,
+                             "%s for set %" PRIu32 ".\n%s",
                              FormatHandle(descriptorUpdateTemplate).c_str(), FormatHandle(template_ci.pipelineLayout).c_str(),
-                             FormatHandle(layout).c_str(), set);
+                             FormatHandle(layout).c_str(), set,
+                             DescribePipelineLayoutSetNonCompatible(set, layout_data.get(), template_layout.get()).c_str());
         }
     }
 

diff --git a/layers/state_tracker/descriptor_sets.cpp b/layers/state_tracker/descriptor_sets.cpp
@@ -202,6 +202,11 @@ bool operator==(const DescriptorSetLayoutDef &lhs, const DescriptorSetLayoutDef
     for (size_t i = 0; i < lhs_bindings.size(); i++) {
         const auto &l = lhs_bindings[i];
         const auto &r = rhs_bindings[i];
+        // For things where we are comparing with the bound pipeline, the binding will always be right, but when comparing two
+        // arbitrary layouts (ex. templates, Device Generated Commands, etc) the bindings might be different
+        if (l.binding != r.binding) {
+            return false;
+        }
         if (l.descriptorType != r.descriptorType || l.descriptorCount != r.descriptorCount || l.stageFlags != r.stageFlags) {
             return false;
         }
@@ -247,7 +252,10 @@ std::string DescriptorSetLayoutDef::DescribeDifference(uint32_t index, const Des
         for (size_t i = 0; i < lhs_bindings.size(); i++) {
             const auto &l = lhs_bindings[i];
             const auto &r = rhs_bindings[i];
-            if (l.descriptorType != r.descriptorType) {
+            if (l.binding != r.binding) {
+                ss << "VkDescriptorSetLayoutBinding::binding " << l.binding << " doesn't match " << r.binding;
+                break;
+            } else if (l.descriptorType != r.descriptorType) {
                 ss << "binding " << i << " descriptorType " << string_VkDescriptorType(l.descriptorType) << " doesn't match "
                    << string_VkDescriptorType(r.descriptorType);
                 break;

diff --git a/layers/state_tracker/pipeline_state.cpp b/layers/state_tracker/pipeline_state.cpp
@@ -1350,6 +1350,32 @@ std::string LastBound::DescribeNonCompatibleSet(uint32_t set, const vvl::ShaderO
     return ss.str();
 }
 
+bool IsPipelineLayoutSetCompatible(uint32_t set, const vvl::PipelineLayout *a, const vvl::PipelineLayout *b) {
+    if (!a || !b) {
+        return false;
+    }
+    if ((set >= a->set_compat_ids.size()) || (set >= b->set_compat_ids.size())) {
+        return false;
+    }
+    return *(a->set_compat_ids[set]) == *(b->set_compat_ids[set]);
+}
+
+std::string DescribePipelineLayoutSetNonCompatible(uint32_t set, const vvl::PipelineLayout *a, const vvl::PipelineLayout *b) {
+    std::ostringstream ss;
+    if (!a || !b) {
+        ss << "The set (" << set << ") has a null VkPipelineLayout object\n";
+    } else if (set >= a->set_compat_ids.size()) {
+        ss << "The set (" << set << ") is out of bounds for the number of sets in the non-compatible VkDescriptorSetLayout ("
+           << a->set_compat_ids.size() << ")\n";
+    } else if (set >= b->set_compat_ids.size()) {
+        ss << "The set (" << set << ") is out of bounds for the number of sets in the non-compatible VkDescriptorSetLayout ("
+           << b->set_compat_ids.size() << ")\n";
+    } else {
+        return a->set_compat_ids[set]->DescribeDifference(*(b->set_compat_ids[set]));
+    }
+    return ss.str();
+}
+
 const spirv::EntryPoint *LastBound::GetFragmentEntryPoint() const {
     if (pipeline_state && pipeline_state->fragment_shader_state) {
         return pipeline_state->fragment_shader_state->fragment_entry_point.get();

diff --git a/layers/state_tracker/pipeline_state.h b/layers/state_tracker/pipeline_state.h
@@ -751,15 +751,9 @@ struct LastBound {
     const spirv::EntryPoint *GetFragmentEntryPoint() const;
 };
 
-static inline bool IsPipelineLayoutSetCompat(uint32_t set, const vvl::PipelineLayout *a, const vvl::PipelineLayout *b) {
-    if (!a || !b) {
-        return false;
-    }
-    if ((set >= a->set_compat_ids.size()) || (set >= b->set_compat_ids.size())) {
-        return false;
-    }
-    return a->set_compat_ids[set] == b->set_compat_ids[set];
-}
+// Used to compare 2 layouts independently when not tied to the last bound object
+bool IsPipelineLayoutSetCompatible(uint32_t set, const vvl::PipelineLayout *a, const vvl::PipelineLayout *b);
+std::string DescribePipelineLayoutSetNonCompatible(uint32_t set, const vvl::PipelineLayout *a, const vvl::PipelineLayout *b);
 
 enum LvlBindPoint {
     BindPoint_Graphics = VK_PIPELINE_BIND_POINT_GRAPHICS,