Skip to content

Conversation

@inner-daemons
Copy link
Collaborator

@inner-daemons inner-daemons commented Oct 19, 2025

Connections
Closes #8341
Required for #8376

Description
The CreateGraphicsPipelineState function works great for basic render pipelines, but for more advanced features such as mesh shaders (#8110) or multiview/view instancing (#8206), you must use a streaming descriptor. The streaming descriptor corresponds more or less to a vulkan pipeline descriptor with the state described in a pNext chain, allowing you to use all sorts of extensions.

This PR switches all pipeline creation to use this function. Mesh shaders already used it but there was no reason to use different code paths for mesh shader pipelines vs standard pipelines when 99% of the descriptor is the same.

One thing to note: if it is ever possible in some esoteric system to use the DX12 backend on a 32-bit system, this code might not function properly. It probably still would, but it might not. Not that this is a setup that any real person has of course.

Testing
Existing testing (pretty much all tests use render pipelines)

Squash or Rebase?

Squash

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We likely can't use stream descriptors unconditionally unless we can prove that support for it goes back far enough

Comment on lines 1623 to 1647
struct RenderPipelineStateStreamDesc {
root_signature: *mut Direct3D12::ID3D12RootSignature,
task_shader: Direct3D12::D3D12_SHADER_BYTECODE,
mesh_shader: Direct3D12::D3D12_SHADER_BYTECODE,
pixel_shader: Direct3D12::D3D12_SHADER_BYTECODE,
blend_state: Direct3D12::D3D12_BLEND_DESC,
sample_mask: u32,
rasterizer_state: Direct3D12::D3D12_RASTERIZER_DESC,
depth_stencil_state: Direct3D12::D3D12_DEPTH_STENCIL_DESC,
primitive_topology_type: Direct3D12::D3D12_PRIMITIVE_TOPOLOGY_TYPE,
rtv_formats: Direct3D12::D3D12_RT_FORMAT_ARRAY,
dsv_format: Dxgi::Common::DXGI_FORMAT,
sample_desc: Dxgi::Common::DXGI_SAMPLE_DESC,
node_mask: u32,
cached_pso: Direct3D12::D3D12_CACHED_PIPELINE_STATE,
flags: Direct3D12::D3D12_PIPELINE_STATE_FLAGS,

// Vertex pipeline specific
vertex_shader: Direct3D12::D3D12_SHADER_BYTECODE,
input_layout: Direct3D12::D3D12_INPUT_LAYOUT_DESC,
index_buffer_strip_cut_value: Direct3D12::D3D12_INDEX_BUFFER_STRIP_CUT_VALUE,
stream_output: Direct3D12::D3D12_STREAM_OUTPUT_DESC,

// Mesh pipeline specific
task_shader: Direct3D12::D3D12_SHADER_BYTECODE,
mesh_shader: Direct3D12::D3D12_SHADER_BYTECODE,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should definitely isolate this to its own file so that we can consider its unsafety isolation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm honestly kind of surprised that this kind of code actually got accepted without much fuss in the mesh shaders PR lmao

@cwfitzgerald
Copy link
Member

Alright, so it was the Creators Update (1703) that introduced it, we'd need to hear from mozilla if that's too new.

@inner-daemons
Copy link
Collaborator Author

I can probably switch this so that the pipeline descriptor gets converted into a graphics pipeline descriptor on platforms where this wouldn't work. That probably will be the best for compatibility regardless of what Mozilla wants.

@cwfitzgerald cwfitzgerald self-assigned this Oct 22, 2025
@cwfitzgerald
Copy link
Member

Discussed at today's wgpu matainers meeting. Resolution: lets use this conditionally. mapping from a single struct works fine.

Discussion
  • EG: RE: platform coverage: Please don’t make downstreams use this unconditionally. Firefox needs to support Windows 10, for now.
  • CF: Can we just do this unconditionally? No, because Firefox needs to support Windows 10.
  • EG: Is there a benefit (besides consolidated code) to using this init path unconditionally, even if somebody isn’t using the features blocked on its usage?
  • CF: I’m not concerned about code quality, or continuing to support Windows 10, or anything here, just wanted to drop code we don’t need if we can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Switch DX12 pipeline creation to always use a stream descriptor

2 participants