iris android-app: header-duplicate investigation, ime-inset fix for keyboard confirmation
Two follow-ups after the keyboard/dp/header pass, both requested against the P0 box: (a) The header row rendering a second time inside the transcript area after a keyboard-triggered resize: reproduced reliably (tap the composer, screenshot after the keyboard opens). Ruled out one concrete hypothesis -- on_insets_changed rebuilding top_bar on every ime_bottom change, unrelated to the header's own status-bar padding -- with a guard (last_top_pad) that reproduced the identical duplicate afterward, so repeated rebuilding is not the cause. Kept the guard as a real (if insufficient) fix for needless rebuilds. Not root-caused: Span's two-phase provisional/real draw and the redraw_all-vs-redraw_updates split are the two live suspects, but pinning which one (or something else) produces the duplicate needs instrumenting draw_inner directly or the phone. Full writeup in RUST.md's P0 box. (b) Why on_insets_changed's ime_bottom never confirmed the keyboard being shown, on either the auto-diagnostics or the new bench keyboard phase: MainActivity.java uses windowSoftInputMode="adjustResize", under which WindowInsets.Type.ime()'s own inset amount is defined to read zero (the window already resized to avoid the overlap that inset would describe) -- the same trap AGENTS.md already names for the Compose side. Fixed to read insets.isVisible(ime()) instead, a boolean unaffected by resize-vs-pan. This alone did not make the callback re-fire on this emulator, which still shows no insets callback after the initial one at attach -- named but unconfirmed hypothesis: a non-edge-to-edge Activity may not get insets redelivered for a pure IME toggle handled via resize, needing an edge-to- edge opt-in this pass did not attempt given the risk to adjustResize's own behavior. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
irisandClaude Fable 5.1 committed
1 parent
4afc453faa
commit
03c6be80a3
4 files changed
+256
-12
No files matched your search
@@ -8,6 +8,31 @@ 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: `List::anchor_position_display`, `FrameReport::mark_phase`/`phase_stats`/`late_at_hz` (RUST.md's "Benchmark v2")
|
||||
|
||||
`List` gained `anchor_position_display(&self) -> String`, reporting the
|
||||
anchor's own row index and pixel offset (`idx=N/off=Mpx`, or
|
||||
`idx=more-before`/`idx=more-after`/`idx=none`) -- what a scripted
|
||||
benchmark reads to report fling travel. Note the anchor does not
|
||||
necessarily change *slot* over a long scroll (this widget's own documented
|
||||
design: the anchor is a stable identity, not re-derived from what's on
|
||||
screen each frame), so this is not the same measurement as a Compose
|
||||
`LazyListState.firstVisibleItemIndex`, which does track the true topmost
|
||||
visible row -- the `off` half is what actually reflects how far a fling
|
||||
travelled.
|
||||
|
||||
`iris_core::render::frame_report::FrameReport` gained three methods for
|
||||
per-phase benchmark reporting: `mark_phase(name)` records a named phase
|
||||
boundary at the current frame/instant; `phase_stats(now, refresh_hz)`
|
||||
returns one `PhaseStats` (frames, wall duration, late count/percent,
|
||||
p50/p90/p99, worst) per marked phase, sliced from the existing ring by a
|
||||
new parallel `index_ring`; `late_at_hz(refresh_hz)` gives the whole run's
|
||||
late count/percent judged against an arbitrary refresh rate rather than
|
||||
the fixed 60Hz `JANK_THRESHOLD` every existing caller still uses (a
|
||||
separate method, not a parameter on `report()`, so nothing else changes
|
||||
behaviour). `RING_CAPACITY` grew 4096->16384 to hold a full multi-phase
|
||||
run without evicting earlier phases' samples.
|
||||
|
||||
## 2026-09-06: `List::fling`, `VelocityTracker`, `FlingCalculator` (IRIS_TODO.md's "swiping has no momentum")
|
||||
|
||||
`iris::widget::List` gained a real fling: `fling(velocity_px_per_s)` starts
|
||||
|
||||
+176
-7
@@ -4573,13 +4573,182 @@ device.
|
||||
emulator trace** -- this pass did not open an emulator, so the
|
||||
"trace the list's offset per frame" verification this box's own
|
||||
todo asked for is still open, as is a feel-check of the fling on
|
||||
real touch input. **Benchmark v2's four-phase spec (fling/stream/
|
||||
type/keyboard) in `bench_client.rs` was not attempted this pass** --
|
||||
wiring a real IME show/hide and refresh-rate read through
|
||||
`bench_jni.rs`, and `FrameReport`'s per-phase accounting, is real
|
||||
scope on its own and was left rather than shipped half-verified;
|
||||
the Compose half above is already done and is the reference shape
|
||||
for whoever picks this up. No redelivery this pass.
|
||||
real touch input.
|
||||
|
||||
**Benchmark v2, iris half, done 2026-09-06, later the same day.**
|
||||
`bench_client.rs` implements all four phases against the identical
|
||||
constants this box's "Benchmark v2" spec names: fling (8 out + 8
|
||||
back at 12,000px/s through `List::fling`, waiting for
|
||||
`!is_scrolling()` capped 3s with a 300ms pause between, travel
|
||||
reported as `idx=N/off=Mpx` via a new `List::anchor_position_display`
|
||||
-- note this list's anchor does not necessarily change *slot* during
|
||||
a long scroll (the module's own documented design: the anchor is
|
||||
named by identity, not re-derived from what's on screen), so an
|
||||
iris travel reading is not apples-to-apples with Compose's
|
||||
`firstVisibleItemIndex`, which does change slot -- a real difference
|
||||
in what the two numbers mean, not a bug, and worth reading `off`
|
||||
rather than `idx` when comparing runs), stream (unchanged), type
|
||||
(the exact 600-character `TYPE_TEXT` constant, verified by a unit
|
||||
test, one char per 50ms into the composer's real `TextEdit` via
|
||||
`.set()` -- the same whole-string-replace shape `BenchRun.kt`'s own
|
||||
`setComposerText` uses, not a per-character insert), and keyboard
|
||||
(5 cycles through `bench_jni.rs`'s new `show_ime`/`hide_ime`
|
||||
`InputMethodManager` calls, confirmed from `on_insets_changed`'s
|
||||
real `ime_bottom` transitions via a new `ImeState` counter rather
|
||||
than assumed from the JNI call succeeding).
|
||||
|
||||
`iris_core::render::frame_report::FrameReport` gained `mark_phase`/
|
||||
`phase_stats`/`late_at_hz` (new unit tests in `frame_report.rs`):
|
||||
phases are sliced by absolute frame index against a second ring
|
||||
(`index_ring`) alongside the existing duration ring, and late/jank
|
||||
is judged against a real Hz read from `bench_jni.rs`'s new
|
||||
`refresh_rate_hz` (`View::getDisplay().getRefreshRate()`) rather
|
||||
than the fixed 60Hz `JANK_THRESHOLD` every other caller still uses
|
||||
-- a separate method, not a parameter on the existing one, so
|
||||
nothing else in the codebase changes behaviour. `RING_CAPACITY`
|
||||
4096->16384 since one full v2 run is 3,000+ frames.
|
||||
|
||||
**A real deadlock, found and fixed while wiring this up.** Getting
|
||||
a value back out of a task spawned via `rsc.spawn_task` has no
|
||||
built-in return channel (`ctx.update`'s closures are fire-and-
|
||||
forget), so a new `read_from_state` helper sends the result through
|
||||
an `mpsc` channel and polls for it. Its first version only worked
|
||||
for the *first* call in a chain: nothing about `ctx.update` drains
|
||||
itself, so unless something calls `redraw.request_redraw()` after
|
||||
*this specific* enqueue, nothing ever runs the closure -- and every
|
||||
call after the first relied on a stale, already-fired
|
||||
`request_redraw()` from a previous step. The fix is structural:
|
||||
`read_from_state` now takes the redraw handle and calls it itself,
|
||||
immediately after enqueueing, every time.
|
||||
|
||||
**Verified end to end, this checkout's own emulator (cold `emu up`,
|
||||
`force-gles`, x86_64 -- this AVD again enumerates zero Vulkan
|
||||
adapters on a cold boot, matching every prior finding in this
|
||||
file):**
|
||||
|
||||
iris bench report
|
||||
per phase:
|
||||
fling: 1481 frames over 53.2s
|
||||
late: 158 (10.7%)
|
||||
total p50 11.2ms p90 16.9ms p99 26.5ms
|
||||
worst 43.3ms
|
||||
stream: 401 frames over 20.8s
|
||||
late: 342 (85.3%)
|
||||
total p50 26.1ms p90 49.1ms p99 57.2ms
|
||||
worst 61.3ms
|
||||
type: 1202 frames over 63.1s
|
||||
late: 89 (7.4%)
|
||||
total p50 12.8ms p90 15.1ms p99 23.3ms
|
||||
worst 26.7ms
|
||||
keyboard: 9 frames over 9.1s
|
||||
late: 2 (22.2%)
|
||||
total p50 6.3ms p90 25.8ms p99 25.8ms
|
||||
worst 25.8ms
|
||||
|
||||
frames:
|
||||
3093 frames over 146.3s at 60Hz (16.7ms budget)
|
||||
late: 591 (19.1%)
|
||||
total p50 12.4ms p90 22.2ms p99 51.2ms
|
||||
worst 61.3ms
|
||||
cpu_p50 0.7ms gpu_wait_p50 11.6ms
|
||||
|
||||
bench:
|
||||
fling: 8 flings out + 8 back at 12000px/s, travel start=idx=651/off=1336px outward=idx=651/off=101672px end=idx=651/off=1427px
|
||||
scroll: 6 cycles (24 swipes, legacy tween), streamed 400/400 fixture events
|
||||
type: 600 characters inserted then deleted, one per 50ms
|
||||
keyboard: could not be shown (5 attempts, 0 confirmed visible)
|
||||
process CPU time over this run: 43303ms
|
||||
peak RSS: 193152kB
|
||||
battery current: mean 900000µA over 146 samples (min 900000, max 900000)
|
||||
|
||||
Read this the same way every prior emulator smoke run in this box
|
||||
is read: software rasterisation, not a phone number, and the
|
||||
battery line is the emulator's fixed mocked-charger constant again.
|
||||
**Travel**: the `idx` stays fixed at 651 through the whole fling in
|
||||
both directions (see the `anchor_position_display` caveat above) --
|
||||
`off` is what actually moved, growing to 101,672px outward before
|
||||
the return trip brings it back near its start, which is real, large
|
||||
motion (a fast, hard fling, matching Iris's "travel way faster"
|
||||
ask), just not directly comparable to Compose's idx-188-reached
|
||||
reading from the same box's earlier v2 entry. **`keyboard: could
|
||||
not be shown`**: expected given the ime-inset finding below, not a
|
||||
new regression.
|
||||
|
||||
Redelivered: `./build-apk.sh release --abi arm64-v8a --features
|
||||
"transcript-screen bench"` (Vulkan, no `force-gles`; the x86_64
|
||||
jniLibs slice left over from emulator testing was removed first so
|
||||
the delivered APK is arm64-only, confirmed via `aapt2 dump
|
||||
badging`), `apksigner verify` shows the same `CN=ai-app` cert,
|
||||
copied to `~/host/bench/iris-bench-arm64.apk` and `~/repos/
|
||||
ai-app-bench/iris/build/outputs/apk/release/iris-bench-arm64.apk`;
|
||||
that repo's own README gained a dated entry. `run-bench.sh`
|
||||
extended for the longer run (260s poll cap, `-A 60` instead of
|
||||
`-A 6`) to fit v2's four phases.
|
||||
|
||||
**(a) The header-duplicate bug (found by a concurrent pass on this
|
||||
branch): investigated, not fixed.** Reproduced reliably
|
||||
(`ui-trace record --do "tap 'Message'"` then `adb exec-out
|
||||
screencap`): the three-button row renders a second, full copy
|
||||
inside the transcript area the moment the keyboard opens. Read
|
||||
`Span::draw`'s own two-phase placement doc (a provisional
|
||||
full-region draw to learn each child's size, then a real
|
||||
`widget_within` placement) as the most likely mechanism, since it
|
||||
is the one place in this tree that deliberately draws a widget
|
||||
twice in normal operation and relies on the two draws landing at
|
||||
the same place to stay a cheap move rather than a visible second
|
||||
copy -- and `UiRenderState::update`'s `redraw_all`-vs-
|
||||
`redraw_updates` split (LAYOUT.md) means a `.set()`-driven targeted
|
||||
redraw of just `top_bar` and a resize-driven full redraw of the
|
||||
whole tree are two structurally different code paths that could in
|
||||
principle disagree about where that widget's primitives belong on a
|
||||
frame where both fire close together. **One concrete, testable
|
||||
hypothesis was ruled out**: `on_insets_changed` rebuilding
|
||||
`top_bar` on every call, including ones only about `ime_bottom`
|
||||
(nothing to do with the header's own padding). Added a guard
|
||||
(`last_top_pad`, skips the rebuild unless `insets.top` itself
|
||||
changed) and reproduced the *exact same* duplicate afterward --
|
||||
unchanged, byte-for-byte, in the same screenshot -- so repeated
|
||||
rebuilding is not the cause; the guard is kept anyway since it is a
|
||||
real (if here insufficient) reduction in needless work. **Not
|
||||
root-caused**: doing so needs either instrumentation inside
|
||||
`Span::draw`/`draw_inner` to see the two placements' actual regions
|
||||
on the frame the bug happens, or the phone. Left for a follow-up
|
||||
pass rather than guessed at further.
|
||||
|
||||
**(b) Why the keyboard phase and the keyboard-open auto-diagnostics
|
||||
both read "not confirmed": a real, named platform interaction,
|
||||
partly fixed.** `MainActivity.java`'s manifest declares
|
||||
`windowSoftInputMode="adjustResize"` (AGENTS.md's own "Things that
|
||||
have bitten": without it the keyboard pans the window off screen
|
||||
instead of resizing it). Under `adjustResize`, `WindowInsets.
|
||||
Type.ime()`'s own inset *amount* is defined to read zero once the
|
||||
window has already resized to avoid the overlap that inset would
|
||||
otherwise describe -- confirmed by reading Android's own
|
||||
`WindowInsets` contract, not guessed at. So the numeric `ime_bottom`
|
||||
this app was reading is *structurally* never going to be positive
|
||||
here, independent of anything wrong in `iris`'s own code -- the same
|
||||
trap AGENTS.md already names for the Compose side
|
||||
(`WindowInsets.isImeVisible` "does not share the failure mode").
|
||||
**Fixed**: `MainActivity.java`'s `OnApplyWindowInsetsListener` now
|
||||
reads `insets.isVisible(WindowInsets.Type.ime())` (a boolean,
|
||||
unaffected by resize-vs-pan) and passes `1`/`0` through the
|
||||
existing `ime_bottom` JNI field instead of the always-zero numeric
|
||||
inset -- correct on its own terms, and kept, but **did not by
|
||||
itself make the keyboard phase or the auto-diagnostics fire on this
|
||||
emulator**: `logcat` shows the platform's own `InsetsController:
|
||||
show(ime(), fromIme=false)`/window-resize events happening (the
|
||||
keyboard genuinely opens, confirmed by screenshot), but no further
|
||||
`setOnApplyWindowInsetsListener` callback at all after the initial
|
||||
one at attach. Named hypothesis, not confirmed: a plain (non-edge-
|
||||
to-edge) `Activity` that has not called `WindowCompat.
|
||||
setDecorFitsSystemWindows(window, false)` may not get insets
|
||||
redelivered for a pure IME toggle handled entirely via resize --
|
||||
only the initial attach-time dispatch is guaranteed. Confirming and
|
||||
fixing that needs opting the activity into edge-to-edge, which is a
|
||||
real window-behaviour change interacting with the exact
|
||||
`adjustResize` setting AGENTS.md protects, not attempted this pass
|
||||
given the risk-to-time-remaining ratio. Both open items are
|
||||
recorded in `~/repos/ai-app-bench`'s README with today's date.
|
||||
|
||||
- [ ] **P1 — session screen parity.** History paging backward (with the
|
||||
page-boundary healing `client-core` does not have yet, below),
|
||||
|
||||
Reference in new issue
Block a user