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

Window despawns during Update schedule #13231

Closed
gcx11 opened this issue May 4, 2024 · 0 comments · Fixed by #13236
Closed

Window despawns during Update schedule #13231

gcx11 opened this issue May 4, 2024 · 0 comments · Fixed by #13236
Labels
C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled

Comments

@gcx11
Copy link

gcx11 commented May 4, 2024

Bevy version

5ee1b40

Relevant system information

Windows 11 + Vulkan

What you did

Close window with the close button when running following snippet

use bevy::app::{App, Startup, Update};
use bevy::DefaultPlugins;
use bevy::prelude::{Camera2dBundle, Commands, Query, Window};

fn setup(
    mut commands: Commands,
) {
    commands.spawn(Camera2dBundle::default());
}

fn check_window_status(
    window_query: Query<&Window>,
) {
    let window = window_query.get_single().unwrap();
    println!("Title: {}", window.title);
}

fn main() {
    let mut app = App::new();

    app.add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .add_systems(Update, check_window_status);

    app.run();
}

What went wrong

check_window_status triggers panic

thread 'Compute Task Pool (2)' panicked at src\main.rs:14:44:
called `Result::unwrap()` on an `Err` value: NoEntities("bevy_ecs::query::state::QueryState<&bevy_window::window::Window>")

Additional information

Previous commit (b8832dc) won't trigger panic

@gcx11 gcx11 added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels May 4, 2024
@gcx11 gcx11 changed the title Window despaws during Update schedule Window despawns during Update schedule May 4, 2024
tychedelia added a commit to tychedelia/bevy that referenced this issue May 4, 2024
Fixes two issues related to bevyengine#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 bevyengine#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.
    - 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 bevyengine#13231.
tychedelia added a commit to tychedelia/bevy that referenced this issue May 4, 2024
Fixes two issues related to bevyengine#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 bevyengine#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.
    - 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 bevyengine#13231.
tychedelia added a commit to tychedelia/bevy that referenced this issue May 4, 2024
Fixes two issues related to bevyengine#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 bevyengine#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.
    - 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 bevyengine#13231.
github-merge-queue bot pushed a commit that referenced this issue May 12, 2024
# 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant