From 6d7ad87e38c2561ab019bb41c69281ee862f1b27 Mon Sep 17 00:00:00 2001 From: catbot Date: Tue, 16 Jun 2026 18:29:24 +0000 Subject: [PATCH] perf(ui): flush only the written descriptor range, not the whole heap (#61) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WriteBufferDescriptor / WriteSampledImageDescriptor / RegisterSampler / WriteSwapchainDescriptors each FlushDevice()'d the entire multi-KB per-frame descriptor heap after writing one few-byte descriptor. Add a ranged VulkanBuffer::FlushDevice(offset, bytes) that flushes only the touched bytes, rounding the range outward to nonCoherentAtomSize as vkFlushMappedMemoryRanges requires (the WHOLE_SIZE path sidestepped that). The coherent-memory gate from issue #60 is preserved — coherent memory still skips the flush entirely. The descriptor writers now self-flush their written range, so the redundant whole-heap flushes in UIRenderer::Initialize and the resize callback are gone. Rounding lives in AlignMappedFlushRange (pure math) and is unit-tested without a device in the new VulkanBufferRangedFlush test; the change was also verified end-to-end by running HelloUI on a real GPU with validation layers (font-atlas glyphs, sampler and storage-image descriptors all flush correctly, no VUIDs). Co-Authored-By: Claude Opus 4.8 --- implementations/Crafter.Graphics-Device.cpp | 4 + implementations/Crafter.Graphics-UI.cpp | 32 ++++- interfaces/Crafter.Graphics-Device.cppm | 8 ++ interfaces/Crafter.Graphics-VulkanBuffer.cppm | 48 +++++++ project.cpp | 27 ++++ tests/VulkanBufferRangedFlush/main.cpp | 124 ++++++++++++++++++ 6 files changed, 236 insertions(+), 7 deletions(-) create mode 100644 tests/VulkanBufferRangedFlush/main.cpp diff --git a/implementations/Crafter.Graphics-Device.cpp b/implementations/Crafter.Graphics-Device.cpp index 25b18fb..7428f29 100644 --- a/implementations/Crafter.Graphics-Device.cpp +++ b/implementations/Crafter.Graphics-Device.cpp @@ -604,6 +604,10 @@ void Device::Initialize() { }; vkGetPhysicalDeviceProperties2(physDevice, &properties2); + // 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) diff --git a/implementations/Crafter.Graphics-UI.cpp b/implementations/Crafter.Graphics-UI.cpp index 4ec52e8..00f131e 100644 --- a/implementations/Crafter.Graphics-UI.cpp +++ b/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. - for (auto& h : heap_->resourceHeap) h.FlushDevice(); - for (auto& h : heap_->samplerHeap) h.FlushDevice(); + // No whole-heap flush here: WriteSwapchainDescriptors / WriteFontAtlasDescriptor + // / RegisterSampler each flush their own written byte range above, so the GPU + // 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}; } diff --git a/interfaces/Crafter.Graphics-Device.cppm b/interfaces/Crafter.Graphics-Device.cppm index 11853d2..be8b8e2 100644 --- a/interfaces/Crafter.Graphics-Device.cppm +++ b/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; + inline static VkPhysicalDeviceDescriptorHeapPropertiesEXT descriptorHeapProperties = { .sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_DESCRIPTOR_HEAP_PROPERTIES_EXT }; diff --git a/interfaces/Crafter.Graphics-VulkanBuffer.cppm b/interfaces/Crafter.Graphics-VulkanBuffer.cppm index ce0d862..ae66111 100644 --- a/interfaces/Crafter.Graphics-VulkanBuffer.cppm +++ b/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 — @@ -108,6 +133,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(&(VulkanBufferMappedConditional::value)))); } } @@ -184,6 +210,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 = { @@ -227,6 +274,7 @@ namespace Crafter { buffer = other.buffer; memory = other.memory; size = other.size; + mappedSize = other.mappedSize; memoryPropertyFlagsChosen = other.memoryPropertyFlagsChosen; other.buffer = VK_NULL_HANDLE; address = other.address; diff --git a/project.cpp b/project.cpp index 5cab540..e1e63f8 100644 --- a/project.cpp +++ b/project.cpp @@ -472,6 +472,33 @@ extern "C" Configuration CrafterBuildProject(std::span 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 rangedFlushImpls(impls.begin(), impls.end()); + rangedFlushImpls.emplace_back("tests/VulkanBufferRangedFlush/main"); + rfc.GetInterfacesAndImplementations(ifaces, rangedFlushImpls); + cfg.tests.push_back(std::move(rangedFlushTest)); + // Issue #89: Device::PreferDirectDeviceWrite chooses the upload strategy // for a CPU-written, GPU-read buffer — direct HOST_VISIBLE|DEVICE_LOCAL // map+write on ReBAR/UMA vs. staged-into-pure-DEVICE_LOCAL on a small diff --git a/tests/VulkanBufferRangedFlush/main.cpp b/tests/VulkanBufferRangedFlush/main.cpp new file mode 100644 index 0000000..2dd5ff8 --- /dev/null +++ b/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 +#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 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 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; +}