layers: MR77, Add draw-time checks for descriptorSet compatibility
When PSO is created, gather the group of sets that are actually used by that PSO.
Then, at Draw time, verify that for each set used by the bound PSO, there is a
corresponding valid descriptorSet bound which also matches the associated
setLayout from the PSO's pipelineLayout. Added two more tests to the
DescriptorSetCompatibility test case to verify that these checks correctly fire.
diff --git a/layers/draw_state.cpp b/layers/draw_state.cpp
index 06b2c8d..2dd1542 100644
--- a/layers/draw_state.cpp
+++ b/layers/draw_state.cpp
@@ -21,10 +21,10 @@
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
*
- * Author: Cody Northrop <cody@lunarg.com>
+ * Author: Cody Northrop <cnorthrop@google.com>
* Author: Michael Lentine <mlentine@google.com>
- * Author: Tobin Ehlis <tobin@lunarg.com>
- * Author: Chia-I Wu <olv@lunarg.com>
+ * Author: Tobin Ehlis <tobine@google.com>
+ * Author: Chia-I Wu <olv@google.com>
* Author: Chris Forbes <chrisf@ijw.co.nz>
*/
@@ -40,6 +40,7 @@
#include <algorithm>
#include <list>
#include <spirv.hpp>
+#include <set>
#include "vk_loader_platform.h"
#include "vk_dispatch_table_helper.h"
@@ -80,7 +81,6 @@
struct layer_data {
debug_report_data* report_data;
- // TODO: put instance data here
std::vector<VkDbgMsgCallback> logging_callback;
VkLayerDispatchTable* device_dispatch_table;
VkLayerInstanceDispatchTable* instance_dispatch_table;
@@ -1127,65 +1127,12 @@
return true;
}
-// Check if given pipeline layouts are compatible from set 0 up through descriptorSet numDescriptorSets-1
-//static VkBool32 validate_pipeline_layout_compatibility(layer_data* my_data, const VkPipelineLayout psoLayout, const VkPipelineLayout dsLayout, const uint32_t numDescriptorSets)
-//{
-// VkBool32 skipCall = VK_FALSE;
-// // Handle trivial case of same pipeline layouts, which are obviously compatible
-// if (psoLayout == dsLayout)
-// return skipCall;
-// // Check for trivial fail cases
-// if (my_data->pipelineLayoutMap.find(psoLayout) == my_data->pipelineLayoutMap.end()) {
-// skipCall |= log_msg(my_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_PIPELINE_LAYOUT, (uint64_t) psoLayout, 0, DRAWSTATE_INVALID_PIPELINE_LAYOUT, "DS",
-// "PSO bound invalid pipelineLayout object %#" PRIxLEAST64, (uint64_t) psoLayout);
-//// skipCall |= log_msg(my_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_PIPELINE_LAYOUT, psoLayout, 0, DRAWSTATE_INVALID_PIPELINE_LAYOUT, "DS",
-//// "Created Descriptor Pool %#" PRIxLEAST64, (uint64_t) psoLayout);
-// //"PSO bound invalid pipelineLayout object %#" PRIxLEAST64, (uint64_t) psoLayout);
-// }
-// if (my_data->pipelineLayoutMap.find(dsLayout) == my_data->pipelineLayoutMap.end()) {
-// skipCall |= log_msg(my_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_PIPELINE_LAYOUT, (uint64_t) dsLayout, 0, DRAWSTATE_INVALID_PIPELINE_LAYOUT, "DS",
-// "Invalid pipelineLayout object (%#" PRIxLEAST64 ") bound at last vkCmdBindDescriptorSets() call.", (uint64_t) dsLayout);
-// }
-// if (skipCall) // If we hit fail case above, return early to avoid crash
-// return skipCall;
-// // Two pipeline layouts are defined to be "compatible for set N" if they were created with matching
-// // (the same, or identically defined) descriptor set layouts for sets zero through N.
-// PIPELINE_LAYOUT_NODE psoPL = my_data->pipelineLayoutMap[psoLayout];
-// PIPELINE_LAYOUT_NODE dsPL = my_data->pipelineLayoutMap[dsLayout];
-// // If either pipelineLayout doesn't have enough layouts, they can't be compatible
-// if (numDescriptorSets > psoPL.descriptorSetLayouts.size()) {
-// skipCall |= log_msg(my_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_PIPELINE_LAYOUT, (uint64_t) dsLayout, 0, DRAWSTATE_INVALID_PIPELINE_LAYOUT, "DS",
-// "Invalid pipelineLayout object (%#" PRIxLEAST64 ") bound at last vkCmdBindDescriptorSets() call.", (uint64_t) dsLayout);
-// }
-// if (numDescriptorSets > dsPL.descriptorSetLayouts.size()) {
-// // Not enough layouts, can't be compatible
-// }
-// // Now check each matching layout to confirm compatibility
-// LAYOUT_NODE *pPSOLayout, *pDSLayout;
-// for (uint32_t i=0; i<numDescriptorSets; ++i) {
-// pPSOLayout = my_data->descriptorSetLayoutMap[psoPL.descriptorSetLayouts[i]];
-// pDSLayout = my_data->descriptorSetLayoutMap[dsPL.descriptorSetLayouts[i]];
-// if (pPSOLayout == pDSLayout)
-// continue;
-// if (pPSOLayout->descriptorTypes.size() != pDSLayout->descriptorTypes.size()) {
-// // ERROR : different sizes in layouts
-// }
-// // Need to verify that layouts are identically defined
-// // TODO : Is below sufficient? Making sure that types & stageFlags match per descriptor
-// for (uint32_t j=0; j<pPSOLayout->descriptorTypes.size(); ++j) {
-// if (pPSOLayout->descriptorTypes[j] != pDSLayout->descriptorTypes[j]) {
-// // ERROR : Report type mismatch
-// }
-// if (pPSOLayout->stageFlags[j] != pDSLayout->stageFlags[j]) {
-// // ERROR : Report flags mismatch
-// }
-// }
-// }
-// return skipCall;
-//}
+// Validate that the shaders used by the given pipeline
+// As a side effect this function also records the sets that are actually used by the pipeline
static bool
-validate_pipeline_shaders(layer_data *my_data, VkDevice dev, VkGraphicsPipelineCreateInfo const *pCreateInfo)
+validate_pipeline_shaders(layer_data *my_data, VkDevice dev, PIPELINE_NODE* pPipeline)
{
+ VkGraphicsPipelineCreateInfo const *pCreateInfo = &pPipeline->graphicsPipelineCI;
/* We seem to allow pipeline stages to be specified out of order, so collect and identify them
* before trying to do anything more: */
int vertex_stage = get_shader_stage_id(VK_SHADER_STAGE_VERTEX_BIT);
@@ -1222,6 +1169,8 @@
&(my_data->pipelineLayoutMap[pCreateInfo->layout].descriptorSetLayouts) : nullptr;
for (auto it = descriptor_uses.begin(); it != descriptor_uses.end(); it++) {
+ // As a side-effect of this function, capture which sets are used by the pipeline
+ pPipeline->active_sets.insert(it->first.first);
/* find the matching binding */
auto found = has_descriptor_binding(my_data, layouts, it->first);
@@ -1294,8 +1243,19 @@
// There is probably a better way to gate when this check happens, and to know if something *should* have been bound
// We should have that check separately and then gate this check based on that check
if (pPipe && (pCB->lastBoundPipelineLayout)) {
- // TODO : For all sets used by the PSO, need to make sure that something is bound AND that PSO pipelineLayout is compatible with each bound set
- // first trick is how to get the sets used by PSO. ShaderChecker does this so perhaps move that code to utils
+ string errorString;
+ for (auto setIndex : pPipe->active_sets) {
+ // If valid set is not bound throw an error
+ if ((pCB->boundDescriptorSets.size() <= setIndex) || (!pCB->boundDescriptorSets[setIndex])) {
+ result |= log_msg(my_data->report_data, VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) 0, 0, 0, DRAWSTATE_DESCRIPTOR_SET_NOT_BOUND, "DS",
+ "VkPipeline %#" PRIxLEAST64 " uses set #%u but that set is not bound.", (uint64_t)pPipe->pipeline, setIndex);
+ } else if (!verify_set_layout_compatibility(my_data, my_data->setMap[pCB->boundDescriptorSets[setIndex]], pPipe->graphicsPipelineCI.layout, setIndex, errorString)) {
+ // Set is bound but not compatible w/ overlapping pipelineLayout from PSO
+ VkDescriptorSet setHandle = my_data->setMap[pCB->boundDescriptorSets[setIndex]]->set;
+ result |= log_msg(my_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_DESCRIPTOR_SET, (uint64_t)setHandle, 0, DRAWSTATE_PIPELINE_LAYOUTS_INCOMPATIBLE, "DS",
+ "VkDescriptorSet (%#" PRIxLEAST64 ") bound as set #%u is not compatible with overlapping VkPipelineLayout %#" PRIxLEAST64 " due to: %s", (uint64_t)setHandle, setIndex, (uint64_t)pPipe->graphicsPipelineCI.layout, errorString.c_str());
+ }
+ }
}
// Verify Vtx binding
if (MAX_BINDING != pCB->lastVtxBinding) {
@@ -1331,10 +1291,11 @@
}
// Verify that create state for a pipeline is valid
-static VkBool32 verifyPipelineCreateState(layer_data* my_data, const VkDevice device, const PIPELINE_NODE* pPipeline)
+static VkBool32 verifyPipelineCreateState(layer_data* my_data, const VkDevice device, PIPELINE_NODE* pPipeline)
{
VkBool32 skipCall = VK_FALSE;
- if (!validate_pipeline_shaders(my_data, device, &(pPipeline->graphicsPipelineCI))) {
+ //if (!validate_pipeline_shaders(my_data, device, &(pPipeline->graphicsPipelineCI))) {
+ if (!validate_pipeline_shaders(my_data, device, pPipeline)) {
skipCall = VK_TRUE;
}
// VS is required
@@ -1406,6 +1367,7 @@
// Init the pipeline mapping info based on pipeline create info LL tree
// Threading note : Calls to this function should wrapped in mutex
+// TODO : this should really just be in the constructor for PIPELINE_NODE
static PIPELINE_NODE* initGraphicsPipeline(layer_data* dev_data, const VkGraphicsPipelineCreateInfo* pCreateInfo, PIPELINE_NODE* pBasePipeline)
{
PIPELINE_NODE* pPipeline = new PIPELINE_NODE;
@@ -1523,7 +1485,7 @@
}
pPipeline->graphicsPipelineCI.pDynamicState = &pPipeline->dynStateCI;
}
-
+ pPipeline->active_sets.clear();
return pPipeline;
}
diff --git a/layers/draw_state.h b/layers/draw_state.h
index ffc106d..c430419 100755
--- a/layers/draw_state.h
+++ b/layers/draw_state.h
@@ -91,6 +91,7 @@
DRAWSTATE_INVALID_RENDERPASS_CMD, // Invalid cmd submitted while a RenderPass is active
DRAWSTATE_NO_ACTIVE_RENDERPASS, // Rendering cmd submitted without an active RenderPass
DRAWSTATE_DESCRIPTOR_SET_NOT_UPDATED, // DescriptorSet bound but it was never updated. This is a warning code.
+ DRAWSTATE_DESCRIPTOR_SET_NOT_BOUND, // DescriptorSet used by pipeline at draw time is not bound, or has been disturbed (which would have flagged previous warning)
DRAWSTATE_INVALID_DYNAMIC_OFFSET_COUNT, // DescriptorSets bound with different number of dynamic descriptors that were included in dynamicOffsetCount
DRAWSTATE_CLEAR_CMD_BEFORE_DRAW, // Clear cmd issued before any Draw in CommandBuffer, should use RenderPass Ops instead
DRAWSTATE_BEGIN_CB_INVALID_STATE, // Primary/Secondary CB created with mismatched FB/RP information
@@ -147,6 +148,8 @@
VkComputePipelineCreateInfo computePipelineCI;
// Flag of which shader stages are active for this pipeline
uint32_t active_shaders;
+ // Capture which sets are actually used by the shaders of this pipeline
+ std::set<unsigned> active_sets;
// Vtx input info (if any)
uint32_t vtxBindingCount; // number of bindings
VkVertexInputBindingDescription* pVertexBindingDescriptions;
diff --git a/layers/vk_validation_layer_details.md b/layers/vk_validation_layer_details.md
index 27e6023..26a3bce 100644
--- a/layers/vk_validation_layer_details.md
+++ b/layers/vk_validation_layer_details.md
@@ -43,11 +43,12 @@
| Dynamic Depth Bias State Binding | Verify that depth bias dynamic state bound when depth enabled | DEPTH_BIAS_NOT_BOUND |vkCmdDraw vkCmdDrawIndexed vkCmdDrawIndirect vkCmdDrawIndexedIndirect | TODO | Verify this check and Write targeted test |
| Dynamic Blend State Binding | Verify that blend dynamic state bound when color blend enabled | BLEND_NOT_BOUND |vkCmdDraw vkCmdDrawIndexed vkCmdDrawIndirect vkCmdDrawIndexedIndirect | TODO | Verify this check and Write targeted test |
| Dynamic Depth Bounds State Binding | Verify that depth bounds dynamic state bound when depth enabled | DEPTH_BOUNDS_NOT_BOUND |vkCmdDraw vkCmdDrawIndexed vkCmdDrawIndirect vkCmdDrawIndexedIndirect | TODO | Verify this check and Write targeted test |
-| Dynamic Stencil State Binding | Verify that stencil dynamic state bound when depth enabled | STENCIL_NOT_BOUND |vkCmdDraw vkCmdDrawIndexed vkCmdDrawIndirect vkCmdDrawIndexedIndirect | TODO | Verify this check and Write targeted test |
+| Dynamic Stencil State Binding | Verify that stencil dynamic state bound when depth enabled | STENCIL_NOT_BOUND | vkCmdDraw vkCmdDrawIndexed vkCmdDrawIndirect vkCmdDrawIndexedIndirect | TODO | Verify this check and Write targeted test |
| RenderPass misuse | Tests for the following: that vkCmdDispatch, vkCmdDispatchIndirect, vkCmdCopyBuffer, vkCmdCopyImage, vkCmdBlitImage, vkCmdCopyBufferToImage, vkCmdCopyImageToBuffer, vkCmdUpdateBuffer, vkCmdFillBuffer, vkCmdClearColorImage, vkCmdClearDepthStencilImage, vkCmdResolveImage, vkCmdSetEvent, vkCmdResetEvent, vkCmdResetQueryPool, vkCmdCopyQueryPoolResults, vkCmdBeginRenderPass are not called during an active Renderpass, and that binding compute descriptor sets or pipelines does not take place during an active Renderpass | INVALID_RENDERPASS_CMD | vkCmdBindPipeline vkCmdBindDescriptorSets vkCmdDispatch vkCmdDispatchIndirect vkCmdCopyBuffer vkCmdCopyImage vkCmdBlitImage vkCmdCopyBufferToImage vkCmdCopyImageToBuffer vkCmdUpdateBuffer vkCmdFillBuffer vkCmdClearColorImage vkCmdClearDepthStencilImage vkCmdResolveImage vkCmdSetEvent vkCmdResetEvent vkCmdResetQueryPool vkCmdCopyQueryPoolResults vkCmdBeginRenderPass | RenderPassWithinRenderPass UpdateBufferWithinRenderPass ClearColorImageWithinRenderPass ClearDepthStencilImageWithinRenderPass FillBufferWithinRenderPass | NA |
| Correct use of RenderPass | Validates that the following rendering commands are issued inside an active RenderPass: vkCmdDraw, vkCmdDrawIndexed, vkCmdDrawIndirect, vkCmdDrawIndexedIndirect, vkCmdClearAttachments, vkCmdNextSubpass, vkCmdEndRenderPass | NO_ACTIVE_RENDERPASS | vkCmdBindPipeline vkCmdBindDescriptorSets vkCmdDraw vkCmdDrawIndexed vkCmdDrawIndirect vkCmdDrawIndexedIndirect vkCmdClearAttachments vkCmdNextSubpass vkCmdEndRenderPass | BindPipelineNoRenderPass ClearAttachmentsOutsideRenderPass | NA |
| Valid RenderPass | Flag error if attempt made to Begin/End/Continue a NULL or otherwise invalid RenderPass object | INVALID_RENDERPASS | vkCmdBeginRenderPass vkCmdEndRenderPass vkBeginCommandBuffer | NullRenderPass | NA |
-| DescriptorSet Updated | Warn user if DescriptorSet bound that was never updated | DESCRIPTOR_SET_NOT_UPDATED | vkCmdBindDescriptorSets | DescriptorSetNotUpdated | NA |
+| DescriptorSet Updated | Warn user if DescriptorSet bound that was never updated | DESCRIPTOR_SET_NOT_UPDATED | vkCmdDraw vkCmdDrawIndexed vkCmdDrawIndirect vkCmdDrawIndexedIndirect | DescriptorSetCompatibility | NA |
+| DescriptorSet Bound | Error if DescriptorSet not bound that is used by currently bound VkPipeline at draw time | DESCRIPTOR_SET_NOT_BOUND | vkCmdBindDescriptorSets | DescriptorSetNotUpdated | NA |
| Dynamic Offset Count | Error if dynamicOffsetCount at CmdBindDescriptorSets time is not equal to the actual number of dynamic descriptors in all sets being bound. | INVALID_DYNAMIC_OFFSET_COUNT | vkCmdBindDescriptorSets | TODO | Write a test that hits this case |
| Correct Clear Use | Warn user if CmdClear for Color or DepthStencil issued to Cmd Buffer prior to a Draw Cmd. RenderPass LOAD_OP_CLEAR is preferred in this case. | CLEAR_CMD_BEFORE_DRAW | vkCmdClearColorImage vkCmdClearDepthStencilImage | ClearCmdNoDraw | NA |
| Index Buffer Binding | Verify that an index buffer is bound at the point when an indexed draw is attempted. | INDEX_BUFFER_NOT_BOUND | vkCmdDrawIndexed vkCmdDrawIndexedIndirect | TODO | Implement validation test |