docs: the defect pass's findings -- RUST.md boxes, IRIS_TODO ticks, DECISIONS and IRIS entries
This commit is contained in:
1 parent
c02152a4f4
commit
3e72a4ef19
4 files changed
+166
-12
No files matched your search
@@ -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
|
||||
|
||||
+25
-1
@@ -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
|
||||
|
||||
+19
-1
@@ -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-
|
||||
|
||||
+91
-10
@@ -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
|
||||
|
||||
Reference in new issue
Block a user