Refactor window creation logic

[kristoff3r]

Objective

Fixes #21948

Currently we tell winit to spawn any new Windows 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 Windows, 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.

[IceSentry]

I think, if possible, the change to remove all the custom_user_event and generic stuff should be in a separate PR. The main reason for this is if we ever find a user that does need it then it's easier to only revert that change instead of all your changes together.

[kristoff3r]

I think, if possible, the change to remove all the custom_user_event and generic stuff should be in a separate PR. The main reason for this is if we ever find a user that does need it then it's easier to only revert that change instead of all your changes together.

That's not easily possible, this change uses the generic in EventLoop<T> to send internal messages now. Keeping the custom_user_event functionality would mean stuffing the user generic inside the new WinitUserMessage enum.

[IceSentry]

Ah, your comment said you were happy to re-add it so I interpreted that as being easy to do 😅

[kristoff3r]

Ah, your comment said you were happy to re-add it so I interpreted that as being easy to do 😅

It is easy, but it would have to build on top of this PR, as the 2 things conflict. I'd prefer if we figure out what we want first so I can make this PR do it, instead of having 2 conflicting PRs.

[kristoff3r]

winit apparently agrees with me, as they have also removed the generic user messages in the beta for version 0.31, see https://github.com/rust-windowing/winit/blob/master/winit/src/changelog/v0.31.md?plain=1#L77

[IceSentry]

Alright, well, I can't think of a reason to keep it anyway so LGTM.

This needs a migration guide though

[atlv24]

nice improvement

[tychedelia]

Thanks, this code is always gnarly.