Add validation for unbound descriptor sets

Source/Base/Base/Container/DynamicArray.h

@@ -39,12 +39,9 @@ class DynamicArray
     {
         assert(_count > index);
 
-        for (int i = index; index < _count; i++)
+        for (size_t i = index; i + 1 < _count; i++)
         {
-            if (i + 1 < _count)
-            {
-                _data[i] = _data[i + 1];
-            }
+            _data[i] = _data[i + 1];
         }
 
         _count--;

Source/Renderer/Renderer/Renderers/Vulkan/Backend/CommandListHandlerVK.cpp

@@ -31,6 +31,7 @@ namespace Renderer
 
             i8 renderPassOpenCount = 0;
             i8 pipelineOpenCount = 0;
+            u8 unboundDescriptorSetsMask = 0; // bits set = descriptor set slots required by the bound pipeline that haven't been bound on this command list yet
             QueueType queueType = QueueType::Graphics;
         };
 
@@ -321,6 +322,24 @@ namespace Renderer
             commandList.pipelineOpenCount = count;
         }
 
+        u8 CommandListHandlerVK::GetUnboundDescriptorSets(CommandListID id)
+        {
+            CommandListHandlerVKData& data = static_cast<CommandListHandlerVKData&>(*_data);
+
+            CommandList& commandList = data.commandLists[static_cast<CommandListID::type>(id)];
+
+            return commandList.unboundDescriptorSetsMask;
+        }
+
+        void CommandListHandlerVK::SetUnboundDescriptorSets(CommandListID id, u8 mask)
+        {
+            CommandListHandlerVKData& data = static_cast<CommandListHandlerVKData&>(*_data);
+
+            CommandList& commandList = data.commandLists[static_cast<CommandListID::type>(id)];
+
+            commandList.unboundDescriptorSetsMask = mask;
+        }
+
         tracy::VkCtxManualScope*& CommandListHandlerVK::GetTracyScope(CommandListID id)
         {
             CommandListHandlerVKData& data = static_cast<CommandListHandlerVKData&>(*_data);

Source/Renderer/Renderer/Renderers/Vulkan/Backend/CommandListHandlerVK.h

@@ -56,6 +56,11 @@ namespace Renderer
             i8 GetPipelineOpenCount(CommandListID id);
             void SetPipelineOpenCount(CommandListID id, i8 count);
 
+            // Bitmask of descriptor set slots the currently bound pipeline statically uses but which haven't been bound on this command list yet.
+            // Initialized in BeginPipeline from the pipeline's used-set mask, cleared bit-by-bit by BindDescriptorSet, validated by Draw / Dispatch, reset on EndPipeline.
+            u8 GetUnboundDescriptorSets(CommandListID id);
+            void SetUnboundDescriptorSets(CommandListID id, u8 mask);
+
             tracy::VkCtxManualScope*& GetTracyScope(CommandListID id);
 
             VkFence GetCurrentFence();

Source/Renderer/Renderer/Renderers/Vulkan/Backend/PipelineHandlerVK.cpp

@@ -43,6 +43,7 @@ namespace Renderer
 
             std::unordered_set<u32> setsUsed;
             std::unordered_map<u32, PersistentBitSet> usedBindingsPerSlot;
+            u8 usedDescriptorSetMask = 0; // bit `slot` set for each statically-used descriptor set slot (including DEBUG)
         };
 
         struct ComputePipelineCacheDesc
@@ -65,6 +66,7 @@ namespace Renderer
 
             std::unordered_set<u32> setsUsed;
             std::unordered_map<u32, PersistentBitSet> usedBindingsPerSlot;
+            u8 usedDescriptorSetMask = 0; // bit `slot` set for each statically-used descriptor set slot (including DEBUG)
         };
 
         struct PipelineHandlerVKData : IPipelineHandlerVKData
@@ -282,6 +284,18 @@ namespace Renderer
             return static_cast<u32>(data.computePipelines[static_cast<cIDType>(id)].setsUsed.contains(setNumber));
         }
 
