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

delegate layout reflection to RenderResourceContext #691

Merged
merged 2 commits into from
Nov 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 2 additions & 17 deletions crates/bevy_pbr/src/entity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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(),
Expand Down
7 changes: 3 additions & 4 deletions crates/bevy_render/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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>,
}

Expand Down Expand Up @@ -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>()
Expand Down
67 changes: 2 additions & 65 deletions crates/bevy_render/src/pipeline/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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);
}
}
43 changes: 32 additions & 11 deletions crates/bevy_render/src/pipeline/pipeline_compiler.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -18,7 +18,7 @@ use std::borrow::Cow;
pub struct PipelineSpecialization {
pub shader_specialization: ShaderSpecialization,
pub primitive_topology: PrimitiveTopology,
pub dynamic_bindings: Vec<DynamicBinding>,
Copy link
Member

Choose a reason for hiding this comment

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

Moving this out of PipelineSpecialization feels a bit wrong. I totally get that by directly consuming RenderResourceBindings, we can cut down on abstraction (and the extra Vec allocation), but having everything that "specializes" a pipeline in one place seems like a good design to keep. If in the future we want to do something like "hash pipeline specialization to identify a pipeline", we would need to undo the change you made here to ensure the dynamic bindings are accounted for.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right, changed.

pub dynamic_bindings: Vec<String>,
pub index_format: IndexFormat,
pub vertex_buffer_descriptor: VertexBufferDescriptor,
pub sample_count: u32,
Expand Down Expand Up @@ -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>>,
Expand Down Expand Up @@ -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
Expand Down
32 changes: 22 additions & 10 deletions crates/bevy_render/src/pipeline/render_pipelines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand All @@ -31,6 +34,7 @@ impl RenderPipeline {
RenderPipeline {
pipeline,
specialization,
dynamic_bindings_generation: std::usize::MAX,
}
}
}
Expand Down Expand Up @@ -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,
Expand All @@ -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])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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)]
Expand Down
32 changes: 30 additions & 2 deletions crates/bevy_render/src/renderer/render_resource_context.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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 {
Expand Down
Loading