docs/RUST.md: Iris's 2026-09-08 phone report -- tap-jump, nested scroll, folded cards, and the bench's 60Hz gesture
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
2756087e1c
commit
476609e1d3
1 file changed
+136
+136
@@ -7804,3 +7804,139 @@ bigger design question in shared tooling.
|
|||||||
- uniffi: [repo](https://github.com/mozilla/uniffi-rs), [KMP bindings fork](https://github.com/UbiqueInnovation/uniffi-kotlin-multiplatform-bindings)
|
- uniffi: [repo](https://github.com/mozilla/uniffi-rs), [KMP bindings fork](https://github.com/UbiqueInnovation/uniffi-kotlin-multiplatform-bindings)
|
||||||
- GPUI mobile: [gpui-mobile](https://github.com/itsbalamurali/gpui-mobile)
|
- GPUI mobile: [gpui-mobile](https://github.com/itsbalamurali/gpui-mobile)
|
||||||
- The earlier Dioxus spike's findings on `wgpu`/Vulkan in this emulator: `~/repos/tdep-survey/app-dioxus/README.md`
|
- The earlier Dioxus spike's findings on `wgpu`/Vulkan in this emulator: `~/repos/tdep-survey/app-dioxus/README.md`
|
||||||
|
|
||||||
|
## Iris's phone report, 2026-09-08 (three defects, from the app on her phone)
|
||||||
|
|
||||||
|
Sent with a screenshot of an expanded tool group and three `iris
|
||||||
|
diagnostics` dumps taken while scrolling by hand (no benchmark had run).
|
||||||
|
Her words, then what the diagnosis is and who is fixing it. The phone was
|
||||||
|
on Vulkan throughout (`adapter: Mali-G715 (Vulkan)`), with `wgpu errors:
|
||||||
|
none` and frame times that are *not* the complaint: `p50=2.8ms p90=5.4ms
|
||||||
|
p99=55.7ms worst=99.2ms janky%=1.55` on the worst of the three dumps and
|
||||||
|
`janky%=0.21` on the best. So all three are behaviour, not throughput.
|
||||||
|
|
||||||
|
### 1. "Tapping sometimes seems to make the scrolling jump, particularly
|
||||||
|
### when tapping on things that have events like horizontal scrolling"
|
||||||
|
|
||||||
|
**Hypothesis to test first: `List::note_tap`'s re-anchor fires on taps
|
||||||
|
that change no height, and resolves against stale `extents`.** `list.rs`'s
|
||||||
|
module doc ("Hold the edge nearest the tap") is explicit that the *next*
|
||||||
|
`List::draw` re-anchors to whichever row contained the last tap, choosing
|
||||||
|
the nearer edge, using `extents` cached *as of the previous frame*. That is
|
||||||
|
right for the case it was built for -- a row about to change height -- and
|
||||||
|
it is a free position change for every tap that changes nothing, which is
|
||||||
|
most of them: `tool.rs`'s `note_tap` runs on every card tap and every group
|
||||||
|
toggle, and a tap landing on a code fence or a card in the middle of a
|
||||||
|
settle or a fling resolves against extents that no longer describe where
|
||||||
|
those rows are. Re-anchoring to a stale box *is* the jump.
|
||||||
|
|
||||||
|
Pass conditions:
|
||||||
|
|
||||||
|
- A tap that changes no row's height moves nothing on screen, from a
|
||||||
|
settled list and from one still settling, in a layer-1
|
||||||
|
(`transcript-fixture`) test driven from a `.touch` recording -- the
|
||||||
|
layer that owns the clock, so "still settling" is expressible.
|
||||||
|
- A tap on a row that *does* change height still holds the edge nearest
|
||||||
|
the tap; the existing test for that keeps passing.
|
||||||
|
- The fix is in `List`, not in each caller: `note_tap`'s siblings are
|
||||||
|
every widget that taps (cards, the collapse bar, links, the composer),
|
||||||
|
and a rule stated at one caller is the defect class CODE_RULES names.
|
||||||
|
|
||||||
|
**Iris, same day, once she had looked harder -- this is the real shape of
|
||||||
|
it, and it supersedes the "check this too" paragraph below:** *"it seems
|
||||||
|
like mixing horizontal and vertical scrolling is broken. It should only
|
||||||
|
trigger horizontal if you drag left or right, and vertical should fall
|
||||||
|
through if you drag up or down. It also keeps snapping back to some
|
||||||
|
position when horizontally scrolling for some reason."*
|
||||||
|
|
||||||
|
So there are two faults in the nested case, and the first is the one to fix
|
||||||
|
properly:
|
||||||
|
|
||||||
|
- **A horizontal `Scroll` claims a gesture it should have let through.**
|
||||||
|
The rule she states is the one to implement, and it is the rule
|
||||||
|
`DragArbiter` already knows how to express: a press is `Undecided` until
|
||||||
|
it has moved `DRAG_SLOP`, and then it belongs to whichever axis it
|
||||||
|
actually moved along -- the child if that is the child's axis, the
|
||||||
|
enclosing list otherwise. Today the child's arbiter is built with
|
||||||
|
`DragArbiter::on(Axis::X)`, whose `Panning` branch takes the drag on its
|
||||||
|
own axis and answers `Undecided` for everything else, and *nothing hands
|
||||||
|
the rejected gesture to the list* -- the outer list's arbiter never got a
|
||||||
|
`press_start` for it. This is one mechanism for both directions, not a
|
||||||
|
special case for fences: an inner scrollable defers to its parent for
|
||||||
|
any axis it does not own.
|
||||||
|
- **Horizontal scrolling snaps back to a position.** First suspect is
|
||||||
|
`Scroll::update_amt` clamping against `content_len - container_len`,
|
||||||
|
which LAYOUT.md says is a frame behind -- and the fence's content is
|
||||||
|
inside a `masked()` (the `scrollable_on(Axis::X)`-draws-nothing
|
||||||
|
workaround above), so the length being clamped against may be the
|
||||||
|
*clipped* box rather than the text's real width. If so, `amt` is pulled
|
||||||
|
back to 0 on the next frame, every frame, which reads exactly as a snap
|
||||||
|
back. Measure it before fixing it: log `amt`, `content_len` and
|
||||||
|
`container_len` for one horizontal drag.
|
||||||
|
|
||||||
|
Also check, because it is the other half of what she named: what a
|
||||||
|
horizontal `Scroll` (`row.rs`'s `BlockFrame::Verbatim`, `scrollable_on
|
||||||
|
(Axis::X)`) does to the vertical gesture that passes through it. Both
|
||||||
|
arbiters see the same press; `Scroll::drag` passes `PressState::default()`,
|
||||||
|
so it cannot know the list under it was flinging, and the list's own
|
||||||
|
arbiter may never have seen the `press_start` (`DragArbiter::update`'s doc
|
||||||
|
records that recovery path for `Selection`). Say what the arbitration
|
||||||
|
actually is rather than adding a second recovery.
|
||||||
|
|
||||||
|
### 2. "Opening the folded cards seems pretty buggy right now" -- and the
|
||||||
|
### sanity check she asked for
|
||||||
|
|
||||||
|
Her framing is the important part: *"I don't understand how this is even
|
||||||
|
possible if the code is sane, please sanity check it and make sure it's
|
||||||
|
using the framework correctly. The framework should have good built in
|
||||||
|
change detection just by editing widgets in the context normally."*
|
||||||
|
|
||||||
|
She is right to suspect the framework rather than the screen, because
|
||||||
|
`tool.rs` is **already written around four known, unfixed iris defects**,
|
||||||
|
all listed under docs/IRIS_TODO.md's "Found by P1b (2026-09-06)" and all
|
||||||
|
worked around there rather than fixed:
|
||||||
|
|
||||||
|
- a `Span` of `Pad`ded children inside another `Span` places those children
|
||||||
|
a slot out of step (so the group is one flat `Span` and has lost the 4dp
|
||||||
|
inset Compose gives it);
|
||||||
|
- `scrollable_on(Axis::X)` on a non-editable `Text` draws nothing (so a
|
||||||
|
card's raw block is `masked()` and a long command is **clipped**, which
|
||||||
|
is what her screenshot's output box shows -- lines cut at the right edge
|
||||||
|
with nothing saying so);
|
||||||
|
- no overflow ellipsis at all, so a clipped summary looks like a short one;
|
||||||
|
- no drawn chevron, so the mark is a font glyph -- and the bundled fonts
|
||||||
|
were **removed on 2026-09-07** in favour of the platform collection, so
|
||||||
|
the empty box before "Edit" in her screenshot is almost certainly that
|
||||||
|
codepoint missing from the phone's own faces. UI_RULES: "don't rely on
|
||||||
|
characters the platform might not have." This one is a regression the
|
||||||
|
font change introduced and is the cheapest of the four to fix.
|
||||||
|
|
||||||
|
So the work is not "make `tool.rs` behave"; it is **fix the framework
|
||||||
|
defects and delete the workarounds** (the standing rule: solve the setup,
|
||||||
|
don't live with a workaround every caller has to remember). The sanity
|
||||||
|
check she asked for is a second deliverable: whether `tool.rs`'s
|
||||||
|
rebuild-the-whole-row-on-toggle (`build_content` + `Shared::set_content`)
|
||||||
|
is the framework's intended change detection or a way around it, and if
|
||||||
|
iris's change detection is meant to handle "edit the widgets in the context
|
||||||
|
normally", why this screen does not.
|
||||||
|
|
||||||
|
### 3. "The benchmark scrolling seems like it's at 60fps or something, it's
|
||||||
|
### way less smooth than me scrolling manually"
|
||||||
|
|
||||||
|
**Confirmed by reading, before any measurement: the bench drives the
|
||||||
|
gesture at 60Hz by construction on a 120Hz phone.**
|
||||||
|
`iris/android-app/src/bench_client.rs`'s `ANIM_STEP_MS = 16` is a
|
||||||
|
`tokio::time::sleep` between synthetic move samples, and the fling phase
|
||||||
|
"ticks the fling forward in ~60Hz steps" the same way. A finger delivers
|
||||||
|
samples at the panel's rate; the bench delivers 62 a second. So the bench's
|
||||||
|
scroll really is coarser than hers, the difference she sees is the rig
|
||||||
|
rather than the renderer, and every frame number the bench has produced was
|
||||||
|
measured against a gesture no hand would make.
|
||||||
|
|
||||||
|
Fix: drive one step per **real frame**, from the frame callback the
|
||||||
|
renderer already runs on, and derive each step's delta from the elapsed
|
||||||
|
time it reports rather than from a constant. Pass condition: the bench's
|
||||||
|
sample rate matches the display's refresh (say so in the report, next to
|
||||||
|
the adapter line, so a number is never read without it), and the report
|
||||||
|
states the rate it drove at so an old report cannot be compared to a new
|
||||||
|
one by accident.
|
||||||
Reference in new issue
Block a user