docs(adr): ADR 0011 — root data as GPU pointer, thin PSOs, split barriers (#691 Phase 6) - #761
Conversation
…iers (#691 Phase 6) Pre-decides three more Phase 6 design points ahead of that phase starting, the same way §1-§3 already pre-decided Phase 2/3's shape: - §4 — per-draw data (transforms, material scalars, heap indices, buffer pointers) becomes one struct from a new GPU-visible bump allocator, reached via a single push-constant pointer instead of per-draw UBO/SSBO binding. - §4.2 — indirect draw/dispatch takes the same single-pointer contract, so GPU-driven passes (culling, LOD, virtual geometry) write their own indirect args on GPU with no CPU round trip. - §5 — minimize PSO permutation axes before the first VkPipeline exists: vertex-pulling via buffer-device-address, dynamic depth/stencil/blend state. - §6 — split barriers as a GPU-pointer signal/wait pair, unified with Vulkan's existing timeline semaphores instead of a separate abstraction. Prompted by cross-referencing Sebastian Aaltonen's "Reducing Graphics API Complexity" talk against the existing ADR 0010/0011 design, which turned out to already independently match the sharpest parts of it (heap-bindless exclusively, no image-layout tracking, hazard-flag barriers). These four sections close the remaining gaps. Issue #691's Phase 6/7 bullets and Scope section were updated on GitHub to match; the more speculative "one generic shader-kernel type" idea from the same talk is tracked separately in #760 rather than phased here, since it needs hardware/microcode support that doesn't exist yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe ADR records decisions for GPU-visible root data, direct and indirect draw contracts, PSO permutation reduction, and split-barrier synchronization. ChangesRHI model decisions
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/adr/0011-rhi-neutral-resource-and-binding-model.md`:
- Around line 796-805: Update the push-constant discussion to state that
exceeding maxPushConstantsSize makes vkCmdPushConstants invalid rather than
silently truncating data. Preserve the guidance that push constants carry only
the 8-byte GPU pointer, and explicitly reject or avoid inline root payloads that
exceed the device limit.
- Around line 857-861: Update the §4.2 GPU-driven pass access declarations used
by RenderGraph::ComputeBarrierPlan to register the root-data buffer and the
allocator-backed root-data range in addition to the indirect-argument hazard.
Ensure the declared write and consuming access cover the same root-data range so
the compute-to-indirect-draw barrier is generated correctly.
- Around line 838-855: Expand the indirect draw/dispatch contract in section 4.2
to define how commands select root-data records despite Vulkan indirect command
fields lacking a root pointer. Specify whether the pushed pointer targets an
array, how drawId or firstInstance indexes each record, and how GPU-driven
compute writes that mapping; only retain the identical direct-call contract
after documenting the corresponding command and shader layout.
- Around line 927-954: Revise the ADR section describing RHI::GpuFence, Signal,
and Wait to choose a single synchronization model: either map them to Vulkan
VkSemaphore timeline handles and submission-level values, or define a separate
mapped-device-memory protocol with explicit atomic, cache, queue, and host-wait
semantics. Remove claims that timeline semaphores are plain GPU-memory
locations, and specify that GPU signal/wait operations are attached to
submission boundaries rather than arbitrary nodes within one command buffer.
- Around line 900-905: Add a shader-visible vertex-pulling contract before the
Phase 6 checkpoint, defining formats, offsets, strides, index type, index base,
and per-draw selection. Include the pilot shader contract and update the
vertex-pulling sequencing text to make this specification an explicit
prerequisite before implementation.
- Around line 816-818: Update the mapped root-write section to document the
allocation’s host-visible memory flags, explicitly accounting for the absence of
VK_MEMORY_PROPERTY_HOST_COHERENT_BIT. Define the required flush policy: flush
each written range before GPU read access when memory is not host-coherent,
while preserving the existing persistent-mapping behavior.
- Around line 813-815: Revise the lifetime guidance around
TransientPool::ReleaseAll() so allocator storage is reset only after the
relevant RHI::GpuFence or timeline value confirms GPU retirement, rather than at
the frame boundary alone. Update the linear/ring strategy description to
preserve allocations until submitted command buffers finish reading root data
and avoid reusing storage prematurely.
- Around line 891-898: Expand the PSO state-decision table to explicitly cover
cull mode, front face, primitive topology, polygon mode, color write mask,
sample mask, alpha-to-coverage, depth bias, line width, and all other
rasterization or blending flags that can remain static. For each axis, document
its Vulkan feature gate, fallback behavior when dynamic state is unavailable,
and whether it participates in the pipeline key; ensure Phase 6’s capability
audit can verify these decisions.
- Around line 773-785: Add the buffer device-address capability contract to the
ADR section covering root-struct buffer pointers, or centralize it in ADR 0010.
Declare VK_KHR_buffer_device_address and the required
PhysicalStorageBufferAddresses and PhysicalStorageBuffer64 features, along with
buffer usage/allocation flags and address alignment and lifetime requirements;
keep VK_KHR_shader_untyped_pointers documented only for its existing shader-side
use.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 19eae646-1056-4384-91f4-8338857f1d31
📒 Files selected for processing (1)
docs/adr/0011-rhi-neutral-resource-and-binding-model.md
| already pointed at, and the shader-side dependency it needs | ||
| (`VK_KHR_shader_untyped_pointers`) was *already* pinned in ADR 0010's | ||
| capability contract — for the narrower reason of heap-offset dereferencing. | ||
| Widening its use to carry the whole root struct is a design decision, not a | ||
| new dependency. | ||
|
|
||
| **Buffers become pointers, not heap slots.** Vertex, index, and storage data | ||
| are addressed by buffer device address embedded directly in the root struct, | ||
| mirroring how a texture is addressed via `HeapOffset` (§1.1). The resource heap | ||
| therefore continues to hold only texture descriptors, exactly as §1.2a already | ||
| decided for the unrelated reason that texture and buffer descriptors aren't | ||
| the same size — no analogous "buffer heap" is ever built, and the two | ||
| decisions reinforce rather than duplicate each other. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)0011-rhi-neutral-resource-and-binding-model\.md$|(^|/)0010-|VK_KHR|StorageBuffer|buffer_device_address|shader_untyped|PhysicalStorageBuffer' || true
echo
echo "Target section:"
if [ -f docs/adr/0011-rhi-neutral-resource-and-binding-model.md ]; then
sed -n '720,820p' docs/adr/0011-rhi-neutral-resource-and-binding-model.md | cat -n
fi
echo
echo "Search relevant terms in ADRs:"
rg -n "VK_KHR_(shader_untyped_pointers|buffer_device_address)|PhysicalStorageBuffer|BufferDeviceAddress|storageBuffer|buffer address|HeapOffset|pointer" docs/adr -S || trueRepository: drsnuggles8/OloEngineBase
Length of output: 14796
🌐 Web query:
Vulkan VK_KHR_buffer_device_address physicalStorageBuffer buffer device addresses spec
💡 Result:
The Vulkan VK_KHR_buffer_device_address extension, which was promoted to core in Vulkan 1.2 [1][2], provides a mechanism for accessing buffer memory in shaders using 64-bit physical addresses, bypassing the traditional descriptor-based access model [3][4]. This system relies on three main components: 1. API Support (VK_KHR_buffer_device_address): Applications create a VkBuffer with the VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT flag and allocate memory with the VK_MEMORY_ALLOCATE_DEVICE_ADDRESS_BIT flag [1]. Using the command vkGetBufferDeviceAddress, the application retrieves a 64-bit VkDeviceAddress, which represents the start of the buffer [3][5]. This address can be passed to shaders (e.g., via constant buffers or push constants) [3][4]. 2. SPIR-V/Shading Language Support (SPV_KHR_physical_storage_buffer): To use these addresses in shaders, the SPIR-V code must utilize the PhysicalStorageBuffer storage class [3][1]. This is enabled by the PhysicalStorageBufferAddresses capability, which introduces the PhysicalStorageBuffer64 addressing model [6][7]. This model allows for hybrid addressing where pointers in this specific storage class are treated as 64-bit physical pointers, while other storage classes remain logical [6][7]. 3. Usage: Once the shader receives the VkDeviceAddress, it can perform direct reads, writes, and atomic operations on the memory at that location, provided the address is within the range of a valid buffer created with the appropriate usage flags [5][4]. This capability is essential for advanced features like ray tracing and implementing complex, pointer-based data structures directly in GPU memory [3]. Top results: [3][1][6][5][7][2][4]
Citations:
- 1: https://docs.vulkan.org/guide/latest/buffer_device_address.html
- 2: https://docs.vulkan.org/guide/latest/mapping_data_to_shaders.html
- 3: https://docs.vulkan.org/refpages/latest/refpages/source/VK_KHR_buffer_device_address.html
- 4: https://docs.vulkan.org/spec/latest/chapters/descriptors.html
- 5: https://docs.vulkan.org/refpages/latest/refpages/source/vkGetBufferDeviceAddress.html
- 6: https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/KHR/SPV_KHR_physical_storage_buffer.asciidoc
- 7: https://github.khronos.org/SPIRV-Registry/extensions/KHR/SPV_KHR_physical_storage_buffer.html
🌐 Web query:
Vulkan VK_KHR_shader_untyped_pointers spec physicalStorageBuffer storageBuffer
💡 Result:
VK_KHR_shader_untyped_pointers does not introduce a special Vulkan descriptor type for physicalStorageBuffer vs storageBuffer; instead, it enables SPIR-V untyped pointers (OpTypeUntypedPointerKHR / OpUntypedVariableKHR / OpUntypedAccessChainKHR) so shaders can reinterpret memory through an “untyped” pointer while still starting from specific, layout-compatible storage classes/objects [1][2]. Key constraint tying it to physicalStorageBuffer storage buffers: 1) Unturned/untyped pointers are limited to storage classes that have an explicit layout. The VK_KHR_shader_untyped_pointers proposal states that it “limits the use of untyped pointers to storage classes with an explicit layout” to ensure consistent data layout across reinterpretations [1][3]. 2) SPIR-V’s untyped-pointer extension (SPV_KHR_untyped_pointers) defines that untyped pointers can be used with physical-storage-buffer-related pointer variables, but it also states restrictions on what variables/pointers may be declared with the PhysicalStorageBuffer storage class: - “OpVariable must not use the PhysicalStorageBuffer storage class. Variables must not use the PhysicalStorageBuffer storage class.” [2] This means you don’t declare a variable directly in the PhysicalStorageBuffer storage class when using SPV_KHR_untyped_pointers; rather, you use untyped-pointer constructs that can point to objects referenced via the PhysicalStorageBuffer addressing model. 3) For PhysicalStorageBuffer pointers (logical vs physical addressing), Vulkan’s physical storage buffer extension (SPV_KHR_physical_storage_buffer) adds the PhysicalStorageBuffer storage class and defines that it represents physical pointers (64-bit addresses) and only applies when the addressing model is PhysicalStorageBuffer64 [4]. So, how physicalStorageBuffer interacts with “storageBuffer” when using VK_KHR_shader_untyped_pointers: - Your storage buffer descriptor in Vulkan is still a storage buffer descriptor type (VK_DESCRIPTOR_TYPE_STORAGE_BUFFER) described as a struct whose members are load/store/atomic in shaders [5]. - When you want physical addressing (PhysicalStorageBuffer / buffer device address) you use SPV_KHR_physical_storage_buffer (and Vulkan equivalents) to access buffer memory using PhysicalStorageBuffer pointers and buffer device addresses, with all loads/stores/atomics through PhysicalStorageBuffer pointers constrained to addresses that fall within the address range of some buffer [5]. - VK_KHR_shader_untyped_pointers then controls how you can reinterpret what you load/store through those pointer accesses, but only where the pointed-to storage class has explicit layout, and with SPIR-V-level constraints preventing direct PhysicalStorageBuffer variable declarations (OpVariable/Variables must not use the PhysicalStorageBuffer storage class) [1][2]. Practical implication (mapping to your terms): - “storageBuffer” (descriptor type / storage buffer block) is the logical storage buffer object the shader normally declares and indexes. - “physicalStorageBuffer” (PhysicalStorageBuffer storage class in SPIR-V) is about how buffer device addresses are treated for pointer addressing; with untyped pointers enabled, you can reinterpret the data via untyped pointer access chains, but you must follow SPIR-V restrictions that disallow declaring variables directly in the PhysicalStorageBuffer storage class [2] and you must ensure the relevant storage class used with untyped pointers has an explicit layout [1].
Citations:
- 1: https://docs.vulkan.org/features/latest/features/proposals/VK_KHR_shader_untyped_pointers.html
- 2: https://github.khronos.org/SPIRV-Registry/extensions/KHR/SPV_KHR_untyped_pointers.html
- 3: https://github.khronos.org/Vulkan-Site/features/latest/features/proposals/VK_KHR_shader_untyped_pointers.html
- 4: https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/KHR/SPV_KHR_physical_storage_buffer.asciidoc
- 5: https://docs.vulkan.org/spec/latest/chapters/descriptors.html
🌐 Web query:
VK_KHR_untyped_buffer_host pointer VK_KHR_buffer_device_address difference
💡 Result:
There is no technical feature or Vulkan extension named VK_KHR_untyped_buffer_host_pointer. It appears this may be a conflation of several different, distinct Vulkan extensions related to memory, pointers, and shaders. To clarify the concepts you may be combining: VK_KHR_buffer_device_address is a significant, widely used Vulkan feature (promoted to core in Vulkan 1.2) that allows applications to query a 64-bit GPU virtual address (VkDeviceAddress) for a VkBuffer [1][2][3]. This address can be used within shaders to access buffer memory via pointers, effectively bypassing standard descriptor sets and enabling complex data structures like linked lists or trees directly on the GPU [2][4]. VK_KHR_shader_untyped_pointers is a separate extension that introduces support for untyped pointers in SPIR-V [5][6]. It allows shader authors to reinterpret data accessed through memory and atomic instructions without requiring the strict type declarations typically associated with strongly-typed pointers [5][6]. This simplifies shader code by separating type interpretation from addressing logic [6]. VK_EXT_external_memory_host relates to importing memory allocated by the host (CPU) into Vulkan using a host pointer [7]. This is an entirely different mechanism used for zero-copy memory sharing between the CPU and GPU, rather than for shader-based pointer arithmetic or buffer addressing [7]. In summary, VK_KHR_buffer_device_address is about creating and using GPU-side pointers for buffers [2][4], while VK_KHR_shader_untyped_pointers is about the flexibility of pointer typing within shader code [5][6]. Neither extension provides the functionality of the other, and they are distinct from host memory import mechanisms [7].
Citations:
- 1: https://docs.vulkan.org/refpages/latest/refpages/source/VK_KHR_buffer_device_address.html
- 2: https://docs.vulkan.org/tutorial/latest/Advanced_Vulkan_Compute/06_Advanced_Data_Structures/04_device_addressable_buffers.html
- 3: https://github.com/KhronosGroup/Vulkan-Guide/blob/main/chapters/buffer_device_address.adoc
- 4: https://docs.vulkan.org/samples/latest/samples/extensions/buffer_device_address/README.html
- 5: https://docs.vulkan.org/refpages/latest/refpages/source/VK_KHR_shader_untyped_pointers.html
- 6: https://docs.vulkan.org/features/latest/features/proposals/VK_KHR_shader_untyped_pointers.html
- 7: https://github.khronos.org/Vulkan-Site/refpages/latest/refpages/source/VkImportMemoryHostPointerInfoEXT.html
🌐 Web query:
Vulkan PhysicalStorageBuffer storage class spec PhysicalStorageBufferAddresses OpTypePointer PhysicalStorageBuffer shader code examples
💡 Result:
The PhysicalStorageBuffer storage class is a feature of SPIR-V, introduced by the SPV_KHR_physical_storage_buffer extension, that enables "buffer device addressing" in Vulkan [1][2]. It allows shaders to access buffer memory using raw 64-bit GPU virtual addresses, bypassing standard descriptor sets [2][3][4]. PhysicalStorageBuffer Overview The storage class is used within the PhysicalStorageBuffer64 addressing model [1][5][6]. Unlike logical addressing (used in standard Vulkan), where pointers are abstract, PhysicalStorageBuffer64 treats pointers to the PhysicalStorageBuffer storage class as physical 64-bit addresses [1][2][6]. Key Technical Requirements: - Capability: Shaders using this must declare OpCapability PhysicalStorageBufferAddresses [2][5][6]. - Memory Model: The SPIR-V module must use the PhysicalStorageBuffer64 addressing model [1][6]. - Alignment: Because the compiler cannot infer the alignment of arbitrary physical addresses, all OpLoad and OpStore operations on PhysicalStorageBuffer pointers must explicitly specify an alignment (e.g., Aligned 8 or Aligned 16) [6]. - Restrictions: OpVariable cannot use the PhysicalStorageBuffer storage class [1][5]. Instead, these pointers are typically passed to the shader via push constants or stored in other buffers [2][3][4]. When used in a variable or function parameter, the pointer must be decorated with either AliasedPointer or RestrictPointer [1][5]. SPIR-V Code Example (Conceptual) The following illustrates defining a pointer type and accessing it, based on SPIR-V specification patterns [2][6]:; Declaration requirements OpCapability PhysicalStorageBufferAddresses OpExtension "SPV_KHR_physical_storage_buffer" OpMemoryModel PhysicalStorageBuffer64 GLSL450; Define the type to point to %int_type = OpTypeInt 32 0 %ptr_type = OpTypePointer PhysicalStorageBuffer %int_type; Example access (requires a base address provided to the shader); %ptr is a loaded 64-bit address, %val is the data to store OpStore %ptr %val Aligned 4 High-Level Usage (GLSL) In GLSL, this feature is accessed via the GL_EXT_buffer_reference extension [7][4]. Instead of standard descriptors, you define a structure with the buffer_reference layout qualifier [4]: #extension GL_EXT_buffer_reference: enable layout(buffer_reference, std430) buffer MyData { vec4 value; }; layout(push_constant) uniform Constants { MyData myPtr; // Receives 64-bit address }; void main { // Access directly via the physical pointer vec4 v = myPtr.value; } This mechanism is essential for complex data structures like linked lists, trees, or graphs that are difficult to implement with static descriptor sets [7][4]. [1][2][4][6]
Citations:
- 1: https://github.khronos.org/SPIRV-Registry/extensions/KHR/SPV_KHR_physical_storage_buffer.html
- 2: https://docs.vulkan.org/guide/latest/buffer_device_address.html
- 3: https://docs.vulkan.org/spec/latest/chapters/descriptors.html
- 4: https://docs.vulkan.org/tutorial/latest/Advanced_Vulkan_Compute/06_Advanced_Data_Structures/04_device_addressable_buffers.html
- 5: https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/KHR/SPV_KHR_physical_storage_buffer.asciidoc
- 6: https://docs.vulkan.org/samples/latest/samples/extensions/buffer_device_address/README.html
- 7: https://vulkan.lunarg.com/doc/view/1.4.350.1/linux/antora/refpages/latest/refpages/source/VK_KHR_buffer_device_address.html
🌐 Web query:
SPV_KHR_physical_storage_buffer PhysicalStorageBuffer variable declaration StorageBuffer storage class
💡 Result:
The SPV_KHR_physical_storage_buffer extension introduces the PhysicalStorageBuffer storage class, which functions differently from the standard StorageBuffer storage class used in Vulkan and other SPIR-V environments [1][2]. Storage Class Distinction The PhysicalStorageBuffer storage class is specifically designed to support physical addressing (using 64-bit addresses), whereas the StorageBuffer class relies on logical addressing [1][2]. While StorageBuffer is typically used for standard shader storage buffer objects (SSBOs) accessed through descriptors, PhysicalStorageBuffer is used for buffer device address (BDA) pointers, allowing shaders to hold and dereference raw device memory addresses [2][3]. Variable Declaration Rules The extension explicitly prohibits the use of PhysicalStorageBuffer with OpVariable in the same way as standard storage classes [1][4]. Specifically: 1. OpVariable Declaration: An OpVariable must not use the PhysicalStorageBuffer storage class directly [1][4]. 2. Memory Object Definition: A memory object in this context is defined as an OpVariable (or function parameter) that holds a pointer to the PhysicalStorageBuffer storage class, or an array of such pointers [1][4]. 3. Aliasing Decorators: When an OpVariable is used to store a pointer of the PhysicalStorageBuffer storage class, the variable must be decorated with exactly one of either AliasedPointer or RestrictPointer [1][5]. This is necessary because pointers in this storage class are often loaded from memory or generated via conversion (e.g., OpConvertUToPtr) rather than being created directly by an OpVariable, requiring these decorations to clarify potential memory aliasing [1][4]. In contrast, a standard StorageBuffer variable is declared normally using OpVariable with a pointer type in the StorageBuffer storage class [6][7].
Citations:
- 1: https://github.khronos.org/SPIRV-Registry/extensions/KHR/SPV_KHR_physical_storage_buffer.html
- 2: https://docs.vulkan.org/guide/latest/buffer_device_address.html
- 3: https://docs.vulkan.org/guide/latest/mapping_data_to_shaders.html
- 4: https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/KHR/SPV_KHR_physical_storage_buffer.asciidoc
- 5: https://github.com/llvm/llvm-project/blob/main/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
- 6: https://github.khronos.org/SPIRV-Registry/extensions/KHR/SPV_KHR_storage_buffer_storage_class.html
- 7: https://docs.vulkan.org/guide/latest/extensions/shader_features.html
Declare the buffer device-address capability contract.
VK_KHR_shader_untyped_pointers is not the API path for buffer device addresses. The root struct can still pass an address through push constants, but addressable buffers need VK_KHR_buffer_device_address on the Vulkan side, plus PhysicalStorageBufferAddresses, PhysicalStorageBuffer64, buffer usage flags, allocation flags, and alignment/lifetime rules. Add those to this ADR or ADR 0010 so the contract matches the implementation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/0011-rhi-neutral-resource-and-binding-model.md` around lines 773 -
785, Add the buffer device-address capability contract to the ADR section
covering root-struct buffer pointers, or centralize it in ADR 0010. Declare
VK_KHR_buffer_device_address and the required PhysicalStorageBufferAddresses and
PhysicalStorageBuffer64 features, along with buffer usage/allocation flags and
address alignment and lifetime requirements; keep VK_KHR_shader_untyped_pointers
documented only for its existing shader-side use.
| **The Vulkan-specific wrinkle the talk's slides gloss over: push constants are | ||
| small.** The guaranteed minimum is 128 bytes (desktop drivers on the ADR 0010 | ||
| floor typically expose 256), which is nowhere near enough to hold a root | ||
| struct with several matrices and pointers inline. The push constant therefore | ||
| carries **only the 8-byte GPU pointer** to the bump-allocated struct — never | ||
| the struct's fields directly. This matches the talk's own CUDA-kernel-launch | ||
| framing (the "argument" is the pointer, not the payload) but is worth stating | ||
| explicitly because a first implementation reaching for `vkCmdPushConstants` | ||
| with an inline struct will silently truncate past 128/256 bytes rather than | ||
| fail loudly. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Vulkan registry vkCmdPushConstants size validity maxPushConstantsSize silently truncate documentation
💡 Result:
The Vulkan specification does not allow for silent truncation of push constant data when calling vkCmdPushConstants [1][2]. The size parameter provided to vkCmdPushConstants is strictly validated against the device's physical limits, and any call that exceeds the allowed range constitutes a validation error [1][2]. Specifically, the Vulkan specification imposes the following validation requirements for the size parameter: 1. The sum of the offset and size parameters must be less than or equal to the device's maxPushConstantsSize limit (VUID-vkCmdPushConstants-size-00371) [1][2]. 2. The size must be a multiple of 4 bytes (VUID-vkCmdPushConstants-size-00369) [1][2]. 3. The size must be greater than 0 (VUID-vkCmdPushConstants-size-arraylength) [1][2]. Because these are explicitly defined as validation requirements, if an application passes a size that exceeds maxPushConstantsSize, it violates the specification [1][2]. Under the Vulkan validation layers, such a call will trigger a validation error, and behavior is undefined if the application continues execution despite this error. There is no mechanism in the specification that silently truncates the data to fit within the limit; instead, the API call itself is considered invalid [1][2]. Application developers are responsible for ensuring that their push constant updates do not exceed the maxPushConstantsSize limit reported by the physical device [3][4].
Citations:
- 1: https://docs.vulkan.org/refpages/latest/refpages/source/vkCmdPushConstants.html
- 2: https://github.khronos.org/Vulkan-Site/refpages/latest/refpages/source/vkCmdPushConstants.html
- 3: https://github.com/LunarG/VulkanSamples/blob/master/API-Samples/push_constants/push_constants.cpp
- 4: https://docs.vulkan.org/tutorial/latest/Building_a_Simple_Engine/Lighting_Materials/03_push_constants.html
Correct the push-constant failure description.
vkCmdPushConstants does not silently truncate data beyond maxPushConstantsSize. If offset + size exceeds the device’s push-constant size limit, the call is invalid. Reject an inline root payload instead of relying on truncation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/0011-rhi-neutral-resource-and-binding-model.md` around lines 796 -
805, Update the push-constant discussion to state that exceeding
maxPushConstantsSize makes vkCmdPushConstants invalid rather than silently
truncating data. Preserve the guidance that push constants carry only the 8-byte
GPU pointer, and explicitly reject or avoid inline root payloads that exceed the
device limit.
| with a linear/ring strategy, reset at the same frame boundary | ||
| `TransientPool::ReleaseAll()` already uses, so the two lifetimes stay aligned | ||
| without inventing a third one (§1.2's "do not invent a third lifetime class" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)(0011-rhi-neutral-resource-and-binding-model\.md|.*rhi.*|.*RHI.*|.*fence.*|.*TransientPool.*)$|docs/adr/0011-rhi-neutral-resource-and-binding-model.md' || true
echo "== target lines =="
if [ -f docs/adr/0011-rhi-neutral-resource-and-binding-model.md ]; then
wc -l docs/adr/0011-rhi-neutral-resource-and-binding-model.md
sed -n '780,830p' docs/adr/0011-rhi-neutral-resource-and-binding-model.md
echo "== relevant cross-frame lines =="
sed -n '930,970p' docs/adr/0011-rhi-neutral-resource-and-binding-model.md
fi
echo "== search allocation/fence/TransientPool mentions =="
rg -n "TransientPool::ReleaseAll|ReleaseAll|RHI::GpuFence|GpuFence|root data|map|mapped|timeline|retire|Retire|fence complete|command buffer" docs/adr/0011-rhi-neutral-resource-and-binding-model.md .Repository: drsnuggles8/OloEngineBase
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ADR target sections =="
if [ -f docs/adr/0011-rhi-neutral-resource-and-binding-model.md ]; then
echo "target 780-825:"
sed -n '780,825p' docs/adr/0011-rhi-neutral-resource-and-binding-model.md
echo "fence section 930-970:"
sed -n '930,970p' docs/adr/0011-rhi-neutral-resource-and-binding-model.md
echo "lifecycle/fence/timeline mentions in ADR:"
rg -n "TransientPool::ReleaseAll|RHI::GpuFence|GpuFence|timeline|retire|Retire|submitted|GPU|sync|mapped|map|lifetime|frame boundary|cross-frame|Reset|Release" docs/adr/0011-rhi-neutral-resource-and-binding-model.md --max-count 80
fi
echo "== exact allocation/fence references in repo (filtered) =="
rg -n "TransientPool::ReleaseAll|RHI::GpuFence|GpuFence|timeline|Retire|retire|RootData|mapped|ReleaseAll" . \
--glob '!*.png' --glob '!*.jpg' --glob '!*.jpeg' --glob '!*.gif' \
--glob '!*.svg' --glob '!*.tiff' --glob '!*.otf' --glob '!*.ttf' \
--glob '!*.woff' --glob '!*.woff2' --glob '!*.eot' --glob '!*.map' \
--max-count 120Repository: drsnuggles8/OloEngineBase
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
path = Path("docs/adr/0011-rhi-neutral-resource-and-binding-model.md")
text = path.read_text()
start = text.index("reset at the same frame boundary")
end = text.index("This is the one part of", start)
proposal = text[start:end].strip()
checks = {
"resets_at_TransientPool_ReleaseAll_boundary": "TransientPool::ReleaseAll()" in proposal,
"persistently_mapped": "persistently mapped" in proposal.lower(),
"backed_by_VMA": "VMA" in proposal and "Vulkan Memory Allocator" not in proposal,
"cpu_writes_directly": "CPU writes" in proposal and "into GPU memory" in proposal,
"timeline_semaphore_present": re.search(r"\b**timeline semaphore**", text) is not None,
"gpu_fence_uses_same_lifetime": "same per-frame lifetime discipline" in text and "RHI::GpuFence" in text,
}
for k, v in checks.items():
print(f"{k}={v}")
print("\nRelevant source:")
print(proposal)
print("\nFence allocation snippet:")
print(text[text.index("RHI::GpuFence"):text.index("one persistently-mapped counter", text.index("RHI::GpuFence")) + len("one persistently-mapped counter")])
PYRepository: drsnuggles8/OloEngineBase
Length of output: 952
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
path = Path("docs/adr/0011-rhi-neutral-resource-and-binding-model.md")
text = path.read_text()
start = text.index("reset at the same frame boundary")
end = text.index("This is the one part of", start)
proposal = text[start:end].strip()
checks = {
"resets_at_TransientPool_ReleaseAll_boundary": "TransientPool::ReleaseAll()" in proposal,
"persistently_mapped": "persistently mapped" in proposal.lower(),
"backed_by_VMA": "VMA" in proposal and "Vulkan Memory Allocator" not in proposal,
"cpu_writes_directly": "CPU writes" in proposal and "into GPU memory" in proposal,
"timeline_semaphore_present": "**timeline semaphore**" in text,
"gpu_fence_uses_same_lifetime": "same per-frame lifetime discipline" in text and "RHI::GpuFence" in text,
"proposal_mentions_retirement_or_fence_completed": bool(re.search(r"(retir|fence complete|timeline completed)", proposal, flags=re.I)),
}
for k, v in checks.items():
print(f"{k}={v}")
print("\nRelevant source:")
print(proposal)
print("\nFence allocation snippet:")
print(text[text.index("RHI::GpuFence"):text.index("one persistently-mapped counter", text.index("RHI::GpuFence")) + len("one persistently-mapped counter")])
PYRepository: drsnuggles8/OloEngineBase
Length of output: 944
Tie allocator reset to GPU retirement.
TransientPool::ReleaseAll() is a frame boundary, but it does not guarantee submitted command buffers have stopped reading root data. The same lifetime is reused for cross-frame RHI::GpuFence values, so retire allocations after the relevant timeline or fence completes instead of resetting storage while the GPU can still reference it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/0011-rhi-neutral-resource-and-binding-model.md` around lines 813 -
815, Revise the lifetime guidance around TransientPool::ReleaseAll() so
allocator storage is reset only after the relevant RHI::GpuFence or timeline
value confirms GPU retirement, rather than at the frame boundary alone. Update
the linear/ring strategy description to preserve allocations until submitted
command buffers finish reading root data and avoid reusing storage prematurely.
| rule applies here too). Because the allocation is persistently mapped | ||
| (ReBAR/UMA, per the driver floor), the CPU writes the struct's fields directly | ||
| into GPU memory — no staging buffer, no copy command. This is the one part of |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant ADR section, plus nearby allocation/mapping mentions.
file="docs/adr/0011-rhi-neutral-resource-and-binding-model.md"
if [ -f "$file" ]; then
echo "== file exists =="
wc -l "$file"
echo
echo "== relevant lines 780-845 =="
sed -n '780,845p' "$file" | nl -ba -v780
else
echo "missing $file"
fi
echo
echo "== allocate/map/host coherency mentions in ADR =="
rg -n -i 'persist.map|persistently mapped|VMA|vma|vk_allocate_memory|deviceLocal|hostVisible|HOST_COHERENT|memory property|mapping|map memory|flush|invalidate' "$file"Repository: drsnuggles8/OloEngineBase
Length of output: 313
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="docs/adr/0011-rhi-neutral-resource-and-binding-model.md"
echo "== relevant lines 780-845 =="
sed -n '780,845p' "$file"
echo
echo "== allocation/map/host coherency mentions in ADR =="
rg -n -i 'persist.map|persistently mapped|VMA|vma|vk_allocate_memory|deviceLocal|hostVisible|HOST_COHERENT|memory property|mapping|map memory|flush|invalidate' "$file"Repository: drsnuggles8/OloEngineBase
Length of output: 6847
🌐 Web query:
Vulkan deviceLocalHostVisible mapped resource HCU coherence without host coherency memory property
💡 Result:
When working with Vulkan memory that is host-visible but lacks the VK_MEMORY_PROPERTY_HOST_COHERENT_BIT, the memory is considered non-coherent [1][2]. In this state, the host (CPU) and device (GPU) may maintain separate, inconsistent views of the memory in their respective caches [3][4]. To manage visibility and availability of data in non-coherent memory, you must manually manage cache consistency using the following mechanisms: 1. CPU Cache Management: Before the device reads data written by the CPU, you must call vkFlushMappedMemoryRanges to flush the CPU's writes out to the memory domain [5][4]. Conversely, before the CPU reads data written by the device, you must call vkInvalidateMappedMemoryRanges to discard any stale CPU cache lines and pull in the latest data from the memory domain [6][3]. 2. Alignment Constraints: When working with non-coherent memory, operations must be performed on memory ranges aligned to the VkPhysicalDeviceLimits::nonCoherentAtomSize [5][4]. If you do not explicitly flush or invalidate on these boundaries, you may encounter data inconsistencies or race conditions [5][4]. 3. Execution Synchronization: Even if memory is coherent, it does not imply synchronization of execution [3]. You must always use proper Vulkan synchronization primitives (such as barriers, fences, or semaphores) to ensure that the producer has finished writing and the consumer is ready to read [3][5][7]. Coherence only handles the data movement between caches; it does not replace the need to order operations relative to one another [3]. In summary, for memory that is Device Local and Host Visible but not Host Coherent, you are responsible for both explicit cache management (flushing/invalidating on the CPU) and explicit execution synchronization (to prevent data races) [3][5][4]. If a memory type lacks the host coherent property, the implementation will not automatically make writes visible across the CPU/GPU boundary [1][3].
Citations:
- 1: https://docs.vulkan.org/refpages/latest/refpages/source/VkMemoryPropertyFlagBits.html
- 2: https://registry.khronos.org/VulkanSC/specs/1.0-extensions/man/html/VkMemoryPropertyFlagBits.html
- 3: https://github.khronos.org/Vulkan-Site/tutorial/latest/Synchronization/Anatomy_of_a_Dependency/02_execution_vs_memory.html
- 4: https://registry.khronos.org/VulkanSC/specs/1.0-extensions/man/html/vkFlushMappedMemoryRanges.html
- 5: https://docs.vulkan.org/refpages/latest/refpages/source/vkMapMemory.html
- 6: https://www.khronos.org/assets/uploads/developers/library/2018-vulkan-devday/03-Memory.pdf
- 7: https://community.khronos.org/t/clarification-on-coherent-memory-host-access-guarantees/110989
🌐 Web query:
Vulkan specification mapped memory HCU coherence hostVisibleDeviceLocal device writes CPU map memory coherent
💡 Result:
In Vulkan, memory coherence and visibility are governed by specific memory property flags and synchronization requirements [1][2]. Here is the breakdown of how these concepts interact with host-mapped memory and device-local memory. Memory Property Flags VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT: This flag indicates that the memory can be mapped into the host's address space using vkMapMemory, allowing the CPU to read or write to it directly [1][3]. VK_MEMORY_PROPERTY_HOST_COHERENT_BIT: When this flag is set, host cache management (vkFlushMappedMemoryRanges and vkInvalidateMappedMemoryRanges) is not required [1][2]. The hardware automatically ensures that CPU writes to the mapped memory are available to the device, and device writes are visible to the CPU [1][4]. Host-Visible, Device-Local Memory It is possible to allocate memory that is both host-visible and device-local [5][6]. This allows the CPU to access the memory directly while the GPU enjoys high-performance access as if it were VRAM [6][7]. When this memory also has the HOST_COHERENT_BIT set, the CPU and GPU can interact without explicit cache flushing [6][7]. Device Writes and CPU Mapping Regardless of coherence, you must guarantee synchronization when the device and host share access to memory [3]. If the memory is host-coherent, you do not need to call vkFlushMappedMemoryRanges or vkInvalidateMappedMemoryRanges, but you must still provide proper execution dependencies (e.g., using pipeline barriers or semaphores) to ensure that the device has finished writing to or reading from the memory before the CPU accesses it, and vice versa [3][7]. If the memory is NOT host-coherent, you must manually manage cache visibility using vkFlushMappedMemoryRanges and vkInvalidateMappedMemoryRanges [1][3]. Furthermore, when mapping such memory, you are responsible for flushing or invalidating the memory in increments of at least VkPhysicalDeviceLimits::nonCoherentAtomSize to ensure data integrity [3][8]. Summary Table Memory Property | Host Cache Management (Flush/Invalidate) Required? | Notes --- | --- | --- Host Coherent | No [1][7] | Still requires execution synchronization (barriers/semaphores) [3][7]. Non-Coherent | Yes [1][3] | Must manage flush/invalidate ranges (aligned to nonCoherentAtomSize) [3][8]. For optimal performance, applications typically use device-local memory for steady-state data and host-visible/coherent memory for dynamic updates or staging [6][7].
Citations:
- 1: https://docs.vulkan.org/refpages/latest/refpages/source/VkMemoryPropertyFlagBits.html
- 2: https://docs.vulkan.org/spec/latest/chapters/memory.html
- 3: https://docs.vulkan.org/refpages/latest/refpages/source/vkMapMemory.html
- 4: https://stackoverflow.com/questions/45017121/are-host-cached-bit-and-host-coherent-bit-contradicting-each-other
- 5: https://community.khronos.org/t/about-device-memory-and-host-local-memory/112098/1
- 6: https://gpuopen.com/learn/vulkan-device-memory/
- 7: https://github.khronos.org/Vulkan-Site/tutorial/latest/Synchronization/Anatomy_of_a_Dependency/02_execution_vs_memory.html
- 8: https://github.khronos.org/Vulkan-Site/refpages/latest/refpages/source/vkMapMemory.html
Define host visibility for mapped root writes.
This allocation is host-visible, but persistent mapped memory is not host-coherent by default. If the Vulkan/VMA allocation lacks VK_MEMORY_PROPERTY_HOST_COHERENT_BIT, flush each written range before GPU read access; document the allocation flags and flush policy here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/0011-rhi-neutral-resource-and-binding-model.md` around lines 816 -
818, Update the mapped root-write section to document the allocation’s
host-visible memory flags, explicitly accounting for the absence of
VK_MEMORY_PROPERTY_HOST_COHERENT_BIT. Define the required flush policy: flush
each written range before GPU read access when memory is not host-coherent,
while preserving the existing persistent-mapping behavior.
| ### 4.2 GPU-driven indirect root data: no separate shape for a compute-written draw | ||
|
|
||
| Because root data is just a GPU pointer, the struct an ordinary draw call | ||
| points to and the struct an *indirect* draw call points to are the same kind | ||
| of thing — the only difference is who writes it. **Decision: indirect | ||
| draw/dispatch takes the identical single-pointer root-data contract as a | ||
| direct call.** There is no separate "indirect root data" shape, no CPU-side | ||
| indirect-args staging step, and no driver-managed command-signature object | ||
| (the DX12-shaped `ID3D12CommandSignature` has nothing to abstract here, | ||
| because the pointer *is* the argument list already). A GPU-driven pass — | ||
| visibility/occlusion culling, LOD selection, virtual-geometry cluster culling | ||
| (`VirtualClusterCull.comp`, per `docs/agent-rules/cluster-lod-simplification.md`) | ||
| — writes its output root-data structs directly into GPU memory and issues the | ||
| indirect draw pointing at them; nothing round-trips through the CPU. This | ||
| codebase already has prior art for "a GPU-written buffer is the argument list," | ||
| in the two-counter overflow contract issue #725's GPU debug draws use — the | ||
| same shape, applied here to indirect draw/dispatch arguments instead of debug | ||
| primitives. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate ADR and related files =="
fd -a '0011-rhi-neutral-resource-and-binding-model|cluster-lod-simplification|RenderGraph|BarrierPlan|Indirect' . | sed 's#^\./##' | head -200
echo
echo "== Inspect relevant ADR lines =="
if [ -f docs/adr/0011-rhi-neutral-resource-and-binding-model.md ]; then
sed -n '800,870p' docs/adr/0011-rhi-neutral-resource-and-binding-model.md | cat -n
fi
echo
echo "== Search for indirect draw/dispatch/vk indirect definitions and root data terms =="
rg -n "indirect|DrawIndirect|DispatchIndirect|root data|root pointer|RootData|drawId|firstInstance|draw index|pipeline layout|descriptor" docs src include apps test -S 2>/dev/null | head -300Repository: drsnuggles8/OloEngineBase
Length of output: 30606
🌐 Web query:
Vulkan VkDrawIndirectCommand fields VkDispatchIndirectCommand fields documentation
💡 Result:
In Vulkan, the VkDrawIndirectCommand and VkDispatchIndirectCommand structures are used to provide parameters for indirect drawing and compute dispatching commands, respectively. Both structures are composed entirely of 32-bit unsigned integers [1][2]. VkDrawIndirectCommand Structure The VkDrawIndirectCommand structure specifies the parameters for an indirect drawing command [1]: typedef struct VkDrawIndirectCommand { uint32_t vertexCount; uint32_t instanceCount; uint32_t firstVertex; uint32_t firstInstance; } VkDrawIndirectCommand; Fields: vertexCount: The number of vertices to draw [1][3]. instanceCount: The number of instances to draw [1][3]. firstVertex: The index of the first vertex to draw [1][3]. firstInstance: The instance ID of the first instance to draw [1][3]. The members of this structure have the same meaning as the parameters of the vkCmdDraw command [1][4]. VkDispatchIndirectCommand Structure The VkDispatchIndirectCommand structure specifies the parameters for an indirect dispatching command [2]: typedef struct VkDispatchIndirectCommand { uint32_t x; uint32_t y; uint32_t z; } VkDispatchIndirectCommand; Fields: x: The number of local workgroups to dispatch in the X dimension [2][5]. y: The number of local workgroups to dispatch in the Y dimension [2][5]. z: The number of local workgroups to dispatch in the Z dimension [2][5]. The members of this structure have the same meaning as the corresponding parameters of the vkCmdDispatch command [2][5]. These values must be less than or equal to the corresponding limits defined in VkPhysicalDeviceLimits::maxComputeWorkGroupCount [2][5].
Citations:
- 1: https://docs.vulkan.org/refpages/latest/refpages/source/VkDrawIndirectCommand.html
- 2: https://docs.vulkan.org/refpages/latest/refpages/source/VkDispatchIndirectCommand.html
- 3: https://registry.khronos.org/VulkanSC/specs/1.0-extensions/man/html/VkDrawIndirectCommand.html
- 4: https://github.khronos.org/Vulkan-Site/refpages/latest/refpages/source/VkDrawIndirectCommand.html
- 5: https://github.com/KhronosGroup/Vulkan-Docs/blob/main/chapters/dispatch.adoc
Define how indirect commands select root data.
VkDrawIndirectCommand contains only vertexCount, instanceCount, firstVertex, and firstInstance; VkDispatchIndirectCommand contains only x, y, and z. Neither carries the 64-bit root-pointer value.
State whether one pushed pointer addresses an array of root records, how drawId or firstInstance selects each record, and how the GPU-driven compute pass writes that mapping. Do not claim the indirect path uses the same direct-call contract until that selection is part of the command and shader layout.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/0011-rhi-neutral-resource-and-binding-model.md` around lines 838 -
855, Expand the indirect draw/dispatch contract in section 4.2 to define how
commands select root-data records despite Vulkan indirect command fields lacking
a root pointer. Specify whether the pushed pointer targets an array, how drawId
or firstInstance indexes each record, and how GPU-driven compute writes that
mapping; only retain the identical direct-call contract after documenting the
corresponding command and shader layout.
| The only new requirement is the ordinary one: a barrier between the writing | ||
| compute dispatch and the consuming indirect draw, using §1.5's existing | ||
| hazard-flag model (an indirect-args hazard, the same category §1.5 already | ||
| names for descriptor writes). No new barrier machinery — this is an | ||
| application of §1.5, not an extension of it. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files around ADR 0011:"
if [ -f docs/adr/0011-rhi-neutral-resource-and-binding-model.md ]; then
wc -l docs/adr/0011-rhi-neutral-resource-and-binding-model.md
sed -n '820,880p' docs/adr/0011-rhi-neutral-resource-and-binding-model.md | cat -n
else
echo "doc file missing"
fi
echo
echo "Search for RenderGraph::ComputeBarrierPlan and hazard terminology:"
rg -n "RenderGraph::ComputeBarrierPlan|ComputeBarrierPlan|indirect-args|indirect args|indirect[ -]arg|root-data|root_data|hazard|barrier" docs src . --glob '!*' 2>/dev/null | head -n 200Repository: drsnuggles8/OloEngineBase
Length of output: 4213
Register the root-data buffer hazard too.
§4.2 says a GPU-driven pass writes root-data structs directly and issues the indirect draw. If RenderGraph::ComputeBarrierPlan builds barriers from declared pass access ranges, add a root-data access/range declaration for the same buffer. An indirect-argument barrier does not cover the writing compute dispatch against the consumer’s allocator-backed root-data range.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/0011-rhi-neutral-resource-and-binding-model.md` around lines 857 -
861, Update the §4.2 GPU-driven pass access declarations used by
RenderGraph::ComputeBarrierPlan to register the root-data buffer and the
allocator-backed root-data range in addition to the indirect-argument hazard.
Ensure the declared write and consuming access cover the same root-data range so
the compute-to-indirect-draw barrier is generated correctly.
| **Decision, per state axis, following Aaltonen's thin-PSO argument:** | ||
|
|
||
| | Axis | Decision | Why it's safe on this ADR's driver floor | | ||
| | --- | --- | --- | | ||
| | Vertex input layout | **Not baked at all.** Extend Phase 3's bindless-texture pulling pattern to vertex data: vertex shaders fetch through a buffer-device-address pointer with manual indexing, not `VkPipelineVertexInputStateCreateInfo`. | Removes the axis entirely rather than making it cheaper — the same move §4 makes for buffers generally, applied to the one buffer type (vertex) that still has a dedicated binding mechanism. | | ||
| | Depth/stencil state | Dynamic, via `VK_EXT_extended_dynamic_state` / `…state2` (core-promoted). | Universal on desktop; Metal has proven the same split safe across Nvidia/AMD/Intel/Apple since its first version (talk, §4.8). | | ||
| | Blend state | Dynamic where `VK_EXT_extended_dynamic_state3`'s blend-enable/equation states are available; baked into the PSO only where they aren't. | The talk's finding is that *only* Apple/PowerVR mobile GPUs genuinely require blend baked into the shader — irrelevant to this ADR's Vulkan desktop floor (NVIDIA/AMD), so the fallback path should never actually trigger here. Verify at Phase 6's device-capability audit rather than assume — same discipline ADR 0010 used for the heap extension's feature bits. | | ||
| | Rasterizer/render-target description | Stays minimal: target formats/count, depth/stencil format, sample count, dual-source blending flag. Nothing else. | This is already the shape §1's survey implies; stating it here makes it a decision instead of an accident. | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## repo files matching ADR 0011"
git ls-files | rg '(^|/)0011-rhi-neutral-resource-and-binding-model\.md$|ads/adr/0011|docs/adr/0011' || true
echo "## file size"
wc -l docs/adr/0011-rhi-neutral-resource-and-binding-model.md
echo "## relevant lines 860-920"
sed -n '860,920p' docs/adr/0011-rhi-neutral-resource-and-binding-model.md | cat -n
echo "## search for dynamic state and PSO keys"
rg -n "ExtendedDynamicState|extended_dynamic_state|pipeline binding|pipeline key|pipelineKeys|pipeline key|cull|front face|primitive topology|polygon|sample mask|alpha-to-coverage|blend-enable|blend equation" docs/adr/0011-rhi-neutral-resource-and-binding-model.mdRepository: drsnuggles8/OloEngineBase
Length of output: 6517
🌐 Web query:
Vulkan VK_EXT_extended_dynamic_state3 dynamicStateBlendEnable dynamicStateBlendEquationsEXT dynamicStateAlphaToCoverageEnableEXT dynamicStateSampleMaskEXT dynamicStateExclusiveScissorEnableNV
💡 Result:
The extension VK_EXT_extended_dynamic_state3 is a Vulkan device extension that allows applications to specify various pipeline states dynamically, reducing the need for creating and binding a large number of monolithic pipelines [1][2]. The states you mentioned are part of the functionality added by this extension [2][3]. Below is a breakdown of these specific dynamic states: VK_DYNAMIC_STATE_COLOR_BLEND_ENABLE_EXT This allows the blendEnable state (defined per attachment in VkPipelineColorBlendAttachmentState) to be set dynamically using vkCmdSetColorBlendEnableEXT [4][5]. VK_DYNAMIC_STATE_COLOR_BLEND_EQUATION_EXT This enables dynamic control over blending equations (specifically srcColorBlendFactor, dstColorBlendFactor, colorBlendOp, srcAlphaBlendFactor, dstAlphaBlendFactor, and alphaBlendOp) for each color attachment using vkCmdSetColorBlendEquationEXT [4][6]. VK_DYNAMIC_STATE_ALPHA_TO_COVERAGE_ENABLE_EXT This allows the alphaToCoverageEnable state (originally from VkPipelineMultisampleStateCreateInfo) to be set dynamically using vkCmdSetAlphaToCoverageEnableEXT [7][8]. VK_DYNAMIC_STATE_SAMPLE_MASK_EXT This permits the sample mask to be set dynamically using vkCmdSetSampleMaskEXT instead of being baked into the pipeline via VkPipelineMultisampleStateCreateInfo::pSampleMask [7][9]. VK_DYNAMIC_STATE_EXCLUSIVE_SCISSOR_ENABLE_NV This state is associated with the VK_NV_scissor_exclusive extension and allows toggling the exclusive scissor test dynamically if supported by the implementation [2][7]. To use these, an application must enable the VK_EXT_extended_dynamic_state3 extension and ensure the corresponding feature bits in the VkPhysicalDeviceExtendedDynamicState3FeaturesEXT structure are supported and enabled (e.g., extendedDynamicState3ColorBlendEnable, extendedDynamicState3SampleMask, etc.) [3][10][6]. When these states are enabled, the corresponding values in the pipeline state objects are ignored at draw time, and the command buffer must have the state explicitly set [5][8][9].
Citations:
- 1: https://docs.vulkan.org/refpages/latest/refpages/source/VK_EXT_extended_dynamic_state3.html
- 2: https://docs.vulkan.org/features/latest/features/proposals/VK_EXT_extended_dynamic_state3.html
- 3: https://docs.vulkan.org/refpages/latest/refpages/source/VkPhysicalDeviceExtendedDynamicState3FeaturesEXT.html
- 4: https://docs.vulkan.org/refpages/latest/refpages/source/VkDynamicState.html
- 5: https://github.khronos.org/Vulkan-Site/refpages/latest/refpages/source/vkCmdSetColorBlendEnableEXT.html
- 6: https://github.com/KhronosGroup/Vulkan-Samples/blob/ac9edc79/samples/extensions/dynamic_blending/README.adoc
- 7: https://docs.vulkan.org/guide/latest/dynamic_state_map.html
- 8: https://docs.vulkan.org/refpages/latest/refpages/source/vkCmdSetAlphaToCoverageEnableEXT.html
- 9: https://docs.vulkan.org/refpages/latest/refpages/source/vkCmdSetSampleMaskEXT.html
- 10: https://github.com/KhronosGroup/Vulkan-Samples/blob/ac9edc79/samples/extensions/dynamic_blending/dynamic_blending.cpp
🌐 Web query:
Vulkan VK_EXT_extended_dynamic_state2 dynamicStateColorBlendEnable dynamicStateColorBlendFuncEXT dynamicStateViewportWithCountEXT dynamicStateScissorWithCountEXT dynamicStateDepthBiasEXT dynamicStateLinePolygonOffsetEXT dynamicStateDepthBounds dynamicStateStencilCompareMask dynamicStateStencilReference dynamicStateStencilWriteMask dynamicStateSampleLocationsEXT
💡 Result:
The Vulkan dynamic states you listed are part of various "extended dynamic state" extensions designed to reduce the number of Pipeline State Objects (PSOs) an application must compile and bind [1][2]. They are not all part of the same extension; they are distributed across VK_EXT_extended_dynamic_state, VK_EXT_extended_dynamic_state2, and VK_EXT_extended_dynamic_state3 [3][1][4]. Below is the classification of the requested states and their corresponding commands/extensions: 1. VK_EXT_extended_dynamic_state: - dynamicStateViewportWithCountEXT: Maps to VK_DYNAMIC_STATE_VIEWPORT_WITH_COUNT and the command vkCmdSetViewportWithCountEXT [5][4]. - dynamicStateScissorWithCountEXT: Maps to VK_DYNAMIC_STATE_SCISSOR_WITH_COUNT and the command vkCmdSetScissorWithCountEXT [6][4]. 2. VK_EXT_extended_dynamic_state2: - This extension primarily introduced states for primitive topology, patch control points, rasterizer discard, depth bias enable, and primitive restart enable [7][8]. - Note: The states you listed such as dynamicStateDepthBiasEXT (often associated with vkCmdSetDepthBias) and stencil-related states (e.g., dynamicStateStencilCompareMask, dynamicStateStencilReference, dynamicStateStencilWriteMask) are part of core Vulkan (1.0+) [9][4]. They are not introduced by VK_EXT_extended_dynamic_state2. 3. VK_EXT_extended_dynamic_state3: - dynamicStateColorBlendEnable: Maps to VK_DYNAMIC_STATE_COLOR_BLEND_ENABLE_EXT and the command vkCmdSetColorBlendEnableEXT [10][9][11]. - dynamicStateColorBlendFuncEXT: While there is no direct command named this, VK_EXT_extended_dynamic_state3 provides vkCmdSetColorBlendEquationEXT and vkCmdSetColorBlendAdvancedEXT to dynamically set blending equations and factors [3][12][11]. - dynamicStateSampleLocationsEXT: Maps to VK_DYNAMIC_STATE_SAMPLE_LOCATIONS_EXT [9][4]. 4. Other/Miscellaneous: - dynamicStateLinePolygonOffsetEXT: There is no dynamic state by this exact name. Line rasterization and polygon offset states are managed via various extensions (e.g., VK_EXT_line_rasterization for line stipple, or standard pipeline state for depth bias/polygon offset) [3][9][4]. - dynamicStateDepthBounds: This is part of core Vulkan (1.0+) [9]. - dynamicStateStencilCompareMask, dynamicStateStencilReference, dynamicStateStencilWriteMask: These are all part of core Vulkan (1.0+) [9]. In summary, these states allow for granular control of pipeline parameters at command buffer recording time [1][2]. Always check VkPhysicalDeviceFeatures to ensure your target hardware supports these specific extensions and their corresponding features [13][3][14].
Citations:
- 1: https://vulkan.lunarg.com/doc/view/1.4.313.0/windows/antora/guide/latest/dynamic_state.html
- 2: https://github.khronos.org/Vulkan-Site/guide/latest/dynamic_state.html
- 3: https://docs.vulkan.org/refpages/latest/refpages/source/VK_EXT_extended_dynamic_state3.html
- 4: https://registry.khronos.org/VulkanSC/specs/1.0-extensions/man/html/VkDynamicState.html
- 5: https://registry.khronos.org/VulkanSC/specs/1.0-extensions/man/html/vkCmdSetViewportWithCountEXT.html
- 6: https://docs.vulkan.org/refpages/latest/refpages/source/vkCmdSetScissorWithCount.html
- 7: https://docs.vulkan.org/samples/latest/samples/extensions/extended_dynamic_state2/README.html
- 8: https://github.com/KhronosGroup/Vulkan-Samples/tree/main/samples/extensions/extended_dynamic_state2
- 9: https://docs.vulkan.org/refpages/latest/refpages/source/VkDynamicState.html
- 10: https://github.khronos.org/Vulkan-Site/refpages/latest/refpages/source/vkCmdSetColorBlendEnableEXT.html
- 11: https://github.khronos.org/Vulkan-Site/features/latest/features/proposals/VK_EXT_extended_dynamic_state3.html
- 12: https://github.com/KhronosGroup/Vulkan-Samples/blob/ac9edc79/samples/extensions/dynamic_blending/README.adoc
- 13: https://docs.vulkan.org/refpages/latest/refpages/source/VkPhysicalDeviceExtendedDynamicState2FeaturesEXT.html
- 14: https://github.com/KhronosGroup/Vulkan-Samples/blob/ac9edc79/samples/extensions/dynamic_blending/dynamic_blending.cpp
🌐 Web query:
Vulkan Khronos Extension registry VK_EXT_extended_dynamic_state2 dynamicStatePrimitiveTopologyEXT dynamicStatePatchControlPointsEXT dynamicStateRasterizerDiscardEnableEXT dynamicStateDepthBiasEnableEXT dynamicStateLogicOpEXT dynamicStatePrimitiveRestartEnableEXT
💡 Result:
The Vulkan extension VK_EXT_extended_dynamic_state2 adds several dynamic states to the graphics pipeline, allowing applications to modify specific states without rebinding or re-creating pipeline objects [1][2]. Key aspects of this extension include: Promotion to Core: Several features originally introduced in VK_EXT_extended_dynamic_state2 were promoted to core functionality in Vulkan 1.3 [1][3]. The following states were promoted, with the EXT suffix removed in the core API [1][4]: - VK_DYNAMIC_STATE_DEPTH_BIAS_ENABLE (via vkCmdSetDepthBiasEnable) [1][2] - VK_DYNAMIC_STATE_PRIMITIVE_RESTART_ENABLE (via vkCmdSetPrimitiveRestartEnable) [1][2] - VK_DYNAMIC_STATE_RASTERIZER_DISCARD_ENABLE (via vkCmdSetRasterizerDiscardEnable) [1][2] Non-Promoted Features: The following states and their corresponding commands remain as extension-specific functionality and were not promoted to core Vulkan 1.3 [1][3]: - dynamicStateLogicOpEXT (via vkCmdSetLogicOpEXT; uses VK_DYNAMIC_STATE_LOGIC_OP_EXT) [1][5][6] - dynamicStatePatchControlPointsEXT (via vkCmdSetPatchControlPointsEXT; uses VK_DYNAMIC_STATE_PATCH_CONTROL_POINTS_EXT) [1][5][6] Feature Requirements: To use these features, the application must enable them via the VkPhysicalDeviceExtendedDynamicState2FeaturesEXT structure when creating a device [2][5]. This structure contains the following boolean fields [5][7]: - extendedDynamicState2: Enables the base dynamic states (Depth Bias, Primitive Restart, and Rasterizer Discard) [5]. - extendedDynamicState2LogicOp: Enables support for dynamic logic operations [5]. - extendedDynamicState2PatchControlPoints: Enables support for dynamic patch control points [5]. Note on Primitive Topology: While your query mentions dynamicStatePrimitiveTopologyEXT, this specific state is not part of VK_EXT_extended_dynamic_state2. Dynamic primitive topology is typically handled by other extensions, such as VK_EXT_primitive_topology_list_restart or features available in later Vulkan versions and other extensions like VK_EXT_shader_object [8].
Citations:
- 1: https://docs.vulkan.org/refpages/latest/refpages/source/VK_EXT_extended_dynamic_state2.html
- 2: https://docs.vulkan.org/samples/latest/samples/extensions/extended_dynamic_state2/README.html
- 3: https://registry.khronos.org/VulkanSC/specs/1.0-extensions/man/html/VK_EXT_extended_dynamic_state2.html
- 4: https://github.khronos.org/Vulkan-Site/refpages/latest/refpages/source/VK_EXT_extended_dynamic_state2.html
- 5: https://docs.vulkan.org/refpages/latest/refpages/source/VkPhysicalDeviceExtendedDynamicState2FeaturesEXT.html
- 6: https://docs.vulkan.org/refpages/latest/refpages/source/VkDynamicState.html
- 7: https://github.khronos.org/Vulkan-Site/refpages/latest/refpages/source/VkPhysicalDeviceExtendedDynamicState2FeaturesEXT.html
- 8: https://docs.vulkan.org/guide/latest/dynamic_state_map.html
🌐 Web query:
Vulkan dynamic state pipeline binding VK_DYNAMIC_STATE_CULL_MODE_EXT VK_DYNAMIC_STATE_FRONT_FACE_EXT VK_DYNAMIC_STATE_PRIMITIVE_TOPOLOGY_EXT VK_DYNAMIC_STATE_LINE_RASTERIZATION_MODE_EXT VK_DYNAMIC_STATE_COLOR_WRITE_MASK_EXT
💡 Result:
In Vulkan, these dynamic state constants allow applications to modify specific pipeline states during command buffer recording rather than baking them into monolithic pipeline objects [1][2]. This flexibility helps reduce the number of pipeline state objects (PSOs) that need to be created and compiled [1][3]. The specified dynamic states fall into different categories and extension support levels: 1. Core Vulkan 1.3 States: The following states were originally introduced via the VK_EXT_extended_dynamic_state extension and were promoted to core functionality in Vulkan 1.3 (with the _EXT suffix omitted in core) [4][3]: - VK_DYNAMIC_STATE_CULL_MODE: Allows dynamic setting of the cull mode via vkCmdSetCullMode [4][5]. - VK_DYNAMIC_STATE_FRONT_FACE: Allows dynamic setting of the front-facing triangle orientation via vkCmdSetFrontFace [4][6]. - VK_DYNAMIC_STATE_PRIMITIVE_TOPOLOGY: Allows dynamic setting of the input assembly primitive topology via vkCmdSetPrimitiveTopology [4][7]. 2. Extended Dynamic State 3 (VK_EXT_extended_dynamic_state3): These states are provided by the VK_EXT_extended_dynamic_state3 extension [8]: - VK_DYNAMIC_STATE_LINE_RASTERIZATION_MODE_EXT: Allows dynamic setting of the line rasterization mode via vkCmdSetLineRasterizationModeEXT (requires support for VK_EXT_line_rasterization or similar) [8][9]. - VK_DYNAMIC_STATE_COLOR_WRITE_MASK_EXT: Allows dynamic setting of the color write masks (this is part of the state managed by extended_dynamic_state3) [8]. Usage: To use these states, you must include the corresponding enumeration value in the pDynamicStates array within the VkPipelineDynamicStateCreateInfo structure when creating a graphics pipeline [1][4]. If a state is enabled as dynamic, the corresponding static value defined in the original pipeline creation structure (e.g., VkPipelineRasterizationStateCreateInfo) is ignored, and the state must be set using the associated command (e.g., vkCmdSetCullMode) before any drawing commands that rely on that state [4][5][6][9]. For states provided by extensions like VK_EXT_extended_dynamic_state3, hardware support is optional, and you should check the appropriate feature structures to ensure the device supports the specific dynamic states you intend to use [8][2].
Citations:
- 1: https://docs.vulkan.org/guide/latest/dynamic_state.html
- 2: https://docs.vulkan.org/features/latest/features/proposals/VK_EXT_extended_dynamic_state3.html
- 3: https://registry.khronos.org/VulkanSC/specs/1.0-extensions/man/html/VK_EXT_extended_dynamic_state.html
- 4: https://docs.vulkan.org/refpages/latest/refpages/source/VkDynamicState.html
- 5: https://registry.khronos.org/VulkanSC/specs/1.0-extensions/man/html/vkCmdSetCullModeEXT.html
- 6: https://registry.khronos.org/VulkanSC/specs/1.0-extensions/man/html/vkCmdSetFrontFace.html
- 7: https://docs.vulkan.org/guide/latest/primitive_topology.html
- 8: https://docs.vulkan.org/refpages/latest/refpages/source/VK_EXT_extended_dynamic_state3.html
- 9: https://docs.vulkan.org/refpages/latest/refpages/source/vkCmdSetLineRasterizationModeEXT.html
🌐 Web query:
Vulkan VK_EXT_extended_dynamic_state_depth_bias clamp color blend advanced polygon offset line width
💡 Result:
In the Vulkan API, the specific functionality you are asking about is spread across multiple "Extended Dynamic State" extensions rather than a single extension named "extended_dynamic_state_depth_bias" [1][2]. The primary extensions that address these dynamic state requirements are VK_EXT_extended_dynamic_state, VK_EXT_extended_dynamic_state2, and later additions in core Vulkan 1.3 [1][2][3]. Key breakdown of these states: Depth Bias and Polygon Offset: The dynamic state for depth bias (often associated with polygon offset) is managed via: - VK_DYNAMIC_STATE_DEPTH_BIAS [4]: Allows setting depth bias factors (constant, clamp, and slope) dynamically [4]. - VK_DYNAMIC_STATE_DEPTH_BIAS_ENABLE [4]: Introduced in VK_EXT_extended_dynamic_state2 (and later core Vulkan 1.3), this allows toggling whether depth bias is enabled [5][1][4]. Line Width: - VK_DYNAMIC_STATE_LINE_WIDTH is part of core Vulkan and has been available since version 1.0, not requiring an extension to be used dynamically [4]. Color Blend and Advanced States: - While basic blend state is standard, advanced dynamic state control for blending and other fixed-function states is typically handled by extensions like VK_EXT_extended_dynamic_state3 (which covers many more states not included in the earlier versions) or through specific hardware features [6]. - "Clamp color" is not a standard dynamic state enum; color clamping is generally handled via pipeline configuration or specific platform-dependent features [7][8]. In summary, if you are looking to move static pipeline states to dynamic command buffer calls to reduce pipeline creation overhead, you should use the combination of VK_EXT_extended_dynamic_state, VK_EXT_extended_dynamic_state2, and evaluate VK_EXT_extended_dynamic_state3 if you require further dynamic control [1][2][6]. Core Vulkan 1.3 incorporates most of the functionality from the first two extensions [1][2][3].
Citations:
- 1: https://docs.vulkan.org/sandbox/refpages/site/refpages/latest/refpages/source/VK_EXT_extended_dynamic_state2.html
- 2: https://docs.vulkan.org/refpages/latest/refpages/source/VK_EXT_extended_dynamic_state.html
- 3: https://docs.vulkan.org/refpages/latest/refpages/source/VK_VERSION_1_3.html
- 4: https://docs.vulkan.org/refpages/latest/refpages/source/VkDynamicState.html
- 5: https://registry.khronos.org/VulkanSC/specs/1.0-extensions/man/html/vkCmdSetDepthBiasEnableEXT.html
- 6: https://docs.vulkan.org/guide/latest/dynamic_state_map.html
- 7: https://docs.vulkan.org/spec/latest/chapters/features.html
- 8: https://chromium.googlesource.com/chromiumos/third_party/mesa/+/master/src/mesa/main/mtypes.h
Define every PSO dynamic-state axis and fallback.
The table currently uses only high-level labels, while the remaining static state can still multiply PSOs if Phase 6 does not lock it down. List cull mode, front face, primitive topology, polygon mode, color write mask, sample mask, alpha-to-coverage, depth bias, line width, and any rasterization or blending flags that may bake into a PSO. For each axis, specify the feature gate, fallback behavior when the dynamic feature is absent, and whether it contributes to the pipeline key.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/0011-rhi-neutral-resource-and-binding-model.md` around lines 891 -
898, Expand the PSO state-decision table to explicitly cover cull mode, front
face, primitive topology, polygon mode, color write mask, sample mask,
alpha-to-coverage, depth bias, line width, and all other rasterization or
blending flags that can remain static. For each axis, document its Vulkan
feature gate, fallback behavior when dynamic state is unavailable, and whether
it participates in the pipeline key; ensure Phase 6’s capability audit can
verify these decisions.
| **Sequencing note:** vertex-pulling is comparable in size to Phase 3's | ||
| bindless-texture rehearsal (a real conversion across every draw call, not a | ||
| config flag), so it should be an explicit Phase 6 sub-step completed before | ||
| the "one pass renders correctly" checkpoint — not deferred to Phase 7, where | ||
| retrofitting it after 35 passes have been ported against a baked vertex-input | ||
| PSO would cost far more than doing it once, up front, on the one pilot pass. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching vertex pull / OpenGLRendererAPI:"
git ls-files | rg '(^docs/adr/0011-rhi-neutral-resource-and-binding-model\.md$|OloEngine/src/Platform/OpenGL/OpenGLRendererAPI\.cpp$|vertex.?pull|drawing-indirect|vertex-input)' || true
echo
echo "ADR lines 880-920:"
sed -n '880,920p' docs/adr/0011-rhi-neutral-resource-and-binding-model.md || true
echo
echo "OpenGLRendererAPI lines 740-770:"
sed -n '740,770p' OloEngine/src/Platform/OpenGL/OpenGLRendererAPI.cpp || true
echo
echo "Search draw/vertex input symbols in OpenGLRendererAPI:"
rg -n "glMultiDrawElements|VkPipelineVertexInputStateCreateInfo|vertex.?input|VertexInput|DrawIndexedIndirect|MultiDraw|Stride|Binding|Index" OloEngine/src/Platform/OpenGL/OpenGLRendererAPI.cpp OloEngine/src -g '*.{cpp,h,hpp,c,hpp}' || trueRepository: drsnuggles8/OloEngineBase
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "ADR line count:"
wc -l docs/adr/0011-rhi-neutral-resource-and-binding-model.md
echo
echo "Focused ADR lines around vertex-pulling note:"
sed -n '860,915p' docs/adr/0011-rhi-neutral-resource-and-binding-model.md
echo
echo "Focused OpenGLRendererAPI lines 730-775:"
sed -n '730,775p' OloEngine/src/Platform/OpenGL/OpenGLRendererAPI.cpp
echo
echo "Vertex pulling mentions in ADR:"
rg -n -C 2 -i "vertex.?pull|vertex pulling|pulling" docs/adr/0011-rhi-neutral-resource-and-binding-model.md || true
echo
echo "Vertex format/layout/schema mentions:"
rg -n -C 2 -i "vertex.?format|vertex.?binding|vertex.?stride|vertex.?offset|VkVertexInput|PipelineVertex|vertex-input|vertex_input|draw.?vertex|vertex.?selection|vertex.?contracts|shader-visible" docs OloEngine -g '*.{md,cpp,h,hpp,hlsl,glsl,hlslc}' | head -n 240 || trueRepository: drsnuggles8/OloEngineBase
Length of output: 34520
Define the vertex-pulling contract before the Phase 6 checkpoint.
Vertex pulling currently describes the VkPipelineVertexInputStateCreateInfo behavior to remove, but no shader-visible layout schema exists for formats, offsets, strides, index type, index base, or per-draw selection. Add this contract and pilot shader contract before making vertex pulling a Phase 6 prerequisite.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/0011-rhi-neutral-resource-and-binding-model.md` around lines 900 -
905, Add a shader-visible vertex-pulling contract before the Phase 6 checkpoint,
defining formats, offsets, strides, index type, index base, and per-draw
selection. Include the pilot shader contract and update the vertex-pulling
sequencing text to make this specification an explicit prerequisite before
implementation.
| **Decision: a split barrier is a plain GPU-memory location plus two | ||
| operations — `Signal(pointer, value, op)` and `Wait(pointer, value, | ||
| compareOp)` — with `op`/`compareOp` covering at minimum `{Set/Equal, | ||
| AtomicMax/GreaterEqual}`.** The max/greater-equal pair is what generalizes | ||
| this from a one-shot flag into a monotonically increasing counter, i.e. a | ||
| timeline. This is deliberately not new Vulkan machinery: it is the existing | ||
| **timeline semaphore** primitive (core since Vulkan 1.2, well inside the ADR | ||
| 0010 1.4 floor), promoted to the render graph's one mechanism for any | ||
| cross-pass or cross-frame dependency, rather than inventing a second, | ||
| Vulkan-events-shaped abstraction alongside it the way DX12 keeps both split | ||
| barriers and fences as separate concepts. | ||
|
|
||
| **How this plugs into the existing model.** `RenderGraphBarrierPlanner` | ||
| (§1.5) keeps computing ordinary same-submission dependencies unchanged — nothing | ||
| here replaces that. A dependency that spans further — a compute culling pass | ||
| whose tail latency can be hidden behind independent work later in the same | ||
| frame, or a cross-frame dependency the render graph currently expresses as an | ||
| ad hoc CPU/GPU fence — becomes an `RHI::GpuFence`: one GPU pointer, allocated | ||
| from the same per-frame lifetime discipline §4's temp allocator already | ||
| established, with `Signal` attached to the producing render-graph node and | ||
| `Wait` attached to the consuming one (or to nothing, if the wait is a CPU-side | ||
| frame-pacing check instead). | ||
|
|
||
| **One primitive, both sides of the API.** The same `Signal`/`Wait` pair | ||
| replaces ad hoc CPU-side fence chains too — a single persistently-mapped | ||
| counter, incremented by `Signal`, observed via a CPU `WaitGreaterEqual` — so | ||
| the render graph's existing CPU/GPU frame-pacing mechanism and its future | ||
| GPU-side split-barrier mechanism are the same code, not two. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find ADR and related files"
git ls-files | rg 'docs/adr/0011|rhi|GpuFence|RenderGraphBarrierPlanner|gpu fence|split barrier' || true
echo
echo "Relevant sections in ADR 0011"
sed -n '880,970p' docs/adr/0011-rhi-neutral-resource-and-binding-model.md
echo
echo "Search for RHI GpuFence / Signal / Wait definitions or mentions"
rg -n "RHI::GpuFence|GpuFence|Signal\\(|Wait\\(|WaitGreaterEqual|split barrier|timeline semaphore|DeviceMemory|mapped|counter" docs/adr -S || trueRepository: drsnuggles8/OloEngineBase
Length of output: 11929
🌐 Web query:
Vulkan VkSemaphore timeline semaphore opaque payload vkWaitSemaphores host wait memory mapped semaphore payload
💡 Result:
In Vulkan, a timeline semaphore is a synchronization primitive that utilizes a monotonically increasing 64-bit unsigned integer as its payload [1][2][3]. Unlike binary semaphores, which have only two states (signaled/unsignaled), timeline semaphores allow for more complex synchronization by tracking progress against specific integer values [2][4][3]. Key aspects regarding the host and payload operations include: Host Wait and Signal: The host can block until a timeline semaphore reaches a specified value using vkWaitSemaphores [1][5][6]. Conversely, the host can advance the timeline semaphore value using vkSignalSemaphore [7][8][3]. Host Query: Applications can perform non-blocking queries of the current payload value of a timeline semaphore at any time using vkGetSemaphoreCounterValue [1][2][8]. Payload and Memory Mapping: The term "payload" refers to the internal data representing the semaphore's current state [3]. While external semaphore mechanisms allow for importing and exporting these payloads (e.g., between different APIs or processes), Vulkan does not provide a mechanism to directly "memory map" a semaphore payload for host access [3]. Instead, host-side interaction is explicitly handled through the formal API commands (vkWaitSemaphores, vkSignalSemaphore, and vkGetSemaphoreCounterValue), which abstract the underlying implementation [1][2][3]. This design replaces many requirements for VkFence objects, as the host can wait for specific points in a GPU's timeline rather than just waiting for an entire command buffer to complete [2][4].
Citations:
- 1: https://docs.vulkan.org/refpages/latest/refpages/source/VK_KHR_timeline_semaphore.html
- 2: https://docs.vulkan.org/tutorial/latest/Advanced_Vulkan_Compute/08_Asynchronous_Compute/03_timeline_semaphores.html
- 3: https://registry.khronos.org/VulkanSC/specs/1.0-extensions/man/html/VkSemaphore.html
- 4: https://www.khronos.org/blog/vulkan-timeline-semaphores
- 5: https://docs.vulkan.org/refpages/latest/refpages/source/vkWaitSemaphores.html
- 6: https://github.khronos.org/Vulkan-Site/refpages/latest/refpages/source/vkWaitSemaphores.html
- 7: https://github.khronos.org/Vulkan-Site/refpages/latest/refpages/source/VK_KHR_timeline_semaphore.html
- 8: https://github.com/KhronosGroup/Vulkan-Docs/blob/master/appendices/VK_KHR_timeline_semaphore.txt
Separate timeline semaphores from mapped GPU-memory signaling.
Vulkan timeline semaphores are backend VkSemaphore objects with opaque counter values. Signal/wait operations reference the semaphore, and host waits use vkWaitSemaphores; the payload is not an application-mapped GPU memory location.
Choose one model:
- Map
RHI::GpuFenceto backend semaphore handles and submission-level timeline values. - Define a separate device-memory protocol with explicit atomic, cache, queue, and CPU-wait semantics.
Also define submission boundaries. Vulkan semaphore signal/wait operations occur around submission command buffers; they are not arbitrary commands between render-graph nodes inside one command buffer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/0011-rhi-neutral-resource-and-binding-model.md` around lines 927 -
954, Revise the ADR section describing RHI::GpuFence, Signal, and Wait to choose
a single synchronization model: either map them to Vulkan VkSemaphore timeline
handles and submission-level values, or define a separate mapped-device-memory
protocol with explicit atomic, cache, queue, and host-wait semantics. Remove
claims that timeline semaphores are plain GPU-memory locations, and specify that
GPU signal/wait operations are attached to submission boundaries rather than
arbitrary nodes within one command buffer.
1 participant
Summary
VK_KHR_shader_untyped_pointers, already pinned in ADR 0010's capability contract), replacing per-draw UBO/SSBO binding.VkPipeline: vertex-pulling via buffer-device-address (extends Phase 3's bindless-texture pattern to vertex data), dynamic depth/stencil/blend state viaVK_EXT_extended_dynamic_state{,2,3}.Signal/Waitpair, unified with Vulkan's existing timeline semaphores rather than a separate split-barrier/events abstraction.Context
Prompted by cross-referencing Sebastian Aaltonen's Reducing Graphics API Complexity talk against the existing #691 design. The headline finding was that ADR 0010/0011 already independently match the sharpest parts of that talk (heap-bindless exclusively, no image-layout tracking, hazard-flag barriers instead of a resource list) — these four sections close the remaining gaps, all scoped to Phase 6 (not started) and consumed pass-by-pass in Phase 7.
Issue #691's Phase 6/7 bullets and Scope section were already updated to reference these sections. The talk's more speculative closing idea (collapsing ~16 shader-stage entry points into one generic kernel type + intrinsics) is deliberately not phased into #691 — it's tracked separately as a watch/re-check issue in #760, since it needs hardware/microcode support that doesn't exist yet.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit