diff --git a/implementations/Crafter.Graphics-RenderingElement3D.cpp b/implementations/Crafter.Graphics-RenderingElement3D.cpp index 21359d8..78c24ee 100644 --- a/implementations/Crafter.Graphics-RenderingElement3D.cpp +++ b/implementations/Crafter.Graphics-RenderingElement3D.cpp @@ -9,6 +9,40 @@ import std; using namespace Crafter; +TlasWithBuffer::TlasWithBuffer(TlasWithBuffer&& other) + : address(std::exchange(other.address, 0)) + , buffer(std::move(other.buffer)) + , accelerationStructure(std::exchange(other.accelerationStructure, VK_NULL_HANDLE)) + , instanceBuffer(std::move(other.instanceBuffer)) + , scratchBuffer(std::move(other.scratchBuffer)) + , metadataBuffer(std::move(other.metadataBuffer)) + , builtInstanceCount(other.builtInstanceCount) + , builtFlags(other.builtFlags) + , uploadedVersion(std::move(other.uploadedVersion)) { +} + +TlasWithBuffer& TlasWithBuffer::operator=(TlasWithBuffer&& other) { + if (this == &other) return *this; + // Ahead of buffer, which it lives in and which queues its own as it + // takes other's over. + Device::EnqueueDeletion(accelerationStructure); + address = std::exchange(other.address, 0); + buffer = std::move(other.buffer); + accelerationStructure = std::exchange(other.accelerationStructure, VK_NULL_HANDLE); + instanceBuffer = std::move(other.instanceBuffer); + scratchBuffer = std::move(other.scratchBuffer); + metadataBuffer = std::move(other.metadataBuffer); + builtInstanceCount = other.builtInstanceCount; + builtFlags = other.builtFlags; + uploadedVersion = std::move(other.uploadedVersion); + return *this; +} + +TlasWithBuffer::~TlasWithBuffer() { + // The body runs before the members' destructors, so the handle is queued + // ahead of buffer, which it lives in. + Device::EnqueueDeletion(accelerationStructure); +} std::vector RenderingElement3D::elements; @@ -50,8 +84,8 @@ void RenderingElement3D::BuildTLAS(VkCommandBuffer cmd, std::uint32_t index, RTB // Refit (UPDATE) is allowed when the count matches the count this AS // was last built for and the build flags are unchanged. A change forces - // a full rebuild because the AS storage and instance buffer were sized - // for the old count (and a refit must inherit the original build flags). + // a full rebuild, since a refit must keep the primitive count and the + // build flags of the build it updates. // Refit is dramatically cheaper at scale (millions of instances) — it // walks the existing BVH and updates AABBs rather than reconstructing // topology. @@ -60,17 +94,8 @@ void RenderingElement3D::BuildTLAS(VkCommandBuffer cmd, std::uint32_t index, RTB || primitiveCount != tlas.builtInstanceCount || buildFlags != tlas.builtFlags; - { - VkMemoryBarrier asBarrier { - .sType = VK_STRUCTURE_TYPE_MEMORY_BARRIER, - .srcAccessMask = VK_ACCESS_ACCELERATION_STRUCTURE_WRITE_BIT_KHR, - .dstAccessMask = VK_ACCESS_ACCELERATION_STRUCTURE_READ_BIT_KHR | VK_ACCESS_ACCELERATION_STRUCTURE_WRITE_BIT_KHR - }; - vkCmdPipelineBarrier(cmd, - VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR, - VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR, - 0, 1, &asBarrier, 0, nullptr, 0, nullptr); - } + const TlasBarrier writeBarrier = BuildTlasWriteBarrier(); + vkCmdPipelineBarrier(cmd, writeBarrier.srcStage, writeBarrier.dstStage, 0, 1, &writeBarrier.memory, 0, nullptr, 0, nullptr); if (topologyChanged) { // Grow the host-visible inputs on a high-water mark rather than @@ -81,8 +106,9 @@ void RenderingElement3D::BuildTLAS(VkCommandBuffer cmd, std::uint32_t index, RTB // over from a larger earlier frame is reused as-is. This drops the // two host-buffer reallocations on a count decrease (and on an // increase that still fits the previous high-water capacity), - // leaving only the AS storage + scratch rebuild below, which is tied - // to the AS itself and dominates this path regardless. + // leaving only the AS build below, which dominates this path + // regardless; the AS storage and scratch follow a high-water mark + // of their own. // // instanceBuffer and metadataBuffer grow in lockstep (always resized // together to the same entry count), so one capacity check covers @@ -241,7 +267,7 @@ void RenderingElement3D::BuildTLAS(VkCommandBuffer cmd, std::uint32_t index, RTB }; if (topologyChanged) { - // Query sizes for the fresh build, allocate AS storage + scratch. + // Query sizes for the fresh build, and make room for it. VkAccelerationStructureBuildSizesInfoKHR tlasBuildSizes { .sType = VK_STRUCTURE_TYPE_ACCELERATION_STRUCTURE_BUILD_SIZES_INFO_KHR }; @@ -258,36 +284,50 @@ void RenderingElement3D::BuildTLAS(VkCommandBuffer cmd, std::uint32_t index, RTB // An empty TLAS may need no scratch at all, but the buffer can't be // zero-size. tlas.scratchBuffer.Resize(VK_BUFFER_USAGE_STORAGE_BUFFER_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, std::max(tlasBuildSizes.buildScratchSize, 1)); - tlas.buffer.Resize(VK_BUFFER_USAGE_ACCELERATION_STRUCTURE_STORAGE_BIT_KHR | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT | VK_BUFFER_USAGE_ACCELERATION_STRUCTURE_BUILD_INPUT_READ_ONLY_BIT_KHR, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, tlasBuildSizes.accelerationStructureSize); - // Destroy the previous AS handle before creating a new one — the - // pre-refit path leaked here on every frame. - if (tlas.accelerationStructure != VK_NULL_HANDLE) { - Device::vkDestroyAccelerationStructureKHR(Device::device, tlas.accelerationStructure, nullptr); + // The acceleration structure spans all of its storage, so a build + // that fits the storage builds into it again: a BUILD may write any + // acceleration structure created at least as large as it needs. The + // handle and address then stay the same across count and flag + // changes, and nothing is queued. Only a TLAS that outgrows its + // storage gets new storage and a new acceleration structure. + if (tlas.accelerationStructure == VK_NULL_HANDLE + || tlasBuildSizes.accelerationStructureSize > tlas.buffer.capacity) { + // The old one may still be used earlier in this command buffer, + // by an earlier build of this index or a trace of it, so it's + // queued rather than destroyed; ahead of the storage it lives in, + // which the Resize below queues. Nothing in flight uses it (see + // BuildTLAS's precondition). The address goes with it, so a + // Resize or create that throws doesn't leave the address of a + // queued acceleration structure behind. + Device::EnqueueDeletion(tlas.accelerationStructure); tlas.accelerationStructure = VK_NULL_HANDLE; + tlas.address = 0; + + tlas.buffer.Resize(VK_BUFFER_USAGE_ACCELERATION_STRUCTURE_STORAGE_BIT_KHR | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT | VK_BUFFER_USAGE_ACCELERATION_STRUCTURE_BUILD_INPUT_READ_ONLY_BIT_KHR, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, tlasBuildSizes.accelerationStructureSize); + + VkAccelerationStructureCreateInfoKHR tlasCreateInfo { + .sType = VK_STRUCTURE_TYPE_ACCELERATION_STRUCTURE_CREATE_INFO_KHR, + .buffer = tlas.buffer.buffer, + .offset = 0, + .size = tlas.buffer.capacity, + .type = VK_ACCELERATION_STRUCTURE_TYPE_TOP_LEVEL_KHR, + }; + Device::CheckVkResult(Device::vkCreateAccelerationStructureKHR(Device::device, &tlasCreateInfo, nullptr, &tlas.accelerationStructure)); + + VkAccelerationStructureDeviceAddressInfoKHR addrInfo { + .sType = VK_STRUCTURE_TYPE_ACCELERATION_STRUCTURE_DEVICE_ADDRESS_INFO_KHR, + .accelerationStructure = tlas.accelerationStructure + }; + tlas.address = Device::vkGetAccelerationStructureDeviceAddressKHR(Device::device, &addrInfo); } - VkAccelerationStructureCreateInfoKHR tlasCreateInfo { - .sType = VK_STRUCTURE_TYPE_ACCELERATION_STRUCTURE_CREATE_INFO_KHR, - .buffer = tlas.buffer.buffer, - .offset = 0, - .size = tlasBuildSizes.accelerationStructureSize, - .type = VK_ACCELERATION_STRUCTURE_TYPE_TOP_LEVEL_KHR, - }; - Device::CheckVkResult(Device::vkCreateAccelerationStructureKHR(Device::device, &tlasCreateInfo, nullptr, &tlas.accelerationStructure)); - - VkAccelerationStructureDeviceAddressInfoKHR addrInfo { - .sType = VK_STRUCTURE_TYPE_ACCELERATION_STRUCTURE_DEVICE_ADDRESS_INFO_KHR, - .accelerationStructure = tlas.accelerationStructure - }; - tlas.address = Device::vkGetAccelerationStructureDeviceAddressKHR(Device::device, &addrInfo); - tlas.builtInstanceCount = primitiveCount; tlas.builtFlags = buildFlags; } // For UPDATE mode, src == dst (in-place refit). For BUILD, src is - // VK_NULL_HANDLE and dst is the freshly-created handle. + // VK_NULL_HANDLE and dst is the current handle, kept or fresh. tlasBuildGeometryInfo.scratchData.deviceAddress = tlas.scratchBuffer.address; tlasBuildGeometryInfo.dstAccelerationStructure = tlas.accelerationStructure; tlasBuildGeometryInfo.srcAccelerationStructure = @@ -302,6 +342,6 @@ void RenderingElement3D::BuildTLAS(VkCommandBuffer cmd, std::uint32_t index, RTB VkAccelerationStructureBuildRangeInfoKHR* tlasRangeInfoPP = &tlasRangeInfo; Device::vkCmdBuildAccelerationStructuresKHR(cmd, 1, &tlasBuildGeometryInfo, &tlasRangeInfoPP); - const TlasReadBarrier readBarrier = BuildTlasReadBarrier(); + const TlasBarrier readBarrier = BuildTlasReadBarrier(); vkCmdPipelineBarrier(cmd, readBarrier.srcStage, readBarrier.dstStage, 0, 1, &readBarrier.memory, 0, nullptr, 0, nullptr); } \ No newline at end of file diff --git a/interfaces/Crafter.Graphics-RenderingElement3D.cppm b/interfaces/Crafter.Graphics-RenderingElement3D.cppm index 576736a..d180a25 100644 --- a/interfaces/Crafter.Graphics-RenderingElement3D.cppm +++ b/interfaces/Crafter.Graphics-RenderingElement3D.cppm @@ -34,7 +34,8 @@ export namespace Crafter { // (UPDATE mode) which is dramatically cheaper than a full rebuild // — refit walks the existing BVH and updates AABBs, while rebuild // reconstructs the topology from scratch. A change in count forces - // a fresh rebuild because the AS is sized for that primitive count. + // a fresh rebuild, since a refit must keep the primitive count of + // the build it updates. std::uint32_t builtInstanceCount = 0; // Build flags the current AS was created with — ALLOW_UPDATE (always // set, since BuildTLAS refits in place) plus the chosen @@ -53,6 +54,19 @@ export namespace Crafter { // each slot (and may have reallocated the buffers). See // RenderingElement3D::hostDataVersion. std::vector uploadedVersion; + + TlasWithBuffer() = default; + // Moving hands the TLAS over: the moved-from TlasWithBuffer keeps no + // acceleration structure, so only the last owner releases it, and its + // next BuildTLAS builds a new one. Assigning queues the acceleration + // structure this one held, as the destructor would; assign an empty + // TlasWithBuffer to reset one. + TlasWithBuffer(TlasWithBuffer&& other); + TlasWithBuffer& operator=(TlasWithBuffer&& other); + // Queues the acceleration structure for deferred deletion, ahead of + // the storage it lives in (the buffers queue themselves): a command + // buffer still being recorded or a frame in flight may still use it. + ~TlasWithBuffer(); }; class RenderingElement3D { @@ -133,6 +147,21 @@ export namespace Crafter { // is not exposed as an option. Changing `preference` between frames // forces a full rebuild on the next call, since a refit must inherit // the original build's flags. + // + // The build writes tlases[index] in place: its instance and metadata + // buffers on the host as it records, its acceleration structure and + // scratch on the GPU. So call it only once every earlier submission + // that used tlases[index] has completed. Window::Render runs onUpdate + // after waiting for the fence of the image it renders, so building + // tlases[currentBuffer] there is safe. Earlier builds, traces and + // compute ray queries in the same command buffer are ordered before + // the build by the build itself. + // + // A rebuild for a new instance count or preference builds into the + // same acceleration structure while it fits its storage, so + // tlases[index].address stays the same. A TLAS that outgrows its + // storage moves: read the address again after a build that grows the + // scene. static void BuildTLAS(VkCommandBuffer cmd, std::uint32_t index, RTBuildPreference preference = RTBuildPreference::FastTrace); // Register / unregister with `elements`. Use these instead of touching @@ -142,18 +171,42 @@ export namespace Crafter { static void Remove(RenderingElement3D* e); }; - // The dependency BuildTLAS records after its build, ordering it before - // the shaders that read the TLAS later in the same command buffer: - // traceRayEXT in a ray-tracing pass, rayQueryEXT in a compute pass. It - // has to carry the acceleration-structure memory dependency; ordering - // execution alone never makes the build's writes visible to those reads. - struct TlasReadBarrier { + // A dependency BuildTLAS records around its build. + struct TlasBarrier { VkPipelineStageFlags srcStage; VkPipelineStageFlags dstStage; VkMemoryBarrier memory; }; - inline TlasReadBarrier BuildTlasReadBarrier() { + // The dependency BuildTLAS records before its build, ordering the build + // after the earlier work in the same command buffer that it must follow: + // BLAS builds whose results it reads, an earlier build of this TLAS, + // whose storage and scratch it writes again, and traces and compute ray + // queries of the TLAS, which a refit or a rebuild in place would + // otherwise overwrite while they read it. + inline TlasBarrier BuildTlasWriteBarrier() { + return { + .srcStage = VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR | VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR | VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT, + .dstStage = VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR, + .memory = { + .sType = VK_STRUCTURE_TYPE_MEMORY_BARRIER, + // Writes only: the reads need just the execution dependency + // the stages give them. A read bit here does nothing, except + // that synchronization validation (layers 1.4.363) then + // reports the second of two BuildTLAS calls in one command + // buffer as a write-after-write hazard. + .srcAccessMask = VK_ACCESS_ACCELERATION_STRUCTURE_WRITE_BIT_KHR, + .dstAccessMask = VK_ACCESS_ACCELERATION_STRUCTURE_READ_BIT_KHR | VK_ACCESS_ACCELERATION_STRUCTURE_WRITE_BIT_KHR, + }, + }; + } + + // The dependency BuildTLAS records after its build, ordering it before + // the shaders that read the TLAS later in the same command buffer: + // traceRayEXT in a ray-tracing pass, rayQueryEXT in a compute pass. It + // has to carry the acceleration-structure memory dependency; ordering + // execution alone never makes the build's writes visible to those reads. + inline TlasBarrier BuildTlasReadBarrier() { return { .srcStage = VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR, .dstStage = VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR | VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT, diff --git a/project.cpp b/project.cpp index fd12733..bf84248 100644 --- a/project.cpp +++ b/project.cpp @@ -437,16 +437,27 @@ extern "C" Configuration CrafterBuildProject(std::span a // instance counts — and asserts that a shrink (and any growth within // the high-water capacity) reuses the existing allocation while a // growth past it reallocates, with the validation layer reporting no - // errors when an oversized instance buffer is fed to the build. Needs a - // Vulkan RT device at runtime, so it shares the native build settings. + // errors when an oversized instance buffer is fed to the build. The + // acceleration structure follows the same high-water mark: a rebuild + // that fits its storage builds into the same handle. Needs a Vulkan + // RT device at runtime, so it shares the native build settings. cfg.tests.push_back(LibraryTest("TLASHighWaterMark")); - // The barrier BuildTLAS records after the build carries the + // The barriers BuildTLAS records around the build: the one before it + // orders earlier builds, traces and compute ray queries before the + // build overwrites the TLAS, and the one after it carries the // acceleration-structure memory dependency to the shaders that read - // the TLAS, not just execution order. BuildTlasReadBarrier is pure - // CPU logic, so no GPU device is needed at runtime. + // the TLAS, not just execution order. BuildTlasWriteBarrier and + // BuildTlasReadBarrier are pure CPU logic, so no GPU device is needed + // at runtime. cfg.tests.push_back(LibraryTest("TLASBuildBarrier")); + // A TlasWithBuffer queues its acceleration structure for deferred + // deletion, ahead of its storage, when it is destroyed or assigned + // over, and moving one hands the acceleration structure over. + // Headless; needs a Vulkan RT device at runtime, like MeshLifetime. + cfg.tests.push_back(LibraryTest("TLASLifetime")); + // BuildTLAS on an empty scene (no elements registered yet) builds a // valid empty TLAS instead of creating zero-size buffers, and adding // and removing instances afterwards still rebuilds it. Needs a Vulkan diff --git a/tests/TLASBuildBarrier/main.cpp b/tests/TLASBuildBarrier/main.cpp index f57b8e2..587d60a 100644 --- a/tests/TLASBuildBarrier/main.cpp +++ b/tests/TLASBuildBarrier/main.cpp @@ -1,15 +1,23 @@ //SPDX-License-Identifier: LGPL-3.0-only //SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® -// The dependency RenderingElement3D::BuildTLAS records after its build, which -// orders the build before the shaders that read the TLAS: traceRayEXT in a -// ray-tracing pass, rayQueryEXT in a compute pass, recorded after it in the -// same command buffer (BuildTLAS in onUpdate, then RTPass, is the documented -// per-frame flow). It used to be an execution-only barrier with no memory -// barrier, which orders the commands but never makes the build's -// acceleration-structure writes available and visible to those reads. The -// dependency is built by BuildTlasReadBarrier, pure CPU logic that this test -// drives directly with no GPU device, like SwapchainBarrierScope. +// The dependencies RenderingElement3D::BuildTLAS records around its build. +// +// After it, the build is ordered before the shaders that read the TLAS: +// traceRayEXT in a ray-tracing pass, rayQueryEXT in a compute pass, recorded +// after it in the same command buffer (BuildTLAS in onUpdate, then RTPass, is +// the documented per-frame flow). It used to be an execution-only barrier with +// no memory barrier, which orders the commands but never makes the build's +// acceleration-structure writes available and visible to those reads. +// +// Before it, the build is ordered after the earlier work it must follow. That +// used to cover only earlier acceleration-structure builds, so a refit, or a +// rebuild into the same storage, recorded after a trace or a compute ray query +// of the TLAS could overwrite it while they still read it. +// +// The dependencies are built by BuildTlasWriteBarrier and BuildTlasReadBarrier, +// pure CPU logic that this test drives directly with no GPU device, like +// SwapchainBarrierScope. #include #include "vulkan/vulkan.h" @@ -31,19 +39,39 @@ void Check(bool ok, std::string_view what) { } // namespace int main() { - const TlasReadBarrier barrier = BuildTlasReadBarrier(); + const TlasBarrier before = BuildTlasWriteBarrier(); - Check(barrier.srcStage == VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR, - "the dependency starts at the acceleration-structure build"); - Check((barrier.dstStage & VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR) != 0, + Check(before.dstStage == VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR, + "the dependency before the build ends at the acceleration-structure build"); + Check((before.srcStage & VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR) != 0, + "it follows earlier builds (the BLASes it reads, an earlier build of this TLAS)"); + Check((before.srcStage & VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR) != 0, + "and earlier ray-tracing shaders (traceRayEXT reading the TLAS)"); + Check((before.srcStage & VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT) != 0, + "and earlier compute shaders (rayQueryEXT reading the TLAS)"); + + Check(before.memory.sType == VK_STRUCTURE_TYPE_MEMORY_BARRIER, "it carries a memory barrier"); + Check(before.memory.srcAccessMask == VK_ACCESS_ACCELERATION_STRUCTURE_WRITE_BIT_KHR, + "earlier builds' acceleration-structure writes are made available, and no reads " + "(a read bit does nothing but trip synchronization validation)"); + constexpr VkAccessFlags kBuildAccess = + VK_ACCESS_ACCELERATION_STRUCTURE_READ_BIT_KHR | VK_ACCESS_ACCELERATION_STRUCTURE_WRITE_BIT_KHR; + Check((before.memory.dstAccessMask & kBuildAccess) == kBuildAccess, + "and visible to the build's reads and writes"); + + const TlasBarrier after = BuildTlasReadBarrier(); + + Check(after.srcStage == VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR, + "the dependency after the build starts at the acceleration-structure build"); + Check((after.dstStage & VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR) != 0, "it reaches ray-tracing shaders (traceRayEXT)"); - Check((barrier.dstStage & VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT) != 0, + Check((after.dstStage & VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT) != 0, "it reaches compute shaders (rayQueryEXT)"); - Check(barrier.memory.sType == VK_STRUCTURE_TYPE_MEMORY_BARRIER, "it carries a memory barrier"); - Check((barrier.memory.srcAccessMask & VK_ACCESS_ACCELERATION_STRUCTURE_WRITE_BIT_KHR) != 0, + Check(after.memory.sType == VK_STRUCTURE_TYPE_MEMORY_BARRIER, "it carries a memory barrier"); + Check((after.memory.srcAccessMask & VK_ACCESS_ACCELERATION_STRUCTURE_WRITE_BIT_KHR) != 0, "the build's acceleration-structure writes are made available"); - Check((barrier.memory.dstAccessMask & VK_ACCESS_ACCELERATION_STRUCTURE_READ_BIT_KHR) != 0, + Check((after.memory.dstAccessMask & VK_ACCESS_ACCELERATION_STRUCTURE_READ_BIT_KHR) != 0, "and visible to acceleration-structure reads"); std::println("{}", failures == 0 ? "all checks passed" : "check(s) failed"); diff --git a/tests/TLASHighWaterMark/main.cpp b/tests/TLASHighWaterMark/main.cpp index 746a66e..7bc3a11 100644 --- a/tests/TLASHighWaterMark/main.cpp +++ b/tests/TLASHighWaterMark/main.cpp @@ -27,9 +27,17 @@ // governs both). // - A same-count rebuild takes the refit (UPDATE) path: builtInstanceCount // is unchanged and the inputs are obviously not reallocated. -// - builtInstanceCount tracks the live count across every build (the AS -// itself is still rebuilt on a count change — the fix only spares the two -// host buffers, not the AS storage/scratch). +// - builtInstanceCount tracks the live count across every build. A count +// change still takes a full build, not a refit, but the acceleration +// structure follows a high-water mark too: a build that fits its storage +// builds into the same handle (same storage, same device address), and +// only one that outgrows it gets new storage and a new handle, queueing +// the old handle before the storage it lives in. +// - Builds with a change of count recorded into ONE command buffer: one that +// fits builds into the same acceleration structure, and one that outgrows +// it queues the acceleration structure the earlier build still holds +// instead of destroying it while the command buffer is being recorded. It +// stays queued until its retire frame. // - The Vulkan validation layer reports ZERO errors across all of the above // — the strongest check that feeding an oversized instance buffer to the // AS build (with tlasRangeInfo.primitiveCount < capacity) is spec-correct. @@ -51,8 +59,9 @@ // FlushDevice() added after the copy loop in BuildTLAS makes the host writes // available on a non-coherent type and self-gates to a no-op on a coherent one. // -// Validation layers are required for the last check to be meaningful; the build -// marks this test as needing the SDK layers. +// Needs a Vulkan ray-tracing device (exits 77 without one). Validation layers +// are required for the last check to be meaningful; the build marks this test +// as needing the SDK layers. #include "vulkan/vulkan.h" #include @@ -173,11 +182,42 @@ bool Reused(const InputSnapshot& a, const InputSnapshot& b) { constexpr std::uint32_t kEntrySize = sizeof(VkAccelerationStructureInstanceKHR); +// The acceleration structure's identity: handle, storage and device address. +struct ASIdentity { + VkAccelerationStructureKHR handle; + VkBuffer storage; + VkDeviceAddress address; + bool operator==(const ASIdentity&) const = default; +}; +ASIdentity CurrentAS() { + const auto& tlas = RenderingElement3D::tlases[0]; + return { tlas.accelerationStructure, tlas.buffer.buffer, tlas.address }; +} + +// Where an entry sits in the deletion queue, or the queue size when it isn't +// there. +std::size_t QueuePosition(auto matches) { + const auto it = std::ranges::find_if(Device::deletionQueue, matches); + return static_cast(it - Device::deletionQueue.begin()); +} + +std::size_t QueuedCount(VkAccelerationStructureKHR handle) { + return static_cast(std::ranges::count_if(Device::deletionQueue, + [&](const Device::PendingDeletion& e) { return e.accelerationStructure == handle; })); +} + } // namespace int main() { Device::Initialize(); + if (!Device::rayTracingSupported) { + std::println("SKIP no Vulkan ray-tracing device"); + return 77; + } Device::validationErrorCount = 0; + // No window: the test steps Device::frameCounter itself. + Device::framesInFlight = Window::numFrames; + Device::frameCounter = 0; // One real cube BLAS that every TLAS instance references. Mesh cube; @@ -193,7 +233,7 @@ int main() { // Stable backing store for the elements. Reserve to the max count this // test ever registers so the vector never reallocates (the static // `elements` array holds raw pointers into it). - constexpr std::uint32_t kMaxElems = 32; + constexpr std::uint32_t kMaxElems = 256; std::vector pool(kMaxElems); for (auto& e : pool) e.instance = MakeInstance(cube.blasAddr); @@ -305,6 +345,7 @@ int main() { Check(s16.instSize == 16 * kEntrySize, "instance buffer grew to 16 entries"); Check(RenderingElement3D::tlases[0].builtInstanceCount == 16, "builtInstanceCount == 16 after growth"); + const ASIdentity as16 = CurrentAS(); // ── 3. Shrink to 2 → REUSES the 16-entry allocation (the core win). ───── SetCount(pool, 2); @@ -316,6 +357,8 @@ int main() { "instance buffer kept its 16-entry high-water size after shrink"); Check(RenderingElement3D::tlases[0].builtInstanceCount == 2, "builtInstanceCount tracks the live count (2) even on the reuse path"); + Check(CurrentAS() == as16, + "shrinking rebuilt the acceleration structure in place (same handle, storage and address)"); // ── 4. Grow to 10 (still within the high-water capacity) → REUSES. ────── SetCount(pool, 10); @@ -323,6 +366,7 @@ int main() { InputSnapshot s10 = Snapshot(); Check(Reused(s16, s10), "growing to 10 (≤ capacity 16) reused the existing buffers"); + Check(CurrentAS() == as16, "and rebuilt the acceleration structure in place"); // ── 5. Grow to exactly 16 (== capacity) → REUSES (boundary: > not >=). ── SetCount(pool, 16); @@ -330,6 +374,7 @@ int main() { InputSnapshot s16b = Snapshot(); Check(Reused(s16, s16b), "growing to exactly the capacity (16) reused the existing buffers"); + Check(CurrentAS() == as16, "and rebuilt the acceleration structure in place"); // ── 6. Same count again → refit (UPDATE) path, inputs untouched. ──────── BuildOnce(); @@ -338,12 +383,56 @@ int main() { Check(RenderingElement3D::tlases[0].builtInstanceCount == 16, "same-count rebuild kept builtInstanceCount == 16 (took the refit path)"); - // ── 7. Grow to 32 (past the high-water) → REALLOCATES again. ──────────── + // ── 7. Grow to 32 (past the high-water) → REALLOCATES again. The + // acceleration structure outgrows its storage too, so both are + // replaced, the old handle queued ahead of the storage it lives + // in. ────────────────────────────────────────────────────────────── SetCount(pool, 32); BuildOnce(); InputSnapshot s32 = Snapshot(); Check(s32.instBuf != s16.instBuf, "growing past the high-water (32) reallocated again"); Check(s32.instSize == 32 * kEntrySize, "instance buffer grew to 32 entries"); + const ASIdentity as32 = CurrentAS(); + const std::size_t handleAt = QueuePosition([&](const Device::PendingDeletion& e) { return e.accelerationStructure == as16.handle; }); + const std::size_t storageAt = QueuePosition([&](const Device::PendingDeletion& e) { return e.buffer == as16.storage; }); + Check(as32.handle != as16.handle && as32.storage != as16.storage && storageAt < Device::deletionQueue.size(), + "the acceleration structure outgrew its storage: a new handle in new storage, the old storage queued"); + Check(handleAt < storageAt, "the old handle is queued before the storage it lives in"); + + // ── 8. Builds with a change of count in ONE command buffer (a scene and + // then a model shown in one setup). One that fits builds into the + // same acceleration structure. One that outgrows it replaces the + // acceleration structure the earlier builds in this command buffer + // still hold. That one is queued rather than destroyed, which would + // leave the command buffer invalid, and is freed only on its retire + // frame. ────────────────────────────────────────────────────────── + { + Device::DrainDeletions(); + SetCount(pool, 8); + VkCommandBuffer cmd = BeginCmd(); + RenderingElement3D::BuildTLAS(cmd, 0); + const ASIdentity first = CurrentAS(); + SetCount(pool, 24); + RenderingElement3D::BuildTLAS(cmd, 0); + Check(CurrentAS() == first && Device::deletionQueue.empty(), + "a second build in one command buffer that fits builds into the same acceleration structure, queueing nothing"); + + SetCount(pool, kMaxElems); + RenderingElement3D::BuildTLAS(cmd, 0); + const VkAccelerationStructureKHR grown = RenderingElement3D::tlases[0].accelerationStructure; + Check(QueuedCount(first.handle) == 1, + "one that outgrows the storage queues the old acceleration structure instead of destroying it while recorded"); + Check(grown != VK_NULL_HANDLE && grown != first.handle, "and creates a new one"); + + // A reclaim before its retire frame leaves it queued while this + // command buffer is still being recorded. + Device::ReclaimDeletions(); + Check(QueuedCount(first.handle) == 1, "reclaiming before the command buffer completes keeps it queued"); + SubmitWait(cmd); + Device::frameCounter += Window::numFrames; + Device::ReclaimDeletions(); + Check(QueuedCount(first.handle) == 0, "it is freed on its retire frame"); + } // Unregister everything so nothing dangles past the pool's lifetime. SetCount(pool, 0); diff --git a/tests/TLASLifetime/main.cpp b/tests/TLASLifetime/main.cpp new file mode 100644 index 0000000..eb7da79 --- /dev/null +++ b/tests/TLASLifetime/main.cpp @@ -0,0 +1,221 @@ +//SPDX-License-Identifier: LGPL-3.0-only +//SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® + +// A TlasWithBuffer hands its acceleration structure to the deferred-deletion +// queue (issue #101) when it lets go of it, the way a Mesh does its BLAS +// (MeshLifetime). Its special members used to be implicit, so only +// BuildTLAS's replacement of the handle went through the queue: +// - Destroying one dropped the handle while its buffers queued themselves, +// leaking the acceleration structure over freed storage. The handle is now +// queued too, ahead of the storage it lives in. +// - Assigning over one, the way to reset a scene's TLAS, dropped the handle it +// held the same way. It is now queued the same way. +// - Moving one copied the handle, leaving two owners, and left the moved-from +// TlasWithBuffer with its count but none of its buffers, so its next +// same-count BuildTLAS refit through buffers it no longer had. Moving now +// hands the handle over, and the moved-from one builds a new TLAS. +// +// Needs a Vulkan ray-tracing device (exits 77 without one) and no window: the +// test steps Device::frameCounter itself. The validation check only means +// something with CRAFTER_GRAPHICS_VALIDATION set. + +#include +#include "vulkan/vulkan.h" + +import Crafter.Graphics; +import Crafter.Math; +import std; + +using namespace Crafter; + +namespace { + +int failures = 0; + +void Check(bool ok, std::string_view what) { + std::println("{} {}", ok ? "PASS" : "FAIL", what); + if (!ok) ++failures; +} + +VkCommandBuffer BeginCmd() { + VkCommandBufferAllocateInfo allocInfo { + .sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO, + .commandPool = Device::commandPool, + .level = VK_COMMAND_BUFFER_LEVEL_PRIMARY, + .commandBufferCount = 1, + }; + VkCommandBuffer cmd = VK_NULL_HANDLE; + Device::CheckVkResult(vkAllocateCommandBuffers(Device::device, &allocInfo, &cmd)); + VkCommandBufferBeginInfo beginInfo { + .sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO, + .flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT, + }; + Device::CheckVkResult(vkBeginCommandBuffer(cmd, &beginInfo)); + return cmd; +} + +void SubmitWait(VkCommandBuffer cmd) { + Device::CheckVkResult(vkEndCommandBuffer(cmd)); + VkSubmitInfo submitInfo { + .sType = VK_STRUCTURE_TYPE_SUBMIT_INFO, + .commandBufferCount = 1, + .pCommandBuffers = &cmd, + }; + Device::CheckVkResult(vkQueueSubmit(Device::queue, 1, &submitInfo, VK_NULL_HANDLE)); + Device::CheckVkResult(vkQueueWaitIdle(Device::queue)); + vkFreeCommandBuffers(Device::device, Device::commandPool, 1, &cmd); +} + +// Build the TLASes of the given frames for the registered elements. +void Build(std::initializer_list frames) { + VkCommandBuffer cmd = BeginCmd(); + for (std::uint32_t frame : frames) RenderingElement3D::BuildTLAS(cmd, frame); + SubmitWait(cmd); +} + +// Where an entry sits in the queue, or the queue size when it isn't there. +std::size_t QueuePosition(auto matches) { + const auto it = std::ranges::find_if(Device::deletionQueue, matches); + return static_cast(it - Device::deletionQueue.begin()); +} + +std::size_t QueuedCount(VkAccelerationStructureKHR handle) { + return static_cast(std::ranges::count_if(Device::deletionQueue, + [&](const Device::PendingDeletion& e) { return e.accelerationStructure == handle; })); +} + +bool Queued(VkBuffer buffer) { + return std::ranges::any_of(Device::deletionQueue, + [&](const Device::PendingDeletion& e) { return e.buffer == buffer; }); +} + +// The handle is queued exactly once, ahead of the storage it lives in. +bool QueuedAhead(VkAccelerationStructureKHR handle, VkBuffer storage) { + const std::size_t handleAt = QueuePosition([&](const Device::PendingDeletion& e) { return e.accelerationStructure == handle; }); + const std::size_t storageAt = QueuePosition([&](const Device::PendingDeletion& e) { return e.buffer == storage; }); + return QueuedCount(handle) == 1 && storageAt < Device::deletionQueue.size() && handleAt < storageAt; +} + +} // namespace + +int main() { + Device::Initialize(); + if (!Device::rayTracingSupported) { + std::println("SKIP no Vulkan ray-tracing device"); + return 77; + } + Device::validationErrorCount = 0; + Device::framesInFlight = Window::numFrames; + Device::frameCounter = 0; + Device::DrainDeletions(); + + // A cube BLAS and a few instances of it, so the TLASes have something in + // them. + Mesh cube; + { + std::vector> verts { + {-1,-1,-1}, { 1,-1,-1}, { 1, 1,-1}, {-1, 1,-1}, + {-1,-1, 1}, { 1,-1, 1}, { 1, 1, 1}, {-1, 1, 1}, + }; + std::vector indices { + 0,1,2, 0,2,3, 4,6,5, 4,7,6, + 0,4,5, 0,5,1, 3,2,6, 3,6,7, + 1,5,6, 1,6,2, 0,3,7, 0,7,4, + }; + VkCommandBuffer cmd = BeginCmd(); + cube.Build(verts, indices, cmd); + SubmitWait(cmd); + } + std::vector elements(4); + for (RenderingElement3D& element : elements) { + element.instance = { + .mask = 0xFF, + .accelerationStructureReference = cube.blasAddr, + }; + MatrixRowMajor::Identity().Store(reinterpret_cast(element.instance.transform.matrix)); + RenderingElement3D::Add(&element); + } + std::array& tlases = RenderingElement3D::tlases; + Device::DrainDeletions(); + + // ── Moving hands the TLAS over; destroying it queues the handle first ──── + { + Build({0}); + const VkAccelerationStructureKHR handle = tlases[0].accelerationStructure; + const VkDeviceAddress address = tlases[0].address; + const VkBuffer storage = tlases[0].buffer.buffer; + const VkBuffer scratch = tlases[0].scratchBuffer.buffer; + const VkBuffer instances = tlases[0].instanceBuffer.buffer; + const VkBuffer metadata = tlases[0].metadataBuffer.buffer; + { + TlasWithBuffer taken(std::move(tlases[0])); + Check(tlases[0].accelerationStructure == VK_NULL_HANDLE && tlases[0].address == 0, + "a moved-from TlasWithBuffer no longer holds the acceleration structure"); + Check(taken.accelerationStructure == handle && taken.address == address && taken.buffer.buffer == storage, + "the one it moved to does"); + Check(Device::deletionQueue.empty(), "moving queues nothing"); + } + Check(QueuedCount(handle) == 1, "destroying a TlasWithBuffer queues its acceleration structure"); + Check(Queued(storage) && Queued(scratch) && Queued(instances) && Queued(metadata), + "and its storage, scratch, instance and metadata buffers"); + Check(!Device::deletionQueue.empty() && Device::deletionQueue.front().accelerationStructure == handle, + "the acceleration structure is queued before the storage it lives in"); + + // The moved-from TLAS still has its count; it has to build afresh + // rather than refit through the buffers it gave away. + Build({0}); + Check(tlases[0].accelerationStructure != VK_NULL_HANDLE && tlases[0].accelerationStructure != handle + && tlases[0].address != 0 && tlases[0].builtInstanceCount == elements.size(), + "the moved-from TlasWithBuffer builds a new TLAS"); + + Device::frameCounter += Window::numFrames; + Device::ReclaimDeletions(); + Check(Device::deletionQueue.empty(), "everything a destroyed TlasWithBuffer queued is freed on its retire frame"); + } + + // ── Assigning an empty TlasWithBuffer resets the TLAS ─────────────────── + { + const VkAccelerationStructureKHR handle = tlases[0].accelerationStructure; + const VkBuffer storage = tlases[0].buffer.buffer; + tlases[0] = TlasWithBuffer{}; + Check(tlases[0].accelerationStructure == VK_NULL_HANDLE && tlases[0].address == 0 + && tlases[0].buffer.buffer == VK_NULL_HANDLE, + "assigning an empty TlasWithBuffer leaves no TLAS"); + Check(QueuedAhead(handle, storage), + "and queues the acceleration structure it held once, ahead of its storage"); + Build({0}); + Check(tlases[0].accelerationStructure != VK_NULL_HANDLE && tlases[0].builtInstanceCount == elements.size(), + "the reset TLAS builds again"); + Device::DrainDeletions(); + } + + // ── Move assignment hands the TLAS over and queues the one it replaces ─── + { + Build({1}); + const VkAccelerationStructureKHR kept = tlases[0].accelerationStructure; + const VkDeviceAddress keptAddress = tlases[0].address; + const VkAccelerationStructureKHR replaced = tlases[1].accelerationStructure; + const VkBuffer replacedStorage = tlases[1].buffer.buffer; + + tlases[1] = std::move(tlases[0]); + Check(tlases[0].accelerationStructure == VK_NULL_HANDLE && tlases[0].address == 0 + && tlases[1].accelerationStructure == kept && tlases[1].address == keptAddress, + "move assignment hands the acceleration structure over"); + Check(QueuedAhead(replaced, replacedStorage) && QueuedCount(kept) == 0, + "and queues the one it replaces once, ahead of its storage"); + + Build({0, 1}); + Check(tlases[0].accelerationStructure != VK_NULL_HANDLE && tlases[1].accelerationStructure == kept, + "both TLASes build again, the one moved in in place"); + Device::DrainDeletions(); + } + + for (RenderingElement3D& element : elements) RenderingElement3D::Remove(&element); + + if (std::getenv("CRAFTER_GRAPHICS_VALIDATION") != nullptr) { + Check(Device::validationErrorCount == 0, + std::format("no Vulkan validation errors across the run ({} seen)", Device::validationErrorCount)); + } + std::println("{}", failures == 0 ? "all checks passed" : "check(s) failed"); + return failures == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +}