diff --git a/interfaces/Crafter.Graphics-VulkanBuffer.cppm b/interfaces/Crafter.Graphics-VulkanBuffer.cppm index fe0e60a..a561487 100644 --- a/interfaces/Crafter.Graphics-VulkanBuffer.cppm +++ b/interfaces/Crafter.Graphics-VulkanBuffer.cppm @@ -280,21 +280,47 @@ namespace Crafter { srcStageMask = VK_PIPELINE_STAGE_HOST_BIT; } else { Resize(usageFlags | VK_BUFFER_USAGE_TRANSFER_DST_BIT, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, count); - VulkanBuffer staging; + // Persistent staging instead of a Create+DeferredClear per call + // (issue #120): on no-/small-BAR hardware Mesh::Refit / + // RecordProceduralBuild hit this branch every frame for deforming + // meshes, so a fresh alloc+map+free cycle per upload was pure + // churn on exactly the hardware this path targets. Keep a small + // ring of staging buffers, each grown to a high-water mark and + // reused — reallocated (by Resize, which defers the outgrown + // allocation) only when a larger upload arrives. + // + // The ring is grown lazily, on first entry into this staged + // branch, so ReBAR/UMA hardware (which always takes the direct + // `if` branch above) never constructs a staging allocation it + // does not use. + // + // Why a ring and not one shared buffer: the vkCmdCopyBuffer below + // still reads the staging buffer after this call returns — the + // reason the old code used DeferredClear. With frames pipelined + // framesInFlight deep, overwriting one shared staging buffer next + // frame would clobber data the previous frame's copy is still + // reading. Indexing by frameCounter % framesInFlight gives each + // in-flight frame its own slot; a slot is rewritten only after + // framesInFlight frames have elapsed — the exact window after + // which single-queue submission order (the same guarantee the + // #101 deletion queue relies on) ensures that copy has completed. + const std::uint32_t ringSize = + Device::framesInFlight ? Device::framesInFlight : 1; + if (stagingRing.size() < ringSize) { + stagingRing.resize(ringSize); + } + VulkanBuffer& staging = + stagingRing[Device::frameCounter % ringSize]; // SHADER_DEVICE_ADDRESS: Create always queries the buffer device // address (and allocates with the device-address bit); the - // staging buffer's own address is otherwise unused. - staging.Create(VK_BUFFER_USAGE_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, count); + // staging buffer's own address is otherwise unused. Resize reuses + // the existing allocation (and its mapped pointer) when the + // upload still fits the slot's high-water capacity. + staging.Resize(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 = 0, .size = bytes }; vkCmdCopyBuffer(cmd, staging.buffer, buffer, 1, ®ion); - // The queued copy still reads the staging buffer after this call - // returns, so a plain Clear() would be a use-after-free; hand it - // to the fence-keyed queue (#101/#102), which frees it once the - // copy's frame clears its fence. DeferredClear nulls the handle, - // so `staging`'s destructor here is a no-op. - staging.DeferredClear(); srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; srcStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; } @@ -387,6 +413,17 @@ namespace Crafter { vkInvalidateMappedMemoryRanges(Device::device, 1, &range); } + // Persistent per-frame-in-flight staging ring for the staged + // UploadDeviceLocal path (issue #120). Empty — and so holding zero + // host-visible allocations — until the first upload that actually + // stages, which only happens on no-/small-BAR hardware (ReBAR/UMA always + // takes the direct-write branch and never touches this, so the lazy ring + // keeps the change a no-op there). Sized to Device::framesInFlight so + // each in-flight frame copies from its own slot; each slot grows to a + // high-water mark via Resize and is reused across frames rather than + // reallocated every call — mirroring the TLAS instance/metadata reuse. + std::vector> stagingRing; + VulkanBuffer(VulkanBuffer&& other) { buffer = other.buffer; memory = other.memory; @@ -397,6 +434,7 @@ namespace Crafter { memoryPropertyFlagsChosen = other.memoryPropertyFlagsChosen; other.buffer = VK_NULL_HANDLE; address = other.address; + stagingRing = std::move(other.stagingRing); if constexpr(Mapped) { VulkanBufferMappedConditional::value = other.VulkanBufferMappedConditional::value; }