Skip to content

Commit

Permalink
Only show id clash warnings in debug builds by default (#2930)
Browse files Browse the repository at this point in the history
Change with `egui_ctx.options_mut(|opt| opt.warn_on_id_clash = true);`
  • Loading branch information
emilk authored Apr 19, 2023
1 parent d486c76 commit b8e7987
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 55 deletions.
113 changes: 59 additions & 54 deletions crates/egui/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,67 +463,72 @@ impl Context {
/// because that's where the warning will be painted. If you don't know what size to pick, just pick [`Vec2::ZERO`].
pub fn check_for_id_clash(&self, id: Id, new_rect: Rect, what: &str) {
let prev_rect = self.frame_state_mut(move |state| state.used_ids.insert(id, new_rect));
if let Some(prev_rect) = prev_rect {
// it is ok to reuse the same ID for e.g. a frame around a widget,
// or to check for interaction with the same widget twice:
if prev_rect.expand(0.1).contains_rect(new_rect)
|| new_rect.expand(0.1).contains_rect(prev_rect)
{
return;
}

let show_error = |widget_rect: Rect, text: String| {
let text = format!("🔥 {}", text);
let color = self.style().visuals.error_fg_color;
let painter = self.debug_painter();
painter.rect_stroke(widget_rect, 0.0, (1.0, color));

let below = widget_rect.bottom() + 32.0 < self.input(|i| i.screen_rect.bottom());

let text_rect = if below {
painter.debug_text(
widget_rect.left_bottom() + vec2(0.0, 2.0),
Align2::LEFT_TOP,
color,
text,
)
} else {
painter.debug_text(
widget_rect.left_top() - vec2(0.0, 2.0),
Align2::LEFT_BOTTOM,
color,
text,
)
};

if let Some(pointer_pos) = self.pointer_hover_pos() {
if text_rect.contains(pointer_pos) {
let tooltip_pos = if below {
text_rect.left_bottom() + vec2(2.0, 4.0)
} else {
text_rect.left_top() + vec2(2.0, -4.0)
};

painter.error(
tooltip_pos,
format!("Widget is {} this text.\n\n\
if !self.options(|opt| opt.warn_on_id_clash) {
return;
}

let Some(prev_rect) = prev_rect else { return };

// it is ok to reuse the same ID for e.g. a frame around a widget,
// or to check for interaction with the same widget twice:
if prev_rect.expand(0.1).contains_rect(new_rect)
|| new_rect.expand(0.1).contains_rect(prev_rect)
{
return;
}

let show_error = |widget_rect: Rect, text: String| {
let text = format!("🔥 {}", text);
let color = self.style().visuals.error_fg_color;
let painter = self.debug_painter();
painter.rect_stroke(widget_rect, 0.0, (1.0, color));

let below = widget_rect.bottom() + 32.0 < self.input(|i| i.screen_rect.bottom());

let text_rect = if below {
painter.debug_text(
widget_rect.left_bottom() + vec2(0.0, 2.0),
Align2::LEFT_TOP,
color,
text,
)
} else {
painter.debug_text(
widget_rect.left_top() - vec2(0.0, 2.0),
Align2::LEFT_BOTTOM,
color,
text,
)
};

if let Some(pointer_pos) = self.pointer_hover_pos() {
if text_rect.contains(pointer_pos) {
let tooltip_pos = if below {
text_rect.left_bottom() + vec2(2.0, 4.0)
} else {
text_rect.left_top() + vec2(2.0, -4.0)
};

painter.error(
tooltip_pos,
format!("Widget is {} this text.\n\n\
ID clashes happens when things like Windows or CollapsingHeaders share names,\n\
or when things like Plot and Grid:s aren't given unique id_source:s.\n\n\
Sometimes the solution is to use ui.push_id.",
if below { "above" } else { "below" })
);
}
if below { "above" } else { "below" })
);
}
};
}
};

let id_str = id.short_debug_format();
let id_str = id.short_debug_format();

if prev_rect.min.distance(new_rect.min) < 4.0 {
show_error(new_rect, format!("Double use of {} ID {}", what, id_str));
} else {
show_error(prev_rect, format!("First use of {} ID {}", what, id_str));
show_error(new_rect, format!("Second use of {} ID {}", what, id_str));
}
if prev_rect.min.distance(new_rect.min) < 4.0 {
show_error(new_rect, format!("Double use of {} ID {}", what, id_str));
} else {
show_error(prev_rect, format!("First use of {} ID {}", what, id_str));
show_error(new_rect, format!("Second use of {} ID {}", what, id_str));
}
}

Expand Down
1 change: 0 additions & 1 deletion crates/egui/src/frame_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ pub(crate) struct AccessKitFrameState {
#[derive(Clone)]
pub(crate) struct FrameState {
/// All [`Id`]s that were used this frame.
/// Used to debug [`Id`] clashes of widgets.
pub(crate) used_ids: IdMap<Rect>,

/// Starts off as the screen_rect, shrinks as panels are added.
Expand Down
6 changes: 6 additions & 0 deletions crates/egui/src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ pub struct Options {
/// This can lead to fewer texture operations, but may use up the texture atlas quicker
/// if you are changing [`Style::text_styles`], of have a lot of text styles.
pub preload_font_glyphs: bool,

/// Check reusing of [`Id`]s, and show a visual warning on screen when one is found.
///
/// By default this is `true` in debug builds.
pub warn_on_id_clash: bool,
}

impl Default for Options {
Expand All @@ -130,6 +135,7 @@ impl Default for Options {
tessellation_options: Default::default(),
screen_reader: false,
preload_font_glyphs: true,
warn_on_id_clash: cfg!(debug_assertions),
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions crates/egui_demo_lib/src/demo/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ impl super::Demo for IdTest {

impl super::View for IdTest {
fn ui(&mut self, ui: &mut egui::Ui) {
// Make sure the warnings are on (by default they are only on in debug builds).
ui.ctx().options_mut(|opt| opt.warn_on_id_clash = true);

ui.heading("Name collision example");

ui.label("\
Expand Down

0 comments on commit b8e7987

Please sign in to comment.