Skip to content

Commit

Permalink
Add ViewNode to simplify render node management (#8118)
Browse files Browse the repository at this point in the history
# Objective

- When writing render nodes that need a view, you always need to define
a `Query` on the associated view and make sure to update it manually and
query it manually. This is verbose and error prone.

## Solution

- Introduce a new `ViewNode` trait and `ViewNodeRunner` `Node` that will
take care of managing the associated view query automatically.
- The trait is currently a passthrough of the `Node` trait. So it still
has the update/run with all the same data passed in.
- The `ViewNodeRunner` is the actual node that is added to the render
graph and it contains the custom node. This is necessary because it's
the one that takes care of updating the node.

---

## Changelog

- Add `ViewNode`
- Add `ViewNodeRunner`

## Notes

Currently, this only handles the view query, but it could probably have
a ReadOnlySystemState that would also simplify querying all the readonly
resources that most render nodes currently query manually. The issue is
that I don't know how to do that without a `&mut self`.

At first, I tried making this a default feature of all `Node`, but I
kept hitting errors related to traits and generics and stuff I'm not
super comfortable with. This implementations is much simpler and keeps
the default Node behaviour so isn't a breaking change

## Reviewer Notes

The PR looks quite big, but the core of the PR is the changes in
`render_graph/node.rs`. Every other change is simply updating existing
nodes to use this new feature.

## Open questions

~~- Naming is not final, I'm opened to anything. I named it
ViewQueryNode because it's a node with a managed Query on a View.~~
~~- What to do when the query fails? All nodes using this pattern
currently just `return Ok(())` when it fails, so I chose that, but
should it be more flexible?~~
~~- Is the ViewQueryFilter actually necessary? All view queries run on
the entity that is already guaranteed to be a view. Filtering won't do
much, but maybe someone wants to control an effect with the presence of
a component instead of a flag.~~
~~- What to do with Nodes that are empty struct? Implementing
`FromWorld` is pretty verbose but not implementing it means there's 2
ways to create a `ViewNodeRunner` which seems less ideal. This is an
issue now because most node simply existed to hold the query, but now
that they don't hold the query state we are left with a bunch of empty
structs.~~
- Should we have a `RenderGraphApp::add_render_graph_view_node()`, this
isn't necessary, but it could make the code a bit shorter.

---------

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
  • Loading branch information
IceSentry and cart authored May 8, 2023
1 parent fe57b9f commit 613b5a6
Show file tree
Hide file tree
Showing 11 changed files with 230 additions and 274 deletions.
54 changes: 23 additions & 31 deletions crates/bevy_core_pipeline/src/bloom/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
};
use bevy_app::{App, Plugin};
use bevy_asset::{load_internal_asset, HandleUntyped};
use bevy_ecs::prelude::*;
use bevy_ecs::{prelude::*, query::QueryItem};
use bevy_math::UVec2;
use bevy_reflect::TypeUuid;
use bevy_render::{
Expand All @@ -19,7 +19,7 @@ use bevy_render::{
ComponentUniforms, DynamicUniformIndex, ExtractComponentPlugin, UniformComponentPlugin,
},
prelude::Color,
render_graph::{Node, NodeRunError, RenderGraphApp, RenderGraphContext},
render_graph::{NodeRunError, RenderGraphApp, RenderGraphContext, ViewNode, ViewNodeRunner},
render_resource::*,
renderer::{RenderContext, RenderDevice},
texture::{CachedTexture, TextureCache},
Expand Down Expand Up @@ -73,7 +73,10 @@ impl Plugin for BloomPlugin {
),
)
// Add bloom to the 3d render graph
.add_render_graph_node::<BloomNode>(CORE_3D, core_3d::graph::node::BLOOM)
.add_render_graph_node::<ViewNodeRunner<BloomNode>>(
CORE_3D,
core_3d::graph::node::BLOOM,
)
.add_render_graph_edges(
CORE_3D,
&[
Expand All @@ -83,7 +86,10 @@ impl Plugin for BloomPlugin {
],
)
// Add bloom to the 2d render graph
.add_render_graph_node::<BloomNode>(CORE_2D, core_2d::graph::node::BLOOM)
.add_render_graph_node::<ViewNodeRunner<BloomNode>>(
CORE_2D,
core_2d::graph::node::BLOOM,
)
.add_render_graph_edges(
CORE_2D,
&[
Expand All @@ -106,8 +112,10 @@ impl Plugin for BloomPlugin {
}
}

pub struct BloomNode {
view_query: QueryState<(
#[derive(Default)]
struct BloomNode;
impl ViewNode for BloomNode {
type ViewQuery = (
&'static ExtractedCamera,
&'static ViewTarget,
&'static BloomTexture,
Expand All @@ -116,36 +124,16 @@ pub struct BloomNode {
&'static BloomSettings,
&'static UpsamplingPipelineIds,
&'static BloomDownsamplingPipelineIds,
)>,
}

impl FromWorld for BloomNode {
fn from_world(world: &mut World) -> Self {
Self {
view_query: QueryState::new(world),
}
}
}

impl Node for BloomNode {
fn update(&mut self, world: &mut World) {
self.view_query.update_archetypes(world);
}
);

// Atypically for a post-processing effect, we do not need to
// use a secondary texture normally provided by view_target.post_process_write(),
// instead we write into our own bloom texture and then directly back onto main.
fn run(
&self,
graph: &mut RenderGraphContext,
_graph: &mut RenderGraphContext,
render_context: &mut RenderContext,
world: &World,
) -> Result<(), NodeRunError> {
let downsampling_pipeline_res = world.resource::<BloomDownsamplingPipeline>();
let pipeline_cache = world.resource::<PipelineCache>();
let uniforms = world.resource::<ComponentUniforms<BloomUniforms>>();
let view_entity = graph.view_entity();
let Ok((
(
camera,
view_target,
bloom_texture,
Expand All @@ -154,8 +142,12 @@ impl Node for BloomNode {
bloom_settings,
upsampling_pipeline_ids,
downsampling_pipeline_ids,
)) = self.view_query.get_manual(world, view_entity)
else { return Ok(()) };
): QueryItem<Self::ViewQuery>,
world: &World,
) -> Result<(), NodeRunError> {
let downsampling_pipeline_res = world.resource::<BloomDownsamplingPipeline>();
let pipeline_cache = world.resource::<PipelineCache>();
let uniforms = world.resource::<ComponentUniforms<BloomUniforms>>();

let (
Some(uniforms),
Expand Down
36 changes: 17 additions & 19 deletions crates/bevy_core_pipeline/src/core_2d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use bevy_ecs::prelude::*;
use bevy_render::{
camera::Camera,
extract_component::ExtractComponentPlugin,
render_graph::{EmptyNode, RenderGraphApp},
render_graph::{EmptyNode, RenderGraphApp, ViewNodeRunner},
render_phase::{
batch_phase_system, sort_phase_system, BatchedPhaseItem, CachedRenderPipelinePhaseItem,
DrawFunctionId, DrawFunctions, PhaseItem, RenderPhase,
Expand Down Expand Up @@ -65,24 +65,22 @@ impl Plugin for Core2dPlugin {
),
);

{
use graph::node::*;
render_app
.add_render_sub_graph(CORE_2D)
.add_render_graph_node::<MainPass2dNode>(CORE_2D, MAIN_PASS)
.add_render_graph_node::<TonemappingNode>(CORE_2D, TONEMAPPING)
.add_render_graph_node::<EmptyNode>(CORE_2D, END_MAIN_PASS_POST_PROCESSING)
.add_render_graph_node::<UpscalingNode>(CORE_2D, UPSCALING)
.add_render_graph_edges(
CORE_2D,
&[
MAIN_PASS,
TONEMAPPING,
END_MAIN_PASS_POST_PROCESSING,
UPSCALING,
],
);
}
use graph::node::*;
render_app
.add_render_sub_graph(CORE_2D)
.add_render_graph_node::<MainPass2dNode>(CORE_2D, MAIN_PASS)
.add_render_graph_node::<ViewNodeRunner<TonemappingNode>>(CORE_2D, TONEMAPPING)
.add_render_graph_node::<EmptyNode>(CORE_2D, END_MAIN_PASS_POST_PROCESSING)
.add_render_graph_node::<ViewNodeRunner<UpscalingNode>>(CORE_2D, UPSCALING)
.add_render_graph_edges(
CORE_2D,
&[
MAIN_PASS,
TONEMAPPING,
END_MAIN_PASS_POST_PROCESSING,
UPSCALING,
],
);
}
}

Expand Down
72 changes: 27 additions & 45 deletions crates/bevy_core_pipeline/src/core_3d/main_opaque_pass_3d_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,64 +4,46 @@ use crate::{
prepass::{DepthPrepass, MotionVectorPrepass, NormalPrepass},
skybox::{SkyboxBindGroup, SkyboxPipelineId},
};
use bevy_ecs::prelude::*;
use bevy_ecs::{prelude::*, query::QueryItem};
use bevy_render::{
camera::ExtractedCamera,
render_graph::{Node, NodeRunError, RenderGraphContext},
render_graph::{NodeRunError, RenderGraphContext, ViewNode},
render_phase::RenderPhase,
render_resource::{
LoadOp, Operations, PipelineCache, RenderPassDepthStencilAttachment, RenderPassDescriptor,
},
renderer::RenderContext,
view::{ExtractedView, ViewDepthTexture, ViewTarget, ViewUniformOffset},
view::{ViewDepthTexture, ViewTarget, ViewUniformOffset},
};
#[cfg(feature = "trace")]
use bevy_utils::tracing::info_span;

use super::{AlphaMask3d, Camera3dDepthLoadOp};

/// A [`Node`] that runs the [`Opaque3d`] and [`AlphaMask3d`] [`RenderPhase`].
pub struct MainOpaquePass3dNode {
query: QueryState<
(
&'static ExtractedCamera,
&'static RenderPhase<Opaque3d>,
&'static RenderPhase<AlphaMask3d>,
&'static Camera3d,
&'static ViewTarget,
&'static ViewDepthTexture,
Option<&'static DepthPrepass>,
Option<&'static NormalPrepass>,
Option<&'static MotionVectorPrepass>,
Option<&'static SkyboxPipelineId>,
Option<&'static SkyboxBindGroup>,
&'static ViewUniformOffset,
),
With<ExtractedView>,
>,
}

impl FromWorld for MainOpaquePass3dNode {
fn from_world(world: &mut World) -> Self {
Self {
query: world.query_filtered(),
}
}
}

impl Node for MainOpaquePass3dNode {
fn update(&mut self, world: &mut World) {
self.query.update_archetypes(world);
}
/// A [`bevy_render::render_graph::Node`] that runs the [`Opaque3d`] and [`AlphaMask3d`] [`RenderPhase`].
#[derive(Default)]
pub struct MainOpaquePass3dNode;
impl ViewNode for MainOpaquePass3dNode {
type ViewQuery = (
&'static ExtractedCamera,
&'static RenderPhase<Opaque3d>,
&'static RenderPhase<AlphaMask3d>,
&'static Camera3d,
&'static ViewTarget,
&'static ViewDepthTexture,
Option<&'static DepthPrepass>,
Option<&'static NormalPrepass>,
Option<&'static MotionVectorPrepass>,
Option<&'static SkyboxPipelineId>,
Option<&'static SkyboxBindGroup>,
&'static ViewUniformOffset,
);

fn run(
&self,
graph: &mut RenderGraphContext,
render_context: &mut RenderContext,
world: &World,
) -> Result<(), NodeRunError> {
let view_entity = graph.view_entity();
let Ok((
(
camera,
opaque_phase,
alpha_mask_phase,
Expand All @@ -74,11 +56,9 @@ impl Node for MainOpaquePass3dNode {
skybox_pipeline,
skybox_bind_group,
view_uniform_offset,
)) = self.query.get_manual(world, view_entity) else {
// No window
return Ok(());
};

): QueryItem<Self::ViewQuery>,
world: &World,
) -> Result<(), NodeRunError> {
// Run the opaque pass, sorted front-to-back
// NOTE: Scoped to drop the mutable borrow of render_context
#[cfg(feature = "trace")]
Expand Down Expand Up @@ -125,6 +105,8 @@ impl Node for MainOpaquePass3dNode {
render_pass.set_camera_viewport(viewport);
}

let view_entity = graph.view_entity();

// Opaque draws
opaque_phase.render(&mut render_pass, world, view_entity);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,58 +1,35 @@
use crate::core_3d::Transparent3d;
use bevy_ecs::prelude::*;
use bevy_ecs::{prelude::*, query::QueryItem};
use bevy_render::{
camera::ExtractedCamera,
render_graph::{Node, NodeRunError, RenderGraphContext},
render_graph::{NodeRunError, RenderGraphContext, ViewNode},
render_phase::RenderPhase,
render_resource::{LoadOp, Operations, RenderPassDepthStencilAttachment, RenderPassDescriptor},
renderer::RenderContext,
view::{ExtractedView, ViewDepthTexture, ViewTarget},
view::{ViewDepthTexture, ViewTarget},
};
#[cfg(feature = "trace")]
use bevy_utils::tracing::info_span;

/// A [`Node`] that runs the [`Transparent3d`] [`RenderPhase`].
pub struct MainTransparentPass3dNode {
query: QueryState<
(
&'static ExtractedCamera,
&'static RenderPhase<Transparent3d>,
&'static ViewTarget,
&'static ViewDepthTexture,
),
With<ExtractedView>,
>,
}

impl FromWorld for MainTransparentPass3dNode {
fn from_world(world: &mut World) -> Self {
Self {
query: world.query_filtered(),
}
}
}

impl Node for MainTransparentPass3dNode {
fn update(&mut self, world: &mut World) {
self.query.update_archetypes(world);
}
/// A [`bevy_render::render_graph::Node`] that runs the [`Transparent3d`] [`RenderPhase`].
#[derive(Default)]
pub struct MainTransparentPass3dNode;

impl ViewNode for MainTransparentPass3dNode {
type ViewQuery = (
&'static ExtractedCamera,
&'static RenderPhase<Transparent3d>,
&'static ViewTarget,
&'static ViewDepthTexture,
);
fn run(
&self,
graph: &mut RenderGraphContext,
render_context: &mut RenderContext,
(camera, transparent_phase, target, depth): QueryItem<Self::ViewQuery>,
world: &World,
) -> Result<(), NodeRunError> {
let view_entity = graph.view_entity();
let Ok((
camera,
transparent_phase,
target,
depth,
)) = self.query.get_manual(world, view_entity) else {
// No window
return Ok(());
};

if !transparent_phase.items.is_empty() {
// Run the transparent pass, sorted back-to-front
Expand Down
18 changes: 12 additions & 6 deletions crates/bevy_core_pipeline/src/core_3d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use bevy_render::{
camera::{Camera, ExtractedCamera},
extract_component::ExtractComponentPlugin,
prelude::Msaa,
render_graph::{EmptyNode, RenderGraphApp},
render_graph::{EmptyNode, RenderGraphApp, ViewNodeRunner},
render_phase::{
sort_phase_system, CachedRenderPipelinePhaseItem, DrawFunctionId, DrawFunctions, PhaseItem,
RenderPhase,
Expand Down Expand Up @@ -93,14 +93,20 @@ impl Plugin for Core3dPlugin {
use graph::node::*;
render_app
.add_render_sub_graph(CORE_3D)
.add_render_graph_node::<PrepassNode>(CORE_3D, PREPASS)
.add_render_graph_node::<ViewNodeRunner<PrepassNode>>(CORE_3D, PREPASS)
.add_render_graph_node::<EmptyNode>(CORE_3D, START_MAIN_PASS)
.add_render_graph_node::<MainOpaquePass3dNode>(CORE_3D, MAIN_OPAQUE_PASS)
.add_render_graph_node::<MainTransparentPass3dNode>(CORE_3D, MAIN_TRANSPARENT_PASS)
.add_render_graph_node::<ViewNodeRunner<MainOpaquePass3dNode>>(
CORE_3D,
MAIN_OPAQUE_PASS,
)
.add_render_graph_node::<ViewNodeRunner<MainTransparentPass3dNode>>(
CORE_3D,
MAIN_TRANSPARENT_PASS,
)
.add_render_graph_node::<EmptyNode>(CORE_3D, END_MAIN_PASS)
.add_render_graph_node::<TonemappingNode>(CORE_3D, TONEMAPPING)
.add_render_graph_node::<ViewNodeRunner<TonemappingNode>>(CORE_3D, TONEMAPPING)
.add_render_graph_node::<EmptyNode>(CORE_3D, END_MAIN_PASS_POST_PROCESSING)
.add_render_graph_node::<UpscalingNode>(CORE_3D, UPSCALING)
.add_render_graph_node::<ViewNodeRunner<UpscalingNode>>(CORE_3D, UPSCALING)
.add_render_graph_edges(
CORE_3D,
&[
Expand Down
Loading

0 comments on commit 613b5a6

Please sign in to comment.