-
-
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
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: |
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. |
|
@IceSentry Cleaned up the logic a little bit, this seems better to me |
|
Yeah, it's much better now |
tychedelia
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.
Makes sense. LGTM!
Objective
Fix crash when a window is closed on current main
Crash log
Solution
ViewTargetwhen it becomes unavailableTesting