diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index 1a3ac23..e2c91f2 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -5,6 +5,21 @@ 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-08 (later: a scroll area that grew redraws once) + +From Iris's phone report about the composer's padding while typing +newlines. IRIS.md's entry has the account. + +- **A `Scroll` whose measured content length differs from the length it + offered its child asks for one more draw** (`Painter::draw_again`), + rather than the stale box being the last one drawn. Pays one extra + draw of the scroll's subtree when the content's length changes -- + including its first frame, where the offered length is a placeholder -- + and nothing on an ordinary scroll tick. +- **A frame that leaves any widget dirty now requests another frame** on + both backends. Previously only an animation did, so any use of + `draw_again` depended on some later input to deliver its correction. + ## 2026-09-08 (every crate to its latest version, wgpu 28 -> 30) At Iris's request. RUST.md's "Every crate to its latest version" box has diff --git a/docs/IRIS.md b/docs/IRIS.md index 7c9b80f..869343e 100644 --- a/docs/IRIS.md +++ b/docs/IRIS.md @@ -12,6 +12,49 @@ things still stay out. 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-08 (later): a `Scroll` whose content grew asks to be drawn again + +Iris's phone: "when typing with the keyboard up and entering enough +newlines ... the text drops down close to the bottom and seems to ignore +the padding. If I close (and optionally reopen) the keyboard it seems to +fix itself." + +`Scroll::draw` offers its child **last** frame's content length rather +than measuring afresh, deliberately: an ordinary scroll tick then hands +the child the same box it already has, which is what makes a tick an O(1) +move instead of a redraw (LAYOUT.md sections 2 and 4). The comment there +said the lag "self-corrects the next frame". It does not, because nothing +asks for that frame: a keystroke dirties the *field*, the frame it +requests draws the field in a box one line short of its new text, and the +tree is clean afterwards -- so the stale placement is simply the last one +drawn. The composer's text is centred in its box, so a box one line short +put half a line past each end, and the caret's line box landed a full +12dp below the bar's inside edge, flush against its bottom. Closing the +keyboard rewrote the bar's inset, which dirtied it, which is why it +"fixed itself". + +Two halves to the fix, and the second is the general one: + +- **`Scroll::draw` calls `Painter::draw_again` when what it just measured + differs from what it offered** (by more than half a pixel). Costs one + extra draw when the content's length actually changes, and nothing on + an ordinary scroll tick, so the O(1)-move path is untouched. It + converges: the next draw offers the measured length and measures the + same thing again. +- **A frame that leaves anything dirty now asks for another frame**, on + both backends (`android::view`'s `post_frame_callback`, + `default`'s `request_redraw`). `draw_again` sets its mark *during* the + update, after the input path's own "does anything need redrawing?" + check has run, so before this nothing asked -- which quietly applied to + `List::clamp_to_content`'s use of the same mechanism too. + +Covered by `a_newline_leaves_the_caret_inside_the_composers_padding` +(layer 1, `transcript-fixture/tests/phone_screen.rs`), which fails on the +old code with the caret exactly on the bar's edge. `phone.rs` grew a +`--typed TEXT` argument beside `--message`: the two are different cases, +since one lays the composer out from scratch and the other grows one +already drawn, and only the second reproduces this. + ## 2026-09-08: what a cancel means, what a row's box is, and one fling for every scroll area Iris's second 2026-09-08 report, from the bench on her phone. Four items, diff --git a/iris/Cargo.lock b/iris/Cargo.lock index 34ad3ea..b6e76c3 100644 --- a/iris/Cargo.lock +++ b/iris/Cargo.lock @@ -3790,6 +3790,7 @@ dependencies = [ "iris", "log", "serde_json", + "tokio", "transcript-ui", "winit", ] diff --git a/iris/src/android/view.rs b/iris/src/android/view.rs index 39ce3d7..8752d56 100644 --- a/iris/src/android/view.rs +++ b/iris/src/android/view.rs @@ -518,7 +518,19 @@ impl IrisViewPeer { // A frame callback is one-shot, so an animation that wants // another frame has to say so every frame -- unlike `after_input`, // which only has to ask when input dirtied something. - if animating { + // `animating` is not the only thing a frame can leave + // unfinished: a widget that can only discover a correction to + // itself by laying out once asks for another draw with + // `Painter::draw_again` (`Scroll`, learning its content's real + // length; `List::clamp_to_content`). That mark is set *during* + // `update` above, after `after_input`'s own check has run, so + // without this nothing asks for the frame that applies it and + // the stale placement is the last one drawn. + let unfinished = { + let ui_state = self.state.android_state(); + self.render.needs_redraw(&ui_state.root, self.rsc.widgets()) + }; + if animating || unfinished { ctx.view.post_frame_callback(&mut ctx.env); } if crate::diagnostics::trace_enabled() { diff --git a/iris/src/default/mod.rs b/iris/src/default/mod.rs index b707ada..45b89c2 100644 --- a/iris/src/default/mod.rs +++ b/iris/src/default/mod.rs @@ -347,7 +347,17 @@ impl AppState for DefaultApp { let draw_start = std::time::Instant::now(); ui_state.renderer.draw(); crate::diagnostics::log_frame(render, frame_start, draw_start.elapsed(), animating); - if animating { + // `animating` is not the only thing a frame can leave + // unfinished: a widget that can only discover a + // correction to itself by laying out once asks for + // another draw with `Painter::draw_again` + // (`Scroll`, learning its content's real length; + // `List::clamp_to_content`). That mark is set *during* + // this update, so the input path's own check + // (`window_event` below) has already run and nothing + // else would ask -- leaving the stale placement as the + // last one drawn. + if animating || render.needs_redraw(&ui_state.root, rsc.widgets()) { ui_state.window.request_redraw(); } // I4 (RUST.md): only produces a `TreeUpdate` when the named diff --git a/iris/src/layout_tests.rs b/iris/src/layout_tests.rs index 0d5720b..73541bf 100644 --- a/iris/src/layout_tests.rs +++ b/iris/src/layout_tests.rs @@ -68,7 +68,14 @@ fn an_unchanged_frame_draws_and_rewrites_nothing() { render.resize((800.0, 20000.0)); render.update(&root, &mut rsc); - render.take_counters(); // discard the first, real draw + // Two, not one: the first offers `Scroll`'s content the container's + // own length as a placeholder (nothing has been measured yet) and + // `Scroll::draw` asks to be drawn again once it knows the real one, + // which the second update is. Only after that is the tree settled -- + // see `scrolling_moves_in_o1_without_a_redraw`'s own note on the + // same first draw. + render.update(&root, &mut rsc); + render.take_counters(); // discard the first, real draws render.update(&root, &mut rsc); let (draws, rewrites, moves, _shapes) = render.take_counters(); diff --git a/iris/src/widget/position/scroll.rs b/iris/src/widget/position/scroll.rs index 80451df..0457088 100644 --- a/iris/src/widget/position/scroll.rs +++ b/iris/src/widget/position/scroll.rs @@ -115,11 +115,39 @@ impl Widget for Scroll { // A child reporting `rel` means "this fraction of what I was // offered", and what it was offered is this scroll area -- so the // container, again, is what that resolves against. - self.content_len = Some( - used.axis(axis) - .apply_rest(painter.density()) - .to_abs(container_len), - ); + let measured = used + .axis(axis) + .apply_rest(painter.density()) + .to_abs(container_len); + self.content_len = Some(measured); + + // The content grew or shrank *this* frame, so the box it was just + // drawn in (`offered`, last frame's length) is the wrong one -- + // and the correction the comment above promises only happens if + // something draws this again. Nothing else will: an ordinary + // keystroke dirties the field, not this widget, and after that + // frame the tree is clean, so the stale placement is simply the + // last one drawn. + // + // What that looked like: every newline typed into the composer + // left the field drawn in a box one line short of its text, and + // since the text is centred in its box, it hung half a line past + // each end -- the caret on the new last line landing 31px below + // the box, flush against the bar's bottom edge with the 12dp + // padding eaten (Iris's phone, 2026-09-08; it "fixed itself" + // when the keyboard closed because the composer's inset rewrite + // dirtied the bar and forced exactly the redraw that is missing + // here). Costs one extra draw when the content's length actually + // changes, and nothing on an ordinary scroll tick, which is what + // keeps `draw`'s O(1)-move path above intact. Converges: the + // next draw offers `measured` and measures the same thing again. + // Half a pixel rather than exact inequality: a length that only + // differs in float noise is not a content change, and asking for + // a redraw on it would be the "redraws forever" case + // `Painter::draw_again` warns about. + if (measured - offered).abs() > 0.5 { + painter.draw_again(); + } // The **content's** size, not the container's. A parent that can // grow (the composer's bar) should hug the text until its own cap diff --git a/iris/transcript-fixture/Cargo.toml b/iris/transcript-fixture/Cargo.toml index 9732af9..525b285 100644 --- a/iris/transcript-fixture/Cargo.toml +++ b/iris/transcript-fixture/Cargo.toml @@ -22,6 +22,11 @@ serde_json = { version = "1", features = ["float_roundtrip"] } [dev-dependencies] winit = { workspace = true } +# `examples/phone.rs`'s `--typed` only: it spaces its insertions out over +# real frames, and a task spawned through `iris`'s own runtime +# (`iris/src/task.rs`) is where the sleep has to happen. Same version the +# workspace already pins for `iris-android-app`. +tokio = { workspace = true, features = ["time"] } # For the `iris::input`/`iris::frame` round-trip test: a capturing `log::Log` # to read back what `iris::diagnostics::log_frame`/`sense::log_input_event` # wrote, pinned to the same version `iris/Cargo.toml` already carries. diff --git a/iris/transcript-fixture/examples/phone.rs b/iris/transcript-fixture/examples/phone.rs index f480b94..53a0451 100644 --- a/iris/transcript-fixture/examples/phone.rs +++ b/iris/transcript-fixture/examples/phone.rs @@ -15,7 +15,11 @@ //! already in the composer, `\n` for a newline -- the composer's grown //! and overflowing states are otherwise unreachable here, since this //! window has no keyboard to type into (UI_RULES.md's "check the states -//! you can't see by default"). +//! you can't see by default"). `--typed TEXT` *enters* the same text +//! instead, one character per 100ms: laying the composer out from +//! scratch and growing one already on screen are different cases, and +//! only the second reproduced the caret landing in the bar's padding +//! (IRIS.md, 2026-09-08). //! //! No server: `transcript-fixture` embeds the transcript. Colour, //! spacing, type and anything a person has to *see* is answered here; @@ -52,6 +56,22 @@ fn message_argv() -> Option { None } +/// The `--typed TEXT` argument: the same text as `--message`, but +/// *entered* rather than preloaded -- one insertion per 100ms, into a +/// focused field, the way a person types. The two are different cases +/// for layout: `--message` is laid out from scratch on the first frame, +/// while this grows an already-drawn composer, which is the path +/// Iris's 2026-09-08 phone report is about. +fn typed_argv() -> Option { + let mut args = std::env::args().skip(1); + while let Some(arg) = args.next() { + if arg == "--typed" { + return Some(args.next()?.replace("\\n", "\n")); + } + } + None +} + fn main() { DefaultApp::::run(); } @@ -83,6 +103,25 @@ impl DefaultAppState for Client { if let Some(message) = message_argv() { opened.screen.composer.field.edit(rsc).set(&message); } + if let Some(text) = typed_argv() { + let field = opened.screen.composer.field; + let redraw = rsc.tasks.redraw_handle(); + rsc.spawn_task(async move |mut ctx| { + for ch in text.chars() { + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + ctx.update(move |state: &mut Client, rsc| { + state.set_focus(Some(field)); + let end = rsc[field].text().len(); + let mut edit = field.edit(rsc); + if edit.text.caret().is_none() { + edit.set_cursor_byte(end); + } + edit.insert(&ch.to_string()); + }); + redraw.request_redraw(); + } + }); + } if let Some(inset) = ime_argv() { opened.screen.composer.set_bottom_inset(rsc, inset); } diff --git a/iris/transcript-fixture/tests/phone_screen.rs b/iris/transcript-fixture/tests/phone_screen.rs index 872d373..c3e58b5 100644 --- a/iris/transcript-fixture/tests/phone_screen.rs +++ b/iris/transcript-fixture/tests/phone_screen.rs @@ -219,3 +219,63 @@ fn the_composer_sits_above_a_simulated_ime_inset() { closed - open ); } + +/// A newline typed into the composer must leave the caret inside the +/// bar's own padding, not flush against its bottom edge. +/// +/// Iris's phone, 2026-09-08: "when typing with the keyboard up and +/// entering enough newlines ... the text drops down close to the bottom +/// and seems to ignore the padding. If I close (and optionally reopen) +/// the keyboard it seems to fix itself." The cause was `Scroll::draw` +/// offering its child *last* frame's content length (deliberate, so an +/// ordinary scroll tick is an O(1) move): each newline drew the field in +/// a box one line short of its text, and since the text is centred in +/// its box it hung half a line past each end, putting the caret's line +/// box a full padding below the bar's inside edge. Nothing dirtied that +/// subtree again, so the stale placement was simply the last one drawn +/// -- until the keyboard closed and the inset rewrite forced a redraw, +/// which is the "fixes itself" half of the report. +#[test] +fn a_newline_leaves_the_caret_inside_the_composers_padding() { + let (mut h, screen) = opened(); + let height = h.size().y; + let ime = 1000.0; + screen.composer.set_bottom_inset(&mut h.rsc, ime); + h.frame(PHONE_FRAME_MS * 2); + + h.state.set_focus(Some(screen.composer.field)); + screen.composer.field.edit(&mut h.rsc).set_cursor_byte(0); + // Past `composer::MAX_LINES`, so the bar is capped and scrolling + // rather than still growing -- the state the report is about. + for _ in 0..12 { + screen.composer.field.edit(&mut h.rsc).insert("a\n"); + h.frame(PHONE_FRAME_MS); + } + // The frame `Scroll`'s `draw_again` asks for. On a device this is + // the `post_frame_callback`/`request_redraw` the backends make when + // an update leaves anything dirty; here the harness drives it. + h.frame(PHONE_FRAME_MS); + + // The caret is the last primitive `TextEdit::draw` emits. + let caret = { + let slot = *h + .render + .debug(h.rsc.widgets(), "Message") + .flat_map(|a| a.primitives.iter().map(|p| p.slot)) + .collect::>() + .last() + .expect("the focused field draws a caret"); + h.render.primitive_corners(slot, &h.rsc) + }; + // The bar sits directly on the IME, so its inside edge is one + // `FIELD_PAD_DP` above `height - ime`. Stated in pixels rather than + // read back from the composer, which is the thing under test. + let bar_bottom = height - ime; + let padding = 12.0 * PHONE_SCALE; + assert!( + caret.bot_right.y < bar_bottom - padding / 2.0, + "the caret is in the bar's bottom padding: it ends at {}, the bar's edge is {bar_bottom} \ + and its padding is {padding}px", + caret.bot_right.y, + ); +}