delegate layout reflection to RenderResourceContext (#691)

* delegate layout reflection to RenderResourceContext Also: * auto-reflect DynamicBindings * use RenderPipeline::new, update dynamic_bindings linting. * add dynamic binding generation Co-authored-by: Carter Anderson <[email protected]>
bevyengine · Nov 10, 2020 · 60fa2d5 · 60fa2d5
1 parent 8607ccf
commit 60fa2d5
Show file tree

Hide file tree

Showing 12 changed files with 134 additions and 258 deletions.
diff --git a/crates/bevy_pbr/src/entity.rs b/crates/bevy_pbr/src/entity.rs
@@ -4,7 +4,7 @@ use bevy_ecs::Bundle;
 use bevy_render::{
     draw::Draw,
     mesh::Mesh,
-    pipeline::{DynamicBinding, PipelineSpecialization, RenderPipeline, RenderPipelines},
+    pipeline::{RenderPipeline, RenderPipelines},
     render_graph::base::MainPass,
 };
 use bevy_transform::prelude::{GlobalTransform, Transform};
@@ -24,23 +24,8 @@ pub struct PbrComponents {
 impl Default for PbrComponents {
     fn default() -> Self {
         Self {
-            render_pipelines: RenderPipelines::from_pipelines(vec![RenderPipeline::specialized(
+            render_pipelines: RenderPipelines::from_pipelines(vec![RenderPipeline::new(
                 FORWARD_PIPELINE_HANDLE,
-                PipelineSpecialization {
-                    dynamic_bindings: vec![
-                        // Transform
-                        DynamicBinding {
-                            bind_group: 2,
-                            binding: 0,
-                        },
-                        // StandardMaterial_albedo
-                        DynamicBinding {
-                            bind_group: 3,
-                            binding: 0,
-                        },
-                    ],
-                    ..Default::default()
-                },
             )]),
             mesh: Default::default(),
             material: Default::default(),

diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs
@@ -37,8 +37,8 @@ use camera::{
     ActiveCameras, Camera, OrthographicProjection, PerspectiveProjection, VisibleEntities,
 };
 use pipeline::{
-    DynamicBinding, IndexFormat, PipelineCompiler, PipelineDescriptor, PipelineSpecialization,
-    PrimitiveTopology, ShaderSpecialization,
+    IndexFormat, PipelineCompiler, PipelineDescriptor, PipelineSpecialization, PrimitiveTopology,
+    ShaderSpecialization,
 };
 use render_graph::{
     base::{self, BaseRenderGraphBuilder, BaseRenderGraphConfig},
@@ -66,7 +66,7 @@ pub mod stage {
 
 /// Adds core render types and systems to an App
 pub struct RenderPlugin {
-    /// configures the "base render graph". If this is not `None`, the "base render graph" will be added  
+    /// configures the "base render graph". If this is not `None`, the "base render graph" will be added
     pub base_render_graph_config: Option<BaseRenderGraphConfig>,
 }
 
@@ -112,7 +112,6 @@ impl Plugin for RenderPlugin {
             .register_property::<Color>()
             .register_property::<Range<f32>>()
             .register_property::<ShaderSpecialization>()
-            .register_property::<DynamicBinding>()
             .register_property::<PrimitiveTopology>()
             .register_property::<IndexFormat>()
             .register_properties::<PipelineSpecialization>()

diff --git a/crates/bevy_render/src/pipeline/pipeline.rs b/crates/bevy_render/src/pipeline/pipeline.rs
@@ -4,13 +4,9 @@ use super::{
         CompareFunction, CullMode, DepthStencilStateDescriptor, FrontFace, IndexFormat,
         PrimitiveTopology, RasterizationStateDescriptor, StencilStateFaceDescriptor,
     },
-    BindType, DynamicBinding, PipelineLayout, StencilStateDescriptor,
+    PipelineLayout, StencilStateDescriptor,
 };
-use crate::{
-    shader::{Shader, ShaderStages},
-    texture::TextureFormat,
-};
-use bevy_asset::Assets;
+use crate::{shader::ShaderStages, texture::TextureFormat};
 use bevy_type_registry::TypeUuid;
 
 #[derive(Clone, Debug, TypeUuid)]
@@ -117,63 +113,4 @@ impl PipelineDescriptor {
     pub fn get_layout_mut(&mut self) -> Option<&mut PipelineLayout> {
         self.layout.as_mut()
     }
-
-    /// Reflects the pipeline layout from its shaders.
-    ///
-    /// If `bevy_conventions` is true, it will be assumed that the shader follows "bevy shader conventions". These allow
-    /// richer reflection, such as inferred Vertex Buffer names and inferred instancing.
-    ///
-    /// If `dynamic_bindings` has values, shader uniforms will be set to "dynamic" if there is a matching binding in the list
-    ///
-    /// If `vertex_buffer_descriptors` is set, the pipeline's vertex buffers
-    /// will inherit their layouts from global descriptors, otherwise the layout will be assumed to be complete / local.
-    pub fn reflect_layout(
-        &mut self,
-        shaders: &Assets<Shader>,
-        bevy_conventions: bool,
-        dynamic_bindings: &[DynamicBinding],
-    ) {
-        let vertex_spirv = shaders.get(&self.shader_stages.vertex).unwrap();
-        let fragment_spirv = self
-            .shader_stages
-            .fragment
-            .as_ref()
-            .map(|handle| shaders.get(handle).unwrap());
-
-        let mut layouts = vec![vertex_spirv.reflect_layout(bevy_conventions).unwrap()];
-        if let Some(ref fragment_spirv) = fragment_spirv {
-            layouts.push(fragment_spirv.reflect_layout(bevy_conventions).unwrap());
-        }
-
-        let mut layout = PipelineLayout::from_shader_layouts(&mut layouts);
-
-        if !dynamic_bindings.is_empty() {
-            // set binding uniforms to dynamic if render resource bindings use dynamic
-            for bind_group in layout.bind_groups.iter_mut() {
-                let mut binding_changed = false;
-                for binding in bind_group.bindings.iter_mut() {
-                    let current = DynamicBinding {
-                        bind_group: bind_group.index,
-                        binding: binding.index,
-                    };
-
-                    if dynamic_bindings.contains(&current) {
-                        if let BindType::Uniform {
-                            ref mut dynamic, ..
-                        } = binding.bind_type
-                        {
-                            *dynamic = true;
-                            binding_changed = true;
-                        }
-                    }
-                }
-
-                if binding_changed {
-                    bind_group.update_id();
-                }
-            }
-        }
-
-        self.layout = Some(layout);
-    }
 }
diff --git a/crates/bevy_render/src/pipeline/pipeline_compiler.rs b/crates/bevy_render/src/pipeline/pipeline_compiler.rs
@@ -1,7 +1,7 @@
 use super::{state_descriptors::PrimitiveTopology, IndexFormat, PipelineDescriptor};
 use crate::{
     pipeline::{
-        InputStepMode, VertexAttributeDescriptor, VertexBufferDescriptor, VertexFormat,
+        BindType, InputStepMode, VertexAttributeDescriptor, VertexBufferDescriptor, VertexFormat,
         VERTEX_FALLBACK_LAYOUT_NAME,
     },
     renderer::RenderResourceContext,
@@ -18,7 +18,7 @@ use std::borrow::Cow;
 pub struct PipelineSpecialization {
     pub shader_specialization: ShaderSpecialization,
     pub primitive_topology: PrimitiveTopology,
-    pub dynamic_bindings: Vec<DynamicBinding>,
+    pub dynamic_bindings: Vec<String>,
     pub index_format: IndexFormat,
     pub vertex_buffer_descriptor: VertexBufferDescriptor,
     pub sample_count: u32,
@@ -61,12 +61,6 @@ struct SpecializedPipeline {
     specialization: PipelineSpecialization,
 }
 
-#[derive(Clone, Eq, PartialEq, Debug, Default, Serialize, Deserialize, Property)]
-pub struct DynamicBinding {
-    pub bind_group: u32,
-    pub binding: u32,
-}
-
 #[derive(Debug, Default)]
 pub struct PipelineCompiler {
     specialized_shaders: HashMap<Handle<Shader>, Vec<SpecializedShader>>,
@@ -163,12 +157,39 @@ impl PipelineCompiler {
                 )
             });
 
-        specialized_descriptor.reflect_layout(
-            shaders,
+        let mut layout = render_resource_context.reflect_pipeline_layout(
+            &shaders,
+            &specialized_descriptor.shader_stages,
             true,
-            &pipeline_specialization.dynamic_bindings,
         );
 
+        if !pipeline_specialization.dynamic_bindings.is_empty() {
+            // set binding uniforms to dynamic if render resource bindings use dynamic
+            for bind_group in layout.bind_groups.iter_mut() {
+                let mut binding_changed = false;
+                for binding in bind_group.bindings.iter_mut() {
+                    if pipeline_specialization
+                        .dynamic_bindings
+                        .iter()
+                        .any(|b| b == &binding.name)
+                    {
+                        if let BindType::Uniform {
+                            ref mut dynamic, ..
+                        } = binding.bind_type
+                        {
+                            *dynamic = true;
+                            binding_changed = true;
+                        }
+                    }
+                }
+
+                if binding_changed {
+                    bind_group.update_id();
+                }
+            }
+        }
+        specialized_descriptor.layout = Some(layout);
+
         // create a vertex layout that provides all attributes from either the specialized vertex buffers or a zero buffer
         let mut pipeline_layout = specialized_descriptor.layout.as_mut().unwrap();
         // the vertex buffer descriptor of the mesh

diff --git a/crates/bevy_render/src/pipeline/render_pipelines.rs b/crates/bevy_render/src/pipeline/render_pipelines.rs
@@ -14,13 +14,16 @@ use bevy_property::Properties;
 pub struct RenderPipeline {
     pub pipeline: Handle<PipelineDescriptor>,
     pub specialization: PipelineSpecialization,
+    /// used to track if PipelineSpecialization::dynamic_bindings is in sync with RenderResourceBindings
+    pub dynamic_bindings_generation: usize,
 }
 
 impl RenderPipeline {
     pub fn new(pipeline: Handle<PipelineDescriptor>) -> Self {
         RenderPipeline {
+            specialization: Default::default(),
             pipeline,
-            ..Default::default()
+            dynamic_bindings_generation: std::usize::MAX,
         }
     }
 
@@ -31,6 +34,7 @@ impl RenderPipeline {
         RenderPipeline {
             pipeline,
             specialization,
+            dynamic_bindings_generation: std::usize::MAX,
         }
     }
 }
@@ -100,10 +104,24 @@ pub fn draw_render_pipelines_system(
         let render_pipelines = &mut *render_pipelines;
         for pipeline in render_pipelines.pipelines.iter_mut() {
             pipeline.specialization.sample_count = msaa.samples;
-            // TODO: move these to mesh.rs?
+            if pipeline.dynamic_bindings_generation
+                != render_pipelines.bindings.dynamic_bindings_generation()
+            {
+                pipeline.specialization.dynamic_bindings = render_pipelines
+                    .bindings
+                    .iter_dynamic_bindings()
+                    .map(|name| name.to_string())
+                    .collect::<Vec<String>>();
+                pipeline.dynamic_bindings_generation =
+                    render_pipelines.bindings.dynamic_bindings_generation();
+            }
         }
 
-        for render_pipeline in render_pipelines.pipelines.iter() {
+        for render_pipeline in render_pipelines.pipelines.iter_mut() {
+            let render_resource_bindings = &mut [
+                &mut render_pipelines.bindings,
+                &mut render_resource_bindings,
+            ];
             draw_context
                 .set_pipeline(
                     &mut draw,
@@ -112,13 +130,7 @@ pub fn draw_render_pipelines_system(
                 )
                 .unwrap();
             draw_context
-                .set_bind_groups_from_bindings(
-                    &mut draw,
-                    &mut [
-                        &mut render_pipelines.bindings,
-                        &mut render_resource_bindings,
-                    ],
-                )
+                .set_bind_groups_from_bindings(&mut draw, render_resource_bindings)
                 .unwrap();
             draw_context
                 .set_vertex_buffers_from_bindings(&mut draw, &[&render_pipelines.bindings])

diff --git a/crates/bevy_render/src/renderer/render_resource/render_resource_bindings.rs b/crates/bevy_render/src/renderer/render_resource/render_resource_bindings.rs
@@ -35,6 +35,13 @@ impl RenderResourceBinding {
         }
     }
 
+    pub fn is_dynamic_buffer(&self) -> bool {
+        matches!(self, RenderResourceBinding::Buffer {
+            dynamic_index: Some(_),
+            ..
+        })
+    }
+
     pub fn get_sampler(&self) -> Option<SamplerId> {
         if let RenderResourceBinding::Sampler(sampler) = self {
             Some(*sampler)
@@ -103,7 +110,7 @@ pub enum BindGroupStatus {
 // PERF: if the bindings are scoped to a specific pipeline layout, then names could be replaced with indices here for a perf boost
 #[derive(Eq, PartialEq, Debug, Default, Clone)]
 pub struct RenderResourceBindings {
-    bindings: HashMap<String, RenderResourceBinding>,
+    pub bindings: HashMap<String, RenderResourceBinding>,
     /// A Buffer that contains all attributes a mesh has defined
     pub vertex_attribute_buffer: Option<BufferId>,
     /// A Buffer that is filled with zeros that will be used for attributes required by the shader, but undefined by the mesh.
@@ -112,6 +119,7 @@ pub struct RenderResourceBindings {
     bind_groups: HashMap<BindGroupId, BindGroup>,
     bind_group_descriptors: HashMap<BindGroupDescriptorId, Option<BindGroupId>>,
     dirty_bind_groups: HashSet<BindGroupId>,
+    dynamic_bindings_generation: usize,
 }
 
 impl RenderResourceBindings {
@@ -124,9 +132,17 @@ impl RenderResourceBindings {
         self.bindings.insert(name.to_string(), binding);
     }
 
+    /// The current "generation" of dynamic bindings. This number increments every time a dynamic binding changes
+    pub fn dynamic_bindings_generation(&self) -> usize {
+        self.dynamic_bindings_generation
+    }
+
     fn try_set_dirty(&mut self, name: &str, binding: &RenderResourceBinding) {
         if let Some(current_binding) = self.bindings.get(name) {
             if current_binding != binding {
+                if current_binding.is_dynamic_buffer() {
+                    self.dynamic_bindings_generation += 1;
+                }
                 // TODO: this is crude. we shouldn't need to invalidate all bind groups
                 for id in self.bind_groups.keys() {
                     self.dirty_bind_groups.insert(*id);
@@ -236,6 +252,18 @@ impl RenderResourceBindings {
 
         Some(bind_group_builder.finish())
     }
+
+    pub fn iter_dynamic_bindings(&self) -> impl Iterator<Item = &str> {
+        self.bindings
+            .iter()
+            .filter(|(_, binding)| {
+                matches!(binding, RenderResourceBinding::Buffer {
+                    dynamic_index: Some(_),
+                    ..
+                })
+            })
+            .map(|(name, _)| name.as_str())
+    }
 }
 
 #[derive(Debug, Default)]

diff --git a/crates/bevy_render/src/renderer/render_resource_context.rs b/crates/bevy_render/src/renderer/render_resource_context.rs
@@ -1,7 +1,7 @@
 use crate::{
-    pipeline::{BindGroupDescriptorId, PipelineDescriptor},
+    pipeline::{BindGroupDescriptorId, PipelineDescriptor, PipelineLayout},
     renderer::{BindGroup, BufferId, BufferInfo, RenderResourceId, SamplerId, TextureId},
-    shader::Shader,
+    shader::{Shader, ShaderLayout, ShaderStages},
     texture::{SamplerDescriptor, TextureDescriptor},
 };
 use bevy_asset::{Asset, Assets, Handle, HandleUntyped};
@@ -60,6 +60,34 @@ pub trait RenderResourceContext: Downcast + Send + Sync + 'static {
         bind_group: &BindGroup,
     );
     fn clear_bind_groups(&self);
+    /// Reflects the pipeline layout from its shaders.
+    ///
+    /// If `bevy_conventions` is true, it will be assumed that the shader follows "bevy shader conventions". These allow
+    /// richer reflection, such as inferred Vertex Buffer names and inferred instancing.
+    ///
+    /// If `dynamic_bindings` has values, shader uniforms will be set to "dynamic" if there is a matching binding in the list
+    ///
+    /// If `vertex_buffer_descriptors` is set, the pipeline's vertex buffers
+    /// will inherit their layouts from global descriptors, otherwise the layout will be assumed to be complete / local.
+    fn reflect_pipeline_layout(
+        &self,
+        shaders: &Assets<Shader>,
+        shader_stages: &ShaderStages,
+        enforce_bevy_conventions: bool,
+    ) -> PipelineLayout {
+        // TODO: maybe move this default implementation to PipelineLayout?
+        let mut shader_layouts: Vec<ShaderLayout> = shader_stages
+            .iter()
+            .map(|handle| {
+                shaders
+                    .get(&handle)
+                    .unwrap()
+                    .reflect_layout(enforce_bevy_conventions)
+                    .unwrap()
+            })
+            .collect();
+        PipelineLayout::from_shader_layouts(&mut shader_layouts)
+    }
 }
 
 impl dyn RenderResourceContext {