Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implicit layout behavior of create_render_pipeline is not documented. #6254

Closed
BGR360 opened this issue Sep 11, 2024 · 1 comment · Fixed by #6275
Closed

Implicit layout behavior of create_render_pipeline is not documented. #6254

BGR360 opened this issue Sep 11, 2024 · 1 comment · Fixed by #6275
Labels
area: documentation Documentation for crate items, public or private type: bug Something isn't working

Comments

@BGR360
Copy link
Contributor

BGR360 commented Sep 11, 2024

I was reading wgpu examples when I saw this code:

let pipeline = device.create_render_pipeline(&wgpu::RenderPipelineDescriptor {
label: Some("blit"),
layout: None,
vertex: wgpu::VertexState {
module: &shader,
entry_point: Some("vs_main"),
compilation_options: Default::default(),
buffers: &[],
},
fragment: Some(wgpu::FragmentState {
module: &shader,
entry_point: Some("fs_main"),
compilation_options: Default::default(),
targets: &[Some(TEXTURE_FORMAT.into())],
}),
primitive: wgpu::PrimitiveState {
topology: wgpu::PrimitiveTopology::TriangleList,
..Default::default()
},
depth_stencil: None,
multisample: wgpu::MultisampleState::default(),
multiview: None,
cache: None,
});
let bind_group_layout = pipeline.get_bind_group_layout(0);

This code supplies layout: None in RenderPipelineDescriptor and then uses pipeline..get_bind_group_layout(0) to get a BindGroupLayout.

This is new to me; all the examples of wgpu I had seen online up to this point manually specified BindGroupLayouts in the RenderPipelineDescriptor.

I didn't know what this code should do when reading it. Does None mean that the pipeline expects no bindings? Does None mean that wgpu will deduce the layout from the shader module(s)? I guessed that it must be the latter, since the shaders in this example do require bindings.

So I went to the docs and looked up RenderPipeline::get_bind_group_layout and RenderPipelineDescriptor::layout, but neither of them mention anything about this implicit layout behavior. In the source code though, I found evidence for "implicit layout" behavior.

My request: please document the implicit layout behavior of RenderPipelineDescriptor

Furthermore, it's not documented what happens if you call RenderPipeline::get_bind_group_layout with a bind group index that doesn't exist in the layout.

Request 2: Document behavior of RenderPipeline::get_bind_group_layout when index is out of bounds

Here's what I suggest based on my current understanding. I don't know if it's all correct.

pub struct RenderPipelineDescriptor<'a> {
    ...
    /// The layout of bind groups for this pipeline.
    ///
    /// If set, `wgpu` will validate that the layout matches what the shader modules expect,
    /// otherwise [`Device::create_render_pipeline`] will panic.
    ///
    /// If `None`, `wgpu` will deduce the pipeline layout from the shader modules. The deduced
    /// bind group layouts can then be read by calling [`RenderPipeline::get_bind_group_layout`].
    pub layout: Option<&'a PipelineLayout>,
impl RenderPipeline {
    /// Get an object representing the bind group layout at a given index.
    ///
    /// If [`RenderPipelineDescriptor::layout`] was set to `None` when creating this pipeline,
    /// this method will return the bind group layout that `wgpu` deduced for this index based
    /// on the shader modules. Otherwise, the returned layout will match the one at this index
    /// in the `RenderPipelineDescriptor`.
    ///
    /// # Panics
    /// If the pipeline has no bind group at the provided `index`, this method will panic.
    pub fn get_bind_group_layout(&self, index: u32) -> BindGroupLayout {
        ...
    }
}
@Wumpf Wumpf added type: bug Something isn't working area: documentation Documentation for crate items, public or private labels Sep 11, 2024
@Wumpf
Copy link
Member

Wumpf commented Sep 11, 2024

For reference, here's how WebGPU spec documents this: https://www.w3.org/TR/webgpu/#default-pipeline-layout
Updated documentation should likely borrow from there (and also link to it?)

BGR360 added a commit to BGR360/wgpu that referenced this issue Sep 14, 2024
@Wumpf Wumpf closed this as completed in 434f197 Sep 15, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in WebGPU for Firefox Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: documentation Documentation for crate items, public or private type: bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants