From 0d065f9e78ae611ea57ff4159fcb74f186731cb6 Mon Sep 17 00:00:00 2001 From: Lucas Meurer Date: Tue, 24 Mar 2026 16:22:44 +0100 Subject: [PATCH] Add `Response::parent_id` and improve `warn_if_rect_changes_id` (#8010) Reduces the amount of false positives --------- Co-authored-by: Emil Ernerfeldt --- crates/egui/src/containers/area.rs | 1 + crates/egui/src/containers/window.rs | 1 + crates/egui/src/context.rs | 10 +++++ crates/egui/src/hit_test.rs | 1 + crates/egui/src/response.rs | 17 +++++++++ crates/egui/src/style.rs | 3 +- crates/egui/src/ui.rs | 4 ++ crates/egui/src/widget_rect.rs | 8 ++++ tests/egui_tests/tests/regression_tests.rs | 43 ++++++++++++++++++++++ 9 files changed, 87 insertions(+), 1 deletion(-) diff --git a/crates/egui/src/containers/area.rs b/crates/egui/src/containers/area.rs index 4333cf73a..d44c0ae41 100644 --- a/crates/egui/src/containers/area.rs +++ b/crates/egui/src/containers/area.rs @@ -516,6 +516,7 @@ impl Area { let move_response = ctx.create_widget( WidgetRect { id: interact_id, + parent_id: id, layer_id, rect: state.rect(), interact_rect: state.rect().intersect(constrain_rect), diff --git a/crates/egui/src/containers/window.rs b/crates/egui/src/containers/window.rs index c6b739589..ae5fbfc8f 100644 --- a/crates/egui/src/containers/window.rs +++ b/crates/egui/src/containers/window.rs @@ -962,6 +962,7 @@ fn do_resize_interaction( WidgetRect { layer_id, id, + parent_id: layer_id.id, rect, interact_rect: rect, sense: Sense::DRAG, // Don't use Sense::drag() since we don't want these to be focusable diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index af6c0e6d0..51a663ce5 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -1360,6 +1360,7 @@ impl Context { let WidgetRect { id, + parent_id: _, layer_id, rect, interact_rect, @@ -4324,6 +4325,15 @@ fn warn_if_rect_changes_id( continue; } + // Only warn if at least one widget has the same parent_id in both frames. + // If all parent_ids changed too, this is a cascading id shift, not a widget bug. + if !prev_at_rect + .iter() + .any(|pw| new_at_rect.iter().any(|nw| nw.parent_id == pw.parent_id)) + { + continue; + } + let rect = new_at_rect[0].rect; log::warn!( diff --git a/crates/egui/src/hit_test.rs b/crates/egui/src/hit_test.rs index 8fe962b36..c7ffd7cda 100644 --- a/crates/egui/src/hit_test.rs +++ b/crates/egui/src/hit_test.rs @@ -450,6 +450,7 @@ mod tests { fn wr(id: Id, sense: Sense, rect: Rect) -> WidgetRect { WidgetRect { id, + parent_id: Id::NULL, layer_id: LayerId::background(), rect, interact_rect: rect, diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index 241f5a381..a0dd6bd91 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -151,6 +151,22 @@ bitflags::bitflags! { } impl Response { + /// The [`Id`] of the parent [`crate::Ui`] that hosts this widget. + /// + /// Looks up the [`WidgetRect`] from the current (or previous) pass. + pub fn parent_id(&self) -> Id { + let id = self.ctx.viewport(|viewport| { + viewport + .this_pass + .widgets + .get(self.id) + .or_else(|| viewport.prev_pass.widgets.get(self.id)) + .map(|w| w.parent_id) + }); + debug_assert!(id.is_some(), "WidgetRect for Response not found!"); + id.unwrap_or(Id::NULL) + } + /// Returns true if this widget was clicked this frame by the primary button. /// /// A click is registered when the mouse or touch is released within @@ -761,6 +777,7 @@ impl Response { WidgetRect { layer_id: self.layer_id, id: self.id, + parent_id: self.parent_id(), rect: self.rect, interact_rect: self.interact_rect, sense: self.sense | sense, diff --git a/crates/egui/src/style.rs b/crates/egui/src/style.rs index 5cd980f6c..4f9749663 100644 --- a/crates/egui/src/style.rs +++ b/crates/egui/src/style.rs @@ -1303,7 +1303,8 @@ pub struct DebugOptions { /// Show interesting widgets under the mouse cursor. pub show_widget_hits: bool, - /// Show a warning if the same `Rect` had different `Id` on the previous frame. + /// Show a warning if the same `Rect` had different `Id` and the same parent `Id` on the + /// previous frame. pub warn_if_rect_changes_id: bool, /// If true, highlight widgets that are not aligned to [`emath::GUI_ROUNDING`]. diff --git a/crates/egui/src/ui.rs b/crates/egui/src/ui.rs index f0b270951..d55f4174f 100644 --- a/crates/egui/src/ui.rs +++ b/crates/egui/src/ui.rs @@ -173,6 +173,7 @@ impl Ui { ui.ctx().create_widget( WidgetRect { id: ui.unique_id, + parent_id: ui.id, layer_id: ui.layer_id(), rect: start_rect, interact_rect: start_rect, @@ -339,6 +340,7 @@ impl Ui { child_ui.ctx().create_widget( WidgetRect { id: child_ui.unique_id, + parent_id: self.id, layer_id: child_ui.layer_id(), rect: start_rect, interact_rect: start_rect, @@ -1043,6 +1045,7 @@ impl Ui { self.ctx().create_widget( WidgetRect { id, + parent_id: self.id, layer_id: self.layer_id(), rect, interact_rect: self.clip_rect().intersect(rect), @@ -1112,6 +1115,7 @@ impl Ui { let mut response = self.ctx().create_widget( WidgetRect { id: self.unique_id, + parent_id: self.id, layer_id: self.layer_id(), rect: self.min_rect(), interact_rect: self.clip_rect().intersect(self.min_rect()), diff --git a/crates/egui/src/widget_rect.rs b/crates/egui/src/widget_rect.rs index a84dde519..b6fc9f7bb 100644 --- a/crates/egui/src/widget_rect.rs +++ b/crates/egui/src/widget_rect.rs @@ -17,6 +17,12 @@ pub struct WidgetRect { /// You can ensure globally unique ids using [`crate::Ui::push_id`]. pub id: Id, + /// The [`Id`] of the parent [`crate::Ui`] that hosts this widget. + /// + /// Used by debug checks to distinguish true id-instability from + /// cascading id shifts caused by a parent Ui's auto-id changing. + pub parent_id: Id, + /// What layer the widget is on. pub layer_id: LayerId, @@ -46,6 +52,7 @@ impl WidgetRect { pub fn transform(self, transform: emath::TSTransform) -> Self { let Self { id, + parent_id, layer_id, rect, interact_rect, @@ -54,6 +61,7 @@ impl WidgetRect { } = self; Self { id, + parent_id, layer_id, rect: transform * rect, interact_rect: transform * interact_rect, diff --git a/tests/egui_tests/tests/regression_tests.rs b/tests/egui_tests/tests/regression_tests.rs index 2d6ab5c67..f32ff7ff3 100644 --- a/tests/egui_tests/tests/regression_tests.rs +++ b/tests/egui_tests/tests/regression_tests.rs @@ -280,6 +280,49 @@ fn warn_if_rect_changes_id() { ); } +/// When a parent Ui's id changes (e.g. via `push_id` with a dynamic value), +/// all child widget ids shift too. This should NOT trigger `warn_if_rect_changes_id` because the +/// `parent_id` also changed — it's a cascading id shift, not a widget bug. +#[test] +fn warn_if_rect_changes_id_false_positive_parent_shift() { + use std::cell::Cell; + + let counter = Cell::new(0); + let button_rect = egui::Rect::from_min_size(egui::pos2(10.0, 10.0), egui::vec2(100.0, 30.0)); + + let mut harness = Harness::builder().with_size((200.0, 100.0)).build_ui(|ui| { + // push_id with a changing value causes the child Ui's id to shift, + // which in turn shifts all widget ids inside it. + ui.push_id(counter.get(), |ui| { + let id = ui.id().with("my_widget"); + let _response = ui.interact(button_rect, id, Sense::click()); + }); + }); + + // Frame 1: counter=0 — establishes prev_pass + harness.step(); + assert!( + !has_red_warning_rect(harness.output()), + "Should not warn on first frame" + ); + + // Frame 2: counter=0 — prev_pass == this_pass + harness.step(); + assert!( + !has_red_warning_rect(harness.output()), + "Should not warn when nothing changed" + ); + + // Now change the parent id, shifting all child widget ids + counter.set(1); + harness.step(); + + assert!( + !has_red_warning_rect(harness.output()), + "Should NOT warn when parent Ui's id shifted (cascading id change)" + ); +} + #[test] fn horizontal_wrapped_multiline_row_height() { let mut harness = Harness::builder().with_size((350.0, 300.0)).build_ui(|ui| {