+        u8 PipelineHandlerVK::GetUsedDescriptorSetMask(GraphicsPipelineID id)
+        {
+            PipelineHandlerVKData& data = static_cast<PipelineHandlerVKData&>(*_data);
+            return data.graphicsPipelines[static_cast<gIDType>(id)].usedDescriptorSetMask;
+        }
+
+        u8 PipelineHandlerVK::GetUsedDescriptorSetMask(ComputePipelineID id)
+        {
+            PipelineHandlerVKData& data = static_cast<PipelineHandlerVKData&>(*_data);
+            return data.computePipelines[static_cast<cIDType>(id)].usedDescriptorSetMask;
+        }
+
         const PersistentBitSet* PipelineHandlerVK::GetUsedBindings(GraphicsPipelineID id, u32 slot)
         {
             PipelineHandlerVKData& data = static_cast<PipelineHandlerVKData&>(*_data);
@@ -476,6 +490,15 @@ namespace Renderer
                 bindInfoPushConstants.insert(bindInfoPushConstants.end(), bindReflection.pushConstants.begin(), bindReflection.pushConstants.end());
             }
 
+            // Build the used-set bitmask from reflection. DEBUG is included: if a shader actively uses the DEBUG set we want
+            // the draw validator to flag a missing bind. The asymmetric "binding DEBUG that the pipeline doesn't use" case
+            // is still tolerated because BindDescriptorSet silently early-returns for DEBUG when the pipeline doesn't use it.
+            pipeline.usedDescriptorSetMask = 0;
+            for (u32 slot : pipeline.setsUsed)
+            {
+                pipeline.usedDescriptorSetMask |= static_cast<u8>(1u << slot);
+            }
+
             // -- Create Descriptor Set Layout from reflected SPIR-V --
             for (BindInfo& bindInfo : bindInfos)
             {
@@ -822,6 +845,15 @@ namespace Renderer
                 pipeline.usedBindingsPerSlot[bindInfo.set].Set(bindInfo.binding);
             }
 
+            // Build the used-set bitmask from reflection. DEBUG is included: if a shader actively uses the DEBUG set we want
+            // the draw validator to flag a missing bind. The asymmetric "binding DEBUG that the pipeline doesn't use" case
+            // is still tolerated because BindDescriptorSet silently early-returns for DEBUG when the pipeline doesn't use it.
+            pipeline.usedDescriptorSetMask = 0;
+            for (u32 slot : pipeline.setsUsed)
+            {
+                pipeline.usedDescriptorSetMask |= static_cast<u8>(1u << slot);
+            }
+
             std::vector<BindInfo> bindInfos;
             std::vector<BindInfoPushConstant> bindInfoPushConstants;
             const BindReflection& bindReflection = _shaderHandler->GetFullBindReflection(desc.computeShader);

Source/Renderer/Renderer/Renderers/Vulkan/Backend/PipelineHandlerVK.h

@@ -74,6 +74,10 @@ namespace Renderer
             bool UsesDescriptorSet(GraphicsPipelineID id, u32 setNumber);
             bool UsesDescriptorSet(ComputePipelineID id, u32 setNumber);
 
+            // Returns a bitmask where bit `slot` is set for each descriptor set slot the pipeline statically uses (including DEBUG).
+            u8 GetUsedDescriptorSetMask(GraphicsPipelineID id);
+            u8 GetUsedDescriptorSetMask(ComputePipelineID id);
+
             const PersistentBitSet* GetUsedBindings(GraphicsPipelineID id, u32 slot);
             const PersistentBitSet* GetUsedBindings(ComputePipelineID id, u32 slot);
 

Source/Renderer/Renderer/Renderers/Vulkan/RendererVK.cpp

@@ -601,6 +601,39 @@ namespace Renderer
         _device->TransitionImageLayout(commandBuffer, image, range.aspectMask, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL, 1, 1);
     }
 
