From 1f4c77000a5809ae23271c690638e1e20de5a17e Mon Sep 17 00:00:00 2001 From: catbot Date: Wed, 17 Jun 2026 19:50:48 +0000 Subject: [PATCH] perf(mesh): dirty-range vertex upload for deforming-mesh Refit (#119) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mesh::Refit re-uploaded the entire vertex array every frame on the in-place UPDATE path (full host write + flush + barrier on the direct path; full re-stage + copy on the staged path), even when only a handful of vertices moved. Add VulkanBuffer::UploadDeviceLocalRange — a dirty-range counterpart to UploadDeviceLocal that writes/flushes/stages/copies and barriers only the half-open element range [offset, offset+count) of an already-allocated device-local buffer. It picks direct-map vs staged-copy from the memory type the buffer was actually allocated with (not the sub-range size, which PreferDirectDeviceWrite would mis-route), and the direct path's ranged flush is rounded to nonCoherentAtomSize and clamped to the allocation size (mappedSize is now recorded for every buffer, not just mapped ones). Add a dirty-range Refit overload taking the full vertex/index arrays plus a (dirtyVertexOffset, dirtyVertexCount) window. The full-span Refit now delegates to it with the whole array as the window. On the in-place UPDATE path only the declared window is uploaded — the rest of the device buffer retains last refit's positions; when an UPDATE isn't possible it falls back to the full-span rebuild, which is why the full arrays are still passed. The WebGPU/DOM backend keeps API symmetry: it has no hardware AS, so it ignores the window and rebuilds the host BVH from the full geometry. BLASBuildOptions exercises the dirty-range refit on the direct path, the staged path, and the count-change rebuild fallback, asserting AS-handle / blasAddr stability and zero Vulkan validation errors. Resolves #119 Co-Authored-By: Claude Opus 4.8 --- .../Crafter.Graphics-Mesh-WebGPU.cpp | 12 +++ implementations/Crafter.Graphics-Mesh.cpp | 31 +++++-- interfaces/Crafter.Graphics-Mesh.cppm | 27 ++++++ interfaces/Crafter.Graphics-VulkanBuffer.cppm | 88 ++++++++++++++++++- tests/BLASBuildOptions/main.cpp | 49 +++++++++++ 5 files changed, 199 insertions(+), 8 deletions(-) diff --git a/implementations/Crafter.Graphics-Mesh-WebGPU.cpp b/implementations/Crafter.Graphics-Mesh-WebGPU.cpp index e85317c..edb4065 100644 --- a/implementations/Crafter.Graphics-Mesh-WebGPU.cpp +++ b/implementations/Crafter.Graphics-Mesh-WebGPU.cpp @@ -377,6 +377,18 @@ void Mesh::Refit(std::span> vertices, Build(vertices, indices, cmd); } +void Mesh::Refit(std::span> vertices, + std::span indices, + std::uint32_t /*dirtyVertexOffset*/, + std::uint32_t /*dirtyVertexCount*/, + WebGPUCommandEncoderRef cmd) { + // No hardware AS, so there is no sub-range to update in place — the + // software path rebuilds the host BVH over the full geometry regardless. + // The dirty window only matters to the hardware backend's ranged upload + // (#119); here it is ignored and this is identical to the full-span Refit. + Build(vertices, indices, cmd); +} + void Mesh::RefitProcedural(std::span aabbs, WebGPUCommandEncoderRef cmd) { BuildProcedural(aabbs, opaque, cmd); diff --git a/implementations/Crafter.Graphics-Mesh.cpp b/implementations/Crafter.Graphics-Mesh.cpp index 90649a7..01572da 100644 --- a/implementations/Crafter.Graphics-Mesh.cpp +++ b/implementations/Crafter.Graphics-Mesh.cpp @@ -371,6 +371,11 @@ void Mesh::BuildProcedural(VkDeviceAddress aabbAddress, std::uint32_t count, boo } void Mesh::Refit(std::span> verticies, std::span indicies, VkCommandBuffer cmd) { + // Whole-vertex-array refit: the entire array is the dirty range. + Refit(verticies, indicies, 0, static_cast(verticies.size()), cmd); +} + +void Mesh::Refit(std::span> verticies, std::span indicies, std::uint32_t dirtyVertexOffset, std::uint32_t dirtyVertexCount, VkCommandBuffer cmd) { // A hardware in-place UPDATE is only valid when the original build asked // for it, an AS already exists, and the topology is unchanged. Otherwise // fall back to a full rebuild (mode BUILD) of the same buffers — still @@ -383,7 +388,9 @@ void Mesh::Refit(std::span> verticies, std::span> verticies, std::span> verticies, std::span(verticies.size()), cmd, VK_ACCESS_MEMORY_READ_BIT, VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR); + // + // Re-upload only the dirty sub-range [dirtyVertexOffset, +dirtyVertexCount): + // the rest of the vertex buffer already holds last refit's positions, so a + // ranged upload patches just the vertices that moved (#119). The window is + // clamped to the array — a caller window past the end would otherwise read + // out of bounds / overflow the copy. UploadDeviceLocalRange writes into the + // existing same-sized allocation (no Resize), so the address the UPDATE + // reads stays stable, then barriers just that sub-range before the build. On + // the staged path this re-stages only the dirty slice (the deforming-mesh + // fallback, #73). + std::uint32_t offset = std::min(dirtyVertexOffset, static_cast(verticies.size())); + std::uint32_t count = std::min(dirtyVertexCount, static_cast(verticies.size()) - offset); + if (count != 0) { + vertexBuffer.UploadDeviceLocalRange(verticies.data() + offset, offset, count, cmd, VK_ACCESS_MEMORY_READ_BIT, VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR); + } RecordBLASBuild(*this, static_cast(verticies.size()), static_cast(indicies.size()), buildFlags, /*update*/ true, cmd); } diff --git a/interfaces/Crafter.Graphics-Mesh.cppm b/interfaces/Crafter.Graphics-Mesh.cppm index 1a84574..6176f3c 100644 --- a/interfaces/Crafter.Graphics-Mesh.cppm +++ b/interfaces/Crafter.Graphics-Mesh.cppm @@ -164,6 +164,22 @@ export namespace Crafter { // both buffers. Lifetime contract matches Build: the spans need only // outlive this call. Call this per frame to track a deforming mesh. void Refit(std::span> verticies, std::span indicies, VkCommandBuffer cmd); + // Dirty-range refit: same as Refit above, but only the contiguous block + // of vertices in [dirtyVertexOffset, dirtyVertexOffset + dirtyVertexCount) + // actually moved, so on the in-place UPDATE path only that sub-range is + // re-uploaded — host-write + flush + barrier (direct) or re-stage + copy + // (staged) scale with the moved vertices, not the whole array (#119). Use + // this for a deforming mesh that nudges a small, known window each frame. + // `verticies` / `indicies` are still the *full* arrays (same contract and + // topology rule as the full-span Refit — the index buffer is read only + // for its count on the UPDATE path); the dirty window simply tells the + // upload which slice changed, and is clamped to the array bounds. When an + // in-place UPDATE is not possible (allowUpdate was not set, or the counts + // changed) this falls back to the full-span Refit, which re-uploads + // everything from `verticies` / `indicies` — so passing the full arrays + // keeps that fallback correct. The AS handle / blasAddr are preserved on + // the UPDATE path exactly as in the full-span Refit. + void Refit(std::span> verticies, std::span indicies, std::uint32_t dirtyVertexOffset, std::uint32_t dirtyVertexCount, VkCommandBuffer cmd); // Procedural analog of Refit: new object-space boxes, same count. void RefitProcedural(std::span aabbs, VkCommandBuffer cmd); // Zero-copy procedural refit: the device-buffer counterpart of @@ -307,6 +323,17 @@ export namespace Crafter { void Refit(std::span> vertices, std::span indices, WebGPUCommandEncoderRef cmd = 0); + // Dirty-range refit (#119). The software path has no hardware AS to + // update a sub-range of — it rebuilds the host BVH over the full + // geometry regardless — so the dirty window is ignored here and this + // behaves exactly like the full-span Refit above (a fresh build that + // re-publishes blasAddr). The overload exists so portable deforming-mesh + // code that passes a dirty window compiles and stays correct on WebGPU. + void Refit(std::span> vertices, + std::span indices, + std::uint32_t dirtyVertexOffset, + std::uint32_t dirtyVertexCount, + WebGPUCommandEncoderRef cmd = 0); void RefitProcedural(std::span aabbs, WebGPUCommandEncoderRef cmd = 0); // Zero-copy procedural refit: re-copy the boxes from the device buffer diff --git a/interfaces/Crafter.Graphics-VulkanBuffer.cppm b/interfaces/Crafter.Graphics-VulkanBuffer.cppm index fe0e60a..d606921 100644 --- a/interfaces/Crafter.Graphics-VulkanBuffer.cppm +++ b/interfaces/Crafter.Graphics-VulkanBuffer.cppm @@ -140,8 +140,14 @@ namespace Crafter { }; address = vkGetBufferDeviceAddress(Device::device, &addressInfo); + // Record the allocation's byte size (≥ `size`) for every buffer, + // not just mapped ones: UploadDeviceLocalRange's direct path flushes + // only the dirty sub-range and clamps its rounded-up upper bound to + // this (the nonCoherentAtomSize-aligned-end exception, see + // AlignMappedFlushRange). A non-mapped buffer never maps persistently, + // but its memory is still the allocation a transient map writes into. + mappedSize = memReqs.size; if constexpr(Mapped) { - mappedSize = memReqs.size; Device::CheckVkResult(vkMapMemory(Device::device, memory, 0, memReqs.size, 0, reinterpret_cast(&(VulkanBufferMappedConditional::value)))); } } @@ -312,6 +318,86 @@ namespace Crafter { vkCmdPipelineBarrier(cmd, srcStageMask, dstStageMask, 0, 0, NULL, 1, &barrier, 0, NULL); } + // Re-upload only the half-open sub-range [offset, offset+count) of an + // already-allocated device-local buffer — the dirty-range counterpart of + // UploadDeviceLocal. `src` points at the first changed element (not the + // start of the whole array); `offset` is that element's index in this + // buffer. The buffer must already exist at its full size (a prior + // UploadDeviceLocal / Build sized it): this never Resizes, so the device + // address stays stable and the untouched elements keep their last + // contents. Only the dirty bytes are written + flushed (direct path) or + // staged + copied (staged path), and the post-upload barrier covers only + // that sub-range — so a deforming-mesh refit that nudges a few vertices + // pays for those vertices, not a full-array re-upload + flush + copy (#119). + // + // The direct-vs-staged choice is read from the memory type the buffer was + // actually allocated with (HOST_VISIBLE → map + write in place; + // device-local-only → stage + GPU copy), NOT from the sub-range size: the + // allocation is fixed, so a small dirty range must not be mis-routed to a + // map of a non-host-visible buffer the way UploadDeviceLocal's size-based + // PreferDirectDeviceWrite check would. The staged path's GPU copy needs + // VK_BUFFER_USAGE_TRANSFER_DST_BIT on the destination — already present, + // since UploadDeviceLocal only allocates device-local-only memory on the + // staged path, where it adds that bit. + void UploadDeviceLocalRange(const T* src, std::uint32_t offset, std::uint32_t count, VkCommandBuffer cmd, VkAccessFlags dstAccessMask, VkPipelineStageFlags dstStageMask) requires(!Mapped) { + VkDeviceSize byteOffset = static_cast(offset) * sizeof(T); + VkDeviceSize bytes = static_cast(count) * sizeof(T); + VkAccessFlags srcAccessMask; + VkPipelineStageFlags srcStageMask; + if (memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) { + void* mapped = nullptr; + Device::CheckVkResult(vkMapMemory(Device::device, memory, 0, VK_WHOLE_SIZE, 0, &mapped)); + std::memcpy(static_cast(mapped) + byteOffset, src, bytes); + // Non-coherent memory needs an explicit flush — but only of the + // sub-range actually written, rounded outward to + // nonCoherentAtomSize (and clamped to the allocation size). + if (!(memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT)) { + MappedFlushRange r = AlignMappedFlushRange( + byteOffset, bytes, Device::nonCoherentAtomSize, mappedSize); + VkMappedMemoryRange range { + .sType = VK_STRUCTURE_TYPE_MAPPED_MEMORY_RANGE, + .memory = memory, + .offset = r.offset, + .size = r.size + }; + vkFlushMappedMemoryRanges(Device::device, 1, &range); + } + vkUnmapMemory(Device::device, memory); + srcAccessMask = VK_ACCESS_HOST_WRITE_BIT; + srcStageMask = VK_PIPELINE_STAGE_HOST_BIT; + } else { + // Device-local-only: stage just the dirty elements and copy them + // into place at byteOffset. The staging buffer outlives the queued + // copy via the fence-keyed deletion queue (#101/#102), exactly as + // the full-buffer staged path does. + VulkanBuffer staging; + staging.Create(VK_BUFFER_USAGE_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, count); + std::memcpy(staging.value, src, bytes); + staging.FlushDevice(); + VkBufferCopy region { .srcOffset = 0, .dstOffset = byteOffset, .size = bytes }; + vkCmdCopyBuffer(cmd, staging.buffer, buffer, 1, ®ion); + staging.DeferredClear(); + srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; + srcStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; + } + + // Order only the written sub-range before the consumer reads it. The + // untouched bytes were made visible by their own prior upload's + // barrier and are not written in this submit, so they need no fresh + // dependency even though the build reads the whole buffer. + VkBufferMemoryBarrier barrier = { + .sType = VK_STRUCTURE_TYPE_BUFFER_MEMORY_BARRIER, + .srcAccessMask = srcAccessMask, + .dstAccessMask = dstAccessMask, + .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .buffer = buffer, + .offset = byteOffset, + .size = bytes + }; + vkCmdPipelineBarrier(cmd, srcStageMask, dstStageMask, 0, 0, NULL, 1, &barrier, 0, NULL); + } + void FlushDevice() requires(Mapped) { // Coherent memory needs no explicit flush — host writes are // automatically visible to the device. diff --git a/tests/BLASBuildOptions/main.cpp b/tests/BLASBuildOptions/main.cpp index 538fae9..827b0ca 100644 --- a/tests/BLASBuildOptions/main.cpp +++ b/tests/BLASBuildOptions/main.cpp @@ -157,6 +157,42 @@ int main() { Check(tri.blasAddr == triAddrBefore, "Refit kept the same blasAddr (instances stay valid)"); + // Dirty-range refit (issue #119): move only a sub-window of the vertices + // and pass it as [offset, count). The ranged upload patches just those + // vertices in place; the UPDATE must still keep the same AS handle / + // blasAddr, and the validation layer (checked at the end) is the real proof + // that the ranged host-write / flush / barrier — and the staged variant + // below — are spec-correct. The untouched vertices retain their last values + // in the device buffer, so the refit BLAS is over the full deformed cube. + { + // Full array of moved positions; we declare only the back half [4,8) + // as the dirty window, so only those 4 vertices are re-uploaded. + auto verts = CubeVerts(2.0f); + auto idx = CubeIndices(); + VkCommandBuffer cmd = BeginCmd(); + tri.Refit(verts, idx, /*dirtyVertexOffset*/ 4, /*dirtyVertexCount*/ 4, cmd); + SubmitWait(cmd); + } + Check(tri.accelerationStructure == triHandleBefore, + "dirty-range Refit kept the same AS handle (in-place UPDATE, #119)"); + Check(tri.blasAddr == triAddrBefore, + "dirty-range Refit kept the same blasAddr (#119)"); + + // A dirty-range refit on a mesh whose counts changed must still fall back + // to a full rebuild (the dirty window is irrelevant on that path) and + // succeed — same robustness as the full-span Refit fallback. + { + auto verts = CubeVerts(1.0f); + verts.push_back({2.0f, 2.0f, 2.0f}); // count change → topology change + auto idx = CubeIndices(); + idx.insert(idx.end(), {0u, 1u, 8u}); + VkCommandBuffer cmd = BeginCmd(); + tri.Refit(verts, idx, /*dirtyVertexOffset*/ 8, /*dirtyVertexCount*/ 1, cmd); + SubmitWait(cmd); + } + Check(tri.blasAddr != 0 && tri.builtPrimitiveCount == 13, + "dirty-range Refit with a count change rebuilt the BLAS (13 prims, #119)"); + // ── Triangle BLAS: fast-build, no update → flags reflect the choice. ─ Mesh triFast; { @@ -340,6 +376,19 @@ int main() { Check(staged.accelerationStructure == stagedHandle, "staged-upload Refit kept the same AS handle (in-place UPDATE, #73)"); + // Dirty-range refit on the staged path (#119): the ranged upload must + // take the stage-a-sub-slice + vkCmdCopyBuffer-at-offset path (the + // buffer is pure DEVICE_LOCAL), keep the AS handle, and stay + // validation-clean. Only the front half [0,4) is declared dirty. + { + auto verts3 = CubeVerts(2.0f); + VkCommandBuffer cmd = BeginCmd(); + staged.Refit(verts3, idx, /*dirtyVertexOffset*/ 0, /*dirtyVertexCount*/ 4, cmd); + SubmitWait(cmd); + } + Check(staged.accelerationStructure == stagedHandle, + "staged-upload dirty-range Refit kept the same AS handle (#119)"); + Device::directWriteBudget = savedBudget; }