Fix incorrect resource release from DynamicUploadHeap when using QueueID::Both

Author: ArtemVetik

Modules/Graphics.Heaps/DynamicUploadHeap.cpp

@@ -30,9 +30,10 @@ namespace EduEngine
         return dynAlloc;
 	}
 
-    void DynamicUploadHeap::FinishFrame(uint64_t fenceValue, uint64_t lastCompletedFenceValue)
+    void DynamicUploadHeap::FinishFrame(FenceValues fenceValue, FenceValues lastCompletedFenceValue)
     {
         size_t numBuffsToDelete = 0;
+
         for (size_t ind = 0; ind < m_RingBuffers.size(); ++ind)
         {
             auto& ringBuff = m_RingBuffers[ind];

Modules/Graphics.Heaps/DynamicUploadHeap.h

@@ -19,7 +19,7 @@ namespace EduEngine
 
         DynamicAllocation Allocate(size_t sizeInBytes, size_t alignment = DEFAULT_ALIGN);
 
-        void FinishFrame(uint64_t fenceValue, uint64_t lastCompletedFenceValue);
+        void FinishFrame(FenceValues fenceValue, FenceValues lastCompletedFenceValue);
 
     private:
         const bool m_bIsCPUAccessible;

Modules/Graphics.Heaps/QueueID.h

@@ -9,4 +9,18 @@ namespace EduEngine
 		Compute = 1,
 		Both = 2,
 	};
+
+	struct GRAPHICS_HEAPS_API FenceValues
+	{
+		uint64_t DirectFence;
+		uint64_t ComputeFence;
+
+		bool operator<=(const FenceValues& other) const {
+			return (DirectFence <= other.DirectFence) && (ComputeFence <= other.ComputeFence);
+		}
+
+		bool operator<(const FenceValues& other) const {
+			return (DirectFence < other.DirectFence) && (ComputeFence <= other.ComputeFence);
+		}
+	};
 }

Modules/Graphics.Heaps/RingBuffer.cpp

@@ -93,7 +93,7 @@ namespace EduEngine
 		return InvalidOffset;
 	}
 
-	void RingBuffer::FinishCurrentFrame(uint64_t fenceValue)
+	void RingBuffer::FinishCurrentFrame(FenceValues fenceValue)
 	{
 		if (m_CurrFrameSize != 0)
 		{
@@ -102,7 +102,7 @@ namespace EduEngine
 		}
 	}
 
-	void RingBuffer::ReleaseCompletedFrames(uint64_t completedFenceValue)
+	void RingBuffer::ReleaseCompletedFrames(FenceValues completedFenceValue)
 	{
 		// We can release all tails whose associated fence value is less than or equal to CompletedFenceValue
 		while (!m_CompletedFrameTails.empty() && m_CompletedFrameTails.front().FenceValue <= completedFenceValue)

Modules/Graphics.Heaps/RingBuffer.h

@@ -1,5 +1,6 @@
 #pragma once
 #include "pch.h"
+#include "QueueID.h"
 
 namespace EduEngine
 {
@@ -8,14 +9,14 @@ namespace EduEngine
 	public:
 		struct GRAPHICS_HEAPS_API FrameTailAttribs
 		{
-			FrameTailAttribs(uint64_t fv, size_t off, size_t sz) :
+			FrameTailAttribs(FenceValues fv, size_t off, size_t sz) :
 				FenceValue(fv),
 				Offset(off),
 				Size(sz)
 			{}
 			// Fence value associated with the command list in which 
 			// the allocation could have been referenced last time
-			uint64_t FenceValue;
+			FenceValues FenceValue;
 			size_t Offset;
 			size_t Size;
 		};
@@ -32,8 +33,8 @@ namespace EduEngine
 
 		size_t Allocate(size_t size);
 
-		void FinishCurrentFrame(uint64_t fenceValue);
-		void ReleaseCompletedFrames(uint64_t completedFenceValue);
+		void FinishCurrentFrame(FenceValues fenceValue);
+		void ReleaseCompletedFrames(FenceValues completedFenceValue);
 
 		size_t GetMaxSize()const { return m_MaxSize; }
 		bool IsFull()const { return m_UsedSize == m_MaxSize; };

Modules/Graphics/BufferD3D12.cpp

@@ -63,11 +63,11 @@ namespace EduEngine
 
 		memcpy(reinterpret_cast<char*>(uploadBuff.CPUAddress), data, uploadBufferSize);
 
-		auto& cmdContext = m_QueueId == QueueID::Compute
+		auto& cmdContext = m_QueueId != QueueID::Direct
 			? m_Device->GetCommandContext(D3D12_COMMAND_LIST_TYPE_COMPUTE)
 			: m_Device->GetCommandContext(D3D12_COMMAND_LIST_TYPE_DIRECT);
 
-		auto beforeState = m_QueueId == QueueID::Compute ? D3D12_RESOURCE_STATE_NON_PIXEL_SHADER_RESOURCE : D3D12_RESOURCE_STATE_GENERIC_READ;
+		auto beforeState = m_QueueId != QueueID::Direct ? D3D12_RESOURCE_STATE_NON_PIXEL_SHADER_RESOURCE : D3D12_RESOURCE_STATE_GENERIC_READ;
 
 		cmdContext.ResourceBarrier(CD3DX12_RESOURCE_BARRIER::Transition(m_d3d12Resource.Get(),
 			beforeState, D3D12_RESOURCE_STATE_COPY_DEST));

Modules/Graphics/CommandQueueD3D12.cpp

@@ -71,7 +71,8 @@ namespace EduEngine
 				break;
 		}
 
-		m_DynUploadHeap->FinishFrame(m_NextCmdList.load(), numCompletedCmdLists);
+		auto nextCmdList = m_NextCmdList.load();
+		m_DynUploadHeap->FinishFrame({ nextCmdList , nextCmdList }, { numCompletedCmdLists , numCompletedCmdLists });
 	}
 
 	void CommandQueueD3D12::Flush()

