Abstract Pipeline creation (including pipeline layouts and root signatures)

[manon-traverse]

Add an API-agnostic way of creating compute and graphics pipelines. This includes the creation of pipeline layouts and root signatures. In Metal, there is no real equivalent due to the use of a top-level argument buffer, so the BindingsDesc variable is ignored.

[MarijnS95]

Metal does store this info in reflection, and it does even have a typed builder. But since Metal IRConverter converts this into an arbitrary "buffer of bytes" (top-level argument buffer) that we can just bind, no type info is needed 🎉

[MarijnS95]

Since this PR mostly seems to move (lots of) code around, I asked Claude to boil down all the semantic changes in the diff as these are omitted from the PR description. Let me know if the below is correct and matches the intent, or if it found nonsense, or if certain things weren't intended to be changed like that. It'll help with the review of close to 1200 lines :)

Semantic changes in moved code

Beyond the refactoring (splitting functions, RAII, new abstract types), these are the behavioral differences I spotted between the old and new code:

DX/Device.cpp

1. Depth/stencil disabled by default in createPipelineVsPs: old had DepthEnable=true, DepthWriteMask=ALL, DepthFunc=LESS. New has DepthEnable=false, StencilEnable=false. The caller now passes DSFormat to control this, but the PSO itself no longer enables depth testing.

2. Input layout supports instancing: new code checks InstanceStepRate to pick PER_INSTANCE_DATA vs PER_VERTEX_DATA. Old hardcoded PER_VERTEX_DATA.

3. Multiple render targets: old hardcoded NumRenderTargets=1. New loops over RTFormats.

MTL/MTLDevice.cpp

4. Vertex attribute mapping rewritten: old reflected on the Metal Function's vertexAttributes() and matched by lowercased name + "0" suffix. New ignores shader reflection and assigns attributes sequentially by index (0, 1, 2, ...). Semantics change if shader attribute indices don't match sequential order.

5. Depth/stencil now conditional on DSFormat.has_value() and stencil only set for D32FloatS8Uint. Old unconditionally set both.

6. Multiple render targets supported via loop, vs old single-RT.

VK/Device.cpp

7. Pipeline cache removed: old created a VkPipelineCache, new passes VK_NULL_HANDLE.

8. Vulkan 1.3 dynamic rendering: old used renderPass = IS.RenderPass. New uses VkPipelineRenderingCreateInfo via pNext with renderPass = VK_NULL_HANDLE. Significant architecture change.

9. Per-shader specialization constants for graphics: old only handled spec constants for compute. New builds separate VkSpecializationInfo for VS and PS independently.

10. Blend attachment count now matches RTFormats.size() instead of hardcoded 1.

11. Better error cleanup in layout creation: new code destroys already-created set layouts if a later one fails. Old didn't.

[manon-traverse]

• Depth/stencil disabled by default in createPipelineVsPs: old had DepthEnable=true, DepthWriteMask=ALL, DepthFunc=LESS. New has DepthEnable=false, StencilEnable=false. The caller now passes DSFormat to control this, but the PSO itself no longer enables depth testing.

Looks like something went wrong here. It seems like it wasn't enabled in DX12 when I initially worked on this branch but it got enabled in the mean-time. Let me enable depth stencil testing.

[MarijnS95]

Claude:

VK dynamic rendering is the biggest risk. The pipeline is now created with renderPass = VK_NULL_HANDLE + VkPipelineRenderingCreateInfo, but the command encoding path still uses the old VkRenderPass-based vkCmdBeginRenderPass. Mixing dynamic-rendering pipeline creation with legacy render pass commands is technically invalid per the Vulkan spec — pipelines created with dynamic rendering must be used with vkCmdBeginRendering, not vkCmdBeginRenderPass. This could silently work on some drivers and fail on others.

[MarijnS95]

Running with golden images and validation layer enabled (-DGOLDENIMAGE_DIR=/path/to/golden-images -DOFFLOADTEST_ENABLE_VALIDATION=ON):

  Validation Error: [ VUID-vkCmdDraw-renderPass-02684 ] | MessageID = 0x50685725
  vkCmdDraw(): subpassCount is incompatible between VkRenderPass 0xf000000000f
  (from VkCommandBuffer 0x5625f3c3bf50) and VkRenderPass 0x0 (from VkPipeline
  0x140000000014), 1 != 0
  The Vulkan spec states: The current render pass must be compatible with the
  renderPass member of the VkGraphicsPipelineCreateInfo structure specified when
  creating the VkPipeline bound to VK_PIPELINE_BIND_POINT_GRAPHICS
  (https://docs.vulkan.org/spec/latest/chapters/drawing.html#VUID-vkCmdDraw-renderPass-02684)
  Objects: 3
      [0] VkCommandBuffer 0x5625f3c3bf50
      [1] VkRenderPass 0xf000000000f
      [2] VkPipeline 0x140000000014

  Validation Error: [ VUID-vkCmdDraw-renderPass-02684 ] | MessageID = 0x50685725
  vkCmdDraw(): dependencyCount is incompatible between VkRenderPass 0xf000000000f
  (from VkCommandBuffer 0x5625f3c3bf50) and VkRenderPass 0x0 (from VkPipeline
  0x140000000014), 2 != 0
  The Vulkan spec states: The current render pass must be compatible with the
  renderPass member of the VkGraphicsPipelineCreateInfo structure specified when
  creating the VkPipeline bound to VK_PIPELINE_BIND_POINT_GRAPHICS
  (https://docs.vulkan.org/spec/latest/chapters/drawing.html#VUID-vkCmdDraw-renderPass-02684)
  Objects: 3
      [0] VkCommandBuffer 0x5625f3c3bf50
      [1] VkRenderPass 0xf000000000f
      [2] VkPipeline 0x140000000014

[manon-traverse]

I have removed the dynamic rendering feature. It's now using classic RenderPass objects.

[MarijnS95]

Nice, looks like after re-adding render passes (switching to "Dynamic Rendering" is a bigger feat that is better suited to its own isolated PR) this seems to be ready to go!

[bogner]

A few minor comments. The biggest one will be the comment about the error handling strategy in various places. Generally LGTM though, shouldn't need another review round from me once those are addressed.

[manon-traverse]

CI failure is unrelated to this PR.