diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e527649b85..f2306c7c8fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,13 +11,14 @@ NOTE: [`epaint`](epaint/CHANGELOG.md), [`eframe`](eframe/CHANGELOG.md), [`egui_w * Added `Frame::canvas` ([#1362](https://github.com/emilk/egui/pull/1362)). * `Context::request_repaint` will wake up UI thread, if integrations has called `Context::set_request_repaint_callback` ([#1366](https://github.com/emilk/egui/pull/1366)). * Added `Plot::allow_scroll`, `Plot::allow_zoom` no longer affects scrolling ([#1382](https://github.com/emilk/egui/pull/1382)). -* Added `Ui::push_id` ([#1374](https://github.com/emilk/egui/pull/1374)). +* Added `Ui::push_id` to resolve id clashes ([#1374](https://github.com/emilk/egui/pull/1374)). * Added `Frame::outer_margin`. ### Changed 🔧 * `ClippedMesh` has been replaced with `ClippedPrimitive` ([#1351](https://github.com/emilk/egui/pull/1351)). * Renamed `Frame::margin` to `Frame::inner_margin`. * Renamed `AlphaImage` to `FontImage` to discourage any other use for it ([#1412](https://github.com/emilk/egui/pull/1412)). +* Warnings will pe painted on screen when there is an `Id` clash for `Grid`, `Plot` or `ScrollArea` ([#1452](https://github.com/emilk/egui/pull/1452)). ### Fixed 🐛 * Fixed ComboBoxes always being rendered left-aligned ([#1304](https://github.com/emilk/egui/pull/1304)). diff --git a/egui/src/containers/scroll_area.rs b/egui/src/containers/scroll_area.rs index 8b6ab960134..7bd5576e7a6 100644 --- a/egui/src/containers/scroll_area.rs +++ b/egui/src/containers/scroll_area.rs @@ -325,6 +325,11 @@ impl ScrollArea { let id_source = id_source.unwrap_or_else(|| Id::new("scroll_area")); let id = ui.make_persistent_id(id_source); + ui.ctx().check_for_id_clash( + id, + Rect::from_min_size(ui.available_rect_before_wrap().min, Vec2::ZERO), + "ScrollArea", + ); let mut state = State::load(&ctx, id).unwrap_or_default(); state.offset.x = offset_x.unwrap_or(state.offset.x); diff --git a/egui/src/context.rs b/egui/src/context.rs index 8f3abf3fd52..e0212496b65 100644 --- a/egui/src/context.rs +++ b/egui/src/context.rs @@ -223,9 +223,16 @@ impl Context { // --------------------------------------------------------------------- - /// If the given [`Id`] is not unique, an error will be printed at the given position. - /// Call this for [`Id`]:s that need interaction or persistence. - pub(crate) fn register_interaction_id(&self, id: Id, new_rect: Rect) { + /// If the given [`Id`] has been used previously the same frame at at different position, + /// then an error will be printed on screen. + /// + /// This function is already called for all widgets that do any interaction, + /// but you can call this from widgets that store state but that does not interact. + /// + /// The given [`Rect`] should be approximately where the widget will be. + /// The most important thing is that [`Rect::min`] is approximately correct, + /// because that's where the warning will be painted. If you don't know what size to pick, just pick [`Vec2::ZERO`]. + pub fn check_for_id_clash(&self, id: Id, new_rect: Rect, what: &str) { let prev_rect = self.frame_state().used_ids.insert(id, new_rect); if let Some(prev_rect) = prev_rect { // it is ok to reuse the same ID for e.g. a frame around a widget, @@ -244,7 +251,8 @@ impl Context { painter.error( rect.left_bottom() + vec2(2.0, 4.0), "ID clashes happens when things like Windows or CollapsingHeaders share names,\n\ - or when things like ScrollAreas and Resize areas aren't given unique id_source:s.", + or when things like Plot and Grid:s aren't given unique id_source:s.\n\n\ + Sometimes the solution is to use ui.push_id.", ); } } @@ -253,10 +261,19 @@ impl Context { let id_str = id.short_debug_format(); if prev_rect.min.distance(new_rect.min) < 4.0 { - show_error(new_rect.min, format!("Double use of ID {}", id_str)); + show_error( + new_rect.min, + format!("Double use of {} ID {}", what, id_str), + ); } else { - show_error(prev_rect.min, format!("First use of ID {}", id_str)); - show_error(new_rect.min, format!("Second use of ID {}", id_str)); + show_error( + prev_rect.min, + format!("First use of {} ID {}", what, id_str), + ); + show_error( + new_rect.min, + format!("Second use of {} ID {}", what, id_str), + ); } } } @@ -322,7 +339,7 @@ impl Context { return response; } - self.register_interaction_id(id, rect); + self.check_for_id_clash(id, rect, "widget"); let clicked_elsewhere = response.clicked_elsewhere(); let ctx_impl = &mut *self.write(); diff --git a/egui/src/grid.rs b/egui/src/grid.rs index 7b6267346ee..780164803f1 100644 --- a/egui/src/grid.rs +++ b/egui/src/grid.rs @@ -80,6 +80,8 @@ impl GridLayout { "Grid not yet available for right-to-left layouts" ); + ui.ctx().check_for_id_clash(id, initial_available, "Grid"); + Self { ctx: ui.ctx().clone(), style: ui.style().clone(), diff --git a/egui/src/ui.rs b/egui/src/ui.rs index 5bd94d3a069..874cc4160c2 100644 --- a/egui/src/ui.rs +++ b/egui/src/ui.rs @@ -1566,9 +1566,10 @@ impl Ui { /// ``` /// # egui::__run_test_ui(|ui| { /// for i in 0..10 { - /// // `ui.make_persistent_id("foo")` here will produce the same id each loop. + /// // ui.collapsing("Same header", |ui| { }); // this will cause an ID clash because of the same title! + /// /// ui.push_id(i, |ui| { - /// // `ui.make_persistent_id("foo")` here will produce different id:s + /// ui.collapsing("Same header", |ui| { }); // this is fine! /// }); /// } /// # }); diff --git a/egui/src/widgets/plot/mod.rs b/egui/src/widgets/plot/mod.rs index e6183bac75d..15a8ce35cb2 100644 --- a/egui/src/widgets/plot/mod.rs +++ b/egui/src/widgets/plot/mod.rs @@ -494,6 +494,7 @@ impl Plot { // Load or initialize the memory. let plot_id = ui.make_persistent_id(id_source); + ui.ctx().check_for_id_clash(plot_id, rect, "Plot"); let mut memory = PlotMemory::load(ui.ctx(), plot_id).unwrap_or_else(|| PlotMemory { auto_bounds: !min_auto_bounds.is_valid(), hovered_entry: None, diff --git a/egui_extras/src/table.rs b/egui_extras/src/table.rs index 6d7b8ec6f53..caa5e13d451 100644 --- a/egui_extras/src/table.rs +++ b/egui_extras/src/table.rs @@ -9,7 +9,7 @@ use crate::{ Size, StripLayout, }; -use egui::{Response, Ui}; +use egui::{Rect, Response, Ui, Vec2}; /// Builder for a [`Table`] with (optional) fixed header and scrolling body. /// @@ -139,12 +139,8 @@ impl<'a> TableBuilder<'a> { } = self; let resize_id = resizable.then(|| ui.id().with("__table_resize")); - let widths = if let Some(resize_id) = resize_id { - ui.data().get_persisted(resize_id) - } else { - None - }; - let widths = widths + + let widths = read_table_widths(ui, resize_id) .unwrap_or_else(|| sizing.to_lengths(available_width, ui.spacing().item_spacing.x)); let table_top = ui.cursor().top(); @@ -190,12 +186,8 @@ impl<'a> TableBuilder<'a> { } = self; let resize_id = resizable.then(|| ui.id().with("__table_resize")); - let widths = if let Some(resize_id) = resize_id { - ui.data().get_persisted(resize_id) - } else { - None - }; - let widths = widths + + let widths = read_table_widths(ui, resize_id) .unwrap_or_else(|| sizing.to_lengths(available_width, ui.spacing().item_spacing.x)); let table_top = ui.cursor().top(); @@ -215,6 +207,16 @@ impl<'a> TableBuilder<'a> { } } +fn read_table_widths(ui: &egui::Ui, resize_id: Option) -> Option> { + if let Some(resize_id) = resize_id { + let rect = Rect::from_min_size(ui.available_rect_before_wrap().min, Vec2::ZERO); + ui.ctx().check_for_id_clash(resize_id, rect, "Table"); + ui.data().get_persisted(resize_id) + } else { + None + } +} + /// Table struct which can construct a [`TableBody`]. /// /// Is created by [`TableBuilder`] by either calling [`TableBuilder::body`] or after creating a header row with [`TableBuilder::header`].