diff --git a/interfaces/Crafter.Graphics-VulkanBuffer.cppm b/interfaces/Crafter.Graphics-VulkanBuffer.cppm index c1d0c25..ce0d862 100644 --- a/interfaces/Crafter.Graphics-VulkanBuffer.cppm +++ b/interfaces/Crafter.Graphics-VulkanBuffer.cppm @@ -35,6 +35,11 @@ namespace Crafter { std::uint32_t size; VkBuffer buffer = VK_NULL_HANDLE; VkDeviceMemory memory; + // Property flags of the memory type actually chosen by GetMemoryType — + // not the requested flags. GetMemoryType may land a request without the + // COHERENT bit on a coherent type (and vice versa), so the flush/ + // invalidate paths gate on this recorded value, never on the request. + VkMemoryPropertyFlags memoryPropertyFlagsChosen = 0; }; export template @@ -79,10 +84,12 @@ namespace Crafter { VkMemoryRequirements memReqs; vkGetBufferMemoryRequirements(Device::device, buffer, &memReqs); + std::uint32_t memoryTypeIndex = Device::GetMemoryType(memReqs.memoryTypeBits, memoryPropertyFlags, preferredPropertyFlags); + memoryPropertyFlagsChosen = Device::memoryProperties.memoryTypes[memoryTypeIndex].propertyFlags; VkMemoryAllocateInfo memAlloc { .sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO, .allocationSize = memReqs.size, - .memoryTypeIndex = Device::GetMemoryType(memReqs.memoryTypeBits, memoryPropertyFlags, preferredPropertyFlags) + .memoryTypeIndex = memoryTypeIndex }; VkMemoryAllocateFlagsInfoKHR allocFlagsInfo { @@ -163,6 +170,11 @@ namespace Crafter { } void FlushDevice() requires(Mapped) { + // Coherent memory needs no explicit flush — host writes are + // automatically visible to the device. + if (memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT) { + return; + } VkMappedMemoryRange range = { .sType = VK_STRUCTURE_TYPE_MAPPED_MEMORY_RANGE, .memory = memory, @@ -197,6 +209,11 @@ namespace Crafter { } void FlushHost() requires(Mapped) { + // Coherent memory needs no explicit invalidate — device writes are + // automatically visible to the host. + if (memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT) { + return; + } VkMappedMemoryRange range = { .sType = VK_STRUCTURE_TYPE_MAPPED_MEMORY_RANGE, .memory = memory, @@ -210,6 +227,7 @@ namespace Crafter { buffer = other.buffer; memory = other.memory; size = other.size; + memoryPropertyFlagsChosen = other.memoryPropertyFlagsChosen; other.buffer = VK_NULL_HANDLE; address = other.address; if constexpr(Mapped) { diff --git a/project.cpp b/project.cpp index 14769fc..bda77a1 100644 --- a/project.cpp +++ b/project.cpp @@ -422,10 +422,39 @@ extern "C" Configuration CrafterBuildProject(std::span a // slip through (the C++ static_assert only guards the C++ side). No GPU // device at runtime, but glslang + spirv-val are required tools. Test fusedTest; - Configuration& fc = fusedTest.config; + Configuration& ftc = fusedTest.config; + ftc.path = cfg.path; + ftc.name = "UIFusedShader"; + ftc.outputName = "UIFusedShader"; + ftc.type = ConfigurationType::Executable; + ftc.target = cfg.target; + ftc.march = cfg.march; + ftc.mtune = cfg.mtune; + ftc.debug = cfg.debug; + ftc.sysroot = cfg.sysroot; + ftc.dependencies = cfg.dependencies; + ftc.externalDependencies = cfg.externalDependencies; + ftc.compileFlags = cfg.compileFlags; + ftc.linkFlags = cfg.linkFlags; + ftc.defines = cfg.defines; + ftc.cFiles = cfg.cFiles; + std::vector fusedImpls(impls.begin(), impls.end()); + fusedImpls.emplace_back("tests/UIFusedShader/main"); + ftc.GetInterfacesAndImplementations(ifaces, fusedImpls); + fusedTest.requires_ = { "tool:glslang", "tool:spirv-val" }; + cfg.tests.push_back(std::move(fusedTest)); + + // Issue #60: VulkanBuffer::FlushDevice / FlushHost now record the chosen + // memory type's propertyFlags at Create time and skip the + // flush/invalidate when the memory is HOST_COHERENT. The gate is pure + // logic over the recorded flags, so this test stamps them directly and + // verifies the coherent path issues no Vulkan call — no GPU device + // needed at runtime. + Test flushTest; + Configuration& fc = flushTest.config; fc.path = cfg.path; - fc.name = "UIFusedShader"; - fc.outputName = "UIFusedShader"; + fc.name = "VulkanBufferFlushGate"; + fc.outputName = "VulkanBufferFlushGate"; fc.type = ConfigurationType::Executable; fc.target = cfg.target; fc.march = cfg.march; @@ -438,11 +467,10 @@ extern "C" Configuration CrafterBuildProject(std::span a fc.linkFlags = cfg.linkFlags; fc.defines = cfg.defines; fc.cFiles = cfg.cFiles; - std::vector fusedImpls(impls.begin(), impls.end()); - fusedImpls.emplace_back("tests/UIFusedShader/main"); - fc.GetInterfacesAndImplementations(ifaces, fusedImpls); - fusedTest.requires_ = { "tool:glslang", "tool:spirv-val" }; - cfg.tests.push_back(std::move(fusedTest)); + std::vector flushImpls(impls.begin(), impls.end()); + flushImpls.emplace_back("tests/VulkanBufferFlushGate/main"); + fc.GetInterfacesAndImplementations(ifaces, flushImpls); + cfg.tests.push_back(std::move(flushTest)); // Issue #57: Font::GetLineWidth memoises per-codepoint advances in // font units (Font::AdvanceUnits) and rescales per call, instead of diff --git a/tests/VulkanBufferFlushGate/main.cpp b/tests/VulkanBufferFlushGate/main.cpp new file mode 100644 index 0000000..eba680c --- /dev/null +++ b/tests/VulkanBufferFlushGate/main.cpp @@ -0,0 +1,89 @@ +/* +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 issue #60: VulkanBuffer::FlushDevice / FlushHost used to +// call vkFlushMappedMemoryRanges / vkInvalidateMappedMemoryRanges +// unconditionally. They now record the *chosen* memory type's propertyFlags at +// Create time and skip the flush/invalidate when the memory is HOST_COHERENT +// (coherent memory makes host/device writes mutually visible with no explicit +// flush). +// +// The gate is pure logic over the recorded flags, so this test drives it +// directly with no GPU device: it stamps memoryPropertyFlagsChosen and verifies +// the coherent path returns without issuing any Vulkan call. With no device +// created, Device::device is VK_NULL_HANDLE — so if the gate were removed, the +// flush call would dereference a null dispatch handle and crash. Reaching the +// line after the calls is the assertion that the gate held. + +#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 DEVICE_LOCAL = VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT; +constexpr auto HOST_VISIBLE = VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT; +constexpr auto HOST_COHERENT = VK_MEMORY_PROPERTY_HOST_COHERENT_BIT; + +} // namespace + +int main() { + // No device is created, so Device::device stays VK_NULL_HANDLE; any real + // flush/invalidate call below would fault. The coherent gate must prevent + // it. + Check(Device::device == VK_NULL_HANDLE, + "no Vulkan device created — a real flush would fault"); + + { + VulkanBuffer buf; + // Coherent type chosen (even though no COHERENT bit was requested): + // both flush paths must early-return without touching Vulkan. + buf.memoryPropertyFlagsChosen = HOST_VISIBLE | HOST_COHERENT; + buf.FlushDevice(); + buf.FlushHost(); + Check(true, "FlushDevice/FlushHost skip the Vulkan call on coherent memory"); + } + + { + // A non-coherent type would require the flush — verify the recorded + // flags reflect the gate's decision input rather than the request. The + // bit test is the exact condition FlushDevice/FlushHost branch on. + VulkanBuffer buf; + buf.memoryPropertyFlagsChosen = HOST_VISIBLE | DEVICE_LOCAL; + Check(!(buf.memoryPropertyFlagsChosen & HOST_COHERENT), + "non-coherent chosen type is not gated as coherent (flush still required)"); + } + + if (failures != 0) { + std::println("{} check(s) failed", failures); + return EXIT_FAILURE; + } + std::println("all checks passed"); + return EXIT_SUCCESS; +}