Add try_* to add_slot_edge, add_node_edge (#6720)

# Objective

`add_node_edge` and `add_slot_edge` are fallible methods, but are always used with `.unwrap()`.
`input_node` is often unwrapped as well.
This points to having an infallible behaviour as default, with an alternative fallible variant if needed.

Improves readability and ergonomics.

## Solution

- Change `add_node_edge` and `add_slot_edge` to panic on error.
- Change `input_node` to panic on `None`.
- Add `try_add_node_edge` and `try_add_slot_edge` in case fallible methods are needed.
- Add `get_input_node` to still be able to get an `Option`.
---

## Changelog

### Added

- `try_add_node_edge`
- `try_add_slot_edge`
- `get_input_node`

### Changed

- `add_node_edge` is now infallible (panics on error)
- `add_slot_edge` is now infallible (panics on error)
- `input_node` now panics on `None`

## Migration Guide

Remove `.unwrap()` from `add_node_edge` and `add_slot_edge`.
For cases where the error was handled, use `try_add_node_edge` and `try_add_slot_edge` instead.

Remove `.unwrap()` from `input_node`.
For cases where the option was handled, use `get_input_node` instead.


Co-authored-by: Torstein Grindvik <52322338+torsteingrindvik@users.noreply.github.com>
This commit is contained in:
Torstein Grindvik 2022-11-21 21:58:39 +00:00
parent e2d1d9dff8
commit daa57fe489
10 changed files with 249 additions and 252 deletions

View File

@ -59,27 +59,21 @@ impl Plugin for BloomPlugin {
.get_sub_graph_mut(crate::core_3d::graph::NAME)
.unwrap();
draw_3d_graph.add_node(core_3d::graph::node::BLOOM, bloom_node);
draw_3d_graph
.add_slot_edge(
draw_3d_graph.input_node().unwrap().id,
crate::core_3d::graph::input::VIEW_ENTITY,
core_3d::graph::node::BLOOM,
BloomNode::IN_VIEW,
)
.unwrap();
draw_3d_graph.add_slot_edge(
draw_3d_graph.input_node().id,
crate::core_3d::graph::input::VIEW_ENTITY,
core_3d::graph::node::BLOOM,
BloomNode::IN_VIEW,
);
// MAIN_PASS -> BLOOM -> TONEMAPPING
draw_3d_graph
.add_node_edge(
crate::core_3d::graph::node::MAIN_PASS,
core_3d::graph::node::BLOOM,
)
.unwrap();
draw_3d_graph
.add_node_edge(
core_3d::graph::node::BLOOM,
crate::core_3d::graph::node::TONEMAPPING,
)
.unwrap();
draw_3d_graph.add_node_edge(
crate::core_3d::graph::node::MAIN_PASS,
core_3d::graph::node::BLOOM,
);
draw_3d_graph.add_node_edge(
core_3d::graph::node::BLOOM,
crate::core_3d::graph::node::TONEMAPPING,
);
}
{
@ -89,27 +83,21 @@ impl Plugin for BloomPlugin {
.get_sub_graph_mut(crate::core_2d::graph::NAME)
.unwrap();
draw_2d_graph.add_node(core_2d::graph::node::BLOOM, bloom_node);
draw_2d_graph
.add_slot_edge(
draw_2d_graph.input_node().unwrap().id,
crate::core_2d::graph::input::VIEW_ENTITY,
core_2d::graph::node::BLOOM,
BloomNode::IN_VIEW,
)
.unwrap();
draw_2d_graph.add_slot_edge(
draw_2d_graph.input_node().id,
crate::core_2d::graph::input::VIEW_ENTITY,
core_2d::graph::node::BLOOM,
BloomNode::IN_VIEW,
);
// MAIN_PASS -> BLOOM -> TONEMAPPING
draw_2d_graph
.add_node_edge(
crate::core_2d::graph::node::MAIN_PASS,
core_2d::graph::node::BLOOM,
)
.unwrap();
draw_2d_graph
.add_node_edge(
core_2d::graph::node::BLOOM,
crate::core_2d::graph::node::TONEMAPPING,
)
.unwrap();
draw_2d_graph.add_node_edge(
crate::core_2d::graph::node::MAIN_PASS,
core_2d::graph::node::BLOOM,
);
draw_2d_graph.add_node_edge(
core_2d::graph::node::BLOOM,
crate::core_2d::graph::node::TONEMAPPING,
);
}
}
}

View File

@ -72,45 +72,33 @@ impl Plugin for Core2dPlugin {
graph::input::VIEW_ENTITY,
SlotType::Entity,
)]);
draw_2d_graph
.add_slot_edge(
input_node_id,
graph::input::VIEW_ENTITY,
graph::node::MAIN_PASS,
MainPass2dNode::IN_VIEW,
)
.unwrap();
draw_2d_graph
.add_slot_edge(
input_node_id,
graph::input::VIEW_ENTITY,
graph::node::TONEMAPPING,
TonemappingNode::IN_VIEW,
)
.unwrap();
draw_2d_graph
.add_slot_edge(
input_node_id,
graph::input::VIEW_ENTITY,
graph::node::UPSCALING,
UpscalingNode::IN_VIEW,
)
.unwrap();
draw_2d_graph
.add_node_edge(graph::node::MAIN_PASS, graph::node::TONEMAPPING)
.unwrap();
draw_2d_graph
.add_node_edge(
graph::node::TONEMAPPING,
graph::node::END_MAIN_PASS_POST_PROCESSING,
)
.unwrap();
draw_2d_graph
.add_node_edge(
graph::node::END_MAIN_PASS_POST_PROCESSING,
graph::node::UPSCALING,
)
.unwrap();
draw_2d_graph.add_slot_edge(
input_node_id,
graph::input::VIEW_ENTITY,
graph::node::MAIN_PASS,
MainPass2dNode::IN_VIEW,
);
draw_2d_graph.add_slot_edge(
input_node_id,
graph::input::VIEW_ENTITY,
graph::node::TONEMAPPING,
TonemappingNode::IN_VIEW,
);
draw_2d_graph.add_slot_edge(
input_node_id,
graph::input::VIEW_ENTITY,
graph::node::UPSCALING,
UpscalingNode::IN_VIEW,
);
draw_2d_graph.add_node_edge(graph::node::MAIN_PASS, graph::node::TONEMAPPING);
draw_2d_graph.add_node_edge(
graph::node::TONEMAPPING,
graph::node::END_MAIN_PASS_POST_PROCESSING,
);
draw_2d_graph.add_node_edge(
graph::node::END_MAIN_PASS_POST_PROCESSING,
graph::node::UPSCALING,
);
graph.add_sub_graph(graph::NAME, draw_2d_graph);
}
}

View File

@ -82,45 +82,33 @@ impl Plugin for Core3dPlugin {
graph::input::VIEW_ENTITY,
SlotType::Entity,
)]);
draw_3d_graph
.add_slot_edge(
input_node_id,
graph::input::VIEW_ENTITY,
graph::node::MAIN_PASS,
MainPass3dNode::IN_VIEW,
)
.unwrap();
draw_3d_graph
.add_slot_edge(
input_node_id,
graph::input::VIEW_ENTITY,
graph::node::TONEMAPPING,
TonemappingNode::IN_VIEW,
)
.unwrap();
draw_3d_graph
.add_slot_edge(
input_node_id,
graph::input::VIEW_ENTITY,
graph::node::UPSCALING,
UpscalingNode::IN_VIEW,
)
.unwrap();
draw_3d_graph
.add_node_edge(graph::node::MAIN_PASS, graph::node::TONEMAPPING)
.unwrap();
draw_3d_graph
.add_node_edge(
graph::node::TONEMAPPING,
graph::node::END_MAIN_PASS_POST_PROCESSING,
)
.unwrap();
draw_3d_graph
.add_node_edge(
graph::node::END_MAIN_PASS_POST_PROCESSING,
graph::node::UPSCALING,
)
.unwrap();
draw_3d_graph.add_slot_edge(
input_node_id,
graph::input::VIEW_ENTITY,
graph::node::MAIN_PASS,
MainPass3dNode::IN_VIEW,
);
draw_3d_graph.add_slot_edge(
input_node_id,
graph::input::VIEW_ENTITY,
graph::node::TONEMAPPING,
TonemappingNode::IN_VIEW,
);
draw_3d_graph.add_slot_edge(
input_node_id,
graph::input::VIEW_ENTITY,
graph::node::UPSCALING,
UpscalingNode::IN_VIEW,
);
draw_3d_graph.add_node_edge(graph::node::MAIN_PASS, graph::node::TONEMAPPING);
draw_3d_graph.add_node_edge(
graph::node::TONEMAPPING,
graph::node::END_MAIN_PASS_POST_PROCESSING,
);
draw_3d_graph.add_node_edge(
graph::node::END_MAIN_PASS_POST_PROCESSING,
graph::node::UPSCALING,
);
graph.add_sub_graph(graph::NAME, draw_3d_graph);
}
}

View File

@ -103,27 +103,21 @@ impl Plugin for FxaaPlugin {
graph.add_node(core_3d::graph::node::FXAA, fxaa_node);
graph
.add_slot_edge(
graph.input_node().unwrap().id,
core_3d::graph::input::VIEW_ENTITY,
core_3d::graph::node::FXAA,
FxaaNode::IN_VIEW,
)
.unwrap();
graph.add_slot_edge(
graph.input_node().id,
core_3d::graph::input::VIEW_ENTITY,
core_3d::graph::node::FXAA,
FxaaNode::IN_VIEW,
);
graph
.add_node_edge(
core_3d::graph::node::TONEMAPPING,
core_3d::graph::node::FXAA,
)
.unwrap();
graph
.add_node_edge(
core_3d::graph::node::FXAA,
core_3d::graph::node::END_MAIN_PASS_POST_PROCESSING,
)
.unwrap();
graph.add_node_edge(
core_3d::graph::node::TONEMAPPING,
core_3d::graph::node::FXAA,
);
graph.add_node_edge(
core_3d::graph::node::FXAA,
core_3d::graph::node::END_MAIN_PASS_POST_PROCESSING,
);
}
{
let fxaa_node = FxaaNode::new(&mut render_app.world);
@ -132,27 +126,21 @@ impl Plugin for FxaaPlugin {
graph.add_node(core_2d::graph::node::FXAA, fxaa_node);
graph
.add_slot_edge(
graph.input_node().unwrap().id,
core_2d::graph::input::VIEW_ENTITY,
core_2d::graph::node::FXAA,
FxaaNode::IN_VIEW,
)
.unwrap();
graph.add_slot_edge(
graph.input_node().id,
core_2d::graph::input::VIEW_ENTITY,
core_2d::graph::node::FXAA,
FxaaNode::IN_VIEW,
);
graph
.add_node_edge(
core_2d::graph::node::TONEMAPPING,
core_2d::graph::node::FXAA,
)
.unwrap();
graph
.add_node_edge(
core_2d::graph::node::FXAA,
core_2d::graph::node::END_MAIN_PASS_POST_PROCESSING,
)
.unwrap();
graph.add_node_edge(
core_2d::graph::node::TONEMAPPING,
core_2d::graph::node::FXAA,
);
graph.add_node_edge(
core_2d::graph::node::FXAA,
core_2d::graph::node::END_MAIN_PASS_POST_PROCESSING,
);
}
}
}

View File

@ -256,19 +256,15 @@ impl Plugin for PbrPlugin {
.get_sub_graph_mut(bevy_core_pipeline::core_3d::graph::NAME)
.unwrap();
draw_3d_graph.add_node(draw_3d_graph::node::SHADOW_PASS, shadow_pass_node);
draw_3d_graph
.add_node_edge(
draw_3d_graph::node::SHADOW_PASS,
bevy_core_pipeline::core_3d::graph::node::MAIN_PASS,
)
.unwrap();
draw_3d_graph
.add_slot_edge(
draw_3d_graph.input_node().unwrap().id,
bevy_core_pipeline::core_3d::graph::input::VIEW_ENTITY,
draw_3d_graph::node::SHADOW_PASS,
ShadowPassNode::IN_VIEW,
)
.unwrap();
draw_3d_graph.add_node_edge(
draw_3d_graph::node::SHADOW_PASS,
bevy_core_pipeline::core_3d::graph::node::MAIN_PASS,
);
draw_3d_graph.add_slot_edge(
draw_3d_graph.input_node().id,
bevy_core_pipeline::core_3d::graph::input::VIEW_ENTITY,
draw_3d_graph::node::SHADOW_PASS,
ShadowPassNode::IN_VIEW,
);
}
}

View File

@ -169,7 +169,7 @@ impl<'a> RenderGraphContext<'a> {
.graph
.get_sub_graph(&name)
.ok_or_else(|| RunSubGraphError::MissingSubGraph(name.clone()))?;
if let Some(input_node) = sub_graph.input_node() {
if let Some(input_node) = sub_graph.get_input_node() {
for (i, input_slot) in input_node.input_slots.iter().enumerate() {
if let Some(input_value) = inputs.get(i) {
if input_slot.slot_type != input_value.slot_type() {

View File

@ -46,7 +46,7 @@ use super::EdgeExistence;
/// let mut graph = RenderGraph::default();
/// graph.add_node("input_node", MyNode);
/// graph.add_node("output_node", MyNode);
/// graph.add_node_edge("output_node", "input_node").unwrap();
/// graph.add_node_edge("output_node", "input_node");
/// ```
#[derive(Resource, Default)]
pub struct RenderGraph {
@ -80,12 +80,30 @@ impl RenderGraph {
id
}
/// Returns the [`NodeState`] of the input node of this graph..
/// Returns the [`NodeState`] of the input node of this graph.
///
/// # See also
///
/// - [`input_node`](Self::input_node) for an unchecked version.
#[inline]
pub fn input_node(&self) -> Option<&NodeState> {
pub fn get_input_node(&self) -> Option<&NodeState> {
self.input_node.and_then(|id| self.get_node_state(id).ok())
}
/// Returns the [`NodeState`] of the input node of this graph.
///
/// # Panics
///
/// Panics if there is no input node set.
///
/// # See also
///
/// - [`get_input_node`](Self::get_input_node) for a version which returns an [`Option`] instead.
#[inline]
pub fn input_node(&self) -> &NodeState {
self.get_input_node().unwrap()
}
/// Adds the `node` with the `name` to the graph.
/// If the name is already present replaces it instead.
pub fn add_node<T>(&mut self, name: impl Into<Cow<'static, str>>, node: T) -> NodeId
@ -209,7 +227,13 @@ impl RenderGraph {
/// Adds the [`Edge::SlotEdge`] to the graph. This guarantees that the `output_node`
/// is run before the `input_node` and also connects the `output_slot` to the `input_slot`.
pub fn add_slot_edge(
///
/// Fails if any invalid [`NodeLabel`]s or [`SlotLabel`]s are given.
///
/// # See also
///
/// - [`add_slot_edge`](Self::add_slot_edge) for an infallible version.
pub fn try_add_slot_edge(
&mut self,
output_node: impl Into<NodeLabel>,
output_slot: impl Into<SlotLabel>,
@ -251,6 +275,27 @@ impl RenderGraph {
Ok(())
}
/// Adds the [`Edge::SlotEdge`] to the graph. This guarantees that the `output_node`
/// is run before the `input_node` and also connects the `output_slot` to the `input_slot`.
///
/// # Panics
///
/// Any invalid [`NodeLabel`]s or [`SlotLabel`]s are given.
///
/// # See also
///
/// - [`try_add_slot_edge`](Self::try_add_slot_edge) for a fallible version.
pub fn add_slot_edge(
&mut self,
output_node: impl Into<NodeLabel>,
output_slot: impl Into<SlotLabel>,
input_node: impl Into<NodeLabel>,
input_slot: impl Into<SlotLabel>,
) {
self.try_add_slot_edge(output_node, output_slot, input_node, input_slot)
.unwrap();
}
/// Removes the [`Edge::SlotEdge`] from the graph. If any nodes or slots do not exist then
/// nothing happens.
pub fn remove_slot_edge(
@ -297,7 +342,13 @@ impl RenderGraph {
/// Adds the [`Edge::NodeEdge`] to the graph. This guarantees that the `output_node`
/// is run before the `input_node`.
pub fn add_node_edge(
///
/// Fails if any invalid [`NodeLabel`] is given.
///
/// # See also
///
/// - [`add_node_edge`](Self::add_node_edge) for an infallible version.
pub fn try_add_node_edge(
&mut self,
output_node: impl Into<NodeLabel>,
input_node: impl Into<NodeLabel>,
@ -322,6 +373,24 @@ impl RenderGraph {
Ok(())
}
/// Adds the [`Edge::NodeEdge`] to the graph. This guarantees that the `output_node`
/// is run before the `input_node`.
///
/// # Panics
///
/// Panics if any invalid [`NodeLabel`] is given.
///
/// # See also
///
/// - [`try_add_node_edge`](Self::try_add_node_edge) for a fallible version.
pub fn add_node_edge(
&mut self,
output_node: impl Into<NodeLabel>,
input_node: impl Into<NodeLabel>,
) {
self.try_add_node_edge(output_node, input_node).unwrap();
}
/// Removes the [`Edge::NodeEdge`] from the graph. If either node does not exist then nothing
/// happens.
pub fn remove_node_edge(
@ -615,9 +684,9 @@ mod tests {
let c_id = graph.add_node("C", TestNode::new(1, 1));
let d_id = graph.add_node("D", TestNode::new(1, 0));
graph.add_slot_edge("A", "out_0", "C", "in_0").unwrap();
graph.add_node_edge("B", "C").unwrap();
graph.add_slot_edge("C", 0, "D", 0).unwrap();
graph.add_slot_edge("A", "out_0", "C", "in_0");
graph.add_node_edge("B", "C");
graph.add_slot_edge("C", 0, "D", 0);
fn input_nodes(name: &'static str, graph: &RenderGraph) -> HashSet<NodeId> {
graph
@ -703,9 +772,9 @@ mod tests {
graph.add_node("B", TestNode::new(0, 1));
graph.add_node("C", TestNode::new(1, 1));
graph.add_slot_edge("A", 0, "C", 0).unwrap();
graph.add_slot_edge("A", 0, "C", 0);
assert_eq!(
graph.add_slot_edge("B", 0, "C", 0),
graph.try_add_slot_edge("B", 0, "C", 0),
Err(RenderGraphError::NodeInputSlotAlreadyOccupied {
node: graph.get_node_id("C").unwrap(),
input_slot: 0,
@ -722,9 +791,9 @@ mod tests {
graph.add_node("A", TestNode::new(0, 1));
graph.add_node("B", TestNode::new(1, 0));
graph.add_slot_edge("A", 0, "B", 0).unwrap();
graph.add_slot_edge("A", 0, "B", 0);
assert_eq!(
graph.add_slot_edge("A", 0, "B", 0),
graph.try_add_slot_edge("A", 0, "B", 0),
Err(RenderGraphError::EdgeAlreadyExists(Edge::SlotEdge {
output_node: graph.get_node_id("A").unwrap(),
output_index: 0,

View File

@ -98,7 +98,7 @@ impl RenderGraphRunner {
.collect();
// pass inputs into the graph
if let Some(input_node) = graph.input_node() {
if let Some(input_node) = graph.get_input_node() {
let mut input_values: SmallVec<[SlotValue; 4]> = SmallVec::new();
for (i, input_slot) in input_node.input_slots.iter().enumerate() {
if let Some(input_value) = inputs.get(i) {

View File

@ -102,32 +102,24 @@ pub fn build_ui_render(app: &mut App) {
draw_ui_graph::node::UI_PASS,
RunGraphOnViewNode::new(draw_ui_graph::NAME),
);
graph_2d
.add_node_edge(
bevy_core_pipeline::core_2d::graph::node::MAIN_PASS,
draw_ui_graph::node::UI_PASS,
)
.unwrap();
graph_2d
.add_slot_edge(
graph_2d.input_node().unwrap().id,
bevy_core_pipeline::core_2d::graph::input::VIEW_ENTITY,
draw_ui_graph::node::UI_PASS,
RunGraphOnViewNode::IN_VIEW,
)
.unwrap();
graph_2d
.add_node_edge(
bevy_core_pipeline::core_2d::graph::node::END_MAIN_PASS_POST_PROCESSING,
draw_ui_graph::node::UI_PASS,
)
.unwrap();
graph_2d
.add_node_edge(
draw_ui_graph::node::UI_PASS,
bevy_core_pipeline::core_2d::graph::node::UPSCALING,
)
.unwrap();
graph_2d.add_node_edge(
bevy_core_pipeline::core_2d::graph::node::MAIN_PASS,
draw_ui_graph::node::UI_PASS,
);
graph_2d.add_slot_edge(
graph_2d.input_node().id,
bevy_core_pipeline::core_2d::graph::input::VIEW_ENTITY,
draw_ui_graph::node::UI_PASS,
RunGraphOnViewNode::IN_VIEW,
);
graph_2d.add_node_edge(
bevy_core_pipeline::core_2d::graph::node::END_MAIN_PASS_POST_PROCESSING,
draw_ui_graph::node::UI_PASS,
);
graph_2d.add_node_edge(
draw_ui_graph::node::UI_PASS,
bevy_core_pipeline::core_2d::graph::node::UPSCALING,
);
}
if let Some(graph_3d) = graph.get_sub_graph_mut(bevy_core_pipeline::core_3d::graph::NAME) {
@ -136,32 +128,24 @@ pub fn build_ui_render(app: &mut App) {
draw_ui_graph::node::UI_PASS,
RunGraphOnViewNode::new(draw_ui_graph::NAME),
);
graph_3d
.add_node_edge(
bevy_core_pipeline::core_3d::graph::node::MAIN_PASS,
draw_ui_graph::node::UI_PASS,
)
.unwrap();
graph_3d
.add_node_edge(
bevy_core_pipeline::core_3d::graph::node::END_MAIN_PASS_POST_PROCESSING,
draw_ui_graph::node::UI_PASS,
)
.unwrap();
graph_3d
.add_node_edge(
draw_ui_graph::node::UI_PASS,
bevy_core_pipeline::core_3d::graph::node::UPSCALING,
)
.unwrap();
graph_3d
.add_slot_edge(
graph_3d.input_node().unwrap().id,
bevy_core_pipeline::core_3d::graph::input::VIEW_ENTITY,
draw_ui_graph::node::UI_PASS,
RunGraphOnViewNode::IN_VIEW,
)
.unwrap();
graph_3d.add_node_edge(
bevy_core_pipeline::core_3d::graph::node::MAIN_PASS,
draw_ui_graph::node::UI_PASS,
);
graph_3d.add_node_edge(
bevy_core_pipeline::core_3d::graph::node::END_MAIN_PASS_POST_PROCESSING,
draw_ui_graph::node::UI_PASS,
);
graph_3d.add_node_edge(
draw_ui_graph::node::UI_PASS,
bevy_core_pipeline::core_3d::graph::node::UPSCALING,
);
graph_3d.add_slot_edge(
graph_3d.input_node().id,
bevy_core_pipeline::core_3d::graph::input::VIEW_ENTITY,
draw_ui_graph::node::UI_PASS,
RunGraphOnViewNode::IN_VIEW,
);
}
}
@ -173,14 +157,12 @@ fn get_ui_graph(render_app: &mut App) -> RenderGraph {
draw_ui_graph::input::VIEW_ENTITY,
SlotType::Entity,
)]);
ui_graph
.add_slot_edge(
input_node_id,
draw_ui_graph::input::VIEW_ENTITY,
draw_ui_graph::node::UI_PASS,
UiPassNode::IN_VIEW,
)
.unwrap();
ui_graph.add_slot_edge(
input_node_id,
draw_ui_graph::input::VIEW_ENTITY,
draw_ui_graph::node::UI_PASS,
UiPassNode::IN_VIEW,
);
ui_graph
}

View File

@ -77,12 +77,10 @@ impl Plugin for GameOfLifeComputePlugin {
let mut render_graph = render_app.world.resource_mut::<RenderGraph>();
render_graph.add_node("game_of_life", GameOfLifeNode::default());
render_graph
.add_node_edge(
"game_of_life",
bevy::render::main_graph::node::CAMERA_DRIVER,
)
.unwrap();
render_graph.add_node_edge(
"game_of_life",
bevy::render::main_graph::node::CAMERA_DRIVER,
);
}
}