Take input per kind, rather than deciding it once a frame
Reviewing this against the process we agreed: the title claimed per-kind routing and the code decided it once for the whole frame. A scroll and a click in the same frame both went to the button, because a widget that matched any momentary sense consumed everything. Consumption is now removing an input from the cursor the layers below see. `CursorSense::take` states what each sense takes -- exhaustively, so a new sense has to answer the question rather than inherit a default -- and `is_momentary` is gone with the enumeration it was written on. `should_run` and consumption share one matcher instead of two copies of the table. Two tests, each checked to fail without the change: a click and a scroll in one frame reach different widgets, and leaving a widget still ends its hover. The second is a regression this review caught in its own first draft, where the skip condition used `is_off`, which counts `End` -- the one frame a hover-end handler has to run on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
f62131eecf
commit
0e7076a01c
3 files changed
+130
-68
No files matched your search
@@ -135,11 +135,7 @@ impl<Rsc: HasEvents + 'static, E: Event> TypeEventManager<Rsc, E> {
|
||||
));
|
||||
}
|
||||
|
||||
/// The event lists this widget was registered with, without running
|
||||
/// anything. Asking what a widget would match is a separate question
|
||||
/// from dispatching to it: input routing needs the first to decide
|
||||
/// whether a widget consumes an event, and `run_fn` can only answer
|
||||
/// the second.
|
||||
/// What this widget registered, without running any of it.
|
||||
pub fn registered(&self, id: WidgetId) -> impl Iterator<Item = &E> {
|
||||
self.map.get(&id).into_iter().flatten().map(|(e, _)| e)
|
||||
}
|
||||
|
||||
+54
-47
@@ -53,16 +53,17 @@ impl CursorSense {
|
||||
matches!(self, CursorSense::Pressing(CursorButton::Left))
|
||||
}
|
||||
|
||||
/// True for a sense that names something happening this frame (a
|
||||
/// button transitioning, a scroll), as opposed to the ambient
|
||||
/// `Hover*` family that is on for as long as the cursor rests there.
|
||||
/// Only a momentary sense can consume an input -- see
|
||||
/// `SensorUi::run_sensors`.
|
||||
pub fn is_momentary(&self) -> bool {
|
||||
!matches!(
|
||||
self,
|
||||
CursorSense::HoverStart | CursorSense::Hovering | CursorSense::HoverEnd
|
||||
)
|
||||
/// Takes what this sense answers to out of `cursor`, so a widget below
|
||||
/// does not also get it. Hovering takes nothing: it goes to the topmost
|
||||
/// widget in shape, which is not a question about the input.
|
||||
fn take(&self, cursor: &mut CursorState) {
|
||||
match self {
|
||||
Self::PressStart(button) | Self::Pressing(button) | Self::PressEnd(button) => {
|
||||
*cursor.buttons.select_mut(button) = ActivationState::Off
|
||||
}
|
||||
Self::Scroll => cursor.scroll_delta = Vec2::ZERO,
|
||||
Self::HoverStart | Self::Hovering | Self::HoverEnd => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -90,6 +91,14 @@ impl CursorButtons {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn select_mut(&mut self, button: &CursorButton) -> &mut ActivationState {
|
||||
match button {
|
||||
CursorButton::Left => &mut self.left,
|
||||
CursorButton::Right => &mut self.right,
|
||||
CursorButton::Middle => &mut self.middle,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn end_frame(&mut self) {
|
||||
self.left.end_frame();
|
||||
self.middle.end_frame();
|
||||
@@ -175,45 +184,34 @@ impl SensorUi for UiRenderState {
|
||||
// this would probably be done through a generic parameter that adds yet another rsc /
|
||||
// state like thing, but local to render state, and is passed to UiRsc events so you can
|
||||
// update it there?
|
||||
// Whether anything momentary is happening this frame at all; see
|
||||
// `consumed` below, which is only judged per kind when it is.
|
||||
let momentary_active =
|
||||
cursor.scroll_delta != Vec2::ZERO || cursor.buttons.iter().any(|(_, a)| !a.is_off());
|
||||
|
||||
let mut active = std::mem::take(&mut rsc.events_mut().get_type::<CursorSense>().active);
|
||||
// Narrowed as it descends: a widget takes what it answers to, and
|
||||
// what is left is what the layers below see.
|
||||
let mut cursor = cursor;
|
||||
let mut hovered = false;
|
||||
for layer in self.layers.indices().rev() {
|
||||
let mut sensed = false;
|
||||
let mut below = cursor.clone();
|
||||
let mut hovered_here = false;
|
||||
for (id, sensor) in active.get_mut(&layer).into_flat_iter() {
|
||||
let shape = self.active.get(id).unwrap().region;
|
||||
let region = shape.to_px(window_size);
|
||||
let in_shape = cursor.exists && region.contains(cursor.pos);
|
||||
sensor.hover.update(in_shape);
|
||||
if sensor.hover == ActivationState::Off {
|
||||
let over = cursor.exists && region.contains(cursor.pos);
|
||||
// Hover goes to the topmost widget in shape and no further.
|
||||
sensor.hover.update(over && !hovered);
|
||||
// `is_off` would be wrong here: it counts `End`, which is
|
||||
// the one frame a hover-end handler has to run on.
|
||||
if !over && sensor.hover == ActivationState::Off {
|
||||
continue;
|
||||
}
|
||||
hovered_here |= over;
|
||||
|
||||
// A widget in shape always still runs: a hover-only
|
||||
// highlight must fire on the topmost thing under the
|
||||
// cursor even while a scroll passes through it. What is
|
||||
// judged per input kind is whether it *consumes* that
|
||||
// input, stopping a lower layer from seeing it. With
|
||||
// nothing momentary happening, being in shape is
|
||||
// consumption and the topmost widget wins the hover; with
|
||||
// a scroll or a press happening, only a widget that
|
||||
// registered a matching momentary sense consumes it, so a
|
||||
// button registered for `click()` alone cannot block a
|
||||
// scroll meant for the list behind it.
|
||||
let consumed = if momentary_active {
|
||||
rsc.events_mut()
|
||||
.get_type::<CursorSense>()
|
||||
.registered(*id)
|
||||
.any(|senses| {
|
||||
matches!(should_run(senses, &cursor, sensor.hover), Some(s) if s.is_momentary())
|
||||
})
|
||||
} else {
|
||||
true
|
||||
};
|
||||
sensed |= consumed;
|
||||
for senses in rsc.events_mut().get_type::<CursorSense>().registered(*id) {
|
||||
for sense in senses.iter() {
|
||||
if matches(sense, &cursor, sensor.hover) {
|
||||
sense.take(&mut below);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let cursor = cursor.clone();
|
||||
|
||||
@@ -230,7 +228,9 @@ impl SensorUi for UiRenderState {
|
||||
};
|
||||
rsc.run_event::<CursorSense>(*id, data, state);
|
||||
}
|
||||
if sensed {
|
||||
hovered |= hovered_here;
|
||||
cursor = below;
|
||||
if hovered && !is_momentary(&cursor) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
@@ -243,8 +243,14 @@ pub fn should_run(
|
||||
cursor: &CursorState,
|
||||
hover: ActivationState,
|
||||
) -> Option<CursorSense> {
|
||||
for sense in senses.iter() {
|
||||
if match sense {
|
||||
senses
|
||||
.iter()
|
||||
.find(|sense| matches(sense, cursor, hover))
|
||||
.copied()
|
||||
}
|
||||
|
||||
fn matches(sense: &CursorSense, cursor: &CursorState, hover: ActivationState) -> bool {
|
||||
match sense {
|
||||
CursorSense::PressStart(button) => cursor.buttons.select(button).is_start(),
|
||||
CursorSense::Pressing(button) => cursor.buttons.select(button).is_on(),
|
||||
CursorSense::PressEnd(button) => cursor.buttons.select(button).is_end(),
|
||||
@@ -252,11 +258,12 @@ pub fn should_run(
|
||||
CursorSense::Hovering => hover.is_on(),
|
||||
CursorSense::HoverEnd => hover.is_end(),
|
||||
CursorSense::Scroll => cursor.scroll_delta != Vec2::ZERO,
|
||||
} {
|
||||
return Some(*sense);
|
||||
}
|
||||
}
|
||||
None
|
||||
|
||||
/// Whether anything is happening to the cursor beyond where it rests.
|
||||
fn is_momentary(cursor: &CursorState) -> bool {
|
||||
cursor.scroll_delta != Vec2::ZERO || cursor.buttons.iter().any(|(_, state)| !state.is_off())
|
||||
}
|
||||
|
||||
impl ActivationState {
|
||||
|
||||
+75
-16
@@ -1,8 +1,6 @@
|
||||
//! A widget that registered only `click()` used to block a `Scroll` meant
|
||||
//! for whatever is behind it: `run_sensors` decided "consumed, stop looking
|
||||
//! at lower layers" from mere hover rather than from anything actually
|
||||
//! matching the input. These drive `run_sensors` directly, which needs no
|
||||
//! GPU and no window.
|
||||
//! A widget takes only what it answers to: a button over a list takes the
|
||||
//! click and leaves the scroll. These drive `run_sensors` directly, which
|
||||
//! needs no GPU and no window.
|
||||
|
||||
use iris::prelude::*;
|
||||
use std::{cell::Cell, rc::Rc};
|
||||
@@ -52,8 +50,9 @@ fn cursor_at(pos: Vec2) -> CursorState {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_button_over_a_list_scrolls_the_list_and_still_clicks() {
|
||||
/// A button covering a list, on the layer above it: the list scrolls, the
|
||||
/// button clicks, and the returned flags say which fired.
|
||||
fn button_over_list() -> (UiRenderState, SenseRsc, Rc<Cell<bool>>, Rc<Cell<bool>>) {
|
||||
let mut rsc = SenseRsc {
|
||||
ui: UiData::default(),
|
||||
events: EventManager::default(),
|
||||
@@ -80,9 +79,7 @@ fn a_button_over_a_list_scrolls_the_list_and_still_clicks() {
|
||||
});
|
||||
}
|
||||
|
||||
// A Stack draws its children on separate layers in order, which is
|
||||
// exactly the "one thing drawn over another" shape `run_sensors`
|
||||
// walks top layer first.
|
||||
// A Stack draws each child on its own layer, in order.
|
||||
let root = rsc
|
||||
.ui
|
||||
.widgets
|
||||
@@ -96,10 +93,17 @@ fn a_button_over_a_list_scrolls_the_list_and_still_clicks() {
|
||||
render.resize((100.0, 100.0));
|
||||
render.update(&root, &mut rsc);
|
||||
|
||||
(render, rsc, scrolled, clicked)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_button_over_a_list_scrolls_the_list_and_still_clicks() {
|
||||
let (render, mut rsc, scrolled, clicked) = button_over_list();
|
||||
let mut state = ();
|
||||
let mut scroll_cursor = cursor_at((50.0, 50.0).into());
|
||||
scroll_cursor.scroll_delta = (0.0, 10.0).into();
|
||||
render.run_sensors(&mut rsc, &mut state, scroll_cursor, (100.0, 100.0).into());
|
||||
|
||||
let mut scroll = cursor_at((50.0, 50.0).into());
|
||||
scroll.scroll_delta = (0.0, 10.0).into();
|
||||
render.run_sensors(&mut rsc, &mut state, scroll, (100.0, 100.0).into());
|
||||
|
||||
assert!(
|
||||
scrolled.get(),
|
||||
@@ -110,12 +114,67 @@ fn a_button_over_a_list_scrolls_the_list_and_still_clicks() {
|
||||
"a scroll is not a click; the button must not have fired"
|
||||
);
|
||||
|
||||
let mut click_cursor = cursor_at((50.0, 50.0).into());
|
||||
click_cursor.buttons.left = ActivationState::Start;
|
||||
render.run_sensors(&mut rsc, &mut state, click_cursor, (100.0, 100.0).into());
|
||||
let mut click = cursor_at((50.0, 50.0).into());
|
||||
click.buttons.left = ActivationState::Start;
|
||||
render.run_sensors(&mut rsc, &mut state, click, (100.0, 100.0).into());
|
||||
|
||||
assert!(
|
||||
clicked.get(),
|
||||
"the button on top must still receive an actual click"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_click_and_a_scroll_in_one_frame_go_to_different_widgets() {
|
||||
let (render, mut rsc, scrolled, clicked) = button_over_list();
|
||||
let mut state = ();
|
||||
|
||||
let mut both = cursor_at((50.0, 50.0).into());
|
||||
both.scroll_delta = (0.0, 10.0).into();
|
||||
both.buttons.left = ActivationState::Start;
|
||||
render.run_sensors(&mut rsc, &mut state, both, (100.0, 100.0).into());
|
||||
|
||||
assert!(
|
||||
clicked.get(),
|
||||
"the button takes the click it registered for"
|
||||
);
|
||||
assert!(
|
||||
scrolled.get(),
|
||||
"taking the click must not take the scroll with it"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn leaving_a_widget_still_ends_its_hover() {
|
||||
let mut rsc = SenseRsc {
|
||||
ui: UiData::default(),
|
||||
events: EventManager::default(),
|
||||
};
|
||||
let widget = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE));
|
||||
let ended = Rc::new(Cell::new(false));
|
||||
{
|
||||
let ended = ended.clone();
|
||||
rsc.register_event(widget.weak(), CursorSense::HoverEnd, move |_ctx, _rsc| {
|
||||
ended.set(true);
|
||||
});
|
||||
}
|
||||
|
||||
let root = widget.any();
|
||||
let mut render = UiRenderState::new();
|
||||
render.resize((100.0, 100.0));
|
||||
render.update(&root, &mut rsc);
|
||||
|
||||
let mut state = ();
|
||||
render.run_sensors(
|
||||
&mut rsc,
|
||||
&mut state,
|
||||
cursor_at((50.0, 50.0).into()),
|
||||
(100.0, 100.0).into(),
|
||||
);
|
||||
assert!(!ended.get(), "the cursor is still on it");
|
||||
|
||||
let mut gone = cursor_at((50.0, 50.0).into());
|
||||
gone.exists = false;
|
||||
render.run_sensors(&mut rsc, &mut state, gone, (100.0, 100.0).into());
|
||||
assert!(ended.get(), "leaving a widget ends its hover");
|
||||
}
|
||||
Reference in new issue
Block a user