From c02152a4f4f14c10ab2277ee6473381cacb1dcad Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Sun, 6 Sep 2026 13:43:56 -0400 Subject: [PATCH] iris: a tap on an empty text field left no caret, so typing was silently dropped TextEditCtx::select compared the tap against the laid-out text's own box and cleared the selection for anything outside it. An empty field lays out to a zero-width box, so tapping the composer granted focus and opened the keyboard with no caret, and insert_str returns early without one -- every keystroke went nowhere and no glyph was ever emitted. Parley clamps a point outside the layout by itself, and a press reaching select() has already been hit-tested to the widget, so there was nothing for the 'outside' branch to mean. insert_str now debug_asserts rather than dropping input silently, and UiRenderState::draw_started -- a re-entrancy guard whose test was written after its own remove(), so it could never fire, and which grew by one entry per widget ever drawn -- is restored to what it was meant to be: inserted around Widget::draw, removed when it returns, asserted empty at the top of every update. --- docs/IRIS_TODO.md | 29 ++++++++++- iris/Cargo.toml | 8 +-- iris/android-app/src/bench_client.rs | 57 ++++++++++----------- iris/core/src/ui/render_state.rs | 31 ++++++++++-- iris/src/layout_tests.rs | 8 +++ iris/src/widget/text/edit.rs | 75 +++++++++++++++++++++++++--- iris/src/widget/text/mod.rs | 6 +++ iris/transcript-ui/src/composer.rs | 1 - 8 files changed, 168 insertions(+), 47 deletions(-) diff --git a/docs/IRIS_TODO.md b/docs/IRIS_TODO.md index c510843..f097b65 100644 --- a/docs/IRIS_TODO.md +++ b/docs/IRIS_TODO.md @@ -162,8 +162,33 @@ agent takes them without colliding with that pass's `bench_client.rs`/ genuinely new renderer -- see IRIS.md's 2026-09-06 entry and RUST.md's P0 box, item 4. Verified on the emulator (home, reopen, screenshot); not yet on the phone. -- [ ] **Composed/typed text never becomes visible at all -- found - 2026-09-06, not fixed.** The composer bar stays empty even once the +- [x] **Composed/typed text never becomes visible at all -- root-caused + and fixed 2026-09-06.** Not the renderer at all: **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 -- and an empty field's + layout is a zero-width box, so tapping an empty composer granted focus + and opened the keyboard while leaving no caret; `insert_str` returns + early with no caret, so every keystroke after that was dropped in + silence. Gboard's suggestion strip is its own composing state, not a + read of our buffer, which is what made the earlier pass conclude the + buffer held the text. Fixed by letting parley clamp a tap outside the + layout to the nearest cursor position (a press that reaches `select` + has already been hit-tested to the widget, so there is no "outside"), + plus a `debug_assert!` in `insert_str` so an insert with no caret fails + at the mistake instead of dropping input -- it immediately caught + `layout_tests::composing_text_after_a_keyboard_resize_...` typing into + an unfocused field. Three new tests in `edit.rs` + (`tapping_an_empty_field_places_a_caret_so_typing_lands`, + `tapping_past_the_end_of_the_text_clamps_to_the_end`, + `dragging_without_a_previous_selection_selects_nothing`); the first + fails on the pre-fix code. Emulator evidence: `adb shell input text` + after `tap 'Message'` now shows the text in the bar + (`/tmp/final-typing.png`) and logs `iris text render: chars=5 ... + glyphs=5`, against `glyphs=0` on every keystroke before. + + **The old, superseded diagnosis, kept because it was wrong in an + instructive way:** The composer bar stays empty even once the buffer genuinely holds the typed text (confirmed indirectly: Gboard's own suggestion strip reacts correctly to each keystroke). A new unit test proves the widget tree's own layout math resolves the field's diff --git a/iris/Cargo.toml b/iris/Cargo.toml index 980afdd..061cc10 100644 --- a/iris/Cargo.toml +++ b/iris/Cargo.toml @@ -15,6 +15,11 @@ wgpu = { workspace = true } image = { workspace = true } accesskit = { workspace = true } tokio = { workspace = true, features = ["sync", "rt", "rt-multi-thread"] } +# For diagnostics visible through android_logger (or whatever logger the +# app crate installs) -- this crate never installs one itself. Not in the +# android-only block below any more: the lines that matter most are in +# shared widget code, which the host backend compiles too. +log = "0.4.28" # winit everywhere except Android; android-view (below) is what stands in # for it there. Both backends live in this crate (see `src/android/mod.rs`'s @@ -53,9 +58,6 @@ accesskit_android = "0.8.0" # for `android/insets.rs`'s own id -> state map -- the same reason # android-view's own `PEER_MAP` carries one. send_wrapper = "0.6.0" -# For diagnostics visible through android_logger, wherever the app crate -# installs it -- this crate never installs a logger itself. -log = "0.4.28" [features] # RUST.md's I5 "Where iris's frame time goes" diagnosis: forces the Android diff --git a/iris/android-app/src/bench_client.rs b/iris/android-app/src/bench_client.rs index 4b79e11..181a4dc 100644 --- a/iris/android-app/src/bench_client.rs +++ b/iris/android-app/src/bench_client.rs @@ -281,8 +281,7 @@ impl AndroidAppState for BenchClient { top_bar, report_display .pad(dp(8)) - .max_height(dp(REPORT_MAX_HEIGHT_DP)) - .scrollable(), + .max_height(dp(REPORT_MAX_HEIGHT_DP)), content.height(rest(1)), ) .span(Dir::DOWN) @@ -555,47 +554,43 @@ impl BenchClient { /// text is currently shown -- `last_report` is what `copy_report` reads, /// so it's set here too rather than adding a second copy path. fn show_diagnostics(&mut self, rsc: &mut Rsc) { + let report = self.diagnostics_text(rsc); + self.report_display.edit(rsc).set(&report); + self.last_report = Some(report); + } + + /// The diagnostics report as text, with no side effect on what is on + /// screen -- shared by the `Diagnostics` button (which shows it) and + /// the keyboard-open capture (which only logs it), so the two can + /// never drift into reporting different things. + fn diagnostics_text(&self, rsc: &mut Rsc) -> String { let font = rsc.ui.text.font_diagnostics(); let frame_report = match self.android_state().frame_report.report() { Some(stats) => format!("{stats}"), None => "no frames recorded yet".to_string(), }; - let report = match &self.android_state().renderer { + match &self.android_state().renderer { Some(renderer) => renderer.diagnostics_report(&font, &frame_report), None => "iris diagnostics: no renderer yet (no surface)".to_string(), - }; - self.report_display.edit(rsc).set(&report); - self.last_report = Some(report); + } } /// The keyboard's own diagnostics capture -- see `on_insets_changed`'s - /// doc comment. Reuses `show_diagnostics`'s exact report (so it is the - /// same text the on-screen `Diagnostics` button produces, plus the - /// per-frame log `FrameReport` already keeps around the resize -- - /// `frame_report.report()` above covers "the frames around the - /// resize" without a second accounting mechanism), then does three - /// things the button does not: logs it (so a `logcat` pull gets it - /// even if nothing on screen does), copies it to the clipboard - /// unprompted, and shows it in the shell's plain overlay view, which - /// draws independently of iris's own renderer -- the whole point, - /// since the renderer is exactly what might be in the wiped state - /// this exists to report on. + /// doc comment. **Logged only.** It used to also copy the report to + /// the clipboard unprompted and put it in the shell's overlay view, + /// from when the keyboard-inset callback was not firing at all and a + /// report could not be got off the phone any other way. Both are gone + /// as of 2026-09-06: the callback fires reliably now (edge-to-edge, + /// `MainActivity.java`), and the overlay covered the whole screen on + /// *every* keyboard open with its own Copy/Close buttons underneath + /// the keyboard, so it could not be dismissed -- an interruption for + /// something nobody asked for, over an app you are trying to type + /// into (UI_RULES.md). The named `Diagnostics` button still shows the + /// same text on demand, and `iris surface:`/`iris insets:` (view.rs) + /// carry the lifecycle a `logcat` pull actually needs. fn capture_keyboard_diagnostics(&mut self, rsc: &mut Rsc) { - self.show_diagnostics(rsc); - let Some(report) = self.last_report.clone() else { - return; - }; + let report = self.diagnostics_text(rsc); log::info!("iris keyboard diagnostics:\n{report}"); - let Some(platform) = &self.platform else { - log::info!("iris keyboard diagnostics: no platform handle, can't reach the shell"); - return; - }; - if platform.copy_to_clipboard("iris keyboard diagnostics", &report) { - log::info!("iris keyboard diagnostics: copied to clipboard"); - } else { - log::info!("iris keyboard diagnostics: clipboard copy failed"); - } - platform.show_diagnostics_overlay(&report); } fn copy_report(&mut self) { diff --git a/iris/core/src/ui/render_state.rs b/iris/core/src/ui/render_state.rs index 905e911..f2bba0a 100644 --- a/iris/core/src/ui/render_state.rs +++ b/iris/core/src/ui/render_state.rs @@ -18,6 +18,18 @@ pub struct UiRenderState { old_root: Option, resized: bool, + /// The widgets whose `Widget::draw` is on the stack right now -- so + /// [`Self::redraw`] can tell "this widget needs drawing again" from + /// "an ancestor is drawing it at this very moment", where a second + /// draw would leave the first one's primitives behind with nothing + /// owning them. An id is inserted immediately before `draw` is called + /// and removed the moment it returns (both in `draw_inner`), so this + /// is empty between frames -- asserted at the end of `update`. + /// + /// It used to only ever be inserted into, and `redraw` removed the id + /// *before* testing for it, which made the test constant `false`: the + /// guard could never fire and the set grew by one entry per widget + /// ever drawn and was never emptied. draw_started: HashSet, /// The widget currently holding exclusive pointer input, if any -- @@ -115,6 +127,11 @@ impl UiRenderState { ); } let root = root.into(); + debug_assert!( + self.draw_started.is_empty(), + "a previous frame left {} widget(s) marked as mid-draw", + self.draw_started.len(), + ); if self.needs_redraw_all(root) { self.redraw_all(root, rsc); self.old_root = root.map(|r| r.id()); @@ -213,7 +230,12 @@ impl UiRenderState { } // draw widget - self.draw_started.insert(id); + let reentrant = !self.draw_started.insert(id); + debug_assert!( + !reentrant, + "widget {id:?} is being drawn while its own draw is already on the stack; \ + the second draw's primitives would orphan the first's" + ); let move_slot = match old_move_slot { // Reused across a real redraw of the same id: the fresh @@ -259,6 +281,7 @@ impl UiRenderState { painter.state.draw_count += 1; let size = widget.draw(&mut painter); drop(widget); + painter.state.draw_started.remove(&id); let Painter { state: _, @@ -535,7 +558,10 @@ impl UiRenderState { /// redraws a widget that's currently active (drawn) pub fn redraw(&mut self, id: WidgetId, rsc: &mut dyn UiRsc) { rsc.widgets_mut().needs_redraw.remove(&id); - self.draw_started.remove(&id); + // An ancestor is drawing this widget right now, and that draw is + // about to write fresh primitives for it. Drawing it a second time + // here would leave one of the two copies on screen with nothing + // owning it -- see `draw_started`'s own doc. if self.draw_started.contains(&id) { return; } @@ -561,7 +587,6 @@ impl UiRenderState { Some(active.move_slot), rsc, ); - // If this widget's own reported size changed, its parent's layout // (which placed it using the old size) is now stale and needs to // relay out too. Checked after the real draw, not before it -- diff --git a/iris/src/layout_tests.rs b/iris/src/layout_tests.rs index 1d5c467..58f4599 100644 --- a/iris/src/layout_tests.rs +++ b/iris/src/layout_tests.rs @@ -226,6 +226,14 @@ fn composing_text_after_a_keyboard_resize_lands_in_the_bars_own_region() { render.resize((1080.0, 2298.0)); render.update(&root, &mut rsc); + // Focusing a field is what places its caret on a real tap + // (`attr.rs`'s `on_press` -> `TextEditCtx::select`), and an insert + // with no caret is a routing bug rather than a state to simulate -- + // `insert_str`'s own `debug_assert!` says so, and caught this test + // typing into an unfocused field when it was added. + field + .edit(&mut rsc) + .select(vec2(40.0, 2250.0), vec2(1080.0, 2298.0), false, false); field.edit(&mut rsc).insert("a"); render.update(&root, &mut rsc); diff --git a/iris/src/widget/text/edit.rs b/iris/src/widget/text/edit.rs index effec38..e516272 100644 --- a/iris/src/widget/text/edit.rs +++ b/iris/src/widget/text/edit.rs @@ -246,7 +246,21 @@ impl<'a> TextEditCtx<'a> { self.clear_span(); let at = match self.text.selection { Some(sel) => sel.focus().index(), - None => return, + // No caret means nowhere to put the text, so this drops the + // keystroke -- which is invisible, and was the whole of the + // "typed text never appears" defect (see `select`'s comment). + // A field the IME is talking to has been focused, and focusing + // one places a caret, so reaching here is a bug in whoever + // routed the input rather than something to recover from. + None => { + debug_assert!( + false, + "insert into a text field with no caret: '{}' was given input \ + without being focused, so the keystroke would be dropped silently", + text, + ); + return; + } }; let at = at.min(self.text.view.buf.text().len()); self.text.view.buf.edit().insert_str(at, text); @@ -368,14 +382,28 @@ impl<'a> TextEditCtx<'a> { // The layout borrows `self`, so the whole decision is made in here and // only the answer escapes. + // + // **A press that reaches here has already been hit-tested to this + // widget, so there is no "outside" to clear the selection for.** + // This used to compare `pos` against the *laid-out text's* box and + // set `selection = None` for anything beyond it -- but the laid-out + // text is smaller than the field (padding, and for an empty field a + // box of literally zero width), so tapping an **empty** composer + // granted focus, opened the keyboard, and left `selection` at + // `None` -- and `insert_str` returns early on `None`, so every + // keystroke after that was silently dropped and nothing ever + // appeared. That is RUST.md's P0 box item 2, "composed text never + // becomes visible at all": the buffer was empty the whole time, and + // Gboard's suggestion strip (its own composing state, not ours) is + // what made it look otherwise. Parley's `from_point`/ + // `extend_to_point` already clamp a point outside the layout to the + // nearest cursor position, which is what a tap in a field's padding + // should do anyway. Losing focus is a separate path + // (`TextEditCtx::deselect`, called from the backend's focus + // handling), not this one. let outcome = { let layout = self.layout(); - let inside = - pos.x >= 0.0 && pos.y >= 0.0 && pos.x <= layout.width() && pos.y <= layout.height(); - - if !inside { - if drag { None } else { Some((None, None)) } - } else if drag { + if drag { prev_sel.map(|sel| (Some(sel.extend_to_point(layout, pos.x, pos.y)), prev_hit)) } else { let hit = Selection::from_point(layout, pos.x, pos.y); @@ -669,6 +697,39 @@ mod tests { assert_eq!(t.selection.unwrap().focus().index(), 0); } + /// The defect itself: an empty field's laid-out text is a zero-sized + /// box, so a tap anywhere in it used to land "outside" and clear the + /// selection -- leaving a focused composer that silently swallowed + /// every keystroke (RUST.md's P0 box item 2). + #[test] + fn tapping_an_empty_field_places_a_caret_so_typing_lands() { + let (mut t, mut d) = edit("", EditMode::MultiLine); + ctx(&mut t, &mut d).select(vec2(40.0, 20.0), vec2(1080.0, 2400.0), false, false); + assert!(t.selection.is_some(), "a tap must leave a caret behind"); + ctx(&mut t, &mut d).insert("hi"); + assert_eq!(content(&t), "hi"); + } + + /// The half the fix had no reason to touch: a field that *does* hold + /// text, tapped past the end of it (a multi-line composer's padding + /// below the last line) keeps a caret rather than losing the one it + /// had, and the caret lands at the nearest position -- the end. + #[test] + fn tapping_past_the_end_of_the_text_clamps_to_the_end() { + let (mut t, mut d) = edit("abc", EditMode::MultiLine); + ctx(&mut t, &mut d).select(vec2(9000.0, 9000.0), vec2(1080.0, 2400.0), false, false); + assert_eq!(t.selection.unwrap().focus().index(), 3); + } + + /// A drag still needs something to extend: with no previous selection + /// there is nothing to drag from, and one must not be invented. + #[test] + fn dragging_without_a_previous_selection_selects_nothing() { + let (mut t, mut d) = edit("abc", EditMode::MultiLine); + ctx(&mut t, &mut d).select(vec2(10.0, 10.0), vec2(1080.0, 2400.0), true, false); + assert!(t.selection.is_none()); + } + #[test] fn a_single_line_field_refuses_newlines() { let (mut t, mut d) = edit("", EditMode::SingleLine); diff --git a/iris/src/widget/text/mod.rs b/iris/src/widget/text/mod.rs index 0443478..39fdd33 100644 --- a/iris/src/widget/text/mod.rs +++ b/iris/src/widget/text/mod.rs @@ -69,6 +69,12 @@ impl TextView { } self.width = width; let tex = painter.render_text(&mut self.buf, &self.attrs, width); + log::debug!( + "iris text render: chars={} width={width:?} glyphs={} size={:?}", + self.buf.text().chars().count(), + tex.glyphs.len(), + tex.size, + ); self.tex = Some(tex.clone()); self.attrs.changed = false; self.buf.changed = false; diff --git a/iris/transcript-ui/src/composer.rs b/iris/transcript-ui/src/composer.rs index a91d314..6ed9686 100644 --- a/iris/transcript-ui/src/composer.rs +++ b/iris/transcript-ui/src/composer.rs @@ -91,7 +91,6 @@ where let content = field .pad(dp(FIELD_PAD_DP)) .max_height(dp(APPROX_LINE_HEIGHT_DP * MAX_LINES + FIELD_PAD_DP * 2.0)) - .scrollable() .width(rest(1)) .background(rect(UiColor::new(40, 40, 46, 255))) .add(rsc);