From 1a9d2af9efde043883036a6c0fc52423df435398 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 3 Sep 2026 15:16:14 +0200 Subject: [PATCH] Fix panic in hit-test when a widget rect contains a NaN (#8479) * Closes * [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. --- crates/egui/src/context.rs | 2 ++ crates/egui/src/hit_test.rs | 16 +++++++++------- crates/emath/src/pos2.rs | 17 ++++++++++++++++- crates/emath/src/vec2.rs | 17 ++++++++++++++++- 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index ac307b0b5..b83ba4bf4 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -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)); diff --git a/crates/egui/src/hit_test.rs b/crates/egui/src/hit_test.rs index f48c409b0..2be646b42 100644 --- a/crates/egui/src/hit_test.rs +++ b/crates/egui/src/hit_test.rs @@ -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 { diff --git a/crates/emath/src/pos2.rs b/crates/emath/src/pos2.rs index d331769dc..ce56bddd1 100644 --- a/crates/emath/src/pos2.rs +++ b/crates/emath/src/pos2.rs @@ -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 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 for Pos2 { diff --git a/crates/emath/src/vec2.rs b/crates/emath/src/vec2.rs index 5422aa79b..7cf38e53e 100644 --- a/crates/emath/src/vec2.rs +++ b/crates/emath/src/vec2.rs @@ -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 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 {