Merge pull request 'perf(ui): flush only the written descriptor range, not the whole heap (#61)' (#99) from claude/issue-61 into master

implementations/Crafter.Graphics-Device.cpp

@@ -698,6 +698,10 @@ void Device::Initialize() {
     // (vendorID / deviceID / pipelineCacheUUID).
     deviceProperties = properties2.properties;
 
+    // Cache the flush/invalidate alignment for ranged host-memory flushes
+    // (VulkanBuffer::FlushDevice(offset,bytes)).
+    nonCoherentAtomSize = properties2.properties.limits.nonCoherentAtomSize;
+
     // NVIDIA's brand-new VK_EXT_descriptor_heap acceleration-structure read
     // path faults (see #7); enable the SPIR-V rewrite workaround there. Other
     // drivers (and any future fixed NVIDIA driver, once this gate is removed)

implementations/Crafter.Graphics-UI.cpp

@@ -67,16 +67,16 @@ void UIRenderer::Initialize(Window& window, GraphicsDescriptorHeap& heap, Graphi
         WriteFontAtlasDescriptor();
     }
 
-    // Flush the host-mapped descriptor heaps so the GPU sees what we wrote.
+    // No whole-heap flush here: WriteSwapchainDescriptors / WriteFontAtlasDescriptor
-    for (auto& h : heap_->resourceHeap) h.FlushDevice();
+    // / RegisterSampler each flush their own written byte range above, so the GPU
-    for (auto& h : heap_->samplerHeap)  h.FlushDevice();
+    // already sees everything written during setup.
 
     // Re-bind the swapchain image descriptors after every resize. Window
     // already drained the queue and rebuilt the imageViews[] before
     // firing the event, so vkWriteResourceDescriptorsEXT is safe here.
+    // WriteSwapchainDescriptors flushes its own range, so no extra flush needed.
     resizeSub_.SetEvent(&window.onResize, [this]() {
         WriteSwapchainDescriptors();
-        for (auto& h : heap_->resourceHeap) h.FlushDevice();
     });
 
     (void)initCmd; // reserved for future image-layout tweaks
@@ -315,6 +315,12 @@ void UIRenderer::WriteSwapchainDescriptors() {
     Device::vkWriteResourceDescriptorsEXT(
         Device::device, Window::numFrames, resources.data(), destinations.data()
     );
+    // Only the one image descriptor at outImageSlot_ changed in each per-frame
+    // heap (same slot offset in every frame), so flush just that byte range
+    // rather than the whole multi-KB heap.
+    const std::uint32_t off = heap_->ImageByteOffset(outImageSlot_);
+    const std::uint32_t sz  = Device::descriptorHeapProperties.imageDescriptorSize;
+    for (auto& h : heap_->resourceHeap) h.FlushDevice(off, sz);
 }
 
 void UIRenderer::WriteFontAtlasDescriptor() {
@@ -366,7 +372,11 @@ void UIRenderer::WriteSampledImageDescriptor(std::uint16_t slot,
     Device::vkWriteResourceDescriptorsEXT(
         Device::device, Window::numFrames, resources.data(), destinations.data()
     );
-    for (auto& h : heap_->resourceHeap) h.FlushDevice();
+    // One image descriptor written per heap, at the same slot offset — flush
+    // just that range rather than the whole heap.
+    const std::uint32_t off = heap_->ImageByteOffset(slot);
+    const std::uint32_t sz  = Device::descriptorHeapProperties.imageDescriptorSize;
+    for (auto& h : heap_->resourceHeap) h.FlushDevice(off, sz);
 }
 
 void UIRenderer::WriteBufferDescriptor(std::uint16_t slot, VkDeviceAddress address, std::uint32_t size) {
@@ -389,7 +399,11 @@ void UIRenderer::WriteBufferDescriptor(std::uint16_t slot, VkDeviceAddress addre
     Device::vkWriteResourceDescriptorsEXT(
         Device::device, Window::numFrames, resources.data(), destinations.data()
     );
-    for (auto& h : heap_->resourceHeap) h.FlushDevice();
+    // One buffer descriptor written per heap, at the same slot offset — flush
+    // just that range rather than the whole heap.
+    const std::uint32_t off = heap_->BufferByteOffset(slot);
+    const std::uint32_t sz  = Device::descriptorHeapProperties.bufferDescriptorSize;
+    for (auto& h : heap_->resourceHeap) h.FlushDevice(off, sz);
 }
 
 SamplerSlot UIRenderer::RegisterSampler(const VkSamplerCreateInfo& info) {
@@ -406,7 +420,11 @@ SamplerSlot UIRenderer::RegisterSampler(const VkSamplerCreateInfo& info) {
     Device::vkWriteSamplerDescriptorsEXT(
         Device::device, Window::numFrames, infos.data(), destinations.data()
     );
-    for (auto& h : heap_->samplerHeap) h.FlushDevice();
+    // One sampler descriptor written per heap, at the same slot offset — flush
+    // just that range rather than the whole sampler heap.
+    const std::uint32_t off = heap_->SamplerByteOffset(range.firstElement);
+    const std::uint32_t sz  = Device::descriptorHeapProperties.samplerDescriptorSize;
+    for (auto& h : heap_->samplerHeap) h.FlushDevice(off, sz);
     return SamplerSlot{heap_, range.firstElement};
 }
 

interfaces/Crafter.Graphics-Device.cppm

@@ -161,6 +161,14 @@ export namespace Crafter {
 
         inline static VkPhysicalDeviceMemoryProperties memoryProperties;
 
+        // VkPhysicalDeviceLimits::nonCoherentAtomSize — the alignment (in bytes,
+        // a power of two) that vkFlushMappedMemoryRanges / vkInvalidateMappedMemoryRanges
+        // require for the offset and size of a sub-buffer range. Whole-buffer
+        // (VK_WHOLE_SIZE) flushes sidestep it; ranged flushes must round outward
+        // to it (see VulkanBuffer::FlushDevice(offset,bytes)). Populated at device
+        // creation; defaults to 1 so the rounding math is well-defined before then.
+        inline static VkDeviceSize nonCoherentAtomSize = 1;
+
         // Core physical-device properties, captured once at Initialize from the
         // VkPhysicalDeviceProperties2 query. Kept because the pipeline-cache
         // persistence path needs vendorID / deviceID / pipelineCacheUUID to

interfaces/Crafter.Graphics-VulkanBuffer.cppm

@@ -29,10 +29,35 @@ import std;
 import :Device;
 
 namespace Crafter {
+    // Round a host-write flush range outward to nonCoherentAtomSize boundaries —
+    // the alignment vkFlushMappedMemoryRanges demands for a sub-buffer range
+    // (whole-buffer VK_WHOLE_SIZE calls sidestep it). `atom` is
+    // VkPhysicalDeviceLimits::nonCoherentAtomSize (a power of two ≥ 1) and
+    // `mappingSize` is the byte size the memory was mapped with (the allocation
+    // size). The start rounds down and the end rounds up to atom multiples; the
+    // end is then clamped to `mappingSize`. Clamping is what keeps the range
+    // valid even when `mappingSize` itself is not atom-aligned: the spec permits
+    // a non-atom-multiple size only when offset+size equals the mapping size, and
+    // the clamp lands exactly on that exception. Pure math, so it is unit-tested
+    // without a device.
+    export struct MappedFlushRange { VkDeviceSize offset; VkDeviceSize size; };
+    export constexpr MappedFlushRange AlignMappedFlushRange(
+            VkDeviceSize offset, VkDeviceSize bytes,
+            VkDeviceSize atom, VkDeviceSize mappingSize) {
+        VkDeviceSize begin = (offset / atom) * atom;
+        VkDeviceSize end   = ((offset + bytes + atom - 1) / atom) * atom;
+        if (end > mappingSize) end = mappingSize;
+        return { begin, end - begin };
+    }
+
     export class VulkanBufferBase {
         public:
         VkDeviceAddress address;
         std::uint32_t size;
+        // Byte size the memory was mapped with — equal to the allocation's
+        // VkMemoryRequirements::size, which is ≥ `size` and what ranged flushes
+        // clamp their upper bound to. Only meaningful for mapped buffers.
+        VkDeviceSize mappedSize = 0;
         VkBuffer buffer = VK_NULL_HANDLE;
         VkDeviceMemory memory;
         // Property flags of the memory type actually chosen by GetMemoryType —
@@ -116,6 +141,7 @@ namespace Crafter {
             address = vkGetBufferDeviceAddress(Device::device, &addressInfo);
 
             if constexpr(Mapped) {
+                mappedSize = memReqs.size;
                 Device::CheckVkResult(vkMapMemory(Device::device, memory, 0, memReqs.size, 0, reinterpret_cast<void**>(&(VulkanBufferMappedConditional<T, true>::value))));
             }
         }
@@ -208,6 +234,27 @@ namespace Crafter {
             vkFlushMappedMemoryRanges(Device::device, 1, &range);
         }
 
+        // Flush only the host writes in [offset, offset+bytes) to the device,
+        // instead of the whole buffer. Use after touching a small sub-range
+        // (e.g. one descriptor in a multi-KB descriptor heap) so cache
+        // maintenance scales with the bytes actually written. The range is
+        // rounded outward to nonCoherentAtomSize as the Vulkan spec requires.
+        // No-op on coherent memory, same gate as the whole-buffer FlushDevice().
+        void FlushDevice(VkDeviceSize offset, VkDeviceSize bytes) requires(Mapped) {
+            if (memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT) {
+                return;
+            }
+            MappedFlushRange r = AlignMappedFlushRange(
+                offset, 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);
+        }
+
         void FlushDevice(VkCommandBuffer cmd, VkAccessFlags dstAccessMask, VkPipelineStageFlags dstStageMask) requires(Mapped) {
             FlushDevice();
             VkBufferMemoryBarrier barrier = {
@@ -251,6 +298,7 @@ namespace Crafter {
             buffer = other.buffer;
             memory = other.memory;
             size = other.size;
+            mappedSize = other.mappedSize;
             capacity = other.capacity;
             usageFlagsCreated = other.usageFlagsCreated;
             memoryPropertyFlagsChosen = other.memoryPropertyFlagsChosen;

project.cpp

@@ -530,6 +530,33 @@ extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> a
         fc.GetInterfacesAndImplementations(ifaces, flushImpls);
         cfg.tests.push_back(std::move(flushTest));
 
+        // Ranged FlushDevice: UI descriptor registration now flushes only the
+        // written descriptor byte range instead of the whole heap, rounding the
+        // range outward to nonCoherentAtomSize via AlignMappedFlushRange. The
+        // rounding is pure math and the coherent gate is pure logic, so this
+        // test drives both directly with no GPU device.
+        Test rangedFlushTest;
+        Configuration& rfc = rangedFlushTest.config;
+        rfc.path                 = cfg.path;
+        rfc.name                 = "VulkanBufferRangedFlush";
+        rfc.outputName           = "VulkanBufferRangedFlush";
+        rfc.type                 = ConfigurationType::Executable;
+        rfc.target               = cfg.target;
+        rfc.march                = cfg.march;
+        rfc.mtune                = cfg.mtune;
+        rfc.debug                = cfg.debug;
+        rfc.sysroot              = cfg.sysroot;
+        rfc.dependencies         = cfg.dependencies;
+        rfc.externalDependencies = cfg.externalDependencies;
+        rfc.compileFlags         = cfg.compileFlags;
+        rfc.linkFlags            = cfg.linkFlags;
+        rfc.defines              = cfg.defines;
+        rfc.cFiles               = cfg.cFiles;
+        std::vector<fs::path> rangedFlushImpls(impls.begin(), impls.end());
+        rangedFlushImpls.emplace_back("tests/VulkanBufferRangedFlush/main");
+        rfc.GetInterfacesAndImplementations(ifaces, rangedFlushImpls);
+        cfg.tests.push_back(std::move(rangedFlushTest));
+
         // Issue #63: VulkanBuffer::Resize now reuses the existing allocation in
         // place when a new request still fits the created capacity and the
         // immutable-at-create properties match (usage flags fixed at create;

tests/VulkanBufferRangedFlush/main.cpp

@@ -0,0 +1,124 @@
+/*
+Crafter®.Graphics
+Copyright (C) 2026 Catcrafts®
+catcrafts.net
+
+This library is free software; you can redistribute it and/or
+modify it under the terms of the GNU Lesser General Public
+License version 3.0 as published by the Free Software Foundation;
+
+This library is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public
+License along with this library; if not, write to the Free Software
+Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
+*/
+
+// Regression test for the ranged FlushDevice (UI descriptor registration):
+// WriteBufferDescriptor / WriteSampledImageDescriptor / RegisterSampler used to
+// FlushDevice() the whole multi-KB descriptor heap after writing one few-byte
+// descriptor. They now flush only the written byte range via
+// VulkanBuffer::FlushDevice(offset, bytes), which rounds the range outward to
+// VkPhysicalDeviceLimits::nonCoherentAtomSize (vkFlushMappedMemoryRanges rejects
+// an unaligned sub-buffer offset/size).
+//
+// The rounding lives in AlignMappedFlushRange — pure math with no Vulkan
+// dependency — so this drives it directly with no GPU device, and also confirms
+// the ranged FlushDevice keeps the issue-#60 coherent-memory gate (a real flush
+// would fault with Device::device == VK_NULL_HANDLE).
+
+#include <cstdlib>
+#include "vulkan/vulkan.h"
+
+import Crafter.Graphics;
+import std;
+using namespace Crafter;
+
+namespace {
+
+int failures = 0;
+
+void Check(bool ok, std::string_view what) {
+    std::println("{}  {}", ok ? "PASS" : "FAIL", what);
+    if (!ok) ++failures;
+}
+
+constexpr auto HOST_VISIBLE  = VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT;
+constexpr auto HOST_COHERENT = VK_MEMORY_PROPERTY_HOST_COHERENT_BIT;
+constexpr auto DEVICE_LOCAL  = VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
+
+} // namespace
+
+int main() {
+    Check(Device::device == VK_NULL_HANDLE,
+          "no Vulkan device created — a real flush would fault");
+
+    // ── AlignMappedFlushRange: the spec demands offset and size be multiples
+    //    of nonCoherentAtomSize (atom). Start rounds down, end rounds up. ──
+    {
+        // A descriptor wholly inside one atom expands to cover that atom.
+        auto r = AlignMappedFlushRange(/*offset*/ 80, /*bytes*/ 16,
+                                       /*atom*/ 64, /*mappingSize*/ 4096);
+        Check(r.offset == 64 && r.size == 64,
+              "sub-atom range rounds out to the enclosing atom [64,128)");
+        Check(r.offset % 64 == 0 && r.size % 64 == 0,
+              "rounded offset and size are atom multiples");
+    }
+    {
+        // A range straddling an atom boundary expands both ways.
+        auto r = AlignMappedFlushRange(/*offset*/ 60, /*bytes*/ 8,
+                                       /*atom*/ 64, /*mappingSize*/ 4096);
+        Check(r.offset == 0 && r.size == 128,
+              "straddling range rounds out to [0,128)");
+    }
+    {
+        // Already-aligned range is left exactly as is.
+        auto r = AlignMappedFlushRange(/*offset*/ 128, /*bytes*/ 64,
+                                       /*atom*/ 64, /*mappingSize*/ 4096);
+        Check(r.offset == 128 && r.size == 64,
+              "already atom-aligned range is unchanged");
+    }
+    {
+        // atom == 1 (no coherence granularity): range passes through verbatim.
+        auto r = AlignMappedFlushRange(/*offset*/ 80, /*bytes*/ 16,
+                                       /*atom*/ 1, /*mappingSize*/ 4096);
+        Check(r.offset == 80 && r.size == 16,
+              "atom==1 leaves the range exact");
+    }
+    {
+        // Near the end of a non-atom-aligned mapping: the rounded-up end is
+        // clamped to mappingSize so offset+size == mapping size (the spec's
+        // exception that permits a non-atom-multiple size).
+        auto r = AlignMappedFlushRange(/*offset*/ 4030, /*bytes*/ 8,
+                                       /*atom*/ 64, /*mappingSize*/ 4040);
+        Check(r.offset == 3968 && r.offset + r.size == 4040,
+              "tail range clamps its end to the mapping size");
+        Check(r.offset % 64 == 0,
+              "clamped range still has an atom-aligned offset");
+    }
+
+    // ── Ranged FlushDevice keeps the coherent-memory gate (issue #60). ──
+    {
+        VulkanBuffer<std::uint8_t, true> buf;
+        buf.memoryPropertyFlagsChosen = HOST_VISIBLE | HOST_COHERENT;
+        buf.mappedSize = 4096;
+        buf.FlushDevice(/*offset*/ 80, /*bytes*/ 16); // must early-return
+        Check(true, "ranged FlushDevice skips the Vulkan call on coherent memory");
+    }
+    {
+        VulkanBuffer<std::uint8_t, true> buf;
+        buf.memoryPropertyFlagsChosen = HOST_VISIBLE | DEVICE_LOCAL;
+        Check(!(buf.memoryPropertyFlagsChosen & HOST_COHERENT),
+              "non-coherent chosen type is not gated as coherent (ranged flush still required)");
+    }
+
+    if (failures != 0) {
+        std::println("{} check(s) failed", failures);
+        return EXIT_FAILURE;
+    }
+    std::println("all checks passed");
+    return EXIT_SUCCESS;
+}