From 4ca5e497227ce7a63b212bbfe7b3fb07d2bd606c Mon Sep 17 00:00:00 2001 From: lucasmerlin Date: Thu, 27 Mar 2025 16:05:37 +0100 Subject: [PATCH] Add IdInfo and only enable this with debug_assertions --- crates/egui/src/containers/scroll_area.rs | 4 +- crates/egui/src/id.rs | 230 ++++++++++++------- crates/egui/src/lib.rs | 2 +- crates/egui/src/viewport.rs | 2 +- crates/egui/src/widgets/text_edit/builder.rs | 4 +- crates/egui_extras/src/table.rs | 4 +- 6 files changed, 152 insertions(+), 94 deletions(-) diff --git a/crates/egui/src/containers/scroll_area.rs b/crates/egui/src/containers/scroll_area.rs index 2940d90ab..d24d5f57c 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 crate::IdTrait) -> Self { + pub fn id_source(self, id_salt: impl crate::AsId) -> 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 crate::IdTrait) -> Self { + pub fn id_salt(mut self, id_salt: impl crate::AsId) -> Self { self.id_salt = Some(Id::new(id_salt)); self } diff --git a/crates/egui/src/id.rs b/crates/egui/src/id.rs index 40deda94b..454ceeeab 100644 --- a/crates/egui/src/id.rs +++ b/crates/egui/src/id.rs @@ -1,11 +1,6 @@ // 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. /// @@ -38,21 +33,10 @@ use std::sync::LazyLock; #[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 {} +pub trait AsId: std::hash::Hash + std::fmt::Debug {} +impl AsId for T {} impl Id { /// A special [`Id`], in particular as a key to [`crate::Memory::data`] @@ -71,82 +55,26 @@ 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: T) -> Self { + 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)); - } + #[cfg(debug_assertions)] + id_source::maybe_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 + std::fmt::Debug) -> Self { + pub fn with(self, child: impl AsId) -> 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); 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))); - } + #[cfg(debug_assertions)] + id_source::maybe_insert(id, &child, Some(self)); id } @@ -168,22 +96,152 @@ impl Id { pub(crate) fn accesskit_id(&self) -> accesskit::NodeId { self.value().into() } + + // TODO: Nice debug ui + // pub fn ui(self, ui: &mut crate::Ui) -> crate::Response { + // ui.code(self.short_debug_format()).on_hover_ui(|ui| { + // let data = self.info(); + // }) + // } +} + +#[cfg(debug_assertions)] +mod id_source { + use crate::{AsId, Id}; + use ahash::HashMap; + use epaint::mutex::RwLock; + use std::hash::Hasher; + use std::sync::LazyLock; + + #[derive(Clone)] + pub struct IdInfo { + /// What was this Id generated from? + pub source: IdSource, + /// If the Id was crated via [`Id::with`], what was the parent Id? + pub parent: Option, + } + + #[derive(Clone)] + pub enum IdSource { + Id(Id), + Other(String), + } + + static ID_MAP: LazyLock>> = LazyLock::new(|| { + let mut map = HashMap::default(); + map.insert( + Id::NULL, + IdInfo { + source: IdSource::Other("Id::NULL".to_owned()), + parent: None, + }, + ); + RwLock::new(map) + }); + + /// Ugly hack to try to determine if T is an Id or not. + #[derive(Default)] + struct ExtractIdHasher { + val: Option, + not_id: bool, + } + + impl ExtractIdHasher { + fn id(&self) -> Option { + self.val.map(Id::from_hash) + } + } + + impl Hasher for ExtractIdHasher { + fn finish(&self) -> u64 { + unreachable!() + } + + fn write(&mut self, _bytes: &[u8]) { + self.not_id = true; + self.val = None; + } + + fn write_u64(&mut self, i: u64) { + if !self.not_id && !self.val.is_some() { + self.val = Some(i); + } else { + self.not_id = true; + self.val = None; + } + } + } + + /// Checks if [`T`] is a [`Id`]. + /// + /// If it is, it returns `IdSource::Id`, otherwise it returns `IdSource::Other`. + fn get_source(t: T) -> IdSource { + let mut hasher = ExtractIdHasher::default(); + + t.hash(&mut hasher); + + let maybe_source_id = hasher.id(); + + // 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)) + } + } + + pub(super) fn maybe_insert(id: Id, source: impl AsId, parent: Option) { + if !ID_MAP.read().contains_key(&id) { + let source1 = get_source(source); + ID_MAP.write().insert( + id, + IdInfo { + source: source1, + parent, + }, + ); + } + } + + impl Id { + pub fn info(&self) -> Option { + ID_MAP.read().get(self).cloned() + } + } + + #[test] + fn test_fake_hasher() { + use std::hash::Hash; + let mut hasher = ExtractIdHasher::default(); + + let id = Id::new("test"); + id.hash(&mut hasher); + + assert_eq!(hasher.id(), Some(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)?; - let lock = ID_MAP.read(); - if let Some((source, parent)) = lock.get(self) { - match source { - IdSource::Id(source_id) => { + + #[cfg(debug_assertions)] + if let Some(info) = self.info() { + match info.source { + id_source::IdSource::Id(source_id) => { write!(f, "({:?})", source_id)?; } - IdSource::Other(label) => { + id_source::IdSource::Other(label) => { write!(f, " ({})", label)?; } } - if let Some(parent) = parent { + if let Some(parent) = info.parent { // Let's hope there are no cycles! write!(f, " <- {:?}", parent)?; } diff --git a/crates/egui/src/lib.rs b/crates/egui/src/lib.rs index 838f0dbd6..f12cb1e8c 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, IdTrait}, + id::{AsId, Id, IdMap}, input_state::{InputState, MultiTouchInfo, PointerState}, layers::{LayerId, Order}, layout::*, diff --git a/crates/egui/src/viewport.rs b/crates/egui/src/viewport.rs index d350e2dd9..1441ae584 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 crate::IdTrait) -> Self { + pub fn from_hash_of(source: impl crate::AsId) -> 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 de8daf589..a07961dd6 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 crate::IdTrait) -> Self { + pub fn id_source(self, id_salt: impl crate::AsId) -> 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 crate::IdTrait) -> Self { + pub fn id_salt(mut self, id_salt: impl crate::AsId) -> 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 afc85331b..4f429ea23 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 egui::IdTrait) -> Self { + pub fn id_source(self, id_salt: impl egui::AsId) -> 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 egui::IdTrait) -> Self { + pub fn id_salt(mut self, id_salt: impl egui::AsId) -> Self { self.id_salt = Id::new(id_salt); self }