-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Add FullscreenPassNode for post-processing #2027
Conversation
@mtsr I'm curious to hear your thoughts on a general "strategy" thing. As you know, we're currently investigating major changes to the renderer apis (and we would like this to land before Bevy 0.6 if possible). I'm currently inclined to hold off on reviewing / merging renderer prs until after we make these changes (which could either be a complete api overhaul or iterative changes to what we have). This both allows us to focus on getting the "renderer rework" changes out asap, cuts down on initial porting efforts, and allows us to review new features like this in the "new context". |
There's two important reasons I'm still working on this.
Then there's the fact that once I got going this was just really enjoyable and I wanted to finish it. Also, I did decide to drop integrating it into the base or pbr rendergraphs, as there's complications there that are not worth it with the upcoming changes in my mind. Still, I think merging this (if technically sound) still makes sense, because this is one of those things that we will want to offer ASAP with renderer improvements anyway, and being forced to update examples is a good way to do it. On the other hand, a lot depends on where you stand, on a day to day basis, regarding the future renderer. It's pretty hard really contributing there, except maybe doing a spike on bevy_rafx. |
Note that I just added a fairly significant addition, GlobalRenderResourcesNode, which is basically like RenderResourcesNode but for ECS Resources, rather than Components. This was done mainly for purposes of the example, where I wanted to access a value from a Resource just to show the possibility of passing uniforms without resorting to copying from complex code like lights_node. This should probably be split off before merging. I would love to have some feedback on the general idea, though. Note that it's still more or less a draft. I'm aware it doesn't currently handle textures in the Resource and there might be other gotchas I haven't covered. Edit: I've now removed GlobalRenderResourcesNode and made a separate PR #2043. |
@mtsr This is super valuable work, but it's going to be a pain to rebase. What would you like done with this PR? |
is this still needed after all the changed to the renderer? an example post processing node can be seen here: https://github.com/bevyengine/bevy/blob/main/examples/shader/post_processing.rs |
I think there might still be value in having a more specialized full screen Node but this PR would need to be completely rewritten from scratch and is indeed less relevant now that we do have post processing support. |
This is one of several PRs coming out of #1988. That PR kept growing, because in order to make the base graph accommodate flexible post processing, significant changes were needed.
It depends on #1998 for the additions to WindowTextureNode.
Waiting for #2039 to be able to cleanup plugins boilerplate.
Could really use #2043 to show passing ECS data into FullscreenPassNode shader.
This PR is a simpler addition of just FullscreenPassNode and an example that uses it.
Causes of the larger than expected length of the example:
Ideally we solve these issues so the example can be simpler.
Edit: Here's the render graph dump.