iris: Scroll pans on a finger drag; a vertical drag in a focused field scrolls rather than selects
IRIS_TODO.md's "the composer has no touch-drag scroll". `Scroll::drag`
takes its pan from the same `sense::DragGesture` `List` is driven by --
arbitration, DRAG_SLOP, velocity and pointer capture all stay in sense.rs
and only what a committed pan *means* is decided per caller -- and
`WidgetLike::scrollable()` registers it beside the wheel handler it already
registered, so every scroll area pans on a finger with nothing added at the
call site. No fling: `Scroll` has no per-frame tick to animate one and the
areas it wraps are at most a screenful. `Scroll::amt()` exposes the pan
position.
`attr.rs`'s `on_press` treated an already-focused field as the plain
click_or_drag case, so every Pressing frame extended a selection. It now
applies the same DRAG_SLOP rule its unfocused branch already did: a press
past the slop vertically abandons its pending selection for the rest of the
gesture, so the scroll area around the field wins it. That is Android
EditText's own behaviour and it is what lets a swipe up over the composer
scroll instead of dragging a highlight through what you typed.
Also fixed, found doing it: `ActiveData::mask` stored the mask a widget
*set* rather than the one it was drawn *under*, and `redraw` feeds that
field back in as the inherited mask -- so a targeted redraw of any `Masked`
handed it its own mask and aborted on `set_mask`'s nested-mask assert. A
real abort on the emulator, `assertion failed: self.mask == MaskIdx::NONE`.
And the per-frame orphan guard from 76b1f99 is now a count comparison
(O(active widgets)); the O(primitives) walk only runs to build the failure
message, because running it per frame made a debug build on the emulator too
slow to finish a bench run at all.
Tests: four in scroll.rs (pan past the slop, a tap inside it, a horizontal
drag, the end clamp), `a_finger_drag_over_a_scroll_area_pans_it` in
sense_tests.rs driving the whole registration/dispatch/capture path (fails
with "got 0" without the new registration), and
`redrawing_a_masked_widget_does_not_nest_its_own_mask` in layout_tests.rs
(aborts on the pre-fix code).
The composer itself is deliberately still not `.scrollable()`: `Scroll`
measures against the window rather than its own offered box, so inside the
`MaxSize` capping it at six lines it pans the field out of the bar --
measured, reverted and written down in RUST.md and DECISIONS.md.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
76b1f99277
commit
fb6b459c2c
13 files changed
+608
-31
No files matched your search
@@ -5,6 +5,29 @@ they can be judged and reversed later. Detail lives in RUST.md (and IRIS.md
|
||||
for iris API changes); this file is only the summary. Newest first. Items
|
||||
marked **DEFERRED** are ones the agent chose not to decide alone.
|
||||
|
||||
## 2026-09-06 (stale-primitives and touch-scroll pass)
|
||||
|
||||
- **A vertical drag inside a focused composer now scrolls rather than
|
||||
selects.** Android's own `EditText` does this -- a vertical drag scrolls
|
||||
the field, and only a long press starts a selection -- so the platform
|
||||
decided it. What it costs: you can no longer drag straight down inside
|
||||
the composer to select several lines of what you typed; use a long press
|
||||
and then drag, or drag sideways. Say if that trade is wrong for you.
|
||||
- **`Scroll` gets a finger pan but no fling.** `List` flings; a scroll area
|
||||
does not, because it has no per-frame tick to animate one and the areas
|
||||
it wraps are at most a screenful (Android does not fling a six-line text
|
||||
box either). Easy to add later if a scroll area ever wraps something long.
|
||||
- **The composer still does not scroll its overflowed text**, though the
|
||||
mechanism it needs is now in place. Wrapping the field in `.scrollable()`
|
||||
was tried and reverted the same day: `Scroll` measures its content and
|
||||
container against the *window*, so inside the `MaxSize` that caps the
|
||||
composer at six lines the two are in different spaces and the field pans
|
||||
itself entirely out of the bar (measured on the emulator with 474
|
||||
characters in it -- the bar collapsed to its padding). Fixing that means
|
||||
`Scroll` measuring against its own offered box, which is a change to a
|
||||
widget the transcript and the bench shell both use, so it is its own
|
||||
piece of work rather than a rider on this one.
|
||||
|
||||
## 2026-09-06 (defect pass)
|
||||
|
||||
- **The keyboard-open diagnostics overlay is gone; the capture only
|
||||
|
||||
@@ -8,6 +8,48 @@ 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-06: `Scroll` pans on a finger drag, and a vertical drag in a focused text field no longer selects
|
||||
|
||||
Three related public changes, all in aid of IRIS_TODO.md's "the composer
|
||||
has no touch-drag scroll".
|
||||
|
||||
**`Scroll::drag(render, id, sense, pos_window, now)` is new**, and
|
||||
`WidgetLike::scrollable()` now registers it alongside the wheel handler it
|
||||
already registered -- so anything built with `.scrollable()` pans on a
|
||||
finger drag with no extra wiring at the call site. It goes through the same
|
||||
`sense::DragGesture` that `transcript-ui::Selection::drag` drives `List`
|
||||
with (arbitration, `DRAG_SLOP`, velocity, pointer capture), rather than a
|
||||
second copy of that widget's wiring: `DragGesture` owns the mechanics and
|
||||
each caller decides only what a committed pan *means*. `Scroll::amt()` is
|
||||
new too, the read-only pan position a test or a scroll indicator needs.
|
||||
|
||||
There is deliberately **no fling** on `Scroll`. Unlike `List` it has no
|
||||
per-frame tick to animate one with (`List::set_redraw_handle`/`tick_fling`),
|
||||
and the areas it wraps today are at most a screenful, where Android does not
|
||||
fling either. The released velocity is dropped rather than approximated.
|
||||
|
||||
**A vertical drag inside an already-focused `TextEdit` no longer extends a
|
||||
selection.** `iris::attr`'s `on_press` used to treat a focused field as the
|
||||
plain `click_or_drag` case -- every `Pressing` frame updated the selection.
|
||||
It now applies the same `DRAG_SLOP` rule the *unfocused* branch already
|
||||
applied: a press that moves past the slop vertically abandons its pending
|
||||
selection for the rest of the gesture, so the scroll area around the field
|
||||
gets the drag instead. Horizontal drag-to-select is unchanged, and a long
|
||||
press still starts a selection. This is Android's own `EditText` behaviour
|
||||
(a vertical drag scrolls; only a long press selects), and it is what makes
|
||||
"swipe up over the composer to scroll the transcript" work without dragging
|
||||
a highlight through the message you were typing.
|
||||
|
||||
**`UiRenderState::orphaned_primitives()` is new**, and `update` now
|
||||
`debug_assert!`s (debug builds only) that nothing is orphaned. An orphan is
|
||||
a primitive still bound for the GPU that no live `ActiveData` names -- a
|
||||
copy nothing can move, clip or free. That was the doubled `Compacted:` row
|
||||
on the phone; see the same date's commit `76b1f99` and docs/RUST.md. The
|
||||
per-frame guard is a count comparison (O(active widgets)); the walk that
|
||||
names the offenders only runs when the counts disagree, because the walk is
|
||||
O(primitives) and made a debug build on a phone too slow to finish a
|
||||
benchmark run.
|
||||
|
||||
## 2026-09-06: a tap on a text field always leaves a caret
|
||||
|
||||
`TextEditCtx::select` used to compare the tap position against the
|
||||
|
||||
+24
-8
@@ -201,12 +201,19 @@ agent takes them without colliding with that pass's `bench_client.rs`/
|
||||
a capped/scrollable height, bottom padding tied to the IME/nav-bar
|
||||
inset) -- structurally in place and unit-tested, but its own visual
|
||||
correctness cannot be screenshotted until text actually renders.
|
||||
- [ ] **The composer has no touch-drag scroll for overflowing text.** The
|
||||
2026-09-06 rebuild caps the field at ~6 lines and wraps it in
|
||||
`.scrollable()` for a wheel/trackpad scroll, but a real finger drag over
|
||||
text that has overflowed the cap does not scroll it -- `Scroll`'s touch
|
||||
handling is a follow-up, the same shape `List`'s own touch-drag pan
|
||||
needed before I3/I5.
|
||||
- [~] **The composer has no touch-drag scroll for overflowing text.**
|
||||
**The mechanism is in, the composer is not, 2026-09-06.** `Scroll::drag`
|
||||
takes its pan from the same `sense::DragGesture` `List` uses and
|
||||
`WidgetLike::scrollable()` registers it beside the wheel handler, so
|
||||
every scroll area in the codebase now pans on a finger (no fling -- see
|
||||
IRIS.md). A vertical drag inside a focused field no longer extends a
|
||||
selection, matching Android's `EditText`. But the composer field is
|
||||
**not** wrapped in `.scrollable()` -- the note above was describing
|
||||
intent, not the code -- and wrapping it was tried and reverted: `Scroll`
|
||||
measures against the window rather than its own offered box, so inside
|
||||
the `MaxSize` that caps the field at six lines it pans itself out of the
|
||||
bar entirely (emulator, 474 characters, the bar collapsed to its
|
||||
padding). docs/RUST.md's plan box has the numbers and the next step.
|
||||
|
||||
## From the phone, 2026-09-06, 11:39 (build delivered 02:07, commit 543f6d9)
|
||||
|
||||
@@ -247,8 +254,17 @@ agent ticks it here with the evidence.
|
||||
field the composer may still read as pixels or dp; a stale value from
|
||||
before the first `on_insets_changed`. Reproduce with the phone's
|
||||
screen size and density on the emulator before guessing.
|
||||
- [ ] **"Swiping still gets caught by the grey bar but keeps working
|
||||
after I go past it."** Not closeable from the emulator, annotated
|
||||
- [~] **"Swiping still gets caught by the grey bar but keeps working
|
||||
after I go past it."** Improved 2026-09-06 by the focused-field rule
|
||||
below, still needs her phone to close. `attr.rs`'s `on_press` treated an
|
||||
already-focused composer as the plain drag-to-select case, so a swipe
|
||||
starting inside it dragged a highlight through the typed text for the
|
||||
whole gesture; it now abandons that the moment the press passes
|
||||
`DRAG_SLOP` vertically (Android `EditText`'s own rule), which removes one
|
||||
of the two things that made the bar feel like it caught the swipe. The
|
||||
residual `DRAG_SLOP` measured from the boundary crossing, described
|
||||
below, is unchanged. Original note follows.
|
||||
Not closeable from the emulator, annotated
|
||||
2026-09-06 after the `DragGesture` merge. `attr.rs`'s `on_press` never
|
||||
calls `capture_pointer` and never consumes a `Pressing` frame past
|
||||
`DRAG_SLOP` (it just stops watching), so once the finger's *current*
|
||||
|
||||
+84
-14
@@ -226,20 +226,90 @@ closes it.
|
||||
at the top of `update` that it is empty between frames. Her build
|
||||
has both.
|
||||
|
||||
- [ ] **Stale primitives, the phone's half** — the `Compacted:` row
|
||||
drawn twice, overlapping, and once more below the composer
|
||||
(`docs/bench/iris-phone-v2-2026-09-06.md`). Everything the box
|
||||
above lists as ruled out is ruled out; what is left needs the
|
||||
phone, and it is worth noting the row is drawn *below the
|
||||
composer*, i.e. outside the list's own clip, which points at
|
||||
`List`'s row placement rather than at `Span::draw`'s two-phase
|
||||
placement the earlier passes assumed. The `iris surface:`/`iris
|
||||
insets:` log lines added 2026-09-06 are in the build she is
|
||||
holding, so her next `adb logcat | grep -i iris` says what the
|
||||
frame it happens on was doing.
|
||||
- [ ] **Composer touch-drag scroll** for overflowed text — now that
|
||||
dragging is a default-input `DragGesture`, `Scroll` should get its
|
||||
touch pan from the same mechanism `List` uses, not a copy.
|
||||
- [x] **Stale primitives, the phone's half** — **root-caused and fixed
|
||||
2026-09-06, commit `76b1f99`.** It was neither `Span::draw` nor
|
||||
`List`: `UiRenderState::draw_inner` *read* `needs_redraw` without
|
||||
consuming it, and used it to skip the whole `if let Some(active)`
|
||||
block — **including the `remove(id, false)` that frees a redrawn
|
||||
widget's previous primitives**. So a widget that was both already
|
||||
active and marked dirty, and was reached by an **ancestor's** draw
|
||||
rather than by `redraw_updates` picking it first (the order a
|
||||
`HashSet` makes arbitrary, which is why it was intermittent), wrote
|
||||
a second full set of primitives and then had `active.insert`
|
||||
overwrite the only handles that could ever have freed the first
|
||||
set. Those instances stay in the layer's buffer for the life of the
|
||||
process, with a leaked move slot and leaked mask refs, redrawn every
|
||||
frame at whatever region they last had — and `List` sets no mask, so
|
||||
a row measured at `GENEROUS_PADDING` leaves its ghost outside the
|
||||
list's own box, which is the copy below the composer.
|
||||
`Painter::draw_twice` (`List::place`'s measurement pass) reaches
|
||||
`draw_inner` twice for one id in one frame and so hits the same
|
||||
fault with no ancestor involved.
|
||||
**Fix**: consume the mark at the top of `draw_inner` — this call *is*
|
||||
the redraw it asked for — and free the old primitives on the dirty
|
||||
path too.
|
||||
**Why the earlier passes could not see it**: `replacing_the_last_row_
|
||||
many_times_does_not_leak_primitives` counts *widgets*, and the
|
||||
orphan's owner is very much alive; it is an earlier set of that same
|
||||
widget's primitives that is stranded.
|
||||
**Guard**: `UiRenderState::orphaned_primitives()` names every live
|
||||
instance no `ActiveData` owns, and `update` `debug_assert!`s it empty
|
||||
every frame in debug builds. The per-frame form is a *count*
|
||||
comparison (`primitive_counts_agree`, O(active widgets)); the
|
||||
O(primitives) walk only runs to build the failure message, because
|
||||
running it per frame made a debug build on the emulator too slow to
|
||||
finish a bench run at all (260s timeout, no report).
|
||||
**Test**: `an_ancestor_redrawing_a_dirty_row_leaves_no_stale_copy`
|
||||
(`iris/src/widget/list.rs`), which fails on the pre-fix code with
|
||||
`1 primitive(s) survived their own widget's redraw`.
|
||||
**Emulator evidence, 2026-09-06** (this checkout's `ai-app-2` AVD,
|
||||
`build-apk.sh debug --abi x86_64 --features "transcript-screen bench
|
||||
force-gles"`, a **debug** build so the guard is live): a complete
|
||||
`run-bench.sh` run — 3,142 frames over 147s across the fling, the
|
||||
400-event stream (which is 400 `apply` calls including the fixture's
|
||||
compaction event), the typing and the keyboard phases — with the
|
||||
assert firing zero times and `logcat` showing no abort. That is the
|
||||
whole of the phone's reported scenario exercised with the invariant
|
||||
checked on every frame.
|
||||
- [~] **Composer touch-drag scroll** for overflowed text — **the
|
||||
mechanism is done, the composer is not.** `Scroll::drag`
|
||||
(`iris/src/widget/position/scroll.rs`) takes its pan from the same
|
||||
`sense::DragGesture` `List` is driven by, and
|
||||
`WidgetLike::scrollable()` registers it beside the wheel handler, so
|
||||
every `.scrollable()` in the codebase pans on a finger with nothing
|
||||
added at the call site. No fling (`Scroll` has no per-frame tick and
|
||||
the areas it wraps are at most a screenful) — see IRIS.md and
|
||||
DECISIONS.md. A vertical drag inside a *focused* field no longer
|
||||
extends a selection either (`attr.rs`'s `on_press` now applies the
|
||||
same `DRAG_SLOP` rule its unfocused branch already did), which is
|
||||
Android `EditText`'s own behaviour and what lets the scroll area
|
||||
around a field win the gesture.
|
||||
**Tests**: four in `scroll.rs` (pan past the slop, a tap inside it,
|
||||
a horizontal drag, the end clamp) plus
|
||||
`a_finger_drag_over_a_scroll_area_pans_it` in `sense_tests.rs`, which
|
||||
drives the whole path — `scrollable()`'s registration, `run_sensors`'
|
||||
dispatch, `Scroll::drag`, arbitration and pointer capture — and fails
|
||||
with `got 0` if the registration is removed.
|
||||
**What is left, with the measurement**: wrapping the composer's field
|
||||
in `.scrollable().masked()` was tried and reverted the same day.
|
||||
`Scroll` resolves `content_len`/`container_len` against
|
||||
`Painter::output_size` — the whole window — so inside the `MaxSize`
|
||||
that caps the composer at six lines the two are in different spaces
|
||||
and the field pans itself entirely out of the bar: measured on the
|
||||
emulator with 474 characters in it (`iris text render: ...
|
||||
size=(1016.7, 623.7)` against a 441px cap) the bar collapsed to its
|
||||
padding with no text in it. Making `Scroll` measure against its own
|
||||
offered box is the next step, and it touches a widget the transcript
|
||||
and the bench shell both use.
|
||||
One real bug **was** found and fixed on the way (`ActiveData::mask`
|
||||
stored the mask a widget *set* rather than the one it was drawn
|
||||
*under*, and `redraw` feeds that field straight back in as the
|
||||
inherited mask — so a targeted redraw of any `Masked` handed it its
|
||||
own mask and aborted on `set_mask`'s nested-mask assert; that is a
|
||||
real abort on the emulator, `assertion failed: self.mask ==
|
||||
MaskIdx::NONE`, reproduced as
|
||||
`redrawing_a_masked_widget_does_not_nest_its_own_mask` in
|
||||
`layout_tests.rs`).
|
||||
- [ ] **Streaming re-layout** (IRIS_TODO.md's last section) — after the
|
||||
above, since they make the stream phase unrepresentative today.
|
||||
- [x] **client-core prerequisites for P1, in parallel** (pure Rust,
|
||||
|
||||
@@ -20,6 +20,14 @@ overlapping, and once more below the composer bar: primitives of a
|
||||
replaced/removed row surviving in the GPU buffers, the same shape as the
|
||||
header drawn twice after a keyboard resize.
|
||||
|
||||
**Root-caused and fixed 2026-09-06** (commit `76b1f99`): the diagnosis in
|
||||
that sentence was right and the location was not -- `UiRenderState::
|
||||
draw_inner` read the `needs_redraw` mark without consuming it and skipped
|
||||
the branch that frees a redrawn widget's old primitives. docs/RUST.md's
|
||||
"Stale primitives, the phone's half" box has the full account, the guard
|
||||
(`orphaned_primitives`, `debug_assert`ed every frame) and the emulator run
|
||||
that exercises it.
|
||||
|
||||
```
|
||||
iris bench report
|
||||
per phase:
|
||||
|
||||
Reference in new issue
Block a user