Skip to content

Treat unready Wayland windows as focused #8953

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+36 −6
Open

Treat unready Wayland windows as focused #8953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d33d44
Add `ready` field to `InternalWindowState`
geieredgar Jun 25, 2023
8a530c9
Fix long startup time when using X11 through XWayland
geieredgar Jun 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions crates/bevy_window/src/window.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,10 @@ pub struct InternalWindowState {
maximize_request: Option<bool>,
/// Unscaled cursor position.
physical_cursor_position: Option<DVec2>,
/// Flag that can be freely used by window backends to distinguish between ready and unready windows.
/// `bevy_winit` considers Wayland windows to be unready until they are visible on screen and ignores
/// `WindowEvent::Focused(false)` events for unready windows.
ready: bool,
}

impl InternalWindowState {
Expand All @@ -765,6 +769,16 @@ impl InternalWindowState {
pub fn take_minimize_request(&mut self) -> Option<bool> {
self.minimize_request.take()
}

/// Marks the window as ready. This should only be called by window backends.
pub fn set_ready(&mut self) {
self.ready = true;
}

/// Checks if the window has been marked as ready. This should only be called by window backends.
pub fn is_ready(&self) -> bool {
self.ready
}
}

/// References a screen monitor.
Expand Down
22 changes: 16 additions & 6 deletions crates/bevy_winit/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,13 +578,23 @@ pub fn winit_runner(mut app: App) {
.set_physical_resolution(new_inner_size.width, new_inner_size.height);
}
WindowEvent::Focused(focused) => {
// Component
window.focused = focused;
// When using Wayland (or sometimes X11 through XWayland) we need to consider the window
// focused after creation and ignore the initial `WindowEvent::Focused(false)` event.
// See https://github.com/bevyengine/bevy/pull/8953 for more details.
#[cfg(any(feature = "wayland", feature = " x11"))]
if focused && !window.internal.is_ready() {
window.internal.set_ready();
}

window_events.window_focused.send(WindowFocused {
window: window_entity,
focused,
});
if window.internal.is_ready() {
// Component
window.focused = focused;

window_events.window_focused.send(WindowFocused {
window: window_entity,
focused,
});
}
}
WindowEvent::DroppedFile(path_buf) => {
file_drag_and_drop_events.send(FileDragAndDrop::DroppedFile {
Expand Down
6 changes: 6 additions & 0 deletions crates/bevy_winit/src/system.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ pub(crate) fn create_window<'a>(
&mut accessibility_requested,
);

// When using Wayland (or sometimes X11 through XWayland) we need to consider the window
// focused after creation and ignore the initial `WindowEvent::Focused(false)` event.
// See https://github.com/bevyengine/bevy/pull/8953 for more details.
#[cfg(not(any(feature = "wayland", feature = "x11")))]
window.internal.set_ready();
Copy link

@JayceFayne JayceFayne Jun 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this line seems to fix wayland as well as x11 (xwayland)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply removing this line would be incorrect, because then the windows would always be considered focused. If we can reassure that Winit will always send a WindowEvent::Focused(true) event after creating the window, we could replace feature = "wayland with any(feature = "wayland", feature = "x11").

However, I couldn't reproduce the long startup time bug using x11 on my machine (XWayland Version 23.1.2).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ic

I am using the same xwayland version and I can reproduce the long startup time on sway (version 1.8.1)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I could also reproduce the issue when using X11 through XWayland on Sway. but not when using X11 through XWayland on Gnome.

I tried out treating X11 the same as Wayland and tested X11 on

  • Gnome (Xorg)
  • Gnome (Wayland) using XWayland
  • Sway using XWayland

The long startup time was fixed in all three cases, but Sway showed a small delay (around 1 second) which I believe had to do with the fact that I have an Nvidia GPU and had to use the --unsupported-gpu option. It would be nice if you could confirm that there is no issue using X11 on Sway with the latest commit.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this! With the latest commit running a xwayland bevy app has no startup delay.

The long startup time was fixed in all three cases, but Sway showed a small delay (around 1 second) which I believe had to do with the fact that I have an Nvidia GPU and had to use the --unsupported-gpu option. It would be nice if you could confirm that there is no issue using X11 on Sway with the latest commit.

I don't think this is a Nvidia GPU problem. I have an AMD GPU and experience around 4 sec startup delay with a native wayland bevy app. This delay does not exist on xwayland bevy for me.


if let Some(theme) = winit_window.theme() {
window.window_theme = Some(convert_winit_theme(theme));
}
Expand Down