Refactor window creation logic (#22088)

# Objective

Fixes #21948

Currently we tell winit to spawn any new `Window`s during the
`about_to_wait` lifecycle method, but that's unreliable: if no new
events appear while we're already waiting it could take an arbitrarily
long time, or no new events could appear at all so we never spawn the
window. This is especially bad when going from 0 to 1 `Window`s, as many
events are related to the existing window.

## Solution

Check for new windows in `resumed` for the initial window (which is also
what winit examples do) and in `user_event` for new windows.

I also did some other improvements while I was investigating this:
* Inlined the `Added<Window>` query filter into `CreateWindowParams`, as
the logic didn't make sense with any other filter.
* Insert the `EventLoopProxyWrapper` resource when the plugin builds
instead of inside the `winit_runner`. This currently makes no practical
difference, but in another version of this PR I had a crash because the
observer tried to use the resource before the runner started. It's also
a more natural way to do it.
* Made `WinitPlugin` no longer generic, and removed the
`custom_user_event` example. The functionality it provides (being able
to send events into bevy from the outside) only works in one direction,
and I don't see why it needs to go via winit instead of a dedicated
plugin with channels. ~~If this feature is useful for reasons I've
missed then I'm happy to re-add it.~~ edit: winit is removing custom
user events in 0.31, which made me even more confident it's the right
direction for us too.

## Testing

Run different examples and see that the window spawns in a timely
fashion, edit the `multiple_windows` example as in #21948 and see that
the window spawns.

Author: kristoff3r

Cargo.toml

@@ -3872,17 +3872,6 @@ description = "Demonstrates creating an animated custom cursor from an image"
 category = "Window"
 wasm = true
 
-[[example]]
-name = "custom_user_event"
-path = "examples/window/custom_user_event.rs"
-doc-scrape-examples = true
-
-[package.metadata.example.custom_user_event]
-name = "Custom User Event"
-description = "Handles custom user events within the event loop"
-category = "Window"
-wasm = true
-
 [[example]]
 name = "low_power"
 path = "examples/window/low_power.rs"

crates/bevy_internal/src/default_plugins.rs

@@ -29,6 +29,7 @@ plugin_group! {
         bevy_asset:::AssetPlugin,
         #[cfg(feature = "bevy_scene")]
         bevy_scene:::ScenePlugin,
+        // NOTE: WinitPlugin needs to be after AssetPlugin because of custom cursors.
         #[cfg(feature = "bevy_winit")]
         bevy_winit:::WinitPlugin,
         #[custom(cfg(all(feature = "dlss", not(feature = "force_disable_dlss"))))]

crates/bevy_winit/src/cursor/mod.rs

@@ -56,7 +56,7 @@ pub enum CursorSource {
 #[derive(Component, Debug)]
 pub struct PendingCursor(pub Option<CursorSource>);
 
-impl<M: Message> WinitAppRunnerState<M> {
+impl WinitAppRunnerState {
     pub(crate) fn update_cursors(
         &mut self,
         #[cfg(feature = "custom_cursor")] event_loop: &ActiveEventLoop,

crates/bevy_winit/src/lib.rs

@@ -15,11 +15,9 @@
 extern crate alloc;
 
 use bevy_derive::Deref;
-use bevy_reflect::prelude::ReflectDefault;
 use bevy_reflect::Reflect;
 use bevy_window::{RawHandleWrapperHolder, WindowEvent};
 use core::cell::RefCell;
-use core::marker::PhantomData;
 use winit::{event_loop::EventLoop, window::WindowId};
 
 use bevy_a11y::AccessibilityRequested;
@@ -71,7 +69,7 @@ thread_local! {
 /// When using eg. `MinimalPlugins` you can add this using `WinitPlugin::<WakeUp>::default()`, where
 /// `WakeUp` is the default event that bevy uses.
 #[derive(Default)]
-pub struct WinitPlugin<M: Message = WakeUp> {
+pub struct WinitPlugin {
     /// Allows the window (and the event loop) to be created on any thread
     /// instead of only the main thread.
     ///
@@ -82,16 +80,15 @@ pub struct WinitPlugin<M: Message = WakeUp> {
     /// Only works on Linux (X11/Wayland) and Windows.
     /// This field is ignored on other platforms.
     pub run_on_any_thread: bool,
-    marker: PhantomData<M>,
 }
 
-impl<T: Message> Plugin for WinitPlugin<T> {
+impl Plugin for WinitPlugin {
     fn name(&self) -> &str {
         "bevy_winit::WinitPlugin"
     }
 
     fn build(&self, app: &mut App) {
-        let mut event_loop_builder = EventLoop::<T>::with_user_event();
+        let mut event_loop_builder = EventLoop::<WinitUserEvent>::with_user_event();
 
         // linux check is needed because x11 might be enabled on other platforms.
         #[cfg(all(target_os = "linux", feature = "x11"))]
@@ -133,6 +130,7 @@ impl<T: Message> Plugin for WinitPlugin<T> {
         app.init_resource::<WinitMonitors>()
             .init_resource::<WinitSettings>()
             .insert_resource(DisplayHandleWrapper(event_loop.owned_display_handle()))
+            .insert_resource(EventLoopProxyWrapper(event_loop.create_proxy()))
             .add_message::<RawWinitWindowEvent>()
             .set_runner(|app| winit_runner(app, event_loop))
             .add_systems(
@@ -150,14 +148,40 @@ impl<T: Message> Plugin for WinitPlugin<T> {
 
         app.add_plugins(AccessKitPlugin);
         app.add_plugins(cursor::WinitCursorPlugin);
+
+        app.add_observer(
+            |_window: On<Add, Window>, event_loop_proxy: Res<EventLoopProxyWrapper>| -> Result {
+                event_loop_proxy.send_event(WinitUserEvent::WindowAdded)?;
+
+                Ok(())
+            },
+        );
     }
 }
 
-/// The default event that can be used to wake the window loop
-/// Wakes up the loop if in wait state
-#[derive(Debug, Default, Clone, Copy, Message, Reflect)]
-#[reflect(Debug, Default, Clone)]
-pub struct WakeUp;
+/// Events that can be sent to perform actions inside the winit event loop.
+///
+/// Sent via the [`EventLoopProxyWrapper`] resource.
+///
+/// # Example
+///
+/// ```
+/// # use bevy_ecs::prelude::*;
+/// # use bevy_winit::{EventLoopProxyWrapper, WinitUserEvent};
+/// fn wakeup_system(event_loop_proxy: Res<EventLoopProxyWrapper>) -> Result {
+///     event_loop_proxy.send_event(WinitUserEvent::WakeUp)?;
+///
+///     Ok(())
+/// }
+/// ```
+#[derive(Debug, Clone, Copy, Reflect)]
+#[reflect(Debug, Clone)]
+pub enum WinitUserEvent {
+    /// Dummy event that just wakes up the winit event loop
+    WakeUp,
+    /// Tell winit that a window needs to be created
+    WindowAdded,
+}
 
 /// The original window event as produced by Winit. This is meant as an escape
 /// hatch for power users that wish to add custom Winit integrations.
@@ -179,9 +203,9 @@ pub struct RawWinitWindowEvent {
 ///
 /// The `EventLoopProxy` can be used to request a redraw from outside bevy.
 ///
-/// Use `Res<EventLoopProxy>` to receive this resource.
+/// Use `Res<EventLoopProxyWrapper>` to retrieve this resource.
 #[derive(Resource, Deref)]
-pub struct EventLoopProxyWrapper<T: 'static>(EventLoopProxy<T>);
+pub struct EventLoopProxyWrapper(EventLoopProxy<WinitUserEvent>);
 
 /// A wrapper around [`winit::event_loop::OwnedDisplayHandle`]
 ///
@@ -203,7 +227,7 @@ impl AppSendEvent for Vec<WindowEvent> {
 }
 
 /// The parameters of the [`create_windows`] system.
-pub type CreateWindowParams<'w, 's, F = ()> = (
+pub type CreateWindowParams<'w, 's> = (
     Commands<'w, 's>,
     Query<
         'w,
@@ -214,7 +238,7 @@ pub type CreateWindowParams<'w, 's, F = ()> = (
             &'static CursorOptions,
             Option<&'static RawHandleWrapperHolder>,
         ),
-        F,
+        Added<Window>,
     >,
     MessageWriter<'w, WindowCreated>,
     ResMut<'w, WinitActionRequestHandlers>,

crates/bevy_winit/src/state.rs

@@ -17,7 +17,6 @@ use bevy_math::{ivec2, DVec2, Vec2};
 use bevy_platform::time::Instant;
 #[cfg(not(target_arch = "wasm32"))]
 use bevy_tasks::tick_global_task_pools_on_main_thread;
-use core::marker::PhantomData;
 #[cfg(target_arch = "wasm32")]
 use winit::platform::web::EventLoopExtWebSys;
 use winit::{
@@ -42,13 +41,13 @@ use crate::{
     accessibility::ACCESS_KIT_ADAPTERS,
     converters, create_windows,
     system::{create_monitors, CachedWindow, WinitWindowPressedKeys},
-    AppSendEvent, CreateMonitorParams, CreateWindowParams, EventLoopProxyWrapper,
-    RawWinitWindowEvent, UpdateMode, WinitSettings, WINIT_WINDOWS,
+    AppSendEvent, CreateMonitorParams, CreateWindowParams, RawWinitWindowEvent, UpdateMode,
+    WinitSettings, WinitUserEvent, WINIT_WINDOWS,
 };
 
 /// Persistent state that is used to run the [`App`] according to the current
 /// [`UpdateMode`].
-pub(crate) struct WinitAppRunnerState<T: Message> {
+pub(crate) struct WinitAppRunnerState {
     /// The running app.
     app: App,
     /// Exit value once the loop is finished.
@@ -78,7 +77,6 @@ pub(crate) struct WinitAppRunnerState<T: Message> {
     bevy_window_events: Vec<bevy_window::WindowEvent>,
     /// Raw Winit window events to send
     raw_winit_events: Vec<RawWinitWindowEvent>,
-    _marker: PhantomData<T>,
 
     message_writer_system_state: SystemState<(
         MessageWriter<'static, WindowResized>,
@@ -98,10 +96,8 @@ pub(crate) struct WinitAppRunnerState<T: Message> {
     scheduled_tick_start: Option<Instant>,
 }
 
-impl<M: Message> WinitAppRunnerState<M> {
+impl WinitAppRunnerState {
     fn new(mut app: App) -> Self {
-        app.add_message::<M>();
-
         let message_writer_system_state: SystemState<(
             MessageWriter<WindowResized>,
             MessageWriter<WindowBackendScaleFactorChanged>,
@@ -125,7 +121,6 @@ impl<M: Message> WinitAppRunnerState<M> {
             startup_forced_updates: 5,
             bevy_window_events: Vec::new(),
             raw_winit_events: Vec::new(),
-            _marker: PhantomData,
             message_writer_system_state,
             scheduled_tick_start: None,
         }
@@ -146,7 +141,7 @@ impl<M: Message> WinitAppRunnerState<M> {
     }
 }
 
-impl<M: Message> ApplicationHandler<M> for WinitAppRunnerState<M> {
+impl ApplicationHandler<WinitUserEvent> for WinitAppRunnerState {
     fn new_events(&mut self, event_loop: &ActiveEventLoop, cause: StartCause) {
         if event_loop.exiting() {
             return;
@@ -179,17 +174,31 @@ impl<M: Message> ApplicationHandler<M> for WinitAppRunnerState<M> {
         };
     }
 
-    fn resumed(&mut self, _event_loop: &ActiveEventLoop) {
+    fn resumed(&mut self, event_loop: &ActiveEventLoop) {
         // Mark the state as `WillResume`. This will let the schedule run one extra time
         // when actually resuming the app
         self.lifecycle = AppLifecycle::WillResume;
+
+        // Create the initial window if needed
+        let mut create_window = SystemState::<CreateWindowParams>::from_world(self.world_mut());
+        create_windows(event_loop, create_window.get_mut(self.world_mut()));
+        create_window.apply(self.world_mut());
     }
 
-    fn user_event(&mut self, _event_loop: &ActiveEventLoop, event: M) {
+    fn user_event(&mut self, event_loop: &ActiveEventLoop, event: WinitUserEvent) {
         self.user_event_received = true;
 
-        self.world_mut().write_message(event);
-        self.redraw_requested = true;
+        match event {
+            WinitUserEvent::WakeUp => {
+                self.redraw_requested = true;
+            }
+            WinitUserEvent::WindowAdded => {
+                let mut create_window =
+                    SystemState::<CreateWindowParams>::from_world(self.world_mut());
+                create_windows(event_loop, create_window.get_mut(self.world_mut()));
+                create_window.apply(self.world_mut());
+            }
+        }
     }
 
     fn window_event(
@@ -450,14 +459,8 @@ impl<M: Message> ApplicationHandler<M> for WinitAppRunnerState<M> {
 
     fn about_to_wait(&mut self, event_loop: &ActiveEventLoop) {
         let mut create_monitor = SystemState::<CreateMonitorParams>::from_world(self.world_mut());
-        // create any new windows
-        // (even if app did not update, some may have been created by plugin setup)
-        let mut create_window =
-            SystemState::<CreateWindowParams<Added<Window>>>::from_world(self.world_mut());
         create_monitors(event_loop, create_monitor.get_mut(self.world_mut()));
         create_monitor.apply(self.world_mut());
-        create_windows(event_loop, create_window.get_mut(self.world_mut()));
-        create_window.apply(self.world_mut());
 
         // TODO: This is a workaround for https://github.com/bevyengine/bevy/issues/17488
         //       while preserving the iOS fix in https://github.com/bevyengine/bevy/pull/11245
@@ -505,7 +508,7 @@ impl<M: Message> ApplicationHandler<M> for WinitAppRunnerState<M> {
     }
 }
 
-impl<M: Message> WinitAppRunnerState<M> {
+impl WinitAppRunnerState {
     fn redraw_requested(&mut self, event_loop: &ActiveEventLoop) {
         let mut redraw_message_cursor = MessageCursor::<RequestRedraw>::default();
         let mut close_message_cursor = MessageCursor::<WindowCloseRequested>::default();
@@ -877,15 +880,12 @@ impl<M: Message> WinitAppRunnerState<M> {
 ///
 /// Overriding the app's [runner](bevy_app::App::runner) while using `WinitPlugin` will bypass the
 /// `EventLoop`.
-pub fn winit_runner<M: Message>(mut app: App, event_loop: EventLoop<M>) -> AppExit {
+pub fn winit_runner(mut app: App, event_loop: EventLoop<WinitUserEvent>) -> AppExit {
     if app.plugins_state() == PluginsState::Ready {
         app.finish();
         app.cleanup();
     }
 
-    app.world_mut()
-        .insert_resource(EventLoopProxyWrapper(event_loop.create_proxy()));
-
     let runner_state = WinitAppRunnerState::new(app);
 
     trace!("starting winit event loop");

crates/bevy_winit/src/system.rs

@@ -7,7 +7,6 @@ use bevy_ecs::{
     lifecycle::RemovedComponents,
     message::MessageWriter,
     prelude::{Changed, Component},
-    query::QueryFilter,
     system::{Local, NonSendMarker, Query, SystemParamItem},
 };
 use bevy_input::keyboard::{Key, KeyCode, KeyboardFocusLost, KeyboardInput};
@@ -48,7 +47,7 @@ use crate::{
 ///
 /// If any of these entities are missing required components, those will be added with their
 /// default values.
-pub fn create_windows<F: QueryFilter + 'static>(
+pub fn create_windows(
     event_loop: &ActiveEventLoop,
     (
         mut commands,
@@ -57,7 +56,7 @@ pub fn create_windows<F: QueryFilter + 'static>(
         mut handlers,
         accessibility_requested,
         monitors,
-    ): SystemParamItem<CreateWindowParams<F>>,
+    ): SystemParamItem<CreateWindowParams>,
 ) {
     WINIT_WINDOWS.with_borrow_mut(|winit_windows| {
         ACCESS_KIT_ADAPTERS.with_borrow_mut(|adapters| {

examples/README.md

@@ -623,7 +623,6 @@ Example | Description
 --- | ---
 [Clear Color](../examples/window/clear_color.rs) | Creates a solid color window
 [Custom Cursor Image](../examples/window/custom_cursor_image.rs) | Demonstrates creating an animated custom cursor from an image
-[Custom User Event](../examples/window/custom_user_event.rs) | Handles custom user events within the event loop
 [Low Power](../examples/window/low_power.rs) | Demonstrates settings to reduce power use for bevy applications
 [Monitor info](../examples/window/monitor_info.rs) | Displays information about available monitors (displays).
 [Multiple Windows](../examples/window/multiple_windows.rs) | Demonstrates creating multiple windows, and rendering to them