Merge pull request 'perf(rt): allocate TLAS instance buffer in BAR/VRAM, not system RAM (#65)' (#105) from claude/issue-65 into master

implementations/Crafter.Graphics-RenderingElement3D.cpp

@@ -106,9 +106,24 @@ 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.
         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.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 | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT, primitiveCount);
         }
     }

tests/TLASHighWaterMark/main.cpp

@@ -50,6 +50,14 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
 //     — the strongest check that feeding an oversized instance buffer to the
 //     AS build (with tlasRangeInfo.primitiveCount < capacity) is spec-correct.
 //
+// Issue #65: this test also guards the instance buffer's memory-type request —
+// HOST_VISIBLE (required) with a best-effort DEVICE_LOCAL preference and no
+// mandatory HOST_COHERENT, so the per-frame GPU accesses stay in local VRAM
+// (BAR) rather than crossing PCIe. See the assertion block after the first
+// build. The zero-validation-errors check below also covers the dropped
+// HOST_COHERENT: the FlushDevice(cmd, ...) in BuildTLAS gates on the chosen
+// type's coherency flag, so a non-coherent BAR type still flushes correctly.
+//
 // Validation layers are required for the last check to be meaningful; the build
 // marks this test as needing the SDK layers.
 
@@ -207,6 +215,51 @@ int main() {
     Check(RenderingElement3D::tlases[0].builtInstanceCount == 4,
           "builtInstanceCount == 4 after first build");
 
+    // ── Issue #65: the instance buffer is allocated HOST_VISIBLE (required,
+    // mappable) with a best-effort DEVICE_LOCAL preference (BAR/VRAM) and the
+    // mandatory HOST_COHERENT dropped — so the per-frame compute-write of the
+    // transform field and the AS build's read of the instances both stay in
+    // local VRAM instead of crossing PCIe. The chosen type must match exactly
+    // what GetMemoryType resolves for this buffer's memoryTypeBits with that
+    // prefer-but-don't-require request: DEVICE_LOCAL when a host-visible
+    // device-local type is reachable (resizable BAR), plain HOST_VISIBLE
+    // otherwise. Asserting equality with the recomputed selection makes the
+    // check device-independent — it tracks the call-site request, not the
+    // particular GPU this runs on. ────────────────────────────────────────────
+    {
+        auto& ib = RenderingElement3D::tlases[0].instanceBuffer;
+        VkMemoryRequirements req{};
+        vkGetBufferMemoryRequirements(Device::device, ib.buffer, &req);
+        std::uint32_t expectIdx = Device::GetMemoryType(
+            req.memoryTypeBits,
+            VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT,
+            VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT);
+        VkMemoryPropertyFlags expectFlags =
+            Device::memoryProperties.memoryTypes[expectIdx].propertyFlags;
+        Check((ib.memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) != 0,
+              "instance buffer memory is HOST_VISIBLE (mappable — the required flag)");
+        Check(ib.memoryPropertyFlagsChosen == expectFlags,
+              "instance buffer memory matches the HOST_VISIBLE | prefer-DEVICE_LOCAL request (#65)");
+
+        bool barReachable = false;
+        constexpr VkMemoryPropertyFlags kBar =
+            VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
+        for (std::uint32_t i = 0; i < Device::memoryProperties.memoryTypeCount; ++i) {
+            if ((req.memoryTypeBits & (1u << i))
+                && (Device::memoryProperties.memoryTypes[i].propertyFlags & kBar) == kBar) {
+                barReachable = true;
+                break;
+            }
+        }
+        if (barReachable) {
+            Check((ib.memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT) != 0,
+                  "instance buffer landed on a DEVICE_LOCAL (BAR/VRAM) type where one is reachable");
+        } else {
+            std::println("INFO  no host-visible device-local type reachable — "
+                         "fell back to plain HOST_VISIBLE (expected on non-BAR GPUs)");
+        }
+    }
+
     // ── 2. Grow to 16 (past capacity) → REALLOCATES. ────────────────────────
     SetCount(pool, 16);
     BuildOnce();