Distinguish ids that need to be unique and warn about name clashes

emigui/src/context.rs

@@ -1,8 +1,8 @@
-use std::sync::Arc;
+use std::{collections::HashMap, sync::Arc};
 
 use parking_lot::Mutex;
 
-use crate::*;
+use crate::{layout::align_rect, *};
 
 /// Contains the input, style and output of all GUI commands.
 pub struct Context {
@@ -12,8 +12,12 @@ pub struct Context {
     pub(crate) input: GuiInput,
     pub(crate) memory: Mutex<Memory>,
     pub(crate) graphics: Mutex<GraphicLayers>,
+
+    /// Used to debug name clashes of e.g. windows
+    used_ids: Mutex<HashMap<Id, Pos2>>,
 }
 
+// TODO: remove this impl.
 impl Clone for Context {
     fn clone(&self) -> Self {
         Context {
@@ -22,6 +26,7 @@ impl Clone for Context {
             input: self.input,
             memory: Mutex::new(self.memory.lock().clone()),
             graphics: Mutex::new(self.graphics.lock().clone()),
+            used_ids: Mutex::new(self.used_ids.lock().clone()),
         }
     }
 }
@@ -34,6 +39,7 @@ impl Context {
             input: Default::default(),
             memory: Default::default(),
             graphics: Default::default(),
+            used_ids: Default::default(),
         }
     }
 
@@ -51,6 +57,7 @@ impl Context {
 
     // TODO: move
     pub fn new_frame(&mut self, gui_input: GuiInput) {
+        self.used_ids.lock().clear();
         self.input = gui_input;
         if !gui_input.mouse_down || gui_input.mouse_pos.is_none() {
             self.memory.lock().active_id = None;
@@ -67,6 +74,42 @@ impl Context {
         self.memory.lock().active_id.is_some()
     }
 
+    /// Generate a id from the given source.
+    /// If it is not unique, an error will be printed at the given position.
+    pub fn make_unique_id<IdSource>(&self, source: &IdSource, pos: Pos2) -> Id
+    where
+        IdSource: std::hash::Hash + std::fmt::Debug,
+    {
+        self.register_unique_id(Id::new(source), source, pos)
+    }
+
+    /// If the given Id is not unique, an error will be printed at the given position.
+    pub fn register_unique_id(&self, id: Id, source_name: &impl std::fmt::Debug, pos: Pos2) -> Id {
+        if let Some(clash_pos) = self.used_ids.lock().insert(id, pos) {
+            if clash_pos.dist(pos) < 4.0 {
+                self.show_error(
+                    pos,
+                    &format!("use of non-unique ID {:?} (name clash?)", source_name),
+                );
+            } else {
+                self.show_error(
+                    clash_pos,
+                    &format!("first use of non-unique ID {:?} (name clash?)", source_name),
+                );
+                self.show_error(
+                    pos,
+                    &format!(
+                        "second use of non-unique ID {:?} (name clash?)",
+                        source_name
+                    ),
+                );
+            }
+            id
+        } else {
+            id
+        }
+    }
+
     pub fn interact(&self, layer: Layer, rect: Rect, interaction_id: Option<Id>) -> InteractInfo {
         let mut memory = self.memory.lock();
 
@@ -100,6 +143,71 @@ impl Context {
             active,
         }
     }
+
+    pub fn show_error(&self, pos: Pos2, text: &str) {
+        let align = (Align::Min, Align::Min);
+        let layer = Layer::Popup; // TODO: Layer::Error
+        let text_style = TextStyle::Monospace;
+        let font = &self.fonts[text_style];
+        let (text, size) = font.layout_multiline(text, std::f32::INFINITY);
+        let rect = align_rect(Rect::from_min_size(pos, size), align);
+        self.add_paint_cmd(
+            layer,
+            PaintCmd::Rect {
+                corner_radius: 0.0,
+                fill_color: Some(color::gray(0, 240)),
+                outline: Some(Outline::new(1.0, color::RED)),
+                rect: rect.expand(2.0),
+            },
+        );
+        self.add_text(layer, rect.min(), text_style, text, Some(color::RED));
+    }
+
+    /// Show some text anywhere on screen.
+    /// To center the text at the given position, use `align: (Center, Center)`.
+    pub fn floating_text(
+        &self,
+        layer: Layer,
+        pos: Pos2,
+        text: &str,
+        text_style: TextStyle,
+        align: (Align, Align),
+        text_color: Option<Color>,
+    ) -> Vec2 {
+        let font = &self.fonts[text_style];
+        let (text, size) = font.layout_multiline(text, std::f32::INFINITY);
+        let rect = align_rect(Rect::from_min_size(pos, size), align);
+        self.add_text(layer, rect.min(), text_style, text, text_color);
+        size
+    }
+
+    /// Already layed out text.
+    pub fn add_text(
+        &self,
+        layer: Layer,
+        pos: Pos2,
+        text_style: TextStyle,
+        text: Vec<font::TextFragment>,
+        color: Option<Color>,
+    ) {
+        let color = color.unwrap_or_else(|| self.style().text_color());
+        for fragment in text {
+            self.add_paint_cmd(
+                layer,
+                PaintCmd::Text {
+                    color,
+                    pos: pos + vec2(0.0, fragment.y_offset),
+                    text: fragment.text,
+                    text_style,
+                    x_offsets: fragment.x_offsets,
+                },
+            );
+        }
+    }
+
+    pub fn add_paint_cmd(&self, layer: Layer, paint_cmd: PaintCmd) {
+        self.graphics.lock().layer(layer).push(paint_cmd)
+    }
 }
 
 impl Context {

emigui/src/emigui.rs

@@ -40,12 +40,13 @@ impl Emigui {
         self.ctx = Arc::new(new_data);
     }
 
-    pub fn whole_screen_region(&mut self) -> Region {
+    /// A region for the entire screen, behind any windows.
+    pub fn background_region(&mut self) -> Region {
         Region {
             ctx: self.ctx.clone(),
             layer: Layer::Background,
             style: self.ctx.style(),
-            id: Id::whole_screen(),
+            id: Id::background(),
             dir: layout::Direction::Vertical,
             align: layout::Align::Center,
             cursor: Default::default(),

emigui/src/example_app.rs

@@ -125,6 +125,28 @@ impl ExampleApp {
         region.foldable("Slider example", |region| {
             value_ui(&mut self.slider_value, region);
         });
+
+        region.foldable("Name clash example", |region| {
+            region.add(label!("\
+                Regions that store state require unique identifiers so we can track their state between frames. \
+                Identifiers are normally derived from the titles of the widget."));
+
+            region.add(label!("\
+                For instance, foldable regions needs to store wether or not they are open. \
+                If you fail to give them unique names then clicking one will open both. \
+                To help you debug this, a error message is printed on screen:"));
+
+            region.foldable("Foldable", |region| {
+                region.add(label!("Contents of first folddable region"));
+            });
+            region.foldable("Foldable", |region| {
+                region.add(label!("Contents of second folddable region"));
+            });
+
+            region.add(label!("Most widgets don't need unique names, but are tracked based on their position on screen. For instance, buttons:"));
+            region.add(Button::new("Button"));
+            region.add(Button::new("Button"));
+        });
     }
 }
 

emigui/src/id.rs

@@ -1,10 +1,41 @@
+//! Emigui tracks widgets frame-to-frame using `Id`s.
+//!
+//! For instance, if you start dragging a slider one frame, emigui stores
+//! the sldiers Id as the current interact_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 GUIpersist some state about the
+//! widgets, such as Window position or wether not a Foldable region is open.
+//!
+//! This implicated 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.
+//!
+//! For things that need to persist state even after moving (windows, foldables)
+//! the location of the widgets is obviously not good enough. For instance,
+//! a fodlable region needs to remember wether or not it is open even
+//! if the layout next frame is different and the foldable is not lower down
+//! on the screen.
+//!
+//! Then there are widgets that need no identifiers at all, like labels,
+//! because they have no state nor are interacted with.
+//!
+//! So we have two type of Ids: PositionId and UniqueId.
+//! TODO: have separate types for PositionId and UniqueId.
+
 use std::{collections::hash_map::DefaultHasher, hash::Hash};
 
+use crate::math::Pos2;
+
 #[derive(Clone, Copy, Debug, Hash, Eq, PartialEq)]
 pub struct Id(u64);
 
 impl Id {
-    pub fn whole_screen() -> Self {
+    pub fn background() -> Self {
         Self(0)
     }
 
@@ -26,4 +57,10 @@ 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)
+    }
 }

emigui/src/layout.rs

@@ -9,7 +9,7 @@ pub struct GuiResponse {
     /// The mouse is hovering above this
     pub hovered: bool,
 
-    /// The mouse went got pressed on this thing this frame
+    /// The mouse clicked this thing this frame
     pub clicked: bool,
 
     /// The mouse is interacting with this thing (e.g. dragging it)
@@ -18,7 +18,7 @@ pub struct GuiResponse {
     /// The region of the screen we are talking about
     pub rect: Rect,
 
-    /// Used for showing a popup (if any)
+    /// Used for optionally showing a tooltip
     pub ctx: Arc<Context>,
 }
 
@@ -78,6 +78,20 @@ impl Default for Align {
     }
 }
 
+pub fn align_rect(rect: Rect, align: (Align, Align)) -> Rect {
+    let x = match align.0 {
+        Align::Min => rect.min().x,
+        Align::Center => rect.min().x - 0.5 * rect.size().x,
+        Align::Max => rect.min().x - rect.size().x,
+    };
+    let y = match align.1 {
+        Align::Min => rect.min().y,
+        Align::Center => rect.min().y - 0.5 * rect.size().y,
+        Align::Max => rect.min().y - rect.size().y,
+    };
+    Rect::from_min_size(pos2(x, y), rect.size())
+}
+
 // ----------------------------------------------------------------------------
 
 /// Show a pop-over window

emigui/src/math.rs

@@ -5,6 +5,11 @@ pub struct Vec2 {
 }
 
 impl Vec2 {
+    pub fn splat(v: impl Into<f32>) -> Vec2 {
+        let v: f32 = v.into();
+        Vec2 { x: v, y: v }
+    }
+
     #[must_use]
     pub fn normalized(self) -> Vec2 {
         let len = self.length();
@@ -129,12 +134,12 @@ pub struct Pos2 {
 }
 
 impl Pos2 {
-    pub fn dist(a: Pos2, b: Pos2) -> f32 {
+    pub fn dist(self: Pos2, other: Pos2) -> f32 {
-        (a - b).length()
+        (self - other).length()
     }
 
-    pub fn dist_sq(a: Pos2, b: Pos2) -> f32 {
+    pub fn dist_sq(self: Pos2, other: Pos2) -> f32 {
-        (a - b).length_sq()
+        (self - other).length_sq()
     }
 
     // TODO: remove?
@@ -230,6 +235,7 @@ impl Rect {
     pub fn expand(self, amnt: f32) -> Self {
         Rect::from_center_size(self.center(), self.size() + 2.0 * vec2(amnt, amnt))
     }
+
     pub fn translate(self, amnt: Vec2) -> Self {
         Rect::from_min_size(self.min() + amnt, self.size())
     }

emigui/src/region.rs

@@ -100,7 +100,7 @@ impl Region {
             "Horizontal foldable is unimplemented"
         );
         let text: String = text.into();
-        let id = self.make_child_id(&text);
+        let id = self.make_unique_id(&text);
         let text_style = TextStyle::Button;
         let font = &self.fonts()[text_style];
         let (text, text_size) = font.layout_multiline(&text, self.width());
@@ -292,7 +292,7 @@ impl Region {
                 ctx: self.ctx.clone(),
                 layer: self.layer,
                 style: self.style,
-                id: self.make_child_id(&("column", col_idx)),
+                id: self.make_child_region_id(&("column", col_idx)),
                 dir: Direction::Vertical,
                 align: self.align,
                 cursor: self.cursor + vec2((col_idx as f32) * (column_width + padding), 0.0),
@@ -355,15 +355,34 @@ impl Region {
         pos
     }
 
-    pub fn make_child_id<H: Hash>(&self, child_id: &H) -> Id {
+    /// Will warn if the returned id is not guaranteed unique.
+    /// Use this to generate widget ids for widgets that have persistent state in Memory.
+    /// If the child_id_source is not unique within this region
+    /// then an error will be printed at the current cursor position.
+    pub fn make_unique_id<IdSource>(&self, child_id_source: &IdSource) -> Id
+    where
+        IdSource: Hash + std::fmt::Debug,
+    {
+        let id = self.id.with(child_id_source);
+        self.ctx
+            .register_unique_id(id, child_id_source, self.cursor)
+    }
+
+    /// Make an Id that is unique to this positon.
+    /// 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))
+    }
+
+    pub fn make_child_region_id<H: Hash>(&self, child_id: &H) -> Id {
         self.id.with(child_id)
     }
 
-    pub fn combined_id(&self, child_id: Option<Id>) -> Option<Id> {
+    /// Show some text anywhere in the region.
-        child_id.map(|child_id| self.id.with(&child_id))
+    /// To center the text at the given position, use `align: (Center, Center)`.
-    }
+    /// If you want to draw text floating on top of everything,
-
+    /// consider using Context.floating_text instead.
-    // Helper function
     pub fn floating_text(
         &mut self,
         pos: Pos2,
@@ -373,22 +392,13 @@ impl Region {
         text_color: Option<Color>,
     ) -> Vec2 {
         let font = &self.fonts()[text_style];
-        let (text, text_size) = font.layout_multiline(text, std::f32::INFINITY);
+        let (text, size) = font.layout_multiline(text, std::f32::INFINITY);
-
+        let rect = align_rect(Rect::from_min_size(pos, size), align);
-        let x = match align.0 {
+        self.add_text(rect.min(), text_style, text, text_color);
-            Align::Min => pos.x,
+        size
-            Align::Center => pos.x - 0.5 * text_size.x,
-            Align::Max => pos.x - text_size.x,
-        };
-        let y = match align.1 {
-            Align::Min => pos.y,
-            Align::Center => pos.y - 0.5 * text_size.y,
-            Align::Max => pos.y - text_size.y,
-        };
-        self.add_text(pos2(x, y), text_style, text, text_color);
-        text_size
     }
 
+    /// Already layed out text.
     pub fn add_text(
         &mut self,
         pos: Pos2,

emigui/src/widgets.rs

@@ -80,7 +80,7 @@ impl Button {
 
 impl Widget for Button {
     fn add_to(self, region: &mut Region) -> GuiResponse {
-        let id = region.make_child_id(&self.text);
+        let id = region.make_position_id();
         let text_style = TextStyle::Button;
         let font = &region.fonts()[text_style];
         let (text, text_size) = font.layout_multiline(&self.text, region.width());
@@ -126,7 +126,7 @@ impl<'a> Checkbox<'a> {
 
 impl<'a> Widget for Checkbox<'a> {
     fn add_to(self, region: &mut Region) -> GuiResponse {
-        let id = region.make_child_id(&self.text);
+        let id = region.make_position_id();
         let text_style = TextStyle::Button;
         let font = &region.fonts()[text_style];
         let (text, text_size) = font.layout_multiline(&self.text, region.width());
@@ -200,7 +200,7 @@ pub fn radio<S: Into<String>>(checked: bool, text: S) -> RadioButton {
 
 impl Widget for RadioButton {
     fn add_to(self, region: &mut Region) -> GuiResponse {
-        let id = region.make_child_id(&self.text);
+        let id = region.make_position_id();
         let text_style = TextStyle::Button;
         let font = &region.fonts()[text_style];
         let (text, text_size) = font.layout_multiline(&self.text, region.width());
@@ -243,11 +243,14 @@ impl Widget for RadioButton {
 
 // ----------------------------------------------------------------------------
 
+/// Combined into one function (rather than two) to make it easier
+/// for the borrow checker.
+type SliderGetSet<'a> = Box<dyn 'a + FnMut(Option<f32>) -> f32>;
+
 pub struct Slider<'a> {
-    get_set_value: Box<dyn 'a + FnMut(Option<f32>) -> f32>,
+    get_set_value: SliderGetSet<'a>,
     min: f32,
     max: f32,
-    id: Option<Id>,
     text: Option<String>,
     precision: usize,
     text_color: Option<Color>,
@@ -255,17 +258,11 @@ pub struct Slider<'a> {
 }
 
 impl<'a> Slider<'a> {
-    pub fn f32(value: &'a mut f32, min: f32, max: f32) -> Self {
+    fn from_get_set(get_set_value: impl 'a + FnMut(Option<f32>) -> f32) -> Self {
         Slider {
-            get_set_value: Box::new(move |v: Option<f32>| {
+            get_set_value: Box::new(get_set_value),
-                if let Some(v) = v {
+            min: std::f32::NAN,
-                    *value = v
+            max: std::f32::NAN,
-                }
-                *value
-            }),
-            min,
-            max,
-            id: None,
             text: None,
             precision: 3,
             text_on_top: None,
@@ -273,47 +270,48 @@ impl<'a> Slider<'a> {
         }
     }
 
+    pub fn f32(value: &'a mut f32, min: f32, max: f32) -> Self {
+        Slider {
+            min,
+            max,
+            precision: 3,
+            ..Self::from_get_set(move |v: Option<f32>| {
+                if let Some(v) = v {
+                    *value = v
+                }
+                *value
+            })
+        }
+    }
+
     pub fn i32(value: &'a mut i32, min: i32, max: i32) -> Self {
         Slider {
-            get_set_value: Box::new(move |v: Option<f32>| {
+            min: min as f32,
+            max: max as f32,
+            precision: 0,
+            ..Self::from_get_set(move |v: Option<f32>| {
                 if let Some(v) = v {
                     *value = v.round() as i32
                 }
                 *value as f32
-            }),
+            })
-            min: min as f32,
-            max: max as f32,
-            id: None,
-            text: None,
-            precision: 0,
-            text_on_top: None,
-            text_color: None,
         }
     }
 
     pub fn usize(value: &'a mut usize, min: usize, max: usize) -> Self {
         Slider {
-            get_set_value: Box::new(move |v: Option<f32>| {
+            min: min as f32,
+            max: max as f32,
+            precision: 0,
+            ..Self::from_get_set(move |v: Option<f32>| {
                 if let Some(v) = v {
                     *value = v.round() as usize
                 }
                 *value as f32
-            }),
+            })
-            min: min as f32,
-            max: max as f32,
-            id: None,
-            text: None,
-            precision: 0,
-            text_on_top: None,
-            text_color: None,
         }
     }
 
-    pub fn id(mut self, id: Id) -> Self {
-        self.id = Some(id);
-        self
-    }
-
     pub fn text<S: Into<String>>(mut self, text: S) -> Self {
         self.text = Some(text.into());
         self
@@ -351,10 +349,8 @@ impl<'a> Widget for Slider<'a> {
             let text_color = self.text_color;
             let value = (self.get_set_value)(None);
             let full_text = format!("{}: {:.*}", text, self.precision, value);
-            let id = Some(self.id.unwrap_or_else(|| Id::new(text)));
+
-            let mut naked = self;
+            let naked = Slider { text: None, ..self };
-            naked.id = id;
-            naked.text = None;
 
             if text_on_top {
                 let (text, text_size) = font.layout_multiline(&full_text, region.width());
@@ -379,16 +375,13 @@ impl<'a> Widget for Slider<'a> {
             let min = self.min;
             let max = self.max;
             debug_assert!(min <= max);
-            let id = region.combined_id(Some(
+            let id = region.make_position_id();
-                self.id
-                    .expect("Sliders must have a text label or an explicit id"),
-            ));
             let interact = region.reserve_space(
                 Vec2 {
                     x: region.available_space.x,
                     y: height,
                 },
-                id,
+                Some(id),
             );
 
             if let Some(mouse_pos) = region.input().mouse_pos {

emigui/src/window.rs

@@ -8,28 +8,39 @@ pub struct WindowState {
     pub rect: Rect,
 }
 
+#[derive(Clone, Debug, Default)]
 pub struct Window {
     /// The title of the window and by default the source of its identity.
     title: String,
+    /// Put the window here the first time
+    default_pos: Option<Pos2>,
 }
 
 impl Window {
     pub fn new<S: Into<String>>(title: S) -> Self {
         Self {
             title: title.into(),
+            ..Default::default()
         }
     }
 
+    pub fn default_pos(mut self, default_pos: Pos2) -> Self {
+        self.default_pos = Some(default_pos);
+        self
+    }
+
     pub fn show<F>(self, ctx: &Arc<Context>, add_contents: F)
     where
         F: FnOnce(&mut Region),
     {
-        let id = Id::new(&self.title);
+        let default_pos = self.default_pos.unwrap_or(pos2(100.0, 100.0)); // TODO
+
+        let id = ctx.make_unique_id(&self.title, default_pos);
 
         let mut state = ctx.memory.lock().get_or_create_window(
             id,
             Rect::from_min_size(
-                pos2(400.0, 200.0), // TODO
+                default_pos,
                 vec2(200.0, 200.0), // TODO
             ),
         );

example_glium/src/main.rs

@@ -81,7 +81,7 @@ fn main() {
         });
 
         emigui.new_frame(raw_input);
-        let mut region = emigui.whole_screen_region();
+        let mut region = emigui.background_region();
         let mut region = region.left_column(region.width().min(480.0));
         region.set_align(Align::Min);
         region.add(label!("Emigui running inside of Glium").text_style(emigui::TextStyle::Heading));
@@ -95,10 +95,12 @@ fn main() {
         Window::new("Test window").show(region.ctx(), |region| {
             region.add(label!("Grab the window and move it around!"));
         });
-        Window::new("Another test window").show(region.ctx(), |region| {
+        Window::new("Another test window")
-            region.add(label!("This might be on top of the other window?"));
+            .default_pos(pos2(400.0, 100.0))
-            region.add(label!("Second line of text"));
+            .show(region.ctx(), |region| {
-        });
+                region.add(label!("This might be on top of the other window?"));
+                region.add(label!("Second line of text"));
+            });
 
         let mesh = emigui.paint();
         painter.paint(&display, mesh, emigui.texture());

example_wasm/src/lib.rs

@@ -42,7 +42,7 @@ impl State {
 
         self.emigui.new_frame(raw_input);
 
-        let mut region = self.emigui.whole_screen_region();
+        let mut region = self.emigui.background_region();
         let mut region = region.centered_column(region.width().min(480.0));
         region.add(label!("Emigui!").text_style(TextStyle::Heading));
         region.add(label!("Emigui is an immediate mode GUI written in Rust, compiled to WebAssembly, rendered with WebGL."));