diff --git a/implementations/Crafter.Graphics-RenderingElement3D.cpp b/implementations/Crafter.Graphics-RenderingElement3D.cpp index b43f43e..5eea69c 100644 --- a/implementations/Crafter.Graphics-RenderingElement3D.cpp +++ b/implementations/Crafter.Graphics-RenderingElement3D.cpp @@ -88,12 +88,29 @@ void RenderingElement3D::BuildTLAS(VkCommandBuffer cmd, std::uint32_t index, RTB } if (topologyChanged) { - // Resize the host-visible inputs to match the new count. + // Grow the host-visible inputs on a high-water mark rather than + // resizing to the exact count every topology change. These buffers + // only need to hold *at least* primitiveCount entries — the AS build + // reads exactly primitiveCount of them via tlasRangeInfo, and the + // copy loop below writes [0, primitiveCount) — so an allocation left + // 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. + // + // instanceBuffer and metadataBuffer grow in lockstep (always resized + // together to the same entry count), so one capacity check covers + // both. size is only meaningful once buffer is non-null, so the + // null check must short-circuit before the division. // STORAGE_BUFFER_BIT is required because the application's compute - // shaders bind this buffer as a storage SSBO (e.g. to write + // shaders bind these buffers as storage SSBOs (e.g. to write // per-instance transforms directly into the TLAS instance data). - tlas.instanceBuffer.Resize(VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_ACCELERATION_STRUCTURE_BUILD_INPUT_READ_ONLY_BIT_KHR | VK_BUFFER_USAGE_STORAGE_BUFFER_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT, primitiveCount); - tlas.metadataBuffer.Resize(VK_BUFFER_USAGE_STORAGE_BUFFER_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT, primitiveCount); + if (tlas.instanceBuffer.buffer == VK_NULL_HANDLE + || primitiveCount > tlas.instanceBuffer.size / sizeof(VkAccelerationStructureInstanceKHR)) { + tlas.instanceBuffer.Resize(VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_ACCELERATION_STRUCTURE_BUILD_INPUT_READ_ONLY_BIT_KHR | VK_BUFFER_USAGE_STORAGE_BUFFER_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT, primitiveCount); + tlas.metadataBuffer.Resize(VK_BUFFER_USAGE_STORAGE_BUFFER_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT, primitiveCount); + } } for(std::uint32_t i = 0; i < primitiveCount; i++) { diff --git a/project.cpp b/project.cpp index 5cab540..b5b0353 100644 --- a/project.cpp +++ b/project.cpp @@ -328,6 +328,37 @@ extern "C" Configuration CrafterBuildProject(std::span a bc.GetInterfacesAndImplementations(ifaces, blasImpls); cfg.tests.push_back(std::move(blasTest)); + // Issue #64: TLAS host-input buffers (instanceBuffer / metadataBuffer) + // grow on a high-water mark instead of reallocating to the exact count + // every topology change. Drives the real hardware AS-build path — a + // cube BLAS plus RenderingElement3D::BuildTLAS at a sequence of + // 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. + Test tlasTest; + Configuration& tlc = tlasTest.config; + tlc.path = cfg.path; + tlc.name = "TLASHighWaterMark"; + tlc.outputName = "TLASHighWaterMark"; + tlc.type = ConfigurationType::Executable; + tlc.target = cfg.target; + tlc.march = cfg.march; + tlc.mtune = cfg.mtune; + tlc.debug = cfg.debug; + tlc.sysroot = cfg.sysroot; + tlc.dependencies = cfg.dependencies; + tlc.externalDependencies = cfg.externalDependencies; + tlc.compileFlags = cfg.compileFlags; + tlc.linkFlags = cfg.linkFlags; + tlc.defines = cfg.defines; + tlc.cFiles = cfg.cFiles; + std::vector tlasImpls(impls.begin(), impls.end()); + tlasImpls.emplace_back("tests/TLASHighWaterMark/main"); + tlc.GetInterfacesAndImplementations(ifaces, tlasImpls); + cfg.tests.push_back(std::move(tlasTest)); + // Issue #51: FontAtlas only re-uploads the dirty sub-rect now, // tracked via FontAtlas::DirtyRect. The accumulation/clamp math is // pure CPU, so this test drives it directly — no GPU device needed diff --git a/tests/TLASHighWaterMark/main.cpp b/tests/TLASHighWaterMark/main.cpp new file mode 100644 index 0000000..d35de8a --- /dev/null +++ b/tests/TLASHighWaterMark/main.cpp @@ -0,0 +1,287 @@ +/* +Crafter®.Graphics +Copyright (C) 2026 Catcrafts® +catcrafts.net + +This library is free software; you can redistribute it and/or +modify it under the terms of the GNU Lesser General Public +License version 3.0 as published by the Free Software Foundation; + +This library is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +Lesser General Public License for more details. + +You should have received a copy of the GNU Lesser General Public +License along with this library; if not, write to the Free Software +Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +*/ + +// Issue #64: TLAS host-visible input buffers (instanceBuffer / metadataBuffer) +// grow on a high-water mark instead of being reallocated to the exact instance +// count on every topology change. These buffers only ever need to hold *at +// least* primitiveCount entries — the AS build reads exactly primitiveCount of +// them — so an allocation left over from a larger earlier frame is reused. +// +// This drives the real hardware path: a headless Vulkan RT device (no swapchain +// needed — a TLAS build only touches the queue + command pool), a real cube +// BLAS, and RenderingElement3D::BuildTLAS recorded into one-time command +// buffers at a sequence of instance counts. +// +// What is asserted: +// - First build at a given count ALLOCATES the host inputs (non-null handle, +// non-zero device address, size == count·sizeof(entry)). +// - Growing PAST the current capacity REALLOCATES (new VkBuffer handle, new +// address, larger size) — the only case that still pays the realloc. +// - SHRINKING reuses the existing allocation unchanged (same handle, same +// address, same size) — the core win of this issue, and the case the +// pre-fix code reallocated on. +// - Growing back up but still WITHIN the high-water capacity also reuses it. +// - Growing to EXACTLY the capacity reuses it (boundary: count > capacity, +// not >=). +// - instanceBuffer and metadataBuffer grow in lockstep (one capacity check +// 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). +// - 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. +// +// 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 + +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; +} + +// One-time command buffer helpers — record a build, submit, block. +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); +} + +// A unit cube (8 verts, 12 triangles) — enough topology for a real BLAS. +std::vector> CubeVerts(float s) { + return { + {-s,-s,-s}, { s,-s,-s}, { s, s,-s}, {-s, s,-s}, + {-s,-s, s}, { s,-s, s}, { s, s, s}, {-s, s, s}, + }; +} +std::vector CubeIndices() { + return { + 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, + }; +} + +// One TLAS instance referencing `blasAddr`, identity transform, visible mask. +VkAccelerationStructureInstanceKHR MakeInstance(VkDeviceAddress blasAddr) { + VkAccelerationStructureInstanceKHR inst{}; + inst.transform = VkTransformMatrixKHR{{ + {1.0f, 0.0f, 0.0f, 0.0f}, + {0.0f, 1.0f, 0.0f, 0.0f}, + {0.0f, 0.0f, 1.0f, 0.0f}, + }}; + inst.mask = 0xFF; + inst.accelerationStructureReference = blasAddr; + return inst; +} + +// Register exactly `n` elements from a stable (reserved, never-reallocated) +// pool. Clears the current registration first so the live count is exactly n. +void SetCount(std::vector& pool, std::uint32_t n) { + while (!RenderingElement3D::elements.empty()) { + RenderingElement3D::Remove(RenderingElement3D::elements.back()); + } + for (std::uint32_t i = 0; i < n; ++i) { + RenderingElement3D::Add(&pool[i]); + } +} + +// Build the frame-0 TLAS for the currently-registered elements. +void BuildOnce() { + VkCommandBuffer cmd = BeginCmd(); + RenderingElement3D::BuildTLAS(cmd, 0); + SubmitWait(cmd); +} + +// Snapshot of the host-input buffer identity, for before/after comparison. +struct InputSnapshot { + VkBuffer instBuf; + VkDeviceAddress instAddr; + std::uint32_t instSize; + VkBuffer metaBuf; + VkDeviceAddress metaAddr; + std::uint32_t metaSize; +}; +InputSnapshot Snapshot() { + auto& tlas = RenderingElement3D::tlases[0]; + return { + tlas.instanceBuffer.buffer, tlas.instanceBuffer.address, tlas.instanceBuffer.size, + tlas.metadataBuffer.buffer, tlas.metadataBuffer.address, tlas.metadataBuffer.size, + }; +} + +// Both host inputs untouched (same VkBuffer handle, address and size). +bool Reused(const InputSnapshot& a, const InputSnapshot& b) { + return a.instBuf == b.instBuf && a.instAddr == b.instAddr && a.instSize == b.instSize + && a.metaBuf == b.metaBuf && a.metaAddr == b.metaAddr && a.metaSize == b.metaSize; +} + +constexpr std::uint32_t kEntrySize = sizeof(VkAccelerationStructureInstanceKHR); + +} // namespace + +int main() { + Device::Initialize(); + Device::validationErrorCount = 0; + + // One real cube BLAS that every TLAS instance references. + Mesh cube; + { + auto verts = CubeVerts(1.0f); + auto idx = CubeIndices(); + VkCommandBuffer cmd = BeginCmd(); + cube.Build(verts, idx, cmd, RTBuildOptions{ .allowUpdate = true }); + SubmitWait(cmd); + } + Check(cube.blasAddr != 0, "cube BLAS produced a non-zero blasAddr"); + + // 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; + std::vector pool(kMaxElems); + for (auto& e : pool) e.instance = MakeInstance(cube.blasAddr); + + // ── 1. First build at 4 instances → allocates the host inputs. ────────── + SetCount(pool, 4); + BuildOnce(); + InputSnapshot s4 = Snapshot(); + Check(s4.instBuf != VK_NULL_HANDLE, "first build allocated the instance buffer"); + Check(s4.metaBuf != VK_NULL_HANDLE, "first build allocated the metadata buffer"); + Check(s4.instAddr != 0, "instance buffer has a non-zero device address"); + Check(s4.instSize == 4 * kEntrySize, "instance buffer sized for 4 entries"); + Check(RenderingElement3D::tlases[0].builtInstanceCount == 4, + "builtInstanceCount == 4 after first build"); + + // ── 2. Grow to 16 (past capacity) → REALLOCATES. ──────────────────────── + SetCount(pool, 16); + BuildOnce(); + InputSnapshot s16 = Snapshot(); + Check(s16.instBuf != s4.instBuf, "growing past capacity reallocated the instance buffer"); + Check(s16.metaBuf != s4.metaBuf, "growing past capacity reallocated the metadata buffer"); + Check(s16.instSize == 16 * kEntrySize, "instance buffer grew to 16 entries"); + Check(RenderingElement3D::tlases[0].builtInstanceCount == 16, + "builtInstanceCount == 16 after growth"); + + // ── 3. Shrink to 2 → REUSES the 16-entry allocation (the core win). ───── + SetCount(pool, 2); + BuildOnce(); + InputSnapshot s2 = Snapshot(); + Check(Reused(s16, s2), + "shrinking to 2 reused the existing buffers (no realloc — high-water mark)"); + Check(s2.instSize == 16 * kEntrySize, + "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"); + + // ── 4. Grow to 10 (still within the high-water capacity) → REUSES. ────── + SetCount(pool, 10); + BuildOnce(); + InputSnapshot s10 = Snapshot(); + Check(Reused(s16, s10), + "growing to 10 (≤ capacity 16) reused the existing buffers"); + + // ── 5. Grow to exactly 16 (== capacity) → REUSES (boundary: > not >=). ── + SetCount(pool, 16); + BuildOnce(); + InputSnapshot s16b = Snapshot(); + Check(Reused(s16, s16b), + "growing to exactly the capacity (16) reused the existing buffers"); + + // ── 6. Same count again → refit (UPDATE) path, inputs untouched. ──────── + BuildOnce(); + InputSnapshot s16c = Snapshot(); + Check(Reused(s16, s16c), "same-count rebuild left the inputs untouched"); + 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. ──────────── + 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"); + + // Unregister everything so nothing dangles past the pool's lifetime. + SetCount(pool, 0); + + Check(Device::validationErrorCount == 0, + std::format("no Vulkan validation errors ({} seen)", Device::validationErrorCount)); + + // Tear down the frame-0 TLAS while the Vulkan device is still alive. The + // static tlases[] array is destroyed at process exit, by which point the + // device (also static) may already be gone — so its VulkanBuffer + // destructors would fault. Releasing here keeps the exit path clean. + { + auto& t0 = RenderingElement3D::tlases[0]; + if (t0.accelerationStructure != VK_NULL_HANDLE) { + Device::vkDestroyAccelerationStructureKHR(Device::device, t0.accelerationStructure, nullptr); + t0.accelerationStructure = VK_NULL_HANDLE; + } + if (t0.instanceBuffer.buffer != VK_NULL_HANDLE) t0.instanceBuffer.Clear(); + if (t0.metadataBuffer.buffer != VK_NULL_HANDLE) t0.metadataBuffer.Clear(); + if (t0.scratchBuffer.buffer != VK_NULL_HANDLE) t0.scratchBuffer.Clear(); + if (t0.buffer.buffer != VK_NULL_HANDLE) t0.buffer.Clear(); + } + + if (failures != 0) { + std::println("{} check(s) failed", failures); + return EXIT_FAILURE; + } + std::println("all checks passed"); + return EXIT_SUCCESS; +}