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.
This commit is contained in:
iris committed 2026-09-06 13:43:56 -04:00
1 parent d9872989fa
commit c02152a4f4
8 files changed
+168 -47

No files matched your search

+27 -2
View File
@@ -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
+5 -3
View File
@@ -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
+26 -31
View File
@@ -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) {
+28 -3
View File
@@ -18,6 +18,18 @@ pub struct UiRenderState {
old_root: Option<WidgetId>,
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<WidgetId>,
/// 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 --
+8
View File
@@ -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);
+68 -7
View File
@@ -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);
+6
View File
@@ -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;
-1
View File
@@ -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);