From 3e72a4ef19682d3083641139f174056fbf3bad70 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Sun, 6 Sep 2026 13:47:28 -0400 Subject: [PATCH] docs: the defect pass's findings -- RUST.md boxes, IRIS_TODO ticks, DECISIONS and IRIS entries --- docs/DECISIONS.md | 31 ++++++++++++++ docs/IRIS.md | 26 +++++++++++- docs/IRIS_TODO.md | 20 ++++++++- docs/RUST.md | 101 +++++++++++++++++++++++++++++++++++++++++----- 4 files changed, 166 insertions(+), 12 deletions(-) diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index ec8b1ce..430adae 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -5,6 +5,37 @@ 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 (defect pass) + +- **The keyboard-open diagnostics overlay is gone; the capture only + logs now.** It was added when `on_insets_changed` was not firing at all + and there was no way to get a report off the phone. It fires reliably + since the activity went edge-to-edge -- and what that looks like in + use is a full-screen report covering the app **every time the keyboard + opens**, with its own Copy/Close buttons sitting underneath the + keyboard, so it cannot be dismissed (reproduced on the emulator this + pass: two `tap 'CLOSE'` runs left it up). An interruption for something + nobody asked for, over the app you are trying to type into. The named + `Diagnostics` button still shows the same text on demand, and the new + `iris surface:`/`iris insets:` log lines carry the lifecycle a `logcat` + pull needs. Reversible: `capture_keyboard_diagnostics` is still the one + place this is decided, and `PlatformHandle::show_diagnostics_overlay` + is still there. + +- **The bench shell's report pane is sized to its report, not to a share + of the window.** It held `.height(rest(1))` beside the transcript's + `rest(2)`, so an *empty* `TextEdit` reserved a third of every screen -- + which is what Iris's "the app does not start with keyboard spacing + correct" screenshot was showing, with the composer two thirds down and + black below it. It is `.max_height(dp(260))` now and sits above the + transcript rather than under the composer, where it was eating the + navigation-bar clearance. Cost: a filled report is clipped at 260dp + rather than scrolling (a `Scroll` there drew itself off the top of the + screen, since `Scroll` pins to the end of its content and reports its + content's full length to the parent -- worth fixing in `Scroll`, not + worked around here). "Copy report" and `logcat` still have the whole + thing. + ## 2026-09-05 - **iris no longer asks every device for compute-shader limits it never diff --git a/docs/IRIS.md b/docs/IRIS.md index c1a17f2..9b03367 100644 --- a/docs/IRIS.md +++ b/docs/IRIS.md @@ -8,7 +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") +## 2026-09-06: a tap on a text field always leaves a caret + +`TextEditCtx::select` used to compare the tap position against the +*laid-out text's* own box and set `selection = None` for anything outside +it. A press only reaches `select` after being hit-tested to the widget, so +that "outside" meant the field's own padding -- or, for an **empty** field, +everything, since an empty layout is a zero-width box. So tapping an empty +composer focused it and opened the keyboard while leaving no caret, and +`TextEditCtx::insert`/`insert_str` return early with no caret: every +keystroke was dropped in silence, and no glyph ever appeared. Parley's +`from_point`/`extend_to_point` already clamp a point outside the layout to +the nearest cursor position, which is also what a tap in a field's padding +should do. + +Behaviour change a caller would notice, in one line: **`select` with a +non-drag position now always produces a selection; it no longer clears +one.** Clearing is `TextEditCtx::deselect`, which is what the backends' +focus handling already calls. A drag is unchanged -- with no previous +selection there is still nothing to extend, so it produces none. + +`insert_str` also gained a `debug_assert!` for the no-caret case, so an +insert routed to an unfocused field fails at the mistake in a debug build +instead of silently swallowing input. + +## 2026-09-06: `List::anchor_position_display`## 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 diff --git a/docs/IRIS_TODO.md b/docs/IRIS_TODO.md index f097b65..b2135ea 100644 --- a/docs/IRIS_TODO.md +++ b/docs/IRIS_TODO.md @@ -282,7 +282,25 @@ agent ticks it here with the evidence. since only she can say whether it *feels* like a fling now; the emulator's screenshot timing could not always catch the tail of a fast-settling one visually (same caveat noted in RUST.md). -- [ ] **"Text still disappears if I leave and come back to the app."** +- [~] **"Text still disappears if I leave and come back to the app."** + **Instrumented 2026-09-06 so the phone can answer it**, since no + emulator here has a Vulkan adapter. `iris/src/android/view.rs` now logs + one `log::info!` line per surface event with the glyph/atlas counts: + `iris surface: surface_destroyed, tearing the renderer down + (glyphs_cached=387 atlas_pages=1)`, `iris surface: surface_changed + 1080x2424 already_live=false glyphs_cached=387 atlas_pages=1`, `iris + surface: new renderer built (Gl), clearing glyph atlas: glyphs=387 + pages=1`, plus `iris insets: ... window=(1080, 2424)` on every insets + change. That is the emulator's own healthy app-switch cycle, verified + this pass (home, reopen, screenshot: all text intact, + `/tmp/appswitch.png`). **The one line to look for on the phone is + `already_live=`**: `true` on the return from backgrounding would mean + the surface came back *without* a `surface_destroyed`, so + `surface_changed` reconfigured a renderer whose Vulkan swapchain and + atlas textures belong to a window that is gone -- the reuse branch + never clears the atlas, by design. `false` with no `new renderer built` + line after it would mean the renderer failed to rebuild. Either answer + names the fix; guessing between them from here does not. The `GlyphAtlas::clear`/`Textures::reset` fix was verified on the emulator under `force-gles` only; the phone runs Vulkan. So either the reset is not reached on the phone's path (a different surface- diff --git a/docs/RUST.md b/docs/RUST.md index 986acd8..5862732 100644 --- a/docs/RUST.md +++ b/docs/RUST.md @@ -155,7 +155,7 @@ closes it. "Docs" section asked for. See `docs/REVIEW-2026-09-06.md`'s own "Fixed, 2026-09-06" section for the per-finding account. Committed together with the review file. -- [ ] **Iris's 11:39 phone report on the 02:07 build** (four items, +- [~] **Iris's 11:39 phone report on the 02:07 build** (four items, verbatim in `IRIS_TODO.md`'s "From the phone, 2026-09-06, 11:39"): composer floating two thirds down the screen at launch with black below it; a swipe starting on the composer held until the finger @@ -163,15 +163,80 @@ closes it. lost on app-switch on the phone despite the emulator-verified atlas reset. The first and last are the same class as the next box and go to that agent; the middle two are the merge box's. -- [ ] **Stale primitives and invisible composer text** — the header drawn - twice after a keyboard resize, the `Compacted:` row drawn twice on - Iris's phone, and typed text never appearing (P0 box item 2). All - three sit on the `redraw_updates` targeted-redraw path and may be - one bug; the P0 box says the next step is instrumentation inside - `Span::draw`/`draw_inner` showing where each placement's - primitives actually land on the frame it goes wrong. - `list.rs`'s new `replacing_the_last_row_many_times_does_not_leak_ - primitives` test already pins the widget arena as *not* the leak. +- [~] **Stale primitives and invisible composer text**, 2026-09-06: + **typed text is fixed and was never a renderer bug at all**; the two + duplicate-drawing halves are **not reproducible** on this checkout's + emulator any more and are recorded below with what changed. + + **1. Typed text (P0 box item 2, `IRIS_TODO.md`'s own item) -- + fixed.** The composer's buffer was empty the whole time. + `TextEditCtx::select` (`iris/src/widget/text/edit.rs`) compared the + tap against the *laid-out text's* box and set `selection = None` for + anything outside it; an empty field lays out to a zero-width box, so + tapping an empty composer granted focus and opened the keyboard with + no caret, and `insert_str` returns early without one -- every + keystroke was dropped in silence. **Gboard's suggestion strip is + Gboard's own composing state, not a read of our buffer**, which is + what made the earlier pass conclude the buffer held the text and + send the search downstream into the renderer; the `accessibility` + dump saying `text=""` for the `Message` node was the first + contradicting evidence. Parley clamps a point outside the layout by + itself, and a press reaching `select` has already been hit-tested to + the widget, so the "outside" branch had nothing left to mean. Three + new tests in `edit.rs`, one of which fails on the pre-fix code, plus + a `debug_assert!` in `insert_str` so an insert with no caret fails at + the mistake rather than dropping input -- it caught + `layout_tests::composing_text_after_a_keyboard_resize_...` typing + into an unfocused field the moment it was added. **Emulator + evidence**: `ui-trace record --do "tap 'Message'"` then `adb shell + input text` shows the text in the bar with a caret + (`/tmp/final-typing.png`) and logs `iris text render: chars=5 ... + glyphs=5`, against `glyphs=0` per keystroke before. + + **2. The header drawn twice after a keyboard resize (this box's own + "(a)") no longer has a path to happen on this emulator, for a + measured reason**: since `MainActivity.java` went edge-to-edge + (`e12c708`), **opening the keyboard no longer resizes the surface at + all**. Measured: `render()` reports `out_size=(1080, 2282)` unchanged + across an IME open, while the new `iris insets:` line reports + `bottom=63 ime_bottom=0` -> `bottom=883 ime_bottom=1`. So the IME is + an inset now, not a `surface_changed`, and the two-phase `Span::draw` + the duplicate was blamed on is not re-entered. Reproduction attempts + this pass, all negative: `tap 'Message'` + `ui-trace elements` + (exactly one "Run benchmark" in every frame of the trace), a + screenshot with the keyboard open, and a real `adb shell wm size + 1080x2200` *while the keyboard was open* (a genuine + `surface_changed`) -- one header row, no stray copy + (`/tmp/resize-dup.png`). + + **3. The `Compacted:` row drawn twice on Iris's phone is still + open**, and nothing here reproduces it. What was ruled out this + pass: the widget arena (`list.rs`'s + `replacing_the_last_row_many_times_does_not_leak_primitives`), the + `top_bar` rebuild (`last_top_pad`, a previous pass), and now the + keyboard-resize trigger above. One real defect *was* found by + reading the path and is fixed, though it cannot be shown to be her + bug: `UiRenderState::draw_started` -- the guard whose whole job is + "do not redraw a widget an ancestor is drawing right now, or one of + the two copies is orphaned" -- **tested its own set after removing + the id from it**, so the test was constant `false` and the guard + could never fire, while the set grew by one entry per widget ever + drawn and was never emptied. It is now inserted around + `Widget::draw` and removed when it returns, with a `debug_assert!` + 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. @@ -4858,6 +4923,22 @@ device. extended for the longer run (260s poll cap, `-A 60` instead of `-A 6`) to fit v2's four phases. + **Redelivered, 2026-09-06, the defect pass.** `./build-apk.sh + release --abi arm64-v8a --features "transcript-screen bench"` + (Vulkan, no `force-gles`; the x86_64 `jniLibs` slice from this + pass's emulator work was removed first, confirmed arm64-only by + listing the APK's `lib/` entries), `apksigner verify` showing the + same `CN=ai-app` cert, copied to `~/host/bench/ + iris-bench-arm64.apk`; that README gained a dated entry naming what + to look for. What changed: the composer sits at the bottom of the + screen at launch again (the black third was the bench shell's empty + report pane, not an inset -- see the plan box above), typing into it + works at all (the empty-field caret bug), the keyboard no longer + throws up an undismissable diagnostics overlay, and every surface + and insets event is logged so a phone `logcat` can answer the + app-switch text loss. Not fixable from here and still open: the + duplicated `Compacted:` row. + **(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