iris: masking is opt in, and a LazySpan only culls
Iris, correcting the previous commit: "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." So `LazySpan` sets no mask. It culls -- a row entirely outside the box it was offered is never drawn, which `intersects_viewport` already did -- and draws a straddling row in full, because virtualisation decides which rows and never how much of one. Cutting off that overhang is `.masked()`, added by whoever wants it. The transcript wants it (it is a list under a header bar) and opts back in; the benchmark does not and needs no ceremony. `top_edge.rs` goes back to reading the mask the list *inherited*, which is now also the test that the transcript is still asking for one. The previous commit had the span mask itself, which fixes the panic and is still the widget deciding what is not its to decide.
This commit is contained in:
1 parent
afbc2ad132
commit
e9a6562dc6
4 files changed
+58
-47
No files matched your search
+16
-10
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in new issue
Block a user