diff --git a/Source/Renderer/Renderer/Renderer.h b/Source/Renderer/Renderer/Renderer.h index f4f4804e..a3966894 100644 --- a/Source/Renderer/Renderer/Renderer.h +++ b/Source/Renderer/Renderer/Renderer.h @@ -207,6 +207,9 @@ namespace Renderer virtual bool ShouldWaitForUpload() = 0; virtual void SetHasWaitedForUpload() = 0; virtual SemaphoreID GetUploadFinishedSemaphore() = 0; + // Re-opens staging uploads for the new frame's Update phase. Call before any uploads, after + // the previous frame's FlipFrame locked them. Uploads requested while locked assert. + virtual void UnlockUploads() = 0; [[nodiscard]] BufferID CreateBuffer(BufferID bufferID, BufferDesc& desc); [[nodiscard]] BufferID CreateAndFillBuffer(BufferID bufferID, BufferDesc desc, void* data, size_t dataSize); // Deletes the current BufferID if it's not invalid diff --git a/Source/Renderer/Renderer/Renderers/Vulkan/Backend/DescriptorHandlerVK.cpp b/Source/Renderer/Renderer/Renderers/Vulkan/Backend/DescriptorHandlerVK.cpp index a947ecf8..9ac1d8cf 100644 --- a/Source/Renderer/Renderer/Renderers/Vulkan/Backend/DescriptorHandlerVK.cpp +++ b/Source/Renderer/Renderer/Renderers/Vulkan/Backend/DescriptorHandlerVK.cpp @@ -28,6 +28,14 @@ namespace Renderer }; constexpr u32 maxDescriptorSets = 128; + // [Frame-safe descriptor rebind] A buffer descriptor write that must wait until its target frame + // slot is no longer being read by an in-flight frame before it can safely be applied. + struct PendingBufferWrite + { + BufferID bufferID; + DescriptorType type; + }; + struct DescriptorSet { DescriptorSetDesc desc; @@ -50,6 +58,11 @@ namespace Renderer // Reverse map: buffer -> binding (for fast lookup during validation) std::unordered_map bufferToBinding; + + // [Frame-safe descriptor rebind] Buffer-binding writes recorded per frame-copy and applied in + // FlushPendingBufferWrites once that slot's fence has been waited (its previous frame is done), + // so we never rewrite a descriptor copy an in-flight frame is still reading. + std::unordered_map pendingBufferWritesPerSlot[RenderDeviceVK::FRAME_INDEX_COUNT]; }; struct DescriptorHandlerData : public IDescriptorHandlerData @@ -339,7 +352,7 @@ namespace Renderer } } - void DescriptorHandlerVK::BindDescriptor(DescriptorSetID setID, u32 binding, BufferID bufferID, VkBuffer buffer, DescriptorType type, u32 frameIndex) + void DescriptorHandlerVK::BindDescriptor(DescriptorSetID setID, u32 binding, BufferID bufferID, DescriptorType type, u32 frameIndex) { ZoneScoped; DescriptorHandlerData& data = *static_cast(_data); @@ -385,15 +398,26 @@ namespace Renderer // Mark binding as bound descriptorSet.unboundBindings.Unset(binding); - - // Vulkan descriptor update + + // [Frame-safe descriptor rebind] A GPUVector resize swaps in a NEW VkBuffer, BufferIDs are + // recycled, and two frames are in flight sharing this set's per-frame descriptor copies. The only + // moment at which writing a slot's copy is both safe (the slot's previous frame is fully done, so + // no in-flight read races the write under UPDATE_AFTER_BIND) and current is right after that slot's + // fence has been waited. So record the desired buffer per (slot, binding) here and apply it in + // FlushPendingBufferWrites, which runs in FlipFrame immediately after that fence wait. The actual + // VkBuffer is resolved from bufferID at flush time, so it always reflects the latest generation. + descriptorSet.pendingBufferWritesPerSlot[frameIndex][binding] = PendingBufferWrite{ bufferID, type }; + } + + void DescriptorHandlerVK::WriteBufferDescriptor(DescriptorSet& descriptorSet, u32 binding, VkBuffer buffer, DescriptorType type, u32 frameIndex) + { VkDescriptorBufferInfo bufferInfo{}; bufferInfo.buffer = buffer; bufferInfo.offset = 0; bufferInfo.range = VK_WHOLE_SIZE; VkWriteDescriptorSet descriptorWrite{ VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET }; - descriptorWrite.dstSet = GetVkDescriptorSet(setID, frameIndex); + descriptorWrite.dstSet = descriptorSet.sets[frameIndex]; descriptorWrite.dstBinding = binding; descriptorWrite.descriptorCount = 1; descriptorWrite.descriptorType = FormatConverterVK::ToVkDescriptorType(type); @@ -402,6 +426,23 @@ namespace Renderer vkUpdateDescriptorSets(_device->_device, 1, &descriptorWrite, 0, nullptr); } + void DescriptorHandlerVK::FlushPendingBufferWrites(u32 frameIndex) + { + ZoneScoped; + DescriptorHandlerData& data = *static_cast(_data); + + for (DescriptorSet& descriptorSet : data.descriptorSets) + { + auto& pending = descriptorSet.pendingBufferWritesPerSlot[frameIndex]; + for (auto& [binding, write] : pending) + { + VkBuffer buffer = _bufferHandler->GetBuffer(write.bufferID); + WriteBufferDescriptor(descriptorSet, binding, buffer, write.type, frameIndex); + } + pending.clear(); + } + } + void DescriptorHandlerVK::BindDescriptor(DescriptorSetID setID, u32 binding, VkImageView image, DescriptorType type, bool isRT, u32 frameIndex) { ZoneScoped; diff --git a/Source/Renderer/Renderer/Renderers/Vulkan/Backend/DescriptorHandlerVK.h b/Source/Renderer/Renderer/Renderers/Vulkan/Backend/DescriptorHandlerVK.h index 901a4b83..beb26df9 100644 --- a/Source/Renderer/Renderer/Renderers/Vulkan/Backend/DescriptorHandlerVK.h +++ b/Source/Renderer/Renderer/Renderers/Vulkan/Backend/DescriptorHandlerVK.h @@ -33,7 +33,11 @@ namespace Renderer DescriptorSetID CreateDescriptorSet(const DescriptorSetDesc& desc); - void BindDescriptor(DescriptorSetID setID, u32 binding, BufferID bufferID, VkBuffer buffer, DescriptorType type, u32 frameIndex); + void BindDescriptor(DescriptorSetID setID, u32 binding, BufferID bufferID, DescriptorType type, u32 frameIndex); + + // [Frame-safe descriptor rebind] Apply buffer descriptor writes that were deferred while their + // frame slot was in flight. Call once per frame, right after that slot's fence has been waited. + void FlushPendingBufferWrites(u32 frameIndex); void BindDescriptor(DescriptorSetID setID, u32 binding, VkImageView image, DescriptorType type, bool isRT, u32 frameIndex); void BindDescriptorArray(DescriptorSetID setID, u32 binding, VkImageView image, u32 arrayOffset, DescriptorType type, bool isRT, u32 frameIndex); void BindDescriptorArray(DescriptorSetID setID, u32 binding, std::vector& images, u32 arrayOffset, DescriptorType type, bool isRT, u32 frameIndex); @@ -51,6 +55,7 @@ namespace Renderer private: void CreateDescriptorPool(); void CreateDescriptorSet(DescriptorSet& descriptorSet); + void WriteBufferDescriptor(DescriptorSet& descriptorSet, u32 binding, VkBuffer buffer, DescriptorType type, u32 frameIndex); bool ValidatePermissionViolations(u32 slot, const DescriptorSet& descriptorSet, const PersistentBitSet& accesses, const BitSet& permissions, const char* permissionName, const PersistentBitSet* usedBindings = nullptr); private: diff --git a/Source/Renderer/Renderer/Renderers/Vulkan/Backend/UploadBufferHandlerVK.cpp b/Source/Renderer/Renderer/Renderers/Vulkan/Backend/UploadBufferHandlerVK.cpp index 59cc3724..371205e9 100644 --- a/Source/Renderer/Renderer/Renderers/Vulkan/Backend/UploadBufferHandlerVK.cpp +++ b/Source/Renderer/Renderer/Renderers/Vulkan/Backend/UploadBufferHandlerVK.cpp @@ -115,6 +115,11 @@ namespace Renderer std::mutex submitMutex; SemaphoreID uploadFinishedSemaphore; + // An upload on the render-loop thread while locked would land a frame late, so it asserts. + // Worker-thread (async streaming) uploads are exempt. + std::atomic uploadsLocked = false; + std::thread::id renderThreadID; + // These are copies of the barriers which needs to run on the main commandlist moodycamel::ConcurrentQueue bufferMemoryBarriers; }; @@ -168,6 +173,11 @@ namespace Renderer { UploadBufferHandlerVKData* data = static_cast(_data); + // Per-frame submit point: lock until UnlockUploads() at the end of Render, and capture the + // render-loop thread so the assert is scoped to it. + data->renderThreadID = std::this_thread::get_id(); + data->uploadsLocked = true; + for (u32 i = 0; i < data->stagingBuffers.Num; i++) { StagingBuffer& stagingBuffer = data->stagingBuffers.Get(i); @@ -293,6 +303,10 @@ namespace Renderer UploadBufferHandlerVKData* data = static_cast(_data); + // Thread check first so worker-thread (async streaming) uploads short-circuit out. + NC_ASSERT(!(std::this_thread::get_id() == data->renderThreadID && data->uploadsLocked), + "UploadBufferHandlerVK : Staging upload requested after this frame's uploads were submitted (FlipFrame). Move it to the Update phase, before Render."); + void* mappedMemory = nullptr; StagingBufferID stagingBufferID; @@ -356,6 +370,10 @@ namespace Renderer UploadBufferHandlerVKData* data = static_cast(_data); + // Thread check first so worker-thread (async streaming) uploads short-circuit out. + NC_ASSERT(!(std::this_thread::get_id() == data->renderThreadID && data->uploadsLocked), + "UploadBufferHandlerVK : Staging upload requested after this frame's uploads were submitted (FlipFrame). Move it to the Update phase, before Render."); + void* mappedMemory = nullptr; StagingBufferID stagingBufferID; size_t offset = Allocate(size, stagingBufferID, mappedMemory); @@ -483,6 +501,13 @@ namespace Renderer data->needsWait = false; } + void UploadBufferHandlerVK::UnlockUploads() + { + // Called at the end of Render; the next frame's Update phase may upload again. + UploadBufferHandlerVKData* data = static_cast(_data); + data->uploadsLocked = false; + } + size_t UploadBufferHandlerVK::Allocate(size_t size, StagingBufferID& stagingBufferID, void*& mappedMemory) { UploadBufferHandlerVKData* data = static_cast(_data); diff --git a/Source/Renderer/Renderer/Renderers/Vulkan/Backend/UploadBufferHandlerVK.h b/Source/Renderer/Renderer/Renderers/Vulkan/Backend/UploadBufferHandlerVK.h index 06a0b0fd..7ac4ea7a 100644 --- a/Source/Renderer/Renderer/Renderers/Vulkan/Backend/UploadBufferHandlerVK.h +++ b/Source/Renderer/Renderer/Renderers/Vulkan/Backend/UploadBufferHandlerVK.h @@ -46,6 +46,7 @@ namespace Renderer SemaphoreID GetUploadFinishedSemaphore(); bool ShouldWaitForUpload(); void SetHasWaitedForUpload(); + void UnlockUploads(); private: size_t Allocate(size_t size, StagingBufferID& stagingBufferID, void*& mappedMemory); void ExecuteStagingBuffer(VkCommandBuffer commandBuffer, StagingBuffer& stagingBuffer); diff --git a/Source/Renderer/Renderer/Renderers/Vulkan/RendererVK.cpp b/Source/Renderer/Renderer/Renderers/Vulkan/RendererVK.cpp index e75967aa..b673f3c9 100644 --- a/Source/Renderer/Renderer/Renderers/Vulkan/RendererVK.cpp +++ b/Source/Renderer/Renderer/Renderers/Vulkan/RendererVK.cpp @@ -253,8 +253,10 @@ namespace Renderer void RendererVK::BindDescriptor(DescriptorSetID descriptorSetID, u32 bindingIndex, BufferID bufferID, DescriptorType type, u32 frameIndex) { - VkBuffer buffer = _bufferHandler->GetBuffer(bufferID); - _descriptorHandler->BindDescriptor(descriptorSetID, bindingIndex, bufferID, buffer, type, frameIndex); + // [Frame-safe descriptor rebind] The handler records this write per frame-copy and applies it in + // FlushPendingBufferWrites (FlipFrame), after that slot's fence has been waited. The VkBuffer is + // resolved from bufferID at that point, so it always reflects the latest GPUVector generation. + _descriptorHandler->BindDescriptor(descriptorSetID, bindingIndex, bufferID, type, frameIndex); } void RendererVK::BindDescriptor(DescriptorSetID descriptorSetID, u32 bindingIndex, ImageID imageID, u32 mipLevel, DescriptorType type, u32 frameIndex) @@ -409,6 +411,11 @@ namespace Renderer timeWaited = timer.GetLifeTime(); } + // [Frame-safe descriptor rebind] This slot's previous frame has now completed (fence waited above), + // so it is finally safe to apply any buffer descriptor writes that were deferred while it was in + // flight. Done before any of this frame's command lists record/use the descriptor sets. + _descriptorHandler->FlushPendingBufferWrites(_frameIndex); + _imageHandler->FlipFrame(frameIndex); if (_renderSizeChanged) { @@ -1997,6 +2004,11 @@ namespace Renderer return _uploadBufferHandler->GetUploadFinishedSemaphore(); } + void RendererVK::UnlockUploads() + { + _uploadBufferHandler->UnlockUploads(); + } + void RendererVK::CopyBuffer(BufferID dstBuffer, u64 dstOffset, BufferID srcBuffer, u64 srcOffset, u64 range) { _uploadBufferHandler->CopyBufferToBuffer(dstBuffer, dstOffset, srcBuffer, srcOffset, range); diff --git a/Source/Renderer/Renderer/Renderers/Vulkan/RendererVK.h b/Source/Renderer/Renderer/Renderers/Vulkan/RendererVK.h index a67b87f6..c5a9b37c 100644 --- a/Source/Renderer/Renderer/Renderers/Vulkan/RendererVK.h +++ b/Source/Renderer/Renderer/Renderers/Vulkan/RendererVK.h @@ -175,6 +175,7 @@ namespace Renderer [[nodiscard]] bool ShouldWaitForUpload() override; void SetHasWaitedForUpload() override; [[nodiscard]] SemaphoreID GetUploadFinishedSemaphore() override; + void UnlockUploads() override; // Uses the upload handler to schedule it for next frames command list void CopyBuffer(BufferID dstBuffer, u64 dstOffset, BufferID srcBuffer, u64 srcOffset, u64 range) override;