mirror of
https://github.com/emilk/egui.git
synced 2026-09-02 14:50:03 -04:00
Add Response::parent_id and improve warn_if_rect_changes_id (#8010)
Reduces the amount of false positives --------- Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
This commit is contained in:
@@ -516,6 +516,7 @@ impl Area {
|
|||||||
let move_response = ctx.create_widget(
|
let move_response = ctx.create_widget(
|
||||||
WidgetRect {
|
WidgetRect {
|
||||||
id: interact_id,
|
id: interact_id,
|
||||||
|
parent_id: id,
|
||||||
layer_id,
|
layer_id,
|
||||||
rect: state.rect(),
|
rect: state.rect(),
|
||||||
interact_rect: state.rect().intersect(constrain_rect),
|
interact_rect: state.rect().intersect(constrain_rect),
|
||||||
|
|||||||
@@ -962,6 +962,7 @@ fn do_resize_interaction(
|
|||||||
WidgetRect {
|
WidgetRect {
|
||||||
layer_id,
|
layer_id,
|
||||||
id,
|
id,
|
||||||
|
parent_id: layer_id.id,
|
||||||
rect,
|
rect,
|
||||||
interact_rect: rect,
|
interact_rect: rect,
|
||||||
sense: Sense::DRAG, // Don't use Sense::drag() since we don't want these to be focusable
|
sense: Sense::DRAG, // Don't use Sense::drag() since we don't want these to be focusable
|
||||||
|
|||||||
@@ -1360,6 +1360,7 @@ impl Context {
|
|||||||
|
|
||||||
let WidgetRect {
|
let WidgetRect {
|
||||||
id,
|
id,
|
||||||
|
parent_id: _,
|
||||||
layer_id,
|
layer_id,
|
||||||
rect,
|
rect,
|
||||||
interact_rect,
|
interact_rect,
|
||||||
@@ -4324,6 +4325,15 @@ fn warn_if_rect_changes_id(
|
|||||||
continue;
|
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;
|
let rect = new_at_rect[0].rect;
|
||||||
|
|
||||||
log::warn!(
|
log::warn!(
|
||||||
|
|||||||
@@ -450,6 +450,7 @@ mod tests {
|
|||||||
fn wr(id: Id, sense: Sense, rect: Rect) -> WidgetRect {
|
fn wr(id: Id, sense: Sense, rect: Rect) -> WidgetRect {
|
||||||
WidgetRect {
|
WidgetRect {
|
||||||
id,
|
id,
|
||||||
|
parent_id: Id::NULL,
|
||||||
layer_id: LayerId::background(),
|
layer_id: LayerId::background(),
|
||||||
rect,
|
rect,
|
||||||
interact_rect: rect,
|
interact_rect: rect,
|
||||||
|
|||||||
@@ -151,6 +151,22 @@ bitflags::bitflags! {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl Response {
|
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.
|
/// 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
|
/// A click is registered when the mouse or touch is released within
|
||||||
@@ -761,6 +777,7 @@ impl Response {
|
|||||||
WidgetRect {
|
WidgetRect {
|
||||||
layer_id: self.layer_id,
|
layer_id: self.layer_id,
|
||||||
id: self.id,
|
id: self.id,
|
||||||
|
parent_id: self.parent_id(),
|
||||||
rect: self.rect,
|
rect: self.rect,
|
||||||
interact_rect: self.interact_rect,
|
interact_rect: self.interact_rect,
|
||||||
sense: self.sense | sense,
|
sense: self.sense | sense,
|
||||||
|
|||||||
@@ -1303,7 +1303,8 @@ pub struct DebugOptions {
|
|||||||
/// Show interesting widgets under the mouse cursor.
|
/// Show interesting widgets under the mouse cursor.
|
||||||
pub show_widget_hits: bool,
|
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,
|
pub warn_if_rect_changes_id: bool,
|
||||||
|
|
||||||
/// If true, highlight widgets that are not aligned to [`emath::GUI_ROUNDING`].
|
/// If true, highlight widgets that are not aligned to [`emath::GUI_ROUNDING`].
|
||||||
|
|||||||
@@ -173,6 +173,7 @@ impl Ui {
|
|||||||
ui.ctx().create_widget(
|
ui.ctx().create_widget(
|
||||||
WidgetRect {
|
WidgetRect {
|
||||||
id: ui.unique_id,
|
id: ui.unique_id,
|
||||||
|
parent_id: ui.id,
|
||||||
layer_id: ui.layer_id(),
|
layer_id: ui.layer_id(),
|
||||||
rect: start_rect,
|
rect: start_rect,
|
||||||
interact_rect: start_rect,
|
interact_rect: start_rect,
|
||||||
@@ -339,6 +340,7 @@ impl Ui {
|
|||||||
child_ui.ctx().create_widget(
|
child_ui.ctx().create_widget(
|
||||||
WidgetRect {
|
WidgetRect {
|
||||||
id: child_ui.unique_id,
|
id: child_ui.unique_id,
|
||||||
|
parent_id: self.id,
|
||||||
layer_id: child_ui.layer_id(),
|
layer_id: child_ui.layer_id(),
|
||||||
rect: start_rect,
|
rect: start_rect,
|
||||||
interact_rect: start_rect,
|
interact_rect: start_rect,
|
||||||
@@ -1043,6 +1045,7 @@ impl Ui {
|
|||||||
self.ctx().create_widget(
|
self.ctx().create_widget(
|
||||||
WidgetRect {
|
WidgetRect {
|
||||||
id,
|
id,
|
||||||
|
parent_id: self.id,
|
||||||
layer_id: self.layer_id(),
|
layer_id: self.layer_id(),
|
||||||
rect,
|
rect,
|
||||||
interact_rect: self.clip_rect().intersect(rect),
|
interact_rect: self.clip_rect().intersect(rect),
|
||||||
@@ -1112,6 +1115,7 @@ impl Ui {
|
|||||||
let mut response = self.ctx().create_widget(
|
let mut response = self.ctx().create_widget(
|
||||||
WidgetRect {
|
WidgetRect {
|
||||||
id: self.unique_id,
|
id: self.unique_id,
|
||||||
|
parent_id: self.id,
|
||||||
layer_id: self.layer_id(),
|
layer_id: self.layer_id(),
|
||||||
rect: self.min_rect(),
|
rect: self.min_rect(),
|
||||||
interact_rect: self.clip_rect().intersect(self.min_rect()),
|
interact_rect: self.clip_rect().intersect(self.min_rect()),
|
||||||
|
|||||||
@@ -17,6 +17,12 @@ pub struct WidgetRect {
|
|||||||
/// You can ensure globally unique ids using [`crate::Ui::push_id`].
|
/// You can ensure globally unique ids using [`crate::Ui::push_id`].
|
||||||
pub id: 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.
|
/// What layer the widget is on.
|
||||||
pub layer_id: LayerId,
|
pub layer_id: LayerId,
|
||||||
|
|
||||||
@@ -46,6 +52,7 @@ impl WidgetRect {
|
|||||||
pub fn transform(self, transform: emath::TSTransform) -> Self {
|
pub fn transform(self, transform: emath::TSTransform) -> Self {
|
||||||
let Self {
|
let Self {
|
||||||
id,
|
id,
|
||||||
|
parent_id,
|
||||||
layer_id,
|
layer_id,
|
||||||
rect,
|
rect,
|
||||||
interact_rect,
|
interact_rect,
|
||||||
@@ -54,6 +61,7 @@ impl WidgetRect {
|
|||||||
} = self;
|
} = self;
|
||||||
Self {
|
Self {
|
||||||
id,
|
id,
|
||||||
|
parent_id,
|
||||||
layer_id,
|
layer_id,
|
||||||
rect: transform * rect,
|
rect: transform * rect,
|
||||||
interact_rect: transform * interact_rect,
|
interact_rect: transform * interact_rect,
|
||||||
|
|||||||
@@ -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]
|
#[test]
|
||||||
fn horizontal_wrapped_multiline_row_height() {
|
fn horizontal_wrapped_multiline_row_height() {
|
||||||
let mut harness = Harness::builder().with_size((350.0, 300.0)).build_ui(|ui| {
|
let mut harness = Harness::builder().with_size((350.0, 300.0)).build_ui(|ui| {
|
||||||
|
|||||||
Reference in New Issue
Block a user