+    namespace
+    {
+        const char* DescriptorSetSlotToString(u32 slot)
+        {
+            switch (slot)
+            {
+                case DescriptorSetSlot::DEBUG:    return "DEBUG";
+                case DescriptorSetSlot::GLOBAL:   return "GLOBAL";
+                case DescriptorSetSlot::LIGHT:    return "LIGHT";
+                case DescriptorSetSlot::TERRAIN:  return "TERRAIN";
+                case DescriptorSetSlot::MODEL:    return "MODEL";
+                case DescriptorSetSlot::PER_PASS: return "PER_PASS";
+                case DescriptorSetSlot::PER_DRAW: return "PER_DRAW";
+                default:                          return "UNKNOWN";
+            }
+        }
+    }
+
+    void RendererVK::ValidateBoundDescriptorSets(CommandListID commandListID, const char* opName)
+    {
+        u8 mask = _commandListHandler->GetUnboundDescriptorSets(commandListID);
+        if (mask == 0)
+            return;
+
+        for (u32 slot = 0; slot < 8; slot++)
+        {
+            if ((mask & (1u << slot)) == 0)
+                continue;
+
+            NC_LOG_CRITICAL("{} called with descriptor set slot {} ({}) statically used by the bound pipeline but never bound on this command list", opName, slot, DescriptorSetSlotToString(slot));
+        }
+    }
+
     void RendererVK::Draw(CommandListID commandListID, u32 numVertices, u32 numInstances, u32 vertexOffset, u32 instanceOffset)
     {
         VkCommandBuffer commandBuffer = _commandListHandler->GetCommandBuffer(commandListID);
@@ -609,6 +642,7 @@ namespace Renderer
         {
             NC_LOG_CRITICAL("You tried to draw without first calling BeginPipeline!");
         }
+        ValidateBoundDescriptorSets(commandListID, "vkCmdDraw");
 
         vkCmdDraw(commandBuffer, numVertices, numInstances, vertexOffset, instanceOffset);
     }
@@ -621,6 +655,7 @@ namespace Renderer
         {
             NC_LOG_CRITICAL("You tried to draw without first calling BeginPipeline!");
         }
+        ValidateBoundDescriptorSets(commandListID, "vkCmdDrawIndirect");
 
         VkBuffer vkArgumentBuffer = _bufferHandler->GetBuffer(argumentBuffer);
 
@@ -635,6 +670,7 @@ namespace Renderer
         {
             NC_LOG_CRITICAL("You tried to draw without first calling BeginPipeline!");
         }
+        ValidateBoundDescriptorSets(commandListID, "vkCmdDrawIndirectCount");
 
         VkBuffer vkArgumentBuffer = _bufferHandler->GetBuffer(argumentBuffer);
         VkBuffer vkDrawCountBuffer = _bufferHandler->GetBuffer(drawCountBuffer);
@@ -650,6 +686,7 @@ namespace Renderer
         {
             NC_LOG_CRITICAL("You tried to draw without first calling BeginPipeline!");
         }
+        ValidateBoundDescriptorSets(commandListID, "vkCmdDrawIndexed");
 
         vkCmdDrawIndexed(commandBuffer, numIndices, numInstances, indexOffset, vertexOffset, instanceOffset);
     }
@@ -662,6 +699,7 @@ namespace Renderer
         {
             NC_LOG_CRITICAL("You tried to draw without first calling BeginPipeline!");
         }
+        ValidateBoundDescriptorSets(commandListID, "vkCmdDrawIndexedIndirect");
 
         VkBuffer vkArgumentBuffer = _bufferHandler->GetBuffer(argumentBuffer);
 
@@ -676,6 +714,7 @@ namespace Renderer
         {
             NC_LOG_CRITICAL("You tried to draw without first calling BeginPipeline!");
         }
+        ValidateBoundDescriptorSets(commandListID, "vkCmdDrawIndexedIndirectCount");
 
         VkBuffer vkArgumentBuffer = _bufferHandler->GetBuffer(argumentBuffer);
         VkBuffer vkDrawCountBuffer = _bufferHandler->GetBuffer(drawCountBuffer);
@@ -687,6 +726,8 @@ namespace Renderer
     {
         VkCommandBuffer commandBuffer = _commandListHandler->GetCommandBuffer(commandListID);
 
+        ValidateBoundDescriptorSets(commandListID, "vkCmdDispatch");
+
         vkCmdDispatch(commandBuffer, threadGroupCountX, threadGroupCountY, threadGroupCountZ);
     }
 
@@ -695,6 +736,8 @@ namespace Renderer
         VkCommandBuffer commandBuffer = _commandListHandler->GetCommandBuffer(commandListID);
         VkBuffer vkArgumentBuffer = _bufferHandler->GetBuffer(argumentBuffer);
 
