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

Reduce render Node boilerplate #7985

Closed
JMS55 opened this issue Mar 8, 2023 · 2 comments · Fixed by #8007
Closed

Reduce render Node boilerplate #7985

JMS55 opened this issue Mar 8, 2023 · 2 comments · Fixed by #8007
Labels
A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy
Milestone

Comments

@JMS55
Copy link
Contributor

JMS55 commented Mar 8, 2023

  1. Setting up a render node in Plugin::build() often looks like this:
// Create node
let taa_node = TAANode::new(&mut render_app.world);

// Get core_3d subgraph
let mut graph = render_app.world.resource_mut::<RenderGraph>();
let draw_3d_graph = graph
    .get_sub_graph_mut(crate::core_3d::graph::NAME)
    .unwrap();

// Add node, connect to view
draw_3d_graph.add_node(draw_3d_graph::node::TAA, taa_node);
draw_3d_graph.add_slot_edge(
    draw_3d_graph.input_node().id,
    crate::core_3d::graph::input::VIEW_ENTITY,
    draw_3d_graph::node::TAA,
    TAANode::IN_VIEW,
);

// Order nodes: MAIN_PASS -> TAA -> BLOOM -> TONEMAPPING
draw_3d_graph.add_node_edge(
    crate::core_3d::graph::node::MAIN_PASS,
    draw_3d_graph::node::TAA,
);
draw_3d_graph.add_node_edge(draw_3d_graph::node::TAA, crate::core_3d::graph::node::BLOOM);
draw_3d_graph.add_node_edge(
    draw_3d_graph::node::TAA,
    crate::core_3d::graph::node::TONEMAPPING,
);

We could add a helper to simplify it to something like this:

render_app.add_view_node<TAANode>(
    subgraph: crate::core_3d::graph::NAME,
    name: draw_3d_graph::node::TAA,
    order: &[
        crate::core_3d::graph::node::MAIN_PASS,
        draw_3d_graph::node::TAA,
        crate::core_3d::graph::node::BLOOM,
        crate::core_3d::graph::node::TONEMAPPING,
    ],
);

  1. Declaring a render Node often looks like this:
struct TAANode {
    view_query: QueryState<(
        &'static ExtractedCamera,
        &'static ViewTarget,
        &'static TAAHistoryTextures,
        &'static ViewPrepassTextures,
        &'static TAAPipelineId,
    )>,
}

impl TAANode {
    const IN_VIEW: &'static str = "view";

    fn new(world: &mut World) -> Self {
        Self {
            view_query: QueryState::new(world),
        }
    }
}

impl Node for TAANode {
    fn input(&self) -> Vec<SlotInfo> {
        vec![SlotInfo::new(Self::IN_VIEW, SlotType::Entity)]
    }

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

    fn run(
        &self,
        graph: &mut RenderGraphContext,
        render_context: &mut RenderContext,
        world: &World,
    ) -> Result<(), NodeRunError> {
        // Trace span
        #[cfg(feature = "trace")]
        let _taa_span = info_span!("taa").entered();

        // Get view_query
        let view_entity = graph.get_input_entity(Self::IN_VIEW)?;
        let (
            Ok((camera, view_target, taa_history_textures, prepass_textures, taa_pipeline_id)),
            Some(pipelines),
            Some(pipeline_cache),
        ) = (
            self.view_query.get_manual(world, view_entity),
            world.get_resource::<TAAPipeline>(),
            world.get_resource::<PipelineCache>(),
        ) else {
            return Ok(());
        };

        // ...
    }
}

We could provide an easier and more ergonomic Node impl:

struct TAANode {
    view_query: QueryState<(
        &'static ExtractedCamera,
        &'static ViewTarget,
        &'static TAAHistoryTextures,
        &'static ViewPrepassTextures,
        &'static TAAPipelineId,
    )>,
    // If possible, allow resource access declaratively 
    taa_pipeline: Res<TAAPipeline>,
    pipeline_cache: Res<PipelineCache>,
}

impl SimpleNode for TAANode {
    fn run(
        &self,
        graph: &mut RenderGraphContext,
        render_context: &mut RenderContext,
        world: &World,
        // Automatically get view_query
        ((camera, view_target, taa_history_textures, ...)): (...),
    ) -> Result<(), NodeRunError> {
        // Trace span automatically added

        // ...
    }
}
@JMS55 JMS55 added D-Trivial Nice and easy! A great choice to get started with Bevy A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Mar 8, 2023
@JMS55 JMS55 added this to the 0.11 milestone Mar 8, 2023
@IceSentry
Copy link
Contributor

I really like 1. Although, I'd probably suggest making it a free function that takes a &mut App because I don't really like the idea of adding an extension to App that only works when it's the render_app.

For 2., I'm not sure how the implementation would work, but it really feels even more like it should just be a regular system. I think we should try to go in that direction.

@JMS55
Copy link
Contributor Author

JMS55 commented Mar 8, 2023

Yeah a free function for 1. is probably better. Whatever syntax ends up best, but you get the basic idea.

For 2. I feel the same. It does feel like the render graph should just be replaced with systems, but that's a lot more controversial. Something like 2. would be a (hopefully) easy improvement we could make now, without having to change the fundamental setup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants