diff --git a/implementations/Crafter.Graphics-Device.cpp b/implementations/Crafter.Graphics-Device.cpp index bcd668c..cb09fe5 100644 --- a/implementations/Crafter.Graphics-Device.cpp +++ b/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) 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 a5b4a68..e7530b9 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; + // Core physical-device properties, captured once at Initialize from the // VkPhysicalDeviceProperties2 query. Kept because the pipeline-cache // persistence path needs vendorID / deviceID / pipelineCacheUUID to diff --git a/interfaces/Crafter.Graphics-VulkanBuffer.cppm b/interfaces/Crafter.Graphics-VulkanBuffer.cppm index 037d786..a130c1d 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 — @@ -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(&(VulkanBufferMappedConditional::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; diff --git a/project.cpp b/project.cpp index 59c6ca1..a08ce34 100644 --- a/project.cpp +++ b/project.cpp @@ -530,6 +530,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 #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; 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; +}