Warn about Id clashes for Grid, Plot, ScrollArea, Table (#1452)

Id clashes can cause subtle bugs.

egui already warns when the same Id is used to interact with different
parts of the screen.

This adds warnings about id clashes for some widgets that store state:
Grid, Plot, ScrollArea, Table.

The PR also adds `Context::check_for_id_clash` so users who create
their own widgets can add the same type of check.

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)).

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);

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();

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(),

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!
     ///     });
     /// }
     /// # });

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,

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<egui::Id>) -> Option<Vec<f32>> {
+    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`].