From ed9b3f67a73df1e723b15ffa33d1d5df533069fb Mon Sep 17 00:00:00 2001 From: catbot Date: Wed, 17 Jun 2026 18:05:13 +0000 Subject: [PATCH] test+usage: device-local geometry readback + isolate #67 staging count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add TRANSFER_SRC to RT geometry usage so device-local geometry (now in VRAM, no longer host-mappable) stays copyable/inspectable. - MeshDecompressStagingRelease: read decompressed vertex/index back via a device->host copy instead of the removed host-mapped .value/FlushHost, and build the mesh with allowUpdate=true so the retained per-mesh scratch (#66) doesn't also land in the deletion queue — isolating the assertion to the released compressed staging (#67). Co-Authored-By: Claude Opus 4.8 --- implementations/Crafter.Graphics-Mesh.cpp | 10 ++++- tests/MeshDecompressStagingRelease/main.cpp | 41 +++++++++++++++++---- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/implementations/Crafter.Graphics-Mesh.cpp b/implementations/Crafter.Graphics-Mesh.cpp index e518f2d..90649a7 100644 --- a/implementations/Crafter.Graphics-Mesh.cpp +++ b/implementations/Crafter.Graphics-Mesh.cpp @@ -32,10 +32,16 @@ using namespace Crafter; namespace { // Buffer-usage flag set shared by both Build paths. The compressed path - // appends VK_BUFFER_USAGE_2_MEMORY_DECOMPRESSION_BIT_EXT. + // appends VK_BUFFER_USAGE_2_MEMORY_DECOMPRESSION_BIT_EXT, and the staged + // upload path appends VK_BUFFER_USAGE_TRANSFER_DST_BIT. TRANSFER_SRC is in + // the base because the geometry is now device-local (#73): once it no longer + // lives in host-mappable memory, a transfer copy is the only way to read it + // back (debugging, GPU-driven workflows, decompression validation) — a free + // usage flag that keeps device-local geometry inspectable. constexpr VkBufferUsageFlags2 kVertexUsageBase = VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT - | VK_BUFFER_USAGE_2_ACCELERATION_STRUCTURE_BUILD_INPUT_READ_ONLY_BIT_KHR; + | VK_BUFFER_USAGE_2_ACCELERATION_STRUCTURE_BUILD_INPUT_READ_ONLY_BIT_KHR + | VK_BUFFER_USAGE_TRANSFER_SRC_BIT; constexpr VkBufferUsageFlags2 kIndexUsageBase = kVertexUsageBase | VK_BUFFER_USAGE_STORAGE_BUFFER_BIT; diff --git a/tests/MeshDecompressStagingRelease/main.cpp b/tests/MeshDecompressStagingRelease/main.cpp index 9f93e77..9298bf7 100644 --- a/tests/MeshDecompressStagingRelease/main.cpp +++ b/tests/MeshDecompressStagingRelease/main.cpp @@ -146,7 +146,13 @@ int main() { Mesh mesh; VkCommandBuffer cmd = BeginCmd(); - mesh.Build(asset, cmd); + // allowUpdate=true so the per-mesh BLAS scratch is RETAINED for refit + // (issue #66) rather than deferred-cleared after the build. A static + // (allowUpdate=false) build would also hand its scratch to the deletion + // queue, making the count below 2; keeping it isolates this test to the + // one allocation it cares about — the released compressed staging (#67). + // The staging-release behavior under test is independent of allowUpdate. + mesh.Build(asset, cmd, RTBuildOptions{ .allowUpdate = true }); // ── The fix: staging is released to the queue at record time, not pinned. ── Check(mesh.compressedStaging.buffer == VK_NULL_HANDLE, @@ -168,16 +174,35 @@ int main() { Check(Device::validationErrorCount == 0, "decompress + BLAS build raised no validation errors"); - // The GPU decompress wrote into the host-visible vertex/index buffers; - // invalidate and read them back — releasing the staging must not corrupt - // the decompressed result. - mesh.vertexBuffer.FlushHost(); - mesh.indexBuffer.FlushHost(); + // The GPU decompress wrote into the device-local vertex/index buffers + // (issue #73 placed RT geometry in VRAM, so they are no longer host-mapped). + // Copy them back to host-visible staging — the proper way to inspect + // device-local memory — and compare: releasing the compressed staging must + // not have corrupted the decompressed result. The geometry buffers carry + // TRANSFER_SRC for exactly this (#73). + VulkanBuffer, true> vertReadback; + VulkanBuffer idxReadback; + vertReadback.Resize(VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT, + VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, static_cast(srcMesh.vertexes.size())); + idxReadback.Resize(VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT, + VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, static_cast(srcMesh.indexes.size())); + { + // Safe to copy without a barrier: SubmitWait above wait-idled the queue, + // so the decompress + BLAS reads of these buffers have fully retired. + VkCommandBuffer rcmd = BeginCmd(); + VkBufferCopy vRegion { .srcOffset = 0, .dstOffset = 0, .size = vertReadback.size }; + vkCmdCopyBuffer(rcmd, mesh.vertexBuffer.buffer, vertReadback.buffer, 1, &vRegion); + VkBufferCopy iRegion { .srcOffset = 0, .dstOffset = 0, .size = idxReadback.size }; + vkCmdCopyBuffer(rcmd, mesh.indexBuffer.buffer, idxReadback.buffer, 1, &iRegion); + SubmitWait(rcmd); + } + vertReadback.FlushHost(); + idxReadback.FlushHost(); const bool vertsMatch = std::memcmp( - mesh.vertexBuffer.value, srcMesh.vertexes.data(), + vertReadback.value, srcMesh.vertexes.data(), srcMesh.vertexes.size() * sizeof(srcMesh.vertexes[0])) == 0; const bool idxMatch = std::memcmp( - mesh.indexBuffer.value, srcMesh.indexes.data(), + idxReadback.value, srcMesh.indexes.data(), srcMesh.indexes.size() * sizeof(srcMesh.indexes[0])) == 0; Check(vertsMatch, "GPU-decompressed vertices are byte-equal to the source"); Check(idxMatch, "GPU-decompressed indices are byte-equal to the source");