-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Fix crash on exit due to destroyed surface texture #21976
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?
Fix crash on exit due to destroyed surface texture #21976
Conversation
|
This is one of our oldest bugs! Can you check the issues quick and identify reports of this? |
|
@alice-i-cecile I searched through all open The issue this PR solves does not happen on 0.17 on my computer. |
|
This PR fixes a crash that recently started happening for me - same repro and error message as the OP. Bisected to #21866. |
|
I removed the linux label because it's not linux specific. I also get the crash on windows. As for the fix, it does avoid the crash but it feels a bit weird. I'm investigating a bit more to try and figure out what's going on. |
|
@IceSentry According to my investigation, prepared assets that are used to populate the But the Since there is no way for a render node to know if the Before #21866 the Relevant Change: |
| for (entity, camera, view, texture_usage, msaa) in cameras.iter() { | ||
| let (Some(target_size), Some(target)) = (camera.physical_target_size, &camera.target) | ||
| else { | ||
| commands.entity(entity).try_remove::<ViewTarget>(); |
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 this one is necessary? In my testing when it happens it doesn't cause a crash.
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 is required because when this happens it skips the check to resolve the view_target_attachment.
So if the user closes the window and removes the camera's target in the same frame then it will cause the same crash this PR is fixing.
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.
Ah, I guess I never ended up in that state while testing. It's a bit niche I guess but it doesn't hurt to keep it.
IceSentry
left a comment
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.
Other than adding a comment explaining why this needs to be done this works for me.
It was not obvious at all to me why this was even necessary.
Co-authored-by: IceSentry <IceSentry@users.noreply.github.com>
|
Yeah, sorry, my suggested comment used tabs and broke CI. You should be able to just run fmt and fix it. Also, feel free to change the comment to anything you prefer. |
Objective
Fix crash when a window is closed on current main
Crash log
Solution
ViewTargetwhen it becomes unavailableTesting