Conversation
|
Preview available at https://egui-pr-preview.github.io/pr/5851-lucasexperimentshelpful-id-debug View snapshot changes at kitdiff |
|
I love this! |
# Conflicts: # crates/egui/src/containers/combo_box.rs # crates/egui/src/id.rs # crates/egui/src/lib.rs # crates/egui/src/ui.rs # crates/egui/src/ui_builder.rs
# Conflicts: # crates/egui/src/ui_builder.rs
|
It's currently gated behind debug_assertions, but maybe it should be a feature flag instead? Or both? |
emilk
left a comment
emilk
left a comment
There was a problem hiding this comment.
Awesome! I think having this always-on in debug builds makes sense. The overhead is one extra lookup in a hashmap, which should be fine (at least once you change it from ahash::HashMap to an IdMap.
There was a problem hiding this comment.
That's not much of a hierarchy though. What is even 152204… there?
There was a problem hiding this comment.
Is that the auto-id? If so, we should maybe create a special path for creating auto-ids just to improve this formatting
There was a problem hiding this comment.
Will be fixed by #7988.
We could also add a extra Id constructor to pass more metadata, so we could display if it's an auto id, or the id for a Ui or Layer (some of this could also be derived by looking if any of those exist in the egui context for the id)
|
Crossposting this comment: Adding DNM until we have a screenshot test with a nicer UI :] |
2 participants
Eguis Ids are awesome, but sometimes it's a pain to figure out where a particular Id is coming from. This is an experiment at improving the situation by requiring the Id's source to implemnt
Debug, storing the Debug representation as a string and the Ids optional parent in a global map, and showing the additional info it in the Debug implementation. Since most Ids are derived from a string this works really well:I also plan on adding a
Id::uithat always shows the source and parent and more info on hover (the parents parent and maybe it could also look for widget rects with the Id and higlight those).Since the String only has to be created once for each unique Id this doesn't seem to have a huge performance impact, so I feel we can always enable this in debug builds.
The downside is of course that creating an Id from something now requires that thing to implement debug, but I don't think it would be a huge problem in practise. We could also add a
Id::new_without_debug_info()that just requires the Hash trait.Also, the current Ui auto ids don't work well with this since they just implement an u64, but I'm sure there is a nice way to make that work.