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.
This commit is contained in:
parent
d06c28cb15
commit
0ef57d5a1d
|
|
@ -724,10 +724,9 @@ impl Response {
|
||||||
/// ```
|
/// ```
|
||||||
#[must_use]
|
#[must_use]
|
||||||
pub fn interact(&self, sense: Sense) -> Self {
|
pub fn interact(&self, sense: Sense) -> Self {
|
||||||
if (self.sense | sense) == self.sense {
|
// We could check here if the new Sense equals the old one to avoid the extra create_widget
|
||||||
// Early-out: we already sense everything we need to sense.
|
// call. But that would break calling `interact` on a response from `Context::read_response`
|
||||||
return self.clone();
|
// or `Ui::response`. (See https://github.com/emilk/egui/pull/7713 for more details.)
|
||||||
}
|
|
||||||
|
|
||||||
self.ctx.create_widget(
|
self.ctx.create_widget(
|
||||||
WidgetRect {
|
WidgetRect {
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
use egui::accesskit::Role;
|
use egui::accesskit::Role;
|
||||||
use egui::{Align, Color32, Image, Label, Layout, RichText, TextWrapMode, include_image};
|
use egui::{Align, Color32, Image, Label, Layout, RichText, Sense, TextWrapMode, include_image};
|
||||||
use egui_kittest::Harness;
|
use egui_kittest::Harness;
|
||||||
use egui_kittest::kittest::Queryable as _;
|
use egui_kittest::kittest::Queryable as _;
|
||||||
|
|
||||||
|
|
@ -75,3 +75,46 @@ fn combobox_should_have_value() {
|
||||||
Some("Option 1")
|
Some("Option 1")
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// This test ensures that `ui.response().interact(...)` works correctly.
|
||||||
|
///
|
||||||
|
/// This was broken, because there was an optimization in [`egui::Response::interact`]
|
||||||
|
/// which caused the [`Sense`] of the original response to flip-flop between `click` and `hover`
|
||||||
|
/// between frames.
|
||||||
|
///
|
||||||
|
/// See <https://github.com/emilk/egui/pull/7713> for more details.
|
||||||
|
#[test]
|
||||||
|
fn interact_on_ui_response_should_be_stable() {
|
||||||
|
let mut first_frame = true;
|
||||||
|
let mut click_count = 0;
|
||||||
|
let mut harness = Harness::new_ui(|ui| {
|
||||||
|
let ui_response = ui.response();
|
||||||
|
if !first_frame {
|
||||||
|
assert!(
|
||||||
|
ui_response.sense.contains(Sense::click()),
|
||||||
|
"ui.response() didn't have click sense even though we called interact(Sense::click()) last frame"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Add a label so we have something to click with kittest
|
||||||
|
ui.add(
|
||||||
|
Label::new("senseless label")
|
||||||
|
.sense(Sense::hover())
|
||||||
|
.selectable(false),
|
||||||
|
);
|
||||||
|
|
||||||
|
let click_response = ui_response.interact(Sense::click());
|
||||||
|
if click_response.clicked() {
|
||||||
|
click_count += 1;
|
||||||
|
}
|
||||||
|
first_frame = false;
|
||||||
|
});
|
||||||
|
|
||||||
|
for i in 0..=10 {
|
||||||
|
harness.run_steps(i);
|
||||||
|
harness.get_by_label("senseless label").click();
|
||||||
|
}
|
||||||
|
|
||||||
|
drop(harness);
|
||||||
|
assert_eq!(click_count, 10, "We missed some clicks!");
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue