Skip to content

Add bevy_post_processing #18425

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

IceSentry
Copy link
Contributor

@IceSentry IceSentry commented Mar 19, 2025

Objective

Solution

Testing

  • Ran various post processing effects examples and things still worked

Showcase

main:

image

this PR:

image

Notes

This one is probably a bit more controversial than anti_aliasing because it's harder to move every post processing effect to this new crate. Also, the line between what is and isn't post processing isn't as clear. We might want to simply use a subfolder instead of a new crate.

@IceSentry IceSentry added A-Rendering Drawing game state to the screen C-Dependencies A change to the crates that Bevy depends on X-Controversial There is active debate or serious implications around merging this PR S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 19, 2025
Copy link
Contributor

You added a new feature but didn't update the readme. Please run cargo run -p build-templated-pages -- update features to update it, and commit the file change.

@JMS55
Copy link
Contributor

JMS55 commented Mar 19, 2025

This gets tricky.

Auto exposure shouldn't be here I don't think. DLSS is going to need to depend on it likely (haven't written that part yet). Which means that the anti aliasing crate would have to depend on the post processing crate.

Other potential issue, although I think you've handled this(?), is things like DOF and motion blur want to run before TAA/DLSS.

@IceSentry
Copy link
Contributor Author

The main limitation is the render graph being limited in how and when you can add nodes more than the crate itself. Right now things work because the PostProcessingPlugin gets insert bevy the AntiAliasing plugin. But it will start to break when they depend on each other. I'm fine with waiting on a better render graph. But I had the code written already so I opened the PR since for 0.16 it would help a bit with compile time.

Copy link
Contributor

@robtfm robtfm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this is worthwhile, even if other things end up depending on both crates it'll still be faster overall (since both compile faster than a single unit with everything in).

i haven't reviewed in detail since it's mostly copy paste. the node ordering looks the same and the examples seem fine.

approved pending ci obviously

@IceSentry IceSentry added this to the 0.17 milestone Mar 31, 2025
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-Dependencies A change to the crates that Bevy depends on S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Controversial There is active debate or serious implications around merging this PR
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants