From 03eb9151c42da0ca72e6eea510c59f3fafbfc8c1 Mon Sep 17 00:00:00 2001
From: Emil Ernerfeldt <emilernerfeldt@gmail.com>
Date: Sat, 14 Nov 2020 18:20:06 +0100
Subject: [PATCH] Improve automatic Id generation to make Id clashes less
 likely

---
 CHANGELOG.md                  |  1 +
 egui/src/id.rs                |  4 ++++
 egui/src/ui.rs                | 29 +++++++++++++++--------------
 egui/src/widgets/text_edit.rs |  9 ++++++++-
 4 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a7bbcd56..d236a615 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 * Pressing enter in a single-line `TextEdit` will now surrender keyboard focus for it.
 * You must now be explicit when creating a `TextEdit` if you want it to be singeline or multiline.
+* Improved automatic `Id` generation, making `Id` clashes less likely.
 
 ### Fixed 🐛
 
diff --git a/egui/src/id.rs b/egui/src/id.rs
index 4f9fa45e..2b82b1d1 100644
--- a/egui/src/id.rs
+++ b/egui/src/id.rs
@@ -58,6 +58,10 @@ impl Id {
     pub(crate) fn short_debug_format(&self) -> String {
         format!("{:04X}", self.0 as u16)
     }
+
+    pub(crate) fn value(&self) -> u64 {
+        self.0
+    }
 }
 
 impl std::fmt::Debug for Id {
diff --git a/egui/src/ui.rs b/egui/src/ui.rs
index 50dc0e24..8f90079d 100644
--- a/egui/src/ui.rs
+++ b/egui/src/ui.rs
@@ -11,9 +11,17 @@ pub struct Ui {
     /// Generated based on id of parent ui together with
     /// another source of child identity (e.g. window title).
     /// Acts like a namespace for child uis.
-    /// Hopefully unique.
+    /// Should be unique and persist predictably from one frame to next
+    /// so it can be used as a source for storing state (e.g. window position, or if a collapsing header is open).
     id: Id,
 
+    /// This is used to create a unique interact ID for some widgets.
+    /// This value is based on where in the hierarchy of widgets this Ui is in,
+    /// and the value is increment with each added child widget.
+    /// This works as an Id source only as long as new widgets aren't added or removed.
+    /// They are therefore only good for Id:s that has no state.
+    next_auto_id: u64,
+
     painter: Painter,
 
     /// This is the minimal size of the `Ui`.
@@ -49,11 +57,6 @@ pub struct Ui {
     /// If something has already been added, this will point ot style.spacing.item_spacing beyond the latest child.
     /// The cursor can thus be style.spacing.item_spacing pixels outside of the min_rect.
     cursor: Pos2, // TODO: move into Layout?
-
-    /// How many children has been added to us?
-    /// This is only used to create a unique interact ID for some widgets
-    /// that work as long as no other widgets are added/removed while interacting.
-    child_count: usize,
 }
 
 impl Ui {
@@ -74,33 +77,31 @@ impl Ui {
         let min_rect = layout.rect_from_cursor_size(cursor, min_size);
         Ui {
             id,
+            next_auto_id: id.with("auto").value(),
             painter: Painter::new(ctx, layer_id, clip_rect),
             min_rect,
             max_rect,
             style,
             layout,
             cursor,
-            child_count: 0,
         }
     }
 
     pub fn child_ui(&mut self, max_rect: Rect, layout: Layout) -> Self {
-        self.child_count += 1;
-        let id = self.make_position_id(); // TODO: ui.id should be persistent and not position based.
-
+        self.next_auto_id = self.next_auto_id.wrapping_add(1);
         let cursor = layout.initial_cursor(max_rect);
         let min_size = Vec2::zero(); // TODO: From Style
         let min_rect = layout.rect_from_cursor_size(cursor, min_size);
 
         Ui {
-            id,
+            id: self.id,
+            next_auto_id: Id::new(self.next_auto_id).with("child").value(),
             painter: self.painter.clone(),
             min_rect,
             max_rect,
             style: self.style.clone(),
             layout,
             cursor,
-            child_count: 0,
         }
     }
 
@@ -379,7 +380,7 @@ impl Ui {
     /// Call AFTER allocating new space for your widget.
     // TODO: return from `allocate_space` ?
     pub fn make_position_id(&self) -> Id {
-        self.id.with(self.child_count)
+        Id::new(self.next_auto_id)
     }
 }
 
@@ -472,7 +473,7 @@ impl Ui {
         let item_spacing = self.style().spacing.item_spacing;
         self.layout.advance_cursor2(&mut self.cursor, item_spacing);
         self.expand_to_include_rect(child_rect);
-        self.child_count += 1;
+        self.next_auto_id = self.next_auto_id.wrapping_add(1);
         child_rect
     }
 }
diff --git a/egui/src/widgets/text_edit.rs b/egui/src/widgets/text_edit.rs
index 8a67888e..0159b0bf 100644
--- a/egui/src/widgets/text_edit.rs
+++ b/egui/src/widgets/text_edit.rs
@@ -129,7 +129,14 @@ impl<'t> Widget for TextEdit<'t> {
             desired_height_rows,
         } = self;
 
-        let id = id.unwrap_or_else(|| ui.make_persistent_id(id_source));
+        let id = id.unwrap_or_else(|| {
+            if let Some(id_source) = id_source {
+                ui.make_persistent_id(id_source)
+            } else {
+                // Since we are only storing cursor, perfect persistence Id not super important
+                ui.make_position_id()
+            }
+        });
 
         let mut state = ui.memory().text_edit.get(&id).cloned().unwrap_or_default();