diff --git a/destrum/CMakeLists.txt b/destrum/CMakeLists.txt index 72b36ce..40e93fd 100644 --- a/destrum/CMakeLists.txt +++ b/destrum/CMakeLists.txt @@ -58,6 +58,7 @@ set(SRC_FILES "src/Graphics/BindlessSetManager.cpp" "src/Graphics/Camera.cpp" "src/Graphics/DebugDraw.cpp" + "src/Graphics/DeletionQueue.cpp" "src/Graphics/ComputePipeline.cpp" "src/Graphics/Frustum.cpp" "src/Graphics/GfxDevice.cpp" diff --git a/destrum/include/destrum/Graphics/Caches/ImageCache.h b/destrum/include/destrum/Graphics/Caches/ImageCache.h index b4fb5f9..d4d8394 100644 --- a/destrum/include/destrum/Graphics/Caches/ImageCache.h +++ b/destrum/include/destrum/Graphics/Caches/ImageCache.h @@ -4,6 +4,7 @@ #pragma once #include +#include #include #include @@ -28,6 +29,7 @@ public: ImageID addImage(GPUImage image); ImageID addImage(ImageID id, GPUImage image); [[nodiscard]] const GPUImage& getImage(ImageID id) const; + void removeImage(ImageID id); [[nodiscard]] ImageID getFreeImageId() const; [[nodiscard]] std::uint32_t getMaxImageCount() const { @@ -52,6 +54,7 @@ public: private: std::vector images; + std::shared_ptr> freeIds = std::make_shared>(); GfxDevice& gfxDevice; struct LoadedImageInfo { diff --git a/destrum/include/destrum/Graphics/Caches/MaterialCache.h b/destrum/include/destrum/Graphics/Caches/MaterialCache.h index 7f61fff..636c575 100644 --- a/destrum/include/destrum/Graphics/Caches/MaterialCache.h +++ b/destrum/include/destrum/Graphics/Caches/MaterialCache.h @@ -47,7 +47,7 @@ public: Material& getMaterialMutable(MaterialID id); - void updateMaterialGPU(MaterialID id); + [[nodiscard]] MaterialData getMaterialData(MaterialID id) const; private: [[nodiscard]] MaterialData toMaterialData(const Material& material) const; diff --git a/destrum/include/destrum/Graphics/DeletionQueue.h b/destrum/include/destrum/Graphics/DeletionQueue.h new file mode 100644 index 0000000..402347a --- /dev/null +++ b/destrum/include/destrum/Graphics/DeletionQueue.h @@ -0,0 +1,29 @@ +#ifndef DELETIONQUEUE_H +#define DELETIONQUEUE_H + +#include +#include +#include +#include + +class DeletionQueue { +public: + void push(std::uint64_t retiredInFrame, std::function&& deleter); + + void flush(std::uint64_t lastCompletedFrame); + + void flushAll(); + + [[nodiscard]] std::size_t size() const { return entries.size(); } + [[nodiscard]] bool empty() const { return entries.empty(); } + +private: + struct Entry { + std::uint64_t frame; + std::function deleter; + }; + + std::deque entries; +}; + +#endif // DELETIONQUEUE_H diff --git a/destrum/include/destrum/Graphics/FrameConfig.h b/destrum/include/destrum/Graphics/FrameConfig.h new file mode 100644 index 0000000..56dcb30 --- /dev/null +++ b/destrum/include/destrum/Graphics/FrameConfig.h @@ -0,0 +1,8 @@ +#ifndef FRAMECONFIG_H +#define FRAMECONFIG_H + +#include + +inline constexpr std::uint32_t FRAMES_IN_FLIGHT = 2; + +#endif // FRAMECONFIG_H diff --git a/destrum/include/destrum/Graphics/GfxDevice.h b/destrum/include/destrum/Graphics/GfxDevice.h index 0ed3b04..094ee98 100644 --- a/destrum/include/destrum/Graphics/GfxDevice.h +++ b/destrum/include/destrum/Graphics/GfxDevice.h @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -113,7 +114,8 @@ public: return memoryManager.createBuffer(allocSize, usage, memoryUsage); } - void destroyBuffer(GPUBuffer& buffer) const + void destroyBuffer(GPUBuffer& buffer); + void destroyBufferNow(GPUBuffer& buffer) const { memoryManager.destroyBuffer(buffer); } @@ -148,11 +150,24 @@ public: imageManager.uploadImageData(image, pixelData, byteSize, layer); } - void destroyImage(GPUImage& image) const + void destroyImage(GPUImage& image); + void destroyImageNow(GPUImage& image) const { imageManager.destroyImage(image); } + void deferDestroy(std::function&& deleter); + + [[nodiscard]] std::uint64_t getFrameNumber() const + { + return frameManager.getFrameNumber(); + } + + [[nodiscard]] std::size_t getPendingDeletionCount() const + { + return deletionQueue.size(); + } + [[nodiscard]] VkDevice getVkDevice() const { return instanceManager.getDevice(); @@ -228,6 +243,7 @@ private: ImageManager imageManager; VulkanImmediateExecutor executor; + DeletionQueue deletionQueue; Swapchain swapchain; VkFormat swapchainFormat{VK_FORMAT_UNDEFINED}; diff --git a/destrum/include/destrum/Graphics/Managers/FrameManager.h b/destrum/include/destrum/Graphics/Managers/FrameManager.h index e591d65..f33b8fb 100644 --- a/destrum/include/destrum/Graphics/Managers/FrameManager.h +++ b/destrum/include/destrum/Graphics/Managers/FrameManager.h @@ -6,6 +6,8 @@ #include +#include + class FrameManager { public: struct FrameData { @@ -27,10 +29,12 @@ public: [[nodiscard]] VkCommandBuffer beginFrame(); void endFrame(VkCommandBuffer cmd); - void waitIdle() const; - [[nodiscard]] std::uint32_t getCurrentFrameIndex() const { - return frameNumber % 2; + return static_cast(frameNumber % FRAMES_IN_FLIGHT); + } + + [[nodiscard]] std::uint64_t getFrameNumber() const { + return frameNumber; } [[nodiscard]] FrameData& getCurrentFrame() { @@ -48,9 +52,8 @@ public: void nextFrame(); private: - static constexpr std::size_t FRAMES_IN_FLIGHT = 2; std::array frames{}; - std::uint32_t frameNumber{0}; + std::uint64_t frameNumber{0}; VkDevice device{VK_NULL_HANDLE}; }; diff --git a/destrum/include/destrum/Graphics/RenderResources.h b/destrum/include/destrum/Graphics/RenderResources.h index 7cdb960..b858850 100644 --- a/destrum/include/destrum/Graphics/RenderResources.h +++ b/destrum/include/destrum/Graphics/RenderResources.h @@ -46,6 +46,7 @@ public: TextureIntent intent = TextureIntent::ColorSrgb); ImageID addImageToCache(GPUImage image); + void destroyImage(ImageID id); // Imports a file-backed model and uploads its mesh/material resources. // References are keyed as "source#subresource" in the resource caches. diff --git a/destrum/include/destrum/Graphics/Renderer.h b/destrum/include/destrum/Graphics/Renderer.h index a1c41e5..4f35216 100644 --- a/destrum/include/destrum/Graphics/Renderer.h +++ b/destrum/include/destrum/Graphics/Renderer.h @@ -65,7 +65,6 @@ public: void setSkyboxTexture(ImageID skyboxImageId); - void flushMaterialUpdates(GfxDevice& gfxDevice); [[nodiscard]] SkinningPipeline& getSkinningPipeline() const { @@ -119,6 +118,8 @@ private: RenderResources* resources = nullptr; + void recordMaterialUpdates(VkCommandBuffer cmd); + std::vector pendingMaterialUploads; diff --git a/destrum/include/destrum/Graphics/Swapchain.h b/destrum/include/destrum/Graphics/Swapchain.h index d373a34..a7e1312 100644 --- a/destrum/include/destrum/Graphics/Swapchain.h +++ b/destrum/include/destrum/Graphics/Swapchain.h @@ -8,7 +8,7 @@ #include -static constexpr int FRAMES_IN_FLIGHT = 2; +#include class Swapchain { public: diff --git a/destrum/include/destrum/Graphics/Util.h b/destrum/include/destrum/Graphics/Util.h index 65bbce6..2cd383e 100644 --- a/destrum/include/destrum/Graphics/Util.h +++ b/destrum/include/destrum/Graphics/Util.h @@ -83,6 +83,18 @@ namespace vkutil { VkDeviceSize offset = 0, VkDeviceSize size = VK_WHOLE_SIZE); + void bufferShaderReadToTransferWriteBarrier( + VkCommandBuffer cmd, + VkBuffer buffer, + VkDeviceSize offset = 0, + VkDeviceSize size = VK_WHOLE_SIZE); + + void bufferHostAndTransferWriteToShaderReadBarrier( + VkCommandBuffer cmd, + VkBuffer buffer, + VkDeviceSize offset = 0, + VkDeviceSize size = VK_WHOLE_SIZE); + void bufferHostWriteToTransferReadBarrier( VkCommandBuffer cmd, VkBuffer buffer, diff --git a/destrum/src/App.cpp b/destrum/src/App.cpp index 6308217..b5e6a30 100644 --- a/destrum/src/App.cpp +++ b/destrum/src/App.cpp @@ -331,8 +331,6 @@ if (!isRunning) break; } } } - - gfxDevice.waitIdle(); } void App::cleanup() @@ -355,6 +353,10 @@ void App::cleanup() } }; + // The one place we wait for the GPU on shutdown: the cleanup calls below destroy + // pipelines and other objects directly, which is only valid once nothing is in flight. + attempt([this] { gfxDevice.waitIdle(); }); + if (customInitStarted) { attempt([this] { customCleanup(); }); } diff --git a/destrum/src/Components/MeshRendererComponent.cpp b/destrum/src/Components/MeshRendererComponent.cpp index 9f69609..b917717 100644 --- a/destrum/src/Components/MeshRendererComponent.cpp +++ b/destrum/src/Components/MeshRendererComponent.cpp @@ -47,7 +47,6 @@ void MeshRendererComponent::Destroy() { if (m_skinnedMesh && GameState::GetInstance().HasGfxDevice()) { auto& gfxDevice = GameState::GetInstance().Gfx(); - gfxDevice.waitIdle(); gfxDevice.destroyBuffer(m_skinnedMesh->skinnedVertexBuffer); } m_skinnedMesh.reset(); diff --git a/destrum/src/Graphics/Caches/ImageCache.cpp b/destrum/src/Graphics/Caches/ImageCache.cpp index 03793c4..8de5fb7 100644 --- a/destrum/src/Graphics/Caches/ImageCache.cpp +++ b/destrum/src/Graphics/Caches/ImageCache.cpp @@ -1,8 +1,9 @@ -#include <../../include/destrum/Graphics/Caches/ImageCache.h> +#include #include #include "spdlog/spdlog.h" +#include #include #include @@ -57,7 +58,7 @@ ImageID ImageCache::loadImageFromFile( if (infoInserted) { loadedImagesInfo.erase(id); } - gfxDevice.destroyImage(image); + gfxDevice.destroyImageNow(image); throw; } } @@ -66,7 +67,7 @@ ImageID ImageCache::addImage(GPUImage image) { try { return addImage(getFreeImageId(), std::move(image)); } catch (...) { - gfxDevice.destroyImage(image); + gfxDevice.destroyImageNow(image); throw; } } @@ -82,6 +83,10 @@ ImageID ImageCache::addImage(ImageID id, GPUImage image) { try { image.setBindlessId(static_cast(id)); + if (const auto it = std::find(freeIds->begin(), freeIds->end(), id); it != freeIds->end()) { + freeIds->erase(it); + } + if (id < images.size()) { gfxDevice.destroyImage(images[id]); images[id] = std::move(image); @@ -97,17 +102,35 @@ ImageID ImageCache::addImage(ImageID id, GPUImage image) { return id; } catch (...) { - gfxDevice.destroyImage(image); + gfxDevice.destroyImageNow(image); throw; } } +void ImageCache::removeImage(ImageID id) { + if (id >= images.size() || images[id].image == VK_NULL_HANDLE) { + return; + } + + loadedImagesInfo.erase(id); + gfxDevice.destroyImage(images[id]); + + // In-flight frames may still sample this bindless slot, so it only becomes + // reusable once they are done. freeIds is shared so the deleter can outlive the cache. + gfxDevice.deferDestroy([freeIds = freeIds, id] { + freeIds->push_back(id); + }); +} + const GPUImage& ImageCache::getImage(ImageID id) const { assert(id != NULL_IMAGE_ID && id < images.size()); return images.at(id); } ImageID ImageCache::getFreeImageId() const { + if (!freeIds->empty()) { + return freeIds->back(); + } if (images.size() >= getMaxImageCount()) { throw std::out_of_range("ImageCache bindless capacity exhausted"); } @@ -122,5 +145,6 @@ void ImageCache::destroyImages() { gfxDevice.destroyImage(image); } images.clear(); + freeIds->clear(); loadedImagesInfo.clear(); } diff --git a/destrum/src/Graphics/Caches/MaterialCache.cpp b/destrum/src/Graphics/Caches/MaterialCache.cpp index 1f845e3..acabe4e 100644 --- a/destrum/src/Graphics/Caches/MaterialCache.cpp +++ b/destrum/src/Graphics/Caches/MaterialCache.cpp @@ -1,4 +1,4 @@ -#include <../../include/destrum/Graphics/Caches/MaterialCache.h> +#include #include #include @@ -19,7 +19,8 @@ void MaterialCache::init( materialDataBuffer = gfxDevice.createBuffer( MAX_MATERIALS * sizeof(MaterialData), - VK_BUFFER_USAGE_STORAGE_BUFFER_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT, + VK_BUFFER_USAGE_STORAGE_BUFFER_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT | + VK_BUFFER_USAGE_TRANSFER_DST_BIT, VMA_MEMORY_USAGE_CPU_TO_GPU); vkutil::addDebugLabel( @@ -51,12 +52,6 @@ void MaterialCache::cleanup(GfxDevice& gfxDevice) MaterialID MaterialCache::addMaterial(Material material) { - if (!materials.empty() && device != nullptr) { - // The material buffer is shared by all in-flight frames. A newly - // appended entry may be written while an older frame is reading it. - device->waitIdle(); - } - MaterialData* data = static_cast(materialDataBuffer.info.pMappedData); const auto id = getFreeMaterialId(); @@ -67,17 +62,14 @@ MaterialID MaterialCache::addMaterial(Material material) throw std::runtime_error("MaterialCache material buffer is not mapped"); } + // Safe without waiting: no frame in flight can reference an id that didn't exist yet. + // Changing an existing material goes through GameRenderer::updateMaterialGPU. data[id] = toMaterialData(material); device->getMemoryManager().flushAllocation( materialDataBuffer, static_cast(id) * sizeof(MaterialData), sizeof(MaterialData)); - // The caller may update multiple materials in one frame; the range flush is - // cheap for coherent allocations and required for non-coherent memory. - // The cache does not own a device reference, so the renderer flushes the - // exact range after batching updates. - const auto materialId = static_cast(materials.size()); std::string key; if (!material.assetReference.empty()) { @@ -210,17 +202,8 @@ Material& MaterialCache::getMaterialMutable(MaterialID id) return materials.at(id); } -void MaterialCache::updateMaterialGPU(MaterialID id) +MaterialData MaterialCache::getMaterialData(MaterialID id) const { assert(id < materials.size()); - assert(materialDataBuffer.info.pMappedData && "materialDataBuffer must be mapped"); - - MaterialData* data = - reinterpret_cast(materialDataBuffer.info.pMappedData); - - data[id] = toMaterialData(materials[id]); - device->getMemoryManager().flushAllocation( - materialDataBuffer, - static_cast(id) * sizeof(MaterialData), - sizeof(MaterialData)); + return toMaterialData(materials[id]); } diff --git a/destrum/src/Graphics/DeletionQueue.cpp b/destrum/src/Graphics/DeletionQueue.cpp new file mode 100644 index 0000000..2c945cf --- /dev/null +++ b/destrum/src/Graphics/DeletionQueue.cpp @@ -0,0 +1,39 @@ +#include + +#include +#include + +#include + +void DeletionQueue::push(std::uint64_t retiredInFrame, std::function&& deleter) +{ + // entries stay sorted by frame, flush only has to look at the front + assert(entries.empty() || entries.back().frame <= retiredInFrame); + entries.push_back(Entry{retiredInFrame, std::move(deleter)}); +} + +void DeletionQueue::flush(std::uint64_t lastCompletedFrame) +{ + while (!entries.empty() && entries.front().frame <= lastCompletedFrame) { + auto deleter = std::move(entries.front().deleter); + entries.pop_front(); + try { + deleter(); + } catch (const std::exception& e) { + spdlog::error("DeletionQueue: deleter threw: {}", e.what()); + } + } +} + +void DeletionQueue::flushAll() +{ + while (!entries.empty()) { + auto deleter = std::move(entries.front().deleter); + entries.pop_front(); + try { + deleter(); + } catch (const std::exception& e) { + spdlog::error("DeletionQueue: deleter threw: {}", e.what()); + } + } +} diff --git a/destrum/src/Graphics/GfxDevice.cpp b/destrum/src/Graphics/GfxDevice.cpp index a51f0c9..2f4d124 100644 --- a/destrum/src/Graphics/GfxDevice.cpp +++ b/destrum/src/Graphics/GfxDevice.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -83,6 +84,7 @@ void GfxDevice::cleanup() { if (instanceManager.getDevice() != VK_NULL_HANDLE) { vkDeviceWaitIdle(instanceManager.getDevice()); } + deletionQueue.flushAll(); swapchain.cleanup(instanceManager.getDevice()); frameManager.cleanup(instanceManager.getDevice()); @@ -103,7 +105,6 @@ bool GfxDevice::recreateSwapchain(int width, int height) { if (width <= 0 || height <= 0) { throw std::invalid_argument("GfxDevice::recreateSwapchain requires a non-zero extent"); } - waitIdle(); const bool recreated = swapchain.recreateSwapchain( instanceManager.getDevice(), @@ -129,6 +130,16 @@ VkCommandBuffer GfxDevice::beginFrame() swapchain.beginFrame(instanceManager.getDevice(), frameIndex); } + { + ZoneScopedN("Flush Deletion Queue"); + // The fence we just waited on belongs to frame N - FRAMES_IN_FLIGHT, and the + // queue executes submits in order, so everything retired up to that frame is free. + const auto frameNumber = frameManager.getFrameNumber(); + if (frameNumber >= FRAMES_IN_FLIGHT) { + deletionQueue.flush(frameNumber - FRAMES_IN_FLIGHT); + } + } + const auto acquire = swapchain.acquireNextImage(instanceManager.getDevice(), frameIndex); if (!acquire.hasImage()) { return VK_NULL_HANDLE; @@ -240,8 +251,39 @@ void GfxDevice::endFrame(VkCommandBuffer cmd, const GPUImage& drawImage, const E FrameMark; } +void GfxDevice::deferDestroy(std::function&& deleter) { + if (!initialized) { + deleter(); + return; + } + // Tagged with the frame being recorded (or the next one, between frames); any + // frame up to and including it may still reference the resource. + deletionQueue.push(frameManager.getFrameNumber(), std::move(deleter)); +} + +void GfxDevice::destroyBuffer(GPUBuffer& buffer) { + if (buffer.buffer == VK_NULL_HANDLE) { + buffer = {}; + return; + } + deferDestroy([this, retired = buffer]() mutable { + memoryManager.destroyBuffer(retired); + }); + buffer = {}; +} + +void GfxDevice::destroyImage(GPUImage& image) { + if (image.image == VK_NULL_HANDLE && image.imageView == VK_NULL_HANDLE) { + return; + } + auto retired = std::make_shared(std::move(image)); + deferDestroy([this, retired] { + imageManager.destroyImage(*retired); + }); +} + void GfxDevice::waitIdle() { - frameManager.waitIdle(); + instanceManager.waitIdle(); } void GfxDevice::immediateSubmit(ImmediateExecuteFunction&& f) const { diff --git a/destrum/src/Graphics/Managers/FrameManager.cpp b/destrum/src/Graphics/Managers/FrameManager.cpp index d42cae3..3b5291d 100644 --- a/destrum/src/Graphics/Managers/FrameManager.cpp +++ b/destrum/src/Graphics/Managers/FrameManager.cpp @@ -83,9 +83,3 @@ void FrameManager::nextFrame() { frameNumber++; FrameMark; } - -void FrameManager::waitIdle() const { - if (device != VK_NULL_HANDLE) { - VK_CHECK(vkDeviceWaitIdle(device)); - } -} diff --git a/destrum/src/Graphics/MeshCache.cpp b/destrum/src/Graphics/MeshCache.cpp index 8793ade..d558b2a 100644 --- a/destrum/src/Graphics/MeshCache.cpp +++ b/destrum/src/Graphics/MeshCache.cpp @@ -1,4 +1,4 @@ -#include <../../include/destrum/Graphics/Caches/MeshCache.h> +#include #include #include @@ -136,11 +136,11 @@ void MeshCache::uploadMesh(GfxDevice& gfxDevice, const CPUMesh& cpuMesh, GPUMesh vkCmdPipelineBarrier2(cmd, &destinationDependency); }); } catch (...) { - gfxDevice.destroyBuffer(vertexIndexStaging); + gfxDevice.destroyBufferNow(vertexIndexStaging); throw; } - gfxDevice.destroyBuffer(vertexIndexStaging); + gfxDevice.destroyBufferNow(vertexIndexStaging); const auto vtxBufferName = cpuMesh.name + " (vtx)"; const auto idxBufferName = cpuMesh.name + " (idx)"; @@ -205,16 +205,16 @@ void MeshCache::uploadMesh(GfxDevice& gfxDevice, const CPUMesh& cpuMesh, GPUMesh vkCmdPipelineBarrier2(cmd, &destinationDependency); }); } catch (...) { - gfxDevice.destroyBuffer(skinningStaging); + gfxDevice.destroyBufferNow(skinningStaging); throw; } - gfxDevice.destroyBuffer(skinningStaging); + gfxDevice.destroyBufferNow(skinningStaging); } } catch (...) { - gfxDevice.destroyBuffer(gpuMesh.skinningDataBuffer); - gfxDevice.destroyBuffer(gpuMesh.vertexBuffer); - gfxDevice.destroyBuffer(gpuMesh.indexBuffer); + gfxDevice.destroyBufferNow(gpuMesh.skinningDataBuffer); + gfxDevice.destroyBufferNow(gpuMesh.vertexBuffer); + gfxDevice.destroyBufferNow(gpuMesh.indexBuffer); gpuMesh = {}; throw; } diff --git a/destrum/src/Graphics/Pipelines/ImguiPass.cpp b/destrum/src/Graphics/Pipelines/ImguiPass.cpp index 4cc98b4..8fbcbd8 100644 --- a/destrum/src/Graphics/Pipelines/ImguiPass.cpp +++ b/destrum/src/Graphics/Pipelines/ImguiPass.cpp @@ -242,10 +242,6 @@ void ImguiPass::cleanup() { return; } - if (gfx != nullptr && gfx->getVkDevice() != VK_NULL_HANDLE) { - vkDeviceWaitIdle(gfx->getVkDevice()); - } - if (vulkanInitialized) { releaseAllTextures(); ImGui_ImplVulkan_Shutdown(); diff --git a/destrum/src/Graphics/Pipelines/LightCullingPipeline.cpp b/destrum/src/Graphics/Pipelines/LightCullingPipeline.cpp index e0889e0..19573f0 100644 --- a/destrum/src/Graphics/Pipelines/LightCullingPipeline.cpp +++ b/destrum/src/Graphics/Pipelines/LightCullingPipeline.cpp @@ -100,8 +100,6 @@ void LightCullingPipeline::cleanup(GfxDevice& gfxDevice) void LightCullingPipeline::resize(GfxDevice& gfxDevice, glm::ivec2 drawImageSize) { - gfxDevice.waitIdle(); - gfxDevice.destroyBuffer(tileLightIndicesBuffer); tileLightIndicesBuffer = {}; createTileBuffer(gfxDevice, drawImageSize); diff --git a/destrum/src/Graphics/Pipelines/LineRenderingPass.cpp b/destrum/src/Graphics/Pipelines/LineRenderingPass.cpp index 02c6e23..2e93559 100644 --- a/destrum/src/Graphics/Pipelines/LineRenderingPass.cpp +++ b/destrum/src/Graphics/Pipelines/LineRenderingPass.cpp @@ -221,7 +221,6 @@ void LineRenderingPass::ensureVertexCapacity( newCapacity *= 2; } - gfxDevice.waitIdle(); vertexBuffer.cleanup(gfxDevice); vertexBuffer.init( gfxDevice, diff --git a/destrum/src/Graphics/Pipelines/Renderer2DPass.cpp b/destrum/src/Graphics/Pipelines/Renderer2DPass.cpp index 85081da..93072ad 100644 --- a/destrum/src/Graphics/Pipelines/Renderer2DPass.cpp +++ b/destrum/src/Graphics/Pipelines/Renderer2DPass.cpp @@ -210,7 +210,6 @@ void Renderer2DPass::ensureCapacity(GfxDevice& gfxDevice, std::size_t requiredSh newCapacity *= 2; } - gfxDevice.waitIdle(); shapeBuffer.cleanup(gfxDevice); shapeBuffer.init(gfxDevice, ShapeBufferUsage, newCapacity * sizeof(Shape2D), "2d shapes"); shapeCapacity = newCapacity; diff --git a/destrum/src/Graphics/RenderResources.cpp b/destrum/src/Graphics/RenderResources.cpp index 4924304..f55b05f 100644 --- a/destrum/src/Graphics/RenderResources.cpp +++ b/destrum/src/Graphics/RenderResources.cpp @@ -204,6 +204,10 @@ ImageID RenderResources::addImageToCache(GPUImage image) { return imageCache->addImage(std::move(image)); } +void RenderResources::destroyImage(ImageID id) { + imageCache->removeImage(id); +} + void RenderResources::loadModelAsset( GfxDevice& gfxDevice, std::string_view sourceReference) @@ -322,8 +326,6 @@ void RenderResources::cleanup(GfxDevice& gfxDevice) return; } - gfxDevice.waitIdle(); - if (materialCache) { materialCache->cleanup(gfxDevice); } diff --git a/destrum/src/Graphics/Renderer.cpp b/destrum/src/Graphics/Renderer.cpp index 6eeb0ba..c2699ae 100644 --- a/destrum/src/Graphics/Renderer.cpp +++ b/destrum/src/Graphics/Renderer.cpp @@ -65,7 +65,6 @@ void GameRenderer::init(GfxDevice& gfxDevice, RenderResources& _resources, glm:: void GameRenderer::beginDrawing(GfxDevice& gfxDevice) { - flushMaterialUpdates(gfxDevice); meshDrawCommands.clear(); skinningPipeline->beginDrawing(gfxDevice.getCurrentFrameIndex()); lightCullingPipeline->beginDrawing(gfxDevice.getCurrentFrameIndex()); @@ -175,15 +174,10 @@ void GameRenderer::draw(VkCommandBuffer cmd, GfxDevice& gfxDevice, const Camera& }; { - TracyVkZone(gfxDevice.getTracyVkCtx(), cmd, "Material Buffer Barrier"); - vkutil::ScopedCmdLabel label(cmd, "Material Buffer Barrier", vkutil::labelColors::transfer); + TracyVkZone(gfxDevice.getTracyVkCtx(), cmd, "Material Updates"); + vkutil::ScopedCmdLabel label(cmd, "Material Updates", vkutil::labelColors::transfer); - vkutil::bufferHostWriteToShaderReadBarrier( - cmd, - resources->materials().getMaterialDataBuffer().buffer, - 0, - VK_WHOLE_SIZE - ); + recordMaterialUpdates(cmd); } { @@ -384,11 +378,6 @@ void GameRenderer::cleanup(GfxDevice& gfxDevice) { VkDevice device = gfxDevice.getDevice(); - if (device != VK_NULL_HANDLE) - { - vkDeviceWaitIdle(device); - } - if (skinningPipeline) skinningPipeline->cleanup(gfxDevice); @@ -541,25 +530,32 @@ void GameRenderer::setSkyboxTexture(ImageID skyboxImageId) skyboxPipeline->setSkyboxImage(skyboxImageId); } -void GameRenderer::flushMaterialUpdates(GfxDevice& gfxDevice) +void GameRenderer::recordMaterialUpdates(VkCommandBuffer cmd) { + const VkBuffer materialBuffer = resources->materials().getMaterialDataBuffer().buffer; + if (!pendingMaterialUploads.empty()) { - // Material data is currently shared by all frame contexts. Wait before - // mutating it so an older in-flight frame cannot read the overwritten - // range. This can later be replaced with per-frame material buffers. - gfxDevice.waitIdle(); + // Updates go through the command buffer instead of the mapped pointer: the + // copy then lands after earlier frames finished reading the old values, + // without the CPU having to wait for them. + vkutil::bufferShaderReadToTransferWriteBarrier(cmd, materialBuffer, 0, VK_WHOLE_SIZE); + + for (MaterialID id : pendingMaterialUploads) + { + const MaterialData data = resources->materials().getMaterialData(id); + vkCmdUpdateBuffer( + cmd, + materialBuffer, + static_cast(id) * sizeof(MaterialData), + sizeof(MaterialData), + &data); + } + pendingMaterialUploads.clear(); } - for (MaterialID id : pendingMaterialUploads) - { - resources->materials().updateMaterialGPU(id); - gfxDevice.getMemoryManager().flushAllocation( - resources->materials().getMaterialDataBuffer(), - static_cast(id) * sizeof(MaterialData), - sizeof(MaterialData)); - } - pendingMaterialUploads.clear(); + // newly added materials are written through the mapped pointer, updates by the copies above + vkutil::bufferHostAndTransferWriteToShaderReadBarrier(cmd, materialBuffer, 0, VK_WHOLE_SIZE); } void GameRenderer::createDrawImage(GfxDevice& gfxDevice, @@ -589,8 +585,9 @@ void GameRenderer::createDrawImage(GfxDevice& gfxDevice, .samples = noMsaa, }; - // reuse the same id if creating again - drawImageId = resources->createImage(gfxDevice, createImageInfo, "draw image", nullptr, drawImageId); + // a fresh bindless slot, the old one may still be read by frames in flight + resources->destroyImage(drawImageId); + drawImageId = resources->createImage(gfxDevice, createImageInfo, "draw image"); if (firstCreate) { @@ -609,7 +606,8 @@ void GameRenderer::createDrawImage(GfxDevice& gfxDevice, .samples = noMsaa, }; - depthImageId = resources->createImage(gfxDevice, createInfo, "depth image", nullptr, depthImageId); + resources->destroyImage(depthImageId); + depthImageId = resources->createImage(gfxDevice, createInfo, "depth image"); spdlog::info("Created depth image with id {}", depthImageId); } } diff --git a/destrum/src/Graphics/Resources/Cubemap.cpp b/destrum/src/Graphics/Resources/Cubemap.cpp index 73b0f6c..6904ca2 100644 --- a/destrum/src/Graphics/Resources/Cubemap.cpp +++ b/destrum/src/Graphics/Resources/Cubemap.cpp @@ -262,7 +262,7 @@ void CubeMap::CreateCubeMap( vkDestroyImageView(gfxDevice.getDevice(), view, nullptr); } } - gfxDevice.destroyImage(cubeMapImage); + gfxDevice.destroyImageNow(cubeMapImage); throw; } } diff --git a/destrum/src/Graphics/Swapchain.cpp b/destrum/src/Graphics/Swapchain.cpp index 863aa17..eec741e 100644 --- a/destrum/src/Graphics/Swapchain.cpp +++ b/destrum/src/Graphics/Swapchain.cpp @@ -134,6 +134,8 @@ bool Swapchain::recreateSwapchain( surface = surf; { + // Resizing is rare, and the old swapchain (plus its views and the renderer's + // size-dependent images) can't be destroyed while a present may still use it. ZoneScopedN("vkDeviceWaitIdle"); vkDeviceWaitIdle(vkbDevice); } diff --git a/destrum/src/Graphics/Util.cpp b/destrum/src/Graphics/Util.cpp index 0b77afd..d957fd7 100644 --- a/destrum/src/Graphics/Util.cpp +++ b/destrum/src/Graphics/Util.cpp @@ -165,6 +165,46 @@ void vkutil::bufferHostWriteToShaderReadBarrier( vkCmdPipelineBarrier2(cmd, &dep); } +void vkutil::bufferShaderReadToTransferWriteBarrier( + VkCommandBuffer cmd, VkBuffer buffer, VkDeviceSize offset, VkDeviceSize size) { + VkBufferMemoryBarrier2 barrier{VK_STRUCTURE_TYPE_BUFFER_MEMORY_BARRIER_2}; + barrier.srcStageMask = VK_PIPELINE_STAGE_2_VERTEX_SHADER_BIT | + VK_PIPELINE_STAGE_2_FRAGMENT_SHADER_BIT | + VK_PIPELINE_STAGE_2_COMPUTE_SHADER_BIT; + barrier.srcAccessMask = VK_ACCESS_2_SHADER_READ_BIT; + barrier.dstStageMask = VK_PIPELINE_STAGE_2_TRANSFER_BIT; + barrier.dstAccessMask = VK_ACCESS_2_TRANSFER_WRITE_BIT; + barrier.buffer = buffer; + barrier.offset = offset; + barrier.size = size; + + VkDependencyInfo dep{VK_STRUCTURE_TYPE_DEPENDENCY_INFO}; + dep.bufferMemoryBarrierCount = 1; + dep.pBufferMemoryBarriers = &barrier; + + vkCmdPipelineBarrier2(cmd, &dep); +} + +void vkutil::bufferHostAndTransferWriteToShaderReadBarrier( + VkCommandBuffer cmd, VkBuffer buffer, VkDeviceSize offset, VkDeviceSize size) { + VkBufferMemoryBarrier2 barrier{VK_STRUCTURE_TYPE_BUFFER_MEMORY_BARRIER_2}; + barrier.srcStageMask = VK_PIPELINE_STAGE_2_HOST_BIT | VK_PIPELINE_STAGE_2_TRANSFER_BIT; + barrier.srcAccessMask = VK_ACCESS_2_HOST_WRITE_BIT | VK_ACCESS_2_TRANSFER_WRITE_BIT; + barrier.dstStageMask = VK_PIPELINE_STAGE_2_VERTEX_SHADER_BIT | + VK_PIPELINE_STAGE_2_FRAGMENT_SHADER_BIT | + VK_PIPELINE_STAGE_2_COMPUTE_SHADER_BIT; + barrier.dstAccessMask = VK_ACCESS_2_SHADER_READ_BIT; + barrier.buffer = buffer; + barrier.offset = offset; + barrier.size = size; + + VkDependencyInfo dep{VK_STRUCTURE_TYPE_DEPENDENCY_INFO}; + dep.bufferMemoryBarrierCount = 1; + dep.pBufferMemoryBarriers = &barrier; + + vkCmdPipelineBarrier2(cmd, &dep); +} + void vkutil::bufferHostWriteToTransferReadBarrier( VkCommandBuffer cmd, VkBuffer buffer, diff --git a/lightkeeper/src/Lightkeeper.cpp b/lightkeeper/src/Lightkeeper.cpp index e135f68..d8f5d60 100644 --- a/lightkeeper/src/Lightkeeper.cpp +++ b/lightkeeper/src/Lightkeeper.cpp @@ -171,8 +171,6 @@ void LightKeeper::customDraw() void LightKeeper::customCleanup() { - gfxDevice.waitIdle(); - SceneManager::GetInstance().Destroy(); if (skyboxCubemap)