Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bevy UI example does not exit when window is closed. #5384

Closed
DGriffin91 opened this issue Jul 20, 2022 · 1 comment
Closed

Bevy UI example does not exit when window is closed. #5384

DGriffin91 opened this issue Jul 20, 2022 · 1 comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior

Comments

@DGriffin91
Copy link
Contributor

Bevy version

Bevy Main 7dcfaae

What you did

  • cargo run --example ui
  • Close the window

What went wrong

When closing the window, the bevy app continues to run even after the window is closed.

Additional information

Seems to be related to using WinitSettings::desktop_app(). If I comment this out the app exits as expected.

@DGriffin91 DGriffin91 added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jul 20, 2022
@alice-i-cecile alice-i-cecile added A-Windowing Platform-agnostic interface layer to run your app in A-UI Graphical user interfaces, styles, layouts, and widgets and removed S-Needs-Triage This issue needs to be labelled labels Jul 20, 2022
@spkjp
Copy link

spkjp commented Jul 20, 2022

It happens only for ReactiveLowPower, because it never sees the winit_state.low_power_event that is emitted in response to WindowEvent::CloseRequested.

let update = if winit_state.active {
let windows = app.world.resource::<Windows>();
let focused = windows.iter().any(|w| w.is_focused());
match winit_config.update_mode(focused) {
UpdateMode::Continuous | UpdateMode::Reactive { .. } => true,
UpdateMode::ReactiveLowPower { .. } => {
winit_state.low_power_event
|| winit_state.redraw_request_sent
|| winit_state.timeout_reached
}

Therefore update is false and nothing happens.

The root cause seems to be that the window is removed here

if !removed_windows.is_empty() {
for id in removed_windows {
// Close the OS window. (The `Drop` impl actually closes the window)
let _ = winit_windows.remove_window(id);
// Clean up our own data structures
windows.remove(id);
window_close_events.send(WindowClosed { id });
}

before the event loop can handle the low_power_event and thus bails out early as it cannot find a window:

let window_id =
if let Some(window_id) = winit_windows.get_window_id(winit_window_id) {
window_id
} else {
warn!(
"Skipped event for unknown winit Window Id {:?}",
winit_window_id
);
return;
};

This workaround seems to work for me, but it feels like a hack and not robust enough. There's probably a cleaner solution as the event loop just needs to run for the low_power_event before the windows are cleared.

diff --git a/crates/bevy_winit/src/lib.rs b/crates/bevy_winit/src/lib.rs
index 135ad332..47bf9882 100644
--- a/crates/bevy_winit/src/lib.rs
+++ b/crates/bevy_winit/src/lib.rs
@@ -331,6 +331,7 @@ pub fn winit_runner_with(mut app: App) {
         .0;
     let mut app_exit_event_reader = ManualEventReader::<AppExit>::default();
     let mut redraw_event_reader = ManualEventReader::<RequestRedraw>::default();
+    let mut window_closed_reader = ManualEventReader::<WindowClosed>::default();
     let mut winit_state = WinitPersistentState::default();
     app.world
         .insert_non_send_resource(event_loop.create_proxy());
@@ -608,6 +609,13 @@ pub fn winit_runner_with(mut app: App) {
                             winit_state.low_power_event
                                 || winit_state.redraw_request_sent
                                 || winit_state.timeout_reached
+                                || app
+                                    .world
+                                    .get_resource::<Events<WindowClosed>>()
+                                    .and_then(|window_closed_events| {
+                                        window_closed_reader.iter(window_closed_events).last()
+                                    })
+                                    .is_some()
                         }
                     }
                 } else {

@bors bors bot closed this as completed in 07d5769 Aug 3, 2022
maccesch pushed a commit to Synphonyte/bevy that referenced this issue Aug 5, 2022
# Objective

Fixes bevyengine#5384 and maybe other issues around window closing/app not exiting

## Solution

There are three systems involved in exiting when closing a window:
- `close_when_requested` asking Winit to close the window in stage `Update`
- `exit_on_all_closed` exiting when no window remains opened in stage `Update`
- `change_window` removing windows that are closed in stage `PostUpdate`

This ordering meant that when closing a window, we had to run one more frame to actually exit. As there was no window, panics could occur in systems assuming there was a window. In case of Bevy app using a low power options, that means waiting for the timeout before actually exiting the app (60 seconds by default)

This PR changes the ordering so that `exit_on_all_closed` happens after `change_window` in the same frame, so there isn't an extra frame without window
inodentry pushed a commit to IyesGames/bevy that referenced this issue Aug 8, 2022
# Objective

Fixes bevyengine#5384 and maybe other issues around window closing/app not exiting

## Solution

There are three systems involved in exiting when closing a window:
- `close_when_requested` asking Winit to close the window in stage `Update`
- `exit_on_all_closed` exiting when no window remains opened in stage `Update`
- `change_window` removing windows that are closed in stage `PostUpdate`

This ordering meant that when closing a window, we had to run one more frame to actually exit. As there was no window, panics could occur in systems assuming there was a window. In case of Bevy app using a low power options, that means waiting for the timeout before actually exiting the app (60 seconds by default)

This PR changes the ordering so that `exit_on_all_closed` happens after `change_window` in the same frame, so there isn't an extra frame without window
cart pushed a commit that referenced this issue Aug 19, 2022
# Objective

Fixes #5384 and maybe other issues around window closing/app not exiting

## Solution

There are three systems involved in exiting when closing a window:
- `close_when_requested` asking Winit to close the window in stage `Update`
- `exit_on_all_closed` exiting when no window remains opened in stage `Update`
- `change_window` removing windows that are closed in stage `PostUpdate`

This ordering meant that when closing a window, we had to run one more frame to actually exit. As there was no window, panics could occur in systems assuming there was a window. In case of Bevy app using a low power options, that means waiting for the timeout before actually exiting the app (60 seconds by default)

This PR changes the ordering so that `exit_on_all_closed` happens after `change_window` in the same frame, so there isn't an extra frame without window
james7132 pushed a commit to james7132/bevy that referenced this issue Oct 28, 2022
# Objective

Fixes bevyengine#5384 and maybe other issues around window closing/app not exiting

## Solution

There are three systems involved in exiting when closing a window:
- `close_when_requested` asking Winit to close the window in stage `Update`
- `exit_on_all_closed` exiting when no window remains opened in stage `Update`
- `change_window` removing windows that are closed in stage `PostUpdate`

This ordering meant that when closing a window, we had to run one more frame to actually exit. As there was no window, panics could occur in systems assuming there was a window. In case of Bevy app using a low power options, that means waiting for the timeout before actually exiting the app (60 seconds by default)

This PR changes the ordering so that `exit_on_all_closed` happens after `change_window` in the same frame, so there isn't an extra frame without window
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
# Objective

Fixes bevyengine#5384 and maybe other issues around window closing/app not exiting

## Solution

There are three systems involved in exiting when closing a window:
- `close_when_requested` asking Winit to close the window in stage `Update`
- `exit_on_all_closed` exiting when no window remains opened in stage `Update`
- `change_window` removing windows that are closed in stage `PostUpdate`

This ordering meant that when closing a window, we had to run one more frame to actually exit. As there was no window, panics could occur in systems assuming there was a window. In case of Bevy app using a low power options, that means waiting for the timeout before actually exiting the app (60 seconds by default)

This PR changes the ordering so that `exit_on_all_closed` happens after `change_window` in the same frame, so there isn't an extra frame without window
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants