From ac5c61203deb8f820cc6efb4596e5fd7a06f56b2 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Fri, 3 Apr 2026 18:30:52 -0700 Subject: [PATCH] fix(rendering): defer descriptor set destruction during streaming unload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M2 destroyInstanceBones and WMO destroyGroupGPU freed descriptor sets and buffers immediately during tile streaming, while in-flight command buffers still referenced them — causing DEVICE_LOST on AMD RADV. Now defers GPU resource destruction via deferAfterFrameFence in streaming paths (removeInstance, removeInstances, unloadModel). Immediate destruction preserved for shutdown/clear paths that vkDeviceWaitIdle first. Also: vkDeviceWaitIdle before WMO backfillNormalMaps descriptor rebinds, and fillModeNonSolid added to required device features for wireframe pipelines on AMD. --- include/rendering/m2_renderer.hpp | 6 ++- include/rendering/wmo_renderer.hpp | 6 ++- src/rendering/m2_renderer.cpp | 48 +++++++++++++------ src/rendering/vk_context.cpp | 1 + src/rendering/wmo_renderer.cpp | 75 ++++++++++++++++++++++-------- 5 files changed, 97 insertions(+), 39 deletions(-) diff --git a/include/rendering/m2_renderer.hpp b/include/rendering/m2_renderer.hpp index 6ee5c06d..dbeeeae8 100644 --- a/include/rendering/m2_renderer.hpp +++ b/include/rendering/m2_renderer.hpp @@ -641,8 +641,10 @@ private: // Helper to destroy model GPU resources void destroyModelGPU(M2ModelGPU& model); - // Helper to destroy instance bone buffers - void destroyInstanceBones(M2Instance& inst); + // Helper to destroy instance bone buffers. + // When defer=true, destruction is scheduled via deferAfterFrameFence so + // in-flight command buffers are not invalidated (use for streaming unload). + void destroyInstanceBones(M2Instance& inst, bool defer = false); }; } // namespace rendering diff --git a/include/rendering/wmo_renderer.hpp b/include/rendering/wmo_renderer.hpp index e69d8e59..33eae7da 100644 --- a/include/rendering/wmo_renderer.hpp +++ b/include/rendering/wmo_renderer.hpp @@ -599,9 +599,11 @@ private: VkDescriptorSet allocateMaterialSet(); /** - * Destroy GPU resources for a single group + * Destroy GPU resources for a single group. + * When defer=true, destruction is scheduled via deferAfterFrameFence + * so in-flight command buffers are not invalidated. */ - void destroyGroupGPU(GroupResources& group); + void destroyGroupGPU(GroupResources& group, bool defer = false); struct GridCell { int x; diff --git a/src/rendering/m2_renderer.cpp b/src/rendering/m2_renderer.cpp index c121da85..456acb7e 100644 --- a/src/rendering/m2_renderer.cpp +++ b/src/rendering/m2_renderer.cpp @@ -844,23 +844,41 @@ void M2Renderer::destroyModelGPU(M2ModelGPU& model) { model.ribbonTexSets.clear(); } -void M2Renderer::destroyInstanceBones(M2Instance& inst) { +void M2Renderer::destroyInstanceBones(M2Instance& inst, bool defer) { if (!vkCtx_) return; VkDevice device = vkCtx_->getDevice(); VmaAllocator alloc = vkCtx_->getAllocator(); for (int i = 0; i < 2; i++) { - // Free bone descriptor set so the pool slot is immediately reusable. - // Without this, the pool fills up over a play session as tiles stream - // in/out, eventually causing vkAllocateDescriptorSets to fail and - // making animated instances invisible (perceived as flickering). - if (inst.boneSet[i] != VK_NULL_HANDLE) { - vkFreeDescriptorSets(device, boneDescPool_, 1, &inst.boneSet[i]); - inst.boneSet[i] = VK_NULL_HANDLE; - } - if (inst.boneBuffer[i]) { - vmaDestroyBuffer(alloc, inst.boneBuffer[i], inst.boneAlloc[i]); - inst.boneBuffer[i] = VK_NULL_HANDLE; - inst.boneMapped[i] = nullptr; + // Snapshot handles before clearing the instance — needed for both + // immediate and deferred paths. + VkDescriptorSet boneSet = inst.boneSet[i]; + ::VkBuffer boneBuf = inst.boneBuffer[i]; + VmaAllocation boneAlloc = inst.boneAlloc[i]; + inst.boneSet[i] = VK_NULL_HANDLE; + inst.boneBuffer[i] = VK_NULL_HANDLE; + inst.boneMapped[i] = nullptr; + + if (!defer) { + // Immediate destruction (safe after vkDeviceWaitIdle) + if (boneSet != VK_NULL_HANDLE) { + vkFreeDescriptorSets(device, boneDescPool_, 1, &boneSet); + } + if (boneBuf) { + vmaDestroyBuffer(alloc, boneBuf, boneAlloc); + } + } else if (boneSet != VK_NULL_HANDLE || boneBuf) { + // Deferred destruction — previous frame's command buffer may still + // reference these descriptor sets and buffers. + VkDescriptorPool pool = boneDescPool_; + vkCtx_->deferAfterFrameFence([device, alloc, pool, boneSet, boneBuf, boneAlloc]() { + if (boneSet != VK_NULL_HANDLE) { + VkDescriptorSet s = boneSet; + vkFreeDescriptorSets(device, pool, 1, &s); + } + if (boneBuf) { + vmaDestroyBuffer(alloc, boneBuf, boneAlloc); + } + }); } } } @@ -3901,7 +3919,7 @@ void M2Renderer::removeInstance(uint32_t instanceId) { instanceDedupMap_.erase(dk); } - destroyInstanceBones(inst); + destroyInstanceBones(inst, /*defer=*/true); // Swap-remove: move last element to the hole and pop_back to avoid O(n) shift instanceIndexById.erase(instanceId); @@ -3951,7 +3969,7 @@ void M2Renderer::removeInstances(const std::vector& instanceIds) { const size_t oldSize = instances.size(); for (auto& inst : instances) { if (toRemove.count(inst.id)) { - destroyInstanceBones(inst); + destroyInstanceBones(inst, /*defer=*/true); } } instances.erase(std::remove_if(instances.begin(), instances.end(), diff --git a/src/rendering/vk_context.cpp b/src/rendering/vk_context.cpp index 3a1a0d44..546870b6 100644 --- a/src/rendering/vk_context.cpp +++ b/src/rendering/vk_context.cpp @@ -290,6 +290,7 @@ bool VkContext::selectPhysicalDevice() { vkb::PhysicalDeviceSelector selector{vkbInstance_}; VkPhysicalDeviceFeatures requiredFeatures{}; requiredFeatures.samplerAnisotropy = VK_TRUE; + requiredFeatures.fillModeNonSolid = VK_TRUE; // wireframe debug pipelines selector.set_surface(surface) .set_minimum_version(1, 1) .set_required_features(requiredFeatures) diff --git a/src/rendering/wmo_renderer.cpp b/src/rendering/wmo_renderer.cpp index 0396868a..7eaae82a 100644 --- a/src/rendering/wmo_renderer.cpp +++ b/src/rendering/wmo_renderer.cpp @@ -827,9 +827,10 @@ void WMORenderer::unloadModel(uint32_t id) { return; } - // Free GPU resources + // Free GPU resources — defer because in-flight command buffers may + // still reference this model's vertex/index buffers and descriptors. for (auto& group : it->second.groups) { - destroyGroupGPU(group); + destroyGroupGPU(group, /*defer=*/true); } loadedModels.erase(it); @@ -1925,33 +1926,64 @@ bool WMORenderer::createGroupResources(const pipeline::WMOGroup& group, GroupRes // renderGroup removed — draw calls are inlined in render() -void WMORenderer::destroyGroupGPU(GroupResources& group) { +void WMORenderer::destroyGroupGPU(GroupResources& group, bool defer) { if (!vkCtx_) return; + VkDevice device = vkCtx_->getDevice(); VmaAllocator allocator = vkCtx_->getAllocator(); - if (group.vertexBuffer) { - vmaDestroyBuffer(allocator, group.vertexBuffer, group.vertexAlloc); + if (!defer) { + // Immediate destruction (safe after vkDeviceWaitIdle) + if (group.vertexBuffer) { + vmaDestroyBuffer(allocator, group.vertexBuffer, group.vertexAlloc); + group.vertexBuffer = VK_NULL_HANDLE; + } + if (group.indexBuffer) { + vmaDestroyBuffer(allocator, group.indexBuffer, group.indexAlloc); + group.indexBuffer = VK_NULL_HANDLE; + } + for (auto& mb : group.mergedBatches) { + if (mb.materialSet) { + vkFreeDescriptorSets(device, materialDescPool_, 1, &mb.materialSet); + mb.materialSet = VK_NULL_HANDLE; + } + if (mb.materialUBO) { + vmaDestroyBuffer(allocator, mb.materialUBO, mb.materialUBOAlloc); + mb.materialUBO = VK_NULL_HANDLE; + } + } + } else { + // Deferred destruction — previous frame's command buffer may still + // reference these buffers and descriptor sets. + ::VkBuffer vb = group.vertexBuffer; + VmaAllocation vbAlloc = group.vertexAlloc; + ::VkBuffer ib = group.indexBuffer; + VmaAllocation ibAlloc = group.indexAlloc; group.vertexBuffer = VK_NULL_HANDLE; - group.vertexAlloc = VK_NULL_HANDLE; - } - if (group.indexBuffer) { - vmaDestroyBuffer(allocator, group.indexBuffer, group.indexAlloc); group.indexBuffer = VK_NULL_HANDLE; - group.indexAlloc = VK_NULL_HANDLE; - } - // Destroy material UBOs and free descriptor sets back to pool - VkDevice device = vkCtx_->getDevice(); - for (auto& mb : group.mergedBatches) { - if (mb.materialSet) { - vkFreeDescriptorSets(device, materialDescPool_, 1, &mb.materialSet); + // Snapshot material handles (::VkBuffer = raw Vulkan handle, not RAII wrapper) + struct MatSnapshot { VkDescriptorSet set; ::VkBuffer ubo; VmaAllocation uboAlloc; }; + std::vector mats; + mats.reserve(group.mergedBatches.size()); + for (auto& mb : group.mergedBatches) { + mats.push_back({mb.materialSet, mb.materialUBO, mb.materialUBOAlloc}); mb.materialSet = VK_NULL_HANDLE; - } - if (mb.materialUBO) { - vmaDestroyBuffer(allocator, mb.materialUBO, mb.materialUBOAlloc); mb.materialUBO = VK_NULL_HANDLE; - mb.materialUBOAlloc = VK_NULL_HANDLE; } + + VkDescriptorPool pool = materialDescPool_; + vkCtx_->deferAfterFrameFence([device, allocator, pool, vb, vbAlloc, ib, ibAlloc, + mats = std::move(mats)]() { + if (vb) vmaDestroyBuffer(allocator, vb, vbAlloc); + if (ib) vmaDestroyBuffer(allocator, ib, ibAlloc); + for (auto& m : mats) { + if (m.set) { + VkDescriptorSet s = m.set; + vkFreeDescriptorSets(device, pool, 1, &s); + } + if (m.ubo) vmaDestroyBuffer(allocator, m.ubo, m.uboAlloc); + } + }); } } @@ -2467,6 +2499,9 @@ void WMORenderer::backfillNormalMaps() { if (generated > 0) { VkDevice device = vkCtx_->getDevice(); + // Wait for in-flight command buffers before updating descriptor sets — + // the previous frame may still reference them via binding 2. + vkDeviceWaitIdle(device); int rebound = 0; // Update merged batches: assign normal map pointer and rebind descriptor set for (auto& [modelId, model] : loadedModels) {