From 96dea5b1f99d7a9a3fc38b35f849a597dc4126bd Mon Sep 17 00:00:00 2001 From: SeanOMik Date: Fri, 28 Jun 2024 13:16:47 -0400 Subject: [PATCH] render: code cleanup --- lyra-game/src/render/avec.rs | 5 + lyra-game/src/render/graph/mod.rs | 143 ++++++++---------- lyra-game/src/render/graph/node.rs | 1 + lyra-game/src/render/graph/passes/base.rs | 1 - .../src/render/graph/passes/present_pass.rs | 2 +- lyra-game/src/render/graph/passes/tint.rs | 14 +- lyra-game/src/render/graph/render_target.rs | 10 +- lyra-game/src/render/renderer.rs | 2 +- lyra-game/src/render/resource/mod.rs | 5 +- lyra-game/src/render/resource/pass.rs | 16 ++ lyra-game/src/render/resource/pipeline.rs | 1 + 11 files changed, 99 insertions(+), 101 deletions(-) create mode 100644 lyra-game/src/render/resource/pass.rs diff --git a/lyra-game/src/render/avec.rs b/lyra-game/src/render/avec.rs index 7bf1cff..1e785d3 100644 --- a/lyra-game/src/render/avec.rs +++ b/lyra-game/src/render/avec.rs @@ -240,6 +240,11 @@ impl AVec { self.len } + #[inline(always)] + pub fn is_empty(&self) -> bool { + self.len == 0 + } + /// Returns the capacity of the vector. /// /// The capacity is the amount of elements that the vector can store without reallocating. diff --git a/lyra-game/src/render/graph/mod.rs b/lyra-game/src/render/graph/mod.rs index 1e61ec3..3b65593 100644 --- a/lyra-game/src/render/graph/mod.rs +++ b/lyra-game/src/render/graph/mod.rs @@ -1,6 +1,6 @@ mod node; use std::{ - cell::{Ref, RefCell, RefMut}, collections::{HashMap, VecDeque}, fmt::Debug, hash::Hash, rc::Rc, sync::Arc + cell::{Ref, RefCell, RefMut}, collections::VecDeque, fmt::Debug, hash::Hash, rc::Rc, sync::Arc }; use lyra_ecs::World; @@ -20,14 +20,13 @@ pub use render_target::*; use rustc_hash::FxHashMap; use tracing::{debug_span, instrument, trace, warn}; -use wgpu::{CommandEncoder, ComputePass}; +use wgpu::CommandEncoder; -use super::resource::{ComputePipeline, Pipeline, RenderPipeline}; +use super::resource::{ComputePipeline, Pass, Pipeline, RenderPipeline}; +/// A trait that represents the label of a resource, slot, or node in the [`RenderGraph`]. pub trait RenderGraphLabel: Debug + 'static { fn rc_clone(&self) -> Rc; - //fn as_dyn(&self) -> &dyn RenderGraphLabel; - //fn as_partial_eq(&self) -> &dyn PartialEq; fn as_label_hash(&self) -> u64; fn label_eq_rc(&self, other: &Rc) -> bool { @@ -39,8 +38,7 @@ pub trait RenderGraphLabel: Debug + 'static { } } -pub struct RenderGraphHash(u64); - +/// An owned [`RenderGraphLabel`]. #[derive(Clone)] pub struct RenderGraphLabelValue(Rc); @@ -82,45 +80,39 @@ impl PartialEq for RenderGraphLabelValue { impl Eq for RenderGraphLabelValue {} -struct PassEntry { +struct NodeEntry { + /// The Node inner: Arc>, + /// The Node descriptor desc: Rc>, - /// The index of the pass in the execution graph + /// The index of the node in the execution graph graph_index: petgraph::matrix_graph::NodeIndex, - pipeline: Rc>>, + /// The Node's optional pipeline + pipeline: Rc>>, } #[derive(Clone)] -pub struct BindGroupEntry { - pub label: RenderGraphLabelValue, +struct BindGroupEntry { + label: RenderGraphLabelValue, /// BindGroup - pub bg: Rc, + bg: Rc, /// BindGroupLayout - pub layout: Option>, + layout: Option>, } #[allow(dead_code)] #[derive(Clone)] -struct ResourcedSlot { +struct ResourceSlot { label: RenderGraphLabelValue, ty: SlotType, value: SlotValue, } -/// Stores the pipeline and other resources it uses. -/// -/// This stores the bind groups that have been created for it -pub struct PipelineResource { - pub pipeline: Pipeline, - /// Lookup map for bind groups using names - pub bg_layout_name_lookup: HashMap, -} - pub struct RenderGraph { device: Arc, queue: Arc, - slots: FxHashMap, - nodes: FxHashMap, + slots: FxHashMap, + nodes: FxHashMap, sub_graphs: FxHashMap, bind_groups: FxHashMap, /// A directed graph used to determine dependencies of nodes. @@ -156,39 +148,28 @@ impl RenderGraph { /// * This means that the id of insert slots **ARE NOT STABLE**. **DO NOT** rely on them to /// not change. The IDs of output slots do stay the same. /// 3. Ensuring that no two slots share the same ID when the names do not match. - #[instrument(skip(self, pass), level = "debug")] - pub fn add_node(&mut self, label: impl RenderGraphLabel, mut pass: P) { - let mut desc = pass.desc(self); + #[instrument(skip(self, node), level = "debug")] + pub fn add_node(&mut self, label: impl RenderGraphLabel, mut node: P) { + let mut desc = node.desc(self); - // collect all the slots of the pass + // collect all the slots of the node for slot in &mut desc.slots { if let Some(other) = self .slots .get_mut(&slot.label) - //.map(|s| (id, s)) - //.and_then(|id| self.slots.get_mut(id).map(|s| (id, s))) { debug_assert_eq!( slot.ty, other.ty, - "slot {:?} in pass {:?} does not match existing slot of same name", + "slot {:?} in node {:?} does not match existing slot of same name", slot.label, label ); - - /* trace!( - "Found existing slot for {:?}, changing id to {}", - slot.label, - id - ); */ - - // if there is a slot of the same name - //slot.id = *id; } else { debug_assert!(!self.slots.contains_key(&slot.label), - "Reuse of id detected in render graph! Pass: {:?}, slot: {:?}", + "Reuse of id detected in render graph! Node: {:?}, slot: {:?}", label, slot.label, ); - let res_slot = ResourcedSlot { + let res_slot = ResourceSlot { label: slot.label.clone(), ty: slot.ty, value: slot.value.clone().unwrap_or(SlotValue::None), @@ -212,8 +193,8 @@ impl RenderGraph { self.nodes.insert( label, - PassEntry { - inner: Arc::new(RefCell::new(pass)), + NodeEntry { + inner: Arc::new(RefCell::new(node)), desc: Rc::new(RefCell::new(desc)), graph_index: index, pipeline: Rc::new(RefCell::new(None)), @@ -221,42 +202,38 @@ impl RenderGraph { ); } - /// Creates all buffers required for the passes, also creates an internal execution path. + /// Creates all buffers required for the nodes. /// /// This only needs to be ran when the [`Node`]s in the graph change, or they are removed or /// added. #[instrument(skip(self, device))] pub fn setup(&mut self, device: &wgpu::Device) { - // For all passes, create their pipelines - for pass in self.nodes.values_mut() { - let desc = (*pass.desc).borrow(); + // For all nodes, create their pipelines + for node in self.nodes.values_mut() { + let desc = (*node.desc).borrow(); if let Some(pipeline_desc) = &desc.pipeline_desc { let pipeline = match desc.ty { NodeType::Render => Pipeline::Render(RenderPipeline::create( device, pipeline_desc .as_render_pipeline_descriptor() - .expect("got compute pipeline descriptor in a render pass"), + .expect("got compute pipeline descriptor in a render node"), )), NodeType::Compute => Pipeline::Compute(ComputePipeline::create( device, pipeline_desc .as_compute_pipeline_descriptor() - .expect("got render pipeline descriptor in a compute pass"), + .expect("got render pipeline descriptor in a compute node"), )), NodeType::Presenter | NodeType::Node | NodeType::Graph => { - panic!("Present or Node RenderGraph passes should not have a pipeline descriptor!"); + panic!("Present or Node RenderGraph nodes should not have a pipeline descriptor!"); }, }; drop(desc); - let res = PipelineResource { - pipeline, - bg_layout_name_lookup: Default::default(), - }; - let mut pipeline = pass.pipeline.borrow_mut(); - *pipeline = Some(res); + let mut node_pipeline = node.pipeline.borrow_mut(); + *node_pipeline = Some(pipeline); } } @@ -278,15 +255,15 @@ impl RenderGraph { .map(|i| self.node_graph[i.clone()].clone()) .collect(); - while let Some(pass_label) = sorted.pop_front() { - let pass = self.nodes.get(&pass_label).unwrap(); + while let Some(node_label) = sorted.pop_front() { + let node = self.nodes.get(&node_label).unwrap(); let device = self.device.clone(); let queue = self.queue.clone(); - let inner = pass.inner.clone(); + let inner = node.inner.clone(); let mut inner = inner.borrow_mut(); - let mut context = RenderGraphContext::new(device, queue, None, pass_label.clone()); + let mut context = RenderGraphContext::new(device, queue, None, node_label.clone()); inner.prepare(self, world, &mut context); buffer_writes.append(&mut context.buffer_writes); } @@ -335,12 +312,12 @@ impl RenderGraph { // the encoder will be submitted and a new one will be made. let mut encoder = Some(self.create_encoder()); - while let Some(pass_label) = sorted.pop_front() { - let pass = self.nodes.get(&pass_label).unwrap(); - let pass_inn = pass.inner.clone(); + while let Some(node_label) = sorted.pop_front() { + let node = self.nodes.get(&node_label).unwrap(); + let node_inn = node.inner.clone(); - let pass_desc = pass.desc.clone(); - let pass_desc = (*pass_desc).borrow(); + let node_desc = node.desc.clone(); + let node_desc = (*node_desc).borrow(); // clone of the Rc's is required to appease the borrow checker let device = self.device.clone(); @@ -351,11 +328,11 @@ impl RenderGraph { encoder = Some(self.create_encoder()); } - let mut context = RenderGraphContext::new(device, queue, encoder.take(), pass_label.clone()); + let mut context = RenderGraphContext::new(device, queue, encoder.take(), node_label.clone()); - trace!("Executing {:?}", pass_label.0); - let mut inner = pass_inn.borrow_mut(); - inner.execute(self, &pass_desc, &mut context); + trace!("Executing {:?}", node_label.0); + let mut inner = node_inn.borrow_mut(); + inner.execute(self, &node_desc, &mut context); // take back the encoder from the context encoder = context.encoder; @@ -385,7 +362,7 @@ impl RenderGraph { let v = p.pipeline.borrow(); match &*v { - Some(_) => Some(Ref::map(v, |p| &p.as_ref().unwrap().pipeline)), + Some(_) => Some(Ref::map(v, |p| p.as_ref().unwrap())), None => None, } }) @@ -448,27 +425,26 @@ impl RenderGraph { /// &mut pass, /// &[ /// // retrieves the `BasePassSlots::DepthTexture` bind group and sets the index 0 in the - /// // pass to it. - /// (&BasePassSlots::DepthTexture, 0), - /// (&BasePassSlots::Camera, 1), - /// (&LightBasePassSlots::Lights, 2), - /// (&LightCullComputePassSlots::LightIndicesGridGroup, 3), - /// (&BasePassSlots::ScreenSize, 4), + /// // node to it. + /// (&BaseNodeSlots::DepthTexture, 0), + /// (&BaseNodeSlots::Camera, 1), + /// (&LightBaseNodeSlots::Lights, 2), + /// (&LightCullComputeNodeSlots::LightIndicesGridGroup, 3), + /// (&BaseNodeSlots::ScreenSize, 4), /// ], /// ); /// ``` /// /// # Panics /// Panics if a bind group of a provided name is not found. - pub fn set_bind_groups<'a>( + pub fn set_bind_groups<'a, P: Pass<'a>>( &'a self, - pass: &mut ComputePass<'a>, + pass: &mut P, bind_groups: &[(&dyn RenderGraphLabel, u32)], ) { for (label, index) in bind_groups { let bg = self .bind_group(label.rc_clone()); - //.expect(&format!("Could not find bind group '{:?}'", label)); pass.set_bind_group(*index, bg, &[]); } @@ -478,6 +454,9 @@ impl RenderGraph { self.sub_graphs.get_mut(&label.into()) } + /// Add a sub graph. + /// + /// > Note: the sub graph is not ran unless you add a node that executes it. See [`SubGraphNode`]. pub fn add_sub_graph>(&mut self, label: L, sub: RenderGraph) { self.sub_graphs.insert(label.into(), sub); } diff --git a/lyra-game/src/render/graph/node.rs b/lyra-game/src/render/graph/node.rs index 1437205..be47263 100644 --- a/lyra-game/src/render/graph/node.rs +++ b/lyra-game/src/render/graph/node.rs @@ -145,6 +145,7 @@ pub struct RenderGraphPipelineInfo { pub multiview: Option, } +#[allow(clippy::too_many_arguments)] impl RenderGraphPipelineInfo { pub fn new( label: &str, diff --git a/lyra-game/src/render/graph/passes/base.rs b/lyra-game/src/render/graph/passes/base.rs index bb1a301..7090c96 100644 --- a/lyra-game/src/render/graph/passes/base.rs +++ b/lyra-game/src/render/graph/passes/base.rs @@ -135,7 +135,6 @@ impl Node for BasePass { ) { let mut vt = graph.view_target_mut(); vt.primary.create_frame(); - //vt.next_chain(); vt.primary.create_frame_view(); /* debug_assert!( !rt.current_texture.is_some(), diff --git a/lyra-game/src/render/graph/passes/present_pass.rs b/lyra-game/src/render/graph/passes/present_pass.rs index f7ffb66..0dfba66 100644 --- a/lyra-game/src/render/graph/passes/present_pass.rs +++ b/lyra-game/src/render/graph/passes/present_pass.rs @@ -15,7 +15,7 @@ pub struct PresentPass; impl PresentPass { pub fn new() -> Self { - Self::default() + Self } } diff --git a/lyra-game/src/render/graph/passes/tint.rs b/lyra-game/src/render/graph/passes/tint.rs index fb406c8..48f3c29 100644 --- a/lyra-game/src/render/graph/passes/tint.rs +++ b/lyra-game/src/render/graph/passes/tint.rs @@ -7,15 +7,6 @@ use crate::render::{ resource::{FragmentState, PipelineDescriptor, RenderPipelineDescriptor, Shader, VertexState}, }; -#[derive(Debug, Clone, Copy, Hash, RenderGraphLabel)] -pub enum TintPassSlots { - InputRenderTarget, - InputTextureView, - - TextureViewBindGroup, - Frame, -} - #[derive(Default, Debug, Clone, Copy, Hash, RenderGraphLabel)] pub struct TintPassLabel; @@ -23,6 +14,9 @@ pub struct TintPassLabel; pub struct TintPass { target_sampler: Option, bgl: Option>, + /// Store bind groups for the input textures. + /// The texture may change due to resizes, or changes to the view target chain + /// from other nodes. bg_cache: HashMap, } @@ -118,7 +112,7 @@ impl Node for TintPass { ) { let pipeline = graph .pipeline(context.label.clone()) - .expect("Failed to find pipeline for MeshPass"); + .expect("Failed to find pipeline for TintPass"); let mut vt = graph.view_target_mut(); let chain = vt.get_chain(); diff --git a/lyra-game/src/render/graph/render_target.rs b/lyra-game/src/render/graph/render_target.rs index af1ee41..a125e5d 100644 --- a/lyra-game/src/render/graph/render_target.rs +++ b/lyra-game/src/render/graph/render_target.rs @@ -116,8 +116,6 @@ pub enum FrameTexture { /// Represents the current frame that is being rendered to. //#[allow(dead_code)] pub struct Frame { - /* pub(crate) device: Arc, - pub(crate) queue: Arc, */ pub(crate) size: math::UVec2, pub(crate) texture: FrameTexture, } @@ -189,8 +187,6 @@ impl FrameTarget { } } -//struct TargetViewChainPrimary - pub struct TargetViewChain<'a> { pub source: &'a mut FrameTarget, pub dest: &'a mut FrameTarget, @@ -204,6 +200,7 @@ struct ViewChain { } impl ViewChain { + /// Returns the currently active [`FrameTarget`]. fn active(&self) -> &FrameTarget { if self.active == 0 { &self.source @@ -234,14 +231,17 @@ impl ViewTarget { s } + /// Returns the size of the target. pub fn size(&self) -> math::UVec2 { self.primary.size() } + /// Returns the [`wgpu::TextureFormat`] pub fn format(&self) -> wgpu::TextureFormat { self.primary.format() } + /// Resize all the targets, causes the chain to be recreated. pub fn resize(&mut self, device: &wgpu::Device, size: math::UVec2) { if size != self.primary.size() { self.primary.render_target.resize(device, size); @@ -300,7 +300,7 @@ impl ViewTarget { } } - /// Get the [`wgpu::TextureView`] to render to + /// Get the [`wgpu::TextureView`] to render to. pub fn render_view(&self) -> &wgpu::TextureView { let chain = self.chain.as_ref().unwrap(); chain.active().frame_view.as_ref().unwrap() diff --git a/lyra-game/src/render/renderer.rs b/lyra-game/src/render/renderer.rs index 83bfc09..fd33b58 100755 --- a/lyra-game/src/render/renderer.rs +++ b/lyra-game/src/render/renderer.rs @@ -168,7 +168,7 @@ impl BasicRenderer { main_graph.add_edge(TestSubGraphLabel, TintPassLabel); //let present_pass_label = PresentPassLabel::new(BasePassSlots::Frame);//TintPassSlots::Frame); - let p = PresentPass::default(); + let p = PresentPass; main_graph.add_node(PresentPassLabel, p); main_graph.add_edge(BasePassLabel, TestSubGraphLabel); diff --git a/lyra-game/src/render/resource/mod.rs b/lyra-game/src/render/resource/mod.rs index 34ccb1d..68adfe2 100644 --- a/lyra-game/src/render/resource/mod.rs +++ b/lyra-game/src/render/resource/mod.rs @@ -8,4 +8,7 @@ mod compute_pipeline; pub use compute_pipeline::*; mod render_pipeline; -pub use render_pipeline::*; \ No newline at end of file +pub use render_pipeline::*; + +mod pass; +pub use pass::*; \ No newline at end of file diff --git a/lyra-game/src/render/resource/pass.rs b/lyra-game/src/render/resource/pass.rs new file mode 100644 index 0000000..db66e95 --- /dev/null +++ b/lyra-game/src/render/resource/pass.rs @@ -0,0 +1,16 @@ +/// A trait that represents a [`wgpu::ComputePass`] or [`wgpu::RenderPass`]. +pub trait Pass<'a> { + fn set_bind_group(&mut self, index: u32, bind_group: &'a wgpu::BindGroup, offsets: &[wgpu::DynamicOffset]); +} + +impl<'a> Pass<'a> for wgpu::ComputePass<'a> { + fn set_bind_group(&mut self, index: u32, bind_group: &'a wgpu::BindGroup, offsets: &[wgpu::DynamicOffset]) { + self.set_bind_group(index, bind_group, offsets); + } +} + +impl<'a> Pass<'a> for wgpu::RenderPass<'a> { + fn set_bind_group(&mut self, index: u32, bind_group: &'a wgpu::BindGroup, offsets: &[wgpu::DynamicOffset]) { + self.set_bind_group(index, bind_group, offsets); + } +} \ No newline at end of file diff --git a/lyra-game/src/render/resource/pipeline.rs b/lyra-game/src/render/resource/pipeline.rs index d66eb45..0ec09a6 100644 --- a/lyra-game/src/render/resource/pipeline.rs +++ b/lyra-game/src/render/resource/pipeline.rs @@ -1,5 +1,6 @@ use super::{compute_pipeline::ComputePipeline, render_pipeline::RenderPipeline, ComputePipelineDescriptor, RenderPipelineDescriptor}; +#[allow(clippy::large_enum_variant)] pub enum PipelineDescriptor { Render(RenderPipelineDescriptor), Compute(ComputePipelineDescriptor),