Merge pull request 'perf(mesh): dirty-range vertex upload for deforming-mesh Refit (#119)' (#138) from claude/issue-119 into master

implementations/Crafter.Graphics-Mesh-WebGPU.cpp

@@ -377,6 +377,18 @@ void Mesh::Refit(std::span<Vector<float, 3, 3>> vertices,
     Build(vertices, indices, cmd);
 }
 
+void Mesh::Refit(std::span<Vector<float, 3, 3>> vertices,
+                 std::span<std::uint32_t>       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<const RTAabb> aabbs,
                            WebGPUCommandEncoderRef cmd) {
     BuildProcedural(aabbs, opaque, cmd);

implementations/Crafter.Graphics-Mesh.cpp

@@ -371,6 +371,11 @@ void Mesh::BuildProcedural(VkDeviceAddress aabbAddress, std::uint32_t count, boo
 }
 
 void Mesh::Refit(std::span<Vector<float, 3, 3>> verticies, std::span<std::uint32_t> indicies, VkCommandBuffer cmd) {
+    // Whole-vertex-array refit: the entire array is the dirty range.
+    Refit(verticies, indicies, 0, static_cast<std::uint32_t>(verticies.size()), cmd);
+}
+
+void Mesh::Refit(std::span<Vector<float, 3, 3>> verticies, std::span<std::uint32_t> 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<Vector<float, 3, 3>> verticies, std::span<std::uint32
 
     if (!update) {
         // Counts may have changed (or update wasn't permitted): take the
-        // resizing build path, preserving the caller's original flags.
+        // resizing build path, preserving the caller's original flags. The
+        // dirty window is irrelevant here — a fresh build re-uploads the whole
+        // array, which is why the full arrays are passed even on this overload.
         Build(verticies, indicies, cmd, RTBuildOptions{
             .preference = (buildFlags & VK_BUILD_ACCELERATION_STRUCTURE_PREFER_FAST_BUILD_BIT_KHR)
                               ? RTBuildPreference::FastBuild : RTBuildPreference::FastTrace,
@@ -392,7 +399,7 @@ void Mesh::Refit(std::span<Vector<float, 3, 3>> verticies, std::span<std::uint32
         return;
     }
 
-    // Same-sized buffers: overwrite the vertex positions in place so the
+    // Same-sized buffers: overwrite the moved vertex positions in place so the
     // device address (and thus the geometry the UPDATE reads) stays stable.
     // The index buffer is deliberately left untouched: a hardware AS UPDATE
     // cannot change topology, so the indices are immutable here — re-uploading
@@ -400,11 +407,21 @@ void Mesh::Refit(std::span<Vector<float, 3, 3>> verticies, std::span<std::uint32
     // refit. This means a bitwise-different-but-topologically-equivalent index
     // array passed on refit is ignored, consistent with the documented
     // contract that a refit may only move vertex positions.
-    // Re-upload only the vertex positions; UploadDeviceLocal reuses the same
+    //
-    // device-local allocation (count unchanged) so the address the UPDATE reads
+    // Re-upload only the dirty sub-range [dirtyVertexOffset, +dirtyVertexCount):
-    // stays stable, then barriers the write before the build. On the staged
+    // the rest of the vertex buffer already holds last refit's positions, so a
-    // path this re-stages per refit (the deforming-mesh fallback, #73).
+    // ranged upload patches just the vertices that moved (#119). The window is
-    vertexBuffer.UploadDeviceLocal(kVertexUsageBase, verticies.data(), static_cast<std::uint32_t>(verticies.size()), cmd, VK_ACCESS_MEMORY_READ_BIT, VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR);
+    // 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<std::uint32_t>(verticies.size()));
+    std::uint32_t count  = std::min(dirtyVertexCount, static_cast<std::uint32_t>(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<std::uint32_t>(verticies.size()), static_cast<std::uint32_t>(indicies.size()), buildFlags, /*update*/ true, cmd);
 }

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<Vector<float, 3, 3>> verticies, std::span<std::uint32_t> 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<Vector<float, 3, 3>> verticies, std::span<std::uint32_t> 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<const RTAabb> aabbs, VkCommandBuffer cmd);
         // Zero-copy procedural refit: the device-buffer counterpart of
@@ -307,6 +323,17 @@ export namespace Crafter {
         void Refit(std::span<Crafter::Vector<float, 3, 3>> vertices,
                    std::span<std::uint32_t>                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<Crafter::Vector<float, 3, 3>> vertices,
+                   std::span<std::uint32_t>                indices,
+                   std::uint32_t                           dirtyVertexOffset,
+                   std::uint32_t                           dirtyVertexCount,
+                   WebGPUCommandEncoderRef                 cmd = 0);
         void RefitProcedural(std::span<const RTAabb> aabbs,
                              WebGPUCommandEncoderRef cmd = 0);
         // Zero-copy procedural refit: re-copy the boxes from the device buffer

interfaces/Crafter.Graphics-VulkanBuffer.cppm

@@ -140,8 +140,14 @@ namespace Crafter {
             };
             address = vkGetBufferDeviceAddress(Device::device, &addressInfo);
 
-            if constexpr(Mapped) {
+            // 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) {
                 Device::CheckVkResult(vkMapMemory(Device::device, memory, 0, memReqs.size, 0, reinterpret_cast<void**>(&(VulkanBufferMappedConditional<T, true>::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<VkDeviceSize>(offset) * sizeof(T);
+            VkDeviceSize bytes      = static_cast<VkDeviceSize>(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<std::byte*>(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<T, true> 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, &region);
+                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.

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;
     }