iris/ai-app
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

iris: fix List placing a fill-shaped background at its oversized measurement size

Browse Source 
Building the I3 example (800 rows, some with images, styled with
.background(rect(tint))) surfaced a real bug: place()'s Bottom-known
branch measured a row at an oversized, fixed-size region and moved it
into its final box with reposition -- a pure translation. That is
correct for wrapped text, whose reported height doesn't depend on the
height it was offered, but Rect (used for every row's background) is
is_size_independent because it fills *whatever region it is given*,
so it painted at the oversized size and reposition never shrank it
back down. The screenshot showed one oversized tinted rectangle
covering the whole visible window instead of per-row backgrounds.

Fixed by caching each row's height once measured and placing an
already-measured row directly at its exact box (one widget_within/
reposition pass, same as any known-size placement) instead of
re-measuring every frame. A first-ever appearance still pays a
two-draw measurement (draw_twice), and a row whose real height
changed since it was cached is corrected the same frame it redraws
(not a one-frame lag) via an explicit reposition when the two
disagree. Steady-state scroll cost is unaffected: an unchanged row's
single placement call still hits draw_inner's existing skip-or-move
fast path.

Also fixes repair_anchor unconditionally re-snapping a bottom-anchored
list's offset to the viewport's edge on every frame snap_end was true
-- which discarded a live scroll() call the moment it ran, since
snap_end is only recomputed at the end of a layout pass and so still
read true from before the scroll. Now only re-snaps when the viewport
itself actually resized (tracked via last_viewport_len).

Added a_fill_shaped_background_is_not_left_oversized, a direct
regression test for the background bug (checks the background rect's
own painted pixel size, not just the row's reported extent, which was
already correct). cargo test -p iris (26 passed), clippy --all-targets
and --benches --release, fmt --all -- --check all clean. Rebenched:
all five scenarios still flat across N = 100/1,000/10,000 (numbers in
RUST.md's I3 box). Visually verified via
run-headless.sh message_list --shot, cropped with a throwaway PNG
decoder since no image tooling is installed here.

Co-Authored-By: Claude Sonnet <noreply@anthropic.com>
This commit is contained in:
irisandClaude Sonnet committed 2026-09-05 06:26:02 -04:00
1 parent 03da47e550
commit e898370bf4
2 files changed
+314 -42
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

iris/examples/message_list.rs
+116
View File
@@ -0,0 +1,116 @@
//! RUST.md's I3: `iris::widget::List` with 800 rows of varied-length
//! wrapped text, one in twelve carrying a small image, scrollable with the
//! mouse wheel. Run headless with `iris/run-headless.sh message_list --shot
//! /tmp/message_list.png` -- there is no display on this machine, so that
//! is the only way to see it rendered; `run-tests.sh`/`cargo test` never
//! touch this file.
//!
//! Rows alternate two background tints so a screenshot can show the
//! boundary between adjacent rows even where the text itself wraps to a
//! different number of lines -- exactly the "variable-height rows" I3
//! asks for, and the thing a virtualised list gets wrong first if it is
//! wrong at all (a gap, an overlap, a row the wrong colour). This example
//! is also what found `List::place`'s oversized-background bug (see
//! list.rs's module doc and its `a_fill_shaped_background_is_not_left_
//! oversized` test) -- a plain unit test could have (and now does) catch
//! it directly, but it was this screenshot rendering as a single blank
//! tinted rectangle that pointed at it first.
use iris::prelude::*;
use winit::{dpi::LogicalSize, window::WindowAttributes};
fn main() {
DefaultApp::<State>::run();
}
#[derive(DefaultUiState)]
struct State {
ui_state: DefaultUiState,
}
const ROWS: usize = 800;
const IMAGE_EVERY: usize = 12;
/// Repeats a short sentence a varying number of times per row so real
/// wrapping happens at every row height from one line to several, rather
/// than every row being identically tall (which would render correctly
/// even with a broken height measurement).
fn row_text(i: usize) -> String {
const SENTENCE: &str =
"Iris lays out this row once and moves it on scroll, never re-laying it out. ";
let repeats = 1 + (i * 7) % 5;
format!("Message {i}: {}", SENTENCE.repeat(repeats))
}
/// A small solid-colour square standing in for a real decoded image --
/// what matters for I3 is that a row can carry an `Image` widget at all,
/// not what the picture shows.
fn row_image(i: usize) -> image::DynamicImage {
let hue = ((i * 47) % 255) as u8;
image::RgbaImage::from_pixel(48, 48, image::Rgba([hue, 128, 255 - hue, 255])).into()
}
fn build_row<Rsc: UiRsc + 'static>(rsc: &mut Rsc, i: usize) -> StrongWidget {
let tint = if i.is_multiple_of(2) {
Color::rgb(120, 130, 170)
} else {
Color::rgb(70, 80, 140)
};
let text_color = Color::BLACK;
if i.is_multiple_of(IMAGE_EVERY) {
let text = wtext(row_text(i))
.wrap(true)
.color(text_color)
.add_strong(rsc)
.any();
let img = image::<Rsc>(row_image(i))(rsc);
let img = rsc.widgets_mut().add_strong(img).any();
let mut span = Span::empty(Dir::DOWN);
span.push(text);
span.push(img);
span.pad(8.0).background(rect(tint)).add_strong(rsc).any()
} else {
wtext(row_text(i))
.wrap(true)
.color(text_color)
.pad(8.0)
.background(rect(tint))
.add_strong(rsc)
.any()
}
}
impl DefaultAppState for State {
// A phone-plausible portrait shape (the transcript screen this is
// standing in for). The tiling headless compositor `run-headless.sh`
// uses ignores this and fills its own 1920x1200 output regardless, but
// it's a correct hint for any other backend (a real window manager, or
// android-view) and costs nothing to state.
fn window_attributes() -> WindowAttributes {
WindowAttributes::default().with_inner_size(LogicalSize::new(420.0, 900.0))
}
fn new(
mut ui_state: DefaultUiState,
rsc: &mut DefaultRsc<Self>,
_: Proxy<Self::Event>,
) -> Self {
let mut list = List::new(Axis::Y);
for i in 0..ROWS {
let row = build_row(rsc, i);
list.push_back(ListRow::new(i as u64, row));
}
let root = list
.on(CursorSense::Scroll, |ctx, rsc| {
let delta = ctx.data.scroll_delta.y * 50.0;
ctx.widget(rsc).scroll(delta);
})
.masked()
.background(rect(Color::WHITE))
.add_strong(rsc);
ui_state.set_root(root.any());
Self { ui_state }
}
}
iris/src/widget/list.rs
+198 -42
View File
@@ -83,16 +83,22 @@
//! is written for the frame, not a correction applied to an already-drawn
//! wrong frame.
//!
//! **A row's height is cached by key once measured**, and reused directly
//! (one `widget_within` at the exact box, no re-measurement) on every later
//! placement of that row -- not merely an optimisation: see `place`'s doc
//! for why a row that fills whatever it is offered (a `.background(rect
//! (...))`) needs this to ever be placed at the right size at all, and why
//! reusing `draw_twice` every frame instead would defeat `draw_inner`'s own
//! skip-or-move caching. Only a row's first-ever appearance pays the
//! two-draw measurement; nothing here estimates a height for an off-screen
//! row that has never been measured, so this stays independent of how many
//! rows exist outside the loaded window.
//!
//! **What is deliberately not solved here.** No overscroll clamping: a
//! `scroll()` past the first or last row leaves a gap rather than rubber-
//! banding back (mirrors `Scroll`'s own documented one-frame-lag
//! tolerance in LAYOUT.md, just not even auto-corrected -- there is
//! nothing to measure "how much content is left" without walking it). No
//! height estimation for unmeasured, off-screen rows: the walk only ever
//! measures the row it is about to place, one at a time outward from the
//! anchor, so there is no average-height table to keep consistent -- the
//! simplest thing that works, and it is what keeps every operation here
//! independent of how many rows exist off-screen.
//! nothing to measure "how much content is left" without walking it).
use crate::prelude::*;
use iris_core::util::HashMap;
@@ -188,8 +194,25 @@ pub struct List {
/// mirroring `Scroll::snap_end`.
snap_end: bool,
viewport_len: f32,
/// `viewport_len` as of the *previous* draw -- what `repair_anchor`
/// compares against to tell "the container was actually resized" from
/// "an ordinary frame where `snap_end` merely hasn't been recomputed
/// since a `scroll()` call yet." Without this distinction,
/// `repair_anchor` would re-snap a deliberate scroll away from the
/// bottom back to flush on the very next frame, since `snap_end` is
/// only recomputed at the *end* of a layout pass and so still reads
/// `true` (from before the scroll) the next time `repair_anchor` runs.
last_viewport_len: f32,
pending_tap: Option<f32>,
extents: HashMap<RowKey, RowExtent>,
/// Each row's height as of its last real draw, kept across frames so
/// an already-measured row is placed directly at its exact box next
/// time (one `widget_within`, no oversized measurement pass) --
/// see `place`'s doc for why a fresh measurement can't be skipped
/// merely by translating an already-drawn primitive. Pruned when a
/// row is evicted (`pop_front`/`pop_back`) so this cannot grow past
/// however many rows are currently loaded.
heights: HashMap<RowKey, f32>,
}
impl List {
@@ -202,8 +225,10 @@ impl List {
anchor: None,
snap_end: true,
viewport_len: 0.0,
last_viewport_len: 0.0,
pending_tap: None,
extents: HashMap::default(),
heights: HashMap::default(),
}
}
@@ -252,7 +277,7 @@ impl List {
/// answer than "wherever this widget's default now is."
pub fn pop_front(&mut self) -> Option<ListRow> {
let popped = self.items.pop_front();
if popped.is_some() {
if let Some(row) = &popped {
if let Some(a) = &mut self.anchor {
if a.slot == 0 {
self.anchor = None;
@@ -260,6 +285,7 @@ impl List {
a.slot -= 1;
}
}
self.heights.remove(&row.key);
self.extents.clear();
}
popped
@@ -269,12 +295,13 @@ impl List {
pub fn pop_back(&mut self) -> Option<ListRow> {
let old_len = self.items.len() as isize;
let popped = self.items.pop_back();
if popped.is_some() {
if let Some(row) = &popped {
if let Some(a) = &mut self.anchor
&& a.slot == old_len - 1
{
self.anchor = None;
}
self.heights.remove(&row.key);
self.extents.clear();
}
popped
@@ -440,9 +467,10 @@ impl List {
if let Some(a) = self.anchor
&& self.slot_exists(a.slot)
{
if self.snap_end {
if self.snap_end && self.viewport_len != self.last_viewport_len {
self.anchor.as_mut().unwrap().offset = self.viewport_len;
}
self.last_viewport_len = self.viewport_len;
return;
}
let len = self.items.len() as isize;
@@ -506,31 +534,97 @@ impl List {
UiRegion::from_axis(axis, span, UiSpan::FULL)
}
/// Convert a `draw`-returned `Len` (possibly `rel`/`rest`, though every
/// row in practice reports a plain `abs` height) into pixels along
/// `axis`, the same formula `Scroll::draw` uses for its own content
/// length.
fn resolve_len_px(painter: &Painter, axis: Axis, len: Len) -> f32 {
let output_len = painter.output_size().axis(axis);
let container_len = painter.region().axis(axis).len();
len.apply_rest()
.within_len(container_len)
.to_abs(output_len)
}
/// Place one slot (a real row or a sentinel) per `placement`, caching
/// its resolved extent for the next frame's `note_tap` resolution, and
/// return its resolved `(leading, trailing)` edges in viewport pixels.
/// its resolved extent (for the next frame's `note_tap` resolution) and
/// height (for its own next placement, see below), and return its
/// resolved `(leading, trailing)` edges in viewport pixels.
///
/// A row already measured on some earlier frame is placed directly at
/// its cached height's exact box -- one `widget_within`/`reposition`
/// pass, the same as any other widget placed by an already-known
/// region. A row seen for the first time has no cached height to place
/// it *at*, so it is measured first (an oversized, fixed-size region)
/// and then drawn a *second* time at the tight box that measurement
/// implies, via `Painter::draw_twice` -- not `reposition` (a pure
/// translation, no resize). This distinction is required, not just an
/// optimisation: a row is not always plain wrapped text --
/// `.background(rect(tint))` is an ordinary way to style one, and
/// `Rect::draw` is `is_size_independent` specifically because it fills
/// *whatever region it is given* (`Size::REST`, see `rect.rs`).
/// Measuring such a row at the oversized box has it paint an oversized
/// rect there; `reposition` only ever writes an offset, never a size,
/// so an every-frame reposition-only scheme would leave that primitive
/// oversized forever. Using `draw_twice` for *every* frame would fix
/// that but break the opposite property: its two calls use two
/// different regions, so whichever one `ActiveData.region` ends up
/// holding always disagrees with the *next* frame's first call,
/// forcing a real redraw every single frame instead of the cheap
/// skip-or-move `draw_inner` already provides for an unchanged or
/// merely-translated widget. Caching the height once measured is what
/// lets an already-seen row go back to that cheap path while a
/// first-seen one still gets a correctly-sized initial paint. A stale
/// cached height (the row's content changed height since) briefly
/// offers the wrong box; the height recorded from what it *actually*
/// reports this frame corrects it starting next frame -- the same
/// one-frame lag `Scroll`'s own content-length cache accepts, per
/// LAYOUT.md.
fn place(&mut self, painter: &mut Painter, slot: isize, placement: Placement) -> (f32, f32) {
let axis = self.axis;
let (top, bottom) = match placement {
Placement::Top(top) => {
let region = Self::abs_region(axis, top, top + GENEROUS_PADDING);
let output_len = painter.output_size().axis(axis);
let container_len = painter.region().axis(axis).len();
let resolve = move |used: Size| -> f32 {
used.axis(axis)
.apply_rest()
.within_len(container_len)
.to_abs(output_len)
};
let key = self.slot_key(slot);
let cached = key.and_then(|k| self.heights.get(&k).copied());
let (top, bottom, height) = match (placement, cached) {
(Placement::Top(top), Some(h)) => {
// Offered a box sized to the *cached* height (cheap to
// compare against last frame's offer, see `place`'s doc),
// but the returned/recorded height comes from what this
// draw actually reported -- if the row's real content grew
// since it was cached (and was therefore redrawn: an
// unchanged widget never disagrees with its own cache),
// this frame already reflects the new size rather than
// waiting a frame to self-correct.
let region = Self::abs_region(axis, top, top + h);
let used = painter.widget_within(self.slot_widget(slot), region);
let h = Self::resolve_len_px(painter, axis, used.axis(axis));
(top, top + h)
let height = resolve(used);
(top, top + height, height)
}
Placement::Bottom(bottom) => {
(Placement::Top(top), None) => {
let first = Self::abs_region(axis, top, top + GENEROUS_PADDING);
let mut height = 0.0;
painter.draw_twice(self.slot_widget(slot), first, |used| {
height = resolve(used);
Self::abs_region(axis, top, top + height)
});
(top, top + height, height)
}
(Placement::Bottom(bottom), Some(h)) => {
let region = Self::abs_region(axis, bottom - h, bottom);
let used = painter.widget_within(self.slot_widget(slot), region);
let height = resolve(used);
if height != h {
// The row's real height changed since it was cached
// (and was therefore redrawn -- an unchanged widget
// never disagrees with its own cache). It painted
// anchored at the *offered* box's leading edge
// (`bottom - h`, per every widget in this crate's
// top-left-anchoring convention), not where its true
// height means its bottom edge should be; correct with
// an O(1) reposition, `Aligned`'s own trick for this
// exact "learned a size after already drawing" case.
let corrected = Self::abs_region(axis, bottom - height, bottom);
painter.reposition(self.slot_widget(slot), corrected);
}
(bottom - height, bottom, height)
}
(Placement::Bottom(bottom), None) => {
// Measured at a fixed, zero-anchored region rather than
// `painter.region()` (the list's *actual* offered box):
// using the real box would make the measurement's offered
@@ -538,21 +632,19 @@ impl List {
// growing taller (the input-box case) would look like a
// resize to every bottom-known row and force a full
// redraw of each -- despite a row's content depending
// only on width. A region fixed at `[0, GENEROUS_PADDING]`
// is identical frame to frame regardless of what else on
// screen changed, so an unchanged row hits `draw_inner`'s
// exact-match skip (LAYOUT.md's caching section) instead.
let region = Self::abs_region(axis, 0.0, GENEROUS_PADDING);
let used = painter.widget_within(self.slot_widget(slot), region);
let h = Self::resolve_len_px(painter, axis, used.axis(axis));
let top = bottom - h;
let region = Self::abs_region(axis, top, bottom);
painter.reposition(self.slot_widget(slot), region);
(top, bottom)
// only on width.
let first = Self::abs_region(axis, 0.0, GENEROUS_PADDING);
let mut height = 0.0;
painter.draw_twice(self.slot_widget(slot), first, |used| {
height = resolve(used);
Self::abs_region(axis, bottom - height, bottom)
});
(bottom - height, bottom, height)
}
};
if let Some(key) = self.slot_key(slot) {
self.extents.insert(key, RowExtent { slot, top, bottom });
if let Some(k) = key {
self.heights.insert(k, height);
self.extents.insert(k, RowExtent { slot, top, bottom });
}
(top, bottom)
}
@@ -701,6 +793,70 @@ mod tests {
assert!(!list_ref.extents.contains_key(&1));
}
/// A row shaped like the ordinary `.background(rect(tint))` idiom:
/// a `Stack` whose first child is a `Rect` (`is_size_independent`,
/// fills whatever region it is given -- see `rect.rs`) and whose
/// second (the one `StackSize::Child` reports as the row's own size)
/// is a `Sized`-wrapped `Rect` of the given height. Returns the
/// background rect's own id (to check what it actually painted at)
/// alongside the row widget.
fn background_styled_row(rsc: &mut TestRsc, height: f32) -> (WidgetId, StrongWidget) {
let bg = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE));
let bg_id = bg.id();
let fg_rect = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE));
let fg = rsc.ui.widgets.add_strong(Sized {
inner: fg_rect.any(),
x: None,
y: Some(Len::abs(height)),
});
let stack = Stack {
children: vec![bg.any(), fg.any()],
size: StackSize::Child(1),
};
(bg_id, rsc.ui.widgets.add_strong(stack).any())
}
#[test]
fn a_fill_shaped_background_is_not_left_oversized() {
// Regression test for a real bug found building the I3 example:
// `place`'s Bottom-known branch used to measure a row at an
// oversized, fixed-size region and `reposition` (a pure
// translation) it into its final box. A row's own natural height
// is independent of that oversized offer (true for wrapped text),
// but a `Rect` background is *defined* to fill whatever it is
// given -- so it painted at the oversized size, and moving it
// afterward never shrank it back down. Only visible by checking
// what the background rect's own primitive covers, not the row's
// reported extent (which was already correct, since it comes from
// the *foreground* child).
let mut rsc = TestRsc {
ui: UiData::default(),
};
let mut list = List::new(Axis::Y);
let mut bg_ids = Vec::new();
for key in 0..5u64 {
let (bg_id, row) = background_styled_row(&mut rsc, 20.0);
bg_ids.push(bg_id);
list.push_back(ListRow::new(key, row));
}
let root = rsc.ui.widgets.add_strong(list).any();
let mut render = UiRenderState::new();
render.resize((100.0, 100.0));
render.update(&root, &mut rsc);
for &bg_id in &bg_ids {
let region = render.active[&bg_id].region;
let px = region.to_px((100.0, 100.0).into());
let height = px.size().y;
assert!(
(height - 20.0).abs() < 0.5,
"background rect should be exactly the row's height (20px), got {height}px \
-- an oversized measurement region leaking through would show as ~100000px"
);
}
}
#[test]
fn insert_above_anchor_is_o1_and_does_not_move_visible_rows() {
let mut rsc = TestRsc {