Modules/Graphics/RenderDeviceD3D12.cpp

@@ -34,7 +34,7 @@ namespace EduEngine
 			{ m_GPUDescriptorHeaps[0], 2048, "CBV_SRV_UAV_DynSuballocationMgr" },
 			{ m_GPUDescriptorHeaps[1], 2048, "SAMPLER_DynSuballocationMgr" }
 		},
-		m_DynUploadHeap { true, this, 2048 }
+		m_DynUploadHeap{ true, this, 2048 }
 	{
 	}
 
@@ -114,14 +114,13 @@ namespace EduEngine
 		auto numDirectNextCmdLists = m_CommandQueues[0].GetNextCmdListNum();
 		auto numComputeNextCmdLists = m_CommandQueues[1].GetNextCmdListNum();
 
-		auto minCompletedCmdList = std::min(numDirectCompletedCmdLists, numComputeCompletedCmdLists);
-		auto maxNextCmdList = std::max(numDirectNextCmdLists, numComputeNextCmdLists);
+		FenceValues completedFences = { numDirectCompletedCmdLists, numComputeCompletedCmdLists };
 
 		while (!m_ReleaseObjectsQueue.empty())
 		{
 			auto& firstObj = m_ReleaseObjectsQueue.front();
 			// GPU must have been idled when ForceRelease == true 
-			if (firstObj.first < minCompletedCmdList || forceRelease)
+			if (firstObj.first < completedFences || forceRelease)
 				m_ReleaseObjectsQueue.pop_front();
 			else
 				break;
@@ -130,7 +129,7 @@ namespace EduEngine
 		for (size_t i = 0; i < 2; i++)
 			m_DynamicSuballocationMgr[i].DiscardAllocations();
 
-		m_DynUploadHeap.FinishFrame(maxNextCmdList, minCompletedCmdList);
+		m_DynUploadHeap.FinishFrame({ numDirectNextCmdLists, numComputeNextCmdLists }, { numDirectCompletedCmdLists, numComputeCompletedCmdLists });
 	}
 
 	void RenderDeviceD3D12::FlushQueues()
@@ -144,8 +143,6 @@ namespace EduEngine
 		uint64_t directNum = m_CommandQueues[0].GetNextCmdListNum();
 		uint64_t computeNum = m_CommandQueues[1].GetNextCmdListNum();
 
-		uint64_t max = directNum > computeNum ? directNum : computeNum;
-
-		m_ReleaseObjectsQueue.emplace_back(max, std::move(wrapper));
+		m_ReleaseObjectsQueue.emplace_back(FenceValues{ directNum, computeNum }, std::move(wrapper));
 	}
 }

Modules/Graphics/RenderDeviceD3D12.h

@@ -47,7 +47,7 @@ namespace EduEngine
 		GPUDescriptorHeap m_GPUDescriptorHeaps[2];
 		DynamicSuballocationsManager m_DynamicSuballocationMgr[2];
 
-		typedef std::pair<uint64_t, ReleaseResourceWrapper> ReleaseObject;
+		typedef std::pair<FenceValues, ReleaseResourceWrapper> ReleaseObject;
 
 		std::mutex m_ReleasedObjectsMutex;
 		DynamicUploadHeap m_DynUploadHeap; // must be before m_ReleaseObjectsQueue