fix(device): preferred mask + required-only fallback for GetMemoryType (#59) #87
6 changed files with 218 additions and 13 deletions
fix(device): add preferred mask + required-only fallback to GetMemoryType (#59)
GetMemoryType was a bare first-superset match that threw whenever no memory type satisfied the requested property flags. Callers combining a mandatory flag with a perf-only one (HOST_VISIBLE | DEVICE_LOCAL for the descriptor heaps) would then fail allocation on any device without a host-visible device-local heap (no resizable BAR). It now takes a `preferred` mask distinct from `required`: a type satisfying both is chosen first, falling back to a required-only match when the preference is unavailable, and throwing only when even `required` cannot be met. VulkanBuffer::Create/Resize gain an optional trailing `preferredPropertyFlags`, and the descriptor heaps now treat DEVICE_LOCAL as a preference on top of mandatory HOST_VISIBLE. This does not touch any flush/barrier behaviour — coherency is not assumed anywhere here. Adds tests/MemoryTypeFallback, a pure-CPU test that installs synthetic memory layouts and exercises selection, preference, fallback, and the unsatisfiable-required throw. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
commit
294c1378b5
|
|
@ -811,17 +811,32 @@ void Device::Initialize() {
|
|||
}
|
||||
}
|
||||
|
||||
std::uint32_t Device::GetMemoryType(uint32_t typeBits, VkMemoryPropertyFlags properties) {
|
||||
for (uint32_t i = 0; i < memoryProperties.memoryTypeCount; i++)
|
||||
std::uint32_t Device::GetMemoryType(uint32_t typeBits, VkMemoryPropertyFlags required, VkMemoryPropertyFlags preferred) {
|
||||
// First pass: honour the preferred bits on top of the required ones.
|
||||
// Second pass (when nothing matched, or no preferences were given): drop
|
||||
// the preferences and match the required bits alone. Vulkan orders the
|
||||
// memory types so first-superset-match is generally optimal, so a plain
|
||||
// linear scan suffices for each pass. We only throw when even `required`
|
||||
// cannot be satisfied — i.e. no valid memory exists for the allocation.
|
||||
for (VkMemoryPropertyFlags wanted : {required | preferred, required})
|
||||
{
|
||||
if ((typeBits & 1) == 1)
|
||||
uint32_t bits = typeBits;
|
||||
for (uint32_t i = 0; i < memoryProperties.memoryTypeCount; i++)
|
||||
{
|
||||
if ((memoryProperties.memoryTypes[i].propertyFlags & properties) == properties)
|
||||
if ((bits & 1) == 1)
|
||||
{
|
||||
return i;
|
||||
if ((memoryProperties.memoryTypes[i].propertyFlags & wanted) == wanted)
|
||||
{
|
||||
return i;
|
||||
}
|
||||
}
|
||||
bits >>= 1;
|
||||
}
|
||||
|
||||
// No preferences to fall back from — the second pass would be identical.
|
||||
if (preferred == 0) {
|
||||
break;
|
||||
}
|
||||
typeBits >>= 1;
|
||||
}
|
||||
|
||||
throw std::runtime_error("Could not find a matching memory type");
|
||||
|
|
|
|||
|
|
@ -83,9 +83,14 @@ export namespace Crafter {
|
|||
samplerFreelist.clear();
|
||||
samplerFreelist.reserve(samplers);
|
||||
|
||||
// The heaps are written by the CPU every frame (HOST_VISIBLE is
|
||||
// mandatory) and read by the GPU, so DEVICE_LOCAL is a perf
|
||||
// preference, not a requirement: devices without a host-visible
|
||||
// device-local heap (no resizable BAR) get a host-visible-only
|
||||
// allocation instead of a hard allocation failure.
|
||||
for(std::uint8_t i = 0; i < Window::numFrames; i++) {
|
||||
resourceHeap[i].Resize(VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT | VK_BUFFER_USAGE_DESCRIPTOR_HEAP_BIT_EXT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, resourceSize);
|
||||
samplerHeap[i].Resize(VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT | VK_BUFFER_USAGE_DESCRIPTOR_HEAP_BIT_EXT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, samplerSize);
|
||||
resourceHeap[i].Resize(VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT | VK_BUFFER_USAGE_DESCRIPTOR_HEAP_BIT_EXT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, resourceSize, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT);
|
||||
samplerHeap[i].Resize(VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT | VK_BUFFER_USAGE_DESCRIPTOR_HEAP_BIT_EXT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, samplerSize, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -199,7 +199,12 @@ export namespace Crafter {
|
|||
// ComputeShader read the offset off the pipeline they record.
|
||||
|
||||
static void CheckVkResult(VkResult result);
|
||||
static std::uint32_t GetMemoryType(std::uint32_t typeBits, VkMemoryPropertyFlags properties);
|
||||
// Selects a memory type index from typeBits that satisfies `required`.
|
||||
// When `preferred` bits are also given, a type satisfying both is
|
||||
// chosen first; if none exists we fall back to required-only rather
|
||||
// than throwing. Throws only when even `required` cannot be met (no
|
||||
// valid memory exists for the allocation).
|
||||
static std::uint32_t GetMemoryType(std::uint32_t typeBits, VkMemoryPropertyFlags required, VkMemoryPropertyFlags preferred = 0);
|
||||
|
||||
// ─── Wayland key repeat ────────────────────────────────────────
|
||||
// TickKeyRepeats fires onRawKeyDown / onRawKeyHold / onTextInput on
|
||||
|
|
|
|||
|
|
@ -55,7 +55,12 @@ namespace Crafter {
|
|||
class VulkanBuffer : public VulkanBufferBase, public VulkanBufferMappedConditional<T, Mapped> {
|
||||
public:
|
||||
VulkanBuffer() = default;
|
||||
void Create(VkBufferUsageFlags2 usageFlags, VkMemoryPropertyFlags memoryPropertyFlags, std::uint32_t count) {
|
||||
// `preferredPropertyFlags` are best-effort: the allocation prefers a
|
||||
// memory type satisfying them on top of `memoryPropertyFlags`, but
|
||||
// falls back to the required flags alone rather than failing. Use it
|
||||
// for perf hints (e.g. DEVICE_LOCAL on a mapped buffer) that aren't
|
||||
// available on every device — see Device::GetMemoryType.
|
||||
void Create(VkBufferUsageFlags2 usageFlags, VkMemoryPropertyFlags memoryPropertyFlags, std::uint32_t count, VkMemoryPropertyFlags preferredPropertyFlags = 0) {
|
||||
size = count * sizeof(T);
|
||||
|
||||
// Carry usage in the maintenance5 flags2 chain so 64-bit bits
|
||||
|
|
@ -77,7 +82,7 @@ namespace Crafter {
|
|||
VkMemoryAllocateInfo memAlloc {
|
||||
.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO,
|
||||
.allocationSize = memReqs.size,
|
||||
.memoryTypeIndex = Device::GetMemoryType(memReqs.memoryTypeBits, memoryPropertyFlags)
|
||||
.memoryTypeIndex = Device::GetMemoryType(memReqs.memoryTypeBits, memoryPropertyFlags, preferredPropertyFlags)
|
||||
};
|
||||
|
||||
VkMemoryAllocateFlagsInfoKHR allocFlagsInfo {
|
||||
|
|
@ -109,11 +114,11 @@ namespace Crafter {
|
|||
buffer = VK_NULL_HANDLE;
|
||||
}
|
||||
|
||||
void Resize(VkBufferUsageFlags2 usageFlags, VkMemoryPropertyFlags memoryPropertyFlags, std::uint32_t count) {
|
||||
void Resize(VkBufferUsageFlags2 usageFlags, VkMemoryPropertyFlags memoryPropertyFlags, std::uint32_t count, VkMemoryPropertyFlags preferredPropertyFlags = 0) {
|
||||
if(buffer != VK_NULL_HANDLE) {
|
||||
Clear();
|
||||
}
|
||||
Create(usageFlags, memoryPropertyFlags, count);
|
||||
Create(usageFlags, memoryPropertyFlags, count, preferredPropertyFlags);
|
||||
}
|
||||
|
||||
void Copy(VkCommandBuffer cmd, VulkanBuffer& dst) {
|
||||
|
|
|
|||
29
project.cpp
29
project.cpp
|
|
@ -384,6 +384,35 @@ extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> a
|
|||
textImpls.emplace_back("tests/ShapeTextCache/main");
|
||||
xc.GetInterfacesAndImplementations(ifaces, textImpls);
|
||||
cfg.tests.push_back(std::move(textTest));
|
||||
|
||||
// Issue #59: Device::GetMemoryType gained a `preferred` mask and a
|
||||
// required-only fallback so callers combining a mandatory flag with a
|
||||
// perf-only one (HOST_VISIBLE | DEVICE_LOCAL descriptor heaps) no
|
||||
// longer throw on devices without a host-visible device-local heap.
|
||||
// The selection is pure CPU logic over Device::memoryProperties, so
|
||||
// this test installs synthetic memory layouts and drives it directly —
|
||||
// no GPU device needed at runtime.
|
||||
Test memTest;
|
||||
Configuration& mc = memTest.config;
|
||||
mc.path = cfg.path;
|
||||
mc.name = "MemoryTypeFallback";
|
||||
mc.outputName = "MemoryTypeFallback";
|
||||
mc.type = ConfigurationType::Executable;
|
||||
mc.target = cfg.target;
|
||||
mc.march = cfg.march;
|
||||
mc.mtune = cfg.mtune;
|
||||
mc.debug = cfg.debug;
|
||||
mc.sysroot = cfg.sysroot;
|
||||
mc.dependencies = cfg.dependencies;
|
||||
mc.externalDependencies = cfg.externalDependencies;
|
||||
mc.compileFlags = cfg.compileFlags;
|
||||
mc.linkFlags = cfg.linkFlags;
|
||||
mc.defines = cfg.defines;
|
||||
mc.cFiles = cfg.cFiles;
|
||||
std::vector<fs::path> memImpls(impls.begin(), impls.end());
|
||||
memImpls.emplace_back("tests/MemoryTypeFallback/main");
|
||||
mc.GetInterfacesAndImplementations(ifaces, memImpls);
|
||||
cfg.tests.push_back(std::move(memTest));
|
||||
}
|
||||
|
||||
return cfg;
|
||||
|
|
|
|||
146
tests/MemoryTypeFallback/main.cpp
Normal file
146
tests/MemoryTypeFallback/main.cpp
Normal file
|
|
@ -0,0 +1,146 @@
|
|||
/*
|
||||
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 #59: Device::GetMemoryType used to be a bare
|
||||
// first-superset match that threw whenever no memory type satisfied the
|
||||
// requested property flags. Callers that combine a mandatory flag with a
|
||||
// perf-only one (e.g. HOST_VISIBLE | DEVICE_LOCAL for the descriptor heaps)
|
||||
// would then crash at allocation time on any device without a host-visible
|
||||
// device-local heap (no resizable BAR).
|
||||
//
|
||||
// GetMemoryType now takes a `preferred` mask distinct from `required`:
|
||||
// - prefers a type satisfying required+preferred,
|
||||
// - falls back to a required-only match when the preference is unavailable,
|
||||
// - still throws only when even `required` cannot be met.
|
||||
//
|
||||
// The selection is pure CPU logic over Device::memoryProperties, so this test
|
||||
// drives it directly with synthetic memory layouts — no Vulkan device needed.
|
||||
|
||||
#include <cstdlib>
|
||||
#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;
|
||||
}
|
||||
|
||||
// Install a synthetic memory layout: one VkMemoryType per entry, in order.
|
||||
void SetMemoryTypes(std::initializer_list<VkMemoryPropertyFlags> flags) {
|
||||
Device::memoryProperties = {};
|
||||
Device::memoryProperties.memoryTypeCount = static_cast<std::uint32_t>(flags.size());
|
||||
std::uint32_t i = 0;
|
||||
for (VkMemoryPropertyFlags f : flags) {
|
||||
Device::memoryProperties.memoryTypes[i].propertyFlags = f;
|
||||
Device::memoryProperties.memoryTypes[i].heapIndex = 0;
|
||||
++i;
|
||||
}
|
||||
}
|
||||
|
||||
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;
|
||||
|
||||
// Call GetMemoryType, reporting whether it threw rather than returning.
|
||||
bool Threw(std::uint32_t typeBits, VkMemoryPropertyFlags required, VkMemoryPropertyFlags preferred) {
|
||||
try {
|
||||
Device::GetMemoryType(typeBits, required, preferred);
|
||||
return false;
|
||||
} catch (const std::runtime_error&) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
int main() {
|
||||
constexpr std::uint32_t kAll = ~0u;
|
||||
|
||||
// --- required-only: first superset match wins -------------------------
|
||||
{
|
||||
SetMemoryTypes({DEVICE_LOCAL, HOST_VISIBLE, HOST_VISIBLE | DEVICE_LOCAL});
|
||||
Check(Device::GetMemoryType(kAll, HOST_VISIBLE) == 1,
|
||||
"required-only picks the first type satisfying the required bits");
|
||||
Check(Device::GetMemoryType(kAll, DEVICE_LOCAL) == 0,
|
||||
"required-only honours superset ordering for a different mask");
|
||||
}
|
||||
|
||||
// --- typeBits gates which types are eligible --------------------------
|
||||
{
|
||||
SetMemoryTypes({HOST_VISIBLE, HOST_VISIBLE, HOST_VISIBLE});
|
||||
// Only bit 2 is set -> type index 2 is the sole candidate.
|
||||
Check(Device::GetMemoryType(0b100, HOST_VISIBLE) == 2,
|
||||
"typeBits mask excludes types the allocation can't use");
|
||||
}
|
||||
|
||||
// --- preferred bits are honoured when available -----------------------
|
||||
{
|
||||
SetMemoryTypes({HOST_VISIBLE, HOST_VISIBLE | HOST_COHERENT, HOST_VISIBLE | DEVICE_LOCAL});
|
||||
Check(Device::GetMemoryType(kAll, HOST_VISIBLE, DEVICE_LOCAL) == 2,
|
||||
"preferred DEVICE_LOCAL selects the host-visible device-local type");
|
||||
Check(Device::GetMemoryType(kAll, HOST_VISIBLE, HOST_COHERENT) == 1,
|
||||
"preferred HOST_COHERENT selects the host-visible coherent type");
|
||||
}
|
||||
|
||||
// --- preferred unavailable: fall back to required-only ----------------
|
||||
{
|
||||
// No host-visible type is also device-local (the no-resizable-BAR case).
|
||||
SetMemoryTypes({DEVICE_LOCAL, HOST_VISIBLE | HOST_COHERENT});
|
||||
Check(Device::GetMemoryType(kAll, HOST_VISIBLE, DEVICE_LOCAL) == 1,
|
||||
"unsatisfiable preference falls back to a required-only match instead of throwing");
|
||||
Check(!Threw(kAll, HOST_VISIBLE, DEVICE_LOCAL),
|
||||
"the fallback path does not throw when required is still satisfiable");
|
||||
}
|
||||
|
||||
// --- preferred is satisfiable only via the second-best type -----------
|
||||
{
|
||||
// First host-visible type lacks the preference; a later one has it.
|
||||
SetMemoryTypes({HOST_VISIBLE, HOST_VISIBLE | DEVICE_LOCAL});
|
||||
Check(Device::GetMemoryType(kAll, HOST_VISIBLE, DEVICE_LOCAL) == 1,
|
||||
"preference outranks ordering: skips an earlier required-only type");
|
||||
}
|
||||
|
||||
// --- required unsatisfiable still throws ------------------------------
|
||||
{
|
||||
SetMemoryTypes({DEVICE_LOCAL, HOST_VISIBLE | HOST_COHERENT});
|
||||
Check(Threw(kAll, HOST_CACHED, 0),
|
||||
"no type satisfies the required bits -> throws (no valid memory)");
|
||||
Check(Threw(kAll, HOST_CACHED, DEVICE_LOCAL),
|
||||
"unsatisfiable required throws even when a preference is given");
|
||||
// typeBits that masks out the only matching type is also unsatisfiable.
|
||||
SetMemoryTypes({HOST_VISIBLE});
|
||||
Check(Threw(0b10, HOST_VISIBLE, 0),
|
||||
"typeBits excluding every match -> throws");
|
||||
}
|
||||
|
||||
if (failures != 0) {
|
||||
std::println("{} check(s) failed", failures);
|
||||
return EXIT_FAILURE;
|
||||
}
|
||||
std::println("all checks passed");
|
||||
return EXIT_SUCCESS;
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue