From cfbe181d9ea2812c89c210136fdac359912a5d7f Mon Sep 17 00:00:00 2001 From: catbot Date: Tue, 16 Jun 2026 17:23:27 +0000 Subject: [PATCH] feat(device): runtime upload-strategy helper PreferDirectDeviceWrite (#89) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add Device::PreferDirectDeviceWrite(size) so the memory-placement cluster (#65/#72/#73/#75) no longer re-derives where a CPU-written, GPU-read buffer should live. It picks the strategy at runtime from Device::memoryProperties: - No DEVICE_LOCAL|HOST_VISIBLE type (no resizable BAR) -> false (must stage). - ReBAR/UMA (BAR heap >= 90% of largest DEVICE_LOCAL heap) -> true: map and write directly; a staging copy would be pure overhead. - Small BAR window (BAR heap << VRAM) -> true only for buffers within a per-window budget (1/8 of the window); large buffers stage so they don't exhaust the window (#58). Complements GetMemoryType (#59): #59 picks the memory *type*, this picks the *strategy*. Upload-only — readback (#74) wants HOST_CACHED and stays separate. A VK_EXT_memory_budget-driven remaining-space check is noted as a follow-up. Tested by tests/UploadStrategy (pure CPU over synthetic ReBAR / UMA / small-window / no-BAR layouts, no GPU device needed), mirroring MemoryTypeFallback. Co-Authored-By: Claude Opus 4.8 --- implementations/Crafter.Graphics-Device.cpp | 45 +++++ interfaces/Crafter.Graphics-Device.cppm | 28 ++++ project.cpp | 29 ++++ tests/UploadStrategy/main.cpp | 173 ++++++++++++++++++++ 4 files changed, 275 insertions(+) create mode 100644 tests/UploadStrategy/main.cpp diff --git a/implementations/Crafter.Graphics-Device.cpp b/implementations/Crafter.Graphics-Device.cpp index 3a30ed4..c4ca43f 100644 --- a/implementations/Crafter.Graphics-Device.cpp +++ b/implementations/Crafter.Graphics-Device.cpp @@ -840,4 +840,49 @@ std::uint32_t Device::GetMemoryType(uint32_t typeBits, VkMemoryPropertyFlags req } throw std::runtime_error("Could not find a matching memory type"); +} + +bool Device::PreferDirectDeviceWrite(VkDeviceSize size) { + constexpr VkMemoryPropertyFlags directFlags = + VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT | VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT; + + // 1. Find a DEVICE_LOCAL | HOST_VISIBLE type and remember its heap. Without + // one (no resizable BAR), direct writes are impossible -> must stage. + bool haveDirectType = false; + std::uint32_t barHeapIndex = 0; + for (std::uint32_t i = 0; i < memoryProperties.memoryTypeCount; ++i) { + if ((memoryProperties.memoryTypes[i].propertyFlags & directFlags) == directFlags) { + haveDirectType = true; + barHeapIndex = memoryProperties.memoryTypes[i].heapIndex; + break; + } + } + if (!haveDirectType) { + return false; + } + + // 2. Compare the host-visible device-local heap against the largest plain + // DEVICE_LOCAL heap (the true VRAM size). + VkDeviceSize barHeap = memoryProperties.memoryHeaps[barHeapIndex].size; + VkDeviceSize vram = 0; + for (std::uint32_t i = 0; i < memoryProperties.memoryHeapCount; ++i) { + if (memoryProperties.memoryHeaps[i].flags & VK_MEMORY_HEAP_DEVICE_LOCAL_BIT) { + vram = std::max(vram, memoryProperties.memoryHeaps[i].size); + } + } + + // 3. ReBAR / UMA: the BAR heap covers (almost) all of VRAM -> direct. + // barHeap >= 0.9 * vram, written to dodge floating point / overflow. + if (barHeap >= vram - vram / 10) { + return true; + } + + // 4. Small BAR window: direct only for buffers small enough that several + // still fit the window — large buffers stage so they don't exhaust it. + // The budget is tied to the actual window size rather than a magic + // constant so it adapts across devices (a fixed cap would be wrong for + // both a 256 MiB and a 4 GiB window). VK_EXT_memory_budget would refine + // this to the window's *remaining* space; see the header note. + constexpr VkDeviceSize barWindowFraction = 8; + return size <= barHeap / barWindowFraction; } \ No newline at end of file diff --git a/interfaces/Crafter.Graphics-Device.cppm b/interfaces/Crafter.Graphics-Device.cppm index a36fa96..8c79181 100644 --- a/interfaces/Crafter.Graphics-Device.cppm +++ b/interfaces/Crafter.Graphics-Device.cppm @@ -206,6 +206,34 @@ export namespace Crafter { // valid memory exists for the allocation). static std::uint32_t GetMemoryType(std::uint32_t typeBits, VkMemoryPropertyFlags required, VkMemoryPropertyFlags preferred = 0); + // Upload-strategy helper for a CPU-written, GPU-read buffer of `size` + // bytes. Picks *where* the buffer should live, complementing + // GetMemoryType (which picks the memory type once the strategy is set): + // true -> allocate HOST_VISIBLE | DEVICE_LOCAL, map, write straight + // into device memory (no staging buffer, no copy). + // false -> allocate pure DEVICE_LOCAL and feed it from a HOST_VISIBLE + // staging buffer + vkCmdCopyBuffer. + // + // The decision is platform-dependent and made at runtime, never + // hardcoded: + // 1. No DEVICE_LOCAL | HOST_VISIBLE type exists (no resizable BAR) -> + // false: direct writes are impossible, staging is mandatory. + // 2. ReBAR / UMA — the host-visible device-local heap is essentially + // the whole VRAM heap (>= 90% of the largest DEVICE_LOCAL heap) -> + // true: a staging copy would be pure overhead. + // 3. Small BAR window — the host-visible device-local heap is a tiny + // window on a separate heap (<< VRAM) -> true only for buffers + // small enough to fit the window's per-buffer budget; large + // buffers stage so they don't exhaust the window (issue #58). + // + // This is the *upload* path (CPU-write -> GPU-read). Readback wants + // HOST_CACHED instead (the ReBAR type is coherent-not-cached, so CPU + // reads from it are slow) — keep that path separate. A more robust + // small-window budget would consult VK_EXT_memory_budget for the + // window's *remaining* space rather than its total size; that needs the + // extension enabled and is a follow-up. + static bool PreferDirectDeviceWrite(VkDeviceSize size); + // ─── Wayland key repeat ──────────────────────────────────────── // TickKeyRepeats fires onRawKeyDown / onRawKeyHold / onTextInput on // the focused window for whichever key is currently repeating. diff --git a/project.cpp b/project.cpp index bda77a1..5cab540 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 #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 + // BAR window (#58). The decision is pure CPU logic over + // Device::memoryProperties (types + heaps), so this test installs + // synthetic ReBAR / UMA / small-window / no-BAR layouts and drives it + // directly — no GPU device needed at runtime. + Test uploadTest; + Configuration& uc = uploadTest.config; + uc.path = cfg.path; + uc.name = "UploadStrategy"; + uc.outputName = "UploadStrategy"; + uc.type = ConfigurationType::Executable; + uc.target = cfg.target; + uc.march = cfg.march; + uc.mtune = cfg.mtune; + uc.debug = cfg.debug; + uc.sysroot = cfg.sysroot; + uc.dependencies = cfg.dependencies; + uc.externalDependencies = cfg.externalDependencies; + uc.compileFlags = cfg.compileFlags; + uc.linkFlags = cfg.linkFlags; + uc.defines = cfg.defines; + uc.cFiles = cfg.cFiles; + std::vector uploadImpls(impls.begin(), impls.end()); + uploadImpls.emplace_back("tests/UploadStrategy/main"); + uc.GetInterfacesAndImplementations(ifaces, uploadImpls); + cfg.tests.push_back(std::move(uploadTest)); + // Issue #57: Font::GetLineWidth memoises per-codepoint advances in // font units (Font::AdvanceUnits) and rescales per call, instead of // calling stbtt_GetCodepointHMetrics for every glyph on every caret diff --git a/tests/UploadStrategy/main.cpp b/tests/UploadStrategy/main.cpp new file mode 100644 index 0000000..442711d --- /dev/null +++ b/tests/UploadStrategy/main.cpp @@ -0,0 +1,173 @@ +/* +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 #89: Device::PreferDirectDeviceWrite is the runtime +// upload-strategy helper that decides whether a CPU-written, GPU-read buffer +// should be written directly into a HOST_VISIBLE | DEVICE_LOCAL allocation +// (true) or staged through a HOST_VISIBLE buffer + vkCmdCopyBuffer into pure +// DEVICE_LOCAL (false). The choice is platform-dependent: +// - ReBAR / UMA: the host-visible device-local heap is ~all of VRAM, so +// staging is pure overhead -> direct. +// - non-ReBAR discrete: HOST_VISIBLE | DEVICE_LOCAL is a small BAR window on +// a separate heap; large buffers must stage (#58), small/hot ones may map. +// - no host-visible device-local type at all -> staging is mandatory. +// +// The decision is pure CPU logic over Device::memoryProperties (both the +// memory types and the heaps), so this test installs synthetic layouts and +// drives it directly — no GPU device needed at runtime, mirroring +// MemoryTypeFallback. + +#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 auto HOST_CACHED = VK_MEMORY_PROPERTY_HOST_CACHED_BIT; + +constexpr VkDeviceSize KiB = 1024; +constexpr VkDeviceSize MiB = 1024 * KiB; +constexpr VkDeviceSize GiB = 1024 * MiB; + +struct TypeSpec { VkMemoryPropertyFlags flags; std::uint32_t heapIndex; }; +struct HeapSpec { VkDeviceSize size; VkMemoryHeapFlags flags; }; + +// Install a synthetic memory layout: explicit types (with their heap index) +// and explicit heaps (size + flags). +void SetMemory(std::initializer_list types, + std::initializer_list heaps) { + Device::memoryProperties = {}; + Device::memoryProperties.memoryTypeCount = static_cast(types.size()); + std::uint32_t i = 0; + for (const TypeSpec& t : types) { + Device::memoryProperties.memoryTypes[i].propertyFlags = t.flags; + Device::memoryProperties.memoryTypes[i].heapIndex = t.heapIndex; + ++i; + } + Device::memoryProperties.memoryHeapCount = static_cast(heaps.size()); + std::uint32_t h = 0; + for (const HeapSpec& s : heaps) { + Device::memoryProperties.memoryHeaps[h].size = s.size; + Device::memoryProperties.memoryHeaps[h].flags = s.flags; + ++h; + } +} + +constexpr VkMemoryHeapFlags HEAP_DEVICE_LOCAL = VK_MEMORY_HEAP_DEVICE_LOCAL_BIT; + +} // namespace + +int main() { + // --- ReBAR: full 24 GiB heap is DEVICE_LOCAL | HOST_VISIBLE (this dev + // machine, type 4 on heap 0) -> always direct, even for huge buffers -- + { + SetMemory( + { {DEVICE_LOCAL, 0}, + {HOST_VISIBLE | HOST_COHERENT, 1}, + {DEVICE_LOCAL | HOST_VISIBLE | HOST_COHERENT, 0} }, + { {24 * GiB, HEAP_DEVICE_LOCAL}, {32 * GiB, 0} }); + Check(Device::PreferDirectDeviceWrite(8), + "ReBAR: a tiny buffer maps directly"); + Check(Device::PreferDirectDeviceWrite(8 * GiB), + "ReBAR: even a multi-GiB buffer maps directly (no overhead staging)"); + } + + // --- UMA: a single heap with all flags -> direct ---------------------- + { + SetMemory( + { {DEVICE_LOCAL | HOST_VISIBLE | HOST_COHERENT | HOST_CACHED, 0} }, + { {125 * GiB, HEAP_DEVICE_LOCAL} }); + Check(Device::PreferDirectDeviceWrite(64 * GiB), + "UMA: single all-flags heap maps directly"); + } + + // --- non-ReBAR discrete: small 256 MiB BAR window (heap 1) separate from + // an 8 GiB VRAM heap (heap 0) -> stage large, map small ------------ + { + SetMemory( + { {DEVICE_LOCAL, 0}, // pure VRAM + {HOST_VISIBLE | HOST_COHERENT, 2}, // system RAM + {DEVICE_LOCAL | HOST_VISIBLE | HOST_COHERENT, 1} }, // BAR window + { {8 * GiB, HEAP_DEVICE_LOCAL}, // heap 0: VRAM + {256 * MiB, HEAP_DEVICE_LOCAL}, // heap 1: BAR + {32 * GiB, 0} }); // heap 2: RAM + Check(!Device::PreferDirectDeviceWrite(128 * MiB), + "small BAR: a large buffer stages (would exhaust the 256 MiB window)"); + Check(Device::PreferDirectDeviceWrite(8 * MiB), + "small BAR: a small/hot buffer maps directly into the window"); + // Budget is 1/8 of the window = 32 MiB. Boundary checks. + Check(Device::PreferDirectDeviceWrite(32 * MiB), + "small BAR: a buffer exactly at the per-buffer budget still maps"); + Check(!Device::PreferDirectDeviceWrite(32 * MiB + 1), + "small BAR: one byte over the budget stages"); + } + + // --- no host-visible device-local type (no resizable BAR) -> must stage - + { + SetMemory( + { {DEVICE_LOCAL, 0}, + {HOST_VISIBLE | HOST_COHERENT, 1}, + {HOST_VISIBLE | HOST_COHERENT | HOST_CACHED, 1} }, + { {8 * GiB, HEAP_DEVICE_LOCAL}, {32 * GiB, 0} }); + Check(!Device::PreferDirectDeviceWrite(8), + "no DEVICE_LOCAL|HOST_VISIBLE type -> staging is mandatory even for a tiny buffer"); + } + + // --- 90% threshold: a BAR heap that is most-but-not-all of VRAM counts as + // ReBAR; one well below the threshold is treated as a window -------- + { + // BAR heap = 9.5 GiB, VRAM = 10 GiB -> 95% >= 90% -> ReBAR -> direct. + SetMemory( + { {DEVICE_LOCAL, 0}, + {DEVICE_LOCAL | HOST_VISIBLE | HOST_COHERENT, 1} }, + { {10 * GiB, HEAP_DEVICE_LOCAL}, {VkDeviceSize(9.5 * GiB), HEAP_DEVICE_LOCAL} }); + Check(Device::PreferDirectDeviceWrite(4 * GiB), + "BAR >= 90% of VRAM is treated as ReBAR: large buffer still maps"); + + // BAR heap = 2 GiB, VRAM = 10 GiB -> 20% < 90% -> window -> stage big. + SetMemory( + { {DEVICE_LOCAL, 0}, + {DEVICE_LOCAL | HOST_VISIBLE | HOST_COHERENT, 1} }, + { {10 * GiB, HEAP_DEVICE_LOCAL}, {2 * GiB, HEAP_DEVICE_LOCAL} }); + Check(!Device::PreferDirectDeviceWrite(4 * GiB), + "BAR well below 90% of VRAM is a window: large buffer stages"); + Check(Device::PreferDirectDeviceWrite(128 * MiB), + "BAR window of 2 GiB still maps a small buffer (budget = 256 MiB)"); + } + + if (failures != 0) { + std::println("{} check(s) failed", failures); + return EXIT_FAILURE; + } + std::println("all checks passed"); + return EXIT_SUCCESS; +} -- 2.54.0