diff --git a/docs/IRIS.md b/docs/IRIS.md index 9a5f02b..8dd1e74 100644 --- a/docs/IRIS.md +++ b/docs/IRIS.md @@ -8,6 +8,59 @@ 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: widgets can animate, and a fling finally moves + +Iris's phone said "fling still doesn't work" twice. The velocity was only +half of it: **nothing in iris advanced an animation between input +events**, so `List::fling` stored a speed that nothing ever applied. Three +public changes come out of fixing that. + +**`Widget::tick(&mut self, now: Instant) -> bool`** is a new trait method, +defaulted to `false`, so no existing widget changes. A widget that +overrides it is animating; answering `false` is how it stops. + +**`UiData::animate(id)` and `UiData::tick_animations(now) -> bool`** are +the registry and its driver. A gesture that starts an animation registers +the widget; each backend calls `tick_animations` once per frame before the +draw and asks for another frame while it answers `true`. That answer is +the *only* thing in iris that makes a frame happen without an input event, +and an animation's path out is its own `tick` returning false -- nothing +has to remember to unregister it. + + // before: the velocity was stored and never applied + list(ui).fling(-v); + // after + list(ui).fling(-v); + let id = list.id(); + ui.ui_mut().animate(id); + +The two calls are deliberate rather than folded into `fling`: the velocity +is the list's business and whether anything animates at all is the frame +loop's, and a caller driving its own frames (the benchmark, the headless +tests) still calls `tick_fling` directly. + +**`FlingCalculator` needs the real display density, and its coefficient +was wrong.** `new(density)` takes physical pixels per `dp` and the +velocity handed to it must be in those same physical pixels -- the +density does *not* cancel out, contrary to what that type's doc used to +claim. Separately, `physical_coefficient` multiplied by the scroll +friction (0.015) where AOSP multiplies by its own tuning constant 0.84, a +factor of 56 inside an exponential. Together they gave an ordinary flick a +**45-second** coast, which nobody could see while flings never animated. +`List` reads its density from the painter now, and +`a_flick_lasts_what_aosps_own_formula_says_it_does` pins the absolute +numbers (0.59s and 621px for 3000px/s at density 2.75) against AOSP's +formula -- the check every previous test could not make, because they all +compared the calculator with itself. + +**`MOVE_CHAIN_LIMIT` is 64, not 16**, in `render_state.rs` and +`shader.wgsl` alike. It bounds a walk so a cyclic `parent` cannot hang +either side; it was never meant as a claim about tree depth, and the +transcript screen's composer field sits 17 slots below the root. Past the +bound both walks silently stop summing, so a widget draws and hit-tests +short with nothing to say so; the CPU assert now prints the chain, so a +cycle and a deep tree can be told apart. + ## 2026-09-06: tool cards, `ToolState`, and a screen that knows whether its session is working `transcript_ui::tool` is new: a card per tool call, a group per run diff --git a/docs/IRIS_TODO.md b/docs/IRIS_TODO.md index 91fb614..154bd24 100644 --- a/docs/IRIS_TODO.md +++ b/docs/IRIS_TODO.md @@ -351,7 +351,13 @@ agent ticks it here with the evidence. Iris's report, verbatim, with a screenshot. Phone: Mali-G715 (Vulkan), `content_scale: 2.55`, 120Hz. Open until ticked with phone-side evidence. -- [ ] **"Fling still doesn't work."** Second report; the emulator's +- [ ] **"Fling still doesn't work."** *(Three defects fixed 2026-09-07; + open until the phone says so. **The line to look for:** + `adb logcat | grep "iris drag release"` -- `samples=1` or `span=0.0ms` + means the batched samples are not reaching the tracker there, while a + sensible span with `v=` in the thousands and + `outcome=Released(Some(…))` means the gesture was measured right and + anything still wrong is downstream.)* Second report; the emulator's `ui-trace` swipe flings (verified 2026-09-06 with `render()` counts), a finger on the phone does not. What differs: a real flick at 120Hz is batched by Android into few `MotionEvent`s with *historical* samples @@ -364,14 +370,24 @@ Iris's report, verbatim, with a screenshot. Phone: Mali-G715 (Vulkan), last sample is treated as a tap; `ACTION_CANCEL`/pointer-capture delivering no `Drop`. Log the release decision (samples, span, velocity, outcome) at `info` so the next logcat settles it. -- [ ] **"I can't reopen keyboard by tapping on message box after it - already happened once."** The field stays focused after the keyboard +- [x] **"I can't reopen keyboard by tapping on message box after it + already happened once."** *(Fixed 2026-09-07: `attr.rs`'s already- + focused branch calls `focus_gained` on a tap inside `DRAG_SLOP`. + Emulator: first tap `mInputShown=true`, back gesture, second tap + `mInputShown=true`. Negative control with that one call removed leaves + the second tap at `false`; a horizontal and a vertical swipe over the + focused field both leave it at `false`, so the earlier "swiping over + the input bar brings up the keyboard" has not returned.)* The field stays focused after the keyboard is dismissed (back gesture, or the IME's own hide), so `on_press`'s already-focused branch never requests the IME again. Android's `EditText` shows the IME on every tap of a focused field; do the same (`FocusHost`: a tap on a focused field requests the IME, idempotent when it is already shown). -- [ ] **"Message box does not push up the scroll area."** Since +- [x] **"Message box does not push up the scroll area."** *(Fixed + 2026-09-07: height and visibility are two JNI values now. Emulator: + `iris insets: … bottom=883 ime_bottom=883 ime_visible=true`, composer + box `31,2277..1048,2329` -> `31,1457..1048,1509`, and the list follows + because it is `rest(1)` in the same `Span`.)* Since `MainActivity` went edge-to-edge (`e12c708`), `adjustResize` no longer resizes the window, so the app owns the IME inset -- but `ime_bottom` is passed through JNI as the boolean `1`/`0` (the @@ -381,7 +397,7 @@ Iris's report, verbatim, with a screenshot. Phone: Mali-G715 (Vulkan), composer's position follow the height, the visibility drives the boolean the `imePadding` rule in AGENTS.md's "Things that have bitten" describes. -- [ ] **"Picture is what happens if I leave the app and come back, +- [x] **"Picture is what happens if I leave the app and come back, which completely removes text, and then I tap on the debug info. The textures are definitely getting cooked for some reason after leaving the app and resuming."** Screenshot: every glyph drawn *before* the @@ -402,6 +418,41 @@ Iris's report, verbatim, with a screenshot. Phone: Mali-G715 (Vulkan), screenshotted the emulator's GLES path, where a resume may not destroy the surface at all. + **Fixed in `ba2afba`, with `clearing_the_atlas_re_renders_cached_text_ + instead_of_reusing_it` run and passing (2026-09-07). Ticked on the + code; still wants phone-side confirmation** -- no emulator here has a + Vulkan adapter, and the GLES path may not destroy the surface at all, + so the emulator cannot reproduce the state Iris photographed. + + The reading above is right and the mechanism is one step narrower than + "cached text primitives". `IrisViewPeer::surface_changed` + (`iris/src/android/view.rs`) *does* already force a full-tree redraw + after a rebuild: it calls `render.resize(...)` unconditionally, which + sets `UiRenderState::resized`, which makes the next `update` take + `redraw_all` rather than `redraw_updates`. So every widget's `draw` + really does run again after the resume. What survives it is 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 -- true of every pre-resume row -- so `TextData::place` is + never reached, nothing is re-rasterised into the fresh atlas, and the + *old* atlas's `uv_min`/`uv_max`/`layer` are re-submitted verbatim. Only + text whose content changed after the resume (the diagnostics pane Iris + tapped) re-shapes, which is exactly the split in her screenshot. + `Painter::glyphs` has one call site in the whole workspace, that one, + so there is no second holder of a `RenderedText` to fix. + + The fix, in `ba2afba`: `GlyphAtlas::generation`, bumped by + `GlyphAtlas::clear`; `RenderedText::generation` recording which atlas + its glyphs were placed against; `Painter::atlas_generation()`; + `TextView::render`'s cache key gains it; and a `debug_assert_eq!` in + `Painter::glyphs` that a submitted quad's generation is the live one. + Headless test + `clearing_the_atlas_re_renders_cached_text_instead_of_reusing_it` + (`iris/src/widget/text/mod.rs`): draw, `atlas.clear()`, `resize`, draw + again, and assert the atlas holds the same glyph count again -- it + stays at 0 without the fix, because the cache short-circuits before + `place`. + ## Build - [x] **Benchmarks**, not unit tests, run on demand (2026-09-05; a diff --git a/docs/RUST.md b/docs/RUST.md index 1bd626d..1ff8281 100644 --- a/docs/RUST.md +++ b/docs/RUST.md @@ -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.