diff --git a/iris/examples/message_list.rs b/iris/examples/message_list.rs new file mode 100644 index 0000000..f8a303a --- /dev/null +++ b/iris/examples/message_list.rs @@ -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::::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: &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::(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, + _: Proxy, + ) -> 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 } + } +} diff --git a/iris/src/widget/list.rs b/iris/src/widget/list.rs index d19137c..63d1578 100644 --- a/iris/src/widget/list.rs +++ b/iris/src/widget/list.rs @@ -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, extents: HashMap, + /// 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, } 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 { 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 { 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 {