Catcrafts/Crafter.Graphics
Watch
2
0
Fork
You've already forked Crafter.Graphics
 0
Code Issues 25 Pull requests Projects Releases Packages Wiki Activity Actions

fix(descriptor-heap): round the buffer region up, align the reserved range for buffers too, and never size a heap at zero

Browse source 
DescriptorHeapVulkan
- The buffer region started at images * imageDescriptorSize /
  bufferDescriptorSize, rounded down. A descriptor_heap array indexes
  the heap from byte 0 in strides of its descriptor size, so buffer slot
  0 has to sit on a whole buffer descriptor past the images. Where the
  image descriptors were not a whole number of buffer descriptors, the
  first buffer descriptors overwrote the last image descriptors. Where
  they were less than one, the start was bumped to descriptor 1, but the
  heap was still sized as if the buffers began right after the images,
  so the last buffer descriptors went into the implementation's reserved
  range. Every descriptor size is a power of two, so both cases need
  image descriptors smaller than buffer descriptors, which NVIDIA (32
  and 16 bytes) and llvmpipe (64 and 16) do not have. The start now
  rounds up and the heap is sized from it (#163).
- The reserved range sat at a multiple of imageDescriptorAlignment only,
  where vkCmdBindResourceHeapEXT also wants bufferDescriptorAlignment
  (VUID-vkCmdBindResourceHeapEXT-pBindInfo-11435 and -11436). It now
  starts on a multiple of the larger. The buffer descriptors need no more
  than the rounding: each alignment is at most its descriptor size, so a
  whole number of descriptors is aligned.
- bufferStartElement is 16 bits and took the division unchecked, so an
  image region past 65535 buffer descriptors (16384 images on llvmpipe)
  wrapped the buffer region around onto the images. Slot counts whose
  buffer indices do not fit 16 bits, 0xFFFF being the empty slot, now
  throw, naming the counts. A heap without buffer slots has no buffer
  region, which starts at 0 as in descriptor-set mode, so image-only
  heaps have no index to fit.
- On a driver that reserves nothing (llvmpipe), a heap without sampler
  slots was a zero-size buffer, which every Initialize(1, 1, 0) made,
  and Window::Render bound its address each frame. Each heap now holds at
  least one descriptor, as each binding does in descriptor-set mode. That
  grows NVIDIA's sampler heap by one descriptor when no sampler slots are
  asked for; the resource heaps of the slot counts in the tree keep their
  layouts byte for byte.
- ComputeLayout holds the arithmetic, pure over the descriptor-heap
  properties, and Initialize goes through it. GetBufferOffset and
  GetBufferOffsetElement, which had their own copies of the old
  arithmetic and never had a caller, are gone.
- Bind(cmd, frame) binds a frame's heaps with the reserved ranges
  Initialize stored, or in descriptor-set mode the frame's descriptor
  set. Window::Render and the RTVolume example worked the reserved-range
  offset out from the heap's size with the image-alignment mask, and
  call Bind now. The window's per-heap cache of bind structs goes with
  it: keyed on the heap pointer, it kept binding the old buffers after a
  re-Initialize that reallocated them. The heap still ends with its
  reserved range, so code that derives the offset from the size keeps
  getting the right one.

Tests
- DescriptorHeapLayout: the issue's two layouts, the reserved range where
  buffer descriptors are aligned more strictly than image ones, every
  legal combination of descriptor sizes and alignments up to 256 bytes,
  no heap empty, the NVIDIA and llvmpipe layouts, the 16-bit limit and
  image-only heaps. Pure CPU, no device. Against the old arithmetic it
  fails 12 of its 18 checks.

Docs
- README.md's heap bullet and AGENTS.consumer.md say how to bind the
  heap outside the frame loop, and the consumer notes that Initialize
  throws when buffer indices would pass 16 bits. AGENTS.md lists
  ComputeLayout among the functions the pure-logic tests drive.

Closes #163.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Jorijn van der Graaf 2026-10-31 16:02:58 +01:00
commit 852ae8abe3
Signed by: jorijnvdgraaf
GPG key ID: 2937E59CDCC1BCFB
9 changed files with 452 additions and 99 deletions
Download patch file Download diff file Expand all files Collapse all files

141
interfaces/Crafter.Graphics-DescriptorHeapVulkan.cppm
View file

@ -23,6 +23,10 @@ export namespace Crafter {
VulkanBuffer<std::uint8_t, true> samplerHeap[Window::numFrames];
std::uint32_t bufferStartOffset;
std::uint16_t bufferStartElement;
// Where each heap's reserved range starts; it runs to the heap's end.
// Heap mode only. Bind passes them on.
std::uint32_t resourceReservedRangeOffset = 0;
std::uint32_t samplerReservedRangeOffset = 0;
std::uint16_t imageCapacity = 0;
std::uint16_t bufferCapacity = 0;
@ -119,6 +123,77 @@ export namespace Crafter {
std::ranges::fill(sets, VkDescriptorSet{VK_NULL_HANDLE});
}
// Where Initialize puts things in each frame's two heaps: image
// descriptors from byte 0, the buffer descriptors after them, then
// the implementation's reserved range, with which the heap ends;
// sampler descriptors from byte 0 of the sampler heap, then its
// reserved range.
struct HeapLayout {
std::uint16_t bufferStartElement;
std::uint32_t bufferStartOffset;
std::uint32_t resourceReservedRangeOffset;
std::uint32_t resourceHeapSize;
std::uint32_t samplerReservedRangeOffset;
std::uint32_t samplerHeapSize;
bool operator==(const HeapLayout&) const = default;
};
// The layout of a heap with `images`, `buffers` and `samplers` slots
// under these descriptor sizes and alignments. Heap mode only:
// without the extension the sizes are zero. Pure, so the arithmetic
// can be checked against sizes no local driver reports.
//
// A descriptor_heap array indexes the heap from byte 0 in strides of
// its descriptor size (its ArrayStrideIdEXT is the OpConstantSizeOfEXT
// of the descriptor type), so buffer slot 0 has to sit on a whole
// buffer descriptor: the first one past the image descriptors. The
// start used to be rounded down (#163), which put the buffers over the
// last image descriptors wherever those were not a whole number of
// buffer descriptors, and where they were less than one, sized the
// heap as if the buffers began right after them, so the last buffer
// descriptors landed in the reserved range, which belongs to the
// implementation. Both need image descriptors smaller than buffer
// descriptors; NVIDIA's and llvmpipe's are not. Every descriptor size
// and alignment is a power of two, each alignment at most its size,
// so a whole number of descriptors is aligned for its kind. The
// reserved range has to start on a multiple of both resource
// alignments (VUID-vkCmdBindResourceHeapEXT-pBindInfo-11435 and
// -11436), which for powers of two is a multiple of the larger. The
// heap ends with the reserved range.
static HeapLayout ComputeLayout(const VkPhysicalDeviceDescriptorHeapPropertiesEXT& properties,
std::uint16_t images, std::uint16_t buffers, std::uint16_t samplers) {
const std::uint64_t imageEnd = images * properties.imageDescriptorSize;
// Without buffer slots there is no buffer region to place, and its
// start is 0, as in sets mode.
const std::uint64_t bufferStart = buffers == 0 ? 0 : (imageEnd + properties.bufferDescriptorSize - 1) / properties.bufferDescriptorSize;
// Shaders and BufferSlot carry absolute buffer indices in 16 bits,
// and 0xFFFF is the empty slot.
if (bufferStart + buffers > 0xFFFF) {
throw std::runtime_error(std::format("DescriptorHeapVulkan: {} image and {} buffer slots do not fit 16-bit slot indices (with {}-byte image and {}-byte buffer descriptors the buffer slots start at index {})",
images, buffers, properties.imageDescriptorSize, properties.bufferDescriptorSize, bufferStart));
}
const std::uint64_t bufferStartOffset = bufferStart * properties.bufferDescriptorSize;
// The reserved range follows whichever descriptors end last. Neither
// heap is ever empty, though: a zero-size buffer is invalid
// (VUID-VkBufferCreateInfo-size-00912), and on a driver that
// reserves nothing (llvmpipe) the common Initialize(1, 1, 0) made
// the sampler heap one. Each holds at least one descriptor, as each
// binding does in InitializeSets.
const std::uint64_t descriptorEnd = std::max({ imageEnd, bufferStartOffset + buffers * properties.bufferDescriptorSize,
properties.bufferDescriptorSize });
const std::uint64_t resourceReserved = AlignUp(descriptorEnd, std::max(properties.imageDescriptorAlignment, properties.bufferDescriptorAlignment));
const std::uint64_t samplerReserved = std::max<std::uint64_t>(samplers, 1) * properties.samplerDescriptorSize;
return {
.bufferStartElement = static_cast<std::uint16_t>(bufferStart),
.bufferStartOffset = static_cast<std::uint32_t>(bufferStartOffset),
.resourceReservedRangeOffset = static_cast<std::uint32_t>(resourceReserved),
.resourceHeapSize = static_cast<std::uint32_t>(resourceReserved + properties.minResourceHeapReservedRange),
.samplerReservedRangeOffset = static_cast<std::uint32_t>(samplerReserved),
.samplerHeapSize = static_cast<std::uint32_t>(samplerReserved + properties.minSamplerHeapReservedRange),
};
}
// Call once, before any shader is loaded against this heap. In heap
// mode a second call simply re-sizes the heap buffers; in sets mode it
// throws, because the pipelines already built against the old layout
@ -128,16 +203,11 @@ export namespace Crafter {
InitializeSets(images, buffers, samplers);
return;
}
std::uint32_t descriptorRegion = images * Device::descriptorHeapProperties.imageDescriptorSize + buffers * Device::descriptorHeapProperties.bufferDescriptorSize;
std::uint32_t alignedDescriptorRegion = (descriptorRegion + Device::descriptorHeapProperties.imageDescriptorAlignment - 1) & ~(Device::descriptorHeapProperties.imageDescriptorAlignment - 1);
std::uint32_t resourceSize = alignedDescriptorRegion + Device::descriptorHeapProperties.minResourceHeapReservedRange;
std::uint32_t samplerSize = samplers * Device::descriptorHeapProperties.samplerDescriptorSize + Device::descriptorHeapProperties.minSamplerHeapReservedRange;
bufferStartElement = images * Device::descriptorHeapProperties.imageDescriptorSize / Device::descriptorHeapProperties.bufferDescriptorSize;
if(images > 0 && bufferStartElement == 0) {
bufferStartElement = 1;
}
bufferStartOffset = bufferStartElement * Device::descriptorHeapProperties.bufferDescriptorSize;
const HeapLayout layout = ComputeLayout(Device::descriptorHeapProperties, images, buffers, samplers);
bufferStartElement = layout.bufferStartElement;
bufferStartOffset = layout.bufferStartOffset;
resourceReservedRangeOffset = layout.resourceReservedRangeOffset;
samplerReservedRangeOffset = layout.samplerReservedRangeOffset;
imageCapacity = images;
bufferCapacity = buffers;
@ -161,8 +231,8 @@ export namespace Crafter {
// 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, 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);
resourceHeap[i].Resize(VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT | VK_BUFFER_USAGE_DESCRIPTOR_HEAP_BIT_EXT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, layout.resourceHeapSize, 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, layout.samplerHeapSize, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT);
}
}
@ -503,30 +573,33 @@ export namespace Crafter {
return firstElement * Device::descriptorHeapProperties.samplerDescriptorSize;
}
// Both are 0 in sets mode: buffer slots are plain indices into their
// own binding, and the heap descriptor sizes divided by below are zero.
inline static std::uint32_t GetBufferOffset(std::uint16_t images, std::uint16_t buffers) {
// Binds frame `frame`'s heaps into `cmd`, with the reserved ranges
// where Initialize put them, or in sets mode the frame's descriptor
// set at the compute bind point. Window::Render does it for every
// frame it records; a command buffer outside the frame loop that
// dispatches against the heap does it itself (examples/RTVolume).
void Bind(VkCommandBuffer cmd, std::uint32_t frame) const {
if (!Device::descriptorHeapSupported) {
return 0;
// Every compute pipeline loaded against this heap shares
// pipelineLayout, so one bind stays valid across all of them,
// as the heap bind does.
vkCmdBindDescriptorSets(cmd, VK_PIPELINE_BIND_POINT_COMPUTE, pipelineLayout, 0, 1, &sets[frame], 0, nullptr);
return;
}
std::uint32_t bufferStartElement = images * Device::descriptorHeapProperties.imageDescriptorSize / Device::descriptorHeapProperties.bufferDescriptorSize;
if(images > 0 && bufferStartElement == 0) {
bufferStartElement = 1;
}
return bufferStartElement * Device::descriptorHeapProperties.bufferDescriptorSize;
}
inline static std::uint16_t GetBufferOffsetElement(std::uint16_t images, std::uint16_t buffers) {
if (!Device::descriptorHeapSupported) {
return 0;
}
std::uint16_t bufferStartElement = images * Device::descriptorHeapProperties.imageDescriptorSize / Device::descriptorHeapProperties.bufferDescriptorSize;
if(images > 0 && bufferStartElement == 0) {
bufferStartElement = 1;
}
return bufferStartElement;
const VkBindHeapInfoEXT resourceInfo {
.sType = VK_STRUCTURE_TYPE_BIND_HEAP_INFO_EXT,
.heapRange = { .address = resourceHeap[frame].address, .size = resourceHeap[frame].size },
.reservedRangeOffset = resourceReservedRangeOffset,
.reservedRangeSize = Device::descriptorHeapProperties.minResourceHeapReservedRange,
};
const VkBindHeapInfoEXT samplerInfo {
.sType = VK_STRUCTURE_TYPE_BIND_HEAP_INFO_EXT,
.heapRange = { .address = samplerHeap[frame].address, .size = samplerHeap[frame].size },
.reservedRangeOffset = samplerReservedRangeOffset,
.reservedRangeSize = Device::descriptorHeapProperties.minSamplerHeapReservedRange,
};
Device::vkCmdBindResourceHeapEXT(cmd, &resourceInfo);
Device::vkCmdBindSamplerHeapEXT(cmd, &samplerInfo);
}
};