From f4dcb996da5edc816942a8ee0f3655248618f559 Mon Sep 17 00:00:00 2001 From: lucasmerlin Date: Thu, 27 Mar 2025 11:20:24 +0100 Subject: [PATCH] Do bookkeeping of Ids, showing the Debug of it's source --- .../egui/src/containers/collapsing_header.rs | 5 +- crates/egui/src/containers/combo_box.rs | 7 +- crates/egui/src/containers/resize.rs | 5 +- crates/egui/src/containers/scroll_area.rs | 4 +- crates/egui/src/grid.rs | 3 +- crates/egui/src/id.rs | 114 +++++++++++++++++- crates/egui/src/interaction.rs | 2 +- crates/egui/src/lib.rs | 2 +- crates/egui/src/ui.rs | 20 +-- crates/egui/src/ui_builder.rs | 6 +- crates/egui/src/viewport.rs | 2 +- crates/egui/src/widgets/text_edit/builder.rs | 4 +- crates/egui_extras/src/table.rs | 4 +- 13 files changed, 142 insertions(+), 36 deletions(-) diff --git a/crates/egui/src/containers/collapsing_header.rs b/crates/egui/src/containers/collapsing_header.rs index c34e56294..6dae4849a 100644 --- a/crates/egui/src/containers/collapsing_header.rs +++ b/crates/egui/src/containers/collapsing_header.rs @@ -1,3 +1,4 @@ +use std::fmt::Debug; use std::hash::Hash; use crate::{ @@ -446,7 +447,7 @@ impl CollapsingHeader { /// Explicitly set the source of the [`Id`] of this widget, instead of using title label. /// This is useful if the title label is dynamic or not unique. #[inline] - pub fn id_salt(mut self, id_salt: impl Hash) -> Self { + pub fn id_salt(mut self, id_salt: impl Hash + Debug) -> Self { self.id_salt = Id::new(id_salt); self } @@ -455,7 +456,7 @@ impl CollapsingHeader { /// This is useful if the title label is dynamic or not unique. #[deprecated = "Renamed id_salt"] #[inline] - pub fn id_source(mut self, id_salt: impl Hash) -> Self { + pub fn id_source(mut self, id_salt: impl Hash + Debug) -> Self { self.id_salt = Id::new(id_salt); self } diff --git a/crates/egui/src/containers/combo_box.rs b/crates/egui/src/containers/combo_box.rs index 7a5a160f9..3730d4d20 100644 --- a/crates/egui/src/containers/combo_box.rs +++ b/crates/egui/src/containers/combo_box.rs @@ -1,4 +1,5 @@ use epaint::Shape; +use std::fmt::Debug; use crate::{ epaint, style::StyleModifier, style::WidgetVisuals, vec2, Align2, Context, Id, InnerResponse, @@ -43,7 +44,7 @@ pub struct ComboBox { impl ComboBox { /// Create new [`ComboBox`] with id and label - pub fn new(id_salt: impl std::hash::Hash, label: impl Into) -> Self { + pub fn new(id_salt: impl std::hash::Hash + Debug, label: impl Into) -> Self { Self { id_salt: Id::new(id_salt), label: Some(label.into()), @@ -72,7 +73,7 @@ impl ComboBox { } /// Without label. - pub fn from_id_salt(id_salt: impl std::hash::Hash) -> Self { + pub fn from_id_salt(id_salt: impl std::hash::Hash + Debug) -> Self { Self { id_salt: Id::new(id_salt), label: Default::default(), @@ -87,7 +88,7 @@ impl ComboBox { /// Without label. #[deprecated = "Renamed id_salt"] - pub fn from_id_source(id_salt: impl std::hash::Hash) -> Self { + pub fn from_id_source(id_salt: impl std::hash::Hash + Debug) -> Self { Self::from_id_salt(id_salt) } diff --git a/crates/egui/src/containers/resize.rs b/crates/egui/src/containers/resize.rs index 0df9a1136..577234f08 100644 --- a/crates/egui/src/containers/resize.rs +++ b/crates/egui/src/containers/resize.rs @@ -2,6 +2,7 @@ use crate::{ pos2, vec2, Align2, Color32, Context, CursorIcon, Id, NumExt, Rect, Response, Sense, Shape, Ui, UiBuilder, UiKind, UiStackInfo, Vec2, Vec2b, }; +use std::fmt::Debug; #[derive(Clone, Copy, Debug)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] @@ -72,13 +73,13 @@ impl Resize { /// A source for the unique [`Id`], e.g. `.id_source("second_resize_area")` or `.id_source(loop_index)`. #[inline] #[deprecated = "Renamed id_salt"] - pub fn id_source(self, id_salt: impl std::hash::Hash) -> Self { + pub fn id_source(self, id_salt: impl std::hash::Hash + Debug) -> Self { self.id_salt(id_salt) } /// A source for the unique [`Id`], e.g. `.id_salt("second_resize_area")` or `.id_salt(loop_index)`. #[inline] - pub fn id_salt(mut self, id_salt: impl std::hash::Hash) -> Self { + pub fn id_salt(mut self, id_salt: impl std::hash::Hash + Debug) -> Self { self.id_salt = Some(Id::new(id_salt)); self } diff --git a/crates/egui/src/containers/scroll_area.rs b/crates/egui/src/containers/scroll_area.rs index c430b6ea9..2940d90ab 100644 --- a/crates/egui/src/containers/scroll_area.rs +++ b/crates/egui/src/containers/scroll_area.rs @@ -306,13 +306,13 @@ impl ScrollArea { /// A source for the unique [`Id`], e.g. `.id_source("second_scroll_area")` or `.id_source(loop_index)`. #[inline] #[deprecated = "Renamed id_salt"] - pub fn id_source(self, id_salt: impl std::hash::Hash) -> Self { + pub fn id_source(self, id_salt: impl crate::IdTrait) -> Self { self.id_salt(id_salt) } /// A source for the unique [`Id`], e.g. `.id_salt("second_scroll_area")` or `.id_salt(loop_index)`. #[inline] - pub fn id_salt(mut self, id_salt: impl std::hash::Hash) -> Self { + pub fn id_salt(mut self, id_salt: impl crate::IdTrait) -> Self { self.id_salt = Some(Id::new(id_salt)); self } diff --git a/crates/egui/src/grid.rs b/crates/egui/src/grid.rs index 9ad386cc6..201d5a9a1 100644 --- a/crates/egui/src/grid.rs +++ b/crates/egui/src/grid.rs @@ -1,4 +1,5 @@ use emath::GuiRounding as _; +use std::fmt::Debug; use crate::{ vec2, Align2, Color32, Context, Id, InnerResponse, NumExt, Painter, Rect, Region, Style, Ui, @@ -322,7 +323,7 @@ pub struct Grid { impl Grid { /// Create a new [`Grid`] with a locally unique identifier. - pub fn new(id_salt: impl std::hash::Hash) -> Self { + pub fn new(id_salt: impl std::hash::Hash + Debug) -> Self { Self { id_salt: Id::new(id_salt), num_columns: None, diff --git a/crates/egui/src/id.rs b/crates/egui/src/id.rs index ddc1a59b7..40deda94b 100644 --- a/crates/egui/src/id.rs +++ b/crates/egui/src/id.rs @@ -1,6 +1,11 @@ // TODO(emilk): have separate types `PositionId` and `UniqueId`. ? +use ahash::HashMap; +use epaint::mutex::{Mutex, RwLock}; +use std::any::TypeId; +use std::hash::Hasher; use std::num::NonZeroU64; +use std::sync::LazyLock; /// egui tracks widgets frame-to-frame using [`Id`]s. /// @@ -33,8 +38,22 @@ use std::num::NonZeroU64; #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub struct Id(NonZeroU64); +enum IdSource { + Id(Id), + Other(String), +} + +static ID_MAP: LazyLock)>>> = LazyLock::new(|| { + let mut map = HashMap::default(); + map.insert(Id::NULL, (IdSource::Other("Id::NULL".to_owned()), None)); + RwLock::new(map) +}); + impl nohash_hasher::IsEnabled for Id {} +pub trait IdTrait: std::hash::Hash + std::fmt::Debug {} +impl IdTrait for T {} + impl Id { /// A special [`Id`], in particular as a key to [`crate::Memory::data`] /// for when there is no particular widget to attach the data. @@ -52,18 +71,84 @@ impl Id { } } + /// Checks if [`T`] is a [`Id`]. + /// + /// If it is, it returns `IdSource::Id`, otherwise it returns `IdSource::Other`. + fn get_source(t: T) -> IdSource { + /// Ugly hack to try to determine if T is an Id or not. + struct FakeHasher { + val: Option, + first: bool, + } + + impl Hasher for FakeHasher { + fn finish(&self) -> u64 { + unreachable!() + } + + fn write(&mut self, bytes: &[u8]) { + self.first = false; + } + + fn write_u64(&mut self, i: u64) { + if self.first { + self.val = Some(i); + self.first = false; + } else { + self.val = None; + } + } + } + + let mut hasher = FakeHasher { + val: None, + first: true, + }; + + t.hash(&mut hasher); + + let maybe_source_id = hasher.val.map(Id::from_hash); + + // Ideally we would just implement IdTriat for Id with specialization, but that's not + // a thing yet :( So we check if the hash is already in the map, if so, the source must be + // an Id. + if let Some(maybe_source_id) = maybe_source_id { + if ID_MAP.read().contains_key(&maybe_source_id) { + IdSource::Id(maybe_source_id) + } else { + IdSource::Other(format!("{:?}", t)) + } + } else { + IdSource::Other(format!("{:?}", t)) + } + } + /// Generate a new [`Id`] by hashing some source (e.g. a string or integer). - pub fn new(source: impl std::hash::Hash) -> Self { - Self::from_hash(ahash::RandomState::with_seeds(1, 2, 3, 4).hash_one(source)) + pub fn new(source: T) -> Self { + let id = Self::from_hash(ahash::RandomState::with_seeds(1, 2, 3, 4).hash_one(&source)); + + if !ID_MAP.read().contains_key(&id) { + let source = Self::get_source(source); + ID_MAP.write().insert(id, (source, None)); + } + + id } /// Generate a new [`Id`] by hashing the parent [`Id`] and the given argument. - pub fn with(self, child: impl std::hash::Hash) -> Self { + pub fn with(self, child: impl std::hash::Hash + std::fmt::Debug) -> Self { use std::hash::{BuildHasher, Hasher}; let mut hasher = ahash::RandomState::with_seeds(1, 2, 3, 4).build_hasher(); hasher.write_u64(self.0.get()); - child.hash(&mut hasher); - Self::from_hash(hasher.finish()) + (&child).hash(&mut hasher); + let id = Self::from_hash(hasher.finish()); + + if !ID_MAP.read().contains_key(&id) { + let source = Self::get_source(child); + ID_MAP.write().insert(id, (source, Some(self))); + } + + id } /// Short and readable summary @@ -87,7 +172,24 @@ impl Id { impl std::fmt::Debug for Id { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{:04X}", self.value() as u16) + write!(f, "{:04X}", self.value() as u16)?; + let lock = ID_MAP.read(); + if let Some((source, parent)) = lock.get(self) { + match source { + IdSource::Id(source_id) => { + write!(f, "({:?})", source_id)?; + } + IdSource::Other(label) => { + write!(f, " ({})", label)?; + } + } + if let Some(parent) = parent { + // Let's hope there are no cycles! + write!(f, " <- {:?}", parent)?; + } + } + + Ok(()) } } diff --git a/crates/egui/src/interaction.rs b/crates/egui/src/interaction.rs index b6b63e19d..64a789bd7 100644 --- a/crates/egui/src/interaction.rs +++ b/crates/egui/src/interaction.rs @@ -70,7 +70,7 @@ impl InteractionSnapshot { fn id_ui<'a>(ui: &mut crate::Ui, widgets: impl IntoIterator) { for id in widgets { - ui.label(id.short_debug_format()); + ui.label(format!("{:?}", id)); } } diff --git a/crates/egui/src/lib.rs b/crates/egui/src/lib.rs index 0d8459808..838f0dbd6 100644 --- a/crates/egui/src/lib.rs +++ b/crates/egui/src/lib.rs @@ -492,7 +492,7 @@ pub use self::{ drag_and_drop::DragAndDrop, epaint::text::TextWrapMode, grid::Grid, - id::{Id, IdMap}, + id::{Id, IdMap, IdTrait}, input_state::{InputState, MultiTouchInfo, PointerState}, layers::{LayerId, Order}, layout::*, diff --git a/crates/egui/src/ui.rs b/crates/egui/src/ui.rs index 0e8509bb8..ecc4d4dfa 100644 --- a/crates/egui/src/ui.rs +++ b/crates/egui/src/ui.rs @@ -1,10 +1,6 @@ #![warn(missing_docs)] // Let's keep `Ui` well-documented. #![allow(clippy::use_self)] -use emath::GuiRounding as _; -use epaint::mutex::RwLock; -use std::{any::Any, hash::Hash, sync::Arc}; - use crate::close_tag::ClosableTag; use crate::containers::menu; #[cfg(debug_assertions)] @@ -30,6 +26,10 @@ use crate::{ Style, TextStyle, TextWrapMode, UiBuilder, UiKind, UiStack, UiStackInfo, Vec2, WidgetRect, WidgetText, }; +use emath::GuiRounding as _; +use epaint::mutex::RwLock; +use std::fmt::Debug; +use std::{any::Any, hash::Hash, sync::Arc}; // ---------------------------------------------------------------------------- /// This is what you use to place widgets. @@ -226,7 +226,7 @@ impl Ui { &mut self, max_rect: Rect, layout: Layout, - id_salt: impl Hash, + id_salt: impl Hash + Debug, ui_stack_info: Option, ) -> Self { self.new_child( @@ -1036,7 +1036,7 @@ impl Ui { /// Use this to generate widget ids for widgets that have persistent state in [`Memory`]. pub fn make_persistent_id(&self, id_salt: IdSource) -> Id where - IdSource: Hash, + IdSource: Hash + Debug, { self.id.with(&id_salt) } @@ -1049,7 +1049,7 @@ impl Ui { /// Same as `ui.next_auto_id().with(id_salt)` pub fn auto_id_with(&self, id_salt: IdSource) -> Id where - IdSource: Hash, + IdSource: Hash + Debug, { Id::new(self.next_auto_id_salt).with(id_salt) } @@ -2373,7 +2373,7 @@ impl Ui { /// ``` pub fn push_id( &mut self, - id_salt: impl Hash, + id_salt: impl Hash + Debug, add_contents: impl FnOnce(&mut Ui) -> R, ) -> InnerResponse { self.scope_dyn(UiBuilder::new().id_salt(id_salt), Box::new(add_contents)) @@ -2473,7 +2473,7 @@ impl Ui { #[inline] pub fn indent( &mut self, - id_salt: impl Hash, + id_salt: impl Hash + Debug, add_contents: impl FnOnce(&mut Ui) -> R, ) -> InnerResponse { self.indent_dyn(id_salt, Box::new(add_contents)) @@ -2481,7 +2481,7 @@ impl Ui { fn indent_dyn<'c, R>( &mut self, - id_salt: impl Hash, + id_salt: impl Hash + Debug, add_contents: Box R + 'c>, ) -> InnerResponse { assert!( diff --git a/crates/egui/src/ui_builder.rs b/crates/egui/src/ui_builder.rs index 4d225ae4c..4385a23e1 100644 --- a/crates/egui/src/ui_builder.rs +++ b/crates/egui/src/ui_builder.rs @@ -1,9 +1,9 @@ -use std::{hash::Hash, sync::Arc}; - use crate::close_tag::ClosableTag; #[allow(unused_imports)] // Used for doclinks use crate::Ui; use crate::{Id, LayerId, Layout, Rect, Sense, Style, UiStackInfo}; +use std::fmt::Debug; +use std::{hash::Hash, sync::Arc}; /// Build a [`Ui`] as the child of another [`Ui`]. /// @@ -37,7 +37,7 @@ impl UiBuilder { /// You should give each [`Ui`] an `id_salt` that is unique /// within the parent, or give it none at all. #[inline] - pub fn id_salt(mut self, id_salt: impl Hash) -> Self { + pub fn id_salt(mut self, id_salt: impl Hash + Debug) -> Self { self.id_salt = Some(Id::new(id_salt)); self } diff --git a/crates/egui/src/viewport.rs b/crates/egui/src/viewport.rs index 9aba69910..d350e2dd9 100644 --- a/crates/egui/src/viewport.rs +++ b/crates/egui/src/viewport.rs @@ -131,7 +131,7 @@ impl ViewportId { pub const ROOT: Self = Self(Id::NULL); #[inline] - pub fn from_hash_of(source: impl std::hash::Hash) -> Self { + pub fn from_hash_of(source: impl crate::IdTrait) -> Self { Self(Id::new(source)) } } diff --git a/crates/egui/src/widgets/text_edit/builder.rs b/crates/egui/src/widgets/text_edit/builder.rs index d73ecd3f6..de8daf589 100644 --- a/crates/egui/src/widgets/text_edit/builder.rs +++ b/crates/egui/src/widgets/text_edit/builder.rs @@ -166,13 +166,13 @@ impl<'t> TextEdit<'t> { /// A source for the unique [`Id`], e.g. `.id_source("second_text_edit_field")` or `.id_source(loop_index)`. #[inline] - pub fn id_source(self, id_salt: impl std::hash::Hash) -> Self { + pub fn id_source(self, id_salt: impl crate::IdTrait) -> Self { self.id_salt(id_salt) } /// A source for the unique [`Id`], e.g. `.id_salt("second_text_edit_field")` or `.id_salt(loop_index)`. #[inline] - pub fn id_salt(mut self, id_salt: impl std::hash::Hash) -> Self { + pub fn id_salt(mut self, id_salt: impl crate::IdTrait) -> Self { self.id_salt = Some(Id::new(id_salt)); self } diff --git a/crates/egui_extras/src/table.rs b/crates/egui_extras/src/table.rs index 5fca1f9df..afc85331b 100644 --- a/crates/egui_extras/src/table.rs +++ b/crates/egui_extras/src/table.rs @@ -275,7 +275,7 @@ impl<'a> TableBuilder<'a> { /// This is required if you have multiple tables in the same [`Ui`]. #[inline] #[deprecated = "Renamed id_salt"] - pub fn id_source(self, id_salt: impl std::hash::Hash) -> Self { + pub fn id_source(self, id_salt: impl egui::IdTrait) -> Self { self.id_salt(id_salt) } @@ -283,7 +283,7 @@ impl<'a> TableBuilder<'a> { /// /// This is required if you have multiple tables in the same [`Ui`]. #[inline] - pub fn id_salt(mut self, id_salt: impl std::hash::Hash) -> Self { + pub fn id_salt(mut self, id_salt: impl egui::IdTrait) -> Self { self.id_salt = Id::new(id_salt); self }