Catcrafts/Crafter.Graphics
2
0
Fork 0
Code Issues 1 Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request 'perf(mesh): place RT geometry device-local via #89 upload strategy (#73)' (#109) from claude/issue-73 into master

Browse source
This commit is contained in:
catbot 2026-06-17 20:05:57 +02:00
commit 8b1db01222
5 changed files with 202 additions and 33 deletions
Download patch file Download diff file Expand all files Collapse all files

55
implementations/Crafter.Graphics-Mesh.cpp
View file

@ -32,10 +32,16 @@ using namespace Crafter;
namespace {
// Buffer-usage flag set shared by both Build paths. The compressed path
// appends VK_BUFFER_USAGE_2_MEMORY_DECOMPRESSION_BIT_EXT.
// appends VK_BUFFER_USAGE_2_MEMORY_DECOMPRESSION_BIT_EXT, and the staged
// upload path appends VK_BUFFER_USAGE_TRANSFER_DST_BIT. TRANSFER_SRC is in
// the base because the geometry is now device-local (#73): once it no longer
// lives in host-mappable memory, a transfer copy is the only way to read it
// back (debugging, GPU-driven workflows, decompression validation) — a free
// usage flag that keeps device-local geometry inspectable.
constexpr VkBufferUsageFlags2 kVertexUsageBase =
VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT
| VK_BUFFER_USAGE_2_ACCELERATION_STRUCTURE_BUILD_INPUT_READ_ONLY_BIT_KHR;
| VK_BUFFER_USAGE_2_ACCELERATION_STRUCTURE_BUILD_INPUT_READ_ONLY_BIT_KHR
| VK_BUFFER_USAGE_TRANSFER_SRC_BIT;
constexpr VkBufferUsageFlags2 kIndexUsageBase =
kVertexUsageBase | VK_BUFFER_USAGE_STORAGE_BUFFER_BIT;
@ -201,14 +207,13 @@ namespace {
}
void Mesh::Build(std::span<Vector<float, 3, 3>> verticies, std::span<std::uint32_t> indicies, VkCommandBuffer cmd, RTBuildOptions options) {
vertexBuffer.Resize(kVertexUsageBase, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, verticies.size());
indexBuffer.Resize(kIndexUsageBase, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, indicies.size());
std::memcpy(vertexBuffer.value, verticies.data(), verticies.size() * sizeof(Vector<float, 3, 3>));
std::memcpy(indexBuffer.value, indicies.data(), indicies.size() * sizeof(std::uint32_t));
vertexBuffer.FlushDevice(cmd, VK_ACCESS_MEMORY_READ_BIT, VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR);
indexBuffer.FlushDevice(cmd, VK_ACCESS_MEMORY_READ_BIT, VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR);
// Place both inputs in device-local memory (direct map on ReBAR/UMA,
// staged copy otherwise — UploadDeviceLocal decides) and barrier the
// upload before the BLAS build reads them. Replaces the previous
// HOST_VISIBLE allocation that the build (and any hit-shader fetch) would
// read over PCIe every frame (#73).
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);
indexBuffer.UploadDeviceLocal(kIndexUsageBase, indicies.data(), static_cast<std::uint32_t>(indicies.size()), cmd, VK_ACCESS_MEMORY_READ_BIT, VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR);
allowUpdate = options.allowUpdate;
builtInputCount = static_cast<std::uint32_t>(verticies.size());
@ -235,13 +240,17 @@ void Mesh::Build(const CompressedMeshAsset& asset, VkCommandBuffer cmd, RTBuildO
return;
}
// The GPU decompressor writes vertex/index directly (the build input is
// never host-written on this path), so they want pure DEVICE_LOCAL — no
// host visibility, no map. This is where the BLAS build and any hit-shader
// fetch then read them from (#73).
vertexBuffer.Resize(
kVertexUsageBase | VK_BUFFER_USAGE_2_MEMORY_DECOMPRESSION_BIT_EXT,
VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT,
VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT,
asset.vertexCount);
indexBuffer.Resize(
kIndexUsageBase | VK_BUFFER_USAGE_2_MEMORY_DECOMPRESSION_BIT_EXT,
VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT,
VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT,
asset.indexCount);
compressedStaging.Resize(
@ -327,14 +336,13 @@ namespace {
void RecordProceduralBuild(Mesh& self, std::span<const RTAabb> aabbs, VkBuildAccelerationStructureFlagsKHR flags, bool update, VkCommandBuffer cmd) {
// 24-byte-stride VkAabbPositionsKHR-compatible build input
// (static_assert'd in the interface). Same usage set as the triangle
// inputs: AS-build read-only + device address. A refit reuses the
// existing same-sized buffer (count is unchanged), so the device
// address — and the geometry it feeds — stays stable.
if (!update) {
self.aabbBuffer.Resize(kVertexUsageBase, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, static_cast<std::uint32_t>(aabbs.size()));
}
std::memcpy(self.aabbBuffer.value, aabbs.data(), aabbs.size() * sizeof(RTAabb));
self.aabbBuffer.FlushDevice(cmd, VK_ACCESS_MEMORY_READ_BIT, VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR);
// inputs: AS-build read-only + device address. UploadDeviceLocal places
// it in device-local memory (direct map on ReBAR/UMA, staged otherwise,
// #73) and barriers the upload before the build reads it. A refit
// re-uploads into the same same-sized buffer: Resize reuses the existing
// allocation (count unchanged), so the device address — and the geometry
// it feeds — stays stable for an in-place UPDATE.
self.aabbBuffer.UploadDeviceLocal(kVertexUsageBase, aabbs.data(), static_cast<std::uint32_t>(aabbs.size()), cmd, VK_ACCESS_MEMORY_READ_BIT, VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR);
RecordProceduralBuildFromAddress(self, self.aabbBuffer.address, static_cast<std::uint32_t>(aabbs.size()), sizeof(RTAabb), flags, update, cmd);
}
@ -392,8 +400,11 @@ 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.
std::memcpy(vertexBuffer.value, verticies.data(), verticies.size() * sizeof(Vector<float, 3, 3>));
vertexBuffer.FlushDevice(cmd, VK_ACCESS_MEMORY_READ_BIT, VK_PIPELINE_STAGE_ACCELERATION_STRUCTURE_BUILD_BIT_KHR);
// Re-upload only the vertex positions; UploadDeviceLocal reuses the same
// device-local allocation (count unchanged) so the address the UPDATE reads
// stays stable, then barriers the write before the build. On the staged
// path this re-stages per refit (the deforming-mesh fallback, #73).
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);
RecordBLASBuild(*this, static_cast<std::uint32_t>(verticies.size()), static_cast<std::uint32_t>(indicies.size()), buildFlags, /*update*/ true, cmd);
}

12
interfaces/Crafter.Graphics-Mesh.cppm
View file

@ -71,12 +71,18 @@ export namespace Crafter {
public:
VulkanBuffer<char, false> scratchBuffer;
VulkanBuffer<char, false> blasBuffer;
VulkanBuffer<Vector<float, 3, 3>, true> vertexBuffer;
VulkanBuffer<std::uint32_t, true> indexBuffer;
// Non-mapped (device-local) RT geometry: VulkanBuffer::UploadDeviceLocal
// places these in device memory and picks direct-map vs staged-copy per
// the #89 upload strategy, so they live in VRAM for hit-shader fetch and
// BLAS-build/refit reads instead of being read from system RAM over PCIe
// every trace (#73). The compressed Build path allocates them pure
// DEVICE_LOCAL and lets the GPU decompressor fill them directly.
VulkanBuffer<Vector<float, 3, 3>, false> vertexBuffer;
VulkanBuffer<std::uint32_t, false> indexBuffer;
// AABB build input for the procedural path (BuildProcedural).
// Lifetime contract matches vertexBuffer/indexBuffer: must stay
// alive until the build submitted on `cmd` completes.
VulkanBuffer<RTAabb, true> aabbBuffer;
VulkanBuffer<RTAabb, false> aabbBuffer;
// Transient host-visible staging for the compressed Build path. Kept as
// a member only so the recorded vkCmdDecompressMemoryEXT has a stable
// address to reference; the compressed Build releases it via

77
interfaces/Crafter.Graphics-VulkanBuffer.cppm
View file

@ -235,6 +235,83 @@ namespace Crafter {
);
}
// Upload `count` host elements from `src` into this buffer as
// device-local, GPU-read geometry, choosing placement at runtime via
// the #89 upload strategy (Device::PreferDirectDeviceWrite) and then
// recording a barrier from the upload to (dstStageMask, dstAccessMask)
// on `cmd`:
// ReBAR / UMA (direct) — allocate HOST_VISIBLE with DEVICE_LOCAL as a
// best-effort preference (GetMemoryType lands the host-visible
// device-local type that the strategy already proved exists), map
// transiently, memcpy, flush if the chosen type isn't coherent,
// unmap. No staging buffer: it would be pure overhead on a bar where
// the device-local heap is itself host-writable.
// No / small BAR (staged) — allocate pure DEVICE_LOCAL (+ TRANSFER_DST),
// fill a transient HOST_VISIBLE staging buffer, vkCmdCopyBuffer into
// this buffer, then hand the staging buffer to the fence-keyed
// deferred-deletion queue (#101/#102) so it outlives the copy submit.
// Requires !Mapped: the destination must be free to be device-local-only
// (a persistent map would force HOST_VISIBLE), so the direct path maps
// just long enough to write. A same-size re-upload reuses the allocation
// (Resize), so the device address stays stable across an in-place AS
// UPDATE refit that re-calls this.
void UploadDeviceLocal(VkBufferUsageFlags2 usageFlags, const T* src, std::uint32_t count, VkCommandBuffer cmd, VkAccessFlags dstAccessMask, VkPipelineStageFlags dstStageMask) requires(!Mapped) {
VkDeviceSize bytes = static_cast<VkDeviceSize>(count) * sizeof(T);
VkAccessFlags srcAccessMask;
VkPipelineStageFlags srcStageMask;
if (Device::PreferDirectDeviceWrite(bytes)) {
Resize(usageFlags, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, count, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT);
void* mapped = nullptr;
Device::CheckVkResult(vkMapMemory(Device::device, memory, 0, VK_WHOLE_SIZE, 0, &mapped));
std::memcpy(mapped, src, bytes);
// Match FlushDevice()'s gate: a non-coherent type needs an
// explicit flush before the device reads the written range.
if (!(memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT)) {
VkMappedMemoryRange range {
.sType = VK_STRUCTURE_TYPE_MAPPED_MEMORY_RANGE,
.memory = memory,
.offset = 0,
.size = VK_WHOLE_SIZE
};
vkFlushMappedMemoryRanges(Device::device, 1, &range);
}
vkUnmapMemory(Device::device, memory);
srcAccessMask = VK_ACCESS_HOST_WRITE_BIT;
srcStageMask = VK_PIPELINE_STAGE_HOST_BIT;
} else {
Resize(usageFlags | VK_BUFFER_USAGE_TRANSFER_DST_BIT, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, count);
VulkanBuffer<T, true> staging;
// 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);
std::memcpy(staging.value, src, bytes);
staging.FlushDevice();
VkBufferCopy region { .srcOffset = 0, .dstOffset = 0, .size = bytes };
vkCmdCopyBuffer(cmd, staging.buffer, buffer, 1, &region);
// 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;
}
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 = 0,
.size = VK_WHOLE_SIZE
};
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.

50
tests/BLASBuildOptions/main.cpp
View file

@ -132,6 +132,15 @@ int main() {
// refit below can reuse it (build scratch ≥ update scratch, no resize).
Check(tri.scratchBuffer.buffer != VK_NULL_HANDLE,
"allowUpdate=true → scratch retained for refit (issue #66)");
// Geometry is placed in device-local memory regardless of the upload
// strategy taken (issue #73): the direct ReBAR/UMA path lands a
// HOST_VISIBLE | DEVICE_LOCAL type, the staged path a pure DEVICE_LOCAL
// one — either way the DEVICE_LOCAL bit is set, so the BLAS build and any
// hit-shader fetch read from VRAM, not system RAM.
Check((tri.vertexBuffer.memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT) != 0,
"triangle vertexBuffer placed in device-local memory (#73)");
Check((tri.indexBuffer.memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT) != 0,
"triangle indexBuffer placed in device-local memory (#73)");
const VkDeviceAddress triAddrBefore = tri.blasAddr;
const VkAccelerationStructureKHR triHandleBefore = tri.accelerationStructure;
@ -198,6 +207,8 @@ int main() {
Check(proc.builtPrimitiveCount == 2, "procedural BLAS reports 2 primitives");
Check(proc.scratchBuffer.buffer != VK_NULL_HANDLE,
"procedural allowUpdate=true → scratch retained for refit (issue #66)");
Check((proc.aabbBuffer.memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT) != 0,
"procedural aabbBuffer placed in device-local memory (#73)");
const VkDeviceAddress procAddrBefore = proc.blasAddr;
const VkAccelerationStructureKHR procHandleBefore = proc.accelerationStructure;
@ -293,6 +304,45 @@ int main() {
Check(devProc.aabbBuffer.buffer == VK_NULL_HANDLE,
"device-buffer refit still never touched the host aabbBuffer");
// ── Force the STAGED upload path (issue #73). On this ReBAR dev host the
// direct map path is normally taken; temporarily zero the cached upload
// budget so PreferDirectDeviceWrite returns false, exercising Mesh's
// pure-DEVICE_LOCAL + transient-staging-buffer + vkCmdCopyBuffer path
// (and its deferred-deletion of the staging buffer). The validation
// layer check below is the real assertion that the staged copy and its
// barriers are spec-correct on a fresh build and an in-place refit. ───
{
const VkDeviceSize savedBudget = Device::directWriteBudget;
Device::directWriteBudget = 0; // 0 → PreferDirectDeviceWrite is always false
Mesh staged;
auto verts = CubeVerts(1.0f);
auto idx = CubeIndices();
{
VkCommandBuffer cmd = BeginCmd();
staged.Build(verts, idx, cmd, RTBuildOptions{
.preference = RTBuildPreference::FastTrace, .allowUpdate = true });
SubmitWait(cmd);
}
Check(staged.blasAddr != 0, "staged-upload Build produced a non-zero blasAddr (#73)");
// Staged geometry lives in pure DEVICE_LOCAL — no HOST_VISIBLE bit.
Check((staged.vertexBuffer.memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) == 0,
"staged vertexBuffer is device-local-only, not host-visible (#73)");
const VkAccelerationStructureKHR stagedHandle = staged.accelerationStructure;
{
// Same topology, deformed positions → in-place UPDATE, re-staged.
auto verts2 = CubeVerts(1.5f);
VkCommandBuffer cmd = BeginCmd();
staged.Refit(verts2, idx, cmd);
SubmitWait(cmd);
}
Check(staged.accelerationStructure == stagedHandle,
"staged-upload Refit kept the same AS handle (in-place UPDATE, #73)");
Device::directWriteBudget = savedBudget;
}
Check(Device::validationErrorCount == 0,
std::format("no Vulkan validation errors ({} seen)", Device::validationErrorCount));

41
tests/MeshDecompressStagingRelease/main.cpp
View file

@ -146,7 +146,13 @@ int main() {
Mesh mesh;
VkCommandBuffer cmd = BeginCmd();
mesh.Build(asset, cmd);
// allowUpdate=true so the per-mesh BLAS scratch is RETAINED for refit
// (issue #66) rather than deferred-cleared after the build. A static
// (allowUpdate=false) build would also hand its scratch to the deletion
// queue, making the count below 2; keeping it isolates this test to the
// one allocation it cares about — the released compressed staging (#67).
// The staging-release behavior under test is independent of allowUpdate.
mesh.Build(asset, cmd, RTBuildOptions{ .allowUpdate = true });
// ── The fix: staging is released to the queue at record time, not pinned. ──
Check(mesh.compressedStaging.buffer == VK_NULL_HANDLE,
@ -168,16 +174,35 @@ int main() {
Check(Device::validationErrorCount == 0,
"decompress + BLAS build raised no validation errors");
// The GPU decompress wrote into the host-visible vertex/index buffers;
// invalidate and read them back — releasing the staging must not corrupt
// the decompressed result.
mesh.vertexBuffer.FlushHost();
mesh.indexBuffer.FlushHost();
// The GPU decompress wrote into the device-local vertex/index buffers
// (issue #73 placed RT geometry in VRAM, so they are no longer host-mapped).
// Copy them back to host-visible staging — the proper way to inspect
// device-local memory — and compare: releasing the compressed staging must
// not have corrupted the decompressed result. The geometry buffers carry
// TRANSFER_SRC for exactly this (#73).
VulkanBuffer<Vector<float, 3, 3>, true> vertReadback;
VulkanBuffer<std::uint32_t, true> idxReadback;
vertReadback.Resize(VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT,
VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, static_cast<std::uint32_t>(srcMesh.vertexes.size()));
idxReadback.Resize(VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT,
VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, static_cast<std::uint32_t>(srcMesh.indexes.size()));
{
// Safe to copy without a barrier: SubmitWait above wait-idled the queue,
// so the decompress + BLAS reads of these buffers have fully retired.
VkCommandBuffer rcmd = BeginCmd();
VkBufferCopy vRegion { .srcOffset = 0, .dstOffset = 0, .size = vertReadback.size };
vkCmdCopyBuffer(rcmd, mesh.vertexBuffer.buffer, vertReadback.buffer, 1, &vRegion);
VkBufferCopy iRegion { .srcOffset = 0, .dstOffset = 0, .size = idxReadback.size };
vkCmdCopyBuffer(rcmd, mesh.indexBuffer.buffer, idxReadback.buffer, 1, &iRegion);
SubmitWait(rcmd);
}
vertReadback.FlushHost();
idxReadback.FlushHost();
const bool vertsMatch = std::memcmp(
mesh.vertexBuffer.value, srcMesh.vertexes.data(),
vertReadback.value, srcMesh.vertexes.data(),
srcMesh.vertexes.size() * sizeof(srcMesh.vertexes[0])) == 0;
const bool idxMatch = std::memcmp(
mesh.indexBuffer.value, srcMesh.indexes.data(),
idxReadback.value, srcMesh.indexes.data(),
srcMesh.indexes.size() * sizeof(srcMesh.indexes[0])) == 0;
Check(vertsMatch, "GPU-decompressed vertices are byte-equal to the source");
Check(idxMatch, "GPU-decompressed indices are byte-equal to the source");