From f3ef23bda7eccaa5dd6844d8896d3b398e407dc5 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Fri, 14 Aug 2020 14:02:56 -0400 Subject: [PATCH 1/6] Working on correctly reflecting shader stage for bind groups. --- .../src/pipeline/pipeline_layout.rs | 13 ++++++++-- .../bevy_render/src/shader/shader_reflect.rs | 25 ++++++++++++------- .../renderer/wgpu_render_resource_context.rs | 1 + 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/crates/bevy_render/src/pipeline/pipeline_layout.rs b/crates/bevy_render/src/pipeline/pipeline_layout.rs index d372787fe991a..741c8924db991 100644 --- a/crates/bevy_render/src/pipeline/pipeline_layout.rs +++ b/crates/bevy_render/src/pipeline/pipeline_layout.rs @@ -16,6 +16,7 @@ impl PipelineLayout { } pub fn from_shader_layouts(shader_layouts: &mut [ShaderLayout]) -> Self { + dbg!(&shader_layouts); let mut bind_groups = HashMap::::new(); let mut vertex_buffer_descriptors = Vec::new(); for shader_layout in shader_layouts.iter_mut() { @@ -25,10 +26,15 @@ impl PipelineLayout { for shader_binding in shader_bind_group.bindings.iter() { if let Some(binding) = bind_group .bindings - .iter() + .iter_mut() .find(|binding| binding.index == shader_binding.index) { - if binding != shader_binding { + binding.shader_stage |= shader_binding.shader_stage; + // Not sure we need to panic anymore here.. + if binding.bind_type != shader_binding.bind_type + || binding.name != shader_binding.name + || binding.index != shader_binding.index + { panic!("Binding {} in BindGroup {} does not match across all shader types: {:?} {:?}", binding.index, bind_group.index, binding, shader_binding); } } else { @@ -42,6 +48,7 @@ impl PipelineLayout { } } } + dbg!(&bind_groups); for vertex_buffer_descriptor in shader_layouts[0].vertex_buffer_descriptors.iter() { vertex_buffer_descriptors.push(vertex_buffer_descriptor.clone()); @@ -56,6 +63,8 @@ impl PipelineLayout { // TODO: try removing this bind_groups_result.sort_by(|a, b| a.index.partial_cmp(&b.index).unwrap()); + dbg!(&bind_groups_result); + PipelineLayout { bind_groups: bind_groups_result, vertex_buffer_descriptors, diff --git a/crates/bevy_render/src/shader/shader_reflect.rs b/crates/bevy_render/src/shader/shader_reflect.rs index c71b4dcb92dcd..81febc1810d58 100644 --- a/crates/bevy_render/src/shader/shader_reflect.rs +++ b/crates/bevy_render/src/shader/shader_reflect.rs @@ -9,7 +9,7 @@ use bevy_core::AsBytes; use spirv_reflect::{ types::{ ReflectDescriptorBinding, ReflectDescriptorSet, ReflectDescriptorType, ReflectDimension, - ReflectInterfaceVariable, ReflectTypeDescription, ReflectTypeFlags, + ReflectInterfaceVariable, ReflectTypeDescription, ReflectTypeFlags, ReflectShaderStageFlags }, ShaderModule, }; @@ -30,9 +30,10 @@ impl ShaderLayout { match ShaderModule::load_u8_data(spirv_data.as_bytes()) { Ok(ref mut module) => { let entry_point_name = module.get_entry_point_name(); + let shader_stage = module.get_shader_stage(); let mut bind_groups = Vec::new(); for descriptor_set in module.enumerate_descriptor_sets(None).unwrap() { - let bind_group = reflect_bind_group(&descriptor_set); + let bind_group = reflect_bind_group(&descriptor_set, shader_stage); bind_groups.push(bind_group); } @@ -150,10 +151,10 @@ fn reflect_vertex_attribute_descriptor( } } -fn reflect_bind_group(descriptor_set: &ReflectDescriptorSet) -> BindGroupDescriptor { +fn reflect_bind_group(descriptor_set: &ReflectDescriptorSet, shader_stage: ReflectShaderStageFlags) -> BindGroupDescriptor { let mut bindings = Vec::new(); for descriptor_binding in descriptor_set.bindings.iter() { - let binding = reflect_binding(descriptor_binding); + let binding = reflect_binding(descriptor_binding, shader_stage); bindings.push(binding); } @@ -170,7 +171,7 @@ fn reflect_dimension(type_description: &ReflectTypeDescription) -> TextureViewDi } } -fn reflect_binding(binding: &ReflectDescriptorBinding) -> BindingDescriptor { +fn reflect_binding(binding: &ReflectDescriptorBinding, shader_stage: ReflectShaderStageFlags) -> BindingDescriptor { let type_description = binding.type_description.as_ref().unwrap(); let (name, bind_type) = match binding.descriptor_type { ReflectDescriptorType::UniformBuffer => ( @@ -200,12 +201,18 @@ fn reflect_binding(binding: &ReflectDescriptorBinding) -> BindingDescriptor { _ => panic!("unsupported bind type {:?}", binding.descriptor_type), }; + let shader_stage = match shader_stage { + ReflectShaderStageFlags::COMPUTE => BindingShaderStage::COMPUTE, + ReflectShaderStageFlags::VERTEX => BindingShaderStage::VERTEX, + ReflectShaderStageFlags::FRAGMENT => BindingShaderStage::FRAGMENT, + _ => panic!("Only one specified shader stage is support.") + }; + BindingDescriptor { index: binding.binding, bind_type, name: name.to_string(), - // TODO: We should be able to detect which shader program the binding is being used in.. - shader_stage: BindingShaderStage::VERTEX | BindingShaderStage::FRAGMENT, + shader_stage, } } @@ -416,7 +423,7 @@ mod tests { UniformProperty::Mat4 ])], }, - shader_stage: BindingShaderStage::VERTEX | BindingShaderStage::FRAGMENT, + shader_stage: BindingShaderStage::VERTEX, }] ), BindGroupDescriptor::new( @@ -429,7 +436,7 @@ mod tests { dimension: TextureViewDimension::D2, component_type: TextureComponentType::Float, }, - shader_stage: BindingShaderStage::VERTEX | BindingShaderStage::FRAGMENT, + shader_stage: BindingShaderStage::VERTEX, }] ), ] diff --git a/crates/bevy_wgpu/src/renderer/wgpu_render_resource_context.rs b/crates/bevy_wgpu/src/renderer/wgpu_render_resource_context.rs index 26ab64743ef92..35f9c99de5017 100644 --- a/crates/bevy_wgpu/src/renderer/wgpu_render_resource_context.rs +++ b/crates/bevy_wgpu/src/renderer/wgpu_render_resource_context.rs @@ -133,6 +133,7 @@ impl WgpuRenderResourceContext { bindings: bind_group_layout_binding.as_slice(), label: None, }; + dbg!(&wgpu_descriptor); let bind_group_layout = self.device.create_bind_group_layout(&wgpu_descriptor); bind_group_layouts.insert(descriptor.id, bind_group_layout); } From 76564a486ed41e65d7e396cd0654c19ccd52e787 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Fri, 14 Aug 2020 16:59:18 -0400 Subject: [PATCH 2/6] Removed old comment. --- crates/bevy_render/src/pipeline/pipeline_layout.rs | 1 - crates/bevy_render/src/shader/shader_reflect.rs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bevy_render/src/pipeline/pipeline_layout.rs b/crates/bevy_render/src/pipeline/pipeline_layout.rs index 741c8924db991..db78c20896e44 100644 --- a/crates/bevy_render/src/pipeline/pipeline_layout.rs +++ b/crates/bevy_render/src/pipeline/pipeline_layout.rs @@ -30,7 +30,6 @@ impl PipelineLayout { .find(|binding| binding.index == shader_binding.index) { binding.shader_stage |= shader_binding.shader_stage; - // Not sure we need to panic anymore here.. if binding.bind_type != shader_binding.bind_type || binding.name != shader_binding.name || binding.index != shader_binding.index diff --git a/crates/bevy_render/src/shader/shader_reflect.rs b/crates/bevy_render/src/shader/shader_reflect.rs index 81febc1810d58..c913e4601093a 100644 --- a/crates/bevy_render/src/shader/shader_reflect.rs +++ b/crates/bevy_render/src/shader/shader_reflect.rs @@ -205,7 +205,7 @@ fn reflect_binding(binding: &ReflectDescriptorBinding, shader_stage: ReflectShad ReflectShaderStageFlags::COMPUTE => BindingShaderStage::COMPUTE, ReflectShaderStageFlags::VERTEX => BindingShaderStage::VERTEX, ReflectShaderStageFlags::FRAGMENT => BindingShaderStage::FRAGMENT, - _ => panic!("Only one specified shader stage is support.") + _ => panic!("Only one specified shader stage is supported.") }; BindingDescriptor { From eac53c1ab7bab30dba7a75e8fbe056b3541b2541 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Fri, 14 Aug 2020 23:21:38 -0400 Subject: [PATCH 3/6] Fixed! :tada: --- crates/bevy_render/src/pipeline/pipeline_layout.rs | 7 ++----- .../bevy_wgpu/src/renderer/wgpu_render_resource_context.rs | 1 - 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/bevy_render/src/pipeline/pipeline_layout.rs b/crates/bevy_render/src/pipeline/pipeline_layout.rs index db78c20896e44..6a108471cddb0 100644 --- a/crates/bevy_render/src/pipeline/pipeline_layout.rs +++ b/crates/bevy_render/src/pipeline/pipeline_layout.rs @@ -16,7 +16,6 @@ impl PipelineLayout { } pub fn from_shader_layouts(shader_layouts: &mut [ShaderLayout]) -> Self { - dbg!(&shader_layouts); let mut bind_groups = HashMap::::new(); let mut vertex_buffer_descriptors = Vec::new(); for shader_layout in shader_layouts.iter_mut() { @@ -40,6 +39,7 @@ impl PipelineLayout { bind_group.bindings.push(shader_binding.clone()); } } + bind_group.update_id(); } None => { bind_groups.insert(shader_bind_group.index, shader_bind_group.clone()); @@ -47,8 +47,7 @@ impl PipelineLayout { } } } - dbg!(&bind_groups); - + for vertex_buffer_descriptor in shader_layouts[0].vertex_buffer_descriptors.iter() { vertex_buffer_descriptors.push(vertex_buffer_descriptor.clone()); } @@ -62,8 +61,6 @@ impl PipelineLayout { // TODO: try removing this bind_groups_result.sort_by(|a, b| a.index.partial_cmp(&b.index).unwrap()); - dbg!(&bind_groups_result); - PipelineLayout { bind_groups: bind_groups_result, vertex_buffer_descriptors, diff --git a/crates/bevy_wgpu/src/renderer/wgpu_render_resource_context.rs b/crates/bevy_wgpu/src/renderer/wgpu_render_resource_context.rs index 35f9c99de5017..26ab64743ef92 100644 --- a/crates/bevy_wgpu/src/renderer/wgpu_render_resource_context.rs +++ b/crates/bevy_wgpu/src/renderer/wgpu_render_resource_context.rs @@ -133,7 +133,6 @@ impl WgpuRenderResourceContext { bindings: bind_group_layout_binding.as_slice(), label: None, }; - dbg!(&wgpu_descriptor); let bind_group_layout = self.device.create_bind_group_layout(&wgpu_descriptor); bind_group_layouts.insert(descriptor.id, bind_group_layout); } From e11fa3e316c0815ac644f5a7d396ed06a41e7fb1 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Sat, 15 Aug 2020 15:03:57 -0400 Subject: [PATCH 4/6] Added hack to allow all of the examples to work. --- crates/bevy_render/src/shader/shader_reflect.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/bevy_render/src/shader/shader_reflect.rs b/crates/bevy_render/src/shader/shader_reflect.rs index c913e4601093a..a89bac60836fd 100644 --- a/crates/bevy_render/src/shader/shader_reflect.rs +++ b/crates/bevy_render/src/shader/shader_reflect.rs @@ -201,17 +201,23 @@ fn reflect_binding(binding: &ReflectDescriptorBinding, shader_stage: ReflectShad _ => panic!("unsupported bind type {:?}", binding.descriptor_type), }; - let shader_stage = match shader_stage { + let mut shader_stage = match shader_stage { ReflectShaderStageFlags::COMPUTE => BindingShaderStage::COMPUTE, ReflectShaderStageFlags::VERTEX => BindingShaderStage::VERTEX, ReflectShaderStageFlags::FRAGMENT => BindingShaderStage::FRAGMENT, _ => panic!("Only one specified shader stage is supported.") }; + let name = name.to_string(); + + if name == "Camera" { + shader_stage = BindingShaderStage::VERTEX | BindingShaderStage::FRAGMENT; + } + BindingDescriptor { index: binding.binding, bind_type, - name: name.to_string(), + name, shader_stage, } } From f1b0d04e53ba3683344fb372c36875d38fb0b9c5 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Wed, 19 Aug 2020 10:00:45 -0400 Subject: [PATCH 5/6] Fixed formatting. --- .../bevy_render/src/pipeline/pipeline_layout.rs | 2 +- crates/bevy_render/src/shader/shader_reflect.rs | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/crates/bevy_render/src/pipeline/pipeline_layout.rs b/crates/bevy_render/src/pipeline/pipeline_layout.rs index 6a108471cddb0..52600348ba750 100644 --- a/crates/bevy_render/src/pipeline/pipeline_layout.rs +++ b/crates/bevy_render/src/pipeline/pipeline_layout.rs @@ -47,7 +47,7 @@ impl PipelineLayout { } } } - + for vertex_buffer_descriptor in shader_layouts[0].vertex_buffer_descriptors.iter() { vertex_buffer_descriptors.push(vertex_buffer_descriptor.clone()); } diff --git a/crates/bevy_render/src/shader/shader_reflect.rs b/crates/bevy_render/src/shader/shader_reflect.rs index 9221f36e1fb0b..d41cf64a721b7 100644 --- a/crates/bevy_render/src/shader/shader_reflect.rs +++ b/crates/bevy_render/src/shader/shader_reflect.rs @@ -9,7 +9,8 @@ use bevy_core::AsBytes; use spirv_reflect::{ types::{ ReflectDescriptorBinding, ReflectDescriptorSet, ReflectDescriptorType, ReflectDimension, - ReflectInterfaceVariable, ReflectTypeDescription, ReflectTypeFlags, ReflectShaderStageFlags + ReflectInterfaceVariable, ReflectShaderStageFlags, ReflectTypeDescription, + ReflectTypeFlags, }, ShaderModule, }; @@ -151,7 +152,10 @@ fn reflect_vertex_attribute_descriptor( } } -fn reflect_bind_group(descriptor_set: &ReflectDescriptorSet, shader_stage: ReflectShaderStageFlags) -> BindGroupDescriptor { +fn reflect_bind_group( + descriptor_set: &ReflectDescriptorSet, + shader_stage: ReflectShaderStageFlags, +) -> BindGroupDescriptor { let mut bindings = Vec::new(); for descriptor_binding in descriptor_set.bindings.iter() { let binding = reflect_binding(descriptor_binding, shader_stage); @@ -171,7 +175,10 @@ fn reflect_dimension(type_description: &ReflectTypeDescription) -> TextureViewDi } } -fn reflect_binding(binding: &ReflectDescriptorBinding, shader_stage: ReflectShaderStageFlags) -> BindingDescriptor { +fn reflect_binding( + binding: &ReflectDescriptorBinding, + shader_stage: ReflectShaderStageFlags, +) -> BindingDescriptor { let type_description = binding.type_description.as_ref().unwrap(); let (name, bind_type) = match binding.descriptor_type { ReflectDescriptorType::UniformBuffer => ( @@ -205,7 +212,7 @@ fn reflect_binding(binding: &ReflectDescriptorBinding, shader_stage: ReflectShad ReflectShaderStageFlags::COMPUTE => BindingShaderStage::COMPUTE, ReflectShaderStageFlags::VERTEX => BindingShaderStage::VERTEX, ReflectShaderStageFlags::FRAGMENT => BindingShaderStage::FRAGMENT, - _ => panic!("Only one specified shader stage is supported.") + _ => panic!("Only one specified shader stage is supported."), }; let name = name.to_string(); From d4d68f6670c79f2ba1924fc442ed24894edde617 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Wed, 19 Aug 2020 10:16:35 -0400 Subject: [PATCH 6/6] Fixed tests. --- crates/bevy_render/src/shader/shader_reflect.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_render/src/shader/shader_reflect.rs b/crates/bevy_render/src/shader/shader_reflect.rs index d41cf64a721b7..6ed6b7c31a9d9 100644 --- a/crates/bevy_render/src/shader/shader_reflect.rs +++ b/crates/bevy_render/src/shader/shader_reflect.rs @@ -436,7 +436,7 @@ mod tests { UniformProperty::Mat4 ])], }, - shader_stage: BindingShaderStage::VERTEX, + shader_stage: BindingShaderStage::VERTEX | BindingShaderStage::FRAGMENT, }] ), BindGroupDescriptor::new(