-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Render Recovery #22761
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
base: main
Are you sure you want to change the base?
Render Recovery #22761
Conversation
b896165 to
c848450
Compare
| .init_resource::<renderer::PendingCommandBuffers>() | ||
| .insert_resource(app.world().resource::<AssetServer>().clone()) | ||
| .insert_resource(RenderState::Initializing) | ||
| .add_systems(RenderRecovery, move |world: &mut World| { |
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.
I'm not sure I understand why a new schedule is needed here? What's wrong with using Render?
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.
this schedule runs Render, if it was Render it would infinite loop. the point is to gate running Render. notice RenderRecovery is private, also, so it cannot be circumvented. its an impl detail
| pub(crate) fn update(main_world: &mut World, render_world: &mut World) -> bool { | ||
| match render_world.resource::<RenderState>() { | ||
| RenderState::Initializing => { | ||
| render_world.insert_resource(RenderState::Ready); |
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.
Can we really just instantly transition from Initializing into Ready?
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.
Maybe the transition into Ready should be done by RenderStartup itself (in case it has its own fallibility).
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.
I think its better to keep all the state-machine-y things in the function together
Co-authored-by: Kristoffer Søholm <[email protected]>
Objective
Solution
wgpu::Device::set_device_lost_callbackandwgpu::Device::on_uncaptured_errorto listen for errors.RenderErrorHandlerto let users specify behavior on error by returning a specificRenderErrorPolicyTesting
Note: no release note yet, as recovery does not exactly work well: this PR gets us to the point of being able to care about it, but we currently instantly crash on recover due to gpu resources not existing anymore. We need to build more resilience before publicizing imo.