From 0e7076a01c7cea10824bd56fee2ecc2a315a7b63 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Sun, 13 Sep 2026 19:09:50 -0400 Subject: [PATCH] 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 --- core/src/event/manager.rs | 6 +- src/default/sense.rs | 117 ++++++++++++++++++++------------------ tests/pointer_routing.rs | 91 +++++++++++++++++++++++------ 3 files changed, 138 insertions(+), 76 deletions(-) diff --git a/core/src/event/manager.rs b/core/src/event/manager.rs index 1a92557..9c32c12 100644 --- a/core/src/event/manager.rs +++ b/core/src/event/manager.rs @@ -135,11 +135,7 @@ impl TypeEventManager { )); } - /// 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 { self.map.get(&id).into_iter().flatten().map(|(e, _)| e) } diff --git a/src/default/sense.rs b/src/default/sense.rs index 37c9a4f..937b93a 100644 --- a/src/default/sense.rs +++ b/src/default/sense.rs @@ -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::().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::() - .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::().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::(*id, data, state); } - if sensed { + hovered |= hovered_here; + cursor = below; + if hovered && !is_momentary(&cursor) { break; } } @@ -243,20 +243,27 @@ pub fn should_run( cursor: &CursorState, hover: ActivationState, ) -> Option { - for sense in senses.iter() { - if 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(), - CursorSense::HoverStart => hover.is_start(), - CursorSense::Hovering => hover.is_on(), - CursorSense::HoverEnd => hover.is_end(), - CursorSense::Scroll => cursor.scroll_delta != Vec2::ZERO, - } { - return Some(*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(), + CursorSense::HoverStart => hover.is_start(), + CursorSense::Hovering => hover.is_on(), + CursorSense::HoverEnd => hover.is_end(), + CursorSense::Scroll => cursor.scroll_delta != Vec2::ZERO, } - 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 { diff --git a/tests/pointer_routing.rs b/tests/pointer_routing.rs index 5d31f6e..7a978a2 100644 --- a/tests/pointer_routing.rs +++ b/tests/pointer_routing.rs @@ -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>, Rc>) { 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"); +}