Merge remote-tracking branch 'origin/master' into claude/issue-73

# Conflicts:
#	interfaces/Crafter.Graphics-Mesh.cppm

implementations/Crafter.Graphics-Mesh.cpp

@@ -281,6 +281,16 @@ void Mesh::Build(const CompressedMeshAsset& asset, VkCommandBuffer cmd, RTBuildO
         VK_PIPELINE_STAGE_2_ACCELERATION_STRUCTURE_BUILD_BIT_KHR,
         VK_ACCESS_2_ACCELERATION_STRUCTURE_READ_BIT_KHR);
 
+    // The compressed staging is only read by the decompress recorded above; the
+    // subsequent BLAS build reads the decompressed vertex/index buffers, never
+    // this. So hand it to the fence-keyed deletion queue (#101/#102) now rather
+    // than pinning host-visible memory for the mesh's whole life. The recorded
+    // vkCmdDecompressMemoryEXT still references compressedStaging.address, so it
+    // must outlive this submit — which the queue guarantees: it retires the
+    // allocation only after framesInFlight frames have elapsed, by which point
+    // the decompress submit's fence has cleared.
+    compressedStaging.DeferredClear();
+
     allowUpdate     = options.allowUpdate;
     builtInputCount = asset.vertexCount;
     RecordBLASBuild(*this, asset.vertexCount, asset.indexCount, BlasFlags(options), /*update*/ false, cmd);

implementations/Crafter.Graphics-RenderingElement3D.cpp

@@ -106,10 +106,38 @@ void RenderingElement3D::BuildTLAS(VkCommandBuffer cmd, std::uint32_t index, RTB
         // STORAGE_BUFFER_BIT is required because the application's compute
         // shaders bind these buffers as storage SSBOs (e.g. to write
         // per-instance transforms directly into the TLAS instance data).
+        //
+        // instanceBuffer prefers HOST_VISIBLE | DEVICE_LOCAL (BAR/VRAM) so the
+        // two per-frame GPU accesses stay on-chip instead of crossing PCIe: the
+        // compute shader writes the transform field in place, and the AS build
+        // reads the whole instance. The CPU still writes the host-authored
+        // fields below — those are write-only, sequential, never-read-back
+        // writes, the ideal write-combined BAR workload. DEVICE_LOCAL is a
+        // best-effort preference: GetMemoryType falls back to plain
+        // HOST_VISIBLE (the previous behaviour) when no combined type exists
+        // (no resizable BAR). HOST_COHERENT is intentionally dropped from the
+        // required set — the combined type may not be coherent, and the
+        // FlushDevice(cmd, ...) below already establishes host->build
+        // 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 | 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 | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT, primitiveCount);
+            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, primitiveCount, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT);
         }
     }
 
@@ -132,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,

implementations/Crafter.Graphics-Window.cpp

@@ -1200,11 +1200,29 @@ void Window::SaveFrame(const std::filesystem::path& path) {
 
     VkMemoryRequirements memReqs;
     vkGetBufferMemoryRequirements(Device::device, stagingBuf, &memReqs);
+
+    // Readback path: the CPU *reads* this whole buffer back. The first
+    // HOST_VISIBLE|HOST_COHERENT type is usually write-combined (uncached) on
+    // discrete GPUs, where CPU reads are pathologically slow. Ask for
+    // HOST_CACHED so reads hit the CPU cache, preferring a cached-coherent type
+    // when one exists. HOST_CACHED is near-universal but not spec-guaranteed,
+    // so fall back to any HOST_VISIBLE type (#59) if absent — that path then
+    // relies on the coherency-gated invalidate below. (Deliberately NOT routed
+    // through the upload-strategy helper, which steers toward write-combined
+    // DEVICE_LOCAL|HOST_VISIBLE — the worst type for readback. See issue #89.)
+    std::uint32_t memTypeIndex;
+    try {
+        memTypeIndex = Device::GetMemoryType(memReqs.memoryTypeBits,
+            VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_CACHED_BIT,
+            VK_MEMORY_PROPERTY_HOST_COHERENT_BIT);
+    } catch (const std::runtime_error&) {
+        memTypeIndex = Device::GetMemoryType(memReqs.memoryTypeBits,
+            VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT);
+    }
     VkMemoryAllocateInfo mai{
         .sType           = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO,
         .allocationSize  = memReqs.size,
-        .memoryTypeIndex = Device::GetMemoryType(memReqs.memoryTypeBits,
-            VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT),
+        .memoryTypeIndex = memTypeIndex,
     };
     VkDeviceMemory stagingMem = VK_NULL_HANDLE;
     Device::CheckVkResult(vkAllocateMemory(Device::device, &mai, nullptr, &stagingMem));
@@ -1289,6 +1307,24 @@ void Window::SaveFrame(const std::filesystem::path& path) {
     // Read back, swizzle BGRA → RGBA if needed, write PNG.
     void* mapped = nullptr;
     Device::CheckVkResult(vkMapMemory(Device::device, stagingMem, 0, VK_WHOLE_SIZE, 0, &mapped));
+
+    // If the chosen type is cached-but-not-coherent, the transfer's writes may
+    // not yet be visible to the CPU read; invalidate to pull them in. Gate on
+    // the chosen type's actual coherency (not the requested flags) — a coherent
+    // type needs no invalidate. Whole-range invalidate sidesteps
+    // nonCoherentAtomSize. This is the read-path counterpart of FlushDevice (#60).
+    if (!(Device::memoryProperties.memoryTypes[memTypeIndex].propertyFlags &
+          VK_MEMORY_PROPERTY_HOST_COHERENT_BIT)) {
+        VkMappedMemoryRange invalidateRange{
+            .sType  = VK_STRUCTURE_TYPE_MAPPED_MEMORY_RANGE,
+            .memory = stagingMem,
+            .offset = 0,
+            .size   = VK_WHOLE_SIZE,
+        };
+        Device::CheckVkResult(
+            vkInvalidateMappedMemoryRanges(Device::device, 1, &invalidateRange));
+    }
+
     const std::uint8_t* src = static_cast<const std::uint8_t*>(mapped);
 
     std::vector<std::uint8_t> rgba(static_cast<std::size_t>(width) * height * 4);