bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Divide `MainPass3dNode` sequential passes into separate nodes

Open alexmadeathing opened this issue 3 years ago • 9 comments

Objective

The objective of this PR is to give the user the ability to insert their own logic between render passes, and to further modularize Bevy's code.

There's currently no mechanism for inserting custom render logic between render passes in the core pipeline. E.g. between drawing alpha masked entities and transparent entities. All three passes, opaque, alpha mask, and transparent, execute back-to-back inside MainPass3dNode.

Solution

  • Divide MainPass3dNode into three separate nodes: OpaquePass3dNode, AlphaMaskPass3dNode, and TransparentPass3dNode
  • Update references to core_3d::graph::node::MAIN_PASS to core_3d::graph::node::TRANSPARENT_PASS
  • (optional extra) There's a hack in both MainPass3dNode and MainPass2dNode (search "WebGL2 quirk"); this could be made into something like DummyPassNode and only included in the graph when feature webgl is active - for now, I've moved it to TransparentPass3dNode

Changelog

Changed

  • MainPass3dNode has been divided into three separate nodes: OpaquePass3dNode, AlphaMaskPass3dNode, and TransparentPass3dNode

Migration Guide

  • core_3d::graph::node::MAIN_PASS has been removed and replaced with core_3d::graph::node::OPAQUE_PASS, core_3d::graph::node::ALPHA_MASK_PASS, and core_3d::graph::node::TRANSPARENT_PASS, which represent nodes for each individual pass in the core 3d pipeline

alexmadeathing avatar Sep 25 '22 21:09 alexmadeathing

FYI This is my first PR, just in case you need to poke the CI server.

Also I'm not a huge fan of external nodes referencing core_3d::graph::node::TRANSPARENT_PASS (e.g. bevy_ui), since their intention was "to run after the core 3d graph in it's entirety" and if a new pass node is appended to the core 3d graph, that intention will be violated. Not sure on solution for that yet.

alexmadeathing avatar Sep 25 '22 22:09 alexmadeathing

Would it be possible to add an empty node that's called FINAL_PASS that runs after the TRANSPARENT_PASS? That way other nodes that depend on the main pass won't need to know the transparent pass is the last one.

IceSentry avatar Sep 25 '22 22:09 IceSentry

Would it be possible to add an empty node that's called FINAL_PASS that runs after the TRANSPARENT_PASS? That way other nodes that depend on the main pass won't need to know the transparent pass is the last one.

Potentially, and that could be where the hack I mentioned earlier lives, however I'm not yet sure if empty nodes adds unnecessary overhead.

Another potential would be to add the FINAL_PASS label, but have it point to the last node in the core 3d graph.

alexmadeathing avatar Sep 25 '22 22:09 alexmadeathing

Also I'm not a huge fan of external nodes referencing core_3d::graph::node::TRANSPARENT_PASS (e.g. bevy_ui), since their intention was "to run after the core 3d graph in it's entirety" and if a new pass node is appended to the core 3d graph, that intention will be violated. Not sure on solution for that yet.

Couldn't you make the 3 nodes into a subgraph? I think this would even make these changes non-breaking.

MDeiml avatar Oct 13 '22 21:10 MDeiml

Couldn't you make the 3 nodes into a subgraph? I think this would even make these changes non-breaking.

Nice idea. Yes, I'll reconfigure and update this PR today.

alexmadeathing avatar Oct 14 '22 11:10 alexmadeathing

I ran the transparency_3d example locally on this PR:

https://user-images.githubusercontent.com/52322338/197334148-2b938264-ef71-490c-b6f2-b3dac69f2cf9.mp4

On main (cb5e2d84b):

https://user-images.githubusercontent.com/52322338/197334172-e7584cb5-3f55-4702-a07d-7f3fed9b867b.mp4

As you can see the behaviour is different. Could you try rebasing your PR?

torsteingrindvik avatar Oct 22 '22 10:10 torsteingrindvik

Could you try rebasing your PR?

Sure thing.

alexmadeathing avatar Oct 22 '22 11:10 alexmadeathing

Hi folks, I need to admit that I'm struggling with some ongoing personal issues right now and I can't commit to finishing this right now.

If anyone wants to take over, be my guest.

I did start the rebase and implementing the render graph as a sub-graph of the main render pass, but I'm afraid I haven't been able to finish it right now. You can find my current progress at: https://github.com/alexmadeathing/bevy/tree/separate-render-phases

Here's a copy of the latest commit message, which includes info about the issues I was running into:

WIP attempting to rebase render graph work after recent changes in main

Current issues:
- I'm not yet sure where to define the node names
- I'm not yet sure whether to implement the inner opaque, apha_mask, and transparent nodes as a separate plugin, or whether to do all the setup in Core3dPlugin
- There is an assertion failure in the current code in this commit because the render graph is not set up correctly. I'm sure this should be an easy fix

In addition, since I was working on this, main has moved on a bit. So this branch will need to be rebased again.

Good luck! If I'm better soon, I will of course come back to this. I'll check in here when that time comes up.

alexmadeathing avatar Dec 09 '22 20:12 alexmadeathing

I think all the changes from this PR have already been done in #8090. Should this be closed?

tygyh avatar Feb 08 '24 11:02 tygyh