Skip to content

Commit

Permalink
Fix check that cursor position is within window bounds (bevyengine#9662)
Browse files Browse the repository at this point in the history
# Objective

The recently introduced check that the cursor position returned by
`Window::cursor_position()` is within the bounds of the window
(bevyengine@3cf94e7)
has the following issue:

If *w* is the window width, points within the window satisfy the
condition 0 ≤ *x* < *w*, but the code assumes the condition 0 ≤ *x* ≤
*w*. In other words, if *x* = *w*, the point is not within the window
bounds. Likewise for the height. This program demonstrates the issue:

```rust
use bevy::{prelude::*, window::WindowResolution};

fn main() {
    let mut window = Window {
        resolution: WindowResolution::new(100.0, 100.0),
        ..default()
    };

    window.set_cursor_position(Some(Vec2::new(100.0, 0.0)));

    println!("{:?}", window.cursor_position());
}
```

It prints `Some(Vec2(100.0, 0.0))` instead of the expected `None`.

## Solution

- Exclude the upper bound, i.e., the window width for the *x* position
and the window height for the *y* position.
  • Loading branch information
mdickopp authored and Ray Redondo committed Jan 9, 2024
1 parent 44917ba commit d394ce7
Showing 1 changed file with 61 additions and 10 deletions.
71 changes: 61 additions & 10 deletions crates/bevy_window/src/window.rs
Expand Up @@ -2,7 +2,7 @@ use bevy_ecs::{
entity::{Entity, EntityMapper, MapEntities},
prelude::{Component, ReflectComponent},
};
use bevy_math::{DVec2, IVec2, Rect, Vec2};
use bevy_math::{DVec2, IVec2, Vec2};
use bevy_reflect::{std_traits::ReflectDefault, Reflect};

#[cfg(feature = "serialize")]
Expand Down Expand Up @@ -329,16 +329,12 @@ impl Window {
pub fn physical_cursor_position(&self) -> Option<Vec2> {
match self.internal.physical_cursor_position {
Some(position) => {
let position = position.as_vec2();
if Rect::new(
0.,
0.,
self.physical_width() as f32,
self.physical_height() as f32,
)
.contains(position)
if position.x >= 0.
&& position.y >= 0.
&& position.x < self.physical_width() as f64
&& position.y < self.physical_height() as f64
{
Some(position)
Some(position.as_vec2())
} else {
None
}
Expand Down Expand Up @@ -1075,3 +1071,58 @@ impl Default for EnabledButtons {
}
}
}

#[cfg(test)]
mod tests {
use super::*;

// Checks that `Window::physical_cursor_position` returns the cursor position if it is within
// the bounds of the window.
#[test]
fn cursor_position_within_window_bounds() {
let mut window = Window {
resolution: WindowResolution::new(800., 600.),
..Default::default()
};

window.set_physical_cursor_position(Some(DVec2::new(0., 300.)));
assert_eq!(window.physical_cursor_position(), Some(Vec2::new(0., 300.)));

window.set_physical_cursor_position(Some(DVec2::new(400., 0.)));
assert_eq!(window.physical_cursor_position(), Some(Vec2::new(400., 0.)));

window.set_physical_cursor_position(Some(DVec2::new(799.999, 300.)));
assert_eq!(
window.physical_cursor_position(),
Some(Vec2::new(799.999, 300.)),
);

window.set_physical_cursor_position(Some(DVec2::new(400., 599.999)));
assert_eq!(
window.physical_cursor_position(),
Some(Vec2::new(400., 599.999))
);
}

// Checks that `Window::physical_cursor_position` returns `None` if the cursor position is not
// within the bounds of the window.
#[test]
fn cursor_position_not_within_window_bounds() {
let mut window = Window {
resolution: WindowResolution::new(800., 600.),
..Default::default()
};

window.set_physical_cursor_position(Some(DVec2::new(-0.001, 300.)));
assert!(window.physical_cursor_position().is_none());

window.set_physical_cursor_position(Some(DVec2::new(400., -0.001)));
assert!(window.physical_cursor_position().is_none());

window.set_physical_cursor_position(Some(DVec2::new(800., 300.)));
assert!(window.physical_cursor_position().is_none());

window.set_physical_cursor_position(Some(DVec2::new(400., 600.)));
assert!(window.physical_cursor_position().is_none());
}
}

0 comments on commit d394ce7

Please sign in to comment.