From 92e3d333615620b37a729209150834f2b1a6f610 Mon Sep 17 00:00:00 2001 From: Misaki Date: Tue, 24 Mar 2026 16:46:30 +0900 Subject: [PATCH] feat(render): per-frame render requests & thread safety Refactor RenderSystem to store render requests per-frame within FrameResource, improving encapsulation and resource management. Update render loop and AddRenderRequest to use the new structure, ensuring proper disposal and clearing of requests to prevent memory leaks. Remove the old global renderRequests array and update Dispose logic accordingly. Add spin lock-based thread safety to D3D12ResourceDatabase for AddResource/AddAllocation, and introduce EnterParallelRead/ExitParallelRead methods for explicit locking. Enhance RenderExtractionSystem and Material to support transparent render lists and a MaterialRenderType property, preparing for advanced rendering features. Includes minor code cleanups and comment improvements. --- .../Systems/RenderExtractionSystem.cs | 7 +- .../D3D12GraphicsEngine.cs | 1 - .../D3D12ResourceDatabase.cs | 81 ++++++++++++----- .../Ghost.Graphics.RHI/IResourceDatabase.cs | 4 + src/Runtime/Ghost.Graphics/Core/Material.cs | 6 ++ .../Ghost.Graphics/Core/RenderingContext.cs | 6 +- .../GhostRenderPipeline.Test.cs | 5 +- src/Runtime/Ghost.Graphics/RenderSystem.cs | 88 ++++++++++--------- 8 files changed, 128 insertions(+), 70 deletions(-) diff --git a/src/Runtime/Ghost.Engine/Systems/RenderExtractionSystem.cs b/src/Runtime/Ghost.Engine/Systems/RenderExtractionSystem.cs index 6941944..f4b007a 100644 --- a/src/Runtime/Ghost.Engine/Systems/RenderExtractionSystem.cs +++ b/src/Runtime/Ghost.Engine/Systems/RenderExtractionSystem.cs @@ -93,8 +93,9 @@ public class RenderExtractionSystem : ISystem var rtSize = new uint2(rtResult.Value.TextureDescription.Width, rtResult.Value.TextureDescription.Height); var aspectScreen = (float)rtSize.x / rtSize.y; - var renderList = new RenderList(1, 64, Allocator.Temp); - var shadowCasterRenderList = new RenderList(1, 64, Allocator.Temp); + var renderList = new RenderList(1, 64, Allocator.FreeList); + var transparentRenderList = new RenderList(1, 64, Allocator.FreeList); + var shadowCasterRenderList = new RenderList(1, 64, Allocator.FreeList); // TODO: This chould be done in parallel jobs. foreach (var chunk in meshQuery.GetChunkIterator()) @@ -213,7 +214,7 @@ public class RenderExtractionSystem : ISystem depthTarget = camRef.depthTarget, opaqueRenderList = renderList, shadowCasterRenderList = shadowCasterRenderList, - transparentRenderList = default, + transparentRenderList = default, // TODO: Classify transparent objects into a separate render list and render via oit. renderFunc = camRef.renderFunc, view = new RenderView { diff --git a/src/Runtime/Ghost.Graphics.D3D12/D3D12GraphicsEngine.cs b/src/Runtime/Ghost.Graphics.D3D12/D3D12GraphicsEngine.cs index 7ab0bde..8995bcb 100644 --- a/src/Runtime/Ghost.Graphics.D3D12/D3D12GraphicsEngine.cs +++ b/src/Runtime/Ghost.Graphics.D3D12/D3D12GraphicsEngine.cs @@ -123,7 +123,6 @@ internal class D3D12GraphicsEngine : IGraphicsEngine _commandBufferReturnQueue.Enqueue(new CommandBufferReturnEntry(commandBuffer, _currentFrame)); } - public ISwapChain CreateSwapChain(SwapChainDesc desc) { ThrowIfDisposed(); diff --git a/src/Runtime/Ghost.Graphics.D3D12/D3D12ResourceDatabase.cs b/src/Runtime/Ghost.Graphics.D3D12/D3D12ResourceDatabase.cs index 6aed453..1b66f1e 100644 --- a/src/Runtime/Ghost.Graphics.D3D12/D3D12ResourceDatabase.cs +++ b/src/Runtime/Ghost.Graphics.D3D12/D3D12ResourceDatabase.cs @@ -99,7 +99,7 @@ internal class D3D12ResourceDatabase : IResourceDatabase private readonly D3D12DescriptorAllocator _descriptorAllocator; - // TODO: Change AOS to SOA? Does it even matter since we mostly access resources by handle which is essentially random access? + // TODO: Change AOS to SOA? private UnsafeSlotMap _resources; private UnsafeHashMap> _samplers; #if DEBUG || GHOST_EDITOR @@ -108,6 +108,8 @@ internal class D3D12ResourceDatabase : IResourceDatabase private UnsafeQueue _releaseQueue; + private int _writeLock; + private ulong _currentFrame; private bool _disposed; @@ -141,23 +143,37 @@ internal class D3D12ResourceDatabase : IResourceDatabase return Handle.Invalid; } - var id = _resources.Add(new ResourceRecord(pResource, initialBarrierData, viewGroup), out var generation); - var handle = new Handle(id, generation); + var spinner = new SpinWait(); + while (Interlocked.CompareExchange(ref _writeLock, 1, 0) != 0) + { + spinner.SpinOnce(); + } + + try + { + var id = _resources.Add(new ResourceRecord(pResource, initialBarrierData, viewGroup), out var generation); + var handle = new Handle(id, generation); #if DEBUG || GHOST_EDITOR - if (!string.IsNullOrEmpty(name)) - { - pResource->SetName(name); - _resourceName[handle] = name; - } + if (!string.IsNullOrEmpty(name)) + { + pResource->SetName(name); + _resourceName[handle] = name; + } #endif - return handle; + return handle; + } + finally + { + Interlocked.Exchange(ref _writeLock, 0); + } } internal unsafe Handle AddAllocation(D3D12MA_Allocation* allocation, ResourceBarrierData initialBarrierData, ResourceViewGroup resourceDescriptor, ResourceDesc desc, string? name = null) { Debug.Assert(!_disposed); + if (allocation == null) { #if DEBUG @@ -166,23 +182,48 @@ internal class D3D12ResourceDatabase : IResourceDatabase return Handle.Invalid; } - var id = _resources.Add(new ResourceRecord(allocation, initialBarrierData, resourceDescriptor, desc), out var generation); - var handle = new Handle(id, generation); + var spinner = new SpinWait(); + while (Interlocked.CompareExchange(ref _writeLock, 1, 0) != 0) + { + spinner.SpinOnce(); + } + + try + { + var id = _resources.Add(new ResourceRecord(allocation, initialBarrierData, resourceDescriptor, desc), out var generation); + var handle = new Handle(id, generation); #if DEBUG || GHOST_EDITOR - if (!string.IsNullOrEmpty(name)) - { - allocation->SetName(name); - var pResource = allocation->GetResource(); - if (pResource != null) + if (!string.IsNullOrEmpty(name)) { - pResource->SetName(name); + allocation->SetName(name); + var pResource = allocation->GetResource(); + if (pResource != null) + { + pResource->SetName(name); + } + _resourceName[handle] = name; } - _resourceName[handle] = name; - } #endif - return handle; + return handle; + } + finally + { + Interlocked.Exchange(ref _writeLock, 0); + } + } + + public void EnterParallelRead() + { + Debug.Assert(!_disposed); + Interlocked.Exchange(ref _writeLock, 1); + } + + public void ExitParallelRead() + { + Debug.Assert(!_disposed); + Interlocked.Exchange(ref _writeLock, 0); } public bool HasResource(Handle handle) diff --git a/src/Runtime/Ghost.Graphics.RHI/IResourceDatabase.cs b/src/Runtime/Ghost.Graphics.RHI/IResourceDatabase.cs index fc154fd..ef9cce2 100644 --- a/src/Runtime/Ghost.Graphics.RHI/IResourceDatabase.cs +++ b/src/Runtime/Ghost.Graphics.RHI/IResourceDatabase.cs @@ -47,6 +47,10 @@ public interface IResourceDatabase : IDisposable where T : unmanaged; */ + void EnterParallelRead(); + + void ExitParallelRead(); + /// /// Checks if a resource with the specified handle exists in the database. /// diff --git a/src/Runtime/Ghost.Graphics/Core/Material.cs b/src/Runtime/Ghost.Graphics/Core/Material.cs index 52d24d9..80f99e0 100644 --- a/src/Runtime/Ghost.Graphics/Core/Material.cs +++ b/src/Runtime/Ghost.Graphics/Core/Material.cs @@ -65,6 +65,12 @@ public struct Material : IResourceReleasable get; set; } + // For now, 0 means opaque, 1 means transparent, may be we need 2 means ui, etc. but higher values are reserved for user-defined render types. + public uint MaterialRenderType + { + get; set; + } + public Error SetShader(Identifier shaderId, ResourceManager resourceManager, IResourceDatabase resourceDatabase, IResourceAllocator resourceAllocator) { if (!shaderId.IsValid) diff --git a/src/Runtime/Ghost.Graphics/Core/RenderingContext.cs b/src/Runtime/Ghost.Graphics/Core/RenderingContext.cs index 5bad6be..750e17a 100644 --- a/src/Runtime/Ghost.Graphics/Core/RenderingContext.cs +++ b/src/Runtime/Ghost.Graphics/Core/RenderingContext.cs @@ -235,9 +235,9 @@ public readonly unsafe ref struct RenderingContext worldBoundsMax = meshData.BoundingBox.Max, vertexBuffer = _engine.ResourceDatabase.GetBindlessIndex(meshData.VertexBuffer.AsResource()), indexBuffer = _engine.ResourceDatabase.GetBindlessIndex(meshData.IndexBuffer.AsResource()), - meshletBuffer = meshData.MeshLetBuffer.IsInvalid ? 0 : _engine.ResourceDatabase.GetBindlessIndex(meshData.MeshLetBuffer.AsResource()), - meshletVerticesBuffer = meshData.MeshletVerticesBuffer.IsInvalid ? 0 : _engine.ResourceDatabase.GetBindlessIndex(meshData.MeshletVerticesBuffer.AsResource()), - meshletTrianglesBuffer = meshData.MeshletTrianglesBuffer.IsInvalid ? 0 : _engine.ResourceDatabase.GetBindlessIndex(meshData.MeshletTrianglesBuffer.AsResource()), + meshletBuffer = _engine.ResourceDatabase.GetBindlessIndex(meshData.MeshLetBuffer.AsResource()), + meshletVerticesBuffer = _engine.ResourceDatabase.GetBindlessIndex(meshData.MeshletVerticesBuffer.AsResource()), + meshletTrianglesBuffer = _engine.ResourceDatabase.GetBindlessIndex(meshData.MeshletTrianglesBuffer.AsResource()), }; var bufferHandle = meshData.ObjectDataBuffer.AsResource(); diff --git a/src/Runtime/Ghost.Graphics/RenderPipeline/GhostRenderPipeline.Test.cs b/src/Runtime/Ghost.Graphics/RenderPipeline/GhostRenderPipeline.Test.cs index b5c5850..97d15d5 100644 --- a/src/Runtime/Ghost.Graphics/RenderPipeline/GhostRenderPipeline.Test.cs +++ b/src/Runtime/Ghost.Graphics/RenderPipeline/GhostRenderPipeline.Test.cs @@ -1,3 +1,4 @@ +#if flase using Ghost.Core; using Ghost.Graphics.Core; using Ghost.Graphics.RenderGraphModule; @@ -47,7 +48,7 @@ public partial class GhostRenderPipeline private readonly uint _padding1; private readonly uint _padding2; } -#if flase + private void RenderTest(RenderGraph graph, Identifier backbuffer) { Identifier renderTarget; @@ -107,5 +108,5 @@ public partial class GhostRenderPipeline }); } } -# endif } +# endif diff --git a/src/Runtime/Ghost.Graphics/RenderSystem.cs b/src/Runtime/Ghost.Graphics/RenderSystem.cs index 0c277d0..9c6638d 100644 --- a/src/Runtime/Ghost.Graphics/RenderSystem.cs +++ b/src/Runtime/Ghost.Graphics/RenderSystem.cs @@ -8,6 +8,7 @@ using Misaki.HighPerformance.LowLevel.Collections; using Misaki.HighPerformance.Mathematics; using System.Collections.Concurrent; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; namespace Ghost.Graphics; @@ -37,6 +38,8 @@ public class RenderSystem : IDisposable { private struct FrameResource : IDisposable { + private UnsafeList _renderRequests; + public required AutoResetEvent CpuReadyEvent { get; init; @@ -57,11 +60,21 @@ public class RenderSystem : IDisposable get; set; } - public readonly void Dispose() + [UnscopedRef] + public ref UnsafeList RenderRequests => ref _renderRequests; + + public void Dispose() { CpuReadyEvent.Dispose(); GpuReadyEvent.Dispose(); CommandAllocator.Dispose(); + + for (var i = 0; i < _renderRequests.Count; i++) + { + _renderRequests[i].Dispose(); + } + + _renderRequests.Dispose(); } } @@ -74,7 +87,6 @@ public class RenderSystem : IDisposable private readonly Thread _renderThread; private readonly AutoResetEvent _shutdownEvent; - private UnsafeArray> _renderRequests; private readonly ConcurrentDictionary _resizeRequest; private IRenderPipelineSettings _renderPipelineSettings; @@ -109,6 +121,7 @@ public class RenderSystem : IDisposable return; } + _renderPipeline?.Dispose(); _renderPipelineSettings = value; _renderPipeline = _renderPipelineSettings.CreatePipeline(this); } @@ -152,7 +165,8 @@ public class RenderSystem : IDisposable { CpuReadyEvent = new AutoResetEvent(false), GpuReadyEvent = new AutoResetEvent(true), - CommandAllocator = _graphicsEngine.CreateCommandAllocator(CommandBufferType.Graphics) + CommandAllocator = _graphicsEngine.CreateCommandAllocator(CommandBufferType.Graphics), + RenderRequests = new UnsafeList(2, Allocator.Persistent) }; } @@ -165,11 +179,6 @@ public class RenderSystem : IDisposable _shutdownEvent = new AutoResetEvent(false); _resizeRequest = new ConcurrentDictionary(); - _renderRequests = new UnsafeArray>((int)desc.FrameBufferCount, Allocator.Persistent); - for (var i = 0; i < desc.FrameBufferCount; i++) - { - _renderRequests[i] = new UnsafeList(2, Allocator.Persistent); - } _renderPipelineSettings = new GhostRenderPipelineSettings(); _renderPipeline = _renderPipelineSettings.CreatePipeline(this); @@ -266,28 +275,41 @@ public class RenderSystem : IDisposable // TODO: How can we support async compute and async copy? var cmd = _graphicsEngine.GetPooledCommandBuffer(CommandBufferType.Graphics); - cmd.Begin(frameResource.CommandAllocator); - var renderCtx = new RenderContext + try { - CommandBuffer = cmd - }; + cmd.Begin(frameResource.CommandAllocator); - ref var renderRequests = ref _renderRequests[_frameIndex]; - _renderPipeline.Render(renderCtx, renderRequests.AsSpan()); + var renderCtx = new RenderContext + { + CommandBuffer = cmd + }; - // End recording commands and submit - r = cmd.End(); - if (r.IsFailure) + ref var renderRequests = ref frameResource.RenderRequests; + _renderPipeline.Render(renderCtx, renderRequests.AsSpan()); + + // End recording commands and submit + r = cmd.End(); + if (r.IsFailure) + { + StopRenderLoop(r); + break; + } + + _graphicsEngine.Device.GraphicsQueue.Submit(cmd); + + for (var i = 0; i < renderRequests.Count; i++) + { + renderRequests[i].Dispose(); + } + + renderRequests.Clear(); + } + finally { _graphicsEngine.ReturnPooledCommandBuffer(cmd); - StopRenderLoop(r); - break; } - _graphicsEngine.Device.GraphicsQueue.Submit(cmd); - _graphicsEngine.ReturnPooledCommandBuffer(cmd); - // End the frame and present _resourceManager.EndFrame(_cpuFenceValue); r = _graphicsEngine.EndFrame(_gpuFenceValue); @@ -302,13 +324,6 @@ public class RenderSystem : IDisposable // Prepare for the next frame. - for (var i = 0; i < renderRequests.Count; i++) - { - renderRequests[i].Dispose(); - } - - renderRequests.Clear(); - _gpuFenceValue++; frameResource.GpuReadyEvent.Set(); @@ -363,7 +378,7 @@ public class RenderSystem : IDisposable Debug.Assert(!_disposed, "Cannot add render request to a disposed RenderSystem."); var frameIndex = (int)(_cpuFenceValue % _config.FrameBufferCount); - _renderRequests[frameIndex].Add(request); + _frameResources[frameIndex].RenderRequests.Add(request); } public bool WaitForGPUReady(int timeOut = -1) @@ -395,21 +410,12 @@ public class RenderSystem : IDisposable Stop(); - foreach (var frameResource in _frameResources) + for (int i = 0; i < _frameResources.Length; i++) { + ref var frameResource = ref _frameResources[i]; frameResource.Dispose(); } - foreach (ref var renderRequestList in _renderRequests) - { - foreach (ref var request in renderRequestList) - { - request.Dispose(); - } - - renderRequestList.Dispose(); - } - _graphicsEngine.Dispose(); _shutdownEvent.Dispose();