diff --git a/iris/Cargo.toml b/iris/Cargo.toml index 40e9c6f..c431c37 100644 --- a/iris/Cargo.toml +++ b/iris/Cargo.toml @@ -95,7 +95,7 @@ members = ["core", "macro", "tabs-ui", "transcript-ui", "desktop-app"] # buildable. Cross-compile it from its own directory (its own single-crate # workspace, since it has no `[workspace]` table of its own and this # exclusion stops it inheriting this one): `cd android-app && cargo ndk -# -t x86_64 -P 26 build`. +# -t x86_64 -P 29 build`. exclude = ["android-app"] [workspace.package] diff --git a/iris/android-app/app/build.gradle b/iris/android-app/app/build.gradle index b1f18e9..910e106 100644 --- a/iris/android-app/app/build.gradle +++ b/iris/android-app/app/build.gradle @@ -13,7 +13,15 @@ android { defaultConfig { applicationId = "dev.iris.android.demo" - minSdk = 26 + // 29, not 26: `iris::android::view`'s touch handler dates each + // sample with `MotionEvent.getEventTimeNanos` and + // `getHistoricalEventTimeNanos`, both API 29, and a missing JNI + // method there is a hard crash on the first touch rather than a + // degraded fling. Raised deliberately rather than guarded at + // runtime: nothing this app is built for runs below 29, and an + // untested fallback path is its own defect. `build-apk.sh`'s + // `cargo ndk -P` is kept at the same number. + minSdk = 29 targetSdk = 34 versionCode = 1 versionName = "1.0" diff --git a/iris/android-app/app/src/main/java/dev/iris/android/demo/IrisView.java b/iris/android-app/app/src/main/java/dev/iris/android/demo/IrisView.java index ef53bfc..dbe3fe8 100644 --- a/iris/android-app/app/src/main/java/dev/iris/android/demo/IrisView.java +++ b/iris/android-app/app/src/main/java/dev/iris/android/demo/IrisView.java @@ -27,7 +27,7 @@ public final class IrisView extends RustView { protected native long newViewPeer(Context context); native void applyWindowInsetsNative( - long peer, int left, int top, int right, int bottom, int imeBottom); + long peer, int left, int top, int right, int bottom, int imeBottom, int imeVisible); native void unregisterInsetsNative(long peer); @@ -35,8 +35,9 @@ public final class IrisView extends RustView { super(context); } - void applyWindowInsets(int left, int top, int right, int bottom, int imeBottom) { - applyWindowInsetsNative(mViewPeer, left, top, right, bottom, imeBottom); + void applyWindowInsets( + int left, int top, int right, int bottom, int imeBottom, int imeVisible) { + applyWindowInsetsNative(mViewPeer, left, top, right, bottom, imeBottom, imeVisible); } @Override diff --git a/iris/android-app/app/src/main/java/dev/iris/android/demo/MainActivity.java b/iris/android-app/app/src/main/java/dev/iris/android/demo/MainActivity.java index c822df4..aa87090 100644 --- a/iris/android-app/app/src/main/java/dev/iris/android/demo/MainActivity.java +++ b/iris/android-app/app/src/main/java/dev/iris/android/demo/MainActivity.java @@ -55,30 +55,33 @@ public final class MainActivity extends Activity { int top = insets.getSystemWindowInsetTop(); int right = insets.getSystemWindowInsetRight(); int bottom = insets.getSystemWindowInsetBottom(); - // The manifest declares adjustResize (AGENTS.md: without it the - // keyboard pans the whole window instead of resizing it), and - // under adjustResize the window itself shrinks to make room for - // the keyboard -- which is exactly the condition under which - // WindowInsets.Type.ime()'s own *inset amount* reports zero: it - // measures how much of the window the keyboard overlaps, and - // resize already made that overlap zero by construction. That - // numeric inset is not a usable "is the keyboard open" signal - // here (found while root-causing why bench_client.rs's keyboard - // phase and auto-diagnostics never fired on the emulator despite - // the keyboard visibly opening -- RUST.md's P0 box). What does - // survive adjustResize is the boolean isVisible() answer, set - // from the platform's own start/end of the transition over a - // different path than the inset amount -- the same fact - // AGENTS.md's "Things that have bitten" already names for the - // Compose side's identical trap. Passed through as a 0/1 stand- - // in for the ime_bottom pixel amount, since nothing on the Rust - // side reads it as a real pixel value -- only `> 0.0`. + // **Two separate answers, because they are separate questions** + // (Iris's phone, 2026-09-06: "message box does not push up the + // scroll area"). `isVisible(ime())` says whether the keyboard is + // up; `getInsets(ime()).bottom` says how tall it is. An earlier + // pass sent the boolean *as* the height (0 or 1) because under + // plain `adjustResize` the window shrinks to make room and the + // ime inset therefore measures a zero overlap by construction -- + // true then, and no longer true now: `setDecorFitsSystemWindows + // (false)` above makes this an edge-to-edge window, which is + // exactly the case where the system stops resizing and hands the + // app the real overlap instead. Sending 1 for it left the Rust + // side padding the composer by one physical pixel, so the + // keyboard covered the bar and the transcript alike. + // + // The visibility is still sent in its own right rather than + // inferred from `height > 0`: the two disagree during the + // keyboard's slide-in and -out (visible, height still climbing), + // and "is the IME up" drives the bench's own state machine + // (`bench_client.rs`'s `ime_state`) where a half-open frame + // reading as "closed" is a miscount. int imeBottom = 0; - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R - && insets.isVisible(WindowInsets.Type.ime())) { - imeBottom = 1; + int imeVisible = 0; + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { + imeBottom = insets.getInsets(WindowInsets.Type.ime()).bottom; + imeVisible = insets.isVisible(WindowInsets.Type.ime()) ? 1 : 0; } - ((IrisView) v).applyWindowInsets(left, top, right, bottom, imeBottom); + ((IrisView) v).applyWindowInsets(left, top, right, bottom, imeBottom, imeVisible); return insets; }); } diff --git a/iris/android-app/build-apk.sh b/iris/android-app/build-apk.sh index 7da862f..19f94e0 100755 --- a/iris/android-app/build-apk.sh +++ b/iris/android-app/build-apk.sh @@ -59,9 +59,9 @@ export ANDROID_NDK_HOME="$NDK_DIR" rm -rf app/src/main/jniLibs echo "build-apk.sh: cargo ndk -t $ABI build ${BUILD_TYPE:+(${BUILD_TYPE})} --features \"$FEATURES\"" if [ "$BUILD_TYPE" = "release" ]; then - cargo ndk -t "$ABI" -P 26 -o app/src/main/jniLibs/ build --release --features "$FEATURES" + cargo ndk -t "$ABI" -P 29 -o app/src/main/jniLibs/ build --release --features "$FEATURES" else - cargo ndk -t "$ABI" -P 26 -o app/src/main/jniLibs/ build --features "$FEATURES" + cargo ndk -t "$ABI" -P 29 -o app/src/main/jniLibs/ build --features "$FEATURES" fi GRADLE_TASK="assembleDebug" diff --git a/iris/android-app/src/bench_client.rs b/iris/android-app/src/bench_client.rs index 181a4dc..212b2c7 100644 --- a/iris/android-app/src/bench_client.rs +++ b/iris/android-app/src/bench_client.rs @@ -408,7 +408,12 @@ impl AndroidAppState for BenchClient { .set_bottom_inset(rsc, insets.bottom.max(insets.ime_bottom)); } - let ime_visible = insets.ime_bottom > 0.0; + // The platform's own answer, not `ime_bottom > 0.0` -- see + // `iris::android::WindowInsets::ime_bottom`. The height is still + // climbing while the keyboard slides in, so a frame or two of a + // real opening reads as "closed" when the boolean is inferred from + // it, and `shown_events`/`hidden_events` below count transitions. + let ime_visible = insets.ime_visible; let mut ime = self.ime_state.lock().unwrap(); if ime_visible && !ime.visible { diff --git a/iris/core/src/render/shader.wgsl b/iris/core/src/render/shader.wgsl index 8abe499..99afb2c 100644 --- a/iris/core/src/render/shader.wgsl +++ b/iris/core/src/render/shader.wgsl @@ -80,11 +80,17 @@ var masks: array; @group(3) @binding(1) var move_offsets: array; -// A move chain more than this deep means something else is wrong (an -// accidental cycle) -- kept in step with `MOVE_CHAIN_LIMIT` in -// render_state.rs, which walks the identical bound on the CPU side for -// hit-testing. Bounded so a malformed chain cannot hang the GPU. -const MOVE_CHAIN_LIMIT: u32 = 16u; +// The bound on the parent walk, kept in step with `MOVE_CHAIN_LIMIT` in +// render_state.rs, which walks the identical chain on the CPU side for +// hit-testing. Bounded so a malformed chain (a cyclic `parent`) cannot +// hang the GPU -- not a claim about how deep a real tree gets. It was 16 +// and that was too small: the transcript screen's composer field sits 17 +// slots below the root, measured 2026-09-07 on this checkout's emulator +// by tapping it (the CPU walk's own debug assert names the chain now). +// Past the bound both walks simply stop summing, so the widget draws and +// hit-tests short by whatever the outer slots held, with nothing on +// screen to say so. +const MOVE_CHAIN_LIMIT: u32 = 64u; /// Sums the pixel delta along the parent chain starting at `idx`, shared by /// the vertex stage (a primitive's own corners) and the fragment stage (its diff --git a/iris/core/src/ui/mod.rs b/iris/core/src/ui/mod.rs index efb07cc..966ba70 100644 --- a/iris/core/src/ui/mod.rs +++ b/iris/core/src/ui/mod.rs @@ -24,6 +24,46 @@ pub struct UiData { /// id (never reallocated), so a retained descendant's `parent` index /// never goes stale -- see LAYOUT.md section 2. pub move_offsets: TrackedArena, + /// Every widget whose [`crate::Widget::tick`] should run before the + /// next frame -- today, a `List` coasting through a fling. Added by + /// [`Self::animate`] when the animation starts and removed by + /// [`Self::tick_animations`] the frame its `tick` answers `false`, so + /// a stopped animation costs nothing and a dropped widget cannot be + /// ticked (`get_dyn_mut` answers `None` and it is dropped the same + /// way). + animating: Vec, +} + +impl UiData { + /// Ask for `id`'s [`crate::Widget::tick`] to run every frame until it + /// says it is done. Idempotent -- registering an already-animating + /// widget is the ordinary case (a second fling before the first + /// settled) and must not tick it twice per frame. + pub fn animate(&mut self, id: WidgetId) { + if !self.animating.contains(&id) { + self.animating.push(id); + } + } + + /// Tick every registered widget to `now`, drop the ones that finished, + /// and say whether any is still going -- which is a backend's cue to + /// ask for another frame. Called once per frame *before* the draw, so + /// what the frame draws is this instant's position rather than the + /// previous one's. + pub fn tick_animations(&mut self, now: std::time::Instant) -> bool { + // Taken out and put back rather than iterated in place: `tick` + // needs `&mut` on the widget arena this list lives beside, and a + // widget is free to register another one while ticking. + let mut registered = std::mem::take(&mut self.animating); + registered.retain(|&id| match self.widgets.get_dyn_mut(id) { + Some(widget) => widget.tick(now), + None => false, + }); + for id in registered { + self.animate(id); + } + !self.animating.is_empty() + } } pub trait UiRsc { diff --git a/iris/core/src/ui/render_state.rs b/iris/core/src/ui/render_state.rs index b14f6db..32627f3 100644 --- a/iris/core/src/ui/render_state.rs +++ b/iris/core/src/ui/render_state.rs @@ -60,10 +60,17 @@ pub struct UiRenderState { pub(super) shape_count: u64, } -/// A move chain more than this deep would mean something else is wrong -/// (an accidental cycle) -- see `resolve_move` in shader.wgsl, which walks -/// the identical bound and must be kept in step with this constant. -pub const MOVE_CHAIN_LIMIT: usize = 16; +/// The bound on the parent walk -- see `resolve_move` in shader.wgsl, +/// which walks the identical chain and must be kept in step with this +/// constant. It exists so a cyclic `parent` link cannot hang either walk, +/// not as a statement about how deep a real tree gets: it was 16, and the +/// transcript screen's composer field turned out to sit **17** slots below +/// the root (measured 2026-09-07 on this checkout's emulator, by tapping +/// the composer in a debug build -- the assert in `resolve_move_chain` +/// prints the chain). A chain past the bound is not reported anywhere at +/// run time; both walks just stop summing, so the widget is drawn and hit +/// tested short by whatever the outer slots held. +pub const MOVE_CHAIN_LIMIT: usize = 64; impl UiRenderState { pub fn new() -> Self { @@ -708,26 +715,56 @@ impl UiRenderState { /// pixel delta along the parent chain starting at `slot`. Both walks /// share `MOVE_CHAIN_LIMIT` as their bound so the two cannot disagree /// about where the chain ends. - fn resolve_move_chain(&self, mut slot: MoveIdx, rsc: &dyn UiRsc) -> Vec2 { + fn resolve_move_chain(&self, slot: MoveIdx, rsc: &dyn UiRsc) -> Vec2 { let offsets = &rsc.ui().move_offsets; let mut delta = Vec2::ZERO; + let mut at = slot; for i in 0..MOVE_CHAIN_LIMIT { - let entry = &offsets[slot.idx()]; + let entry = &offsets[at.idx()]; delta.x += entry.delta[0]; delta.y += entry.delta[1]; if entry.parent == MoveOffset::NONE_PARENT { return delta; } - slot = Id::preset(entry.parent); + at = Id::preset(entry.parent); + // The chain itself, not just the fact that it was too long: a + // cycle and a tree genuinely nested deeper than the shader can + // follow are different faults with different fixes, and the + // slot numbers are the only thing that tells them apart. debug_assert!( i + 1 < MOVE_CHAIN_LIMIT, - "move offset chain exceeded MOVE_CHAIN_LIMIT; a widget's `parent` link is \ - probably cyclic" + "move offset chain exceeded MOVE_CHAIN_LIMIT ({MOVE_CHAIN_LIMIT}): {chain} -- a \ + repeated slot means a `parent` link is cyclic, all-distinct slots mean the tree \ + nests deeper than shader.wgsl's own walk of the same bound", + chain = Self::move_chain_debug(slot, offsets) ); } delta } + /// The parent chain from `slot`, as `slot(dx, dy) -> ...`, walked twice + /// `MOVE_CHAIN_LIMIT` so a cycle shows up as a repeated slot number + /// rather than as a chain that merely stops. Only ever called from the + /// failed assertion above. + fn move_chain_debug(slot: MoveIdx, offsets: &[MoveOffset]) -> String { + let mut parts = Vec::new(); + let mut at = slot; + for _ in 0..MOVE_CHAIN_LIMIT * 2 { + let entry = &offsets[at.idx()]; + parts.push(format!( + "{}({}, {})", + at.idx(), + entry.delta[0], + entry.delta[1] + )); + if entry.parent == MoveOffset::NONE_PARENT { + break; + } + at = Id::preset(entry.parent); + } + parts.join(" -> ") + } + pub fn window_region(&self, id: &impl IdLike, rsc: &dyn UiRsc) -> Option { let region = self.resolved_region(id, rsc)?; Some(region.to_px(self.output_size)) diff --git a/iris/core/src/widget/mod.rs b/iris/core/src/widget/mod.rs index 895d161..9b180bd 100644 --- a/iris/core/src/widget/mod.rs +++ b/iris/core/src/widget/mod.rs @@ -41,6 +41,25 @@ pub trait Widget: Any { fn access_role(&self) -> accesskit::Role { accesskit::Role::Unknown } + + /// Advance whatever this widget is animating to `now`, and say whether + /// it is still animating afterwards. Default: nothing is, so a widget + /// opts in by overriding this *and* by something calling + /// [`crate::UiData::animate`] with its id when the animation starts -- + /// which is that animation's path out, since the driver + /// ([`crate::UiData::tick_animations`]) drops every id whose `tick` + /// answers `false`. + /// + /// Called once per frame, before the frame's draw, by whichever + /// backend owns the surface; a `true` answer is what makes that + /// backend ask for another frame. So this is the only thing in iris + /// that moves without an input event, and a widget that animates + /// without registering simply never moves -- which is exactly how a + /// finger fling looked on Iris's phone before this existed. + #[allow(unused_variables)] + fn tick(&mut self, now: std::time::Instant) -> bool { + false + } } impl Widget for () { diff --git a/iris/src/android/insets.rs b/iris/src/android/insets.rs index 6f4383f..98dc5b2 100644 --- a/iris/src/android/insets.rs +++ b/iris/src/android/insets.rs @@ -45,11 +45,21 @@ pub struct Insets { pub top: i32, pub right: i32, pub bottom: i32, - /// The keyboard's own inset (`WindowInsetsCompat.Type.ime()`), separate - /// from `bottom` (the system bars): a layout wants to know about the - /// keyboard specifically, since it usually means "make room" rather - /// than "stay clear of a corner". + /// The keyboard's own inset (`WindowInsets.Type.ime()`), in physical + /// pixels, separate from `bottom` (the system bars): a layout wants to + /// know about the keyboard specifically, since it usually means "make + /// room" rather than "stay clear of a corner". pub ime_bottom: i32, + /// `WindowInsets.isVisible(ime())` -- whether the keyboard is up, which + /// is **not** the same question as `ime_bottom > 0` and is why the two + /// are carried separately. They disagree for the frames the keyboard + /// spends sliding: visible, with a height still on its way to the full + /// one. Anything asking "make how much room" reads `ime_bottom`; + /// anything asking "is the keyboard up" reads this. See + /// `MainActivity.java`'s comment for the history -- the height used to + /// be sent *as* this boolean, which is what left the composer padded by + /// one pixel on Iris's phone. + pub ime_visible: bool, } #[derive(Default)] @@ -89,6 +99,7 @@ extern "system" fn apply_window_insets<'local>( right: jint, bottom: jint, ime_bottom: jint, + ime_visible: jint, ) { if let Some(shared) = map().lock().unwrap().get(&peer) { shared.borrow_mut().insets = Insets { @@ -97,6 +108,7 @@ extern "system" fn apply_window_insets<'local>( right, bottom, ime_bottom, + ime_visible: ime_visible != 0, }; } // Insets can change (the keyboard opening) with no resize and no @@ -115,7 +127,7 @@ pub fn register_native_methods<'local, 'other_local>( &[ NativeMethod { name: "applyWindowInsetsNative".into(), - sig: "(JIIIII)V".into(), + sig: "(JIIIIII)V".into(), fn_ptr: apply_window_insets as *mut c_void, }, NativeMethod { diff --git a/iris/src/android/view.rs b/iris/src/android/view.rs index 57059bd..391cf76 100644 --- a/iris/src/android/view.rs +++ b/iris/src/android/view.rs @@ -7,9 +7,9 @@ use android_view::{ jni::{ JNIEnv, JavaVM, objects::{GlobalRef, JValue}, - sys::jint, + sys::{jint, jlong}, }, - ndk::event::{Keycode, MotionAction}, + ndk::event::{Axis, Keycode, MotionAction}, }; // `marker::Sized` explicitly: `crate::prelude::*` below also brings in the // `Sized` *widget* (`widget::position::sized::Sized`), and an unqualified @@ -20,7 +20,7 @@ use std::{ marker::{PhantomData, Sized}, rc::Rc, sync::Arc, - time::Instant, + time::{Duration, Instant}, }; use super::{ @@ -195,7 +195,12 @@ pub struct WindowInsets { pub top: f32, pub right: f32, pub bottom: f32, + /// How much of the window the keyboard covers, in physical pixels -- + /// what a layout pads by. See `insets::Insets::ime_visible` for why + /// "is the keyboard up" is a separate field rather than this one + /// compared against zero. pub ime_bottom: f32, + pub ime_visible: bool, } impl WindowInsets { @@ -206,6 +211,7 @@ impl WindowInsets { right: insets.right as f32, bottom: insets.bottom as f32, ime_bottom: insets.ime_bottom as f32, + ime_visible: insets.ime_visible, } } } @@ -284,6 +290,12 @@ pub struct IrisViewPeer { pub(super) render: UiRenderState, pub(super) state: State, task_recv: TaskMsgReceiver>, + /// `(an Instant, the input-event nanosecond stamp it was taken at)`, + /// captured from the first `MotionEvent` this view receives and never + /// changed after -- how `on_touch_event` dates every touch sample. Its + /// path out is the peer's own drop: it holds nothing but two numbers + /// and is meaningless to any other view. + input_clock: Option<(Instant, jlong)>, } impl>> std::ops::Index for AndroidRsc { @@ -307,12 +319,13 @@ impl IrisViewPeer { } } - /// Common tail for every callback that might have changed the cursor, - /// the text focus, or the widget tree: run the sensors that touch - /// input feeds, then ask for a frame if the result needs drawing. - /// Mirrors `default::DefaultApp::window_event`'s tail, split across - /// android-view's several entry points instead of winit's one. - pub(super) fn after_input(&mut self, ctx: &mut CallbackCtx) { + /// One pointer sample through the sensors, plus the platform calls a + /// handler can only ask for by raising a flag. Split out of + /// [`Self::after_input`] because a batched `MotionEvent` carries + /// several samples that all belong to the same *frame* + /// (`on_touch_event`): each one is a real input frame the widgets must + /// see, but only the last one ends the frame and asks for a redraw. + fn run_input_frame(&mut self, ctx: &mut CallbackCtx) { let window_size = self.window_size(); let ui_state = self.state.android_state_mut(); let cursor = ui_state.cursor.clone(); @@ -332,6 +345,15 @@ impl IrisViewPeer { if let Some(url) = ui_state.pending_open_url.take() { super::platform::open_url(&mut ctx.env, &ctx.view, &url); } + } + + /// Common tail for every callback that might have changed the cursor, + /// the text focus, or the widget tree: run the sensors that touch + /// input feeds, then ask for a frame if the result needs drawing. + /// Mirrors `default::DefaultApp::window_event`'s tail, split across + /// android-view's several entry points instead of winit's one. + pub(super) fn after_input(&mut self, ctx: &mut CallbackCtx) { + self.run_input_frame(ctx); // RUST.md's P0 box, "doesn't enter it until I hit space, and also // doesn't move cursor forward": Gboard needs `updateSelection` @@ -383,12 +405,14 @@ impl IrisViewPeer { // is actually fed have to reach the log -- "the composer // floats at launch" is unanswerable from a screenshot alone. log::info!( - "iris insets: left={} top={} right={} bottom={} ime_bottom={} window={:?}", + "iris insets: left={} top={} right={} bottom={} ime_bottom={} \ + ime_visible={} window={:?}", physical.left, physical.top, physical.right, physical.bottom, physical.ime_bottom, + physical.ime_visible, self.window_size(), ); self.state.android_state_mut().last_insets = current_insets; @@ -414,6 +438,12 @@ impl IrisViewPeer { // both count. See `iris_core::FrameReport`'s own doc for exactly // what this does and does not measure. let frame_start = Instant::now(); + // Anything moving on its own -- today a `List` coasting through a + // fling -- is advanced here, before the draw, and asks for the + // next frame at the end of this one. See + // `UiData::tick_animations`; `default/mod.rs`'s + // `RedrawRequested` arm is the same two lines for winit. + let animating = self.rsc.ui.tick_animations(frame_start); let ui_state = self.state.android_state_mut(); self.render.update(&ui_state.root, &mut self.rsc); let ui_state = self.state.android_state_mut(); @@ -446,6 +476,12 @@ impl IrisViewPeer { .android_state_mut() .frame_report .record_split(frame_start.elapsed(), submit_to_present); + // A frame callback is one-shot, so an animation that wants + // another frame has to say so every frame -- unlike `after_input`, + // which only has to ask when input dirtied something. + if animating { + ctx.view.post_frame_callback(&mut ctx.env); + } let ui_state = self.state.android_state(); log::debug!( "render(): after update active={} root_px={:?}", @@ -554,7 +590,67 @@ impl ViewPeer for IrisViewPeer { // -- see `AndroidUiState::content_scale`'s field comment. let x = event.x(&mut ctx.env); let y = event.y(&mut ctx.env); + // The event's own clock, converted through one anchor taken on the + // first touch this view ever sees. Android reports sample times in + // the `SystemClock.uptimeMillis()` base, which is the same + // `CLOCK_MONOTONIC` an `Instant` reads, so a single + // `(Instant, nanos)` pair converts every later sample exactly. + // Anchoring **once** rather than per event is what keeps the times + // ordered: a fresh `Instant::now()` per event, minus each sample's + // age inside it, can date a later event's first historical sample + // before the previous event's last one whenever delivery jitters by + // more than the batch spans -- and `VelocityTracker::add_sample`'s + // debug assert would rightly fire on that. See `CursorState::time`. + let event_time = event.event_time_nanos(&mut ctx.env); + let (anchor_at, anchor_nanos) = + *self.input_clock.get_or_insert((Instant::now(), event_time)); + let at = |sample_time: jlong| { + anchor_at + Duration::from_nanos(sample_time.saturating_sub(anchor_nanos).max(0) as u64) + }; + + // **Historical samples first.** A flick on a 120Hz screen is + // delivered as one or two `MotionEvent`s with the intermediate + // positions batched inside them, so reading only `x()`/`y()` threw + // away every sample but the last: the velocity tracker saw one + // `Pan` for the whole gesture, `VelocityTracker::velocity` answers + // 0.0 below two samples, and the release therefore flung at zero -- + // Iris's phone, twice ("fling still doesn't work"), while a + // `ui-trace` swipe, which is many evenly-spaced events, flung fine. + // Replayed one at a time through the sensors rather than summarised, + // so the arbiter, the tracker and any other sensor all see the same + // motion the finger actually made; only the last sample ends the + // frame (`after_input`). + if matches!(action, MotionAction::Move) { + let history = event.history_size(&mut ctx.env); + // Android documents the historical samples as oldest first and + // the event's own sample as the newest of the batch; everything + // downstream (`VelocityTracker`, `DragArbiter`'s long-press + // clock) assumes it, so say so here rather than at each reader. + let mut previous = anchor_nanos; + for pos in 0..history { + let hx = event.historical_axis(&mut ctx.env, Axis::X, 0, pos); + let hy = event.historical_axis(&mut ctx.env, Axis::Y, 0, pos); + let ht = event.historical_event_time_nanos(&mut ctx.env, pos); + debug_assert!( + ht >= previous, + "historical sample {pos} of {history} is dated {ht}ns, before the {previous}ns \ + sample ahead of it -- the input clock is not what this assumes" + ); + previous = ht; + let ui_state = self.state.android_state_mut(); + ui_state.cursor.pos = vec2(hx, hy); + ui_state.cursor.time = at(ht); + self.run_input_frame(ctx); + } + debug_assert!( + event_time >= previous, + "the event's own sample is dated {event_time}ns, before its last historical \ + sample at {previous}ns" + ); + } + let ui_state = self.state.android_state_mut(); + ui_state.cursor.time = at(event_time); match action { MotionAction::Down => { ui_state.cursor.pos = vec2(x, y); @@ -564,6 +660,13 @@ impl ViewPeer for IrisViewPeer { MotionAction::Move => { ui_state.cursor.pos = vec2(x, y); } + // `Cancel` ends the gesture the same way `Up` does, and must: + // a release that never arrives leaves whichever widget took + // pointer capture holding it forever, with every later touch + // delivered to a drag nobody is performing. Confirmed present + // before this pass rather than assumed -- it was one of the + // three suspects listed for the phone's missing fling, and it + // is not the cause. MotionAction::Up | MotionAction::Cancel => { ui_state.cursor.pos = vec2(x, y); ui_state.cursor.buttons.left.update(false); @@ -894,6 +997,7 @@ pub fn new_peer<'local, State: AndroidAppState>( render, state, task_recv, + input_clock: None, }; let id = android_view::register_view_peer(peer); super::insets::register(id, shared); diff --git a/iris/src/attr.rs b/iris/src/attr.rs index 8fc18f4..3c8a473 100644 --- a/iris/src/attr.rs +++ b/iris/src/attr.rs @@ -18,9 +18,14 @@ pub trait FocusHost { /// side effect the way a real double-click timer does. fn recent_click(&mut self) -> bool; fn set_focus(&mut self, id: Option>); - /// Called after a `TextEdit` becomes the focus target, with the region - /// it was hit in (`None` when the widget could not be located, which - /// happens for one it was just deselected from). + /// Called on every tap that should put the IME on `id`: the tap that + /// *makes* a `TextEdit` the focus target, and any later tap on one that + /// already is. `region` is where it was hit (`None` when the widget + /// could not be located, which happens for one it was just deselected + /// from). Implementations must be idempotent -- both backends' calls + /// (`showSoftInput`, `set_ime_cursor_area`) already are, which is what + /// lets the repeat tap be handled by the same call rather than by a + /// second "re-show" entry point beside it. fn focus_gained(&mut self, region: Option); /// Whether `id` is the current focus target -- what [`select`] uses to /// tell a fresh press (which must wait to see whether it becomes a tap @@ -155,10 +160,30 @@ fn on_press( ctx.text.press_origin = None; return; } - if matches!(sense, CursorSense::PressEnd(_)) { + let ended = matches!(sense, CursorSense::PressEnd(_)); + if ended { ctx.text.press_origin = None; } ctx.select(pos, size, true, false); + // A tap on a field that is *already* focused asks for the + // keyboard again (Iris's phone, 2026-09-06: "I can't reopen + // keyboard by tapping on message box after it already + // happened once"). Dismissing the IME -- back gesture, or + // its own hide button -- takes the keyboard away but leaves + // the field focused, so without this the one branch that + // requests it (the unfocused one below) never runs again + // and the field is permanently unable to summon it. + // Android's own `EditText` does exactly this: every tap on + // a focused field calls `showSoftInput`, which is a no-op + // when the keyboard is already up. + // + // Gated on the same tap-vs-drag test the unfocused branch + // uses, not on `PressEnd` alone, so a drag-to-select that + // happens to finish inside the field does not summon a + // keyboard the reader was not asking for. + if ended && dx.abs() <= DRAG_SLOP && dy.abs() <= DRAG_SLOP { + state.focus_gained(render.window_region(&id, &*rsc)); + } } _ => {} } diff --git a/iris/src/default/input.rs b/iris/src/default/input.rs index edf044a..61cfa2e 100644 --- a/iris/src/default/input.rs +++ b/iris/src/default/input.rs @@ -1,4 +1,10 @@ +// `CursorState::time` is the sample's own time on every backend. winit +// carries no timestamp on a pointer event, so the moment it is handed to +// us is the closest measurement available here -- which is also what the +// drag code used to do for itself with `Instant::now()`, before Android's +// batched samples made the difference matter (see `sense::CursorState`). use crate::prelude::*; +use std::time::Instant; use winit::{ event::{MouseButton, MouseScrollDelta, WindowEvent}, keyboard::{Key, NamedKey}, @@ -21,8 +27,10 @@ impl Input { WindowEvent::CursorMoved { position, .. } => { self.cursor.pos = Vec2::new(position.x as f32, position.y as f32) / scale_factor; self.cursor.exists = true; + self.cursor.time = Instant::now(); } WindowEvent::MouseInput { state, button, .. } => { + self.cursor.time = Instant::now(); let buttons = &mut self.cursor.buttons; let pressed = state.is_pressed(); match button { @@ -44,6 +52,7 @@ impl Input { delta.y = 0.0; } self.cursor.scroll_delta = delta; + self.cursor.time = Instant::now(); } WindowEvent::CursorLeft { .. } => { self.cursor.exists = false; diff --git a/iris/src/default/mod.rs b/iris/src/default/mod.rs index cff1fd5..d8b6750 100644 --- a/iris/src/default/mod.rs +++ b/iris/src/default/mod.rs @@ -267,9 +267,21 @@ impl AppState for DefaultApp { match &event { WindowEvent::CloseRequested => event_loop.exit(), WindowEvent::RedrawRequested => { + // Before the draw, so this frame shows this instant's + // position (`UiData::tick_animations`' own doc), and the + // window is asked for another frame while anything is + // still moving -- the winit half of what + // `IrisViewPeer::render`'s `post_frame_callback` does on + // Android. Nothing else in iris moves without an input + // event. + let animating = rsc.ui_mut().tick_animations(std::time::Instant::now()); + let ui_state = state.default_state_mut(); render.update(&ui_state.root, rsc); ui_state.renderer.update(&mut rsc.ui, render); ui_state.renderer.draw(); + if animating { + ui_state.window.request_redraw(); + } // I4 (RUST.md): only produces a `TreeUpdate` when the named // set actually changed this frame -- see `AccessTree`'s doc // comment. `render` reflects the draw that just happened, diff --git a/iris/src/sense.rs b/iris/src/sense.rs index 04f424b..4d31d62 100644 --- a/iris/src/sense.rs +++ b/iris/src/sense.rs @@ -95,12 +95,39 @@ impl CursorSense { } } -#[derive(Default, Clone)] +#[derive(Clone)] pub struct CursorState { pub pos: Vec2, pub exists: bool, pub buttons: CursorButtons, pub scroll_delta: Vec2, + /// When this pointer state was *sampled*, from the platform's own + /// input clock -- not when the handler reading it happened to run. + /// + /// It exists because Android batches touch samples: a flick on a + /// 120Hz screen arrives as one or two `MotionEvent`s carrying the + /// intermediate positions as *historical* samples + /// (`getHistoricalX`/`getHistoricalEventTime`), which + /// `IrisViewPeer::on_touch_event` replays through the sensor pass one + /// at a time. Every one of those replays happens within the same few + /// microseconds, so a gesture timing itself with `Instant::now()` + /// would see a span of nearly zero across the whole flick and divide + /// by it -- the velocity would be an artefact of how fast we looped, + /// which is exactly the inferred-as-measured number UI_RULES.md + /// forbids. Carrying the sample's own time makes the span real. + pub time: Instant, +} + +impl Default for CursorState { + fn default() -> Self { + Self { + pos: Vec2::ZERO, + exists: false, + buttons: CursorButtons::default(), + scroll_delta: Vec2::ZERO, + time: Instant::now(), + } + } } #[derive(Default, Clone)] @@ -732,6 +759,18 @@ impl DragGesture { match sense { CursorSense::PressStart(_) => { self.velocity.reset(); + // The press itself is a sample: nothing has moved yet, but + // *when* the finger went down is real and measured, and + // without it a gesture whose whole motion arrives in one + // frame has a single sample and therefore no time span to + // divide by -- `velocity` answers 0.0 and the release does + // not fling. Batched touch delivery makes that shape + // ordinary rather than rare (see `CursorState::time`), and + // `VELOCITY_WINDOW` trims this entry back out the moment + // the gesture is long enough not to need it, so a slow + // drag's velocity is still its recent motion and not its + // whole history. + self.velocity.add_sample(0.0, now); self.arbiter.press_start(pos_window, now, already_selected); self.dispatch(render, id, pos_window, now) } @@ -743,6 +782,20 @@ impl DragGesture { } else { GestureOutcome::Released(None) }; + // The one line that settles "why did that flick not fling" + // from a logcat, which is the only instrument available on + // Iris's phone (this-machine-android: system tracing does + // not work there). Every input to the decision is here, so + // a zero velocity can be told apart from a gesture that + // never reached `Panning` at all -- the two look identical + // on screen and had to be guessed between twice. + log::info!( + "iris drag release: samples={} span={:.1}ms v={:.0} outcome={:?}", + self.velocity.sample_count(), + self.velocity.span().as_secs_f32() * 1000.0, + self.velocity.velocity(), + outcome, + ); self.arbiter.release(); render.release_pointer(); outcome @@ -752,6 +805,7 @@ impl DragGesture { // landed outside whichever hit region first noticed it. _ if self.arbiter.is_idle() => { self.velocity.reset(); + self.velocity.add_sample(0.0, now); self.arbiter.press_start(pos_window, now, already_selected); self.dispatch(render, id, pos_window, now) } @@ -835,6 +889,22 @@ impl VelocityTracker { } } + /// How many samples are currently inside the window, and how long they + /// span. Reported beside the velocity in `DragGesture`'s release log, + /// because a `v=0` on its own cannot say whether the gesture was slow + /// or whether the tracker was simply never fed -- which is exactly the + /// distinction the phone's missing fling turned on. + pub fn sample_count(&self) -> usize { + self.samples.len() + } + + pub fn span(&self) -> Duration { + match (self.samples.front(), self.samples.back()) { + (Some(&(first, _)), Some(&(last, _))) => last.duration_since(first), + _ => Duration::ZERO, + } + } + /// The estimated speed, in units-per-second, over whatever samples /// currently fall inside the tracking window: total motion divided by /// the elapsed time between the oldest and newest sample still held. @@ -844,13 +914,7 @@ impl VelocityTracker { return 0.0; } let total: f32 = self.samples.iter().map(|&(_, d)| d).sum(); - let span = self - .samples - .back() - .unwrap() - .0 - .duration_since(self.samples.front().unwrap().0) - .as_secs_f32(); + let span = self.span().as_secs_f32(); if span <= 0.0 { 0.0 } else { total / span } } } @@ -964,6 +1028,17 @@ mod android_fling_spline { /// friction of `0.84` per frame at 60Hz corresponds to /// (`ln(0.78)/ln(0.9)`, `SplineOverScroller.DECELERATION_RATE`). const FLING_FRICTION: f32 = 0.015; +/// AOSP's own look-and-feel tuning constant, the argument +/// `SplineOverScroller`'s constructor passes to `computeDeceleration` when +/// it builds `mPhysicalCoeff` -- *not* the scroll friction, which is a +/// different number used a different place in the same formula. This was +/// `FLING_FRICTION` here until 2026-09-07, making the coefficient 56x too +/// small, which put an `ln` of a 56x-too-large ratio through +/// `exp(_/(rate-1))`: an ordinary flick came out lasting **30 seconds** +/// instead of 1.6. Nothing could see it while a finger fling never +/// animated at all (`List::fling`'s doc), which is why two defects had to +/// be fixed before either was visible. +const FLING_TUNING: f32 = 0.84; fn deceleration_rate() -> f32 { (0.78f32.ln()) / (0.9f32.ln()) } @@ -975,11 +1050,15 @@ const GRAVITY_EARTH: f32 = 9.80665; /// ported the same way Compose's `FlingCalculator` is, including its /// `density`-dependent physical coefficient (`computeDeceleration`, /// `GravityEarth * 39.37 * density * 160 * friction`). Density and -/// velocity/distance units cancel algebraically as long as velocity and -/// the returned distance share one pixel space (physical or logical) -- -/// [`crate::widget::List::fling`] relies on exactly that cancellation to -/// avoid needing a display density of its own, since iris's `List` -/// already works in logical (density-independent) pixels throughout. +/// `density` is physical pixels per `dp`, and the velocity handed in has +/// to be in those same physical pixels -- which is what a touch event +/// carries. It does **not** cancel out: `duration` is +/// `exp(ln(k*v/C) / (rate-1))` with `C` proportional to density, so the +/// wrong density changes how long a fling lasts exponentially rather than +/// scaling it. An earlier version of this comment claimed the opposite and +/// `List::fling` passed `1.0`; on a 2.75-density screen that gave a +/// one-second flick a 45-second coast (measured 2026-09-07). `List` reads +/// its density from the painter now. pub struct FlingCalculator { physical_coefficient: f32, } @@ -987,7 +1066,7 @@ pub struct FlingCalculator { impl FlingCalculator { pub fn new(density: f32) -> Self { Self { - physical_coefficient: GRAVITY_EARTH * 39.37 * density * 160.0 * FLING_FRICTION, + physical_coefficient: GRAVITY_EARTH * 39.37 * density * 160.0 * FLING_TUNING, } } @@ -1147,6 +1226,39 @@ mod fling_calculator_tests { } } + /// The absolute numbers, against AOSP's own formula worked by hand -- + /// the one thing every other test here cannot see, because they all + /// compare this calculator with itself (monotonic, signed, integrates + /// to the closed form) and so pass just as happily with a coefficient + /// 56x out. That is exactly the state this file was in: an ordinary + /// flick lasted 30 seconds on the emulator and every test was green. + /// + /// `SplineOverScroller` at ppi = 2.75*160 = 440: + /// `mPhysicalCoeff = 9.80665 * 39.37 * 440 * 0.84 = 142,698`; + /// `l = ln(0.35 * v / (0.015 * mPhysicalCoeff))`; + /// `duration = exp(l / (DECELERATION_RATE - 1))`. + /// For v = 3000 px/s that is 0.592s and 621px; for 11444 px/s, + /// 1.586s. + #[test] + fn a_flick_lasts_what_aosps_own_formula_says_it_does() { + let calc = FlingCalculator::new(2.75); + let slow = calc.duration(3000.0).as_secs_f32(); + assert!( + (slow - 0.592).abs() < 0.02, + "3000px/s at density 2.75 should settle in ~0.59s, got {slow}s" + ); + let distance = calc.distance(3000.0); + assert!( + (distance - 621.5).abs() < 5.0, + "3000px/s at density 2.75 should travel ~621px, got {distance}" + ); + let fast = calc.duration(11444.0).as_secs_f32(); + assert!( + (fast - 1.586).abs() < 0.05, + "11444px/s at density 2.75 should settle in ~1.59s, got {fast}s" + ); + } + #[test] fn position_at_is_monotonic_and_clamped_past_the_end() { let calc = FlingCalculator::new(1.0); @@ -1410,3 +1522,153 @@ mod drag_arbiter_tests { ); } } + +/// [`DragGesture`] end to end, at the shape Android actually delivers a +/// flick in. The arbiter and the tracker each behave correctly on their +/// own (the two modules above); what these cover is the join between them +/// at release, which is where the phone's missing fling lived. +#[cfg(test)] +mod drag_gesture_tests { + use super::*; + use std::sync::LazyLock; + + static BASE: LazyLock = LazyLock::new(Instant::now); + + fn t(ms: u64) -> Instant { + *BASE + Duration::from_millis(ms) + } + + /// A `UiRenderState` with nothing in it. `DragGesture` only ever calls + /// `capture_pointer`/`release_pointer` on it, which are bookkeeping on + /// a `Cell` and need no widget tree behind them. + fn render() -> UiRenderState { + UiRenderState::new() + } + + /// The id `capture_pointer` records. Any id will do -- nothing here + /// resolves it -- so it comes from a real (empty) widget registry + /// rather than being fabricated. + fn some_id(ui: &mut UiData) -> WidgetId { + ui.widgets.add_strong(Rect::new(UiColor::WHITE)).id() + } + + /// **The phone's shape.** A 120Hz flick reaches the app as very few + /// `MotionEvent`s, so before `on_touch_event` replayed the historical + /// samples inside them a whole gesture could be press, one move past + /// the slop, release. That released at `v=0` -- `velocity()` needs two + /// samples and the single `Pan` frame was the only one -- so the list + /// stopped dead under the finger while the same gesture driven as many + /// evenly-spaced `ui-trace` events flung perfectly. The press is a + /// sample now, so even this minimum still carries a real speed. + #[test] + fn a_flick_delivered_as_one_move_frame_still_releases_with_a_velocity() { + let mut ui = UiData::default(); + let id = some_id(&mut ui); + let r = render(); + let mut g = DragGesture::new(); + + g.handle( + &r, + id, + CursorSense::PressStart(CursorButton::Left), + Vec2::ZERO, + t(0), + false, + ); + g.handle( + &r, + id, + CursorSense::Pressing(CursorButton::Left), + Vec2::new(0.0, 100.0), + t(8), + false, + ); + let out = g.handle( + &r, + id, + CursorSense::PressEnd(CursorButton::Left), + Vec2::new(0.0, 100.0), + t(16), + false, + ); + + // (100 - DRAG_SLOP) px over the 8ms between the press and the one + // move that arrived: a real measurement of what was delivered, not + // an estimate of what the finger "probably" did in between. + let expected = (100.0 - DRAG_SLOP) / 0.008; + match out { + GestureOutcome::Released(Some(v)) => { + assert!((v - expected).abs() < 1.0, "expected ~{expected}, got {v}"); + } + other => panic!("expected a released pan, got {other:?}"), + } + } + + /// The other half of the same join, and the case the fix had no + /// reason to touch: a press and release with no motion at all is a + /// tap, and must not acquire a velocity from the seeded press sample. + #[test] + fn a_tap_is_still_a_tap_and_flings_nothing() { + let mut ui = UiData::default(); + let id = some_id(&mut ui); + let r = render(); + let mut g = DragGesture::new(); + + g.handle( + &r, + id, + CursorSense::PressStart(CursorButton::Left), + Vec2::ZERO, + t(0), + false, + ); + let out = g.handle( + &r, + id, + CursorSense::PressEnd(CursorButton::Left), + Vec2::ZERO, + t(20), + false, + ); + assert_eq!(out, GestureOutcome::Tapped); + } + + /// A long-press selection released while the finger was still moving + /// must not fling either -- `Released(None)`, never the tracked + /// velocity. Also untouched by the press-seeding above, which is why + /// it is checked here rather than assumed. + #[test] + fn a_selection_release_carries_no_velocity() { + let mut ui = UiData::default(); + let id = some_id(&mut ui); + let r = render(); + let mut g = DragGesture::new(); + + g.handle( + &r, + id, + CursorSense::PressStart(CursorButton::Left), + Vec2::ZERO, + t(0), + false, + ); + // Held still past LONG_PRESS, which is what starts a selection. + g.handle( + &r, + id, + CursorSense::Pressing(CursorButton::Left), + Vec2::ZERO, + t(0) + LONG_PRESS, + false, + ); + let out = g.handle( + &r, + id, + CursorSense::PressEnd(CursorButton::Left), + Vec2::new(0.0, 50.0), + t(0) + LONG_PRESS + Duration::from_millis(10), + false, + ); + assert_eq!(out, GestureOutcome::Released(None)); + } +} diff --git a/iris/src/sense_tests.rs b/iris/src/sense_tests.rs index 5c75a14..49239f0 100644 --- a/iris/src/sense_tests.rs +++ b/iris/src/sense_tests.rs @@ -51,6 +51,7 @@ fn cursor_at(pos: Vec2) -> CursorState { exists: true, buttons: Default::default(), scroll_delta: Vec2::ZERO, + ..Default::default() } } diff --git a/iris/src/widget/list.rs b/iris/src/widget/list.rs index 62d4dec..50f758e 100644 --- a/iris/src/widget/list.rs +++ b/iris/src/widget/list.rs @@ -225,6 +225,21 @@ pub struct List { /// (headless tests, a caller driving `tick_fling` by hand as /// `bench_client.rs`'s scripted phases do). redraw: Option>, + /// Physical pixels per `dp`, copied from the painter on every `draw` + /// -- what [`Self::fling`] hands `FlingCalculator`. 1.0 until this + /// list has been drawn once, which is also the only state in which a + /// fling is impossible (`fling` needs an anchor, and an anchor comes + /// from a draw). + /// + /// It has to be the real one: the deceleration constant is + /// `GRAVITY * 39.37 * density * 160 * friction`, and the velocity fed + /// in is in the same physical pixels the touch events arrive in, so a + /// hardcoded 1.0 against a 2.75-density screen does not cancel out -- + /// it makes the fling last exponentially too long. Measured on this + /// checkout's emulator, 2026-09-07, once flings could animate at all: + /// a flick that should coast for about a second ran for **45 + /// seconds**. + density: f32, /// Whether the last `draw` found no more content above the topmost /// visible row (its top edge at or past the viewport's own top, with /// no `prev_slot`) -- what `tick_fling` clamps a fling moving toward @@ -261,6 +276,7 @@ impl List { last_viewport_len: 0.0, fling: None, redraw: None, + density: 1.0, at_start: false, at_end: false, pending_tap: None, @@ -423,6 +439,26 @@ impl List { /// `FlingCalculator`'s own doc) -- `List` works entirely in logical /// pixels, so `1.0` here is not a placeholder for "unknown density," /// it is the correct density for a self-consistent unit system. + /// **Sets the fling; it does not drive it.** A fling moves only while + /// something calls [`Self::tick_fling`] once per frame, and what does + /// that in a running app is `UiData::tick_animations`, over the ids + /// `UiData::animate` was given. So a caller starting a fling from a + /// gesture registers the list in the same breath: + /// + /// ```ignore + /// list(ui).fling(-velocity); + /// let id = list.id(); + /// ui.ui_mut().animate(id); + /// ``` + /// + /// Split that way because the two halves have different owners: the + /// velocity is the list's business, and whether anything animates at + /// all is the frame loop's. Missing the second call is what a finger + /// fling did on Iris's phone for two builds -- the velocity was right + /// and nothing ever advanced it, which looks exactly like a list that + /// stops dead under the finger. A caller driving frames itself + /// (`bench_client.rs`'s fling phase, the headless tests) calls + /// `tick_fling` directly instead and does not register. pub fn fling(&mut self, velocity_px_per_s: f32) { // A NaN/inf velocity (a `VelocityTracker::velocity()` divide-by- // near-zero span, or a caller passing a raw device value straight @@ -436,7 +472,7 @@ impl List { return; } self.fling = Some(Fling { - calc: FlingCalculator::new(1.0), + calc: FlingCalculator::new(self.density), velocity: velocity_px_per_s, started_at: Instant::now(), applied: 0.0, @@ -876,8 +912,20 @@ impl List { const GENEROUS_PADDING: f32 = 100_000.0; impl Widget for List { + /// A `List` animates exactly one thing, a fling + /// ([`Self::tick_fling`]). The registration that makes this run is + /// `UiData::animate` beside the `fling` call -- see `fling`'s own doc. + fn tick(&mut self, now: Instant) -> bool { + self.tick_fling(now) + } + fn draw(&mut self, painter: &mut Painter) -> Size { let axis = self.axis; + // Learned from the frame rather than passed in: a fling's + // deceleration is a physical quantity and needs the real display + // density, and `draw` is where this widget meets the only thing + // that knows it. See `fling`. + self.density = painter.density(); let output_len = painter.output_size().axis(axis); self.viewport_len = painter.region().axis(axis).len().to_abs(output_len); @@ -1573,6 +1621,56 @@ mod tests { assert!(!rsc.ui.widgets.get(&list_weak).unwrap().is_scrolling()); } + /// The half `fling` itself does not do: a registered list is advanced + /// by the frame loop's own driver, and unregisters itself when the + /// fling settles. Written against `UiData::tick_animations` rather + /// than `tick_fling` because the defect it pins is exactly the gap + /// between the two -- a fling with a correct velocity that nothing + /// ever advanced, which is what a finger fling did on the phone. + #[test] + fn a_registered_fling_is_driven_by_tick_animations_and_then_unregisters() { + let mut rsc = TestRsc { + ui: UiData::default(), + }; + let (list_weak, root, mut render) = build_flingable_list(&mut rsc); + let before = rsc + .ui + .widgets + .get(&list_weak) + .unwrap() + .anchor_position_display(); + + rsc.ui.widgets.get_mut(&list_weak).unwrap().fling(-8000.0); + rsc.ui.animate(list_weak.id()); + + let start = Instant::now(); + let mut animating = true; + let mut steps = 0; + while animating && steps < 600 { + animating = rsc + .ui + .tick_animations(start + std::time::Duration::from_millis(steps * 16)); + render.update(&root, &mut rsc); + steps += 1; + } + assert!(!animating, "the driver never stopped within 600 frames"); + assert!(steps > 1, "the fling settled without ever moving"); + assert!(!rsc.ui.widgets.get(&list_weak).unwrap().is_scrolling()); + assert_ne!( + before, + rsc.ui + .widgets + .get(&list_weak) + .unwrap() + .anchor_position_display(), + "the list is where it started -- the fling was registered but never applied" + ); + // Nothing left registered, so the next frame costs nothing: the + // path out of `animate` is the `false` answer, not a caller + // remembering to remove it. + assert!(!rsc.ui.tick_animations(start)); + } + #[test] fn fling_distance_is_positive_toward_the_end() { let mut rsc = TestRsc { diff --git a/iris/src/widget/trait_fns.rs b/iris/src/widget/trait_fns.rs index 4e98630..0d3a98a 100644 --- a/iris/src/widget/trait_fns.rs +++ b/iris/src/widget/trait_fns.rs @@ -1,6 +1,5 @@ use super::*; use crate::prelude::*; -use std::time::Instant; // these methods should "not require any context" (require unit) because they're in core widget_trait! { @@ -111,7 +110,7 @@ widget_trait! { let id = ctx.widget.id(); let (sense, pos) = (ctx.data.sense, ctx.data.cursor.pos); ctx.widget(rsc) - .drag(ctx.data.render, id, sense, pos, Instant::now()); + .drag(ctx.data.render, id, sense, pos, ctx.data.cursor.time); }, ) .add(state) diff --git a/iris/transcript-ui/src/lib.rs b/iris/transcript-ui/src/lib.rs index ece235e..3650532 100644 --- a/iris/transcript-ui/src/lib.rs +++ b/iris/transcript-ui/src/lib.rs @@ -52,7 +52,7 @@ pub mod tool; use client_core::transcript_fold::TranscriptRow as FoldedRow; use iris::prelude::*; use selection::Selection; -use std::{cell::RefCell, rc::Rc, time::Instant}; +use std::{cell::RefCell, rc::Rc}; pub struct TranscriptScreen { /// The transcript's own `List` -- exposed so a caller can read @@ -412,7 +412,7 @@ where row, ctx.data.cursor.pos, ctx.data.sense, - Instant::now(), + ctx.data.cursor.time, ctx.data.render, ); }, diff --git a/iris/transcript-ui/src/row.rs b/iris/transcript-ui/src/row.rs index b546222..764ff07 100644 --- a/iris/transcript-ui/src/row.rs +++ b/iris/transcript-ui/src/row.rs @@ -29,7 +29,7 @@ use crate::tool::ToolRow; use client_core::markdown_blocks::{Block, BlockKind, common_prefix, split_blocks}; use client_core::transcript_fold::{QuestionCard, TranscriptItem, TranscriptRow as FoldedRow}; use iris::prelude::*; -use std::{cell::RefCell, rc::Rc, time::Instant}; +use std::{cell::RefCell, rc::Rc}; /// The gap drawn between two markdown blocks of one message. A block used /// to be separated by the blank line `markdown::render_markdown` put in @@ -237,7 +237,7 @@ where Some((key, pos, size)), cursor, ctx.data.sense, - Instant::now(), + ctx.data.cursor.time, ctx.data.render, ); // A *tap*, decided by the same `DragArbiter` the pan and diff --git a/iris/transcript-ui/src/selection.rs b/iris/transcript-ui/src/selection.rs index d349323..69c0e73 100644 --- a/iris/transcript-ui/src/selection.rs +++ b/iris/transcript-ui/src/selection.rs @@ -294,7 +294,14 @@ impl Selection { // happened to end with the finger still moving, and never a // tap/long-press that never left `Undecided` -- exactly what // `DragGesture`'s `Some(v)` already encodes. - GestureOutcome::Released(Some(v)) => list(ui).fling(-v), + GestureOutcome::Released(Some(v)) => { + list(ui).fling(-v); + // The half that actually makes it move -- see + // `List::fling`'s doc. Without it the velocity is + // computed, stored, and never advanced by anything. + let id = list.id(); + ui.ui_mut().animate(id); + } // A tap is nobody's business here -- `row.rs` reads it from // the returned outcome and follows a link if one was under // the finger. diff --git a/iris/transcript-ui/src/tool.rs b/iris/transcript-ui/src/tool.rs index 4bb4630..5c5d58a 100644 --- a/iris/transcript-ui/src/tool.rs +++ b/iris/transcript-ui/src/tool.rs @@ -37,7 +37,7 @@ use crate::selection::Selection; use client_core::tool_summary::{ToolInput, parse_tool_input}; use client_core::transcript_fold::{ToolState, TranscriptItem}; use iris::prelude::*; -use std::{cell::Cell, cell::RefCell, collections::HashMap, rc::Rc, time::Instant}; +use std::{cell::Cell, cell::RefCell, collections::HashMap, rc::Rc}; /// A card's own fill: Surface 0, what Material's filled `Card` resolves to /// under `Theme.kt`'s scheme. One step *above* the page, so a card reads @@ -202,7 +202,7 @@ fn on_tap( None, ctx.data.cursor.pos, ctx.data.sense, - Instant::now(), + ctx.data.cursor.time, ctx.data.render, ); if outcome == GestureOutcome::Tapped {