Fix linux nvidia + xwayland freeze at startup #16123
Conversation
This seems to prevent the whole screen from freezing at startup when using using xwayland and nvidia.
alice-i-cecile
added
C-Bug
|
@Frost2779 can you test this for me? This should help with what you were seeing in #16120. |
alice-i-cecile
added
O-Linux
BenjaminBrienen
added
the
D-Straightforward
|
@alice-i-cecile I tested with the MRE from the original issue thread with 10 separate launches and did not encounter the unexpected behavior. Sprite rendered and systems seemed to function as expected. |
Just out of curiosity, did you test with |
|
I don't really get why we even have this if-else here when the two cases are identical except the else case handles extra stuff that already never happens in the first. #12542 here I also wanted to remove the if-else but didn't because people didn't want to review too much code, but I really do think it's better that way. |
| render_device.configure_surface(&data.surface, &data.configuration); | ||
| } | ||
|
|
||
| window_surfaces.configured_windows.insert(window.entity); |
There was a problem hiding this comment.
Marking windows configured fits better after configuring them instead of in the next system where they are configured only when outdated. The configured_windows is only used in this system and its condition now.
| Err(err) => { | ||
| // This is a common occurrence on X11 and Xwayland with NVIDIA drivers | ||
| // when opening and resizing the window. | ||
| warn!("Couldn't get swap chain texture after configuring. Cause: '{err}'"); |
There was a problem hiding this comment.
By removing the is_nvidia and linux cfg, this now means we don't panic on other platforms where we did before. The removal of the duplication seems to make sense to me, but I'm unsure (as in legitimately don't know) about making this change across all platforms rather than just targeting linux. Would like to better understand the history here.
There was a problem hiding this comment.
We don't have any platforms that instantly panic with an outdated surface, so this doesn't functionally change anything. Also it is better to handle errors instead of just panicing in general, clearly just having an outdated surface isn't fatal. It's also completely possible that some other platform also returns an error here in the future.
There was a problem hiding this comment.
Okay, that's good enough for me. Thanks!
It looks like here https://github.com/bevyengine/bevy/pull/12055/files#diff-b0a34efd7fd2e03ca78f181d1ecce355b927f07669a75501af11498f8b9ec3adL343 we did something extra in the if statement, but now it's not useful anymore. |
alice-i-cecile
added
S-Ready-For-Final-Review
# Objective - Fixes #16122 When the wayland feature is not enabled, xwayland is used on wayland. Nvidia drivers are somewhat bugged on linux and return outdated surfaces on xwayland for seemingly no reason. Oftentimes at startup we get into an infine loop where the surface is permanently outdated and nothing (or sometimes only the first frame) is drawn on the screen. ## Solution After experimenting I found that we can safely call configure again and the issue seems to resolve itsef. After this change I couldn't reproduce the original issue after many tries. More testing is probably needed though. The main issue is that `get_current_texture` fails sometimes because the surface remains outdated even after configuring. It would be better to just properly handle and never panic when `get_current_texture` fails. This way we always call configure when outdated and bail when getting the swapchain fails instead of crashing. The number of special cases is also reduced. ## Testing I tested the example "rotation" manually by trying to move around. It works with X11 and Xwayland and the non panicing code paths didn't change so other platforms aren't affected.
6 participants
Objective
When the wayland feature is not enabled, xwayland is used on wayland. Nvidia drivers are somewhat bugged on linux and return outdated surfaces on xwayland for seemingly no reason. Oftentimes at startup we get into an infine loop where the surface is permanently outdated and nothing (or sometimes only the first frame) is drawn on the screen.
Solution
After experimenting I found that we can safely call configure again and the issue seems to resolve itsef. After this change I couldn't reproduce the original issue after many tries. More testing is probably needed though.
The main issue is that
get_current_texturefails sometimes because the surface remains outdated even after configuring. It would be better to just properly handle and never panic whenget_current_texturefails. This way we always call configure when outdated and bail when getting the swapchain fails instead of crashing. The number of special cases is also reduced.Testing
I tested the example "rotation" manually by trying to move around.
It works with X11 and Xwayland and the non panicing code paths didn't change so other platforms aren't affected.