[Vulkan] Fixed a warning related to vkGetQueryPoolResults which came …

…from a true logic error, this fixes some inaccurate render pass GPU times (would manifest as flactuating values)
ApostolosBouz · Nov 29, 2023 · 160f539 · 160f539
1 parent 3e13c99
commit 160f539
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 36 deletions.
diff --git a/data/shaders/screen_space_shadows/bend_sss_gpu.hlsl b/data/shaders/screen_space_shadows/bend_sss_gpu.hlsl
@@ -467,10 +467,10 @@ void WriteScreenSpaceShadow(struct DispatchParameters inParameters, int3 inGroup
 		// If the first samples are always producing a hard shadow, then compute this value separately.
 		result = min(hard_shadow, result);
 
-        //= PANOS - SCREEN EDGE FADE ============================
+        //= PANOS - SCREEN EDGE FADE ===========================
         float2 uv = write_xy * inParameters.InvDepthTextureSize;
         result    = saturate(result + 1.0 - screen_fade(uv));
-        //=======================================================
+        //======================================================
 
 		//write the result
 		{

diff --git a/runtime/RHI/RHI_CommandList.h b/runtime/RHI/RHI_CommandList.h
@@ -148,21 +148,21 @@ namespace Spartan
         void BeginRenderPass();
         void EndRenderPass();
 
-        // Sync
+        // sync
         std::shared_ptr<RHI_Fence> m_proccessed_fence;
         std::shared_ptr<RHI_Semaphore> m_proccessed_semaphore;
 
-        // Profiling
+        // profiling
         const char* m_timeblock_active         = nullptr;
         uint32_t m_timestamp_index             = 0;
         static const uint32_t m_max_timestamps = 512;
         std::array<uint64_t, m_max_timestamps> m_timestamps;
 
-        // Variables to minimise state changes
+        // variables to minimise state changes
         uint64_t m_vertex_buffer_id = 0;
         uint64_t m_index_buffer_id  = 0;
 
-        // Misc
+        // misc
         RHI_Pipeline* m_pipeline                             = nullptr;
         bool m_render_pass_active                            = false;
         bool m_pipeline_dirty                                = false;
@@ -174,7 +174,7 @@ namespace Spartan
         std::mutex m_mutex_reset;
         RHI_PipelineState m_pso;
 
-        // RHI Resources
+        // rhi resources
         void* m_rhi_resource          = nullptr;
         void* m_rhi_cmd_pool_resource = nullptr;
         void* m_rhi_query_pool        = nullptr;

diff --git a/runtime/RHI/Vulkan/Vulkan_CommandList.cpp b/runtime/RHI/Vulkan/Vulkan_CommandList.cpp
@@ -313,30 +313,24 @@ namespace Spartan
         SP_ASSERT(m_state == RHI_CommandListState::Idle);
 
         // get queries
-        if (m_queue_type != RHI_Queue_Type::Copy)
+        if (RHI_Context::gpu_profiling && m_queue_type != RHI_Queue_Type::Copy)
         {
-            if (RHI_Context::gpu_profiling)
+            if (m_timestamp_index != 0)
             {
-                if (m_rhi_query_pool)
-                {
-                    if (m_timestamp_index != 0)
-                    {
-                        const uint32_t query_count     = m_timestamp_index * 2;
-                        const size_t stride            = sizeof(uint64_t);
-                        const VkQueryResultFlags flags = VK_QUERY_RESULT_64_BIT;
-
-                        vkGetQueryPoolResults(
-                            RHI_Context::device,                    // device
-                            static_cast<VkQueryPool>(m_rhi_query_pool), // queryPool
-                            0,                                      // firstQuery
-                            query_count,                            // queryCount
-                            query_count * stride,                   // dataSize
-                            m_timestamps.data(),                    // pData
-                            stride,                                 // stride
-                            flags                                   // flags
-                        );
-                    }
-                }
+                const uint32_t query_count     = m_timestamp_index;
+                const size_t stride            = sizeof(uint64_t);
+                const VkQueryResultFlags flags = VK_QUERY_RESULT_64_BIT;
+
+                vkGetQueryPoolResults(
+                    RHI_Context::device,                        // device
+                    static_cast<VkQueryPool>(m_rhi_query_pool), // queryPool
+                    0,                                          // firstQuery
+                    query_count,                                // queryCount
+                    query_count * stride,                       // dataSize
+                    m_timestamps.data(),                        // pData
+                    stride,                                     // stride
+                    flags                                       // flags
+                );
             }
 
             m_timestamp_index = 0;
@@ -348,8 +342,8 @@ namespace Spartan
         begin_info.flags                    = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT;
         SP_ASSERT_MSG(vkBeginCommandBuffer(static_cast<VkCommandBuffer>(m_rhi_resource), &begin_info) == VK_SUCCESS, "Failed to begin command buffer");
 
-        // Reset query pool - Has to be done after vkBeginCommandBuffer or a VK_DEVICE_LOST will occur
-        if (m_queue_type != RHI_Queue_Type::Copy)
+        // reset query pool - has to be done after vkBeginCommandBuffer or a VK_DEVICE_LOST will occur
+        if (RHI_Context::gpu_profiling && m_queue_type != RHI_Queue_Type::Copy)
         {
             vkCmdResetQueryPool(static_cast<VkCommandBuffer>(m_rhi_resource), static_cast<VkQueryPool>(m_rhi_query_pool), 0, m_max_timestamps);
         }
@@ -1243,11 +1237,9 @@ namespace Spartan
     {
         SP_ASSERT(m_state == RHI_CommandListState::Recording);
         SP_ASSERT(RHI_Context::gpu_profiling);
-        SP_ASSERT(m_rhi_query_pool != nullptr);
 
         uint32_t timestamp_index = m_timestamp_index;
-        vkCmdWriteTimestamp(static_cast<VkCommandBuffer>(m_rhi_resource), VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, static_cast<VkQueryPool>(m_rhi_query_pool), timestamp_index);
-        m_timestamp_index++;
+        vkCmdWriteTimestamp(static_cast<VkCommandBuffer>(m_rhi_resource), VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, static_cast<VkQueryPool>(m_rhi_query_pool), m_timestamp_index++);
 
         return timestamp_index;
     }
@@ -1256,7 +1248,6 @@ namespace Spartan
     {
         SP_ASSERT(m_state == RHI_CommandListState::Recording);
         SP_ASSERT(RHI_Context::gpu_profiling);
-        SP_ASSERT(m_rhi_query_pool != nullptr);
 
         vkCmdWriteTimestamp(static_cast<VkCommandBuffer>(m_rhi_resource), VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, static_cast<VkQueryPool>(m_rhi_query_pool), m_timestamp_index++);
     }

diff --git a/runtime/Rendering/Renderer_Passes.cpp b/runtime/Rendering/Renderer_Passes.cpp
@@ -829,7 +829,7 @@ namespace Spartan
                     if (light->GetLightType() == LightType::Directional)
                     {
                         // TODO: Why do we need to flip sign?
-                        p = Vector4(-light->GetEntity()->GetForward().Normalized(), 0.0f) * view_projection;
+                        p = Vector4(-light->GetEntity()->GetForward(), 0.0f) * view_projection;
                     }
                     else
                     {