+        ValidateBoundDescriptorSets(commandListID, "vkCmdDispatchIndirect");
+
         vkCmdDispatchIndirect(commandBuffer, vkArgumentBuffer, argumentBufferOffset);
     }
 
@@ -1174,6 +1217,7 @@ namespace Renderer
         vkCmdBindPipeline(commandBuffer, VK_PIPELINE_BIND_POINT_GRAPHICS, pipeline);
 
         _commandListHandler->SetBoundGraphicsPipeline(commandListID, pipelineID);
+        _commandListHandler->SetUnboundDescriptorSets(commandListID, _pipelineHandler->GetUsedDescriptorSetMask(pipelineID));
     }
 
     void RendererVK::EndPipeline(CommandListID commandListID, GraphicsPipelineID pipelineID)
@@ -1194,6 +1238,7 @@ namespace Renderer
 
         VkCommandBuffer commandBuffer = _commandListHandler->GetCommandBuffer(commandListID);
         _commandListHandler->SetBoundGraphicsPipeline(commandListID, GraphicsPipelineID::Invalid());
+        _commandListHandler->SetUnboundDescriptorSets(commandListID, 0);
     }
 
     void RendererVK::BeginPipeline(CommandListID commandListID, ComputePipelineID pipelineID)
@@ -1217,6 +1262,7 @@ namespace Renderer
         vkCmdBindPipeline(commandBuffer, VK_PIPELINE_BIND_POINT_COMPUTE, pipeline);
 
         _commandListHandler->SetBoundComputePipeline(commandListID, pipelineID);
+        _commandListHandler->SetUnboundDescriptorSets(commandListID, _pipelineHandler->GetUsedDescriptorSetMask(pipelineID));
     }
 
     void RendererVK::EndPipeline(CommandListID commandListID, ComputePipelineID pipelineID)
@@ -1236,6 +1282,7 @@ namespace Renderer
         vkCmdBindPipeline(commandBuffer, VK_PIPELINE_BIND_POINT_COMPUTE, pipeline);
 
         _commandListHandler->SetBoundComputePipeline(commandListID, ComputePipelineID::Invalid());
+        _commandListHandler->SetUnboundDescriptorSets(commandListID, 0);
     }
 
     void RendererVK::BeginTimeQuery(CommandListID commandListID, TimeQueryID timeQueryID)
@@ -1420,6 +1467,11 @@ namespace Renderer
 
             // Bind descriptor set
             vkCmdBindDescriptorSets(commandBuffer, VK_PIPELINE_BIND_POINT_GRAPHICS, pipelineLayout, slot, 1, &vkDescriptorSet, 0, nullptr);
+
+            // Clear this slot from the "still required to be bound" mask
+            u8 mask = _commandListHandler->GetUnboundDescriptorSets(commandListID);
+            mask &= ~static_cast<u8>(1u << slot);
+            _commandListHandler->SetUnboundDescriptorSets(commandListID, mask);
         }
         else if (computePipelineID != ComputePipelineID::Invalid())
         {
@@ -1438,6 +1490,11 @@ namespace Renderer
 
             // Bind descriptor set
             vkCmdBindDescriptorSets(commandBuffer, VK_PIPELINE_BIND_POINT_COMPUTE, pipelineLayout, slot, 1, &vkDescriptorSet, 0, nullptr);
+
+            // Clear this slot from the "still required to be bound" mask
+            u8 mask = _commandListHandler->GetUnboundDescriptorSets(commandListID);
+            mask &= ~static_cast<u8>(1u << slot);
+            _commandListHandler->SetUnboundDescriptorSets(commandListID, mask);
         }
     }
 

Source/Renderer/Renderer/Renderers/Vulkan/RendererVK.h

@@ -241,6 +241,9 @@ namespace Renderer
         void RecreateSwapChain(Backend::SwapChainVK* swapChain);
         void CreateDummyPipeline();
 
+        // Logs an NC_LOG_CRITICAL for every descriptor set slot the bound pipeline statically uses but which has not been bound on this command list. Called from every Draw* / Dispatch* variant.
+        void ValidateBoundDescriptorSets(CommandListID commandListID, const char* opName);
+
     private:
         Backend::RenderDeviceVK* _device = nullptr;
         Backend::BufferHandlerVK* _bufferHandler = nullptr;