gpuav: Handle case where indirect cmd stride is 0

KhronosGroup · Dec 17, 2024 · bebbbba · bebbbba
1 parent ac59f25
commit bebbbba
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 9 deletions.
diff --git a/layers/gpu/cmd_validation/gpuav_draw.cpp b/layers/gpu/cmd_validation/gpuav_draw.cpp
@@ -229,17 +229,17 @@ struct FirstInstanceValidationShader {
 
 void FirstInstance(Validator &gpuav, CommandBuffer &cb_state, const Location &loc, VkBuffer draw_buffer,
                    VkDeviceSize draw_buffer_offset, uint32_t draw_cmds_byte_stride, vvl::Struct draw_indirect_struct_name,
-                   uint32_t first_instance_member_pos, uint32_t draws_count, VkBuffer count_buffer,
-                   VkDeviceSize count_buffer_offset, const char *vuid) {
+                   uint32_t first_instance_member_pos, uint32_t draw_count, VkBuffer count_buffer, VkDeviceSize count_buffer_offset,
+                   const char *vuid) {
     if (!gpuav.gpuav_settings.validate_indirect_draws_buffers) {
         return;
     }
 
     if (gpuav.enabled_features.drawIndirectFirstInstance) return;
 
     CommandBuffer::ValidationCommandFunc validation_cmd = [draw_buffer, draw_buffer_offset, draw_cmds_byte_stride,
-                                                           first_instance_member_pos, draws_count, count_buffer,
-                                                           count_buffer_offset, draw_i = cb_state.draw_index,
+                                                           first_instance_member_pos, draw_count, count_buffer, count_buffer_offset,
+                                                           draw_i = cb_state.draw_index,
                                                            error_logger_i = uint32_t(cb_state.per_command_error_loggers.size()),
                                                            loc](Validator &gpuav, CommandBuffer &cb_state) {
         SharedDrawValidationResources &shared_draw_validation_resources =
@@ -285,13 +285,18 @@ void FirstInstance(Validator &gpuav, CommandBuffer &cb_state, const Location &lo
 
             uint32_t max_held_draw_cmds = 0;
             if (draw_buffer_state->create_info.size > draw_buffer_offset) {
-                max_held_draw_cmds =
-                    static_cast<uint32_t>((draw_buffer_state->create_info.size - draw_buffer_offset) / draw_cmds_byte_stride);
+                // If drawCount is less than or equal to one, stride is ignored
+                if (draw_count > 1) {
+                    max_held_draw_cmds =
+                        static_cast<uint32_t>((draw_buffer_state->create_info.size - draw_buffer_offset) / draw_cmds_byte_stride);
+                } else {
+                    max_held_draw_cmds = 1;
+                }
             }
             // It is assumed that the number of draws to validate is fairly low.
             // Otherwise might reconsider having a warp dimension of (1, 1, 1)
             // Maybe another reason to add telemetry?
-            const uint32_t work_group_count = std::min(draws_count, max_held_draw_cmds);
+            const uint32_t work_group_count = std::min(draw_count, max_held_draw_cmds);
 
             if (work_group_count == 0) {
                 return;
@@ -658,7 +663,7 @@ void DrawMeshIndirect(Validator &gpuav, CommandBuffer &cb_state, const Location
                 uint32_t max_held_draw_cmds = 0;
                 if (draw_buffer_full_size > draw_buffer_offset) {
                     // If drawCount is less than or equal to one, stride is ignored
-                    if (draw_cmds_byte_stride > 0) {
+                    if (draw_count > 1) {
                         max_held_draw_cmds =
                             static_cast<uint32_t>((draw_buffer_full_size - draw_buffer_offset) / draw_cmds_byte_stride);
                     } else {

diff --git a/tests/unit/gpu_av_index_buffer_positive.cpp b/tests/unit/gpu_av_index_buffer_positive.cpp
@@ -43,7 +43,7 @@ TEST_F(PositiveGpuAVIndexBuffer, BadVertexIndex) {
     vkt::Buffer index_buffer = vkt::IndexBuffer<uint32_t>(*m_device, {0, std::numeric_limits<uint32_t>::max(), 42});
 
     vk::CmdBindIndexBuffer(m_command_buffer.handle(), index_buffer.handle(), 0, VK_INDEX_TYPE_UINT32);
-    vk::CmdDrawIndexedIndirect(m_command_buffer.handle(), draw_params_buffer.handle(), 0, 1, sizeof(VkDrawIndexedIndirectCommand));
+    vk::CmdDrawIndexedIndirect(m_command_buffer.handle(), draw_params_buffer.handle(), 0, 1, 0);
     m_command_buffer.EndRenderPass();
     m_command_buffer.end();
     m_default_queue->Submit(m_command_buffer);

diff --git a/tests/unit/gpu_av_indirect_buffer_positive.cpp b/tests/unit/gpu_av_indirect_buffer_positive.cpp
@@ -240,3 +240,37 @@ TEST_F(PositiveGpuAVIndirectBuffer, MeshSingleCommand) {
     m_default_queue->Submit(m_command_buffer);
     m_default_queue->Wait();
 }
+
+TEST_F(PositiveGpuAVIndirectBuffer, FirstInstanceSingleDrawIndirectCommand) {
+    TEST_DESCRIPTION("Validate illegal firstInstance values");
+    AddRequiredFeature(vkt::Feature::multiDrawIndirect);
+    // silence MacOS issue
+    RETURN_IF_SKIP(InitGpuAvFramework());
+
+    RETURN_IF_SKIP(InitState());
+    InitRenderTarget();
+
+    CreatePipelineHelper pipe(*this);
+    pipe.rs_state_ci_.lineWidth = 1.0f;
+    pipe.CreateGraphicsPipeline();
+
+    VkDrawIndirectCommand draw_params{};
+    draw_params.vertexCount = 3;
+    draw_params.instanceCount = 1;
+    draw_params.firstVertex = 0;
+    draw_params.firstInstance = 0;
+    vkt::Buffer draw_params_buffer = vkt::IndirectBuffer<VkDrawIndirectCommand>(*m_device, {draw_params});
+
+    VkCommandBufferBeginInfo begin_info = vku::InitStructHelper();
+    m_command_buffer.Begin(&begin_info);
+    m_command_buffer.BeginRenderPass(m_renderPassBeginInfo);
+    vk::CmdBindPipeline(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_GRAPHICS, pipe.Handle());
+
+    vk::CmdDrawIndirect(m_command_buffer.handle(), draw_params_buffer.handle(), 0, 1, 0);
+
+    m_command_buffer.EndRenderPass();
+    m_command_buffer.end();
+
+    m_default_queue->Submit(m_command_buffer);
+    m_default_queue->Wait();
+}