From 6eae91e0282731564923831d539d4927e1715fd4 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Sun, 19 Apr 2020 11:13:24 +0200 Subject: [PATCH] Distinguish ids that need to be unique and warn about name clashes --- emigui/src/context.rs | 112 +++++++++++++++++++++++++++++++++++++- emigui/src/emigui.rs | 5 +- emigui/src/example_app.rs | 22 ++++++++ emigui/src/id.rs | 39 ++++++++++++- emigui/src/layout.rs | 18 +++++- emigui/src/math.rs | 14 +++-- emigui/src/region.rs | 54 ++++++++++-------- emigui/src/widgets.rs | 87 ++++++++++++++--------------- emigui/src/window.rs | 15 ++++- example_glium/src/main.rs | 12 ++-- example_wasm/src/lib.rs | 2 +- 11 files changed, 292 insertions(+), 88 deletions(-) diff --git a/emigui/src/context.rs b/emigui/src/context.rs index 37c9c84c..94e0ee93 100644 --- a/emigui/src/context.rs +++ b/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, pub(crate) graphics: Mutex, + + /// Used to debug name clashes of e.g. windows + used_ids: Mutex>, } +// 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(&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) -> 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, + ) -> 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, + color: Option, + ) { + 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 { diff --git a/emigui/src/emigui.rs b/emigui/src/emigui.rs index c6f2288b..38c3e94d 100644 --- a/emigui/src/emigui.rs +++ b/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(), diff --git a/emigui/src/example_app.rs b/emigui/src/example_app.rs index 64f363ac..c872b13a 100644 --- a/emigui/src/example_app.rs +++ b/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")); + }); } } diff --git a/emigui/src/id.rs b/emigui/src/id.rs index 8a25d354..5d1f93f0 100644 --- a/emigui/src/id.rs +++ b/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) + } } diff --git a/emigui/src/layout.rs b/emigui/src/layout.rs index e8d9c9dc..579849ca 100644 --- a/emigui/src/layout.rs +++ b/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, } @@ -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 diff --git a/emigui/src/math.rs b/emigui/src/math.rs index b154bd30..eaf8e463 100644 --- a/emigui/src/math.rs +++ b/emigui/src/math.rs @@ -5,6 +5,11 @@ pub struct Vec2 { } impl Vec2 { + pub fn splat(v: impl Into) -> 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 { - (a - b).length() + pub fn dist(self: Pos2, other: Pos2) -> f32 { + (self - other).length() } - pub fn dist_sq(a: Pos2, b: Pos2) -> f32 { - (a - b).length_sq() + pub fn dist_sq(self: Pos2, other: Pos2) -> f32 { + (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()) } diff --git a/emigui/src/region.rs b/emigui/src/region.rs index b0461284..ab24fb73 100644 --- a/emigui/src/region.rs +++ b/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(&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(&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(&self, child_id: &H) -> Id { self.id.with(child_id) } - pub fn combined_id(&self, child_id: Option) -> Option { - child_id.map(|child_id| self.id.with(&child_id)) - } - - // Helper function + /// Show some text anywhere in the region. + /// 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. pub fn floating_text( &mut self, pos: Pos2, @@ -373,22 +392,13 @@ impl Region { text_color: Option, ) -> Vec2 { let font = &self.fonts()[text_style]; - let (text, text_size) = font.layout_multiline(text, std::f32::INFINITY); - - let x = match align.0 { - Align::Min => pos.x, - 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 + let (text, size) = font.layout_multiline(text, std::f32::INFINITY); + let rect = align_rect(Rect::from_min_size(pos, size), align); + self.add_text(rect.min(), text_style, text, text_color); + size } + /// Already layed out text. pub fn add_text( &mut self, pos: Pos2, diff --git a/emigui/src/widgets.rs b/emigui/src/widgets.rs index 26d29e55..0920f99e 100644 --- a/emigui/src/widgets.rs +++ b/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 = ®ion.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 = ®ion.fonts()[text_style]; let (text, text_size) = font.layout_multiline(&self.text, region.width()); @@ -200,7 +200,7 @@ pub fn radio>(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 = ®ion.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) -> f32>; + pub struct Slider<'a> { - get_set_value: Box) -> f32>, + get_set_value: SliderGetSet<'a>, min: f32, max: f32, - id: Option, text: Option, precision: usize, text_color: Option, @@ -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) -> Self { Slider { - get_set_value: Box::new(move |v: Option| { - if let Some(v) = v { - *value = v - } - *value - }), - min, - max, - id: None, + get_set_value: Box::new(get_set_value), + min: std::f32::NAN, + max: std::f32::NAN, 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| { + 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| { + min: min as f32, + max: max as f32, + precision: 0, + ..Self::from_get_set(move |v: Option| { 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| { + min: min as f32, + max: max as f32, + precision: 0, + ..Self::from_get_set(move |v: Option| { 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>(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; - naked.id = id; - naked.text = None; + + let naked = Slider { text: None, ..self }; 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( - self.id - .expect("Sliders must have a text label or an explicit id"), - )); + let id = region.make_position_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 { diff --git a/emigui/src/window.rs b/emigui/src/window.rs index d0e7ca7f..e58da044 100644 --- a/emigui/src/window.rs +++ b/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, } impl Window { pub fn new>(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(self, ctx: &Arc, 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 ), ); diff --git a/example_glium/src/main.rs b/example_glium/src/main.rs index 2bf3c0a1..6a9e8e87 100644 --- a/example_glium/src/main.rs +++ b/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| { - region.add(label!("This might be on top of the other window?")); - region.add(label!("Second line of text")); - }); + Window::new("Another test window") + .default_pos(pos2(400.0, 100.0)) + .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()); diff --git a/example_wasm/src/lib.rs b/example_wasm/src/lib.rs index 5998bea8..3516dba8 100644 --- a/example_wasm/src/lib.rs +++ b/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."));