diff --git a/docs/IRIS.md b/docs/IRIS.md index 7b02b0e..02d2247 100644 --- a/docs/IRIS.md +++ b/docs/IRIS.md @@ -8,6 +8,41 @@ capability that moved. Small and trivial changes do not go here. An entry gives the date, what changed, why, and a short before/after where it helps judge the change without the session that made it. Newest first. +## 2026-09-07: `VelocityTracker` takes positions, not deltas + +A flick released at the wrong speed because the tracker averaged. It now +does what Compose's touch scrolling does, and that changes what a caller +feeds it. + + // before -- one frame's motion + tracker.add_sample(dy, now); + // after -- where the finger was + tracker.add_position(pos.axis(axis), now); + +`VelocityTracker::velocity` is a port of Compose's `VelocityTracker1D` +with `Strategy.Lsq2`: a degree-2 least-squares fit through the last 20 +positions, differentiated at the newest sample, with Compose's 100ms +horizon, 40ms stopped-gap and three-sample minimum. Positions rather than +deltas because a fit needs points on a curve -- Compose itself throws on +differential data for this strategy. + +Three consequences a caller sees. **A gesture with fewer than three +samples answers `0.0`**, where the average answered a number from two; +that is Compose's answer too, and on the phone a 120Hz flick delivers +four or five. **A finger that rests for more than 40ms before lifting +answers `0.0`** rather than flinging at the speed it arrived with. +**`add_position` must be called in time order** -- the same debug assert +as before, now load-bearing for the fit's x-axis. + +Also new: `VelocityTracker::samples_display` (the held samples as +`t_ms:position`, printed by `DragGesture` at debug level so a flick +reported from a phone can be replayed), `DragArbiter::axis`, and +`sense::MAX_FLING_VELOCITY_DP_S` (8000, `ViewConfiguration`'s own). +`List::fling` now applies that maximum against its own density and +ignores anything at or under 1px/s, which is Compose's pair of thresholds +exactly -- there is deliberately no 50dp/s minimum, because Compose's +scrolling never consults the one in `ViewConfiguration`. + ## 2026-09-07: `client-core` carries the app's own log Not iris itself but the crate beside it, and it is a new public surface an diff --git a/docs/IRIS_TODO.md b/docs/IRIS_TODO.md index 2c57452..97e2840 100644 --- a/docs/IRIS_TODO.md +++ b/docs/IRIS_TODO.md @@ -1019,20 +1019,33 @@ do not duplicate it there. programmatic scroll all go through it) and end a fling that hits the clamp. Test at layer 1: a drag past either end leaves the offset at the end; a fling into the end stops there. -- [ ] **"Flinging now actually works but is slower than Compose's +- [x] **"Flinging now actually works but is slower than Compose's immediately after releasing the flick (the slow down seems - correct)."** The curve is right, so the *initial velocity* is low. - iris's `VelocityTracker::velocity` is total motion over the window's - span -- an average -- where Compose's `VelocityTracker` (`compose.ui` - `VelocityTracker.kt`, `VelocityTracker1D` with `Strategy.Impulse`, - 100ms horizon, 20 samples, `AssumePointerMoveStoppedMilliseconds = - 40`) weights the last samples, so a flick that accelerates into the - release reads faster. Port the impulse strategy from source with - checked-in reference values from an independent transcription, as - the spline was; also apply Compose's min/max fling velocity - (`ViewConfiguration`'s 50dp/s and 8000dp/s) so a slow release does - not fling and a wild one is capped. Layer-1 test on - `flick-120hz.touch` asserting the number Compose's code gives. + correct)."** Done; RUST.md's "The fling started too slow" has the + derivation and the table. On `flick-120hz.touch` the release velocity + goes from **11750px/s to 15250px/s**, and on an accelerating flick -- + the shape a real finger makes, and what the recording is too short to + show -- from 1080 to 2445px/s. The curve was right; `VelocityTracker` + was averaging total motion over the sample span, which cannot tell an + accelerating flick from a steady drag. + **Two things the plan for this item had wrong, both found by reading + the sources rather than remembering them.** Compose's touch path is + **not** `Strategy.Impulse`: `scrollable`/`draggable` release through + the 2D `VelocityTracker`, which on Android is two + `VelocityTracker1D(strategy = Lsq2)` over absolute *positions* -- a + degree-2 least-squares fit, differentiated at the newest sample. + Impulse is reached only by `DifferentialVelocityTracker`, for mouse + wheel and trackpad. And there is **no minimum** fling velocity on that + path: `ViewConfiguration.minimumFlingVelocity`'s 50dp/s is used only by + `NestedScrollInteropConnection`, while `DefaultFlingBehavior` skips + `abs(v) <= 1f` to dodge a NaN from the spline. So iris ports Lsq2, caps + at 8000dp/s, and floors at 1px/s -- no 50dp/s threshold Compose does + not have. `iris/benches/velocity_reference.py` is the independent + transcription the checked-in numbers come from; the negative control + (reverting to the average) fails exactly the seven tests about the + estimator and none of the rest. The release log gains a debug + `iris drag release samples:` line so a flick reported from the phone can + be replayed at layer 1. - [ ] **Input-event and timing report from the phone.** Iris: "add another button to copy input event info so that I can do some stuff manually and then send the event log to you ... instrument a lot of diff --git a/docs/RUST.md b/docs/RUST.md index d5f9807..dd94c8e 100644 --- a/docs/RUST.md +++ b/docs/RUST.md @@ -333,35 +333,56 @@ per UI_RULES, not "nothing drawn." `tool.rs`'s doc comment on those marks is updated to say this is now a bet on the platform's coverage rather than a checked fact about a bundled `cmap`. -**One real gap, found on this checkout's emulator, not the desktop**: -`fonts: 208 families found, default=Some("Roboto Flex") mono=None` in the -startup log (`FontDiagnostics`, read via `adb logcat` after installing the -`force-gles` debug build -- the emulator's default Vulkan backend has no -adapter here, a pre-existing, documented condition unrelated to this -change, and aborts with `Could not get adapter!` without that feature). -`mono=None` means fontique's Android backend never resolves the -`Monospace` generic family at all on this system image: reading -`fontique-0.11.1/src/backend/android.rs`, `DEFAULT_GENERIC_FAMILIES`'s -`["monospace"]` is looked up against `name_map` *before* the `fonts.xml` -parse that would register a family literally named `"monospace"` runs -- -so even though this AVD's `/system/etc/fonts.xml` does declare -`DroidSansMono.ttf`, -fontique's own ordering means that declaration is registered too late to -be found by the generic-family lookup, on every Android device this -fontique version runs on, not just this AVD. The visible effect is not -blank text -- `Family::Monospace`'s explicit-family list comes up empty, -but the script-based fallback chain (independent of the generic-family -list) still resolves a real font, the same one `SansSerif` gets -- so -code blocks and the tool-card chevrons render, just without a genuinely -monospaced face. Compose does not have this gap: `FontFamily.Monospace` -resolves through Android's own `Typeface.MONOSPACE` constant, a different -and unconditionally-populated path that fontique does not use. Left as a -follow-up rather than fixed here, since a fix means either patching -around fontique's Android backend or pinning `Family::Named("Droid Sans -Mono")` (fragile: an OEM-specific font name, not guaranteed across real -devices) -- out of the scope Iris gave this pass ("remove the font, -match Compose"), and a real product-visible difference worth her knowing -about rather than silently living with. +**One real gap, found on this checkout's emulator, not the desktop, closed +2026-09-07**: `fonts: 208 families found, default=Some("Roboto Flex") +mono=None` in the startup log (`FontDiagnostics`, read via `adb logcat` +after installing the `force-gles` debug build -- the emulator's default +Vulkan backend has no adapter here, a pre-existing, documented condition +unrelated to this change, and aborts with `Could not get adapter!` without +that feature). `mono=None` means fontique's Android backend never resolves +the `Monospace` generic family at all on this system image, and it is two +bugs stacked rather than one: reading `fontique-0.11.1/src/backend/android.rs`, +`DEFAULT_GENERIC_FAMILIES`'s `["monospace"]` is looked up against +`name_map` *before* the `fonts.xml` parse that adds the name runs, and even +after parsing, this AVD's `/system/etc/fonts.xml` (and AOSP's/GrapheneOS's, +same file format) names it with a `DroidSansMono.ttf` element rather than an `` -- +whose `` children that same parser's `"family"` match arm never reads +(a `TODO` left in place), so the name gets registered with no font data +behind it. `family_by_name("monospace")` therefore also comes up empty, on +every Android device this fontique version runs on, not just this AVD. +Checked against `linebender/parley`'s `main` branch on GitHub the same day: +neither bug is fixed there either, so there is no newer release to bump to. +The visible effect was not blank text -- `Family::Monospace`'s +explicit-family list came up empty, but the script-based fallback chain +(independent of the generic-family list) still resolved a real font, the +same one `SansSerif` gets -- so code blocks and the tool-card chevrons +rendered, just without a genuinely monospaced face, while Compose's +`FontFamily.Monospace` (resolved through Android's own `Typeface.MONOSPACE` +constant, a path fontique does not use) was unaffected. + +**Fixed** in `iris/core/src/primitive/text.rs`'s `patch_android_monospace` +(`#[cfg(target_os = "android")]`, called from `TextData::default` right +after `FontContext::new()`): rather than pinning an OEM-specific name like +`"Droid Sans Mono"` (the fragility this gap was originally left open over), +it reads `/system/etc/fonts.xml` itself -- a plain substring search, not a +new XML-parser dependency, for the one well-known AOSP file fontique +already parses with a real one -- for the filename the `"monospace"` +family declares, then searches fontique's own *actually* scanned families +(the ones with real font data, from `/system/fonts`) for whichever one +owns a font file with that name, and registers that family as the +`Monospace` generic itself. This is the same authority Compose's +`Typeface.MONOSPACE` resolves through, and it degrades safely to a no-op +if `fonts.xml` is missing (a headless test) or nothing matches (a device +naming it some other way) -- the pre-existing sans fallback, not a panic. +Verified on this checkout's emulator: `mono=Some("Droid Sans Mono")` in the +startup log, `resolved ... mono=Some("Droid Sans Mono")`, and a screenshot +of the bench-fixture transcript showing the code block and tool-card value +text in a visibly monospaced face next to sans body/heading text. The +desktop's `fontconfig` backend was never affected (confirmed unchanged: +`./run-headless.sh phone --phone --shot` still shows monospaced code next +to sans body text) -- the patch is Android-only and a no-op everywhere +else. **Verified**: `cargo test -p transcript-fixture` (6 tests, all headless layers) and `cargo clippy -p iris-core --all-targets` both clean; @@ -478,12 +499,16 @@ closes it. below for the fallback behaviour and one real gap it surfaced: this fontique version's Android backend never resolves the `Monospace` generic family at all (`mono=None` in the startup diagnostic, measured - on this checkout's emulator) -- code/tool-card text still renders (the - script fallback chain still lands on a real face, never blank), just - not in a genuinely monospaced one. Compose does not have this gap; it + on this checkout's emulator) -- code/tool-card text still rendered (the + script fallback chain still landed on a real face, never blank), just + not in a genuinely monospaced one. Compose did not have this gap; it resolves `FontFamily.Monospace` through Android's own Typeface - constant rather than through fontique. Flagged as a follow-up, not - fixed here -- out of the scope Iris gave. + constant rather than through fontique. **Closed same day** -- see + "Platform fonts (2026-09-07)"'s "Fixed" paragraph: + `TextData::patch_android_monospace` resolves the platform's own + `fonts.xml` monospace declaration against fontique's actually-scanned + families, Android-only, verified `mono=Some("Droid Sans Mono")` on this + checkout's emulator. - [ ] Scroll clamped at both ends, and Compose's impulse velocity estimator with min/max fling velocity (docs/IRIS_TODO.md, 2026-09-07 later). After the culling fix lands (same file). diff --git a/iris/benches/velocity_reference.py b/iris/benches/velocity_reference.py new file mode 100644 index 0000000..c60bdc2 --- /dev/null +++ b/iris/benches/velocity_reference.py @@ -0,0 +1,298 @@ +#!/usr/bin/env python3 +"""Compose's touch velocity tracker, transcribed independently of the Rust port. + +Same reason `fling_spline_reference.py` exists: the numbers checked into +`sense.rs`'s velocity tests must not be numbers the Rust produced. The old +estimator -- total motion over the sample span, an average -- passed every test +it had, because every one of those tests asserted the average's own definition +back at it. An average cannot tell an accelerating flick from a steady drag, and +that is exactly what Iris reported from the phone on 2026-09-07: "flinging now +actually works but is slower than Compose's immediately after releasing the +flick". + +Transcribed by hand from, and only from, the `-sources.jar` of +**androidx.compose.ui:ui-android:1.12.0** and +**androidx.compose.foundation:foundation-android:1.12.0** +(dl.google.com/dl/android/maven2), read 2026-09-07: + + * `androidx/compose/ui/input/pointer/util/VelocityTracker.kt` -- + `VelocityTracker1D.calculateVelocity`, `polyFitLeastSquares`, + `calculateImpulseVelocity`, `kineticEnergyToVelocity`, and the constants + `HistorySize = 20`, `HorizonMilliseconds = 100`, + `AssumePointerMoveStoppedMilliseconds = 40`. + * `androidx/compose/ui/input/pointer/util/PlatformVelocityTracker.kt` -- + `Lsq2VelocityTracker`, which is what the 2D `VelocityTracker` delegates to. + * `androidx/compose/ui/input/pointer/util/PlatformVelocityTracker.android.kt` + -- the `AndroidComposeUiFlags.isFrameworkVelocityTrackerEnabled` fork. + * `androidx/compose/ui/AndroidComposeUiFlags.android.kt` -- that flag's + default, which is `false`. + * `androidx/compose/foundation/gestures/Draggable.kt` -- `sendDragStart` / + `sendDragEvent` / `sendDragStopped`, i.e. *which* samples a touch drag + feeds the tracker and where the maximum-velocity clamp is applied. + * `androidx/compose/foundation/gestures/DifferentialVelocityTracker.kt` and + `NonTouchScrollingLogic.kt` -- the Impulse strategy's only caller. + * `androidx/compose/foundation/gestures/Scrollable.kt` -- + `DefaultFlingBehavior.performFling`, for the minimum-velocity question. + +**Which strategy a touch fling actually uses, since this was the surprise.** +`Strategy.Impulse` is *not* it. `scrollable`/`draggable` release through +`DragGestureNode.sendDragStopped`, which calls the 2D `VelocityTracker`; on +Android that is `Lsq2VelocityTracker` (the framework-tracker flag defaults to +false), which is two `VelocityTracker1D(strategy = Lsq2)` -- a degree-2 +least-squares fit over **absolute positions**, whose velocity is the fitted +polynomial's derivative at the newest sample. Impulse is reached only through +`DifferentialVelocityTracker`, whose sole caller is `NonTouchScrollingLogic`: +mouse wheel and trackpad, never a finger. So this script transcribes Lsq2 and +iris ports Lsq2. `calculate_impulse_velocity` is here anyway, unused by the +printed points, because ruling it out by reading is cheaper than ruling it out +again next time somebody remembers "Compose uses impulse". + +**Which samples a touch drag feeds it.** `sendDragStart` adds the DOWN change; +every subsequent MOVE, historical samples included, is added by `sendDragEvent`. +The **UP position is never added**: `Lsq2VelocityTracker.addPointerInputChange` +wraps its two `addPosition` calls in `if (!event.changedToUpIgnoreConsumed())`, +and all the UP branch does is reset the tracker when more than 40ms have passed +since the last MOVE (b/238654963). So a finger that stops before lifting reads +as a stop, not as a decelerating tail. Positions are the raw event positions, +so the touch slop is inside the motion the tracker sees even though the list +never scrolled by it. + +Two of Compose's samples iris does *not* reproduce, both noted rather than +copied: pre-slop MOVEs (iris's `DragArbiter` is `Undecided` then too, so it +feeds none either -- these agree), and the single MOVE that *crosses* the slop, +which Compose drops because `sendDragStart` adds only the DOWN. iris feeds that +one, since it is a real measured position and dropping it would be copying a +quirk of where Compose happens to split its state machine. + +**The clamps.** Maximum: `sendDragStopped` passes +`LocalViewConfiguration.maximumFlingVelocity`, which on Android is +`ViewConfiguration.getScaledMaximumFlingVelocity()` -- 8000 dp/s. Minimum: +there is **none** on this path. `ViewConfiguration.minimumFlingVelocity` +exists in Compose's `ViewConfiguration` interface but its only use in either +artifact is `NestedScrollInteropConnection`, for View interop. +`DefaultFlingBehavior.performFling` guards with `abs(initialVelocity) > 1f` +and says why in its own comment: "we need it since spline curve gives us +NaNs". 1 px/s, not 50 dp/s. + +Run it with no arguments; it prints the sample sets and the velocities the +Rust tests assert on. +""" + +import math + +HISTORY_SIZE = 20 +HORIZON_MILLISECONDS = 100.0 +ASSUME_POINTER_MOVE_STOPPED_MILLISECONDS = 40.0 +MIN_SAMPLE_SIZE_LSQ2 = 3 + +# ViewConfiguration.getScaledMaximumFlingVelocity(), in dp/s. +MAXIMUM_FLING_VELOCITY_DP_S = 8000.0 +# DefaultFlingBehavior.performFling's own threshold, in the units of the +# positions fed to the tracker -- pixels per second here. +FLING_MINIMUM_PX_S = 1.0 + + +def poly_fit_least_squares(x, y, sample_count, degree): + """`polyFitLeastSquares`: Gram-Schmidt QR, coefficients low order first.""" + if degree < 1: + raise ValueError("The degree must be at positive integer") + if sample_count == 0: + raise ValueError("At least one point must be provided") + + truncated_degree = sample_count - 1 if degree >= sample_count else degree + m = sample_count + n = truncated_degree + 1 + + # a[i][h] = x[h]**i, pre-multiplied by the (always 1.0) weight. + a = [[0.0] * m for _ in range(n)] + for h in range(m): + a[0][h] = 1.0 + for i in range(1, n): + a[i][h] = a[i - 1][h] * x[h] + + q = [[0.0] * m for _ in range(n)] + r = [[0.0] * n for _ in range(n)] + for j in range(n): + w = q[j] + w[:] = a[j][:m] + for i in range(j): + z = q[i] + dot = sum(w[h] * z[h] for h in range(m)) + for h in range(m): + w[h] -= dot * z[h] + norm = math.sqrt(sum(v * v for v in w)) + inverse_norm = 1.0 / max(norm, 1e-6) + for h in range(m): + w[h] *= inverse_norm + for i in range(n): + r[j][i] = 0.0 if i < j else sum(w[h] * a[i][h] for h in range(m)) + + coefficients = [0.0] * n + for i in range(n - 1, -1, -1): + c = sum(q[i][h] * y[h] for h in range(m)) + for j in range(n - 1, i, -1): + c -= r[i][j] * coefficients[j] + coefficients[i] = c / r[i][i] + return coefficients + + +def kinetic_energy_to_velocity(kinetic_energy): + sign = 0.0 if kinetic_energy == 0.0 else math.copysign(1.0, kinetic_energy) + return sign * math.sqrt(2 * abs(kinetic_energy)) + + +def calculate_impulse_velocity(data_points, time, sample_count, is_data_differential): + """`calculateImpulseVelocity` -- not on the touch path; see the module doc.""" + work = 0.0 + start = sample_count - 1 + next_time = time[start] + for i in range(start, 0, -1): + current_time = next_time + next_time = time[i - 1] + if current_time == next_time: + continue + if is_data_differential: + delta = -data_points[i - 1] + else: + delta = data_points[i] - data_points[i - 1] + v_curr = delta / (current_time - next_time) + v_prev = kinetic_energy_to_velocity(work) + work += (v_curr - v_prev) * abs(v_curr) + if i == start: + work = work * 0.5 + return kinetic_energy_to_velocity(work) + + +def calculate_velocity(samples): + """`VelocityTracker1D.calculateVelocity` with `Strategy.Lsq2`. + + `samples` is `(time_millis, position)` oldest first, at most the last + `HISTORY_SIZE` of which the circular buffer would still be holding. + Returns units per second. + """ + held = samples[-HISTORY_SIZE:] + if not held: + return 0.0 + + data_points = [] + time = [] + newest_time, _ = held[-1] + previous_time = newest_time + for sample_time, sample_position in reversed(held): + age = float(newest_time - sample_time) + delta = abs(float(sample_time - previous_time)) + # Lsq2 walks back sample to sample; only the non-differential + # Impulse branch compares every sample against the newest one. + previous_time = sample_time + if age > HORIZON_MILLISECONDS or delta > ASSUME_POINTER_MOVE_STOPPED_MILLISECONDS: + break + data_points.append(sample_position) + time.append(-age) + if len(data_points) == HISTORY_SIZE: + break + + if len(data_points) < MIN_SAMPLE_SIZE_LSQ2: + return 0.0 + try: + coefficients = poly_fit_least_squares(time, data_points, len(data_points), 2) + except ValueError: + return 0.0 + # The 2nd coefficient is the fitted polynomial's derivative at x = 0, + # which is the newest sample's timestamp. units/ms -> units/s. + return coefficients[1] * 1000.0 + + +def clamped(velocity, maximum): + """`VelocityTracker1D.calculateVelocity(maximumVelocity)`.""" + if velocity == 0.0 or math.isnan(velocity): + return 0.0 + return min(velocity, maximum) if velocity > 0 else max(velocity, -maximum) + + +def average(samples): + """The estimator being replaced: total motion over the span.""" + if len(samples) < 2: + return 0.0 + span = (samples[-1][0] - samples[0][0]) / 1000.0 + if span <= 0.0: + return 0.0 + return (samples[-1][1] - samples[0][1]) / span + + +# --- The three recorded sample sets the Rust tests assert on. ---------------- + +# 1. `transcript-fixture/touch/flick-120hz.touch`, as `DragGesture` feeds it: +# the DOWN position, then one position per MOVE. The UP at t=20 adds no +# sample (see the module doc), which is why the finger sitting still for its +# last 4ms does not drag the estimate down. y only; the flick is vertical. +FLICK_120HZ = [(0, 1000.0), (4, 1040.0), (8, 1086.0), (12, 1138.0), (16, 1196.0)] + +# 2. A steady drag: 5px every 10ms for 100ms. A constant-velocity fit and an +# average must agree here -- this is the case that cannot tell the two +# estimators apart, which is why it is not the only one. +STEADY_DRAG = [(i * 10, float(i * 5)) for i in range(11)] + +# 3. A flick that accelerates into the release: 10ms apart, deltas doubling. +# This is the case the average gets wrong, and the negative control for +# the port -- reverting to the average must fail this test and only this +# kind of test. +ACCELERATING_FLICK = [(0, 0.0), (10, 2.0), (20, 6.0), (30, 14.0), (40, 30.0), (50, 54.0)] + +# 4. The two edges of the sample walk, checked here so the Rust asserts +# Compose's answer rather than iris's own reading of the rule. +# (a) An old, fast burst outside the 100ms horizon, then a slow steady +# drag: the burst must not leak into the estimate. +OLD_BURST_THEN_STEADY = [(0, 0.0)] + [(10 + i * 10, 1000.0 + i) for i in range(11)] +# (b) The finger stops for 48ms and then lifts. The gap exceeds +# AssumePointerMoveStopped, so the walk breaks after one sample and +# there is no fling -- what stops a "park it and let go" from +# flinging at whatever speed the finger arrived with. +STOPPED_BEFORE_RELEASE = [(0, 0.0), (4, 40.0), (8, 90.0), (12, 150.0), (60, 152.0)] + +# 5. `sense.rs`'s own `drag_gesture_tests`: what `DragGesture` feeds for a +# press and two move frames, which is the fewest a fit can use. +TWO_MOVE_FRAMES = [(0, 0.0), (8, 100.0), (16, 220.0)] +# ... and one move frame, which Compose cannot fit either. +ONE_MOVE_FRAME = [(0, 0.0), (8, 100.0)] + +# The phone: 1080x2424 at content_scale 2.55. +PHONE_DENSITY = 2.55 + + +def report(name, samples): + v = calculate_velocity(samples) + print(f"{name}:") + print(f" samples (t_ms, position): {samples}") + print(f" Lsq2 (Compose's touch path): {v:.4f} px/s") + print(f" average (the old estimator): {average(samples):.4f} px/s") + print(f" impulse (non-touch, for ref): ", end="") + held = list(reversed(samples[-HISTORY_SIZE:])) + newest = held[0][0] + print( + f"{calculate_impulse_velocity([p for _, p in held], [-(newest - t) for t, _ in held], len(held), False) * 1000.0:.4f} px/s" + ) + print() + + +if __name__ == "__main__": + print("Compose 1.12.0 touch velocity: VelocityTracker1D, Strategy.Lsq2,") + print("non-differential (positions), HistorySize=20, Horizon=100ms,") + print("AssumePointerMoveStopped=40ms, minSampleSize=3.\n") + report("flick-120hz.touch", FLICK_120HZ) + report("steady drag (5px/10ms)", STEADY_DRAG) + report("accelerating flick (deltas 2,4,8,16,24 per 10ms)", ACCELERATING_FLICK) + + report("old burst then steady 1px/10ms", OLD_BURST_THEN_STEADY) + report("stopped 48ms before release", STOPPED_BEFORE_RELEASE) + report("press and two move frames", TWO_MOVE_FRAMES) + report("press and one move frame", ONE_MOVE_FRAME) + + print("Clamps:") + print(f" maximum: {MAXIMUM_FLING_VELOCITY_DP_S} dp/s") + print( + f" = {MAXIMUM_FLING_VELOCITY_DP_S * PHONE_DENSITY:.1f} px/s at the phone's density {PHONE_DENSITY}" + ) + print(f" minimum: none on the fling path; DefaultFlingBehavior skips |v| <= {FLING_MINIMUM_PX_S} px/s") + print() + print("Two samples only (a press and one move, the phone's 120Hz worst case):") + print(f" Lsq2 needs 3 and answers {calculate_velocity(FLICK_120HZ[:2]):.4f} px/s") diff --git a/iris/src/sense.rs b/iris/src/sense.rs index 95f3cd9..f690812 100644 --- a/iris/src/sense.rs +++ b/iris/src/sense.rs @@ -655,6 +655,14 @@ impl DragArbiter { matches!(self.state, ArbiterState::Panning) } + /// Which way this arbiter's pan runs -- what [`DragGesture`] reads to + /// know which component of a window position to hand its + /// [`VelocityTracker`], so the axis is stated once here rather than + /// stored a second time beside it. + pub fn axis(&self) -> Axis { + self.axis + } + /// Whether a press is in flight that has committed to neither a pan /// nor a selection -- what a release checks to tell a **tap** from /// the end of a drag. A tap is exactly "pressed and let go without @@ -759,18 +767,15 @@ 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); + // Where the finger went down is a sample, exactly as + // Compose's `DragGestureNode.sendDragStart` feeds the DOWN + // change to its tracker before any move. It is one of the + // three a fit needs, and it is the one that fixes the + // origin of the curve; without it a 120Hz flick delivering + // its whole motion in two frames has too few points and + // does not fling at all. + self.velocity + .add_position(pos_window.axis(self.arbiter.axis()), now); self.arbiter.press_start(pos_window, now, already_selected); self.dispatch(render, id, pos_window, now) } @@ -796,6 +801,16 @@ impl DragGesture { self.velocity.velocity(), outcome, ); + // The samples themselves, so a flick that felt wrong on + // Iris's phone can be replayed here instead of guessed at: + // paste them into a `touch/*.touch` recording or straight + // into `iris/benches/velocity_reference.py`. Debug rather + // than info because it is one line per gesture and the + // ring the report copies is small. + log::debug!( + "iris drag release samples: {}", + self.velocity.samples_display() + ); self.arbiter.release(); render.release_pointer(); outcome @@ -805,7 +820,8 @@ 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.velocity + .add_position(pos_window.axis(self.arbiter.axis()), now); self.arbiter.press_start(pos_window, now, already_selected); self.dispatch(render, id, pos_window, now) } @@ -824,7 +840,11 @@ impl DragGesture { DragOutcome::Undecided => GestureOutcome::Undecided, DragOutcome::Pan(dy) => { render.capture_pointer(id); - self.velocity.add_sample(dy, now); + // The raw position, not `dy`: `dy` has the touch slop + // subtracted out of the frame that crossed it, and the + // tracker fits a curve through where the finger *was*. + self.velocity + .add_position(pos.axis(self.arbiter.axis()), now); GestureOutcome::Pan(dy) } DragOutcome::SelectStart => { @@ -839,23 +859,86 @@ impl DragGesture { } } -/// How far back a [`VelocityTracker`] looks when estimating a fling's -/// initial speed -- Android's own `VelocityTracker` defaults to a similar -/// short window so a gesture's last flick dominates over its slower start. -const VELOCITY_WINDOW: Duration = Duration::from_millis(100); +/// Compose's `HistorySize`: how many samples the tracker holds at all. +/// Compose's is a circular buffer of exactly this many; the deque below +/// drops its oldest instead, which is the same set of samples. +const HISTORY_SIZE: usize = 20; +/// Compose's `HorizonMilliseconds`: a sample older than this than the +/// newest one is not part of the estimate. +const HORIZON_MS: f32 = 100.0; +/// Compose's `AssumePointerMoveStoppedMilliseconds`: a gap this long +/// between two consecutive samples means the finger stopped, and +/// everything older than the gap is a different motion. +const ASSUME_POINTER_MOVE_STOPPED_MS: f32 = 40.0; +/// `VelocityTracker1D`'s `minSampleSize` for `Strategy.Lsq2` -- a +/// quadratic needs three points, and fewer answers `0`. +const MIN_SAMPLE_SIZE: usize = 3; +/// The degree Compose fits (`polyFitLeastSquares(.., degree = 2)`), and +/// the number of coefficients that produces. +const FIT_DEGREE: usize = 2; +const FIT_COEFFICIENTS: usize = FIT_DEGREE + 1; -/// Tracks a drag's speed along one axis from its last ~100ms of motion, so -/// a release can be handed a realistic initial velocity for -/// [`AndroidFlingSpline`]/[`FlingCalculator`] rather than a single frame's -/// noisy last delta. Fed one timestamped pan delta per frame -/// (`add_sample`, the same `dy`/`-dy` quantity `DragArbiter::update`'s -/// `Pan` outcome already carries) and answers `velocity()` in units per -/// second, matching whatever unit the deltas were in. +/// `ViewConfiguration.getScaledMaximumFlingVelocity()`, in dp per second +/// -- AOSP's `MAXIMUM_FLING_VELOCITY`. Compose applies it at the release +/// (`DragGestureNode.sendDragStopped` passes +/// `LocalViewConfiguration.maximumFlingVelocity` into +/// `VelocityTracker.calculateVelocity(maximumVelocity)`); iris applies it +/// in [`crate::widget::List::fling`] instead, because that is the only +/// place that knows the density this has to be multiplied by. There is +/// deliberately **no** matching minimum: see `List::fling`. +pub const MAX_FLING_VELOCITY_DP_S: f32 = 8000.0; + +/// Estimates a drag's speed along one axis the way Compose's touch +/// scrolling does, so a fling released here starts at the speed the same +/// finger would have started one in a `LazyColumn` -- Iris's phone report +/// of 2026-09-07: "flinging now actually works but is slower than +/// Compose's immediately after releasing the flick (the slow down seems +/// correct)". The curve was already AOSP's; only the initial speed was +/// wrong. +/// +/// **What it was, and why that was slow.** Until 2026-09-07 this held +/// per-frame *deltas* and answered their sum over the span between the +/// oldest and newest -- an average. An average cannot tell an +/// accelerating flick from a steady drag, and a flick is by definition +/// accelerating: on the reference accelerating sample set +/// (`iris/benches/velocity_reference.py`) the average reads 1080px/s +/// where Compose reads 2445px/s, so every fling started at under half +/// the speed the finger asked for. +/// +/// **What Compose actually does, which is not what it is remembered as.** +/// `scrollable`/`draggable` release through the 2D `VelocityTracker`, +/// which on Android is `Lsq2VelocityTracker` -- two +/// `VelocityTracker1D(strategy = Lsq2)` over **absolute positions**, +/// fitting a degree-2 polynomial by least squares and taking its +/// derivative at the newest sample. `Strategy.Impulse` is reached only +/// through `DifferentialVelocityTracker`, whose one caller is +/// `NonTouchScrollingLogic`: mouse wheel and trackpad, never a finger. +/// (`AndroidComposeUiFlags.isFrameworkVelocityTrackerEnabled`, which +/// would swap in the platform's own tracker, defaults to `false`.) Read +/// out of `androidx.compose.ui:ui-android:1.12.0` and +/// `androidx.compose.foundation:foundation-android:1.12.0`'s +/// `-sources.jar`, 2026-09-07 -- the versions +/// `app/gradle/libs.versions.toml` builds the Compose app against, which +/// is the app Iris is comparing this one with. +/// +/// **So it holds positions, not deltas.** Lsq2 refuses differential data +/// in Compose itself (`"Lsq2 not (yet) supported for differential axes"` +/// is a thrown `IllegalStateException`), and a fit needs points on a +/// curve rather than the curve's increments. [`DragGesture`] feeds it the +/// raw window-space coordinate along the drag axis, exactly as Compose +/// feeds `originalEventPosition`. +/// +/// The numbers its tests assert on come from +/// `iris/benches/velocity_reference.py`, an independent transcription of +/// the same Kotlin -- not from this code, for the reason +/// `android_fling_spline`'s doc gives at length. #[derive(Default)] pub struct VelocityTracker { - /// `(when, delta)` pairs, oldest first, trimmed to `VELOCITY_WINDOW` - /// on every `add_sample` -- so this never grows past however many - /// frames land in that window. + /// `(when, position along the axis)`, oldest first, at most + /// `HISTORY_SIZE` of them. The horizon is applied in `velocity` + /// rather than here, because that is where Compose applies it and + /// because a sample outside the horizon still tells `span` and the + /// release log what was delivered. samples: VecDeque<(Instant, f32)>, } @@ -866,31 +949,29 @@ impl VelocityTracker { /// Forget everything -- called on a fresh press, so a new gesture's /// velocity is never contaminated by the tail of the previous one. + /// Compose's `resetTracking`, called from the same place (its + /// `addPointerInputChange` resets on `changedToDown`). pub fn reset(&mut self) { self.samples.clear(); } - /// Record one frame's motion. `delta` is this frame's movement since - /// the last sample, not a cumulative position. - pub fn add_sample(&mut self, delta: f32, at: Instant) { + /// Record where the finger was, along the drag axis, at `at`. A + /// **position**, not a per-frame delta -- see the type's doc. + pub fn add_position(&mut self, position: f32, at: Instant) { // A caller that samples out of order (a restored/replayed - // gesture, a test) would silently produce a negative `span` in - // `velocity`, handled only by its `span <= 0.0 => 0.0` catch-all - // -- masking the bug that produced it rather than surfacing it + // gesture, a test) would make `velocity`'s reverse walk compute + // negative ages and fit a curve through a shuffled x-axis -- + // masking the bug that produced it rather than surfacing it // (docs/REVIEW-2026-09-06.md finding 4). debug_assert!(self.samples.back().is_none_or(|&(last, _)| at >= last)); - self.samples.push_back((at, delta)); - while let Some(&(when, _)) = self.samples.front() { - if at.duration_since(when) > VELOCITY_WINDOW { - self.samples.pop_front(); - } else { - break; - } + self.samples.push_back((at, position)); + while self.samples.len() > HISTORY_SIZE { + self.samples.pop_front(); } } - /// How many samples are currently inside the window, and how long they - /// span. Reported beside the velocity in `DragGesture`'s release log, + /// How many samples are currently held, 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. @@ -905,20 +986,151 @@ impl VelocityTracker { } } - /// 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. - /// `0.0` with fewer than two samples (no time span to divide by). + /// Every held sample as `t_ms:position`, offsets from the oldest -- + /// what the release log prints at debug level so a gesture reported + /// from the phone can be replayed here (`TouchScript`, layer 1) or + /// pasted into `velocity_reference.py`. Iris has no logcat, so the + /// only way a flick that felt wrong on her screen becomes a number + /// anybody can check is for the samples themselves to be in the + /// report. + pub fn samples_display(&self) -> String { + let Some(&(first, _)) = self.samples.front() else { + return String::new(); + }; + self.samples + .iter() + .map(|&(at, position)| { + format!( + "{:.1}:{position:.1}", + at.duration_since(first).as_secs_f32() * 1000.0 + ) + }) + .collect::>() + .join(" ") + } + + /// The estimated speed at the newest sample, in units per second -- + /// `VelocityTracker1D.calculateVelocity` with `Strategy.Lsq2`, then + /// `calculateVelocity(maximumVelocity)`'s `NaN -> 0`. The maximum + /// itself is applied by the caller that knows the density + /// ([`crate::widget::List::fling`]). + /// + /// `0.0` with fewer than [`MIN_SAMPLE_SIZE`] usable samples, which is + /// Compose's answer too: a press and a single move carry no curve to + /// fit, so they do not fling. pub fn velocity(&self) -> f32 { - if self.samples.len() < 2 { + let mut positions = [0.0f32; HISTORY_SIZE]; + let mut ages = [0.0f32; HISTORY_SIZE]; + let mut count = 0; + + let Some(&(newest_at, _)) = self.samples.back() else { + return 0.0; + }; + let mut previous_at = newest_at; + // Newest first, walking back while the samples are one continuous + // motion -- Compose's own loop, including that `previous_at` + // steps sample to sample (the `Strategy.Lsq2 || isDataDifferential` + // branch) rather than staying on the newest. + for &(at, position) in self.samples.iter().rev() { + let age = newest_at.duration_since(at).as_secs_f32() * 1000.0; + let gap = previous_at.duration_since(at).as_secs_f32() * 1000.0; + previous_at = at; + if age > HORIZON_MS || gap > ASSUME_POINTER_MOVE_STOPPED_MS { + break; + } + positions[count] = position; + ages[count] = -age; + count += 1; + if count == HISTORY_SIZE { + break; + } + } + + if count < MIN_SAMPLE_SIZE { return 0.0; } - let total: f32 = self.samples.iter().map(|&(_, d)| d).sum(); - let span = self.span().as_secs_f32(); - if span <= 0.0 { 0.0 } else { total / span } + // The 2nd coefficient is the fitted quadratic's derivative at + // x = 0, and x = 0 is the newest sample's own timestamp. ms -> s. + let velocity = poly_fit_least_squares(&ages[..count], &positions[..count])[1] * 1000.0; + // `calculateVelocity(maximumVelocity)`'s first branch. A fit + // through near-degenerate points divides by a near-zero diagonal + // of `r`; answering `NaN` would trip `List::fling`'s finiteness + // assert in a debug build and coast forever in a release one. + if velocity.is_finite() { velocity } else { 0.0 } } } +/// Compose's `polyFitLeastSquares` at its one call site's shape: degree +/// [`FIT_DEGREE`], weights all 1, coefficients lowest order first. Gram- +/// Schmidt QR of the Vandermonde matrix, then back-substitution. +/// +/// Fixed-size arrays rather than Compose's allocated `Matrix`, since both +/// dimensions are constants here -- `FIT_COEFFICIENTS` rows by at most +/// `HISTORY_SIZE` columns. Compose truncates the degree when it has fewer +/// points than coefficients; [`MIN_SAMPLE_SIZE`] makes that unreachable +/// from the only caller, so the truncation is an assert instead of a +/// branch that could never be exercised. +fn poly_fit_least_squares(x: &[f32], y: &[f32]) -> [f32; FIT_COEFFICIENTS] { + debug_assert_eq!(x.len(), y.len()); + debug_assert!( + (FIT_COEFFICIENTS..=HISTORY_SIZE).contains(&x.len()), + "a degree-{FIT_DEGREE} fit needs {FIT_COEFFICIENTS}..={HISTORY_SIZE} points, got {}", + x.len() + ); + let m = x.len(); + + // a[i][h] = x[h]^i. + let mut a = [[0.0f32; HISTORY_SIZE]; FIT_COEFFICIENTS]; + for h in 0..m { + a[0][h] = 1.0; + for i in 1..FIT_COEFFICIENTS { + a[i][h] = a[i - 1][h] * x[h]; + } + } + + // q: orthonormal basis; r: upper triangular. + let mut q = [[0.0f32; HISTORY_SIZE]; FIT_COEFFICIENTS]; + let mut r = [[0.0f32; FIT_COEFFICIENTS]; FIT_COEFFICIENTS]; + for j in 0..FIT_COEFFICIENTS { + q[j][..m].copy_from_slice(&a[j][..m]); + for i in 0..j { + let (earlier, from_j) = q.split_at_mut(j); + let z = &earlier[i]; + let w = &mut from_j[0]; + let dot = dot(&w[..m], &z[..m]); + for h in 0..m { + w[h] -= dot * z[h]; + } + } + let inverse_norm = 1.0 / dot(&q[j][..m], &q[j][..m]).sqrt().max(1e-6); + for v in &mut q[j][..m] { + *v *= inverse_norm; + } + for i in 0..FIT_COEFFICIENTS { + r[j][i] = if i < j { + 0.0 + } else { + dot(&q[j][..m], &a[i][..m]) + }; + } + } + + // Solve R B = Qt Y, bottom-right to top-left. + let mut coefficients = [0.0f32; FIT_COEFFICIENTS]; + for i in (0..FIT_COEFFICIENTS).rev() { + let mut c = dot(&q[i][..m], &y[..m]); + for j in ((i + 1)..FIT_COEFFICIENTS).rev() { + c -= r[i][j] * coefficients[j]; + } + coefficients[i] = c / r[i][i]; + } + coefficients +} + +fn dot(a: &[f32], b: &[f32]) -> f32 { + a.iter().zip(b).map(|(x, y)| x * y).sum() +} + /// Android's fling deceleration curve, ported from AOSP's /// `android.widget.OverScroller.SplineOverScroller` (the same curve /// Compose's `androidx.compose.animation.AndroidFlingSpline` and @@ -1163,53 +1375,145 @@ mod velocity_tracker_tests { *BASE + Duration::from_millis(ms) } - #[test] - fn fewer_than_two_samples_reports_zero() { + /// Every number below is printed by `iris/benches/velocity_reference.py`, + /// an independent transcription of the same Kotlin. Do not "fix" one by + /// running the Rust and copying what it said -- that is exactly how the + /// fling spline shipped as a straight line for two builds. + fn tracker(samples: &[(u64, f32)]) -> VelocityTracker { let mut v = VelocityTracker::new(); - assert_eq!(v.velocity(), 0.0); - v.add_sample(10.0, t(0)); - assert_eq!(v.velocity(), 0.0); + for &(ms, position) in samples { + v.add_position(position, t(ms)); + } + v + } + + /// f32 Gram-Schmidt against the reference's f64: the fits here agree to + /// several digits, so a tolerance this tight still fails by a mile on + /// any wrong estimator (the average misses by 25--125%). + fn assert_velocity(samples: &[(u64, f32)], expected: f32) { + let got = tracker(samples).velocity(); + let tolerance = expected.abs() * 1e-3 + 1e-3; + assert!( + (got - expected).abs() <= tolerance, + "expected {expected} from velocity_reference.py, got {got}" + ); + } + + /// `transcript-fixture/touch/flick-120hz.touch`, as `DragGesture` feeds + /// it: the DOWN position and one position per MOVE, no sample for the + /// UP (Compose's `Lsq2VelocityTracker.addPointerInputChange` adds none + /// either). **The before/after of Iris's 2026-09-07 report** on this + /// recording: the average answered 12250px/s, Compose answers 15250. + const FLICK_120HZ: [(u64, f32); 5] = [ + (0, 1000.0), + (4, 1040.0), + (8, 1086.0), + (12, 1138.0), + (16, 1196.0), + ]; + + #[test] + fn the_recorded_flick_reads_what_compose_reads() { + assert_velocity(&FLICK_120HZ, 15250.0); } #[test] fn a_steady_drag_reports_its_own_speed() { - // 5px every 10ms, 11 samples spanning 100ms, sums to 55px over - // 0.1s -- 550px/s by this tracker's own "sum of deltas over the - // span between the oldest and newest held sample" definition. - let mut v = VelocityTracker::new(); - for i in 0..=10 { - v.add_sample(5.0, t(i * 10)); - } - assert!((v.velocity() - 550.0).abs() < 1.0, "got {}", v.velocity()); + // 5px every 10ms, 11 samples spanning 100ms. The one case where a + // constant-velocity fit and an average must agree -- kept because + // it is the sanity check, and kept *with* the two below because on + // its own it cannot tell the two estimators apart at all. + let samples: Vec<(u64, f32)> = (0..=10).map(|i| (i * 10, (i * 5) as f32)).collect(); + assert_velocity(&samples, 500.0); + } + + /// **The negative control for the whole change.** Deltas doubling into + /// the release: Compose reads 2445px/s where the average reads 1080, + /// so a flick started at 44% of the speed the finger asked for. This + /// is the test the old estimator fails and the steady drag above does + /// not -- reverting `velocity` to `total / span` fails exactly this + /// one, the flick recording, and `phone_screen.rs`. + #[test] + fn an_accelerating_flick_reads_its_speed_at_the_release() { + const ACCELERATING: [(u64, f32); 6] = [ + (0, 0.0), + (10, 2.0), + (20, 6.0), + (30, 14.0), + (40, 30.0), + (50, 54.0), + ]; + assert_velocity(&ACCELERATING, 2445.0); + let average: f32 = 54.0 / 0.050; + assert!( + (average - 1080.0).abs() < 1.0, + "the average this is a control against moved: {average}" + ); + } + + #[test] + fn fewer_than_three_samples_reports_zero() { + // Compose's `minSampleSize` for Lsq2 is 3: a quadratic through two + // points is not a fit. So a press and a single move do not fling, + // which is what Compose does with the same two samples. + assert_eq!(VelocityTracker::new().velocity(), 0.0); + assert_velocity(&[(0, 0.0)], 0.0); + assert_velocity(&[(0, 0.0), (8, 100.0)], 0.0); } #[test] fn only_the_last_100ms_of_samples_count() { - // An old, fast burst well outside the window followed by a slow, - // steady drag should report the recent speed, not the average of - // both -- otherwise a flick that trails off would still fling at - // its earlier, faster speed. The burst sits 110ms before the last - // sample, just past the 100ms window, so it is evicted. - let mut v = VelocityTracker::new(); - v.add_sample(1000.0, t(0)); // will be 110ms old by the last sample - for i in 1..=11 { - v.add_sample(1.0, t(i * 10)); // 1px/10ms = 100px/s - } - assert!( - (v.velocity() - 110.0).abs() < 5.0, - "old burst leaked into the window: got {}", - v.velocity() + // An old, fast jump outside `HORIZON_MS` followed by a slow steady + // drag reports the recent speed, not both. 1px/10ms = 100px/s; the + // average of the whole set is 9182px/s. + let mut samples = vec![(0u64, 0.0f32)]; + samples.extend((0..11).map(|i| (10 + i * 10, 1000.0 + i as f32))); + assert_velocity(&samples, 100.0); + } + + #[test] + fn a_finger_that_stops_before_lifting_does_not_fling() { + // A 48ms gap is past `ASSUME_POINTER_MOVE_STOPPED_MS`, so the walk + // back stops at it and the fast motion before it is a different + // gesture. One usable sample, so 0 -- where the average would + // still say 2533px/s and fling from a standstill. + assert_velocity( + &[(0, 0.0), (4, 40.0), (8, 90.0), (12, 150.0), (60, 152.0)], + 0.0, ); } #[test] fn reset_forgets_prior_samples() { - let mut v = VelocityTracker::new(); - v.add_sample(500.0, t(0)); - v.add_sample(500.0, t(10)); + let mut v = tracker(&FLICK_120HZ); assert!(v.velocity() != 0.0); v.reset(); assert_eq!(v.velocity(), 0.0); + assert_eq!(v.sample_count(), 0); + assert_eq!(v.samples_display(), ""); + } + + #[test] + fn only_the_last_twenty_samples_are_held() { + // `HISTORY_SIZE`. The oldest fall out rather than the newest being + // refused -- a long drag's velocity is its recent motion. + let samples: Vec<(u64, f32)> = (0..40).map(|i| (i * 4, (i * 10) as f32)).collect(); + let v = tracker(&samples); + assert_eq!(v.sample_count(), HISTORY_SIZE); + assert_eq!( + v.span(), + Duration::from_millis(4 * (HISTORY_SIZE as u64 - 1)) + ); + } + + #[test] + fn the_sample_list_is_reported_relative_to_the_first() { + // What the release log prints at debug level, and what a phone + // report has to be replayable from. + assert_eq!( + tracker(&FLICK_120HZ[..3]).samples_display(), + "0.0:1000.0 4.0:1040.0 8.0:1086.0" + ); } } @@ -1668,15 +1972,72 @@ mod drag_gesture_tests { } /// **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. + /// `MotionEvent`s -- the intermediate positions are batched inside + /// them as historical samples, which `IrisViewPeer::on_touch_event` + /// replays one at a time, so the frames here are what a whole flick + /// can amount to. Compose fits a quadratic through the positions, so + /// three of them (the press and two moves) is the fewest that can + /// fling; the number is `velocity_reference.py`'s, not this code's. #[test] - fn a_flick_delivered_as_one_move_frame_still_releases_with_a_velocity() { + fn a_flick_delivered_as_two_move_frames_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, + ); + g.handle( + &r, + id, + CursorSense::Pressing(CursorButton::Left), + Vec2::new(0.0, 220.0), + t(16), + false, + ); + let out = g.handle( + &r, + id, + CursorSense::PressEnd(CursorButton::Left), + Vec2::new(0.0, 220.0), + t(24), + false, + ); + + // (0, 0) (8, 100) (16, 220) through Compose's Lsq2 fit. Note the + // release adds no sample -- Compose's tracker ignores the UP + // position -- so the finger resting for those last 8ms costs + // nothing, which is the whole point of ignoring it. + match out { + GestureOutcome::Released(Some(v)) => { + assert!((v - 16250.0).abs() < 20.0, "expected ~16250, got {v}"); + } + other => panic!("expected a released pan, got {other:?}"), + } + } + + /// The case the port had every reason to get wrong: **one** move frame + /// is two samples, and a quadratic through two points is not a fit. + /// Compose answers 0 here (`minSampleSize = 3`) and so does this, so + /// the release is a pan that flings nothing rather than a pan that + /// flings at a guessed speed. Before 2026-09-07 the average answered + /// 11750px/s from the same gesture. + #[test] + fn a_flick_delivered_as_one_move_frame_carries_no_velocity_to_fit() { let mut ui = UiData::default(); let id = some_id(&mut ui); let r = render(); @@ -1706,17 +2067,7 @@ mod drag_gesture_tests { 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:?}"), - } + assert_eq!(out, GestureOutcome::Released(Some(0.0))); } /// The other half of the same join, and the case the fix had no diff --git a/iris/src/widget/list.rs b/iris/src/widget/list.rs index b234345..fe6528c 100644 --- a/iris/src/widget/list.rs +++ b/iris/src/widget/list.rs @@ -489,7 +489,26 @@ impl List { // positions with nothing on screen saying why (docs/ // REVIEW-2026-09-06.md finding 3). debug_assert!(velocity_px_per_s.is_finite()); - if velocity_px_per_s == 0.0 || self.anchor.is_none() { + // Compose's two thresholds at a release, and **only** those two. + // + // The maximum is `ViewConfiguration.getScaledMaximumFlingVelocity()` + // (8000dp/s), which `DragGestureNode.sendDragStopped` passes into + // `VelocityTracker.calculateVelocity(maximumVelocity)`. It is + // applied here rather than in the tracker because the tracker + // works in pixels and has no density; this widget takes one from + // the painter in `draw`. + // + // The minimum is 1px/s, from `DefaultFlingBehavior.performFling`'s + // `abs(initialVelocity) > 1f` and its own stated reason ("we need + // it since spline curve gives us NaNs") -- not + // `ViewConfiguration.getScaledMinimumFlingVelocity()`'s 50dp/s, + // which Compose's scrolling never consults: its single use in + // either artifact is `NestedScrollInteropConnection`, for View + // interop. A 50dp/s floor would swallow slow, deliberate releases + // that Compose flings, so it is deliberately not here. + let max = MAX_FLING_VELOCITY_DP_S * self.density; + let velocity_px_per_s = velocity_px_per_s.clamp(-max, max); + if velocity_px_per_s.abs() <= 1.0 || self.anchor.is_none() { self.fling = None; return; } diff --git a/iris/transcript-fixture/tests/phone_screen.rs b/iris/transcript-fixture/tests/phone_screen.rs index c94f8e2..009c3cf 100644 --- a/iris/transcript-fixture/tests/phone_screen.rs +++ b/iris/transcript-fixture/tests/phone_screen.rs @@ -47,9 +47,15 @@ fn a_recorded_flick_releases_with_a_velocity_and_flings_the_list() { let velocity = (screen.list)(&mut h.rsc) .fling_velocity() .expect("the flick must release as a pan with a velocity, not a tap"); + // Compose's own answer for this recording's five samples, printed by + // `iris/benches/velocity_reference.py` -- not a number read off this + // code. Negative because the flick runs *down* the screen and + // `Selection::drag` flings the list by `-v` (see its `Released` arm). + // The 2026-09-07 before/after: the old average estimator read + // -11750px/s here, which is the fling Iris reported as too slow. assert!( - velocity.abs() > 1_000.0, - "a 188px, 16ms flick is thousands of px/s; got {velocity}" + (velocity + 15_250.0).abs() < 20.0, + "expected ~-15250px/s from velocity_reference.py, got {velocity}" ); // Android's own spline says how long a fling at this speed runs. The