bevy_post_process #20778
bevy_post_process #20778
Conversation
|
This was my original PR #18425 |
|
The shape of my PR and yours look roughly similar. I think the main difference is you handled 2D and I didn't 😅 Edit: After looking at it a bit more, yeah, it's almost identical lol |
|
Also, a bit of a bikeshed thing but I think I prefer calling it bevy_post_processing? Not sure yet. I just know I'm annoyed that we ues a mix of post_process and post_processing right now. I was also a bit unsure how to deal with the built in post processing stack because it makes naming things a bit weird. |
crates/bevy_post_process/src/lib.rs
Outdated
| pub mod dof; | ||
| pub mod motion_blur; | ||
| pub mod msaa_writeback; | ||
| pub mod post_process; |
There was a problem hiding this comment.
I have no idea how to solve this and this isn't a blocker but it's a bit annoying to name it this since now you need bevy::post_process::post_process::Something
There was a problem hiding this comment.
Maybe rename src/post_process/ to src/after_effect/ 😁
There was a problem hiding this comment.
mod post_process; only really has chromatic_abberation , so maybe we call it mod camera_effect; or similar
There was a problem hiding this comment.
maybe just name it chromatic aberration :p
There was a problem hiding this comment.
No, the main purpose of that thing is to combine multiple post processing effect in one node. The fact that it's only chromatic aberration right now is just because nobody has implemented anything else yet. Things like film grain or vignette or whatever else should be in there eventually.
There was a problem hiding this comment.
maybe we can call it EffectStack or MultiEffect?
|
Imo bevy_anti_aliasing should have been named bevy_anti_alias, for the same reasons we didnt name bevy_render bevy_rendering or bevy_log bevy_logging. I'd even go as far as saying bevy_animation should have been called bevy_animate, because bevy_reflect wasn't called bevy_reflection. I also think bevy_picking should be called bevy_pick. (i hate superfluous suffixes) |
|
Yeah, I'm fine with either really. The annoying part is how we have a mix of both right now |
|
What i dont know is how to distinguish bevy_post_process::PostProcessPlugin vs bevy_post_process::post_process::PostProcessingPlugin if we standardize. Maybe we rename the inner one to PostProcessStackPlugin? |
alice-i-cecile
added
A-Rendering
| use bevy_app::{App, Plugin}; | ||
|
|
||
| #[derive(Default)] | ||
| pub struct PostProcessPlugin; |
There was a problem hiding this comment.
Two complaints:
a) this should be a plugin group
b) this should document what's in it
atlv24
force-pushed
the
ad/post-process
branch
from
caf8b91 to
52e6ee3
Compare
Objective
Split out post process effects from bevy_core_pipeline because they are not core pipelines.
Solution
@IceSentry proposed something like this, not sure if the split is exactly as he envisioned but this seems reasonable to me.
The goal is to move absolutely everything possible out of bevy_core_pipelines to unblock bevy_pbr/bevy_sprite_render compilation.
Future PRs may attempt to move more little bits out.
Testing