Skip to content

Commit 0ef57d5

Browse files
lucasmerlinemilk
authored andcommitted
Fix ui.response().interact(Sense::click()) being flakey (#7713)
This fixes calls to `ui.response().interact(Sense::click())` being flakey. Since egui checks widget interactions at the beginning of the frame, based on the responses from last frame, we need to ensure that we always call `create_widget` on `interact` calls, otherwise there can be a feedback loop where the `Sense` egui acts on flips back and forth between frames. Without the fix in `interact`, both the asserts in the new test fail. Here is a video where I experienced the bug, showing the sense switching every frame. Every other click would fail to be detected. https://github.com/user-attachments/assets/6be7ca0e-b50f-4d30-bf87-bbb80c319f3b Also note, usually it's better to use `UiBuilder::sense()` to give a Ui some sense, but sometimes you don't have the flexibility, e.g. in a `Ui` callback from some code external to your project.
1 parent d06c28c commit 0ef57d5

File tree

2 files changed

+47
-5
lines changed

2 files changed

+47
-5
lines changed

crates/egui/src/response.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -724,10 +724,9 @@ impl Response {
724724
/// ```
725725
#[must_use]
726726
pub fn interact(&self, sense: Sense) -> Self {
727-
if (self.sense | sense) == self.sense {
728-
// Early-out: we already sense everything we need to sense.
729-
return self.clone();
730-
}
727+
// We could check here if the new Sense equals the old one to avoid the extra create_widget
728+
// call. But that would break calling `interact` on a response from `Context::read_response`
729+
// or `Ui::response`. (See https://github.com/emilk/egui/pull/7713 for more details.)
731730

732731
self.ctx.create_widget(
733732
WidgetRect {

tests/egui_tests/tests/regression_tests.rs

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use egui::accesskit::Role;
2-
use egui::{Align, Color32, Image, Label, Layout, RichText, TextWrapMode, include_image};
2+
use egui::{Align, Color32, Image, Label, Layout, RichText, Sense, TextWrapMode, include_image};
33
use egui_kittest::Harness;
44
use egui_kittest::kittest::Queryable as _;
55

@@ -75,3 +75,46 @@ fn combobox_should_have_value() {
7575
Some("Option 1")
7676
);
7777
}
78+
79+
/// This test ensures that `ui.response().interact(...)` works correctly.
80+
///
81+
/// This was broken, because there was an optimization in [`egui::Response::interact`]
82+
/// which caused the [`Sense`] of the original response to flip-flop between `click` and `hover`
83+
/// between frames.
84+
///
85+
/// See <https://github.com/emilk/egui/pull/7713> for more details.
86+
#[test]
87+
fn interact_on_ui_response_should_be_stable() {
88+
let mut first_frame = true;
89+
let mut click_count = 0;
90+
let mut harness = Harness::new_ui(|ui| {
91+
let ui_response = ui.response();
92+
if !first_frame {
93+
assert!(
94+
ui_response.sense.contains(Sense::click()),
95+
"ui.response() didn't have click sense even though we called interact(Sense::click()) last frame"
96+
);
97+
}
98+
99+
// Add a label so we have something to click with kittest
100+
ui.add(
101+
Label::new("senseless label")
102+
.sense(Sense::hover())
103+
.selectable(false),
104+
);
105+
106+
let click_response = ui_response.interact(Sense::click());
107+
if click_response.clicked() {
108+
click_count += 1;
109+
}
110+
first_frame = false;
111+
});
112+
113+
for i in 0..=10 {
114+
harness.run_steps(i);
115+
harness.get_by_label("senseless label").click();
116+
}
117+
118+
drop(harness);
119+
assert_eq!(click_count, 10, "We missed some clicks!");
120+
}

0 commit comments

Comments
 (0)