diff --git a/AGENTS.consumer.md b/AGENTS.consumer.md index 3cab840..e0bb2a8 100644 --- a/AGENTS.consumer.md +++ b/AGENTS.consumer.md @@ -55,10 +55,11 @@ Frames are pipelined three deep and nothing waits for the GPU at steady state, s ### The bindless heap -`DescriptorHeapVulkan::Initialize(images, buffers, samplers)` sizes one resource heap and one sampler heap per frame (`resourceHeap[Window::numFrames]`, `samplerHeap[...]`, mapped `VulkanBuffer`s) and the slot allocators. `AllocateImageSlots(n)`, `AllocateBufferSlots(n)` and `AllocateSamplerSlots(n)` return `{firstElement, count}` ranges (contiguous; freed runs are reused), `Free*Slots` returns them, and running out throws naming the pool and the counts. +`DescriptorHeapVulkan::Initialize(images, buffers, samplers)` sizes one resource heap and one sampler heap per frame (`resourceHeap[Window::numFrames]`, `samplerHeap[...]`, mapped `VulkanBuffer`s) and the slot allocators. `AllocateImageSlots(n)`, `AllocateBufferSlots(n)` and `AllocateSamplerSlots(n)` return `{firstElement, count}` ranges (contiguous; freed runs are reused), `Free*Slots` returns them, and running out throws naming the pool and the counts. `Initialize` throws too when its buffer slots would need shader-visible indices past 16 bits. - **Buffer slot indices are relative.** `AllocateBufferSlots` indexes the heap's buffer region, which starts after the image region, while a GLSL `descriptor_heap` SSBO array indexes buffer descriptors from heap byte 0. The shader-visible index is `heap.bufferStartElement + raw`. The RAII `BufferSlot` converts to exactly that (its `operator std::uint16_t()`), `UIRenderer::RegisterBuffer` returns one, and raw code passes `heap.bufferStartElement` as a spec constant: the RT examples declare `layout(constant_id = 0) const uint16_t bufferStart = 0us;` and index `topLevelAS[bufferStart]`, which is why they ask for `shaderInt16`. Image slots index the heap as they are, in image-descriptor units; samplers live in their own heap. A wrong buffer index reads zeros, which looks like every field of every item being zero. In descriptor-set mode `bufferStartElement` is 0, so code that goes through the conversion is right in both. - Writing a descriptor by hand: `Device::vkWriteResourceDescriptorsEXT` into `heap.resourceHeap[f].value + heap.ImageByteOffset(imageRange.firstElement)` or `heap.BufferByteOffset(bufferRange.firstElement)`, `Device::descriptorHeapProperties.imageDescriptorSize` or `bufferDescriptorSize` bytes long, for each frame `f`, then `heap.resourceHeap[f].FlushDevice()`. `BufferByteOffset` takes the raw index `AllocateBufferSlots` returned and adds the region start itself; handing it a `BufferSlot`, which converts to the absolute index, adds the start twice and puts the descriptor in the wrong place. `ImageByteOffset` takes an image slot as it is. Per-frame resources (the swapchain image `window.imageViews[f]`, `tlases[f]`) are written into frame `f`'s heap; a shared resource into every frame's. `examples/VulkanTriangle/main.cpp` is the pattern. The `UIRenderer::Register*` helpers do this for you in both binding models. +- Binding the heap outside the frame loop: `Window::Render` binds `window.descriptorHeap` in every frame it records. A command buffer it does not record that dispatches against the heap (`window.StartInit()`'s, say) calls `heap.Bind(cmd, frame)`, which binds that frame's heaps with their reserved ranges, or in descriptor-set mode its descriptor set. Don't build the `VkBindHeapInfoEXT` by hand: where the reserved range sits is the heap's to decide. - **Descriptor-set fallback.** On a driver without `VK_EXT_descriptor_heap` (Mesa Turnip on Adreno) `Device::descriptorHeapSupported` is false and the same slots live in one bindless descriptor set per frame; slot handles, item structs, `UIDispatchHeader` and the `UIRenderer` API are unchanged, so application code does not branch. Ray tracing is unavailable there. `CRAFTER_GRAPHICS_NO_DESCRIPTOR_HEAP` forces this path on a driver that has the heap, to test it. Raw `vkWriteResourceDescriptorsEXT` code is heap-only; a consumer that must run on such a driver stays on the `UIRenderer::Register*` helpers. ### Buffers, images, samplers diff --git a/AGENTS.md b/AGENTS.md index ddb209e..0cde585 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -70,7 +70,7 @@ Tests are declared in `project.cpp`, not discovered, through the `LibraryTest(na A test is an `int main()`: exit 0 passes, nonzero fails, 77 skips (for a missing capability, never for a failed check). Each runs in its own process, in parallel, with a 60 s default timeout, and the output of a failed, crashed or timed-out test is in `build/test-logs/.log`; a later pass does not delete the log, so check its timestamp. What the tests need, in three groups: -- Pure logic, no GPU: they stage `Device`'s statics or feed synthetic data to the decision functions that were factored out for them (`Device::ResolveFeatures`, `ChooseDevice`, `CheckShaderCapabilities`, `GetMemoryType`, `PreferDirectDeviceWrite`, `PipelineCacheDataCompatible`, `AlignMappedFlushRange`, `BuildMipChainFinalBarriers`, `SwapchainStageUnion`, `BuildTlasWriteBarrier`, `UIRenderer::ClipFlags`, `WorkaroundNvidiaAS::Patch`, the font and input-field helpers). `PushConstantRewrite` and `UIFusedShader` compile GLSL with glslang and check the SPIR-V with spirv-val at runtime, and `ExampleShaders` compiles every GLSL shader under `examples/` the way crafter-build would, the descriptor-set build included, so a shader contract change that breaks an example fails here without building the example; `MouseScroll` and `PointerEnterLeave` drive the Wayland listener callbacks directly. +- Pure logic, no GPU: they stage `Device`'s statics or feed synthetic data to the decision functions that were factored out for them (`Device::ResolveFeatures`, `ChooseDevice`, `CheckShaderCapabilities`, `GetMemoryType`, `PreferDirectDeviceWrite`, `PipelineCacheDataCompatible`, `AlignMappedFlushRange`, `BuildMipChainFinalBarriers`, `SwapchainStageUnion`, `BuildTlasWriteBarrier`, `UIRenderer::ClipFlags`, `WorkaroundNvidiaAS::Patch`, `DescriptorHeapVulkan::ComputeLayout`, the font and input-field helpers). `PushConstantRewrite` and `UIFusedShader` compile GLSL with glslang and check the SPIR-V with spirv-val at runtime, and `ExampleShaders` compiles every GLSL shader under `examples/` the way crafter-build would, the descriptor-set build included, so a shader contract change that breaks an example fails here without building the example; `MouseScroll` and `PointerEnterLeave` drive the Wayland listener callbacks directly. - A headless device: `Device::Initialize()` and no `Window` (`HeapSlotReuse`, `DeferredDeletion`, `MeshLifetime`, `ImageStagingRelease`, `BLASBuildOptions`, the `TLAS*` tests, ...). Headless still means a Wayland display on Linux. Most of the RT ones check `Device::rayTracingSupported` and return 77 without it; `BLASBuildOptions`, `TLASInstanceDirtyTracking` and `MeshDecompressStagingRelease` do not, and crash on a GPU without ray tracing (`Device::RequireRayTracing` throws out of `main`). `DeviceSelection` skips with one GPU. - A window and a live compositor: `FrameLoopSync`, `SetupCmdBufferReuse`, `UIFallbackParity`, `SwapchainTransferDst`, `WindowUpdateClock`, `HeapASRead`, `HeapDynamicIndexHit`, `RTPassPushData`, `WindowOptionalGlobals` (hides xdg-decoration, viewporter and fractional scale from the window in turn), and `FeatureRequests` for its retry case. They render, read the frame back with `Window::CaptureNextFrame` (a PNG, decoded with a private `STB_IMAGE_STATIC` copy of stb_image) and check pixels. diff --git a/README.md b/README.md index 820bcba..cbaba4e 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,11 @@ shaded through an intersection shader with an any-hit cut-out. - **DescriptorHeapVulkan / DescriptorHeapWebGPU** — bindless slot allocators. Vulkan side allocates image/buffer/sampler slots in a `VK_EXT_descriptor_heap`; WebGPU side resolves slots to JS-side - handle-table cookies that the dispatch bridge binds per pass. + handle-table cookies that the dispatch bridge binds per pass. The + window binds the Vulkan heap in every frame it records; a command + buffer recorded outside the frame loop that dispatches against it + calls `heap.Bind(cmd, frame)`, which binds the frame's heaps, or its + descriptor set on a driver without `VK_EXT_descriptor_heap`. - **VulkanBuffer\ / WebGPUBuffer\** — typed buffer. Vulkan variant has optional host mapping and a `FlushDevice` that issues the right host-write barrier; WebGPU variant goes through diff --git a/examples/RTVolume/main.cpp b/examples/RTVolume/main.cpp index 7b91828..bf4b160 100644 --- a/examples/RTVolume/main.cpp +++ b/examples/RTVolume/main.cpp @@ -113,18 +113,7 @@ int main() { // descriptor-heap compute pipeline still expects a heap bound for its // push-data path — the render loop binds it every frame, but the init // command buffer hasn't, so bind it here before dispatching. - { - VkBindHeapInfoEXT resourceHeapInfo { - .sType = VK_STRUCTURE_TYPE_BIND_HEAP_INFO_EXT, - .heapRange = { - .address = descriptorHeap.resourceHeap[window.currentBuffer].address, - .size = static_cast(descriptorHeap.resourceHeap[window.currentBuffer].size), - }, - .reservedRangeOffset = (descriptorHeap.resourceHeap[window.currentBuffer].size - Device::descriptorHeapProperties.minResourceHeapReservedRange) & ~(Device::descriptorHeapProperties.imageDescriptorAlignment - 1), - .reservedRangeSize = Device::descriptorHeapProperties.minResourceHeapReservedRange, - }; - Device::vkCmdBindResourceHeapEXT(cmd, &resourceHeapInfo); - } + descriptorHeap.Bind(cmd, window.currentBuffer); ComputeShader aabbWriter; aabbWriter.Load("aabbs.comp.spv", descriptorHeap); diff --git a/implementations/Crafter.Graphics-Window.cpp b/implementations/Crafter.Graphics-Window.cpp index 3339e5f..810ec29 100644 --- a/implementations/Crafter.Graphics-Window.cpp +++ b/implementations/Crafter.Graphics-Window.cpp @@ -1339,44 +1339,8 @@ void Window::Render() { // buffer dictates GPU execution order, so the bind must come first. // Pass-side dispatches still run with the same heaps bound — moving // the bind earlier doesn't change anything for them. - if (descriptorHeap && !Device::descriptorHeapSupported) { - // Sets mode: the heap object holds one bindless descriptor set per - // frame instead of heap buffers. Every compute pipeline loaded against - // this heap shares its pipeline layout, so one bind here stays valid - // across all of them, exactly like the heap bind below. - vkCmdBindDescriptorSets(drawCmdBuffers[currentBuffer], VK_PIPELINE_BIND_POINT_COMPUTE, - descriptorHeap->pipelineLayout, 0, 1, - &descriptorHeap->sets[currentBuffer], 0, nullptr); - } else if (descriptorHeap) { - // Rebuild the cached per-frame bind structs only when the heap changes. - // The address/size of each frame's heap and the reservedRangeOffset - // bitmask are stable for a given heap, so this runs once per assignment - // rather than every frame. - if (descriptorHeap != cachedDescriptorHeap) { - for (std::uint8_t i = 0; i < numFrames; ++i) { - resourceHeapInfos[i] = { - .sType = VK_STRUCTURE_TYPE_BIND_HEAP_INFO_EXT, - .heapRange = { - .address = descriptorHeap->resourceHeap[i].address, - .size = static_cast(descriptorHeap->resourceHeap[i].size) - }, - .reservedRangeOffset = (descriptorHeap->resourceHeap[i].size - Device::descriptorHeapProperties.minResourceHeapReservedRange) & ~(Device::descriptorHeapProperties.imageDescriptorAlignment - 1), - .reservedRangeSize = Device::descriptorHeapProperties.minResourceHeapReservedRange - }; - samplerHeapInfos[i] = { - .sType = VK_STRUCTURE_TYPE_BIND_HEAP_INFO_EXT, - .heapRange = { - .address = descriptorHeap->samplerHeap[i].address, - .size = static_cast(descriptorHeap->samplerHeap[i].size) - }, - .reservedRangeOffset = descriptorHeap->samplerHeap[i].size - Device::descriptorHeapProperties.minSamplerHeapReservedRange, - .reservedRangeSize = Device::descriptorHeapProperties.minSamplerHeapReservedRange - }; - } - cachedDescriptorHeap = descriptorHeap; - } - Device::vkCmdBindResourceHeapEXT(drawCmdBuffers[currentBuffer], &resourceHeapInfos[currentBuffer]); - Device::vkCmdBindSamplerHeapEXT(drawCmdBuffers[currentBuffer], &samplerHeapInfos[currentBuffer]); + if (descriptorHeap) { + descriptorHeap->Bind(drawCmdBuffers[currentBuffer], currentBuffer); } onUpdate.Invoke({startTime, startTime-lastFrameBegin}); diff --git a/interfaces/Crafter.Graphics-DescriptorHeapVulkan.cppm b/interfaces/Crafter.Graphics-DescriptorHeapVulkan.cppm index 2802581..6f72e7b 100644 --- a/interfaces/Crafter.Graphics-DescriptorHeapVulkan.cppm +++ b/interfaces/Crafter.Graphics-DescriptorHeapVulkan.cppm @@ -23,6 +23,10 @@ export namespace Crafter { VulkanBuffer 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(samplers, 1) * properties.samplerDescriptorSize; + return { + .bufferStartElement = static_cast(bufferStart), + .bufferStartOffset = static_cast(bufferStartOffset), + .resourceReservedRangeOffset = static_cast(resourceReserved), + .resourceHeapSize = static_cast(resourceReserved + properties.minResourceHeapReservedRange), + .samplerReservedRangeOffset = static_cast(samplerReserved), + .samplerHeapSize = static_cast(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); } }; diff --git a/interfaces/Crafter.Graphics-Window.cppm b/interfaces/Crafter.Graphics-Window.cppm index e8057ae..d307c91 100644 --- a/interfaces/Crafter.Graphics-Window.cppm +++ b/interfaces/Crafter.Graphics-Window.cppm @@ -443,18 +443,6 @@ export namespace Crafter { VkPipelineStageFlags submitPipelineStages = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; std::vector passes; DescriptorHeapVulkan* descriptorHeap = nullptr; - // Cached per-frame heap-bind structs. The heap address/size per slot - // are stable once a heap is assigned; only currentBuffer varies frame - // to frame, so the structs (and the reservedRangeOffset arithmetic) - // are computed once per heap rather than every frame in Render(). - // Keyed on the heap pointer: whenever descriptorHeap differs from - // cachedDescriptorHeap the cache is rebuilt. This invalidates on any - // heap (re)assignment — heaps never resize today, so a stable pointer - // means stable ranges. Not tied to onResize (the heap is independent - // of swapchain size). - VkBindHeapInfoEXT resourceHeapInfos[numFrames]; - VkBindHeapInfoEXT samplerHeapInfos[numFrames]; - DescriptorHeapVulkan* cachedDescriptorHeap = nullptr; std::optional> clearColor; #else // DOM mode: the page IS the window. WebGPU device and canvas are diff --git a/project.cpp b/project.cpp index d7e14c5..b289507 100644 --- a/project.cpp +++ b/project.cpp @@ -648,6 +648,18 @@ extern "C" Configuration CrafterBuildProject(std::span a // Headless; needs any Vulkan device. cfg.tests.push_back(LibraryTest("HeapSlotReuse")); + // Issue #163: where DescriptorHeapVulkan::Initialize puts the buffer + // descriptors and the reserved ranges. The buffer region's start was + // rounded down, so on a driver whose image descriptors are smaller + // than its buffer descriptors the buffers overlapped the last images + // or ran into the reserved range, which was itself aligned for image + // descriptors only, and on llvmpipe, which reserves nothing, a heap + // without sampler slots was a zero-size buffer. + // DescriptorHeapVulkan::ComputeLayout is pure arithmetic over the + // descriptor sizes, so this test feeds it every legal combination, no + // GPU device needed at runtime. + cfg.tests.push_back(LibraryTest("DescriptorHeapLayout")); + // The debug messenger prints info messages only with validation on. // Headless; needs any Vulkan device. cfg.tests.push_back(LibraryTest("DebugMessengerSeverity")); diff --git a/tests/DescriptorHeapLayout/main.cpp b/tests/DescriptorHeapLayout/main.cpp new file mode 100644 index 0000000..2fd3a54 --- /dev/null +++ b/tests/DescriptorHeapLayout/main.cpp @@ -0,0 +1,322 @@ +//SPDX-License-Identifier: LGPL-3.0-only +//SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® + +// Where DescriptorHeapVulkan::Initialize puts the buffer descriptors and the +// reserved ranges (#163). 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 at or past the end of the image descriptors. Its index +// was rounded down instead: where the image descriptors were not a whole +// number of buffer descriptors, the first buffer descriptors overwrote the +// last image descriptors, and where they were less than one, the heap was +// sized as if the buffers began right after the images, so the last buffer +// descriptors landed in the implementation's reserved range. The reserved +// range sat at a multiple of imageDescriptorAlignment, where the spec asks +// for bufferDescriptorAlignment too, an image region past 65535 buffer +// descriptors wrapped bufferStartElement around, and on a driver that +// reserves nothing (llvmpipe) a heap without sampler slots was a zero-size +// buffer. Most of it needs descriptor sizes neither NVIDIA nor llvmpipe has, +// so the test feeds DescriptorHeapVulkan::ComputeLayout every combination of +// sizes and alignments the spec allows (powers of two, each alignment at +// most its size) and checks where everything lands, that no heap is empty, +// that those two drivers keep the layouts they had, and that slot counts +// whose buffer indices do not fit 16 bits throw while image-only heaps, +// which have no buffer index to fit, do not. +// +// Pure CPU logic, no Vulkan device. + +#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; +} + +using Layout = DescriptorHeapVulkan::HeapLayout; + +VkPhysicalDeviceDescriptorHeapPropertiesEXT Properties(VkDeviceSize imageSize, VkDeviceSize imageAlignment, + VkDeviceSize bufferSize, VkDeviceSize bufferAlignment, + VkDeviceSize samplerSize, VkDeviceSize samplerAlignment, + VkDeviceSize resourceReserved, VkDeviceSize samplerReserved) { + return { + .sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_DESCRIPTOR_HEAP_PROPERTIES_EXT, + .minSamplerHeapReservedRange = samplerReserved, + .minResourceHeapReservedRange = resourceReserved, + .samplerDescriptorSize = samplerSize, + .imageDescriptorSize = imageSize, + .bufferDescriptorSize = bufferSize, + .samplerDescriptorAlignment = samplerAlignment, + .imageDescriptorAlignment = imageAlignment, + .bufferDescriptorAlignment = bufferAlignment, + }; +} + +// The development machine's two drivers, from vulkaninfo: NVIDIA 615.78.08 +// on an RTX 4090, and llvmpipe from Mesa 26.2.5. +const VkPhysicalDeviceDescriptorHeapPropertiesEXT kNvidia = Properties(32, 32, 16, 8, 32, 32, 0x17a00, 0x200); +const VkPhysicalDeviceDescriptorHeapPropertiesEXT kLlvmpipe = Properties(64, 4, 16, 4, 32, 4, 0, 0); + +// 1, 2, 4, ... up to `max`. +std::vector PowersOfTwo(VkDeviceSize max) { + std::vector powers; + for (VkDeviceSize v = 1; v <= max; v *= 2) { + powers.push_back(v); + } + return powers; +} + +std::string Describe(const VkPhysicalDeviceDescriptorHeapPropertiesEXT& p) { + return std::format("{}-byte image descriptors aligned to {}, {}-byte buffer descriptors aligned to {}, a {}-byte reserved range", + p.imageDescriptorSize, p.imageDescriptorAlignment, p.bufferDescriptorSize, p.bufferDescriptorAlignment, + p.minResourceHeapReservedRange); +} + +std::string Format(const Layout& l) { + return std::format("buffers from descriptor {} (byte {}), the resource heap's reserved range at {} of {} bytes, the sampler heap's at {} of {}", + l.bufferStartElement, l.bufferStartOffset, l.resourceReservedRangeOffset, l.resourceHeapSize, + l.samplerReservedRangeOffset, l.samplerHeapSize); +} + +// What is wrong with where the resource heap's descriptors and reserved range +// land, or "" when nothing is. +std::string ResourceProblem(const VkPhysicalDeviceDescriptorHeapPropertiesEXT& p, std::uint16_t images, std::uint16_t buffers) { + const Layout l = DescriptorHeapVulkan::ComputeLayout(p, images, buffers, 0); + const std::uint64_t imageEnd = images * p.imageDescriptorSize; + const std::uint64_t bufferEnd = l.bufferStartOffset + buffers * p.bufferDescriptorSize; + if (buffers == 0) { + if (l.bufferStartElement != 0 || l.bufferStartOffset != 0) { + return std::format("with no buffer slots the buffer region starts at descriptor {}, byte {}, not at 0", l.bufferStartElement, l.bufferStartOffset); + } + } else { + if (l.bufferStartOffset != l.bufferStartElement * p.bufferDescriptorSize) { + return std::format("buffer slot 0 is at byte {}, not at buffer descriptor {}, where a shader reads it", l.bufferStartOffset, l.bufferStartElement); + } + if (l.bufferStartOffset < imageEnd) { + return std::format("the buffer descriptors start at byte {}, over the image descriptors, which end at {}", l.bufferStartOffset, imageEnd); + } + if (l.bufferStartOffset >= imageEnd + p.bufferDescriptorSize) { + return std::format("the buffer descriptors start at byte {}, a whole descriptor past the images' end at {}", l.bufferStartOffset, imageEnd); + } + if (l.bufferStartOffset % p.bufferDescriptorAlignment != 0) { + return std::format("the buffer descriptors start at byte {}, not a multiple of bufferDescriptorAlignment", l.bufferStartOffset); + } + } + const std::uint64_t descriptorEnd = std::max(imageEnd, bufferEnd); + if (l.resourceReservedRangeOffset < descriptorEnd) { + return std::format("the reserved range starts at byte {}, before the descriptors end at {}", l.resourceReservedRangeOffset, descriptorEnd); + } + if (l.resourceReservedRangeOffset % p.imageDescriptorAlignment != 0 || l.resourceReservedRangeOffset % p.bufferDescriptorAlignment != 0) { + return std::format("the reserved range starts at byte {}, not a multiple of both descriptor alignments", l.resourceReservedRangeOffset); + } + if (l.resourceHeapSize == 0) { + return "the heap is zero bytes, which no buffer can be"; + } + if (l.resourceHeapSize != l.resourceReservedRangeOffset + p.minResourceHeapReservedRange) { + return std::format("the heap is {} bytes, where the reserved range runs from {} to {}", l.resourceHeapSize, + l.resourceReservedRangeOffset, l.resourceReservedRangeOffset + p.minResourceHeapReservedRange); + } + return ""; +} + +std::string SamplerProblem(const VkPhysicalDeviceDescriptorHeapPropertiesEXT& p, std::uint16_t samplers) { + const Layout l = DescriptorHeapVulkan::ComputeLayout(p, 0, 0, samplers); + const std::uint64_t samplerEnd = samplers * p.samplerDescriptorSize; + if (l.samplerReservedRangeOffset < samplerEnd) { + return std::format("the reserved range starts at byte {}, before the sampler descriptors end at {}", l.samplerReservedRangeOffset, samplerEnd); + } + if (l.samplerReservedRangeOffset % p.samplerDescriptorAlignment != 0) { + return std::format("the reserved range starts at byte {}, not a multiple of samplerDescriptorAlignment", l.samplerReservedRangeOffset); + } + if (l.samplerHeapSize == 0) { + return "the heap is zero bytes, which no buffer can be"; + } + if (l.samplerHeapSize != l.samplerReservedRangeOffset + p.minSamplerHeapReservedRange) { + return std::format("the heap is {} bytes, where the reserved range runs from {} to {}", l.samplerHeapSize, + l.samplerReservedRangeOffset, l.samplerReservedRangeOffset + p.minSamplerHeapReservedRange); + } + return ""; +} + +std::string Thrown(std::uint16_t images, std::uint16_t buffers) { + try { + (void)DescriptorHeapVulkan::ComputeLayout(kLlvmpipe, images, buffers, 0); + } catch (const std::runtime_error& e) { + return e.what(); + } + return ""; +} + +} // namespace + +int main() { + // ── The two layouts the issue describes ───────────────────────────────── + // Image descriptors smaller than buffer descriptors. The spec makes every + // size a power of two, so this is the only way the image region can end + // inside a buffer descriptor. + { + const VkPhysicalDeviceDescriptorHeapPropertiesEXT p = Properties(32, 32, 64, 64, 32, 32, 0x17a00, 0x200); + // Three images end at byte 96, halfway through buffer descriptor 1. + const Layout overlap = DescriptorHeapVulkan::ComputeLayout(p, 3, 1, 0); + Check(overlap.bufferStartElement == 2 && overlap.bufferStartOffset == 128, + std::format("3 32-byte images, then 64-byte buffers: buffer slot 0 is descriptor 2, byte 128, past the images (got descriptor {}, byte {})", + overlap.bufferStartElement, overlap.bufferStartOffset)); + // One image is less than one buffer descriptor: the four buffers are + // descriptors 1 to 4, bytes 64 to 320, and the reserved range follows. + const Layout small = DescriptorHeapVulkan::ComputeLayout(p, 1, 4, 0); + Check(small.bufferStartOffset == 64 && small.resourceReservedRangeOffset == 320 && small.resourceHeapSize == 320 + 0x17a00, + std::format("1 32-byte image, then 4 64-byte buffers: the buffers end at byte 320 and the reserved range starts there (got buffers from byte {}, the reserved range at {}, a {}-byte heap)", + small.bufferStartOffset, small.resourceReservedRangeOffset, small.resourceHeapSize)); + } + + // ── The reserved range on a multiple of both alignments ───────────────── + // VUID-vkCmdBindResourceHeapEXT-pBindInfo-11435 and -11436. Five 16-byte + // images end at byte 80, a multiple of their own alignment but not of the + // buffer descriptors' 64. + { + const VkPhysicalDeviceDescriptorHeapPropertiesEXT p = Properties(16, 16, 64, 64, 32, 32, 0x17a00, 0x200); + const Layout l = DescriptorHeapVulkan::ComputeLayout(p, 5, 0, 0); + Check(l.resourceReservedRangeOffset == 128, + std::format("5 16-byte images and no buffers, with buffer descriptors aligned to 64: the reserved range starts at byte 128 (got {})", + l.resourceReservedRangeOffset)); + } + + // ── Every descriptor size and alignment the spec allows ───────────────── + // Powers of two from 1 to 256 bytes, each alignment a power of two no + // larger than its size, with and without a reserved range. + { + std::uint64_t layouts = 0; + std::string first; + for (const VkDeviceSize reserved : {VkDeviceSize{0}, VkDeviceSize{0x17a00}}) { + for (const VkDeviceSize imageSize : PowersOfTwo(256)) { + for (const VkDeviceSize imageAlignment : PowersOfTwo(imageSize)) { + for (const VkDeviceSize bufferSize : PowersOfTwo(256)) { + for (const VkDeviceSize bufferAlignment : PowersOfTwo(bufferSize)) { + const VkPhysicalDeviceDescriptorHeapPropertiesEXT p = + Properties(imageSize, imageAlignment, bufferSize, bufferAlignment, 32, 32, reserved, 0x200); + for (const std::uint16_t images : {0, 1, 2, 3, 5, 7, 8, 9, 31, 64}) { + for (const std::uint16_t buffers : {0, 1, 2, 3, 16}) { + ++layouts; + const std::string problem = ResourceProblem(p, images, buffers); + if (!problem.empty() && first.empty()) { + first = std::format("{} images and {} buffers with {}: {}", images, buffers, Describe(p), problem); + } + } + } + } + } + } + } + } + Check(first.empty(), std::format("{} resource heap layouts over every legal descriptor size and alignment ({})", + layouts, first.empty() ? "every one sound" : first)); + } + { + std::uint64_t layouts = 0; + std::string first; + for (const VkDeviceSize reserved : {VkDeviceSize{0}, VkDeviceSize{0x200}}) { + for (const VkDeviceSize size : PowersOfTwo(256)) { + for (const VkDeviceSize alignment : PowersOfTwo(size)) { + const VkPhysicalDeviceDescriptorHeapPropertiesEXT p = Properties(32, 32, 16, 16, size, alignment, 0x17a00, reserved); + for (const std::uint16_t samplers : {0, 1, 2, 3, 7, 16}) { + ++layouts; + const std::string problem = SamplerProblem(p, samplers); + if (!problem.empty() && first.empty()) { + first = std::format("{} {}-byte samplers aligned to {}, a {}-byte reserved range: {}", samplers, size, alignment, reserved, problem); + } + } + } + } + } + Check(first.empty(), std::format("and {} sampler heap layouts ({})", layouts, first.empty() ? "every one sound" : first)); + } + + // ── No heap is empty ──────────────────────────────────────────────────── + // llvmpipe reserves nothing, so a heap with no slots of its kind was a + // zero-size buffer, which Vulkan does not allow, and every Initialize(1, 1, + // 0) made one. + { + const Layout l = DescriptorHeapVulkan::ComputeLayout(kLlvmpipe, 1, 1, 0); + Check(l.samplerHeapSize == 32, std::format("llvmpipe, no sampler slots: the sampler heap holds one 32-byte descriptor (got {} bytes)", l.samplerHeapSize)); + const Layout none = DescriptorHeapVulkan::ComputeLayout(kLlvmpipe, 0, 0, 0); + Check(none.resourceHeapSize > 0, std::format("and with no slots at all the resource heap is not empty either ({} bytes)", none.resourceHeapSize)); + } + + // ── The drivers checked keep their layouts ────────────────────────────── + // On NVIDIA and llvmpipe an image descriptor is a whole number of buffer + // descriptors, and buffer descriptors are aligned no more strictly than + // image ones, so nothing rounds: these are the layouts Initialize gave + // them before, for slot counts the examples and tests use, except that a + // heap with no sampler slots now holds one sampler descriptor. + { + struct Case { + std::string_view driver; + VkPhysicalDeviceDescriptorHeapPropertiesEXT properties; + std::uint16_t images; + std::uint16_t buffers; + std::uint16_t samplers; + Layout expected; + }; + const Case cases[] = { + { "NVIDIA", kNvidia, 1, 1, 0, + { .bufferStartElement = 2, .bufferStartOffset = 32, .resourceReservedRangeOffset = 64, .resourceHeapSize = 64 + 0x17a00, + .samplerReservedRangeOffset = 32, .samplerHeapSize = 32 + 0x200 } }, + { "NVIDIA", kNvidia, 3, 3, 1, + { .bufferStartElement = 6, .bufferStartOffset = 96, .resourceReservedRangeOffset = 160, .resourceHeapSize = 160 + 0x17a00, + .samplerReservedRangeOffset = 32, .samplerHeapSize = 32 + 0x200 } }, + { "NVIDIA", kNvidia, 16, 16, 4, + { .bufferStartElement = 32, .bufferStartOffset = 512, .resourceReservedRangeOffset = 768, .resourceHeapSize = 768 + 0x17a00, + .samplerReservedRangeOffset = 128, .samplerHeapSize = 128 + 0x200 } }, + { "llvmpipe", kLlvmpipe, 1, 1, 0, + { .bufferStartElement = 4, .bufferStartOffset = 64, .resourceReservedRangeOffset = 80, .resourceHeapSize = 80, + .samplerReservedRangeOffset = 32, .samplerHeapSize = 32 } }, + { "llvmpipe", kLlvmpipe, 8, 16, 4, + { .bufferStartElement = 32, .bufferStartOffset = 512, .resourceReservedRangeOffset = 768, .resourceHeapSize = 768, + .samplerReservedRangeOffset = 128, .samplerHeapSize = 128 } }, + { "llvmpipe", kLlvmpipe, 16, 16, 4, + { .bufferStartElement = 64, .bufferStartOffset = 1024, .resourceReservedRangeOffset = 1280, .resourceHeapSize = 1280, + .samplerReservedRangeOffset = 128, .samplerHeapSize = 128 } }, + }; + for (const Case& c : cases) { + const Layout l = DescriptorHeapVulkan::ComputeLayout(c.properties, c.images, c.buffers, c.samplers); + Check(l == c.expected, std::format("{}, Initialize({}, {}, {}): {}{}", c.driver, c.images, c.buffers, c.samplers, + Format(c.expected), l == c.expected ? "" : std::format(" (got {})", Format(l)))); + } + } + + // ── Buffer indices past 16 bits throw ─────────────────────────────────── + // Shaders and BufferSlot carry absolute buffer indices in 16 bits, 0xFFFF + // being the empty slot. llvmpipe's 64-byte image descriptors are four + // 16-byte buffer descriptors each, so 16383 images put buffer slot 0 at + // index 65532. + { + const std::string fits = Thrown(16383, 3); + Check(fits.empty(), std::format("16383 images and 3 buffers on llvmpipe take buffer indices up to 65534 ({})", fits.empty() ? "they fit" : fits)); + const std::string past = Thrown(16383, 4); + Check(!past.empty(), std::format("a fourth buffer would take index 65535, the empty slot, and throws ({})", past.empty() ? "it did not" : past)); + const std::string wrapped = Thrown(20000, 1); + Check(!wrapped.empty(), std::format("20000 images put buffer slot 0 at index 80000 and throw rather than wrap around ({})", + wrapped.empty() ? "it did not" : wrapped)); + // Without buffer slots no buffer index has to fit, however far the + // images reach. + const std::string imageOnly = Thrown(0xFFFF, 0); + Check(imageOnly.empty(), std::format("65535 images and no buffers are accepted ({})", imageOnly.empty() ? "they were" : imageOnly)); + if (imageOnly.empty()) { + const Layout l = DescriptorHeapVulkan::ComputeLayout(kLlvmpipe, 0xFFFF, 0, 0); + Check(l.bufferStartElement == 0 && l.resourceReservedRangeOffset == 0xFFFF * 64, + std::format("with the empty buffer region at 0 and the reserved range right after the images, at byte {} (got buffers from descriptor {}, the reserved range at {})", + 0xFFFF * 64, l.bufferStartElement, l.resourceReservedRangeOffset)); + } + } + + std::println("{}", failures == 0 ? "all checks passed" : "check(s) failed"); + return failures == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +}