Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
# Objective The whole `Cow<'static, str>` naming for nodes and subgraphs in `RenderGraph` is a mess. ## Solution Replaces hardcoded and potentially overlapping strings for nodes and subgraphs inside `RenderGraph` with bevy's labelsystem. --- ## Changelog * Two new labels: `RenderLabel` and `RenderSubGraph`. * Replaced all uses for hardcoded strings with those labels * Moved `Taa` label from its own mod to all the other `Labels3d` * `add_render_graph_edges` now needs a tuple of labels * Moved `ScreenSpaceAmbientOcclusion` label from its own mod with the `ShadowPass` label to `LabelsPbr` * Removed `NodeId` * Renamed `Edges.id()` to `Edges.label()` * Removed `NodeLabel` * Changed examples according to the new label system * Introduced new `RenderLabel`s: `Labels2d`, `Labels3d`, `LabelsPbr`, `LabelsUi` * Introduced new `RenderSubGraph`s: `SubGraph2d`, `SubGraph3d`, `SubGraphUi` * Removed `Reflect` and `Default` derive from `CameraRenderGraph` component struct * Improved some error messages ## Migration Guide For Nodes and SubGraphs, instead of using hardcoded strings, you now pass labels, which can be derived with structs and enums. ```rs // old #[derive(Default)] struct MyRenderNode; impl MyRenderNode { pub const NAME: &'static str = "my_render_node" } render_app .add_render_graph_node::<ViewNodeRunner<MyRenderNode>>( core_3d::graph::NAME, MyRenderNode::NAME, ) .add_render_graph_edges( core_3d::graph::NAME, &[ core_3d::graph::node::TONEMAPPING, MyRenderNode::NAME, core_3d::graph::node::END_MAIN_PASS_POST_PROCESSING, ], ); // new use bevy::core_pipeline::core_3d::graph::{Labels3d, SubGraph3d}; #[derive(Debug, Hash, PartialEq, Eq, Clone, RenderLabel)] pub struct MyRenderLabel; #[derive(Default)] struct MyRenderNode; render_app .add_render_graph_node::<ViewNodeRunner<MyRenderNode>>( SubGraph3d, MyRenderLabel, ) .add_render_graph_edges( SubGraph3d, ( Labels3d::Tonemapping, MyRenderLabel, Labels3d::EndMainPassPostProcessing, ), ); ``` ### SubGraphs #### in `bevy_core_pipeline::core_2d::graph` | old string-based path | new label | |-----------------------|-----------| | `NAME` | `SubGraph2d` | #### in `bevy_core_pipeline::core_3d::graph` | old string-based path | new label | |-----------------------|-----------| | `NAME` | `SubGraph3d` | #### in `bevy_ui::render` | old string-based path | new label | |-----------------------|-----------| | `draw_ui_graph::NAME` | `graph::SubGraphUi` | ### Nodes #### in `bevy_core_pipeline::core_2d::graph` | old string-based path | new label | |-----------------------|-----------| | `node::MSAA_WRITEBACK` | `Labels2d::MsaaWriteback` | | `node::MAIN_PASS` | `Labels2d::MainPass` | | `node::BLOOM` | `Labels2d::Bloom` | | `node::TONEMAPPING` | `Labels2d::Tonemapping` | | `node::FXAA` | `Labels2d::Fxaa` | | `node::UPSCALING` | `Labels2d::Upscaling` | | `node::CONTRAST_ADAPTIVE_SHARPENING` | `Labels2d::ConstrastAdaptiveSharpening` | | `node::END_MAIN_PASS_POST_PROCESSING` | `Labels2d::EndMainPassPostProcessing` | #### in `bevy_core_pipeline::core_3d::graph` | old string-based path | new label | |-----------------------|-----------| | `node::MSAA_WRITEBACK` | `Labels3d::MsaaWriteback` | | `node::PREPASS` | `Labels3d::Prepass` | | `node::DEFERRED_PREPASS` | `Labels3d::DeferredPrepass` | | `node::COPY_DEFERRED_LIGHTING_ID` | `Labels3d::CopyDeferredLightingId` | | `node::END_PREPASSES` | `Labels3d::EndPrepasses` | | `node::START_MAIN_PASS` | `Labels3d::StartMainPass` | | `node::MAIN_OPAQUE_PASS` | `Labels3d::MainOpaquePass` | | `node::MAIN_TRANSMISSIVE_PASS` | `Labels3d::MainTransmissivePass` | | `node::MAIN_TRANSPARENT_PASS` | `Labels3d::MainTransparentPass` | | `node::END_MAIN_PASS` | `Labels3d::EndMainPass` | | `node::BLOOM` | `Labels3d::Bloom` | | `node::TONEMAPPING` | `Labels3d::Tonemapping` | | `node::FXAA` | `Labels3d::Fxaa` | | `node::UPSCALING` | `Labels3d::Upscaling` | | `node::CONTRAST_ADAPTIVE_SHARPENING` | `Labels3d::ContrastAdaptiveSharpening` | | `node::END_MAIN_PASS_POST_PROCESSING` | `Labels3d::EndMainPassPostProcessing` | #### in `bevy_core_pipeline` | old string-based path | new label | |-----------------------|-----------| | `taa::draw_3d_graph::node::TAA` | `Labels3d::Taa` | #### in `bevy_pbr` | old string-based path | new label | |-----------------------|-----------| | `draw_3d_graph::node::SHADOW_PASS` | `LabelsPbr::ShadowPass` | | `ssao::draw_3d_graph::node::SCREEN_SPACE_AMBIENT_OCCLUSION` | `LabelsPbr::ScreenSpaceAmbientOcclusion` | | `deferred::DEFFERED_LIGHTING_PASS` | `LabelsPbr::DeferredLightingPass` | #### in `bevy_render` | old string-based path | new label | |-----------------------|-----------| | `main_graph::node::CAMERA_DRIVER` | `graph::CameraDriverLabel` | #### in `bevy_ui::render` | old string-based path | new label | |-----------------------|-----------| | `draw_ui_graph::node::UI_PASS` | `graph::LabelsUi::UiPass` | --- ## Future work * Make `NodeSlot`s also use types. Ideally, we have an enum with unit variants where every variant resembles one slot. Then to make sure you are using the right slot enum and make rust-analyzer play nicely with it, we should make an associated type in the `Node` trait. With today's system, we can introduce 3rd party slots to a node, and i wasnt sure if this was used, so I didn't do this in this PR. ## Unresolved Questions When looking at the `post_processing` example, we have a struct for the label and a struct for the node, this seems like boilerplate and on discord, @IceSentry (sowy for the ping) [asked](https://discord.com/channels/691052431525675048/743663924229963868/1175197016947699742) if a node could automatically introduce a label (or i completely misunderstood that). The problem with that is, that nodes like `EmptyNode` exist multiple times *inside the same* (sub)graph, so there we need extern labels to distinguish between those. Hopefully we can find a way to reduce boilerplate and still have everything unique. For EmptyNode, we could maybe make a macro which implements an "empty node" for a type, but for nodes which contain code and need to be present multiple times, this could get nasty...
- Loading branch information