docs: the 22:16 report reconciled with what was actually run
RUST.md's "Shell lost" section and IRIS_TODO.md's matching paragraph both
said item 4's fix was written but never built or tested. It was committed
in ba2afba with its test passing, so both were stale the moment that
landed and read as if nothing had been run at all.
Replaced with one section per item, saying what was fixed, what was
measured on this checkout's emulator and what the phone still has to
settle: items 2 and 3 ticked with their numbers, item 4 ticked on the code
with phone confirmation still owed (no Vulkan adapter here), item 1 left
open with the exact logcat line for Iris to look at. The two pre-existing
faults found on the way -- the 16-deep move chain and the API-29 JNI calls
-- are recorded where the next reader will hit them.
IRIS.md gains the public-surface entry: `Widget::tick`,
`UiData::animate`/`tick_animations`, `FlingCalculator`'s density and
coefficient, and `MOVE_CHAIN_LIMIT`.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
ed04d4c735
commit
ba0f2ea93f
3 files changed
+306
-5
No files matched your search
+197
@@ -43,6 +43,203 @@ gated on her verdict**, so this pass works the P0 defects and the pure
|
||||
prerequisites in this order. Each item is ticked here by the agent that
|
||||
closes it.
|
||||
|
||||
### Desktop and phone share the code (Iris, 2026-09-07)
|
||||
|
||||
Iris plans to develop a desktop app as well, and asked that most code be
|
||||
sharable between desktop and phone. The workspace already has that
|
||||
shape -- `iris`, `client-core`, `transcript-ui` and `tabs-ui` are
|
||||
platform-free, and `android-app`/`desktop-app` are the entry points --
|
||||
so the rule is about keeping it: **a platform crate holds only what the
|
||||
platform forces.** Today that is JNI, the IME and insets bridge, the
|
||||
surface lifecycle and the bench JNI on Android; winit, argv and the
|
||||
config file on the desktop. **What differs is the screen layout**, since a phone
|
||||
screen with a finger and a desktop screen with a mouse want different
|
||||
arrangements -- a session list beside the transcript rather than a
|
||||
screen behind it, hover states, keyboard shortcuts. **What does not
|
||||
differ is everything a layout is built from**: the widgets (a tap
|
||||
button, a text field, a list, a card, a tool-call row), gestures,
|
||||
folding, paging, selection, and the styling -- colours, spacing, type,
|
||||
the surface ladder -- which is the exact same code on both, never a
|
||||
desktop palette beside a phone one. Those are written once in a shared
|
||||
crate, with a platform trait underneath when a behaviour genuinely
|
||||
differs (`FocusHost`, `OpenUrl`, and the insets/`ime_visible`
|
||||
feed are the existing examples). Two checks before finishing a change
|
||||
under `iris/`: does `desktop-app` still build and run with it, and is
|
||||
any UI logic newly in `android-app` that a desktop would also need?
|
||||
The bench client (`android-app/src/bench_client.rs`, ~1000 lines) is
|
||||
the first thing to look at moving, since a desktop bench on the same
|
||||
fixture is layer 2 of the test rig below.
|
||||
|
||||
### Three test layers, cheapest first (decided 2026-09-07, rig not yet built)
|
||||
|
||||
Iris's suggestion, adopted and layered: test at the cheapest layer that
|
||||
can answer the question, and go up only when it cannot. The emulator
|
||||
costs minutes a cycle; the desktop window seconds; the headless harness
|
||||
runs inside `cargo test`.
|
||||
|
||||
1. **Headless, in-process, no compositor and no GPU -- the default.**
|
||||
`iris/src/layout_tests.rs` already builds a tree over `UiRenderState`
|
||||
with no window, `sense.rs`'s gesture tests feed fabricated
|
||||
`CursorState`s with their own times, and `List::tick_fling` is
|
||||
driven by hand. Extend that into one harness that opens
|
||||
`transcript-ui`'s screen on `app/bench-fixture` (as `bench_client.rs`
|
||||
does on Android, no server), at the phone's logical size and
|
||||
`content_scale` from `docs/bench/iris-phone-v2-2026-09-06.md` (2.55,
|
||||
120Hz -- read, never typed from memory), ticks frames, and feeds a
|
||||
**replayed touch stream** from a trivial file of `(t_ms, action, x,
|
||||
y)` lines with `cursor.time` taken from the file. That is what the
|
||||
emulator cannot do at all: the batched 120Hz flick from the phone
|
||||
report becomes a deterministic test asserting on scroll offset and on
|
||||
the `iris drag release:` velocity. Anything about layout, scroll
|
||||
position, selection, focus or fold state is answered here, with
|
||||
assertions rather than eyes. Nothing renders; a widget's placed
|
||||
rectangle is the evidence.
|
||||
2. **A phone-shaped desktop window under headless sway -- for looking.**
|
||||
`iris/run-headless.sh` already runs a winit binary under a private
|
||||
sway and screenshots it with `grim`. Add a `--phone` mode (output and
|
||||
window at the phone's size and scale, with the scale reaching iris
|
||||
the way Android's does so dp layout runs at that density) and
|
||||
touch-shaped mouse input: a left-button drag pans and flings through
|
||||
`DragArbiter`, long-press selects, no hover. One input path, not a
|
||||
parallel one (`CODE_RULES`). Colour, spacing, text and anything a
|
||||
person has to see is answered here. `swaymsg seat - cursor
|
||||
move/press/release` drives it when a gesture is needed on screen.
|
||||
3. **The Android emulator -- platform plumbing and the final pass.**
|
||||
JNI, IME, insets, surface lifecycle, the renderer rebuild, and one
|
||||
verification run before a build goes to the phone. Not for iterating
|
||||
on layout.
|
||||
|
||||
Pass condition for the rig: `cargo test` runs a fixture-backed headless
|
||||
transcript screen with a replayed flick and asserts a nonzero release
|
||||
velocity and a moved scroll offset; one command opens the same screen in
|
||||
a phone-shaped window and screenshots it. Record the commands here when
|
||||
it lands.
|
||||
|
||||
### The 22:16 phone report, worked 2026-09-06/07
|
||||
|
||||
Iris's four items are listed in docs/IRIS_TODO.md's "From the phone,
|
||||
2026-09-06, 22:16"; this is what was found and what was run. Item 4 was
|
||||
committed on its own (`ba2afba`); items 1-3 and everything below landed
|
||||
together after the emulator evidence.
|
||||
|
||||
**Item 4, text cooked after a resume -- root cause, fixed in `ba2afba`.**
|
||||
Not "the cached text primitives are never redrawn": they *are*.
|
||||
`IrisViewPeer::surface_changed` (`iris/src/android/view.rs`) calls
|
||||
`render.resize(...)` on every surface event including the new-renderer
|
||||
branch, which sets `UiRenderState::resized`, which makes the next
|
||||
`update` take `redraw_all` rather than `redraw_updates` -- so after a
|
||||
resume every widget's `draw` runs again. The stale coordinates come from
|
||||
one cache further in: `TextView::render` (`iris/src/widget/text/mod.rs`)
|
||||
returns its cached `RenderedText` whenever the wrap width, buffer and
|
||||
attrs are unchanged, so `TextData::place` is never reached, no glyph is
|
||||
re-rasterised into the fresh atlas, and the *previous* atlas's
|
||||
`uv_min`/`uv_max`/`layer` go straight back to the GPU. Text whose content
|
||||
changed after the resume -- the diagnostics pane Iris tapped -- re-shapes
|
||||
and is therefore perfect, which is exactly the split in her screenshot.
|
||||
The fix is one mechanism: a `generation` counter on `GlyphAtlas`, bumped
|
||||
by `clear`, recorded on each `RenderedText`, added to `TextView::render`'s
|
||||
cache key, with a `debug_assert_eq!` in `Painter::glyphs` that a submitted
|
||||
quad's generation is the live one. Test
|
||||
`clearing_the_atlas_re_renders_cached_text_instead_of_reusing_it`, run and
|
||||
passing; **still needs phone-side confirmation**, since no emulator here
|
||||
has a Vulkan adapter and the GLES path may not destroy the surface at all.
|
||||
|
||||
**Item 2, the keyboard would not reopen -- fixed and confirmed.**
|
||||
`attr.rs`'s `on_press`, already-focused branch, now calls `focus_gained`
|
||||
on a tap that stays inside `DRAG_SLOP`, which is what Android's own
|
||||
`EditText` does (`showSoftInput` is idempotent). Emulator, 2026-09-07:
|
||||
first tap `mInputShown=true`; back gesture; second tap `mInputShown=true`
|
||||
and the composer rises again. **Negative control run**: with that one call
|
||||
removed and nothing else changed, the second tap leaves
|
||||
`mInputShown=false` -- Iris's report exactly. **The case the fix had no
|
||||
reason to touch**, also run: a horizontal swipe across the focused
|
||||
composer and a vertical swipe out of it both leave `mInputShown=false`, so
|
||||
her earlier "if I swipe over the input bar it brings up the keyboard" has
|
||||
not come back.
|
||||
|
||||
**Item 3, the IME height -- fixed and confirmed.** `MainActivity.java`
|
||||
sends `getInsets(ime()).bottom` *and* `isVisible(ime())` as two separate
|
||||
values (the height used to be sent as the boolean 1/0, which is why
|
||||
nothing could pad by it); `Insets`/`WindowInsets` carry both, and
|
||||
`bench_client.rs` reads the boolean for its state machine and the height
|
||||
for `Composer::set_bottom_inset`. The list follows for free -- it is
|
||||
`.height(rest(1))` in the same `Span` as the composer bar, so the bar
|
||||
growing shrinks the list. Emulator, 2026-09-07:
|
||||
`iris insets: ... bottom=883 ime_bottom=883 ime_visible=true`, and the
|
||||
composer's box moves from `31,2277..1048,2329` to `31,1457..1048,1509` --
|
||||
820px, which is 883 less the 63px navigation bar it was already clearing.
|
||||
Screenshot checked: the transcript ends above the composer, which sits on
|
||||
the keyboard.
|
||||
|
||||
**Item 1, the fling -- two more defects behind the first, all three
|
||||
fixed here; the phone is what settles it.** The velocity half is what the
|
||||
report predicted: `on_touch_event` read only each `MotionEvent`'s final
|
||||
position, so a batched 120Hz flick fed the tracker one sample and
|
||||
`velocity()` answered 0.0. It now replays every historical sample
|
||||
(`getHistoricalAxisValue`/`getHistoricalEventTimeNanos`) through the
|
||||
sensor pass, `CursorState` carries the sample's *own* time (so a replay
|
||||
loop's speed cannot become the measured velocity), and the press itself is
|
||||
a sample, as Android's own `VelocityTracker` does with `ACTION_DOWN`.
|
||||
`iris drag release: samples=… span=…ms v=… outcome=…` logs the decision.
|
||||
Then the emulator showed the two the report could not have known about:
|
||||
|
||||
1. **Nothing ever advanced a fling.** `List::fling` sets the state;
|
||||
`tick_fling` moves it; and `tick_fling`'s only caller in the workspace
|
||||
was `bench_client.rs`'s own fling phase, which drives it in a loop.
|
||||
So the benchmark flung and a finger never did -- and the earlier
|
||||
"verified flinging on the emulator with `render()` counts" was that
|
||||
benchmark measuring itself. Measured before the fix: frames stop on
|
||||
the same millisecond as `iris drag release`. iris now has one
|
||||
animation mechanism -- `Widget::tick(now) -> bool`, ids registered
|
||||
with `UiData::animate`, drained each frame by
|
||||
`UiData::tick_animations`, which both backends call before the draw
|
||||
and re-request a frame from while it answers true. `List::tick` is
|
||||
`tick_fling`; `Selection::drag` registers on `Released(Some(v))`.
|
||||
Test: `a_registered_fling_is_driven_by_tick_animations_and_then_
|
||||
unregisters`, confirmed to fail without the registration.
|
||||
2. **The fling lasted 45 seconds.** Visible only once flings animated at
|
||||
all. Two causes, both in `FlingCalculator`: `List::fling` hardcoded
|
||||
`FlingCalculator::new(1.0)` while the velocity it is fed is in
|
||||
physical pixels (`List` reads `painter.density()` now), and
|
||||
`physical_coefficient` multiplied by `FLING_FRICTION` (0.015) where
|
||||
AOSP multiplies by its own tuning constant **0.84** -- a coefficient
|
||||
56x too small, put through `exp(ln(…)/(rate-1))`. Every existing test
|
||||
compared the calculator with itself (monotonic, signed, integrates to
|
||||
the closed form) and so passed throughout;
|
||||
`a_flick_lasts_what_aosps_own_formula_says_it_does` pins the absolute
|
||||
numbers against AOSP's formula worked by hand. Emulator after both:
|
||||
release at `v=11064`, frames for **1.62s**, then none -- against
|
||||
AOSP's own 1.586s for that velocity at density 2.75.
|
||||
|
||||
**What Iris should look for on the phone**: `adb logcat | grep "iris
|
||||
drag release"`. `samples=1` or `span=0.0ms` means the historical
|
||||
replay is not reaching the tracker on her device; a sensible
|
||||
`samples`/`span` with `v=` in the thousands and `outcome=Released(Some
|
||||
(…))` means the gesture is measured correctly and anything still wrong
|
||||
is downstream of it. `outcome=Tapped` means the flick never crossed
|
||||
the slop.
|
||||
|
||||
**Two things found on the way, both pre-existing at `ba2afba`.**
|
||||
|
||||
- **`MOVE_CHAIN_LIMIT` was 16 and the composer's chain is 17.** Tapping
|
||||
the composer in any debug build aborted on `resolve_move_chain`'s
|
||||
assert; in a release build (what Iris runs) the walk simply stops
|
||||
summing, on the CPU *and* in shader.wgsl, so a widget past the bound
|
||||
draws and hit-tests short by whatever the outer slots held, with
|
||||
nothing on screen to say so. Both constants are 64 now, and the assert
|
||||
prints the chain (`64(0, 0) -> 63(0, 0) -> … -> 0(0, 0)`) so a cycle
|
||||
and an honestly-deep tree can be told apart -- which is how this one
|
||||
was: 17 distinct slots.
|
||||
- **`minSdk` is 29**, up from 26. `getEventTimeNanos` and
|
||||
`getHistoricalEventTimeNanos` are API 29, and a missing JNI method
|
||||
there is a hard crash on the first touch rather than a degraded fling.
|
||||
`build-apk.sh`'s `cargo ndk -P` matches.
|
||||
|
||||
Still open and **pre-existing**: the composer bar's grey background is not
|
||||
drawn on the `transcript-screen bench` build, so the transcript shows
|
||||
through where the bar should be (`Stack{StackSize::Child(1)}` is the thing
|
||||
to look at). Unchanged by any of the above.
|
||||
|
||||
### Task A, closed 2026-09-06: the composer scrolls on a finger
|
||||
|
||||
`iris/transcript-ui/src/composer.rs` is `field.scrollable().masked()` now.
|
||||
|
||||
Reference in new issue
Block a user