Refactor command list flushing with proper mutex locking and cleanup

Author: hypernewbie

src/vrhi_backend.cpp

@@ -25,8 +25,8 @@
 #include <komihash/komihash.h>
 
 vhCmdBackendState g_vhCmdBackendState;
+void vhCmdListFlushAll_DeviceStateLocked();
 
-// Static member definitions
 std::unordered_map< nvrhi::BindingLayoutHandle, vhBackendShader* > vhCmdBackendState::s_layoutToShader;
 vhStateResolveCache vhCmdBackendState::s_resolveCache;
 std::unordered_map< uint32_t, vhShaderReflectionResource* > vhCmdBackendState::s_slotToReflection;
@@ -1886,33 +1886,27 @@ void vhCmdBackendState::Handle_vhCmdSetStateAttachments( VIDL_vhCmdSetStateAttac
 void vhCmdBackendState::Handle_vhFlushInternal( VIDL_vhFlushInternal* cmd )
 {
     BE_CmdRAII cmdRAII( cmd );
+    std::lock_guard< std::mutex > lock( g_nvRHIStateMutex );
 
-    // TODO: Flush all transient buffer maps here.
-    {
-        std::lock_guard<std::mutex> lock( g_nvRHIStateMutex );
-        m_globalUniformBuffer.Unmap_DeviceStateLocked();
-    }
+    // Flush and step transient buffer maps here.
+    // This needs to be done *before* we flush the command lists to GPU!!
+    m_globalUniformBuffer.Unmap_DeviceStateLocked();
+    m_globalUniformBuffer.Step();
+    m_globalUniformBufferLastHash = 0;
 
+    // Send it!!
+    vhCmdListFlushAll_DeviceStateLocked();
 
     // Free all cmd memory allocations, because hitting this flush means all previous commands have been processed.
     {
-        std::lock_guard<std::mutex> lock( g_vhMemListMutex );
+        std::lock_guard< std::mutex > lock( g_vhMemListMutex );
         g_vhMemList.clear();
     }
-
-    // This uses g_nvRHIStateMutex then gives it up, we need to avoid double-locking.
-    vhCmdListFlushAll();
-
+    if ( cmd->waitForGPU )
     {
-        std::lock_guard< std::mutex > lock( g_nvRHIStateMutex );
-        if ( cmd->waitForGPU )
-        {
-            g_vhDevice->waitForIdle();
-        }
-        g_vhDevice->runGarbageCollection();
-        m_globalUniformBuffer.Step();
-        m_globalUniformBufferLastHash = 0;
+        g_vhDevice->waitForIdle();
     }
+    g_vhDevice->runGarbageCollection();
 
     // Notify caller that we're done.
     // Safety warning : fence is probably from stack of caller

src/vrhi_internal.cpp

@@ -126,14 +126,14 @@ nvrhi::CommandListHandle vhCmdListGet( nvrhi::CommandQueue type )
 // - Copy feeds Compute and Graphics
 // - Compute feeds Graphics
 //
-void vhCmdListFlush_SingleQueueInternal( nvrhi::CommandQueue type )
+void vhCmdListFlush_SingleQueueInternal_DeviceStateLocked( nvrhi::CommandQueue type )
 {
+    // WARNING: Lock g_nvRHIStateMutex before calling this.
     auto typeIdx = ( uint64_t ) type;
     uint64_t instance = 0;
 
     if ( g_vhCmdLists[typeIdx] )
     {
-        std::lock_guard<std::mutex> lock( g_nvRHIStateMutex );
         g_vhCmdLists[typeIdx]->close();
 
         // Execute and get the instance ID for synchronisation
@@ -160,20 +160,22 @@ void vhCmdListFlush_SingleQueueInternal( nvrhi::CommandQueue type )
 
 void vhCmdListFlush( nvrhi::CommandQueue type )
 {
+    std::lock_guard< std::mutex > lock( g_nvRHIStateMutex );
+
     // Both queues depend on copy; flush copy first
     if ( type == nvrhi::CommandQueue::Graphics || type == nvrhi::CommandQueue::Compute )
     {
-        vhCmdListFlush_SingleQueueInternal( nvrhi::CommandQueue::Copy );
+        vhCmdListFlush_SingleQueueInternal_DeviceStateLocked( nvrhi::CommandQueue::Copy );
     }
 
     // Graphics depends on compute; flush compute first
     if ( type == nvrhi::CommandQueue::Graphics )
     {
-        vhCmdListFlush_SingleQueueInternal( nvrhi::CommandQueue::Compute );
+        vhCmdListFlush_SingleQueueInternal_DeviceStateLocked( nvrhi::CommandQueue::Compute );
     }
 
     // Flush the requested queue
-    vhCmdListFlush_SingleQueueInternal( type );
+    vhCmdListFlush_SingleQueueInternal_DeviceStateLocked( type );
 }
 
 void vhCmdListFlushTransferIfNeeded()
@@ -186,13 +188,20 @@ void vhCmdListFlushTransferIfNeeded()
     }
 }
 
-void vhCmdListFlushAll()
+void vhCmdListFlushAll_DeviceStateLocked()
 {
+    // WARNING: Lock g_nvRHIStateMutex before calling this.
     // The order here matters slightly for efficiency ( Flush upsteam first ),
     // but the actual dependency correctness is handled by the waits inserted inside vhCmdListFlush.
-    vhCmdListFlush_SingleQueueInternal( nvrhi::CommandQueue::Copy );
-    vhCmdListFlush_SingleQueueInternal( nvrhi::CommandQueue::Compute );
-    vhCmdListFlush_SingleQueueInternal( nvrhi::CommandQueue::Graphics );
+    vhCmdListFlush_SingleQueueInternal_DeviceStateLocked( nvrhi::CommandQueue::Copy );
+    vhCmdListFlush_SingleQueueInternal_DeviceStateLocked( nvrhi::CommandQueue::Compute );
+    vhCmdListFlush_SingleQueueInternal_DeviceStateLocked( nvrhi::CommandQueue::Graphics );
+}
+
+void vhCmdListFlushAll()
+{
+    std::lock_guard< std::mutex > lock( g_nvRHIStateMutex );
+    vhCmdListFlushAll_DeviceStateLocked();
 }
 
 // Global states for user convenience.

src/vrhi_state.cpp

@@ -506,7 +506,6 @@ bool vhDebugLayoutDiffCheck( const nvrhi::BindingLayoutVector& layouts, const nv
 
 void vhWriteStateToGlobalUniform( const vhState& state, vhGlobalUniform& out )
 {
-    // Clear
     memset( &out, 0, sizeof( vhGlobalUniform ) );
 
     // Viewport / Camera