diff --git a/interfaces/Crafter.Graphics-VulkanBuffer.cppm b/interfaces/Crafter.Graphics-VulkanBuffer.cppm index ce0d862..037d786 100644 --- a/interfaces/Crafter.Graphics-VulkanBuffer.cppm +++ b/interfaces/Crafter.Graphics-VulkanBuffer.cppm @@ -40,6 +40,12 @@ namespace Crafter { // 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; + // Byte capacity the buffer was created with, and the usage flags it was + // created with. Resize reuses the allocation in place when a new + // request still fits within `capacity` and these immutable-at-create + // properties match, avoiding a destroy+reallocate. + std::uint32_t capacity = 0; + VkBufferUsageFlags2 usageFlagsCreated = 0; }; export template @@ -67,6 +73,8 @@ namespace Crafter { // available on every device — see Device::GetMemoryType. void Create(VkBufferUsageFlags2 usageFlags, VkMemoryPropertyFlags memoryPropertyFlags, std::uint32_t count, VkMemoryPropertyFlags preferredPropertyFlags = 0) { size = count * sizeof(T); + capacity = size; + usageFlagsCreated = usageFlags; // Carry usage in the maintenance5 flags2 chain so 64-bit bits // (e.g. VK_BUFFER_USAGE_2_MEMORY_DECOMPRESSION_BIT_EXT, bit 35) @@ -122,6 +130,22 @@ namespace Crafter { } void Resize(VkBufferUsageFlags2 usageFlags, VkMemoryPropertyFlags memoryPropertyFlags, std::uint32_t count, VkMemoryPropertyFlags preferredPropertyFlags = 0) { + // Reuse the existing allocation in place when the request still fits + // and the fixed-at-create properties match: usage flags are + // immutable after creation, and the memory type already chosen must + // still satisfy the required property flags. preferredPropertyFlags + // is a best-effort perf hint and does not affect correctness, so it + // is intentionally not part of the guard. The buffer handle (and its + // device address / mapped pointer) is preserved, only `size` shrinks + // to the new logical extent. + std::uint32_t requestedSize = count * sizeof(T); + if(buffer != VK_NULL_HANDLE + && requestedSize <= capacity + && usageFlags == usageFlagsCreated + && (memoryPropertyFlagsChosen & memoryPropertyFlags) == memoryPropertyFlags) { + size = requestedSize; + return; + } if(buffer != VK_NULL_HANDLE) { Clear(); } @@ -227,6 +251,8 @@ namespace Crafter { buffer = other.buffer; memory = other.memory; size = other.size; + capacity = other.capacity; + usageFlagsCreated = other.usageFlagsCreated; memoryPropertyFlagsChosen = other.memoryPropertyFlagsChosen; other.buffer = VK_NULL_HANDLE; address = other.address; diff --git a/project.cpp b/project.cpp index 5cab540..d928210 100644 --- a/project.cpp +++ b/project.cpp @@ -472,6 +472,35 @@ extern "C" Configuration CrafterBuildProject(std::span a fc.GetInterfacesAndImplementations(ifaces, flushImpls); cfg.tests.push_back(std::move(flushTest)); + // 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; + // chosen memory type still satisfies the required flags), instead of + // always destroying + reallocating. The reuse guard is pure logic over + // recorded fields, so this test stamps a fake handle + capacity/flags and + // verifies the in-place path issues no Vulkan call — no GPU device needed. + Test resizeTest; + Configuration& rc = resizeTest.config; + rc.path = cfg.path; + rc.name = "VulkanBufferResizeReuse"; + rc.outputName = "VulkanBufferResizeReuse"; + rc.type = ConfigurationType::Executable; + rc.target = cfg.target; + rc.march = cfg.march; + rc.mtune = cfg.mtune; + rc.debug = cfg.debug; + rc.sysroot = cfg.sysroot; + rc.dependencies = cfg.dependencies; + rc.externalDependencies = cfg.externalDependencies; + rc.compileFlags = cfg.compileFlags; + rc.linkFlags = cfg.linkFlags; + rc.defines = cfg.defines; + rc.cFiles = cfg.cFiles; + std::vector resizeImpls(impls.begin(), impls.end()); + resizeImpls.emplace_back("tests/VulkanBufferResizeReuse/main"); + rc.GetInterfacesAndImplementations(ifaces, resizeImpls); + cfg.tests.push_back(std::move(resizeTest)); + // 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/VulkanBufferResizeReuse/main.cpp b/tests/VulkanBufferResizeReuse/main.cpp new file mode 100644 index 0000000..9348180 --- /dev/null +++ b/tests/VulkanBufferResizeReuse/main.cpp @@ -0,0 +1,134 @@ +/* +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 #63: VulkanBuffer::Resize used to unconditionally +// destroy + reallocate. It now reuses the existing allocation in place when the +// new request still fits within the created capacity AND the immutable-at-create +// properties match (usage flags are fixed at create; the chosen memory type must +// still satisfy the required property flags). The reuse path only shrinks `size` +// and issues no Vulkan call. +// +// The guard is pure logic over recorded fields, so this test drives it directly +// with no GPU device: it stamps a fake non-null buffer handle plus capacity / +// usage / chosen-flags, then calls Resize. With Device::device == VK_NULL_HANDLE +// any real destroy/reallocate would dereference a null dispatch handle and +// crash — so reaching the line after a reuse Resize is the assertion that the +// in-place path was taken. The realloc path (flag/size mismatch) is intentionally +// not exercised here because it would issue real Vulkan calls. + +#include +#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; + +constexpr VkBufferUsageFlags2 USAGE = + VK_BUFFER_USAGE_2_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_2_SHADER_DEVICE_ADDRESS_BIT; + +// A fake non-null, non-dereferenced handle: Resize's reuse path never touches it. +VkBuffer FakeHandle() { + return reinterpret_cast(static_cast(0x1)); +} + +// Stamp a buffer into the "already created with capacity C" state without +// touching the GPU, then neutralise it so the destructor's Clear() is skipped. +template +void NeutraliseHandle(Buf& buf) { + buf.buffer = VK_NULL_HANDLE; +} + +} // namespace + +int main() { + Check(Device::device == VK_NULL_HANDLE, + "no Vulkan device created — a real reallocate would fault"); + + { + // Shrink within capacity, flags match → reuse in place, only size shrinks. + VulkanBuffer buf; + buf.buffer = FakeHandle(); + buf.capacity = 16 * sizeof(float); + buf.size = 16 * sizeof(float); + buf.usageFlagsCreated = USAGE; + buf.memoryPropertyFlagsChosen = HOST_VISIBLE | HOST_COHERENT; + + buf.Resize(USAGE, HOST_VISIBLE, 4); // 4 floats <= 16-float capacity + Check(buf.buffer == FakeHandle(), + "fitting Resize reuses the existing handle (no realloc)"); + Check(buf.size == 4 * sizeof(float), + "reuse updates size to the new logical extent"); + Check(buf.capacity == 16 * sizeof(float), + "reuse leaves capacity at the original allocation size"); + NeutraliseHandle(buf); + } + + { + // Exact-fit request (size == capacity) is still a reuse. + VulkanBuffer buf; + buf.buffer = FakeHandle(); + buf.capacity = 8 * sizeof(float); + buf.size = 2 * sizeof(float); + buf.usageFlagsCreated = USAGE; + buf.memoryPropertyFlagsChosen = HOST_VISIBLE; + + buf.Resize(USAGE, HOST_VISIBLE, 8); + Check(buf.buffer == FakeHandle() && buf.size == 8 * sizeof(float), + "exact-capacity Resize reuses the allocation"); + NeutraliseHandle(buf); + } + + { + // Reuse requires the chosen memory type to still satisfy the required + // flags. Chosen type carries DEVICE_LOCAL, so a request for it is + // satisfied and reuse is allowed. + VulkanBuffer buf; + buf.buffer = FakeHandle(); + buf.capacity = 8 * sizeof(float); + buf.size = 8 * sizeof(float); + buf.usageFlagsCreated = USAGE; + buf.memoryPropertyFlagsChosen = HOST_VISIBLE | DEVICE_LOCAL; + + buf.Resize(USAGE, DEVICE_LOCAL, 4); + Check(buf.buffer == FakeHandle() && buf.size == 4 * sizeof(float), + "reuse allowed when chosen memory type satisfies required flags"); + NeutraliseHandle(buf); + } + + if (failures != 0) { + std::println("{} check(s) failed", failures); + return EXIT_FAILURE; + } + std::println("all checks passed"); + return EXIT_SUCCESS; +}