diff --git a/docs/IRIS.md b/docs/IRIS.md index 412b07c..177b6d5 100644 --- a/docs/IRIS.md +++ b/docs/IRIS.md @@ -16,18 +16,24 @@ it helps judge the change without the session that made it. Newest first. Four things you asked for, in one change. -**A lazy span no longer cares about masks.** It used to assert that +**A lazy span knows nothing about masks.** It used to assert that something around it had called `.masked()` and refuse to draw otherwise, which is why a plain full-screen list -- the benchmark, any simple app -- -panicked on its second line. Your question was the right one: it cared -only because it draws a row straddling an edge *in full* (virtualisation -decides which rows, never how much of one) and relied on somebody else to -cut off the overhang. It clips itself to the box it was offered now, so a -caller places it like any other widget. That is also strictly stronger -than the assert was: a mask *larger* than the list's box satisfied -`is_masked` and let the overhang through anyway, which is the fault the -assert was written for. The transcript's own `.masked()` wrapper is gone -with it. +panicked on its second line. What it does now is only the part that is +its own: it *culls*, so a row entirely outside the box it was offered is +never drawn, and a row straddling an edge is still drawn in full, because +virtualisation decides which rows and never how much of one. Whoever +wants that overhang cut off adds `.masked()`, exactly as whoever wants +scrolling adds `.scrollable()` -- your words: "masking should be opt in". +The transcript opts in, because it is a list under a header bar; a +full-screen list does not, and the widget has no business assuming either. + +I got this wrong once on the way: the first version had the span set a +mask of *itself*. That fixes the panic and is still the widget deciding +something that is not its to decide -- a caller already clipped by +something bigger ends up double-masked, and one that wants the overhang +has no way to say so. Worth recording as the shape to avoid, since it +looks like the tidy answer. **Everything on the transcript screen is capped now.** One rule in one place -- `client_core::text_cap`, mirrored as `TextCap.kt` with the same diff --git a/iris/src/widget/position/lazy_span.rs b/iris/src/widget/position/lazy_span.rs index 7d233c7..b0d0298 100644 --- a/iris/src/widget/position/lazy_span.rs +++ b/iris/src/widget/position/lazy_span.rs @@ -11,25 +11,31 @@ //! //! ## Design //! -//! **It clips itself to the region it is drawn in.** A row that straddles -//! either edge is drawn in *full* -- virtualisation decides which rows are -//! drawn, never how much of one -- so the overhang past this list's box -//! has to be cut off, and the box it is cut to is the one the list was -//! offered. `draw` sets that clip itself, so **a caller places a lazy span -//! the way it places any other widget** and never has to know a mask is -//! involved. +//! **It knows nothing about masks.** What it does is *cull*: a row that +//! falls entirely outside the region this widget was offered is never +//! drawn (`intersects_viewport`). A row that *straddles* an edge is drawn +//! in full, because virtualisation decides which rows are drawn and never +//! how much of one -- so the overhang past this list's box reaches the +//! screen unless something clips it, and clipping is `.masked()`, which +//! the caller adds when it wants one. Iris, 2026-09-08: **"Why does the +//! mask matter at all. If you want a mask then you add `.masked()`. It +//! should just prevent rows that aren't in its region at all from drawing +//! ... Just like the opt in scrollable, masking should be opt in."** //! -//! It used to be the caller's job, enforced by asserting on -//! `Painter::is_masked` and refusing to draw otherwise -- wrong twice -//! over. An ordinary full-screen list (every benchmark, every simple app) -//! panicked for want of ceremony that would have changed nothing on -//! screen; and the case that actually bites still passed the check, since -//! a mask *larger* than the list's box satisfies `is_masked` and lets the -//! overhang through anyway. That is the fault it was written for: the -//! transcript panned to its top edge, drawing code through the header bar -//! above it on Iris's phone (docs/IRIS_TODO.md, 2026-09-07). Clipping to -//! its own region cannot get that wrong. Iris, 2026-09-08: "why does it -//! care about mask at all?" +//! Two shapes this file went through before that, both worse. It asserted +//! `Painter::is_masked` and refused to draw otherwise, which made an +//! ordinary full-screen list -- every benchmark, every simple app -- +//! panic for want of ceremony it did not need; and the case that actually +//! bites passed the check anyway, since a mask *larger* than the list's +//! box satisfies `is_masked` while still letting the overhang through. +//! Then it set a mask of its own, which is this widget deciding something +//! that is not its to decide: a caller that wants the overhang (or that +//! is already clipped by something bigger) has no way to say so, and the +//! transcript ended up double-masked. +//! +//! The fault a caller is opting *out* of, when it leaves `.masked()` off, +//! is the transcript panned to its top edge drawing code through the +//! header bar above it, on Iris's phone (docs/IRIS_TODO.md, 2026-09-07). //! //! **Rows are keyed by a `u64` (`RowKey`), not a generic type.** Every real //! row source in this codebase (a transcript's monotonic sequence number, a @@ -1249,10 +1255,6 @@ impl Widget for LazySpan { fn draw(&mut self, painter: &mut Painter) -> Size { let axis = self.dir.axis; - // The clip is this list's own box -- see the module doc. Set here - // rather than required of the caller, so that placing a lazy span - // is placing a widget and nothing else. - painter.set_mask(painter.region()); let output_len = painter.output_size().axis(axis); self.viewport_len = painter.region().axis(axis).len().to_abs(output_len); diff --git a/iris/transcript-fixture/tests/top_edge.rs b/iris/transcript-fixture/tests/top_edge.rs index 2b55564..08d6827 100644 --- a/iris/transcript-fixture/tests/top_edge.rs +++ b/iris/transcript-fixture/tests/top_edge.rs @@ -114,20 +114,19 @@ fn the_row_across_the_top_edge_is_drawn() { /// mask -- with none, the phone drew `version = "0.1.0"` behind the "Run /// benchmark" button. /// -/// The clip is the list's **own** mask (`own_mask`), not one it inherited: -/// a `LazySpan` clips itself to the box it is offered, so nothing around -/// it has to (its module doc, 2026-09-08). Reading `mask` -- what it -/// inherited -- is what this asserted while the caller supplied the clip, -/// and it is now `NONE` for a list nothing else wraps. +/// The clip is one the screen **opted into** (`build_tree`'s `.masked()`), +/// so this reads the mask the list *inherited*. A `LazySpan` sets none of +/// its own -- masking is opt-in, like scrolling (Iris, 2026-09-08) -- so +/// this is also the test that the transcript is still asking for one. #[test] fn the_list_is_clipped_to_its_own_box() { let (h, screen) = opened(); let active = h.render.active.get(&screen.list.id()).expect("drawn"); assert!( - active.own_mask != MaskIdx::NONE, + active.mask != MaskIdx::NONE, "the transcript's list is drawn with nothing clipping it", ); - let clip = h.render.mask_region(active.own_mask, &h.rsc); + let clip = h.render.mask_region(active.mask, &h.rsc); let list = list_box(&h, &screen); assert!( clip.top_left.y >= list.top_left.y - 0.5 && clip.bot_right.y <= list.bot_right.y + 0.5, @@ -151,11 +150,11 @@ fn the_list_is_clipped_to_its_own_box() { for row in rows { for prim in primitives_under(&h, row) { assert!( - mask_chain(&h, prim).contains(&active.own_mask), + mask_chain(&h, prim).contains(&active.mask), "a primitive of row {row:?} clips to {:?}, a chain that never reaches the list's \ own mask {:?}", mask_chain(&h, prim), - active.own_mask, + active.mask, ); checked += 1; } diff --git a/iris/transcript-ui/src/lib.rs b/iris/transcript-ui/src/lib.rs index 39ab13f..0aaf6e7 100644 --- a/iris/transcript-ui/src/lib.rs +++ b/iris/transcript-ui/src/lib.rs @@ -448,12 +448,16 @@ where let (composer, composer_bar) = composer::build_composer(rsc); - // No `.masked()` around the list: a `LazySpan` clips itself to the box - // it is offered (its module doc), which is the clip this used to - // supply -- the one that keeps the straddling top row off the header - // bar above it (docs/IRIS_TODO.md, 2026-09-07: "code and a paragraph - // visible behind Run benchmark"). - let tree = (list.width(rest(1)).height(rest(1)), composer_bar) + // `.masked()`, opted into here rather than done by the list: a + // `LazySpan` culls the rows outside its box but draws a *straddling* + // one in full, so without a clip the top of that row is drawn above + // the list -- through whatever the app put there, which on the phone + // is the header bar (docs/IRIS_TODO.md, 2026-09-07: "code and a + // paragraph visible behind Run benchmark"). This screen is a list + // under a header, so this screen wants the clip; a full-screen list + // does not, and the widget is right not to assume either + // (`lazy_span.rs`'s module doc). + let tree = (list.width(rest(1)).height(rest(1)).masked(), composer_bar) .span(Dir::DOWN) .add_strong(rsc) .any();