From 94545409c6cb2c97f68d42b194a29d083c417c47 Mon Sep 17 00:00:00 2001
From: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
Date: Thu, 11 Jun 2020 18:06:23 +0200
Subject: [PATCH] [id] improve default Id generation

Base on child count instead of on screen position.
This is more robust against changing sizes of nearby widgets.
---
 egui/src/id.rs | 14 +++-----------
 egui/src/ui.rs | 25 +++++++++++++++----------
 2 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/egui/src/id.rs b/egui/src/id.rs
index 93e1c436..d741d78b 100644
--- a/egui/src/id.rs
+++ b/egui/src/id.rs
@@ -1,19 +1,19 @@
 //! Egui tracks widgets frame-to-frame using `Id`s.
 //!
 //! For instance, if you start dragging a slider one frame, egui stores
-//! the sldiers Id as the current `interact_id` so that next frame when
+//! the sliders `Id` as the current active id so that next frame when
 //! you move the mouse the same slider changes, even if the mouse has
 //! moved outside the slider.
 //!
 //! For some widgets `Id`s are also used to persist some state about the
 //! widgets, such as Window position or wether not a collapsing header region is open.
 //!
-//! This implicated that the `Id`s must be unqiue.
+//! This implicates that the `Id`s must be unqiue.
 //!
 //! For simple things like sliders and buttons that don't have any memory and
 //! doesn't move we can use the location of the widget as a source of identity.
 //! For instance, a slider only needs a unique and persistent ID while you are
-//! dragging the sldier. As long as it is still while moving, that is fine.
+//! dragging the slider. As long as it is still while moving, that is fine.
 //!
 //! For things that need to persist state even after moving (windows, collapsing headers)
 //! the location of the widgets is obviously not good enough. For instance,
@@ -29,8 +29,6 @@
 
 use std::hash::Hash;
 
-use crate::math::Pos2;
-
 #[derive(Clone, Copy, Debug, Hash, Eq, PartialEq)]
 #[cfg_attr(feature = "with_serde", derive(serde::Deserialize, serde::Serialize))]
 pub struct Id(u64);
@@ -58,10 +56,4 @@ impl Id {
         child.hash(&mut hasher);
         Id(hasher.finish())
     }
-
-    pub fn from_pos(p: Pos2) -> Id {
-        let x = p.x.round() as i32;
-        let y = p.y.round() as i32;
-        Id::new(&x).with(&y)
-    }
 }
diff --git a/egui/src/ui.rs b/egui/src/ui.rs
index 66aabcb5..ae2ec01c 100644
--- a/egui/src/ui.rs
+++ b/egui/src/ui.rs
@@ -49,6 +49,11 @@ pub struct Ui {
     /// If something has already been added, this will point ot style.item_spacing beyond the latest child.
     /// The cursor can thus be style.item_spacing pixels outside of the child_bounds.
     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 {
@@ -67,17 +72,17 @@ impl Ui {
             style,
             layout: Default::default(),
             cursor: rect.min,
+            child_count: 0,
         }
     }
 
-    pub fn child_ui(&self, child_rect: Rect) -> Self {
-        // let clip_rect = self
-        //     .clip_rect
-        //     .intersect(&child_rect.expand(self.style().clip_rect_margin));
-        let clip_rect = self.clip_rect(); // Keep it unless the child explciitly desires differently
+    pub fn child_ui(&mut self, child_rect: Rect) -> Self {
+        let clip_rect = self.clip_rect(); // Keep it unless the child excplicitly desires differently
+        let id = self.make_position_id(); // TODO: is this a good idea?
+        self.child_count += 1;
         Ui {
             ctx: self.ctx.clone(),
-            id: self.id,
+            id,
             layer: self.layer,
             clip_rect,
             desired_rect: child_rect,
@@ -85,6 +90,7 @@ impl Ui {
             style: self.style.clone(),
             layout: self.layout,
             cursor: child_rect.min,
+            child_count: 0,
         }
     }
 
@@ -293,7 +299,7 @@ impl Ui {
     /// Can be used for widgets that do NOT persist state in Memory
     /// but you still need to interact with (e.g. buttons, sliders).
     pub fn make_position_id(&self) -> Id {
-        self.id.with(&Id::from_pos(self.cursor))
+        self.id.with(self.child_count)
     }
 
     pub fn make_child_id(&self, id_seed: impl Hash) -> Id {
@@ -393,6 +399,7 @@ impl Ui {
             self.layout
                 .allocate_space(&mut self.cursor, &self.style, available_size, child_size);
         self.child_bounds = self.child_bounds.union(child_rect);
+        self.child_count += 1;
         child_rect
     }
 
@@ -630,9 +637,7 @@ impl Ui {
         add_contents: impl FnOnce(&mut Self) -> R,
     ) -> (R, Rect) {
         let child_rect = Rect::from_min_max(self.cursor, self.bottom_right());
-        let mut child_ui = Self {
-            ..self.child_ui(child_rect)
-        };
+        let mut child_ui = self.child_ui(child_rect);
         child_ui.set_layout(layout); // HACK: need a separate call right now
         let ret = add_contents(&mut child_ui);
         let size = child_ui.bounding_size();