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.cursor_position can return Some when it shouldn't #8840

Closed
Plonq opened this issue Jun 14, 2023 · 2 comments · Fixed by #8855
Closed

Window.cursor_position can return Some when it shouldn't #8840

Plonq opened this issue Jun 14, 2023 · 2 comments · Fixed by #8855
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior

Comments

@Plonq
Copy link

Plonq commented Jun 14, 2023

Bevy version

0.10.1

What you did

Called the Window.cursor_position method after moving mouse from one window to another.

What went wrong

It intermittently returns Some(...) when it should be None.

Additional information

Using the small app below, this video shows the issue clearly. You can see that sometimes, but not always, when moving the cursor from one window directly to another window, the first window still thinks it has the cursor. This results in multiple windows claiming they have the cursor, while only one can at any one time.

I can see here Bevy sets the cursor position to None when it sees a WindowEvent::CursorLeft event from winit, but I verified that it is fired every time, even when the issue occurs, so I don't believe this is a bug in winit.

Screen.Recording.2023-06-14.at.7.26.15.pm.mov

Minimal app:

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins.set(WindowPlugin {
            primary_window: Some(Window {
                position: WindowPosition::At(IVec2::new(400, 200)),
                ..default()
            }),
            ..default()
        }))
        .add_startup_system(setup)
        .add_system(print_cursor_position)
        .run();
}

fn setup(mut commands: Commands) {
    commands.spawn(Window {
        title: "Second window".to_owned(),
        position: WindowPosition::At(IVec2::new(0, 0)),
        ..default()
    });
}

fn print_cursor_position(windows_q: Query<&Window>) {
    for window in windows_q.iter() {
        println!(
            "Window '{}' cursor position: {:?}",
            window.title,
            window.cursor_position()
        );
    }
}
@Plonq Plonq added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jun 14, 2023
@nicopap nicopap added A-Windowing Platform-agnostic interface layer to run your app in and removed S-Needs-Triage This issue needs to be labelled labels Jun 14, 2023
@Aceeri
Copy link
Member

Aceeri commented Jun 16, 2023

Just noticed this as well, put in a fix for it.

@Plonq
Copy link
Author

Plonq commented Jun 16, 2023

After a little more digging, I've discovered the problem is that sometimes a WindowEvent::CursorMoved event fires after the WindowEvent::CursorLeft event, which sets the cursor's position back to Some. I thought maybe in the moved handler, we could update the position of the cursor only if the existing position is Some, but that's not viable because the only way the cursor position is updated is from WindowEvent::CursorMoved events. If WindowEvent::CursorEntered included a position, we could use that, but alas it does not.

This feels like a winit bug after all, since the out-of-order events are coming from there.

github-merge-queue bot pushed a commit that referenced this issue Aug 28, 2023
# Objective
Fixes #8840 

Make the cursor position more consistent, right now the cursor position
is *sometimes* outside of the window and returns the position and
*sometimes* `None`.

Even in the cases where someone might be using that position that is
outside of the window, it'll probably require some manual
transformations for it to actually be useful.

## Solution
Check the windows width and height for out of bounds positions.

---

## Changelog
- Cursor position is now always `None` when outside of the window.

---------

Co-authored-by: ickshonpe <[email protected]>
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this issue Jan 9, 2024
# Objective
Fixes bevyengine#8840 

Make the cursor position more consistent, right now the cursor position
is *sometimes* outside of the window and returns the position and
*sometimes* `None`.

Even in the cases where someone might be using that position that is
outside of the window, it'll probably require some manual
transformations for it to actually be useful.

## Solution
Check the windows width and height for out of bounds positions.

---

## Changelog
- Cursor position is now always `None` when outside of the window.

---------

Co-authored-by: ickshonpe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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