Skip to content

Commit a166b65

Browse files
authored
Fix check that cursor position is within window bounds (#9662)
# Objective The recently introduced check that the cursor position returned by `Window::cursor_position()` is within the bounds of the window (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.
1 parent ae8a4a8 commit a166b65

File tree

1 file changed

+61
-10
lines changed

1 file changed

+61
-10
lines changed

crates/bevy_window/src/window.rs

Lines changed: 61 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use bevy_ecs::{
22
entity::{Entity, EntityMapper, MapEntities},
33
prelude::{Component, ReflectComponent},
44
};
5-
use bevy_math::{DVec2, IVec2, Rect, Vec2};
5+
use bevy_math::{DVec2, IVec2, Vec2};
66
use bevy_reflect::{std_traits::ReflectDefault, Reflect};
77

88
#[cfg(feature = "serialize")]
@@ -329,16 +329,12 @@ impl Window {
329329
pub fn physical_cursor_position(&self) -> Option<Vec2> {
330330
match self.internal.physical_cursor_position {
331331
Some(position) => {
332-
let position = position.as_vec2();
333-
if Rect::new(
334-
0.,
335-
0.,
336-
self.physical_width() as f32,
337-
self.physical_height() as f32,
338-
)
339-
.contains(position)
332+
if position.x >= 0.
333+
&& position.y >= 0.
334+
&& position.x < self.physical_width() as f64
335+
&& position.y < self.physical_height() as f64
340336
{
341-
Some(position)
337+
Some(position.as_vec2())
342338
} else {
343339
None
344340
}
@@ -1075,3 +1071,58 @@ impl Default for EnabledButtons {
10751071
}
10761072
}
10771073
}
1074+
1075+
#[cfg(test)]
1076+
mod tests {
1077+
use super::*;
1078+
1079+
// Checks that `Window::physical_cursor_position` returns the cursor position if it is within
1080+
// the bounds of the window.
1081+
#[test]
1082+
fn cursor_position_within_window_bounds() {
1083+
let mut window = Window {
1084+
resolution: WindowResolution::new(800., 600.),
1085+
..Default::default()
1086+
};
1087+
1088+
window.set_physical_cursor_position(Some(DVec2::new(0., 300.)));
1089+
assert_eq!(window.physical_cursor_position(), Some(Vec2::new(0., 300.)));
1090+
1091+
window.set_physical_cursor_position(Some(DVec2::new(400., 0.)));
1092+
assert_eq!(window.physical_cursor_position(), Some(Vec2::new(400., 0.)));
1093+
1094+
window.set_physical_cursor_position(Some(DVec2::new(799.999, 300.)));
1095+
assert_eq!(
1096+
window.physical_cursor_position(),
1097+
Some(Vec2::new(799.999, 300.)),
1098+
);
1099+
1100+
window.set_physical_cursor_position(Some(DVec2::new(400., 599.999)));
1101+
assert_eq!(
1102+
window.physical_cursor_position(),
1103+
Some(Vec2::new(400., 599.999))
1104+
);
1105+
}
1106+
1107+
// Checks that `Window::physical_cursor_position` returns `None` if the cursor position is not
1108+
// within the bounds of the window.
1109+
#[test]
1110+
fn cursor_position_not_within_window_bounds() {
1111+
let mut window = Window {
1112+
resolution: WindowResolution::new(800., 600.),
1113+
..Default::default()
1114+
};
1115+
1116+
window.set_physical_cursor_position(Some(DVec2::new(-0.001, 300.)));
1117+
assert!(window.physical_cursor_position().is_none());
1118+
1119+
window.set_physical_cursor_position(Some(DVec2::new(400., -0.001)));
1120+
assert!(window.physical_cursor_position().is_none());
1121+
1122+
window.set_physical_cursor_position(Some(DVec2::new(800., 300.)));
1123+
assert!(window.physical_cursor_position().is_none());
1124+
1125+
window.set_physical_cursor_position(Some(DVec2::new(400., 600.)));
1126+
assert!(window.physical_cursor_position().is_none());
1127+
}
1128+
}

0 commit comments

Comments
 (0)