Ensure clean exit

[tychedelia]

Objective

Fixes two issues related to #13208.

First, we ensure render resources for a window are always dropped first to ensure that the winit::Window always drops on the main thread when it is removed from WinitWindows. Previously, changes in #12978 caused the window to drop in the render world, causing issues.

We accomplish this by delaying despawning the window by a frame by inserting a marker component ClosingWindow that indicates the window has been requested to close and is in the process of closing. The render world now responds to the equivalent WindowClosing event rather than WindowCloseed which now fires after the render resources are guarunteed to be cleaned up.

Secondly, fixing the above caused (revealed?) that additional events were being delivered to the the event loop handler after exit had already been requested: in my testing RedrawRequested and LoopExiting. This caused errors to be reported try to send an exit event on the close channel. There are two options here:

• Guard the handler so no additional events are delivered once the app is exiting. I considered this but worried it might be confusing or bug prone if in the future someone wants to handle LoopExiting or some other event to clean-up while exiting. We are now taking this approach.
• Only send an exit signal if we are not already exiting. It doesn't appear to cause any problems to handle the extra events so this seems safer.

Fixing this also appears to have fixed #13231.

Fixes #10260.

Testing

Tested on mac only.

---

Changelog

Added

• A WindowClosing event has been added that indicates the window will be despawned on the next frame.

Changed

• Windows now close a frame after their exit has been requested.

Migration Guide

• Ensure custom exit logic does not rely on the app exiting the same frame as a window is closed.

[JMS55]

Is the latter issue with events firing after window close related to #13203?

[tychedelia]

Is the latter related to #13203?

Yup, this looks like it's trying to fix the same issue. Happy to remove in favor of that PR.

[BD103]

This works on my MacBook, but it will probably need testing on other platforms just in case.

[mockersf]

works for me, but there may be an issue with ordering in the logs:

2024-05-05T21:46:49.749344Z  INFO bevy_window::system: No windows are open, exiting
2024-05-05T21:46:49.750662Z  INFO bevy_winit::system: Closing window Entity { index: 0, generation: 1 }

I don't think this is sensitive for wasm or mobile, but testing on windows and linux would be good

[tychedelia]

works for me, but there may be an issue with ordering in the logs:

I think this is the same on main: exit_on_all_closed runs on PostUpdate and despawn_windows which releases the actual winit window runs on Last. They both respond to the Window component being de-spawned. But agree it could be more clear, it's really more like "No window components detected".

I don't think this is sensitive for wasm or mobile, but testing on windows and linux would be good

I'll test on windows later.

[Friz64]

Works fine on both Wayland and XWayland for me.

[tychedelia]

Looks like we have some warnings to clean up:

2024-05-06T21:08:35.419078Z  INFO bevy_window::system: No windows are open, exiting
2024-05-06T21:08:35.420490Z  INFO bevy_winit::system: Closing window Entity { index: 0, generation: 1 }
2024-05-06T21:08:35.424675Z  WARN bevy_winit: Skipped event ModifiersChanged(Modifiers { state: ModifiersState(0x0), pressed_mods: ModifiersKeys(0x0) }) for unknown winit Window Id WindowId(WindowId(1182052))
2024-05-06T21:08:35.424813Z  WARN bevy_winit: Skipped event Focused(false) for unknown winit Window Id WindowId(WindowId(1182052))
2024-05-06T21:08:35.429418Z  WARN bevy_winit: Skipped event Destroyed for unknown winit Window Id WindowId(WindowId(1182052))
2024-05-06T21:08:35.431869Z  INFO bevy_window::system: No windows are open, exiting

I think we need to prevent any events being delivered post exit. Right now the PR just skips sending a second exit signal.

[tychedelia]

This should be good now on windows. I changed the behavior to never deliver new events once we start exiting. #13203 does something similar, but I really think the simple approach is honestly fine. Happy to go either way.

[Brezak]

I think this PR should be merged instead of #13203. The only thing this PR is (debatably) missing is logging the winit events we have skipped.

[MiniaczQ]

Windows 11, double closing is fixed, no warnings about unread events.
Not sure if the order is considered a problem

[Friz64]

This fixes #10260.