mirror of
https://github.com/emilk/egui.git
synced 2026-09-03 15:20:05 -04:00
Fix panic in hit-test when a widget rect contains a NaN (#8479)
* Closes <https://github.com/emilk/egui/issues/7870> * [x] I have followed the instructions in the PR template `WidgetRect` derives `PartialEq`, so a rect with a NaN coordinate is not equal even to itself, and `close.iter().position(|w| *w == hit_click).unwrap()` panicked. We now look the hit up by id. NaN rects got that far because `Rect::intersect` scrubs NaNs (`f32::max(NAN, x) == x`), so `interact_rect` can be finite while `rect` is not, sneaking past the existing NaN guard. Hit-testing now discards those widgets too. Finally, `Pos2` claims `Eq` while NaN breaks reflexivity, so `PartialEq` is now hand-written and panics on NaN in debug builds.
This commit is contained in:
@@ -1257,6 +1257,8 @@ impl Context {
|
||||
allow_focus: bool,
|
||||
options: crate::InteractOptions,
|
||||
) -> Response {
|
||||
debug_assert!(!w.rect.any_nan(), "widget rect is NaN: {:?}", w.rect);
|
||||
|
||||
let interested_in_focus = w.enabled
|
||||
&& w.sense.is_focusable()
|
||||
&& self.memory(|mem| mem.allows_interaction(w.layer_id));
|
||||
|
||||
@@ -65,7 +65,7 @@ pub fn hit_test(
|
||||
.filter(|layer| layer.order.allow_interaction())
|
||||
.flat_map(|&layer_id| widgets.get_layer(layer_id))
|
||||
.filter(|&w| {
|
||||
if w.interact_rect.is_negative() || w.interact_rect.any_nan() {
|
||||
if w.interact_rect.is_negative() || w.rect.any_nan() || w.interact_rect.any_nan() {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -91,7 +91,10 @@ pub fn hit_test(
|
||||
}
|
||||
}
|
||||
|
||||
close.retain(|rect| !rect.interact_rect.any_nan()); // Protect against bad input and transforms
|
||||
// Protect against bad input and transforms.
|
||||
// NOTE: `Rect::intersect` scrubs NaNs (`f32::max(NAN, x) == x`),
|
||||
// so `interact_rect` can be finite even when `rect` is not.
|
||||
close.retain(|w| !w.rect.any_nan() && !w.interact_rect.any_nan());
|
||||
|
||||
// When using layer transforms it is common to stack layers close to each other.
|
||||
// For instance, you may have a resize-separator on a panel, with two
|
||||
@@ -359,11 +362,10 @@ fn hit_test_on_close(close: &[WidgetRect], pos: Pos2) -> WidgetHits {
|
||||
|
||||
(Some(hit_click), Some(hit_drag)) => {
|
||||
// We have a perfect hit on both click and drag. Which is the topmost?
|
||||
#[expect(clippy::unwrap_used)]
|
||||
let click_idx = close.iter().position(|w| *w == hit_click).unwrap();
|
||||
|
||||
#[expect(clippy::unwrap_used)]
|
||||
let drag_idx = close.iter().position(|w| *w == hit_drag).unwrap();
|
||||
// We look them up by id, because a `WidgetRect` with a NaN coordinate
|
||||
// is not equal even to itself.
|
||||
let click_idx = close.iter().position(|w| w.id == hit_click.id);
|
||||
let drag_idx = close.iter().position(|w| w.id == hit_drag.id);
|
||||
|
||||
let click_is_on_top_of_drag = drag_idx < click_idx;
|
||||
if click_is_on_top_of_drag {
|
||||
|
||||
@@ -12,7 +12,7 @@ use crate::{Div, Mul, Vec2, lerp};
|
||||
/// Mathematically this is known as a "point", but the term position was chosen so not to
|
||||
/// conflict with the unit (one point = X physical pixels).
|
||||
#[repr(C)]
|
||||
#[derive(Clone, Copy, Default, PartialEq)]
|
||||
#[derive(Clone, Copy, Default)]
|
||||
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
|
||||
#[cfg_attr(feature = "bytemuck", derive(bytemuck::Pod, bytemuck::Zeroable))]
|
||||
pub struct Pos2 {
|
||||
@@ -230,6 +230,21 @@ impl core::ops::IndexMut<usize> for Pos2 {
|
||||
}
|
||||
}
|
||||
|
||||
impl PartialEq for Pos2 {
|
||||
#[track_caller]
|
||||
#[inline]
|
||||
fn eq(&self, other: &Self) -> bool {
|
||||
debug_assert!(
|
||||
!self.any_nan() && !other.any_nan(),
|
||||
"Comparing NaN positions ({self:?} and {other:?}). \
|
||||
A NaN is not even equal to itself, which leads to very confusing bugs."
|
||||
);
|
||||
self.x == other.x && self.y == other.y
|
||||
}
|
||||
}
|
||||
|
||||
/// This is a lie for NaN positions, which are not equal to themselves.
|
||||
/// [`PartialEq`] catches those in debug builds.
|
||||
impl Eq for Pos2 {}
|
||||
|
||||
impl AddAssign<Vec2> for Pos2 {
|
||||
|
||||
@@ -10,7 +10,7 @@ use crate::Vec2b;
|
||||
///
|
||||
/// Normally the units are points (logical pixels).
|
||||
#[repr(C)]
|
||||
#[derive(Clone, Copy, Default, PartialEq)]
|
||||
#[derive(Clone, Copy, Default)]
|
||||
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
|
||||
#[cfg_attr(feature = "bytemuck", derive(bytemuck::Pod, bytemuck::Zeroable))]
|
||||
pub struct Vec2 {
|
||||
@@ -346,6 +346,21 @@ impl core::ops::IndexMut<usize> for Vec2 {
|
||||
}
|
||||
}
|
||||
|
||||
impl PartialEq for Vec2 {
|
||||
#[track_caller]
|
||||
#[inline]
|
||||
fn eq(&self, other: &Self) -> bool {
|
||||
debug_assert!(
|
||||
!self.any_nan() && !other.any_nan(),
|
||||
"Comparing NaN vectors ({self:?} and {other:?}). \
|
||||
A NaN is not even equal to itself, which leads to very confusing bugs."
|
||||
);
|
||||
self.x == other.x && self.y == other.y
|
||||
}
|
||||
}
|
||||
|
||||
/// This is a lie for NaN vectors, which are not equal to themselves.
|
||||
/// [`PartialEq`] catches those in debug builds.
|
||||
impl Eq for Vec2 {}
|
||||
|
||||
impl Neg for Vec2 {
|
||||
|
||||
Reference in New Issue
Block a user