diff --git a/implementations/Crafter.Graphics-RenderingElement3D.cpp b/implementations/Crafter.Graphics-RenderingElement3D.cpp index 1da2a3a..d06630c 100644 --- a/implementations/Crafter.Graphics-RenderingElement3D.cpp +++ b/implementations/Crafter.Graphics-RenderingElement3D.cpp @@ -121,10 +121,23 @@ void RenderingElement3D::BuildTLAS(VkCommandBuffer cmd, std::uint32_t index, RTB // visibility, gating its flush-skip on the *chosen* type's flags. The // single shared buffer is preserved: a whole-buffer copy would clobber // the GPU-written transforms, so staging is deliberately avoided. + // + // metadataBuffer (#75) gets the same HOST_VISIBLE | prefer-DEVICE_LOCAL + // upgrade: it is CPU-written every frame (the copy loop below) and read + // by the ray shaders as a STORAGE_BUFFER every frame, so it has the same + // per-frame-shader-read-over-PCIe cost. Unlike instanceBuffer there is + // no GPU co-write, so the direct upgrade is unconditional — but staging + // is still wrong here: a per-frame-rewritten buffer would pay a stage+ + // copy+barrier every frame, defeating the point. HOST_COHERENT is + // likewise dropped; the FlushDevice() after the copy loop makes the host + // writes available before the consuming submit on a non-coherent type + // (no-op when the chosen type is coherent — the previous behaviour), + // and the queue submit carries the host-write -> shader-read ordering as + // it always has. 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, primitiveCount, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT); - 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); + tlas.metadataBuffer.Resize(VK_BUFFER_USAGE_STORAGE_BUFFER_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, primitiveCount, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT); } } @@ -147,6 +160,18 @@ void RenderingElement3D::BuildTLAS(VkCommandBuffer cmd, std::uint32_t index, RTB tlas.instanceBuffer.FlushDevice(cmd, VK_ACCESS_MEMORY_READ_BIT, VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR); + // Make the per-frame metadata host writes available before the consuming + // submit. metadataBuffer is not an AS build input (the build never reads it) + // and has no GPU co-write — only the ray shaders read it, in a later pass — + // so it needs neither the build-stage barrier instanceBuffer takes above nor + // a HOST->shader command barrier: the queue submit already orders host + // writes made available before it against every command in the submission, + // exactly as it did when this buffer was HOST_COHERENT. This plain host + // flush is what now makes those writes available on a non-coherent + // (BAR/VRAM) type; it self-gates to a no-op on a coherent type (#60), + // preserving the previous behaviour where the type ends up coherent. + tlas.metadataBuffer.FlushDevice(); + VkAccelerationStructureGeometryInstancesDataKHR instancesData { .sType = VK_STRUCTURE_TYPE_ACCELERATION_STRUCTURE_GEOMETRY_INSTANCES_DATA_KHR, .arrayOfPointers = VK_FALSE, diff --git a/tests/TLASHighWaterMark/main.cpp b/tests/TLASHighWaterMark/main.cpp index 55bd544..987b960 100644 --- a/tests/TLASHighWaterMark/main.cpp +++ b/tests/TLASHighWaterMark/main.cpp @@ -58,6 +58,15 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA // HOST_COHERENT: the FlushDevice(cmd, ...) in BuildTLAS gates on the chosen // type's coherency flag, so a non-coherent BAR type still flushes correctly. // +// Issue #75: the metadata buffer gets the identical upgrade — it is CPU-written +// every frame and read by the ray shaders as a STORAGE_BUFFER every frame, so +// the same HOST_VISIBLE | prefer-DEVICE_LOCAL, no-mandatory-HOST_COHERENT +// request keeps those per-frame shader reads in local VRAM. A second assertion +// block after the first build checks its chosen memory type the same way. The +// dropped HOST_COHERENT is covered by the zero-validation-errors check: the +// 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. @@ -260,6 +269,49 @@ int main() { } } + // ── Issue #75: the metadata buffer carries the same memory-type request as + // the instance buffer — HOST_VISIBLE (required, mappable) with a best-effort + // DEVICE_LOCAL preference (BAR/VRAM) and the mandatory HOST_COHERENT dropped. + // It is CPU-written every frame and read by the ray shaders every frame, so + // landing it in local VRAM keeps those per-frame storage-buffer reads off + // PCIe. Same device-independent check: assert the recorded chosen flags equal + // what GetMemoryType resolves for this buffer's memoryTypeBits with that + // request, and that DEVICE_LOCAL is chosen wherever a host-visible + // device-local type is reachable. ─────────────────────────────────────────── + { + auto& mb = RenderingElement3D::tlases[0].metadataBuffer; + VkMemoryRequirements req{}; + vkGetBufferMemoryRequirements(Device::device, mb.buffer, &req); + std::uint32_t expectIdx = Device::GetMemoryType( + req.memoryTypeBits, + VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, + VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT); + VkMemoryPropertyFlags expectFlags = + Device::memoryProperties.memoryTypes[expectIdx].propertyFlags; + Check((mb.memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) != 0, + "metadata buffer memory is HOST_VISIBLE (mappable — the required flag)"); + Check(mb.memoryPropertyFlagsChosen == expectFlags, + "metadata buffer memory matches the HOST_VISIBLE | prefer-DEVICE_LOCAL request (#75)"); + + bool barReachable = false; + constexpr VkMemoryPropertyFlags kBar = + VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT; + for (std::uint32_t i = 0; i < Device::memoryProperties.memoryTypeCount; ++i) { + if ((req.memoryTypeBits & (1u << i)) + && (Device::memoryProperties.memoryTypes[i].propertyFlags & kBar) == kBar) { + barReachable = true; + break; + } + } + if (barReachable) { + Check((mb.memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT) != 0, + "metadata buffer landed on a DEVICE_LOCAL (BAR/VRAM) type where one is reachable"); + } else { + std::println("INFO no host-visible device-local type reachable — " + "metadata fell back to plain HOST_VISIBLE (expected on non-BAR GPUs)"); + } + } + // ── 2. Grow to 16 (past capacity) → REALLOCATES. ──────────────────────── SetCount(pool, 16); BuildOnce();