Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only show id clash warnings in debug builds by default #2930

Merged
merged 1 commit into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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