iris: a scroll area whose content grew asks to be drawn again
Iris's phone: typing newlines into the composer with the keyboard up dropped the caret flush against the bar's bottom edge, eating the 12dp padding, and closing the keyboard fixed it. `Scroll::draw` offers its child last frame's content length on purpose, so an ordinary scroll tick is an O(1) move rather than a redraw. The comment claimed the lag self-corrects on the next frame; nothing asked for that frame. A keystroke dirties the field, that frame draws it in a box one line short of its text, and the tree is clean afterwards -- so the stale placement is the last one drawn. The composer's text is centred in its box, so one line short hung half a line past each end and put the caret's line box a whole padding low. Closing the keyboard rewrote the bar's inset, dirtied it, and forced the missing redraw. `Scroll::draw` now calls `Painter::draw_again` when what it measured differs from what it offered, and a frame that leaves anything dirty asks for another frame on both backends -- `draw_again` sets its mark during the update, after the input path's own check has run, so nothing asked before this (which applied to `List::clamp_to_content` too). Verified at layer 1 (the new test fails on the old code with the caret exactly on the bar's edge) and on the emulator: the caret's bottom moved from 1535 -- the bar's own bottom edge -- to 1509, 26px inside a 31px padding, the remainder being parley's line box overhanging its line height. `phone.rs` grew `--typed TEXT`, which enters text over frames rather than preloading it; only that reproduces this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
1a9655414e
commit
ba57086361
10 files changed
+229
-9
No files matched your search
@@ -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
|
for iris API changes); this file is only the summary. Newest first. Items
|
||||||
marked **DEFERRED** are ones the agent chose not to decide alone.
|
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)
|
## 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
|
At Iris's request. RUST.md's "Every crate to its latest version" box has
|
||||||
|
|||||||
@@ -12,6 +12,49 @@ things still stay out.
|
|||||||
An entry gives the date, what changed, why, and a short before/after where
|
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.
|
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
|
## 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,
|
Iris's second 2026-09-08 report, from the bench on her phone. Four items,
|
||||||
|
|||||||
Generated
+1
@@ -3790,6 +3790,7 @@ dependencies = [
|
|||||||
"iris",
|
"iris",
|
||||||
"log",
|
"log",
|
||||||
"serde_json",
|
"serde_json",
|
||||||
|
"tokio",
|
||||||
"transcript-ui",
|
"transcript-ui",
|
||||||
"winit",
|
"winit",
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -518,7 +518,19 @@ impl<State: AndroidAppState> IrisViewPeer<State> {
|
|||||||
// A frame callback is one-shot, so an animation that wants
|
// A frame callback is one-shot, so an animation that wants
|
||||||
// another frame has to say so every frame -- unlike `after_input`,
|
// another frame has to say so every frame -- unlike `after_input`,
|
||||||
// which only has to ask when input dirtied something.
|
// 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);
|
ctx.view.post_frame_callback(&mut ctx.env);
|
||||||
}
|
}
|
||||||
if crate::diagnostics::trace_enabled() {
|
if crate::diagnostics::trace_enabled() {
|
||||||
|
|||||||
+11
-1
@@ -347,7 +347,17 @@ impl<State: DefaultAppState> AppState for DefaultApp<State> {
|
|||||||
let draw_start = std::time::Instant::now();
|
let draw_start = std::time::Instant::now();
|
||||||
ui_state.renderer.draw();
|
ui_state.renderer.draw();
|
||||||
crate::diagnostics::log_frame(render, frame_start, draw_start.elapsed(), animating);
|
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();
|
ui_state.window.request_redraw();
|
||||||
}
|
}
|
||||||
// I4 (RUST.md): only produces a `TreeUpdate` when the named
|
// I4 (RUST.md): only produces a `TreeUpdate` when the named
|
||||||
|
|||||||
@@ -68,7 +68,14 @@ fn an_unchanged_frame_draws_and_rewrites_nothing() {
|
|||||||
render.resize((800.0, 20000.0));
|
render.resize((800.0, 20000.0));
|
||||||
|
|
||||||
render.update(&root, &mut rsc);
|
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);
|
render.update(&root, &mut rsc);
|
||||||
let (draws, rewrites, moves, _shapes) = render.take_counters();
|
let (draws, rewrites, moves, _shapes) = render.take_counters();
|
||||||
|
|||||||
@@ -115,11 +115,39 @@ impl Widget for Scroll {
|
|||||||
// A child reporting `rel` means "this fraction of what I was
|
// A child reporting `rel` means "this fraction of what I was
|
||||||
// offered", and what it was offered is this scroll area -- so the
|
// offered", and what it was offered is this scroll area -- so the
|
||||||
// container, again, is what that resolves against.
|
// container, again, is what that resolves against.
|
||||||
self.content_len = Some(
|
let measured = used
|
||||||
used.axis(axis)
|
.axis(axis)
|
||||||
.apply_rest(painter.density())
|
.apply_rest(painter.density())
|
||||||
.to_abs(container_len),
|
.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
|
// 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
|
// grow (the composer's bar) should hug the text until its own cap
|
||||||
|
|||||||
@@ -22,6 +22,11 @@ serde_json = { version = "1", features = ["float_roundtrip"] }
|
|||||||
|
|
||||||
[dev-dependencies]
|
[dev-dependencies]
|
||||||
winit = { workspace = true }
|
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`
|
# 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`
|
# to read back what `iris::diagnostics::log_frame`/`sense::log_input_event`
|
||||||
# wrote, pinned to the same version `iris/Cargo.toml` already carries.
|
# wrote, pinned to the same version `iris/Cargo.toml` already carries.
|
||||||
|
|||||||
@@ -15,7 +15,11 @@
|
|||||||
//! already in the composer, `\n` for a newline -- the composer's grown
|
//! already in the composer, `\n` for a newline -- the composer's grown
|
||||||
//! and overflowing states are otherwise unreachable here, since this
|
//! and overflowing states are otherwise unreachable here, since this
|
||||||
//! window has no keyboard to type into (UI_RULES.md's "check the states
|
//! 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,
|
//! No server: `transcript-fixture` embeds the transcript. Colour,
|
||||||
//! spacing, type and anything a person has to *see* is answered here;
|
//! spacing, type and anything a person has to *see* is answered here;
|
||||||
@@ -52,6 +56,22 @@ fn message_argv() -> Option<String> {
|
|||||||
None
|
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<String> {
|
||||||
|
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() {
|
fn main() {
|
||||||
DefaultApp::<Client>::run();
|
DefaultApp::<Client>::run();
|
||||||
}
|
}
|
||||||
@@ -83,6 +103,25 @@ impl DefaultAppState for Client {
|
|||||||
if let Some(message) = message_argv() {
|
if let Some(message) = message_argv() {
|
||||||
opened.screen.composer.field.edit(rsc).set(&message);
|
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() {
|
if let Some(inset) = ime_argv() {
|
||||||
opened.screen.composer.set_bottom_inset(rsc, inset);
|
opened.screen.composer.set_bottom_inset(rsc, inset);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -219,3 +219,63 @@ fn the_composer_sits_above_a_simulated_ime_inset() {
|
|||||||
closed - open
|
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::<Vec<_>>()
|
||||||
|
.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,
|
||||||
|
);
|
||||||
|
}
|
||||||
Reference in new issue
Block a user