fix: rewrite winit loop

[pietrosophya]

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)

[alice-i-cecile]

Does this also fix #12106?

[pietrosophya]

I don't have a Windows machine unfortunately.

I tested these successfully on MacOS:

• the game_menu example (using the Quit button)
• the ecs_guide example (windowless, it exits correctly when the game ends)
• the low_power example (using both CTRL+C from the command line, and the window close button)
• the desk_toy example (right click on the logo)

[BD103]

I tested this on my M1 Macbook, both by using ci and by running random examples. It all seems to work!

[alice-i-cecile]

@pietrosophya I've added a section on platform-specific testing to the PR description: please feel free to update the checklist as reviewer reports come in :)

[pietrosophya]

@alice-i-cecile thank you! I also tested WASM on both Chrome and Firefox, do I have to update the list or is it for reviewers only to check platforms off?

[alice-i-cecile]

Go ahead and check this off yourself :) I've also added Safari: I forgot that its web support is also different and weird.

[mockersf]

wasm/webgl2 support is mostly the same across browsers for the event loop, but wasm/webgpu needs to be tested too

[hymm]

Tested on windows 11 with most of the window example. Seems to work fine.

[kristoff3r]

Tested every example relating to windows and a few other ones on Linux with X11 and XMonad, no issues found.

[wolf-in-space]

Tested the window examples on Linux with Wayland and Sway, with the Wayland feature enabled and disabled: Only issue i noticed was transparent_window not working when the Wayland feature is enabled, but the issue is the same on main / 13.1, was also mentioned in #10929 already. Noticed no issues from this pr.

[BD103]

WASM works on Safari for WebGL2 :)

I was unable to get WASM for WebGPU working on Chrome--it just gave me a black screen--but I might be doing something wrong. Could someone else please test this?

[pietrosophya]

@BD103 I just updated the branch, a few days ago I fixed another bug on WebGPU that was causing the black screen. It works now.

[pietrosophya]

Solved latest main branch conflicts

[mockersf]

Android doesn't compile on this PR, https://github.com/Sophya/bevy/pull/1 should fix that

[mockersf]

on Windows, in CI, with this PR, it takes 6 minutes for the app to close after sending the event AppExit. This will likely break any attempt to merge because the job will timeout

[pietrosophya]

That's weird, since the loop is now super simple, and it checks for the AppExit event every time it runs the cycle. So, if Windows/CI takes that much to handle the AppExit event, I think the possible causes:

1. the loop is not running (or it's stuck for a long time)
2. the event is not sent
3. event_loop.exit() doesn't stop the loop (so a winit bug?)

Do you see any other possible reason? Do you know which test is taking 6 mins to run?

[mockersf]

Do you see any other possible reason? Do you know which test is taking 6 mins to run?

I didn't check every example, but all the ones I checked took around 6 minutes to exit

Also, doesn't work on Android:

https://github.com/bevyengine/bevy/assets/8672791/30065e3b-42e0-4942-9e45-7378a18ed027 Device logs.txt

[pietrosophya]

I fixed the android window setup. It's not perfect, I think it's still redundant in the way it creates the window and/or the RawWindowHandle.

NB: the app doesn't work in android in the same way it doesn't in the main branch, so I think it's unrelated to this PR.

[pietrosophya]

Can anyone help me debug why Windows/CI takes that much to close the app? I have a Mac only and no Windows license near me...

[mockersf]

works for me on iOS and Android 👍

I don't have a Windows either, I usually use Bevy modified CI action to run things on Windows

[msvbg]

Tested on Windows 10. It seems to keep the Window open until I move my mouse, at which point it immediately exits.

[Vrixyz]

I investigated a bit the issue on discord, I'm no expert in the winit event loop so take with a grain of salt:

• Original logs
  • I tried removing the return but that ends up in multiple Exiting events fired : incorrect behaviour logs
  • I tried moving the whole AppExit check after the match(event) produces seemingly correct logs (and exits without delay on windows)

Patch for my suggestion on moving AppExit check

diff --git a/crates/bevy_winit/src/lib.rs b/crates/bevy_winit/src/lib.rs
index 3be139185..161082464 100644
--- a/crates/bevy_winit/src/lib.rs
+++ b/crates/bevy_winit/src/lib.rs
@@ -354,13 +354,6 @@ fn handle_winit_event(
         runner_state.redraw_requested = true;
     }
 
-    if let Some(app_exit_events) = app.world().get_resource::<Events<AppExit>>() {
-        if app_exit_event_reader.read(app_exit_events).last().is_some() {
-            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()));
@@ -785,6 +778,13 @@ fn handle_winit_event(
         _ => (),
     }
 
+    if let Some(app_exit_events) = app.world().get_resource::<Events<AppExit>>() {
+        if app_exit_event_reader.read(app_exit_events).last().is_some() {
+            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.

I'm not sure that's the correct way of doing it, but I hope sharing my observations is useful 🤞

[pietrosophya]

I applied this change, it makes some sense since we check for new AppExit events after a cycle has run. But, generally speaking, the cycle should run every time, at most after the WaitUntil timeout.

I checked on Windows 10 Pro, and I wasn't able to reproduce it (I tested clear_color, low_power, multiple_windows, game_menu). How do I run the CI version?

[Vrixyz]

run CI_TESTING_CONFIG=.github/example-run/alien_cake_addict.ron cargo run --example load_gltf --features "bevy_ci_testing" and check how fast it exits without any input

[pietrosophya]

It closed immediately after 300 frames on Windows 10 Pro... Any other test I can run here?

[mockersf]

I triggered a full example run on this PR, it seems only example window_settings is not finishing in time now

see https://github.com/TheBevyFlock/bevy-example-runner/actions/runs/8587214466/job/23530567606

[pietrosophya]

I think I fixed this: basically, Windows takes some time to create a window, and the loop is put in Poll mode. Then, it was requesting a redraw all the time and never running any App update.

[BD103]

Does this introduce any breaking changes that would prevent it from being added to 0.13.3?

[pietrosophya]

No, it just changes the event loop internals without touching any API.

[extrawurst]

Would love to see this in a patch release