From 1e3ecbefdb93522b65cdc090e296c2fc42904bb0 Mon Sep 17 00:00:00 2001
From: Knar <adbyerly@comcast.net>
Date: Sat, 9 Nov 2024 18:30:57 -0500
Subject: [PATCH] Handle failed cursor grab mode changes so that the cursor
 grab mode change can be attempted again (#16293)

# Objective

- Currently when you attempt to change the cursor_grab_mode it caches
the new value whether the cursor grab succeeded or failed. This change
handles the Result being returned by set_cursor_grab and changes the
cursor_grab_mode back to the cached version in case of an Error.
- Creates a way to handle #16237 and #16238

## Solution

- I changed the signature of winit_windows attempt_grab to return the
Result<(), ExternalError> that winit set_cursor_grab returns. The system
that calls attempt_grab now checks if there's an error returned, and if
there is it sets the grab_mode back to the cached version (similar to
what hit_test does a few lines down).

## Testing

- I tested using this system that previously would not correctly lock
the mouse on Ubuntu/x11
```
pub fn lock_mouse(mut primary_window: Query<&mut Window, With<PrimaryWindow>>) {
    let window = &mut primary_window.single_mut();
    if window.focused {
        window.cursor_options.grab_mode = CursorGrabMode::Confined;
    } else {
        window.cursor_options.grab_mode = CursorGrabMode::None;
    }
}
```
- I only tested on Ubuntu with x11
---
 crates/bevy_winit/src/system.rs        |  7 +++++--
 crates/bevy_winit/src/winit_windows.rs | 11 +++++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/crates/bevy_winit/src/system.rs b/crates/bevy_winit/src/system.rs
index 6348f050ad..f46f0b06ca 100644
--- a/crates/bevy_winit/src/system.rs
+++ b/crates/bevy_winit/src/system.rs
@@ -385,8 +385,11 @@ pub(crate) fn changed_windows(
             }
         }
 
-        if window.cursor_options.grab_mode != cache.window.cursor_options.grab_mode {
-            crate::winit_windows::attempt_grab(winit_window, window.cursor_options.grab_mode);
+        if window.cursor_options.grab_mode != cache.window.cursor_options.grab_mode
+            && crate::winit_windows::attempt_grab(winit_window, window.cursor_options.grab_mode)
+                .is_err()
+        {
+            window.cursor_options.grab_mode = cache.window.cursor_options.grab_mode;
         }
 
         if window.cursor_options.visible != cache.window.cursor_options.visible {
diff --git a/crates/bevy_winit/src/winit_windows.rs b/crates/bevy_winit/src/winit_windows.rs
index c4d76aa084..ff08b20b40 100644
--- a/crates/bevy_winit/src/winit_windows.rs
+++ b/crates/bevy_winit/src/winit_windows.rs
@@ -10,6 +10,7 @@ use bevy_window::{
 
 use winit::{
     dpi::{LogicalSize, PhysicalPosition},
+    error::ExternalError,
     event_loop::ActiveEventLoop,
     monitor::{MonitorHandle, VideoModeHandle},
     window::{CursorGrabMode as WinitCursorGrabMode, Fullscreen, Window as WinitWindow, WindowId},
@@ -279,7 +280,7 @@ impl WinitWindows {
 
         // Do not set the grab mode on window creation if it's none. It can fail on mobile.
         if window.cursor_options.grab_mode != CursorGrabMode::None {
-            attempt_grab(&winit_window, window.cursor_options.grab_mode);
+            let _ = attempt_grab(&winit_window, window.cursor_options.grab_mode);
         }
 
         winit_window.set_cursor_visible(window.cursor_options.visible);
@@ -380,7 +381,10 @@ pub fn get_best_videomode(monitor: &MonitorHandle) -> VideoModeHandle {
     modes.first().unwrap().clone()
 }
 
-pub(crate) fn attempt_grab(winit_window: &WinitWindow, grab_mode: CursorGrabMode) {
+pub(crate) fn attempt_grab(
+    winit_window: &WinitWindow,
+    grab_mode: CursorGrabMode,
+) -> Result<(), ExternalError> {
     let grab_result = match grab_mode {
         CursorGrabMode::None => winit_window.set_cursor_grab(WinitCursorGrabMode::None),
         CursorGrabMode::Confined => winit_window
@@ -398,6 +402,9 @@ pub(crate) fn attempt_grab(winit_window: &WinitWindow, grab_mode: CursorGrabMode
         };
 
         bevy_utils::tracing::error!("Unable to {} cursor: {}", err_desc, err);
+        Err(err)
+    } else {
+        Ok(())
     }
 }