Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Source/Renderer/Renderer/Renderer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -50,6 +58,11 @@ namespace Renderer

// Reverse map: buffer -> binding (for fast lookup during validation)
std::unordered_map<BufferID::type, u32> 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<u32, PendingBufferWrite> pendingBufferWritesPerSlot[RenderDeviceVK::FRAME_INDEX_COUNT];
};

struct DescriptorHandlerData : public IDescriptorHandlerData
Expand Down Expand Up @@ -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<DescriptorHandlerData*>(_data);
Expand Down Expand Up @@ -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 };
}
Comment thread
Pursche marked this conversation as resolved.

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);
Expand All @@ -402,6 +426,23 @@ namespace Renderer
vkUpdateDescriptorSets(_device->_device, 1, &descriptorWrite, 0, nullptr);
}

void DescriptorHandlerVK::FlushPendingBufferWrites(u32 frameIndex)
{
ZoneScoped;
DescriptorHandlerData& data = *static_cast<DescriptorHandlerData*>(_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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<VkImageView>& images, u32 arrayOffset, DescriptorType type, bool isRT, u32 frameIndex);
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> uploadsLocked = false;
std::thread::id renderThreadID;

// These are copies of the barriers which needs to run on the main commandlist
moodycamel::ConcurrentQueue<VkBufferMemoryBarrier> bufferMemoryBarriers;
};
Expand Down Expand Up @@ -168,6 +173,11 @@ namespace Renderer
{
UploadBufferHandlerVKData* data = static_cast<UploadBufferHandlerVKData*>(_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);
Expand Down Expand Up @@ -293,6 +303,10 @@ namespace Renderer

UploadBufferHandlerVKData* data = static_cast<UploadBufferHandlerVKData*>(_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;

Expand Down Expand Up @@ -356,6 +370,10 @@ namespace Renderer

UploadBufferHandlerVKData* data = static_cast<UploadBufferHandlerVKData*>(_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);
Expand Down Expand Up @@ -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<UploadBufferHandlerVKData*>(_data);
data->uploadsLocked = false;
}
Comment thread
Pursche marked this conversation as resolved.

size_t UploadBufferHandlerVK::Allocate(size_t size, StagingBufferID& stagingBufferID, void*& mappedMemory)
{
UploadBufferHandlerVKData* data = static_cast<UploadBufferHandlerVKData*>(_data);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 14 additions & 2 deletions Source/Renderer/Renderer/Renderers/Vulkan/RendererVK.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);

Comment thread
Pursche marked this conversation as resolved.
_imageHandler->FlipFrame(frameIndex);
if (_renderSizeChanged)
{
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions Source/Renderer/Renderer/Renderers/Vulkan/RendererVK.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading