Update winit dependency to 0.29 (#10702)

# Objective - Update winit dependency to 0.29 ## Changelog ### KeyCode changes - Removed `ScanCode`, as it was [replaced by KeyCode](https://github.com/rust-windowing/winit/blob/master/CHANGELOG.md#0292). - `ReceivedCharacter.char` is now a `SmolStr`, [relevant doc](https://docs.rs/winit/latest/winit/event/struct.KeyEvent.html#structfield.text). - Changed most `KeyCode` values, and added more. KeyCode has changed meaning. With this PR, it refers to physical position on keyboard rather than the printed letter on keyboard keys. In practice this means: - On QWERTY keyboard layouts, nothing changes - On any other keyboard layout, `KeyCode` no longer reflects the label on key. - This is "good". In bevy 0.12, when you used WASD for movement, users with non-QWERTY keyboards couldn't play your game! This was especially bad for non-latin keyboards. Now, WASD represents the physical keys. A French player will press the ZQSD keys, which are near each other, Kyrgyz players will use "Цфыв". - This is "bad" as well. You can't know in advance what the label of the key for input is. Your UI says "press WASD to move", even if in reality, they should be pressing "ZQSD" or "Цфыв". You also no longer can use `KeyCode` for text inputs. In any case, it was a pretty bad API for text input. You should use `ReceivedCharacter` now instead. ### Other changes - Use `web-time` rather than `instant` crate. (rust-windowing/winit#2836) - winit did split `run_return` in `run_onDemand` and `pump_events`, I did the same change in bevy_winit and used `pump_events`. - Removed `return_from_run` from `WinitSettings` as `winit::run` now returns on supported platforms. - I left the example "return_after_run" as I think it's still useful. - This winit change is done partly to allow to create a new window after quitting all windows: emilk/egui#1918 ; this PR doesn't address. - added `width` and `height` properties in the `canvas` from wasm example (bevyengine/bevy#10702 (comment)) ## Known regressions (important follow ups?) - Provide an API for reacting when a specific key from current layout was released. - possible solutions: use winit::Key from winit::KeyEvent ; mapping between KeyCode and Key ; or . - We don't receive characters through alt+numpad (e.g. alt + 151 = "ù") anymore ; reproduced on winit example "ime". maybe related to rust-windowing/winit#2945 - (windows) Window content doesn't refresh at all when resizing. By reading rust-windowing/winit#2900 ; I suspect we should just fire a `window.request_redraw();` from `AboutToWait`, and handle actual redrawing within `RedrawRequested`. I'm not sure how to move all that code so I'd appreciate it to be a follow up. - (windows) unreleased winit fix for using set_control_flow in AboutToWait rust-windowing/winit#3215 ; ⚠️ I'm not sure what the implications are, but that feels bad 🤔 ## Follow up I'd like to avoid bloating this PR, here are a few follow up tasks worthy of a separate PR, or new issue to track them once this PR is closed, as they would either complicate reviews, or at risk of being controversial: - remove CanvasParentResizePlugin (bevyengine/bevy#10702 (comment)) - avoid mentionning explicitly winit in docs from bevy_window ? - NamedKey integration on bevy_input: rust-windowing/winit#3143 introduced a new NamedKey variant. I implemented it only on the converters but we'd benefit making the same changes to bevy_input. - Add more info in KeyboardInput bevyengine/bevy#10702 (review) - bevyengine/bevy#9905 added a workaround on a bug allegedly fixed by winit 0.29. We should check if it's still necessary. - update to raw_window_handle 0.6 - blocked by wgpu - Rename `KeyCode` to `PhysicalKeyCode` bevyengine/bevy#10702 (comment) - remove `instant` dependency, [replaced by](rust-windowing/winit#2836) `web_time`), we'd need to update to : - fastrand >= 2.0 - [`async-executor`](https://github.com/smol-rs/async-executor) >= 1.7 - [`futures-lite`](https://github.com/smol-rs/futures-lite) >= 2.0 - Verify license, see [discussion](bevyengine/bevy#8745 (comment)) - we might be missing a short notice or description of changes made - Consider using https://github.com/rust-windowing/cursor-icon directly rather than vendoring it in bevy. - investigate [this unwrap](bevyengine/bevy#8745 (comment)) (`winit_window.canvas().unwrap();`) - Use more good things about winit's update - bevyengine/bevy#10689 (comment) ## Migration Guide This PR should have one.
Subserial · Dec 21, 2023 · 8b1ba8a · 8b1ba8a
1 parent 63811c9
commit 8b1ba8a
Show file tree

Hide file tree

Showing 7 changed files with 301 additions and 302 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -28,19 +28,25 @@ bevy_utils = { path = "../bevy_utils", version = "0.12.0" }
 bevy_tasks = { path = "../bevy_tasks", version = "0.12.0" }
 
 # other
-winit = { version = "0.28.7", default-features = false }
-accesskit_winit = { version = "0.15", default-features = false }
+# feature rwh_05 refers to window_raw_handle@v0.5,
+# updating to rwh_06 is blocked until wgpu 0.19 release lands.
+winit = { version = "0.29", default-features = false, features = ["rwh_05"] }
+accesskit_winit = { version = "0.17", default-features = false, features = [
+  "rwh_05",
+] }
 approx = { version = "0.5", default-features = false }
 raw-window-handle = "0.5"
 
 [target.'cfg(target_os = "android")'.dependencies]
-winit = { version = "0.28.7", default-features = false, features = [
+winit = { version = "0.29", default-features = false, features = [
   "android-native-activity",
+  "rwh_05",
 ] }
 
 [target.'cfg(target_arch = "wasm32")'.dependencies]
 wasm-bindgen = { version = "0.2" }
 web-sys = "0.3"
+
 crossbeam-channel = "0.5"
 
 [package.metadata.docs.rs]

diff --git a/src/converters.rs b/src/converters.rs
diff --git a/src/lib.rs b/src/lib.rs
@@ -15,6 +15,7 @@ mod winit_config;
 mod winit_windows;
 
 use bevy_a11y::AccessibilityRequested;
+use bevy_utils::{Duration, Instant};
 use system::{changed_windows, create_windows, despawn_windows, CachedWindow};
 pub use winit_config::*;
 pub use winit_windows::*;
@@ -32,10 +33,7 @@ use bevy_input::{
 use bevy_math::{ivec2, DVec2, Vec2};
 #[cfg(not(target_arch = "wasm32"))]
 use bevy_tasks::tick_global_task_pools_on_main_thread;
-use bevy_utils::{
-    tracing::{trace, warn},
-    Duration, Instant,
-};
+use bevy_utils::tracing::{error, trace, warn};
 use bevy_window::{
     exit_on_all_closed, ApplicationLifetime, CursorEntered, CursorLeft, CursorMoved,
     FileDragAndDrop, Ime, ReceivedCharacter, RequestRedraw, Window,
@@ -148,7 +146,9 @@ impl Plugin for WinitPlugin {
         #[cfg(target_arch = "wasm32")]
         app.add_plugins(CanvasParentResizePlugin);
 
-        let event_loop = event_loop_builder.build();
+        let event_loop = event_loop_builder
+            .build()
+            .expect("Failed to build event loop");
 
         // iOS, macOS, and Android don't like it if you create windows before the event loop is
         // initialized.
@@ -229,46 +229,6 @@ impl Plugin for WinitPlugin {
     }
 }
 
-fn run<F, T>(event_loop: EventLoop<T>, event_handler: F) -> !
-where
-    F: 'static + FnMut(Event<'_, T>, &EventLoopWindowTarget<T>, &mut ControlFlow),
-{
-    event_loop.run(event_handler)
-}
-
-#[cfg(any(
-    target_os = "windows",
-    target_os = "macos",
-    target_os = "linux",
-    target_os = "dragonfly",
-    target_os = "freebsd",
-    target_os = "netbsd",
-    target_os = "openbsd"
-))]
-fn run_return<F, T>(event_loop: &mut EventLoop<T>, event_handler: F)
-where
-    F: FnMut(Event<'_, T>, &EventLoopWindowTarget<T>, &mut ControlFlow),
-{
-    use winit::platform::run_return::EventLoopExtRunReturn;
-    event_loop.run_return(event_handler);
-}
-
-#[cfg(not(any(
-    target_os = "windows",
-    target_os = "macos",
-    target_os = "linux",
-    target_os = "dragonfly",
-    target_os = "freebsd",
-    target_os = "netbsd",
-    target_os = "openbsd"
-)))]
-fn run_return<F, T>(_event_loop: &mut EventLoop<T>, _event_handler: F)
-where
-    F: FnMut(Event<'_, T>, &EventLoopWindowTarget<T>, &mut ControlFlow),
-{
-    panic!("Run return is not supported on this platform!")
-}
-
 #[derive(SystemParam)]
 struct WindowAndInputEventWriters<'w> {
     // `winit` `WindowEvent`s
@@ -356,13 +316,11 @@ pub fn winit_runner(mut app: App) {
         app.cleanup();
     }
 
-    let mut event_loop = app
+    let event_loop = app
         .world
         .remove_non_send_resource::<EventLoop<()>>()
         .unwrap();
 
-    let return_from_run = app.world.resource::<WinitSettings>().return_from_run;
-
     app.world
         .insert_non_send_resource(event_loop.create_proxy());
 
@@ -406,9 +364,7 @@ pub fn winit_runner(mut app: App) {
     )> = SystemState::from_world(&mut app.world);
 
     // setup up the event loop
-    let event_handler = move |event: Event<()>,
-                              event_loop: &EventLoopWindowTarget<()>,
-                              control_flow: &mut ControlFlow| {
+    let event_handler = move |event: Event<()>, event_loop: &EventLoopWindowTarget<()>| {
         #[cfg(feature = "trace")]
         let _span = bevy_utils::tracing::info_span!("winit event_handler").entered();
 
@@ -423,7 +379,7 @@ pub fn winit_runner(mut app: App) {
 
             if let Some(app_exit_events) = app.world.get_resource::<Events<AppExit>>() {
                 if app_exit_event_reader.read(app_exit_events).last().is_some() {
-                    *control_flow = ControlFlow::Exit;
+                    event_loop.exit();
                     return;
                 }
             }
@@ -507,27 +463,15 @@ pub fn winit_runner(mut app: App) {
                 // the engine.
                 if let Some(adapter) = access_kit_adapters.get(&window_entity) {
                     if let Some(window) = winit_windows.get_window(window_entity) {
-                        // Somewhat surprisingly, this call has meaningful side effects
-                        // See https://github.com/AccessKit/accesskit/issues/300
-                        // AccessKit might later need to filter events based on this, but we currently do not.
-                        // See https://github.com/bevyengine/bevy/pull/10239#issuecomment-1775572176
-                        let _ = adapter.on_event(window, &event);
+                        adapter.process_event(window, &event);
                     }
                 }
 
                 runner_state.window_event_received = true;
 
                 match event {
                     WindowEvent::Resized(size) => {
-                        window
-                            .resolution
-                            .set_physical_resolution(size.width, size.height);
-
-                        event_writers.window_resized.send(WindowResized {
-                            window: window_entity,
-                            width: window.width(),
-                            height: window.height(),
-                        });
+                        react_to_resize(&mut window, size, &mut event_writers, window_entity);
                     }
                     WindowEvent::CloseRequested => {
                         event_writers
@@ -536,10 +480,18 @@ pub fn winit_runner(mut app: App) {
                                 window: window_entity,
                             });
                     }
-                    WindowEvent::KeyboardInput { ref input, .. } => {
-                        event_writers
-                            .keyboard_input
-                            .send(converters::convert_keyboard_input(input, window_entity));
+                    WindowEvent::KeyboardInput { ref event, .. } => {
+                        if event.state.is_pressed() {
+                            if let Some(char) = &event.text {
+                                event_writers.character_input.send(ReceivedCharacter {
+                                    window: window_entity,
+                                    char: char.clone(),
+                                });
+                            }
+                        }
+                        let keyboard_event =
+                            converters::convert_keyboard_input(event, window_entity);
+                        event_writers.keyboard_input.send(keyboard_event);
                     }
                     WindowEvent::CursorMoved { position, .. } => {
                         let physical_position = DVec2::new(position.x, position.y);
@@ -604,15 +556,9 @@ pub fn winit_runner(mut app: App) {
                             .touch_input
                             .send(converters::convert_touch_input(touch, location));
                     }
-                    WindowEvent::ReceivedCharacter(char) => {
-                        event_writers.character_input.send(ReceivedCharacter {
-                            window: window_entity,
-                            char,
-                        });
-                    }
                     WindowEvent::ScaleFactorChanged {
                         scale_factor,
-                        new_inner_size,
+                        mut inner_size_writer,
                     } => {
                         event_writers.window_backend_scale_factor_changed.send(
                             WindowBackendScaleFactorChanged {
@@ -625,13 +571,22 @@ pub fn winit_runner(mut app: App) {
                         window.resolution.set_scale_factor(scale_factor as f32);
                         let new_factor = window.resolution.scale_factor();
 
+                        let mut new_inner_size = winit::dpi::PhysicalSize::new(
+                            window.physical_width(),
+                            window.physical_height(),
+                        );
                         if let Some(forced_factor) = window.resolution.scale_factor_override() {
                             // This window is overriding the OS-suggested DPI, so its physical size
                             // should be set based on the overriding value. Its logical size already
                             // incorporates any resize constraints.
-                            *new_inner_size =
+                            let maybe_new_inner_size =
                                 winit::dpi::LogicalSize::new(window.width(), window.height())
                                     .to_physical::<u32>(forced_factor as f64);
+                            if let Err(err) = inner_size_writer.request_inner_size(new_inner_size) {
+                                warn!("Winit Failed to resize the window: {err}");
+                            } else {
+                                new_inner_size = maybe_new_inner_size;
+                            }
                         } else if approx::relative_ne!(new_factor, prior_factor) {
                             event_writers.window_scale_factor_changed.send(
                                 WindowScaleFactorChanged {
@@ -640,7 +595,6 @@ pub fn winit_runner(mut app: App) {
                                 },
                             );
                         }
-
                         let new_logical_width = new_inner_size.width as f32 / new_factor;
                         let new_logical_height = new_inner_size.height as f32 / new_factor;
                         if approx::relative_ne!(window.width(), new_logical_width)
@@ -807,10 +761,10 @@ pub fn winit_runner(mut app: App) {
 
                         app.world.entity_mut(entity).insert(wrapper);
                     }
-                    *control_flow = ControlFlow::Poll;
+                    event_loop.set_control_flow(ControlFlow::Poll);
                 }
             }
-            Event::MainEventsCleared => {
+            Event::AboutToWait => {
                 if runner_state.active.should_run() {
                     if runner_state.active == ActiveState::WillSuspend {
                         runner_state.active = ActiveState::Suspended;
@@ -822,7 +776,7 @@ pub fn winit_runner(mut app: App) {
                                 app.world.query_filtered::<Entity, With<PrimaryWindow>>();
                             let entity = query.single(&app.world);
                             app.world.entity_mut(entity).remove::<RawHandleWrapper>();
-                            *control_flow = ControlFlow::Wait;
+                            event_loop.set_control_flow(ControlFlow::Wait);
                         }
                     }
                     let (config, windows) = focused_windows_state.get(&app.world);
@@ -855,15 +809,17 @@ pub fn winit_runner(mut app: App) {
                         let (config, windows) = focused_windows_state.get(&app.world);
                         let focused = windows.iter().any(|window| window.focused);
                         match config.update_mode(focused) {
-                            UpdateMode::Continuous => *control_flow = ControlFlow::Poll,
+                            UpdateMode::Continuous => {
+                                event_loop.set_control_flow(ControlFlow::Poll);
+                            }
                             UpdateMode::Reactive { wait }
                             | UpdateMode::ReactiveLowPower { wait } => {
                                 if let Some(next) = runner_state.last_update.checked_add(*wait) {
                                     runner_state.scheduled_update = Some(next);
-                                    *control_flow = ControlFlow::WaitUntil(next);
+                                    event_loop.set_control_flow(ControlFlow::WaitUntil(next));
                                 } else {
                                     runner_state.scheduled_update = None;
-                                    *control_flow = ControlFlow::Wait;
+                                    event_loop.set_control_flow(ControlFlow::Wait);
                                 }
                             }
                         }
@@ -873,13 +829,13 @@ pub fn winit_runner(mut app: App) {
                         {
                             if redraw_event_reader.read(app_redraw_events).last().is_some() {
                                 runner_state.redraw_requested = true;
-                                *control_flow = ControlFlow::Poll;
+                                event_loop.set_control_flow(ControlFlow::Poll);
                             }
                         }
 
                         if let Some(app_exit_events) = app.world.get_resource::<Events<AppExit>>() {
                             if app_exit_event_reader.read(app_exit_events).last().is_some() {
-                                *control_flow = ControlFlow::Exit;
+                                event_loop.exit();
                             }
                         }
                     }
@@ -930,9 +886,24 @@ pub fn winit_runner(mut app: App) {
     };
 
     trace!("starting winit event loop");
-    if return_from_run {
-        run_return(&mut event_loop, event_handler);
-    } else {
-        run(event_loop, event_handler);
+    if let Err(err) = event_loop.run(event_handler) {
+        error!("winit event loop returned an error: {err}");
     }
 }
+
+fn react_to_resize(
+    window: &mut Mut<'_, Window>,
+    size: winit::dpi::PhysicalSize<u32>,
+    event_writers: &mut WindowAndInputEventWriters<'_>,
+    window_entity: Entity,
+) {
+    window
+        .resolution
+        .set_physical_resolution(size.width, size.height);
+
+    event_writers.window_resized.send(WindowResized {
+        window: window_entity,
+        width: window.width(),
+        height: window.height(),
+    });
+}
diff --git a/src/system.rs b/src/system.rs
@@ -27,7 +27,7 @@ use crate::{
         self, convert_enabled_buttons, convert_window_level, convert_window_theme,
         convert_winit_theme,
     },
-    get_best_videomode, get_fitting_videomode, WinitWindows,
+    get_best_videomode, get_fitting_videomode, WindowAndInputEventWriters, WinitWindows,
 };
 
 /// Creates new windows on the [`winit`] backend for each entity with a newly-added
@@ -138,6 +138,7 @@ pub struct CachedWindow {
 pub(crate) fn changed_windows(
     mut changed_windows: Query<(Entity, &mut Window, &mut CachedWindow), Changed<Window>>,
     winit_windows: NonSendMut<WinitWindows>,
+    mut event_writers: WindowAndInputEventWriters<'_>,
 ) {
     for (entity, mut window, mut cache) in &mut changed_windows {
         if let Some(winit_window) = winit_windows.get_window(entity) {
@@ -174,7 +175,9 @@ pub(crate) fn changed_windows(
                     window.resolution.physical_width(),
                     window.resolution.physical_height(),
                 );
-                winit_window.set_inner_size(physical_size);
+                if let Some(size_now) = winit_window.request_inner_size(physical_size) {
+                    crate::react_to_resize(&mut window, size_now, &mut event_writers, entity);
+                }
             }
 
             if window.physical_cursor_position() != cache.window.physical_cursor_position() {
@@ -298,10 +301,10 @@ pub(crate) fn changed_windows(
             }
 
             if window.ime_position != cache.window.ime_position {
-                winit_window.set_ime_position(LogicalPosition::new(
-                    window.ime_position.x,
-                    window.ime_position.y,
-                ));
+                winit_window.set_ime_cursor_area(
+                    LogicalPosition::new(window.ime_position.x, window.ime_position.y),
+                    PhysicalSize::new(10, 10),
+                );
             }
 
             if window.window_theme != cache.window.window_theme {

diff --git a/src/web_resize.rs b/src/web_resize.rs
@@ -31,7 +31,7 @@ fn canvas_parent_resize_event_handler(
 ) {
     for event in resize_events.receiver.try_iter() {
         if let Some(window) = winit_windows.get_window(event.window) {
-            window.set_inner_size(event.size);
+            let _ = window.request_inner_size(event.size);
         }
     }
 }