diff --git a/docs/IRIS.md b/docs/IRIS.md index bc155be..7c9b80f 100644 --- a/docs/IRIS.md +++ b/docs/IRIS.md @@ -1529,3 +1529,82 @@ This does not reopen the 2026-09-07 platform-fonts decision. Body and monospace text still come from the platform's own collection; an icon is the opposite case, a small closed set of codepoints no system font is guaranteed to have, and it is the same division the Compose app makes. + +## 2026-09-08: a press only reaches what the pointer is actually on + +Iris's report -- "if I try to scroll vertically while a horizontal scroll +animation is still active, it stays locked to the horizontal scroll. It +should let it keep going and instead only affect vertical scrolling" -- +and her own diagnosis of it, which was the right one: "it seems like iris +is set up so the animation stuff is global which it definitely should not +be. Tapping outside of something that a fling is currently active for +should have no code in common with the fling that could influence it." + +It was global, and it was in `sense::should_run`. `run_sensors` runs a +widget one frame *after* the pointer leaves it (`ActivationState::End`, +which is not `Off`) so a `HoverEnd` can fire, and `should_run` derived +`PressStart`/`Pressing`/`PressEnd`/`Scroll` from the raw button and wheel +state without consulting `hover` at all. So that farewell frame carried a +press to a widget the finger was nowhere near. + +That alone would have been a stray event; what made it eat the gesture is +the catch added on 2026-09-07 (`PressState::scrolling`), which commits a +press on already-moving content to a pan immediately, with no `DRAG_SLOP` +-- so the widget captured the pointer on that frame and every later sample +went to it. And the widget's hover was stale in the first place because a +gesture that ends while captured returns from `run_sensors`' capture +branch, which never reaches the loop that would have updated it. + +Measured on the real screen before the fix: a fence flicked sideways, then +a finger put down on a row **500px above it** and dragged 160px down the +screen. The list moved by zero, the fence moved by zero, and the fence +held the pointer for the whole gesture -- the report, exactly. + +- **`should_run` now requires `hover.is_on()` for every non-hover sense.** + Press and wheel both, since a wheel event reaching a widget the cursor + has just left is the same fault with a different sense. `Drop` and + `Cancel` are unaffected: they are delivered deliberately to a widget + that is *not* under the pointer, and `run_sensors` hands both an + explicit `On`. +- **`Scroll::is_scrolling`** (new): whether a fling is coasting in this + area, the same question and the same name `List::is_scrolling` already + answers for the other scrolling widget. + +Nothing about the fling, the arbiter or the catch changed. A press outside +a coasting area now has no code in common with it, so the horizontal fling +keeps coasting through a vertical drag on its own -- which is the second +half of what Iris asked for, and it falls out of the fix rather than being +arranged. A press *inside* a coasting area is still a catch on either +axis, which is what Compose does ("Compose does catch no matter what axis +if you tap in the horizontal area"). + +Two tests, one per layer: +`sense_tests::a_press_does_not_reach_a_widget_the_pointer_has_just_left` +is the mechanism with two stacked scroll areas and no screen, and +`fence_fling.rs`'s +`a_drag_away_from_a_coasting_fence_scrolls_the_list_and_leaves_it_coasting` +is the report itself over the real transcript. Both fail on the old code. + +## 2026-09-08: the composer is clipped to its bar, not inside its padding + +Iris: "the message input box doesn't clip correctly ... the box should be +clipped rather than the inset text." + +The composer was `.masked().background(rect(...))` -- two boxes, one +inside the other. The mask sat *inside* the `dp(FIELD_PAD_DP)` padding, so +a message longer than the six lines shown was cut through the middle of a +glyph 12dp in from the bar's edge, with a band of bare surface above the +cut. Measured at the phone's own size and density (1080x2424 at 2.55): the +bar's top edge at y=1995.6 and the text sliced at y=2026.2. + +It is `.masked_by(rect(BAR_FILL))` now: the same rect is the surface drawn +behind the field *and* the shape the field is clipped to, so the two +cannot fall out of step -- the idiom `row.rs` already uses to cut a code +fence to its own rounded panel. Text now disappears under the bar's edge +at 1995.6. The padding still holds text off the edge at the end the +content is anchored to, which is the end anybody is reading. + +The composer's overflowing and keyboard-open states had no way to be +looked at headlessly, since that window has no keyboard: the phone rig +takes `--message TEXT` and `--ime PX` for them +(`transcript-fixture/examples/phone.rs`, through `RUN_HEADLESS_ARGS`). diff --git a/iris/src/sense.rs b/iris/src/sense.rs index 58d7cdc..953b6d5 100644 --- a/iris/src/sense.rs +++ b/iris/src/sense.rs @@ -637,15 +637,40 @@ pub fn should_run( cursor: &CursorState, hover: ActivationState, ) -> Option { + // Every sense below that is about the *pointer* rather than about + // hovering needs the pointer to actually be on this widget, and + // `hover` is the only thing here that knows: `run_sensors` runs a + // widget one more time after the pointer has left it (`ActivationState + // ::End`, which is not `Off`) so that a `HoverEnd` can fire, and + // deriving a press from raw button state alone handed that frame a + // `PressStart` too. A widget the finger is nowhere near then opened a + // gesture and, if it went straight to panning, captured the pointer -- + // which is the whole of Iris's 2026-09-08 "if I try to scroll + // vertically while a horizontal scroll animation is still active, it + // stays locked to the horizontal scroll". Measured: a fence flicked + // sideways is left `hover == On` (the capture branch above returns + // before the loop that would have updated it), so the *next* touch + // down anywhere on the screen decayed it to `End`, ran the fence's + // `Scroll::drag` with a `PressStart`, and -- the press being a catch + // of its own fling, which commits with no slop -- captured the whole + // gesture 500px away from the fence. The list under the finger moved + // by nothing at all. + // + // The rule is the set's, not one member's: press *and* scroll, since + // a wheel event reaching a widget the cursor has just left is the same + // fault with a different sense. `Drop` and `Cancel` are exempt because + // they are delivered deliberately to a widget that is *not* under the + // pointer, and `run_sensors` hands both of those an `On` anyway. + let on_this = hover.is_on(); 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::PressStart(button) => on_this && cursor.buttons.select(button).is_start(), + CursorSense::Pressing(button) => on_this && cursor.buttons.select(button).is_on(), + CursorSense::PressEnd(button) => on_this && 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, + CursorSense::Scroll => on_this && cursor.scroll_delta != Vec2::ZERO, // Never derived here -- `Drop` only ever fires through // `CursorSenses::should_run`'s own special case, ahead of this // loop, for the one widget `run_sensors`' capture branch is diff --git a/iris/src/sense_tests.rs b/iris/src/sense_tests.rs index cf749b6..e8f5dc4 100644 --- a/iris/src/sense_tests.rs +++ b/iris/src/sense_tests.rs @@ -616,3 +616,150 @@ fn a_drag_pans_whichever_nested_scroll_area_owns_its_axis() { ); } } + +/// Iris's 2026-09-08 report: "if I try to scroll vertically while a +/// horizontal scroll animation is still active, it stays locked to the +/// horizontal scroll", with her own diagnosis -- "tapping outside of +/// something that a fling is currently active for should have no code in +/// common with the fling that could influence it." +/// +/// She was right that it was global state, and this is where it lived. +/// `run_sensors` runs a widget one more frame *after* the pointer has +/// left it, so a `HoverEnd` can fire ([`ActivationState::End`], which is +/// not `Off`) -- and `should_run` derived a press from the button alone, +/// so that farewell frame also carried a `PressStart`. A widget nowhere +/// near the finger therefore opened a gesture, and a `Scroll` catching +/// its own fling commits with no slop, so it captured the pointer and the +/// whole gesture went to it. +/// +/// Two areas side by side here rather than one, because "the press went +/// to the wrong widget" and "the press went nowhere" are different +/// failures and only the second area can tell them apart. +#[test] +fn a_press_does_not_reach_a_widget_the_pointer_has_just_left() { + let mut rsc = SenseRsc { + ui: UiData::default(), + events: EventManager::default(), + }; + + // Two 1000px-tall scroll areas, stacked: the top half of the window + // is the first, the bottom half the second. Each area's own handle is + // taken as its chain is built (`with_id`, the same way the nested-axes + // test above does it), since what is under test is `scrollable()`'s + // real registration rather than a `Scroll` assembled by hand. + let seen: [Rc>>>; 2] = Default::default(); + let half = |slot: &Rc>>>| { + let record = slot.clone(); + rect(UiColor::WHITE) + .height(Len::abs(1000.0)) + .scrollable() + .with_id(move |_rsc, id| { + record.set(Some(id)); + id + }) + .height(Len::rel(0.5)) + }; + let root = (half(&seen[0]), half(&seen[1])) + .span(Dir::DOWN) + .add_strong(&mut rsc) + .any(); + let (top_w, bottom_w) = (seen[0].get().unwrap(), seen[1].get().unwrap()); + + let win: Vec2 = (100.0, 200.0).into(); + let mut render = UiRenderState::new(); + render.resize((win.x, win.y)); + render.update(&root, &mut rsc); + // The second frame is the first that knows how long the content is -- + // see `a_finger_drag_over_a_scroll_area_pans_it`. + for w in [&top_w, &bottom_w] { + rsc.ui.widgets.get_mut(w).unwrap().scroll(0.0); + } + render.update(&root, &mut rsc); + + let mut state = (); + // Flick the top area and let go: it is left flinging, and -- because + // the release goes through `run_sensors`' capture branch, which + // returns before the loop that would have updated anybody's hover -- + // its sensor is left `On` with the pointer no longer on it. Both + // halves of the real gesture, since both are what the bug needs. + let base = Instant::now(); + let mut t = 0; + let sample = |render: &mut UiRenderState, + rsc: &mut SenseRsc, + state: &mut (), + y: f32, + button: ActivationState, + at_ms: u64| { + let mut c = cursor_at((50.0, y).into()); + c.buttons.left = button; + c.time = base + std::time::Duration::from_millis(at_ms); + render.run_sensors(rsc, state, c, win); + }; + sample( + &mut render, + &mut rsc, + &mut state, + 50.0, + ActivationState::Start, + t, + ); + for y in [44.0, 32.0, 14.0] { + t += 8; + sample(&mut render, &mut rsc, &mut state, y, ActivationState::On, t); + } + t += 8; + sample( + &mut render, + &mut rsc, + &mut state, + 14.0, + ActivationState::End, + t, + ); + assert!( + rsc.ui.widgets.get(&top_w).unwrap().is_scrolling(), + "the flick must leave the top area coasting -- the press below is \ + only dangerous while something is still moving", + ); + let flung_to = rsc.ui.widgets.get(&top_w).unwrap().amt(); + + // Now press and drag in the *bottom* area: the top area's hover + // decays to `End` on this very sample, which is the frame that used + // to carry a `PressStart` to it. + t += 8; + sample( + &mut render, + &mut rsc, + &mut state, + 150.0, + ActivationState::Start, + t, + ); + t += 8; + sample( + &mut render, + &mut rsc, + &mut state, + 150.0 - (DRAG_SLOP + 40.0), + ActivationState::On, + t, + ); + + let moved = rsc.ui.widgets.get(&bottom_w).unwrap().amt(); + assert!( + (moved - 40.0).abs() < 0.01, + "the area actually under the finger should have panned by the 40px \ + past the slop, got {moved}" + ); + assert_eq!( + rsc.ui.widgets.get(&top_w).unwrap().amt(), + flung_to, + "the area the pointer had left must not have seen the press at all -- \ + a catch would have stopped its fling on the touch-down" + ); + assert_eq!( + pointer_input(&mut rsc).holder(), + Some(bottom_w.id()), + "the gesture belongs to the widget under the finger", + ); +} diff --git a/iris/src/widget/position/scroll.rs b/iris/src/widget/position/scroll.rs index 02f7782..80451df 100644 --- a/iris/src/widget/position/scroll.rs +++ b/iris/src/widget/position/scroll.rs @@ -258,6 +258,15 @@ impl Scroll { self.amt } + /// Whether a fling is coasting here right now -- the same question + /// `List::is_scrolling` answers for the other scrolling widget, under + /// the same name so there is one word for it. What a caller polls to + /// know whether this area is moving on its own (a test, and + /// [`PressState::scrolling`]'s own condition). + pub fn is_scrolling(&self) -> bool { + self.fling.is_flinging() + } + /// Which way this area pans. For a caller that found the widget /// rather than built it -- a test walking what is drawn, a scroll /// indicator asking which edge to sit on. diff --git a/iris/transcript-fixture/examples/phone.rs b/iris/transcript-fixture/examples/phone.rs index 4e26974..f480b94 100644 --- a/iris/transcript-fixture/examples/phone.rs +++ b/iris/transcript-fixture/examples/phone.rs @@ -11,6 +11,12 @@ //! screenshot here and one from the phone is the renderer, never the //! data. //! +//! `--message TEXT` (through `RUN_HEADLESS_ARGS`) starts with that text +//! already in the composer, `\n` for a newline -- the composer's grown +//! and overflowing states are otherwise unreachable here, since this +//! window has no keyboard to type into (UI_RULES.md's "check the states +//! you can't see by default"). +//! //! No server: `transcript-fixture` embeds the transcript. Colour, //! spacing, type and anything a person has to *see* is answered here; //! anything with an assertion behind it belongs in `tests/ @@ -19,6 +25,33 @@ use iris::prelude::*; use winit::{dpi::PhysicalSize, window::WindowAttributes}; +/// The `--ime PX` argument: the bottom inset a keyboard would report, +/// applied after the first frame the way Android's `on_insets_changed` +/// does. The composer's keyboard-open layout is otherwise unreachable +/// here, and it is where its mask went wrong before (see +/// `ActiveData::own_mask`). +fn ime_argv() -> Option { + let mut args = std::env::args().skip(1); + while let Some(arg) = args.next() { + if arg == "--ime" { + return args.next()?.parse().ok(); + } + } + None +} + +/// The `--message TEXT` argument, with `\n` taken as a newline so a +/// multi-line message survives one shell word. +fn message_argv() -> Option { + let mut args = std::env::args().skip(1); + while let Some(arg) = args.next() { + if arg == "--message" { + return Some(args.next()?.replace("\\n", "\n")); + } + } + None +} + fn main() { DefaultApp::::run(); } @@ -47,6 +80,12 @@ impl DefaultAppState for Client { ) -> Self { let screen = match transcript_fixture::open(rsc, &mut ui_state) { Ok(opened) => { + if let Some(message) = message_argv() { + opened.screen.composer.field.edit(rsc).set(&message); + } + if let Some(inset) = ime_argv() { + opened.screen.composer.set_bottom_inset(rsc, inset); + } // A fling coasts only while something asks for the next // frame; on the desktop that is the window's own redraw // request (`List::fling`'s doc). diff --git a/iris/transcript-fixture/tests/fence_fling.rs b/iris/transcript-fixture/tests/fence_fling.rs index e70c8f7..ff26f33 100644 --- a/iris/transcript-fixture/tests/fence_fling.rs +++ b/iris/transcript-fixture/tests/fence_fling.rs @@ -36,6 +36,16 @@ fn fence_scroll_in(h: &Harness, top: f32, bottom: f32) -> Option<(WidgetId, Pixe }) } +fn is_scrolling(h: &Harness, id: WidgetId) -> bool { + h.rsc + .ui + .widgets + .get_dyn(id) + .and_then(|w| w.as_any().downcast_ref::()) + .expect("the fence's scroll area is still drawn") + .is_scrolling() +} + fn amt(h: &Harness, id: WidgetId) -> f32 { h.rsc .ui @@ -121,3 +131,100 @@ fn a_flick_across_a_code_fence_keeps_moving_after_the_finger_leaves() { h.frame(t); assert_eq!(last, amt(&h, fence_scroll), "the fling never settled"); } + +/// Iris's 2026-09-08 report: "if I try to scroll vertically while a +/// horizontal scroll animation is still active, it stays locked to the +/// horizontal scroll. It should let it keep going and instead only affect +/// vertical scrolling." +/// +/// Her own diagnosis was the right one -- "tapping outside of something +/// that a fling is currently active for should have no code in common +/// with the fling that could influence it" -- and +/// `sense_tests::a_press_does_not_reach_a_widget_the_pointer_has_just_left` +/// is the mechanism in isolation. This is the same thing over the real +/// screen, which is where it was found: the finger goes down on an +/// ordinary row 500px above a coasting fence, and what must move is the +/// list, while the fence carries on coasting untouched. +#[test] +fn a_drag_away_from_a_coasting_fence_scrolls_the_list_and_leaves_it_coasting() { + use client_core::transcript_fold::{TranscriptItem, TranscriptRow}; + + let mut h = Harness::new(phone_size(), PHONE_SCALE); + let opened = transcript_fixture::open(&mut h.rsc, &mut h.state).expect("the fixture folds"); + let screen = opened.screen; + h.frame(0); + h.frame(PHONE_FRAME_MS); + + let fence = TranscriptRow::Single(TranscriptItem::AssistantMsg { + seq: 9_000_000, + text: format!( + "```\n{}\n```", + (1..=200) + .map(|i| format!("word{i}")) + .collect::>() + .join(" ") + ), + settled: true, + }); + screen.push_row(&mut h.rsc, &fence); + (screen.list)(&mut h.rsc).jump_to_end(); + h.frame(100); + h.frame(108); + + let key = transcript_ui::row::row_key(&fence.key()); + let (top, bottom) = (screen.list)(&mut h.rsc) + .extent(key) + .expect("the fence row is on screen"); + let (fence_scroll, box_) = fence_scroll_in(&h, top, bottom) + .expect("the pushed fence draws a horizontal scroll area of its own"); + let y = (box_.top_left.y + box_.bot_right.y) / 2.0; + + // Flick the fence sideways and let go, exactly as above. + h.touch(TouchAction::Down, Vec2::new(900.0, y), 200); + for (i, x) in [860.0, 800.0, 720.0, 620.0].into_iter().enumerate() { + h.touch(TouchAction::Move, Vec2::new(x, y), 208 + 8 * i as u64); + } + h.touch(TouchAction::Up, Vec2::new(620.0, y), 240); + h.frame(248); + assert!( + is_scrolling(&h, fence_scroll), + "the fence has to still be coasting for this to be the reported case", + ); + + // A row well clear of the fence, taken by its own extent rather than + // by a coordinate: the gaps between rows are pointer-transparent, so a + // y picked by hand lands on nothing often enough to make a green run + // meaningless. + let probe = box_.top_left.y - 500.0; + let row = (screen.list)(&mut h.rsc) + .key_at(probe) + .expect("a row that far up the screen"); + let (row_top, row_bottom) = (screen.list)(&mut h.rsc).extent(row).expect("its extent"); + let from = (row_top + row_bottom) / 2.0; + + let list_before = (screen.list)(&mut h.rsc).anchor_position_display(); + let fence_before = amt(&h, fence_scroll); + h.touch(TouchAction::Down, Vec2::new(540.0, from), 256); + let mut t = 264; + for i in 1..=8 { + h.touch( + TouchAction::Move, + Vec2::new(540.0, from + 20.0 * i as f32), + t, + ); + t += 8; + } + h.touch(TouchAction::Up, Vec2::new(540.0, from + 160.0), t); + + assert_ne!( + list_before, + (screen.list)(&mut h.rsc).anchor_position_display(), + "the drag was nowhere near the fence, so it belongs to the list", + ); + assert!( + amt(&h, fence_scroll) > fence_before, + "the fence's fling must carry on through a gesture that was never \ + its own: {fence_before} -> {}", + amt(&h, fence_scroll), + ); +} diff --git a/iris/transcript-ui/src/composer.rs b/iris/transcript-ui/src/composer.rs index bd42991..fcf6c1d 100644 --- a/iris/transcript-ui/src/composer.rs +++ b/iris/transcript-ui/src/composer.rs @@ -33,6 +33,10 @@ const MAX_LINES: f32 = 6.0; const APPROX_LINE_HEIGHT_DP: f32 = 24.0; const FIELD_PAD_DP: f32 = 12.0; +/// The bar's own surface -- both what is drawn behind the field and what +/// the field is clipped to, see `build_composer`. +const BAR_FILL: UiColor = UiColor::new(40, 40, 46, 255); + /// `field` is exposed so the caller can read its content on submit /// (`field.edit(rsc).text()`) and clear it afterward /// (`field.edit(rsc).set("")`). @@ -83,31 +87,37 @@ where .label("Message") .add(rsc); - // One widget: an opaque bar sized to its own content (`.background`'s - // `Stack{child: 1}`, the header row's own idiom) wrapping the padded, - // height-capped field -- not a background rect and a field drawn as - // two independent siblings, which is what let the two disagree on - // where the bar actually was. - // `.scrollable().masked()`: the finger pan (`Scroll::drag`) plus the - // clip that keeps six lines' worth of a longer message inside the - // bar. The mask is the caller's job rather than `Scroll`'s own: - // `.masked()` is the one mechanism for clipping and this is one more - // use of it (tabs-ui's message area is the other). A `Scroll` nested - // under another masked area used to abort here; since 2026-09-07 the - // inner mask chains to the outer one (`Mask::parent`) and the content - // is clipped by both. - // Without it the overflow paints *above* the bar, over the - // transcript: measured before this change at 58px of stray text for a - // 475px message in a 417px box. + // One widget: an opaque bar sized to its own content, wrapping the + // padded, height-capped field -- not a background rect and a field + // drawn as two independent siblings, which is what let the two + // disagree on where the bar actually was. + // + // `.masked_by(rect(BAR_FILL))` is that bar *and* the clip, in one: + // the rect is drawn behind the field and is itself what the field is + // cut to (`Masked::shape`), so the surface and the edge content + // disappears at cannot fall out of step. It replaces a + // `.masked().background(rect(...))` pair, which clipped one box + // inside the other: the mask sat *inside* the `dp(FIELD_PAD_DP)` + // padding, so a message longer than the six lines shown was sliced + // through the middle of a glyph 12dp in from the bar's edge, leaving + // a band of bare surface above the cut. Iris, 2026-09-08: "the + // message input box doesn't clip correctly ... the box should be + // clipped rather than the inset text." The padding still holds the + // text off the edge at the end the content is anchored to; what + // scrolls past the other end now passes under the bar's own edge, + // the way `row.rs` already cuts a code fence to its panel. + // + // Without any mask at all the overflow paints *above* the bar, over + // the transcript: measured at 58px of stray text for a 475px message + // in a 417px box. let content = field // `scrollable_to_end`: what is being typed is at the end, so a // message longer than the six lines shown holds that end. .scrollable_to_end(Axis::Y) - .masked() .pad(dp(FIELD_PAD_DP)) .max_height(dp(APPROX_LINE_HEIGHT_DP * MAX_LINES + FIELD_PAD_DP * 2.0)) .width(rest(1)) - .background(rect(UiColor::new(40, 40, 46, 255))) + .masked_by(rect(BAR_FILL)) .add(rsc); let outer_pad: WeakWidget = content.pad(Padding::ZERO).add(rsc);