Skip to content

Commit

Permalink
Add push contant config to layout (bevyengine#7681)
Browse files Browse the repository at this point in the history
# Objective

Allow for creating pipelines that use push constants. To be able to use push constants. Fixes bevyengine#4825

As of right now, trying to call `RenderPass::set_push_constants` will trigger the following error:

```
thread 'main' panicked at 'wgpu error: Validation Error

Caused by:
    In a RenderPass
      note: encoder = `<CommandBuffer-(0, 59, Vulkan)>`
    In a set_push_constant command
    provided push constant is for stage(s) VERTEX | FRAGMENT | VERTEX_FRAGMENT, however the pipeline layout has no push constant range for the stage(s) VERTEX | FRAGMENT | VERTEX_FRAGMENT
```
## Solution

Add a field push_constant_ranges to` RenderPipelineDescriptor` and `ComputePipelineDescriptor`.

This PR supersedes bevyengine#4908 which now contains merge conflicts due to significant changes to `bevy_render`.

Meanwhile, this PR also made the `layout` field of `RenderPipelineDescriptor` and `ComputePipelineDescriptor` non-optional. If the user do not need to specify the bind group layouts, they can simply supply an empty vector here. No need for it to be optional.

---

## Changelog
- Add a field push_constant_ranges to RenderPipelineDescriptor and ComputePipelineDescriptor
- Made the `layout` field of RenderPipelineDescriptor and ComputePipelineDescriptor non-optional.


## Migration Guide

- Add push_constant_ranges: Vec::new() to every `RenderPipelineDescriptor` and `ComputePipelineDescriptor`
- Unwrap the optional values on the `layout` field of `RenderPipelineDescriptor` and `ComputePipelineDescriptor`. If the descriptor has no layout, supply an empty vector.


Co-authored-by: Zhixing Zhang <me@neoto.xin>
  • Loading branch information
2 people authored and myreprise1 committed Feb 18, 2023
1 parent d89849c commit 63d1086
Show file tree
Hide file tree
Showing 17 changed files with 87 additions and 55 deletions.
12 changes: 8 additions & 4 deletions crates/bevy_bloom/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ impl FromWorld for BloomPipelines {
let downsampling_prefilter_pipeline =
pipeline_cache.queue_render_pipeline(RenderPipelineDescriptor {
label: Some("bloom_downsampling_prefilter_pipeline".into()),
layout: Some(vec![downsampling_bind_group_layout.clone()]),
layout: vec![downsampling_bind_group_layout.clone()],
vertex: fullscreen_shader_vertex_state(),
fragment: Some(FragmentState {
shader: BLOOM_SHADER_HANDLE.typed::<Shader>(),
Expand All @@ -449,12 +449,13 @@ impl FromWorld for BloomPipelines {
primitive: PrimitiveState::default(),
depth_stencil: None,
multisample: MultisampleState::default(),
push_constant_ranges: Vec::new(),
});

let downsampling_pipeline =
pipeline_cache.queue_render_pipeline(RenderPipelineDescriptor {
label: Some("bloom_downsampling_pipeline".into()),
layout: Some(vec![downsampling_bind_group_layout.clone()]),
layout: vec![downsampling_bind_group_layout.clone()],
vertex: fullscreen_shader_vertex_state(),
fragment: Some(FragmentState {
shader: BLOOM_SHADER_HANDLE.typed::<Shader>(),
Expand All @@ -469,11 +470,12 @@ impl FromWorld for BloomPipelines {
primitive: PrimitiveState::default(),
depth_stencil: None,
multisample: MultisampleState::default(),
push_constant_ranges: Vec::new(),
});

let upsampling_pipeline = pipeline_cache.queue_render_pipeline(RenderPipelineDescriptor {
label: Some("bloom_upsampling_pipeline".into()),
layout: Some(vec![upsampling_bind_group_layout.clone()]),
layout: vec![upsampling_bind_group_layout.clone()],
vertex: fullscreen_shader_vertex_state(),
fragment: Some(FragmentState {
shader: BLOOM_SHADER_HANDLE.typed::<Shader>(),
Expand All @@ -488,12 +490,13 @@ impl FromWorld for BloomPipelines {
primitive: PrimitiveState::default(),
depth_stencil: None,
multisample: MultisampleState::default(),
push_constant_ranges: Vec::new(),
});

let upsampling_final_pipeline =
pipeline_cache.queue_render_pipeline(RenderPipelineDescriptor {
label: Some("bloom_upsampling_final_pipeline".into()),
layout: Some(vec![downsampling_bind_group_layout.clone()]),
layout: vec![downsampling_bind_group_layout.clone()],
vertex: fullscreen_shader_vertex_state(),
fragment: Some(FragmentState {
shader: BLOOM_SHADER_HANDLE.typed::<Shader>(),
Expand All @@ -515,6 +518,7 @@ impl FromWorld for BloomPipelines {
primitive: PrimitiveState::default(),
depth_stencil: None,
multisample: MultisampleState::default(),
push_constant_ranges: Vec::new(),
});

BloomPipelines {
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_fxaa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ impl SpecializedRenderPipeline for FxaaPipeline {
fn specialize(&self, key: Self::Key) -> RenderPipelineDescriptor {
RenderPipelineDescriptor {
label: Some("fxaa".into()),
layout: Some(vec![self.texture_bind_group.clone()]),
layout: vec![self.texture_bind_group.clone()],
vertex: fullscreen_shader_vertex_state(),
fragment: Some(FragmentState {
shader: FXAA_SHADER_HANDLE.typed(),
Expand All @@ -218,6 +218,7 @@ impl SpecializedRenderPipeline for FxaaPipeline {
primitive: PrimitiveState::default(),
depth_stencil: None,
multisample: MultisampleState::default(),
push_constant_ranges: Vec::new(),
}
}
}
Expand Down
5 changes: 1 addition & 4 deletions crates/bevy_pbr/src/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,7 @@ where
descriptor.fragment.as_mut().unwrap().shader = fragment_shader.clone();
}

// MeshPipeline::specialize's current implementation guarantees that the returned
// specialized descriptor has a populated layout
let descriptor_layout = descriptor.layout.as_mut().unwrap();
descriptor_layout.insert(1, self.material_layout.clone());
descriptor.layout.insert(1, self.material_layout.clone());

M::specialize(self, &mut descriptor, layout, key)?;
Ok(descriptor)
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_pbr/src/prepass/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ where
buffers: vec![vertex_buffer_layout],
},
fragment,
layout: Some(bind_group_layout),
layout: bind_group_layout,
primitive: PrimitiveState {
topology: key.mesh_key.primitive_topology(),
strip_index_format: None,
Expand Down Expand Up @@ -314,6 +314,7 @@ where
mask: !0,
alpha_to_coverage_enabled: false,
},
push_constant_ranges: Vec::new(),
label: Some("prepass_pipeline".into()),
};

Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_pbr/src/render/light.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ impl SpecializedMeshPipeline for ShadowPipeline {
buffers: vec![vertex_buffer_layout],
},
fragment: None,
layout: Some(bind_group_layout),
layout: bind_group_layout,
primitive: PrimitiveState {
topology: key.primitive_topology(),
strip_index_format: None,
Expand Down Expand Up @@ -400,6 +400,7 @@ impl SpecializedMeshPipeline for ShadowPipeline {
}),
multisample: MultisampleState::default(),
label: Some("shadow_pipeline".into()),
push_constant_ranges: Vec::new(),
})
}
}
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_pbr/src/render/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,8 @@ impl SpecializedMeshPipeline for MeshPipeline {
write_mask: ColorWrites::ALL,
})],
}),
layout: Some(bind_group_layout),
layout: bind_group_layout,
push_constant_ranges: Vec::new(),
primitive: PrimitiveState {
front_face: FrontFace::Ccw,
cull_mode: Some(Face::Back),
Expand Down
16 changes: 8 additions & 8 deletions crates/bevy_render/src/render_resource/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ pub use wgpu::{
FrontFace, ImageCopyBuffer, ImageCopyBufferBase, ImageCopyTexture, ImageCopyTextureBase,
ImageDataLayout, ImageSubresourceRange, IndexFormat, Limits as WgpuLimits, LoadOp, MapMode,
MultisampleState, Operations, Origin3d, PipelineLayout, PipelineLayoutDescriptor, PolygonMode,
PrimitiveState, PrimitiveTopology, RenderPassColorAttachment, RenderPassDepthStencilAttachment,
RenderPassDescriptor, RenderPipelineDescriptor as RawRenderPipelineDescriptor,
SamplerBindingType, SamplerDescriptor, ShaderModule, ShaderModuleDescriptor, ShaderSource,
ShaderStages, StencilFaceState, StencilOperation, StencilState, StorageTextureAccess,
TextureAspect, TextureDescriptor, TextureDimension, TextureFormat, TextureSampleType,
TextureUsages, TextureViewDescriptor, TextureViewDimension, VertexAttribute,
VertexBufferLayout as RawVertexBufferLayout, VertexFormat, VertexState as RawVertexState,
VertexStepMode,
PrimitiveState, PrimitiveTopology, PushConstantRange, RenderPassColorAttachment,
RenderPassDepthStencilAttachment, RenderPassDescriptor,
RenderPipelineDescriptor as RawRenderPipelineDescriptor, SamplerBindingType, SamplerDescriptor,
ShaderModule, ShaderModuleDescriptor, ShaderSource, ShaderStages, StencilFaceState,
StencilOperation, StencilState, StorageTextureAccess, TextureAspect, TextureDescriptor,
TextureDimension, TextureFormat, TextureSampleType, TextureUsages, TextureViewDescriptor,
TextureViewDimension, VertexAttribute, VertexBufferLayout as RawVertexBufferLayout,
VertexFormat, VertexState as RawVertexState, VertexStepMode,
};

pub mod encase {
Expand Down
10 changes: 7 additions & 3 deletions crates/bevy_render/src/render_resource/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use bevy_asset::Handle;
use std::{borrow::Cow, ops::Deref};
use wgpu::{
BufferAddress, ColorTargetState, DepthStencilState, MultisampleState, PrimitiveState,
VertexAttribute, VertexFormat, VertexStepMode,
PushConstantRange, VertexAttribute, VertexFormat, VertexStepMode,
};

define_atomic_id!(RenderPipelineId);
Expand Down Expand Up @@ -93,7 +93,10 @@ pub struct RenderPipelineDescriptor {
/// Debug label of the pipeline. This will show up in graphics debuggers for easy identification.
pub label: Option<Cow<'static, str>>,
/// The layout of bind groups for this pipeline.
pub layout: Option<Vec<BindGroupLayout>>,
pub layout: Vec<BindGroupLayout>,
/// The push constant ranges for this pipeline.
/// Supply an empty vector if the pipeline doesn't use push constants.
pub push_constant_ranges: Vec<PushConstantRange>,
/// The compiled vertex stage, its entry point, and the input buffers layout.
pub vertex: VertexState,
/// The properties of the pipeline at the primitive assembly and rasterization level.
Expand Down Expand Up @@ -174,7 +177,8 @@ pub struct FragmentState {
#[derive(Clone, Debug)]
pub struct ComputePipelineDescriptor {
pub label: Option<Cow<'static, str>>,
pub layout: Option<Vec<BindGroupLayout>>,
pub layout: Vec<BindGroupLayout>,
pub push_constant_ranges: Vec<PushConstantRange>,
/// The compiled shader module for this stage.
pub shader: Handle<Shader>,
pub shader_defs: Vec<ShaderDefVal>,
Expand Down
57 changes: 36 additions & 21 deletions crates/bevy_render/src/render_resource/pipeline_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ use bevy_utils::{
use parking_lot::Mutex;
use std::{hash::Hash, iter::FusedIterator, mem, ops::Deref};
use thiserror::Error;
use wgpu::{PipelineLayoutDescriptor, VertexBufferLayout as RawVertexBufferLayout};
use wgpu::{
PipelineLayoutDescriptor, PushConstantRange, VertexBufferLayout as RawVertexBufferLayout,
};

use crate::render_resource::resource_macros::*;

Expand Down Expand Up @@ -298,30 +300,35 @@ impl ShaderCache {
}
}

type LayoutCacheKey = (Vec<BindGroupLayoutId>, Vec<PushConstantRange>);
#[derive(Default)]
struct LayoutCache {
layouts: HashMap<Vec<BindGroupLayoutId>, ErasedPipelineLayout>,
layouts: HashMap<LayoutCacheKey, ErasedPipelineLayout>,
}

impl LayoutCache {
fn get(
&mut self,
render_device: &RenderDevice,
bind_group_layouts: &[BindGroupLayout],
push_constant_ranges: Vec<PushConstantRange>,
) -> &wgpu::PipelineLayout {
let key = bind_group_layouts.iter().map(|l| l.id()).collect();
self.layouts.entry(key).or_insert_with(|| {
let bind_group_layouts = bind_group_layouts
.iter()
.map(|l| l.value())
.collect::<Vec<_>>();
ErasedPipelineLayout::new(render_device.create_pipeline_layout(
&PipelineLayoutDescriptor {
bind_group_layouts: &bind_group_layouts,
..default()
},
))
})
let bind_group_ids = bind_group_layouts.iter().map(|l| l.id()).collect();
self.layouts
.entry((bind_group_ids, push_constant_ranges))
.or_insert_with_key(|(_, push_constant_ranges)| {
let bind_group_layouts = bind_group_layouts
.iter()
.map(|l| l.value())
.collect::<Vec<_>>();
ErasedPipelineLayout::new(render_device.create_pipeline_layout(
&PipelineLayoutDescriptor {
bind_group_layouts: &bind_group_layouts,
push_constant_ranges,
..default()
},
))
})
}
}

Expand Down Expand Up @@ -561,10 +568,14 @@ impl PipelineCache {
})
.collect::<Vec<_>>();

let layout = if let Some(layout) = &descriptor.layout {
Some(self.layout_cache.get(&self.device, layout))
} else {
let layout = if descriptor.layout.is_empty() && descriptor.push_constant_ranges.is_empty() {
None
} else {
Some(self.layout_cache.get(
&self.device,
&descriptor.layout,
descriptor.push_constant_ranges.to_vec(),
))
};

let descriptor = RawRenderPipelineDescriptor {
Expand Down Expand Up @@ -610,10 +621,14 @@ impl PipelineCache {
}
};

let layout = if let Some(layout) = &descriptor.layout {
Some(self.layout_cache.get(&self.device, layout))
} else {
let layout = if descriptor.layout.is_empty() && descriptor.push_constant_ranges.is_empty() {
None
} else {
Some(self.layout_cache.get(
&self.device,
&descriptor.layout,
descriptor.push_constant_ranges.to_vec(),
))
};

let descriptor = RawComputePipelineDescriptor {
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_sprite/src/mesh2d/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,11 @@ where
if let Some(fragment_shader) = &self.fragment_shader {
descriptor.fragment.as_mut().unwrap().shader = fragment_shader.clone();
}
descriptor.layout = Some(vec![
descriptor.layout = vec![
self.mesh2d_pipeline.view_layout.clone(),
self.material2d_layout.clone(),
self.mesh2d_pipeline.mesh_layout.clone(),
]);
];

M::specialize(&mut descriptor, layout, key)?;
Ok(descriptor)
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_sprite/src/mesh2d/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,8 @@ impl SpecializedMeshPipeline for Mesh2dPipeline {
write_mask: ColorWrites::ALL,
})],
}),
layout: Some(vec![self.view_layout.clone(), self.mesh_layout.clone()]),
layout: vec![self.view_layout.clone(), self.mesh_layout.clone()],
push_constant_ranges: Vec::new(),
primitive: PrimitiveState {
front_face: FrontFace::Ccw,
cull_mode: None,
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_sprite/src/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ impl SpecializedRenderPipeline for SpritePipeline {
write_mask: ColorWrites::ALL,
})],
}),
layout: Some(vec![self.view_layout.clone(), self.material_layout.clone()]),
layout: vec![self.view_layout.clone(), self.material_layout.clone()],
primitive: PrimitiveState {
front_face: FrontFace::Ccw,
cull_mode: None,
Expand All @@ -264,6 +264,7 @@ impl SpecializedRenderPipeline for SpritePipeline {
alpha_to_coverage_enabled: false,
},
label: Some("sprite_pipeline".into()),
push_constant_ranges: Vec::new(),
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_tonemapping/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ impl SpecializedRenderPipeline for TonemappingPipeline {
}
RenderPipelineDescriptor {
label: Some("tonemapping pipeline".into()),
layout: Some(vec![self.texture_bind_group.clone()]),
layout: vec![self.texture_bind_group.clone()],
vertex: fullscreen_shader_vertex_state(),
fragment: Some(FragmentState {
shader: TONEMAPPING_SHADER_HANDLE.typed(),
Expand All @@ -88,6 +88,7 @@ impl SpecializedRenderPipeline for TonemappingPipeline {
primitive: PrimitiveState::default(),
depth_stencil: None,
multisample: MultisampleState::default(),
push_constant_ranges: Vec::new(),
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_ui/src/render/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ impl SpecializedRenderPipeline for UiPipeline {
write_mask: ColorWrites::ALL,
})],
}),
layout: Some(vec![self.view_layout.clone(), self.image_layout.clone()]),
layout: vec![self.view_layout.clone(), self.image_layout.clone()],
push_constant_ranges: Vec::new(),
primitive: PrimitiveState {
front_face: FrontFace::Ccw,
cull_mode: None,
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_upscaling/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ impl SpecializedRenderPipeline for UpscalingPipeline {
fn specialize(&self, key: Self::Key) -> RenderPipelineDescriptor {
RenderPipelineDescriptor {
label: Some("upscaling pipeline".into()),
layout: Some(vec![self.texture_bind_group.clone()]),
layout: vec![self.texture_bind_group.clone()],
vertex: fullscreen_shader_vertex_state(),
fragment: Some(FragmentState {
shader: UPSCALING_SHADER_HANDLE.typed(),
Expand All @@ -104,6 +104,7 @@ impl SpecializedRenderPipeline for UpscalingPipeline {
primitive: PrimitiveState::default(),
depth_stencil: None,
multisample: MultisampleState::default(),
push_constant_ranges: Vec::new(),
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions examples/2d/mesh2d_manual.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,13 @@ impl SpecializedRenderPipeline for ColoredMesh2dPipeline {
})],
}),
// Use the two standard uniforms for 2d meshes
layout: Some(vec![
layout: vec![
// Bind group 0 is the view uniform
self.mesh2d_pipeline.view_layout.clone(),
// Bind group 1 is the mesh uniform
self.mesh2d_pipeline.mesh_layout.clone(),
]),
],
push_constant_ranges: Vec::new(),
primitive: PrimitiveState {
front_face: FrontFace::Ccw,
cull_mode: Some(Face::Back),
Expand Down
Loading

0 comments on commit 63d1086

Please sign in to comment.