-
-
Notifications
You must be signed in to change notification settings - Fork 4k
Use RenderStartup
for screen space reflections.
#20194
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
Conversation
.add_systems(Render, prepare_ssr_pipelines.in_set(RenderSystems::Prepare)) | ||
.add_systems( | ||
Render, | ||
prepare_ssr_settings.in_set(RenderSystems::PrepareResources), | ||
) | ||
// Note: we add this node here but then we add edges in | ||
// `add_screen_space_reflections_render_graph_edges`. | ||
.add_render_graph_node::<ViewNodeRunner<ScreenSpaceReflectionsNode>>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason why you didn't move this? Is it just to keep behaviour as close as possible? Oh, could it break something later if the node isn't added in build?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly to try to keep the diff small, and also because initializing ViewNodeRunner requires &mut World, so I was hoping to avoid requiring &mut World in the system.
In fact I was considering going the other way and instead moving the one required graph edge here, and then only do the conditional edge in the system. But alas I decided to try to keep the edges together at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, keeping the diff small is fine by me. We'll likely want to do a cleanup pass after 0.18 but it's probably good to keep everything as close as what it was before for now.
Objective
Solution
FromWorld
impls to systems inRenderStartup
.Testing
ssr
example and it still works.