fix: rewrite winit loop (#12669)

# Objective

- Simplifies/clarifies the winit loop.
- Fixes #12612.

## Solution

The Winit loop runs following this flow:
* NewEvents
* Any number of other events, that can be 0, including RequestRedraw
* AboutToWait

Bevy also uses the UpdateMode, to define how the next loop has to run.
It can be essentially:
* Continuous, using ControlFlow::Wait for windowed apps, and
ControlFlow::Poll for windowless apps
* Reactive/ReactiveLowPower, using ControlFlow::WaitUntil with a
specific wait delay

The changes are made to follow this pattern, so that 
* NewEvents define if the WaitUntil has been canceled because we
received a Winit event.
* AboutToWait:
  * checks if the window has to be redrawn
  * otherwise calls app.update() if the WaitUntil timeout has elapsed
  * updates the ControlFlow accordingly

To make the code more logical:
* AboutToWait checks if any Bevy's RequestRedraw event has been emitted
* create_windows is run every cycle, at the beginning of the loop
* the ActiveState (that could be renamed ActivityState) is updated in
AboutToWait, symmetrically for WillSuspend/WillResume
* the AppExit events are checked every loop cycle, to exit the app early

## Platform-specific testing

- [x] Windows
- [x] MacOs
- [x] Linux (x11)
- [x] Linux (Wayland)
- [x] Android
- [x] iOS
- [x] WASM/WebGL2 (Chrome)
- [x] WASM/WebGL2 (Firefox)
- [x] WASM/WebGL2 (Safari)
- [x] WASM/WebGpu (Chrome)

---------

Co-authored-by: François <francois.mockers@vleue.com>

crates/bevy_winit/Cargo.toml

@@ -23,6 +23,7 @@ bevy_derive = { path = "../bevy_derive", version = "0.14.0-dev" }
 bevy_ecs = { path = "../bevy_ecs", version = "0.14.0-dev" }
 bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.14.0-dev" }
 bevy_input = { path = "../bevy_input", version = "0.14.0-dev" }
+bevy_log = { path = "../bevy_log", version = "0.14.0-dev" }
 bevy_math = { path = "../bevy_math", version = "0.14.0-dev" }
 bevy_reflect = { path = "../bevy_reflect", version = "0.14.0-dev" }
 bevy_window = { path = "../bevy_window", version = "0.14.0-dev" }
@@ -36,6 +37,7 @@ accesskit_winit = { version = "0.17", default-features = false, features = [
   "rwh_06",
 ] }
 approx = { version = "0.5", default-features = false }
+cfg-if = "1.0"
 raw-window-handle = "0.6"
 serde = { version = "1.0", features = ["derive"], optional = true }
 

crates/bevy_winit/src/lib.rs

@@ -184,8 +184,10 @@ impl AppSendEvent for Vec<WinitEvent> {
 /// Persistent state that is used to run the [`App`] according to the current
 /// [`UpdateMode`].
 struct WinitAppRunnerState {
-    /// Current active state of the app.
+    /// Current activity state of the app.
-    active: ActiveState,
+    activity_state: UpdateState,
+    /// Current update mode of the app.
+    update_mode: UpdateMode,
     /// Is `true` if a new [`WindowEvent`] has been received since the last update.
     window_event_received: bool,
     /// Is `true` if a new [`DeviceEvent`] has been received since the last update.
@@ -194,54 +196,51 @@ struct WinitAppRunnerState {
     redraw_requested: bool,
     /// Is `true` if enough time has elapsed since `last_update` to run another update.
     wait_elapsed: bool,
-    /// The time the last update started.
-    last_update: Instant,
     /// Number of "forced" updates to trigger on application start
     startup_forced_updates: u32,
 }
 
 impl WinitAppRunnerState {
     fn reset_on_update(&mut self) {
-        self.redraw_requested = false;
         self.window_event_received = false;
         self.device_event_received = false;
-        self.wait_elapsed = false;
-    }
-}
-
-#[derive(PartialEq, Eq)]
-enum ActiveState {
-    NotYetStarted,
-    Active,
-    Suspended,
-    WillSuspend,
-}
-
-impl ActiveState {
-    #[inline]
-    fn should_run(&self) -> bool {
-        match self {
-            ActiveState::NotYetStarted | ActiveState::Suspended => false,
-            ActiveState::Active | ActiveState::WillSuspend => true,
-        }
     }
 }
 
 impl Default for WinitAppRunnerState {
     fn default() -> Self {
         Self {
-            active: ActiveState::NotYetStarted,
+            activity_state: UpdateState::NotYetStarted,
+            update_mode: UpdateMode::Continuous,
             window_event_received: false,
             device_event_received: false,
             redraw_requested: false,
             wait_elapsed: false,
-            last_update: Instant::now(),
             // 3 seems to be enough, 5 is a safe margin
             startup_forced_updates: 5,
         }
     }
 }
 
+#[derive(PartialEq, Eq, Debug)]
+enum UpdateState {
+    NotYetStarted,
+    Active,
+    Suspended,
+    WillSuspend,
+    WillResume,
+}
+
+impl UpdateState {
+    #[inline]
+    fn is_active(&self) -> bool {
+        match self {
+            Self::NotYetStarted | Self::Suspended => false,
+            Self::Active | Self::WillSuspend | Self::WillResume => true,
+        }
+    }
+}
+
 /// The parameters of the [`create_windows`] system.
 pub type CreateWindowParams<'w, 's, F = ()> = (
     Commands<'w, 's>,
@@ -289,7 +288,7 @@ pub fn winit_runner(mut app: App) -> AppExit {
     // prepare structures to access data in the world
     let mut redraw_event_reader = ManualEventReader::<RequestRedraw>::default();
 
-    let mut focused_windows_state: SystemState<(Res<WinitSettings>, Query<&Window>)> =
+    let mut focused_windows_state: SystemState<(Res<WinitSettings>, Query<(Entity, &Window)>)> =
         SystemState::new(app.world_mut());
 
     let mut event_writer_system_state: SystemState<(
@@ -344,7 +343,7 @@ fn handle_winit_event(
         Query<(&mut Window, &mut CachedWindow)>,
         NonSend<AccessKitAdapters>,
     )>,
-    focused_windows_state: &mut SystemState<(Res<WinitSettings>, Query<&Window>)>,
+    focused_windows_state: &mut SystemState<(Res<WinitSettings>, Query<(Entity, &Window)>)>,
     redraw_event_reader: &mut ManualEventReader<RequestRedraw>,
     winit_events: &mut Vec<WinitEvent>,
     exit_status: &mut AppExit,
@@ -363,80 +362,178 @@ fn handle_winit_event(
             app.cleanup();
         }
         runner_state.redraw_requested = true;
-
-        if let Some(app_exit) = app.should_exit() {
-            *exit_status = app_exit;
-            event_loop.exit();
-            return;
-        }
     }
 
+    // create any new windows
+    // (even if app did not update, some may have been created by plugin setup)
+    create_windows(event_loop, create_window.get_mut(app.world_mut()));
+    create_window.apply(app.world_mut());
+
     match event {
         Event::AboutToWait => {
-            let (config, windows) = focused_windows_state.get(app.world());
+            if let Some(app_redraw_events) = app.world().get_resource::<Events<RequestRedraw>>() {
-            let focused = windows.iter().any(|window| window.focused);
+                if redraw_event_reader.read(app_redraw_events).last().is_some() {
-            let mut should_update = match config.update_mode(focused) {
+                    runner_state.redraw_requested = true;
-                UpdateMode::Continuous => {
-                    runner_state.redraw_requested
-                        || runner_state.window_event_received
-                        || runner_state.device_event_received
                 }
-                UpdateMode::Reactive { .. } => {
+            }
-                    runner_state.wait_elapsed
+
-                        || runner_state.redraw_requested
+            let (config, windows) = focused_windows_state.get(app.world());
-                        || runner_state.window_event_received
+            let focused = windows.iter().any(|(_, window)| window.focused);
-                        || runner_state.device_event_received
+
-                }
+            let mut update_mode = config.update_mode(focused);
-                UpdateMode::ReactiveLowPower { .. } => {
+            let mut should_update = should_update(runner_state, update_mode);
-                    runner_state.wait_elapsed
-                        || runner_state.redraw_requested
-                        || runner_state.window_event_received
-                }
-            };
 
-            // Ensure that an update is triggered on the first iterations for app initialization
             if runner_state.startup_forced_updates > 0 {
                 runner_state.startup_forced_updates -= 1;
+                // Ensure that an update is triggered on the first iterations for app initialization
                 should_update = true;
             }
 
-            // Trigger one last update to enter suspended state
+            if runner_state.activity_state == UpdateState::WillSuspend {
-            if runner_state.active == ActiveState::WillSuspend {
+                runner_state.activity_state = UpdateState::Suspended;
+                // Trigger one last update to enter the suspended state
                 should_update = true;
+
+                #[cfg(target_os = "android")]
+                {
+                    // Remove the `RawHandleWrapper` from the primary window.
+                    // This will trigger the surface destruction.
+                    let mut query = app
+                        .world_mut()
+                        .query_filtered::<Entity, With<PrimaryWindow>>();
+                    let entity = query.single(&app.world());
+                    app.world_mut()
+                        .entity_mut(entity)
+                        .remove::<RawHandleWrapper>();
+                }
             }
 
-            if should_update {
+            if runner_state.activity_state == UpdateState::WillResume {
-                let visible = windows.iter().any(|window| window.visible);
+                runner_state.activity_state = UpdateState::Active;
-                let (_, winit_windows, _, _) = event_writer_system_state.get_mut(app.world_mut());
+                // Trigger the update to enter the active state
-                if visible && runner_state.active != ActiveState::WillSuspend {
+                should_update = true;
-                    for window in winit_windows.windows.values() {
+                // Trigger the next redraw ro refresh the screen immediately
-                        window.request_redraw();
+                runner_state.redraw_requested = true;
-                    }
+
-                } else {
+                #[cfg(target_os = "android")]
-                    // there are no windows, or they are not visible.
+                {
-                    // Winit won't send events on some platforms, so trigger an update manually.
+                    // Get windows that are cached but without raw handles. Those window were already created, but got their
-                    run_app_update_if_should(
+                    // handle wrapper removed when the app was suspended.
-                        runner_state,
+                    let mut query = app
-                        app,
+                        .world_mut()
-                        focused_windows_state,
+                        .query_filtered::<(Entity, &Window), (With<CachedWindow>, Without<bevy_window::RawHandleWrapper>)>();
-                        event_loop,
+                    if let Ok((entity, window)) = query.get_single(&app.world()) {
-                        create_window,
+                        let window = window.clone();
-                        redraw_event_reader,
+
-                        winit_events,
+                        let (
-                        exit_status,
+                            ..,
-                    );
+                            mut winit_windows,
-                    if runner_state.active != ActiveState::Suspended {
+                            mut adapters,
-                        event_loop.set_control_flow(ControlFlow::Poll);
+                            mut handlers,
+                            accessibility_requested,
+                        ) = create_window.get_mut(app.world_mut());
+
+                        let winit_window = winit_windows.create_window(
+                            event_loop,
+                            entity,
+                            &window,
+                            &mut adapters,
+                            &mut handlers,
+                            &accessibility_requested,
+                        );
+
+                        let wrapper = RawHandleWrapper::new(winit_window).unwrap();
+
+                        app.world_mut().entity_mut(entity).insert(wrapper);
                     }
                 }
             }
+
+            // This is recorded before running app.update(), to run the next cycle after a correct timeout.
+            // If the cycle takes more than the wait timeout, it will be re-executed immediately.
+            let begin_frame_time = Instant::now();
+
+            if should_update {
+                // Not redrawing, but the timeout elapsed.
+                run_app_update(runner_state, app, winit_events);
+
+                // Running the app may have changed the WinitSettings resource, so we have to re-extract it.
+                let (config, windows) = focused_windows_state.get(app.world());
+                let focused = windows.iter().any(|(_, window)| window.focused);
+
+                update_mode = config.update_mode(focused);
+            }
+
+            match update_mode {
+                UpdateMode::Continuous => {
+                    // per winit's docs on [Window::is_visible](https://docs.rs/winit/latest/winit/window/struct.Window.html#method.is_visible),
+                    // we cannot use the visibility to drive rendering on these platforms
+                    // so we cannot discern whether to beneficially use `Poll` or not?
+                    cfg_if::cfg_if! {
+                        if #[cfg(not(any(
+                            target_arch = "wasm32",
+                            target_os = "android",
+                            target_os = "ios",
+                            all(target_os = "linux", any(feature = "x11", feature = "wayland"))
+                        )))]
+                        {
+                            let winit_windows = app.world().non_send_resource::<WinitWindows>();
+                            let visible = winit_windows.windows.iter().any(|(_, w)| {
+                                w.is_visible().unwrap_or(false)
+                            });
+
+                            event_loop.set_control_flow(if visible {
+                                ControlFlow::Wait
+                            } else {
+                                ControlFlow::Poll
+                            });
+                        }
+                        else {
+                            event_loop.set_control_flow(ControlFlow::Wait);
+                        }
+                    }
+
+                    // Trigger the next redraw to refresh the screen immediately if waiting
+                    if let ControlFlow::Wait = event_loop.control_flow() {
+                        runner_state.redraw_requested = true;
+                    }
+                }
+                UpdateMode::Reactive { wait } | UpdateMode::ReactiveLowPower { wait } => {
+                    // Set the next timeout, starting from the instant before running app.update() to avoid frame delays
+                    if let Some(next) = begin_frame_time.checked_add(wait) {
+                        if runner_state.wait_elapsed {
+                            event_loop.set_control_flow(ControlFlow::WaitUntil(next));
+                        }
+                    }
+                }
+            }
+
+            if update_mode != runner_state.update_mode {
+                // Trigger the next redraw since we're changing the update mode
+                runner_state.redraw_requested = true;
+                runner_state.update_mode = update_mode;
+            }
+
+            if runner_state.redraw_requested
+                && runner_state.activity_state != UpdateState::Suspended
+            {
+                let winit_windows = app.world().non_send_resource::<WinitWindows>();
+                for window in winit_windows.windows.values() {
+                    window.request_redraw();
+                }
+                runner_state.redraw_requested = false;
+            }
         }
         Event::NewEvents(cause) => {
             runner_state.wait_elapsed = match cause {
                 StartCause::WaitCancelled {
                     requested_resume: Some(resume),
                     ..
-                } => resume >= Instant::now(),
+                } => {
+                    // If the resume time is not after now, it means that at least the wait timeout
+                    // has elapsed.
+                    resume <= Instant::now()
+                }
                 _ => true,
             };
         }
@@ -643,16 +740,7 @@ fn handle_winit_event(
                     winit_events.send(WindowDestroyed { window });
                 }
                 WindowEvent::RedrawRequested => {
-                    run_app_update_if_should(
+                    run_app_update(runner_state, app, winit_events);
-                        runner_state,
-                        app,
-                        focused_windows_state,
-                        event_loop,
-                        create_window,
-                        redraw_event_reader,
-                        winit_events,
-                        exit_status,
-                    );
                 }
                 _ => {}
             }
@@ -675,56 +763,14 @@ fn handle_winit_event(
             winit_events.send(ApplicationLifetime::Suspended);
             // Mark the state as `WillSuspend`. This will let the schedule run one last time
             // before actually suspending to let the application react
-            runner_state.active = ActiveState::WillSuspend;
+            runner_state.activity_state = UpdateState::WillSuspend;
         }
         Event::Resumed => {
-            #[cfg(any(target_os = "android", target_os = "ios", target_os = "macos"))]
+            match runner_state.activity_state {
-            {
+                UpdateState::NotYetStarted => winit_events.send(ApplicationLifetime::Started),
-                if runner_state.active == ActiveState::NotYetStarted {
-                    create_windows(event_loop, create_window.get_mut(app.world_mut()));
-                    create_window.apply(app.world_mut());
-                }
-            }
-
-            match runner_state.active {
-                ActiveState::NotYetStarted => winit_events.send(ApplicationLifetime::Started),
                 _ => winit_events.send(ApplicationLifetime::Resumed),
             }
-            runner_state.active = ActiveState::Active;
+            runner_state.activity_state = UpdateState::WillResume;
-            runner_state.redraw_requested = true;
-            #[cfg(target_os = "android")]
-            {
-                // Get windows that are cached but without raw handles. Those window were already created, but got their
-                // handle wrapper removed when the app was suspended.
-                let mut query = app
-                        .world_mut()
-                        .query_filtered::<(Entity, &Window), (With<CachedWindow>, Without<bevy_window::RawHandleWrapper>)>();
-                if let Ok((entity, window)) = query.get_single(app.world()) {
-                    let window = window.clone();
-
-                    let (
-                        ..,
-                        mut winit_windows,
-                        mut adapters,
-                        mut handlers,
-                        accessibility_requested,
-                    ) = create_window.get_mut(app.world_mut());
-
-                    let winit_window = winit_windows.create_window(
-                        event_loop,
-                        entity,
-                        &window,
-                        &mut adapters,
-                        &mut handlers,
-                        &accessibility_requested,
-                    );
-
-                    let wrapper = RawHandleWrapper::new(winit_window).unwrap();
-
-                    app.world_mut().entity_mut(entity).insert(wrapper);
-                }
-                event_loop.set_control_flow(ControlFlow::Wait);
-            }
         }
         Event::UserEvent(RequestRedraw) => {
             runner_state.redraw_requested = true;
@@ -732,92 +778,45 @@ fn handle_winit_event(
         _ => (),
     }
 
+    if let Some(app_exit) = app.should_exit() {
+        *exit_status = app_exit;
+        event_loop.exit();
+        return;
+    }
+
     // We drain events after every received winit event in addition to on app update to ensure
     // the work of pushing events into event queues is spread out over time in case the app becomes
     // dormant for a long stretch.
     forward_winit_events(winit_events, app);
 }
 
-#[allow(clippy::too_many_arguments)]
+fn should_update(runner_state: &WinitAppRunnerState, update_mode: UpdateMode) -> bool {
-fn run_app_update_if_should(
+    let handle_event = match update_mode {
+        UpdateMode::Continuous | UpdateMode::Reactive { .. } => {
+            runner_state.wait_elapsed
+                || runner_state.window_event_received
+                || runner_state.device_event_received
+        }
+        UpdateMode::ReactiveLowPower { .. } => {
+            runner_state.wait_elapsed || runner_state.window_event_received
+        }
+    };
+
+    handle_event && runner_state.activity_state.is_active()
+}
+
+fn run_app_update(
     runner_state: &mut WinitAppRunnerState,
     app: &mut App,
-    focused_windows_state: &mut SystemState<(Res<WinitSettings>, Query<&Window>)>,
-    event_loop: &EventLoopWindowTarget<UserEvent>,
-    create_window: &mut SystemState<CreateWindowParams<Added<Window>>>,
-    redraw_event_reader: &mut ManualEventReader<RequestRedraw>,
     winit_events: &mut Vec<WinitEvent>,
-    exit_status: &mut AppExit,
 ) {
     runner_state.reset_on_update();
 
-    if !runner_state.active.should_run() {
-        return;
-    }
-
     forward_winit_events(winit_events, app);
 
-    if runner_state.active == ActiveState::WillSuspend {
-        runner_state.active = ActiveState::Suspended;
-        #[cfg(target_os = "android")]
-        {
-            // Remove the `RawHandleWrapper` from the primary window.
-            // This will trigger the surface destruction.
-            let mut query = app
-                .world_mut()
-                .query_filtered::<Entity, With<PrimaryWindow>>();
-            let entity = query.single(app.world());
-            app.world_mut()
-                .entity_mut(entity)
-                .remove::<RawHandleWrapper>();
-            event_loop.set_control_flow(ControlFlow::Wait);
-        }
-    }
-
     if app.plugins_state() == PluginsState::Cleaned {
-        runner_state.last_update = Instant::now();
-
         app.update();
-
-        // decide when to run the next update
-        let (config, windows) = focused_windows_state.get(app.world());
-        let focused = windows.iter().any(|window| window.focused);
-        match config.update_mode(focused) {
-            UpdateMode::Continuous => {
-                runner_state.redraw_requested = true;
-                event_loop.set_control_flow(ControlFlow::Wait);
-            }
-            UpdateMode::Reactive { wait } | UpdateMode::ReactiveLowPower { wait } => {
-                // TODO(bug): this is unexpected behavior.
-                // When Reactive, user expects bevy to actually wait that amount of time,
-                // and not potentially infinitely depending on platform specifics (which this does)
-                // Need to verify the platform specifics (whether this can occur in
-                // rare-but-possible cases) and replace this with a panic or a log warn!
-                if let Some(next) = runner_state.last_update.checked_add(*wait) {
-                    event_loop.set_control_flow(ControlFlow::WaitUntil(next));
-                } else {
-                    event_loop.set_control_flow(ControlFlow::Wait);
-                }
-            }
-        }
-
-        if let Some(app_redraw_events) = app.world().get_resource::<Events<RequestRedraw>>() {
-            if redraw_event_reader.read(app_redraw_events).last().is_some() {
-                runner_state.redraw_requested = true;
-            }
-        }
-
-        if let Some(app_exit) = app.should_exit() {
-            *exit_status = app_exit;
-            event_loop.exit();
-            return;
-        }
     }
-
-    // create any new windows
-    // (even if app did not update, some may have been created by plugin setup)
-    create_windows(event_loop, create_window.get_mut(app.world_mut()));
-    create_window.apply(app.world_mut());
 }
 
 fn react_to_resize(

crates/bevy_winit/src/system.rs

@@ -72,12 +72,14 @@ pub fn create_windows<F: QueryFilter + 'static>(
         window
             .resolution
             .set_scale_factor(winit_window.scale_factor() as f32);
-        commands
+
-            .entity(entity)
+        commands.entity(entity).insert(CachedWindow {
-            .insert(RawHandleWrapper::new(winit_window).unwrap())
+            window: window.clone(),
-            .insert(CachedWindow {
+        });
-                window: window.clone(),
+
-            });
+        if let Ok(handle_wrapper) = RawHandleWrapper::new(winit_window) {
+            commands.entity(entity).insert(handle_wrapper);
+        }
 
         #[cfg(target_arch = "wasm32")]
         {

crates/bevy_winit/src/winit_config.rs

@@ -44,10 +44,10 @@ impl WinitSettings {
     /// Returns the current [`UpdateMode`].
     ///
     /// **Note:** The output depends on whether the window has focus or not.
-    pub fn update_mode(&self, focused: bool) -> &UpdateMode {
+    pub fn update_mode(&self, focused: bool) -> UpdateMode {
         match focused {
-            true => &self.focused_mode,
+            true => self.focused_mode,
-            false => &self.unfocused_mode,
+            false => self.unfocused_mode,
         }
     }
 }
@@ -63,7 +63,7 @@ impl Default for WinitSettings {
 /// **Note:** This setting is independent of VSync. VSync is controlled by a window's
 /// [`PresentMode`](bevy_window::PresentMode) setting. If an app can update faster than the refresh
 /// rate, but VSync is enabled, the update rate will be indirectly limited by the renderer.
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq)]
 pub enum UpdateMode {
     /// The [`App`](bevy_app::App) will update over and over, as fast as it possibly can, until an
     /// [`AppExit`](bevy_app::AppExit) event appears.

examples/window/low_power.rs

@@ -6,8 +6,8 @@
 use bevy::{
     prelude::*,
     utils::Duration,
-    window::{PresentMode, RequestRedraw},
+    window::{PresentMode, RequestRedraw, WindowPlugin},
-    winit::WinitSettings,
+    winit::{EventLoopProxy, WinitSettings},
 };
 
 fn main() {
@@ -55,8 +55,8 @@ enum ExampleMode {
 /// Update winit based on the current `ExampleMode`
 fn update_winit(
     mode: Res<ExampleMode>,
-    mut event: EventWriter<RequestRedraw>,
     mut winit_config: ResMut<WinitSettings>,
+    event_loop_proxy: NonSend<EventLoopProxy>,
 ) {
     use ExampleMode::*;
     *winit_config = match *mode {
@@ -85,7 +85,9 @@ fn update_winit(
             // frame regardless of any user input. For example, your application might use
             // `WinitSettings::desktop_app()` to reduce power use, but UI animations need to play even
             // when there are no inputs, so you send redraw requests while the animation is playing.
-            event.send(RequestRedraw);
+            // Note that in this example the RequestRedraw winit event will make the app run in the same
+            // way as continuous
+            let _ = event_loop_proxy.send_event(RequestRedraw);
             WinitSettings::desktop_app()
         }
     };