Lua Editor Part 2 (#57)

* Add assert to UploadBufferHandler to prevent usage during the Render functions

Add assert to UploadBufferHandler to prevent usage during the Render functions

* Fix issue with descriptor rebinding

Descriptor binding affected in-flight frames

Author: Pursche

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

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<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
@@ -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);
@@ -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<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;

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<VkImageView>& 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:

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<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;
         };
@@ -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);
@@ -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;
 
@@ -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);
@@ -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;
+        }
+
         size_t UploadBufferHandlerVK::Allocate(size_t size, StagingBufferID& stagingBufferID, void*& mappedMemory)
         {
             UploadBufferHandlerVKData* data = static_cast<UploadBufferHandlerVKData*>(_data);

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);

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);

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;