mirror of
https://github.com/emilk/egui.git
synced 2026-06-26 22:53:14 -04:00
Add DebugOptions::warn_if_rect_changes_id (#7984)
If turned on (default in debug builds), if the exact same `Rect` exist in two subsequent frames but with different `Id`s, a warning is logged an a red rect is flashed on the screen.
This commit is contained in:
@@ -2636,6 +2636,19 @@ impl ContextImpl {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(debug_assertions)]
|
||||
let shapes = if self.memory.options.style().debug.warn_if_rect_changes_id {
|
||||
let mut shapes = shapes;
|
||||
warn_if_rect_changes_id(
|
||||
&mut shapes,
|
||||
&viewport.prev_pass.widgets,
|
||||
&viewport.this_pass.widgets,
|
||||
);
|
||||
shapes
|
||||
} else {
|
||||
shapes
|
||||
};
|
||||
|
||||
std::mem::swap(&mut viewport.prev_pass, &mut viewport.this_pass);
|
||||
|
||||
if repaint_needed {
|
||||
@@ -4237,6 +4250,103 @@ fn context_impl_send_sync() {
|
||||
assert_send_sync::<Context>();
|
||||
}
|
||||
|
||||
/// Check if any [`Rect`] appears with different [`Id`]s between two passes.
|
||||
///
|
||||
/// This helps detect cases where the same screen area is claimed by different widget ids
|
||||
/// across passes, which is often a sign of id instability.
|
||||
#[cfg(debug_assertions)]
|
||||
fn warn_if_rect_changes_id(
|
||||
out_shapes: &mut Vec<ClippedShape>,
|
||||
prev_widgets: &crate::WidgetRects,
|
||||
new_widgets: &crate::WidgetRects,
|
||||
) {
|
||||
profiling::function_scope!();
|
||||
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
/// A wrapper around [`Rect`] that implements [`Ord`] using the bit representation of its floats.
|
||||
#[derive(Clone, Copy, PartialEq, Eq)]
|
||||
struct OrderedRect(Rect);
|
||||
|
||||
impl PartialOrd for OrderedRect {
|
||||
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
|
||||
Some(self.cmp(other))
|
||||
}
|
||||
}
|
||||
|
||||
impl Ord for OrderedRect {
|
||||
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
|
||||
let lhs = self.0;
|
||||
let rhs = other.0;
|
||||
lhs.min
|
||||
.x
|
||||
.to_bits()
|
||||
.cmp(&rhs.min.x.to_bits())
|
||||
.then(lhs.min.y.to_bits().cmp(&rhs.min.y.to_bits()))
|
||||
.then(lhs.max.x.to_bits().cmp(&rhs.max.x.to_bits()))
|
||||
.then(lhs.max.y.to_bits().cmp(&rhs.max.y.to_bits()))
|
||||
}
|
||||
}
|
||||
|
||||
fn create_lookup<'a>(
|
||||
widgets: impl Iterator<Item = &'a WidgetRect>,
|
||||
) -> BTreeMap<OrderedRect, Vec<&'a WidgetRect>> {
|
||||
let mut lookup: BTreeMap<OrderedRect, Vec<&'a WidgetRect>> = BTreeMap::default();
|
||||
for w in widgets {
|
||||
lookup.entry(OrderedRect(w.rect)).or_default().push(w);
|
||||
}
|
||||
lookup
|
||||
}
|
||||
|
||||
for (layer_id, new_layer_widgets) in new_widgets.layers() {
|
||||
let prev = create_lookup(prev_widgets.get_layer(*layer_id));
|
||||
let new = create_lookup(new_layer_widgets.iter());
|
||||
|
||||
for (hashable_rect, new_at_rect) in new {
|
||||
let Some(prev_at_rect) = prev.get(&hashable_rect) else {
|
||||
continue; // this rect did not exist in the previous pass
|
||||
};
|
||||
|
||||
if prev_at_rect
|
||||
.iter()
|
||||
.any(|w| new_at_rect.iter().any(|nw| nw.id == w.id))
|
||||
{
|
||||
continue; // at least one id stayed the same, so this is not an id change
|
||||
}
|
||||
|
||||
// Only warn if at least one of the previous ids is gone from this layer entirely.
|
||||
// If they all still exist (just at a different rect), then the rect match
|
||||
// is just a coincidence caused by widgets shifting (e.g. a window being dragged).
|
||||
if prev_at_rect.iter().all(|w| new_widgets.contains(w.id)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
let rect = new_at_rect[0].rect;
|
||||
|
||||
log::warn!(
|
||||
"Widget rect {rect:?} changed id between passes: prev ids: {:?}, new ids: {:?}",
|
||||
prev_at_rect
|
||||
.iter()
|
||||
.map(|w| w.id.short_debug_format())
|
||||
.collect::<Vec<_>>(),
|
||||
new_at_rect
|
||||
.iter()
|
||||
.map(|w| w.id.short_debug_format())
|
||||
.collect::<Vec<_>>(),
|
||||
);
|
||||
out_shapes.push(ClippedShape {
|
||||
clip_rect: Rect::EVERYTHING,
|
||||
shape: epaint::Shape::rect_stroke(
|
||||
rect,
|
||||
0,
|
||||
(2.0, Color32::RED),
|
||||
StrokeKind::Outside,
|
||||
),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod test {
|
||||
use super::Context;
|
||||
|
||||
@@ -1303,6 +1303,9 @@ pub struct DebugOptions {
|
||||
/// Show interesting widgets under the mouse cursor.
|
||||
pub show_widget_hits: bool,
|
||||
|
||||
/// Show a warning if the same `Rect` had different `Id` on the previous frame.
|
||||
pub warn_if_rect_changes_id: bool,
|
||||
|
||||
/// If true, highlight widgets that are not aligned to [`emath::GUI_ROUNDING`].
|
||||
///
|
||||
/// See [`emath::GuiRounding`] for more.
|
||||
@@ -1329,6 +1332,7 @@ impl Default for DebugOptions {
|
||||
show_resize: false,
|
||||
show_interactive_widgets: false,
|
||||
show_widget_hits: false,
|
||||
warn_if_rect_changes_id: cfg!(debug_assertions),
|
||||
show_unaligned: cfg!(debug_assertions),
|
||||
show_focused_widget: false,
|
||||
}
|
||||
@@ -2491,6 +2495,7 @@ impl DebugOptions {
|
||||
show_resize,
|
||||
show_interactive_widgets,
|
||||
show_widget_hits,
|
||||
warn_if_rect_changes_id,
|
||||
show_unaligned,
|
||||
show_focused_widget,
|
||||
} = self;
|
||||
@@ -2522,6 +2527,11 @@ impl DebugOptions {
|
||||
|
||||
ui.checkbox(show_widget_hits, "Show widgets under mouse pointer");
|
||||
|
||||
ui.checkbox(
|
||||
warn_if_rect_changes_id,
|
||||
"Warn if a Rect changes Id between frames",
|
||||
);
|
||||
|
||||
ui.checkbox(
|
||||
show_unaligned,
|
||||
"Show rectangles not aligned to integer point coordinates",
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use egui::accesskit::Role;
|
||||
use egui::epaint::Shape;
|
||||
use egui::{Align, Color32, Image, Label, Layout, RichText, Sense, TextWrapMode, include_image};
|
||||
use egui_kittest::Harness;
|
||||
use egui_kittest::kittest::Queryable as _;
|
||||
@@ -118,3 +119,47 @@ fn interact_on_ui_response_should_be_stable() {
|
||||
drop(harness);
|
||||
assert_eq!(click_count, 10, "We missed some clicks!");
|
||||
}
|
||||
|
||||
fn has_red_warning_rect(output: &egui::FullOutput) -> bool {
|
||||
output.shapes.iter().any(|clipped| {
|
||||
matches!(
|
||||
&clipped.shape,
|
||||
Shape::Rect(rect_shape)
|
||||
if rect_shape.stroke.color == Color32::RED
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
/// A button that changes its text on hover, with the Id derived from the text.
|
||||
/// This is a plausible bug: the widget keeps the same rect, but its Id changes
|
||||
/// between frames because the label (and thus the Id salt) changes on hover.
|
||||
/// The `warn_if_rect_changes_id` debug check should catch this.
|
||||
#[test]
|
||||
fn warn_if_rect_changes_id() {
|
||||
let button_rect = egui::Rect::from_min_size(egui::pos2(10.0, 10.0), egui::vec2(100.0, 30.0));
|
||||
|
||||
let mut harness = Harness::builder().with_size((200.0, 50.0)).build_ui(|ui| {
|
||||
// Simulate a buggy widget whose Id depends on its label text,
|
||||
// and the label changes on hover:
|
||||
let is_hovered = ui.rect_contains_pointer(button_rect);
|
||||
let label = if is_hovered { "Hovering!" } else { "Click me" };
|
||||
let id = ui.id().with(label);
|
||||
let _response = ui.interact(button_rect, id, Sense::click());
|
||||
});
|
||||
|
||||
// no hover — establishes stable prev_pass
|
||||
harness.step();
|
||||
assert!(
|
||||
!has_red_warning_rect(harness.output()),
|
||||
"Should not warn without hover"
|
||||
);
|
||||
|
||||
// Move the pointer over the button
|
||||
harness.hover_at(button_rect.center());
|
||||
|
||||
harness.step();
|
||||
assert!(
|
||||
has_red_warning_rect(harness.output()),
|
||||
"Should warn when a widget rect changes Id between passes"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user