From 3ac8c4d95f00ff6591a6e9ece6576706f5231543 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Fri, 3 Apr 2026 19:48:43 -0700 Subject: [PATCH] fix(rendering): wait all frame fences before freeing shared descriptor sets deferAfterFrameFence only waits for one frame slot's fence, but shared resources (material descriptor sets, vertex/index buffers) are bound by both in-flight frames' command buffers. On AMD RADV this caused vkFreeDescriptorSets errors and eventual SIGSEGV. Add deferAfterAllFrameFences: queues to every frame slot with a shared counter so cleanup runs exactly once, after the last slot is fenced. Use it for WMO, terrain, water, and character model shared resources. Per-frame bone sets keep using deferAfterFrameFence (already correct). Also fix character renderer vertex format: R8G8B8A8_UINT -> _SINT to match shader's ivec4 input (RADV validation rejects the mismatch). --- include/rendering/vk_context.hpp | 4 ++++ src/rendering/character_renderer.cpp | 10 +++++----- src/rendering/terrain_renderer.cpp | 4 ++-- src/rendering/vk_context.cpp | 17 +++++++++++++++++ src/rendering/water_renderer.cpp | 4 ++-- src/rendering/wmo_renderer.cpp | 2 +- 6 files changed, 31 insertions(+), 10 deletions(-) diff --git a/include/rendering/vk_context.hpp b/include/rendering/vk_context.hpp index 50c283fc..105f3c68 100644 --- a/include/rendering/vk_context.hpp +++ b/include/rendering/vk_context.hpp @@ -65,6 +65,10 @@ public: // be referenced by command buffers submitted in the previous frame(s), // such as descriptor sets and buffers freed during streaming/unload. void deferAfterFrameFence(std::function&& fn); + // Like deferAfterFrameFence, but waits until ALL in-flight frame slots have + // been fenced — safe for shared resources bound by multiple frames' command + // buffers (material descriptor sets, vertex/index buffers, etc.). + void deferAfterAllFrameFences(std::function&& fn); // Accessors VkInstance getInstance() const { return instance; } diff --git a/src/rendering/character_renderer.cpp b/src/rendering/character_renderer.cpp index 753b2d73..fb04ca7b 100644 --- a/src/rendering/character_renderer.cpp +++ b/src/rendering/character_renderer.cpp @@ -246,7 +246,7 @@ bool CharacterRenderer::initialize(VkContext* ctx, VkDescriptorSetLayout perFram std::vector charAttrs = { {0, 0, VK_FORMAT_R32G32B32_SFLOAT, static_cast(offsetof(CharVertexGPU, position))}, {1, 0, VK_FORMAT_R8G8B8A8_UNORM, static_cast(offsetof(CharVertexGPU, boneWeights))}, - {2, 0, VK_FORMAT_R8G8B8A8_UINT, static_cast(offsetof(CharVertexGPU, boneIndices))}, + {2, 0, VK_FORMAT_R8G8B8A8_SINT, static_cast(offsetof(CharVertexGPU, boneIndices))}, {3, 0, VK_FORMAT_R32G32B32_SFLOAT, static_cast(offsetof(CharVertexGPU, normal))}, {4, 0, VK_FORMAT_R32G32_SFLOAT, static_cast(offsetof(CharVertexGPU, texCoords))}, {5, 0, VK_FORMAT_R32G32B32A32_SFLOAT, static_cast(offsetof(CharVertexGPU, tangent))}, @@ -492,7 +492,7 @@ void CharacterRenderer::destroyModelGPU(M2ModelGPU& gpuModel, bool defer) { if (ib) vmaDestroyBuffer(alloc, ib, ibAlloc); } else if (vb || ib) { // Streaming path: in-flight command buffers may still reference these - vkCtx_->deferAfterFrameFence([alloc, vb, vbAlloc, ib, ibAlloc]() { + vkCtx_->deferAfterAllFrameFences([alloc, vb, vbAlloc, ib, ibAlloc]() { if (vb) vmaDestroyBuffer(alloc, vb, vbAlloc); if (ib) vmaDestroyBuffer(alloc, ib, ibAlloc); }); @@ -2663,7 +2663,7 @@ bool CharacterRenderer::initializeShadow(VkRenderPass shadowRenderPass) { // Character vertex format (CharVertexGPU): stride = 56 bytes // loc 0: vec3 aPos (R32G32B32_SFLOAT, offset 0) // loc 1: vec4 aBoneWeights (R8G8B8A8_UNORM, offset 12) - // loc 2: ivec4 aBoneIndices (R8G8B8A8_UINT, offset 16) + // loc 2: ivec4 aBoneIndices (R8G8B8A8_SINT, offset 16) // loc 3: vec2 aTexCoord (R32G32_SFLOAT, offset 32) VkVertexInputBindingDescription vertBind{}; vertBind.binding = 0; @@ -2672,7 +2672,7 @@ bool CharacterRenderer::initializeShadow(VkRenderPass shadowRenderPass) { std::vector vertAttrs = { {0, 0, VK_FORMAT_R32G32B32_SFLOAT, static_cast(offsetof(CharVertexGPU, position))}, {1, 0, VK_FORMAT_R8G8B8A8_UNORM, static_cast(offsetof(CharVertexGPU, boneWeights))}, - {2, 0, VK_FORMAT_R8G8B8A8_UINT, static_cast(offsetof(CharVertexGPU, boneIndices))}, + {2, 0, VK_FORMAT_R8G8B8A8_SINT, static_cast(offsetof(CharVertexGPU, boneIndices))}, {3, 0, VK_FORMAT_R32G32_SFLOAT, static_cast(offsetof(CharVertexGPU, texCoords))}, }; @@ -3336,7 +3336,7 @@ void CharacterRenderer::recreatePipelines() { std::vector charAttrs = { {0, 0, VK_FORMAT_R32G32B32_SFLOAT, static_cast(offsetof(CharVertexGPU, position))}, {1, 0, VK_FORMAT_R8G8B8A8_UNORM, static_cast(offsetof(CharVertexGPU, boneWeights))}, - {2, 0, VK_FORMAT_R8G8B8A8_UINT, static_cast(offsetof(CharVertexGPU, boneIndices))}, + {2, 0, VK_FORMAT_R8G8B8A8_SINT, static_cast(offsetof(CharVertexGPU, boneIndices))}, {3, 0, VK_FORMAT_R32G32B32_SFLOAT, static_cast(offsetof(CharVertexGPU, normal))}, {4, 0, VK_FORMAT_R32G32_SFLOAT, static_cast(offsetof(CharVertexGPU, texCoords))}, {5, 0, VK_FORMAT_R32G32B32A32_SFLOAT, static_cast(offsetof(CharVertexGPU, tangent))}, diff --git a/src/rendering/terrain_renderer.cpp b/src/rendering/terrain_renderer.cpp index 0fe8816e..0de9698a 100644 --- a/src/rendering/terrain_renderer.cpp +++ b/src/rendering/terrain_renderer.cpp @@ -1061,8 +1061,8 @@ void TerrainRenderer::destroyChunkGPU(TerrainChunkGPU& chunk) { chunk.materialSet = VK_NULL_HANDLE; chunk.ownedAlphaTextures.clear(); - vkCtx->deferAfterFrameFence([device, allocator, vertexBuffer, vertexAlloc, indexBuffer, indexAlloc, - paramsUBO, paramsAlloc, pool, materialSet, alphaTextures]() { + vkCtx->deferAfterAllFrameFences([device, allocator, vertexBuffer, vertexAlloc, indexBuffer, indexAlloc, + paramsUBO, paramsAlloc, pool, materialSet, alphaTextures]() { if (vertexBuffer) { AllocatedBuffer ab{}; ab.buffer = vertexBuffer; ab.allocation = vertexAlloc; destroyBuffer(allocator, ab); diff --git a/src/rendering/vk_context.cpp b/src/rendering/vk_context.cpp index 546870b6..5b1fdc1c 100644 --- a/src/rendering/vk_context.cpp +++ b/src/rendering/vk_context.cpp @@ -194,6 +194,23 @@ void VkContext::deferAfterFrameFence(std::function&& fn) { deferredCleanup_[currentFrame].push_back(std::move(fn)); } +void VkContext::deferAfterAllFrameFences(std::function&& fn) { + // Shared resources (material descriptor sets, vertex/index buffers) are + // bound by every in-flight frame's command buffer. deferAfterFrameFence + // only waits for ONE slot's fence — the other slot may still be executing. + // Add to every slot; a shared counter ensures the lambda runs exactly once, + // after the LAST slot has been fenced. + auto counter = std::make_shared(MAX_FRAMES_IN_FLIGHT); + auto sharedFn = std::make_shared>(std::move(fn)); + for (uint32_t i = 0; i < MAX_FRAMES_IN_FLIGHT; i++) { + deferredCleanup_[i].push_back([counter, sharedFn]() { + if (--(*counter) == 0) { + (*sharedFn)(); + } + }); + } +} + void VkContext::runDeferredCleanup(uint32_t frameIndex) { auto& q = deferredCleanup_[frameIndex]; if (q.empty()) return; diff --git a/src/rendering/water_renderer.cpp b/src/rendering/water_renderer.cpp index c60b2307..4870e16d 100644 --- a/src/rendering/water_renderer.cpp +++ b/src/rendering/water_renderer.cpp @@ -1405,8 +1405,8 @@ void WaterRenderer::destroyWaterMesh(WaterSurface& surface) { surface.materialAlloc = VK_NULL_HANDLE; surface.materialSet = VK_NULL_HANDLE; - vkCtx->deferAfterFrameFence([device, allocator, vertexBuffer, vertexAlloc, indexBuffer, indexAlloc, - materialUBO, materialAlloc, pool, materialSet]() { + vkCtx->deferAfterAllFrameFences([device, allocator, vertexBuffer, vertexAlloc, indexBuffer, indexAlloc, + materialUBO, materialAlloc, pool, materialSet]() { if (vertexBuffer) { AllocatedBuffer ab{}; ab.buffer = vertexBuffer; ab.allocation = vertexAlloc; destroyBuffer(allocator, ab); diff --git a/src/rendering/wmo_renderer.cpp b/src/rendering/wmo_renderer.cpp index 7eaae82a..79c830ee 100644 --- a/src/rendering/wmo_renderer.cpp +++ b/src/rendering/wmo_renderer.cpp @@ -1972,7 +1972,7 @@ void WMORenderer::destroyGroupGPU(GroupResources& group, bool defer) { } VkDescriptorPool pool = materialDescPool_; - vkCtx_->deferAfterFrameFence([device, allocator, pool, vb, vbAlloc, ib, ibAlloc, + vkCtx_->deferAfterAllFrameFences([device, allocator, pool, vb, vbAlloc, ib, ibAlloc, mats = std::move(mats)]() { if (vb) vmaDestroyBuffer(allocator, vb, vbAlloc); if (ib) vmaDestroyBuffer(allocator, ib, ibAlloc);