From d507ae4c96df40a9002a07ab59fe8ea2155f2c6c Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Mon, 7 Sep 2026 16:05:13 -0400 Subject: [PATCH] iris-core: masks nest instead of aborting, and a widget can ask to be drawn again `Painter::set_mask` refused a widget any mask of its own once an ancestor had set one -- `assertion failed: self.mask == MaskIdx::NONE` -- so clipping was one level deep wherever it was used at all. That is what stopped the transcript's `List` from being clipped to its own box: its rows already use `.masked()` themselves (a code fence, a tool card's one-line title), and giving the list one aborted on the first fence drawn. A mask now carries the mask it was set inside (`Mask::parent`) and the fragment stage walks that chain, so a pixel has to be inside every mask on it. Chained rather than intersected on the CPU because each mask moves with its own widget: a fence inside a transcript row carries the row's scroll and the list's box does not, and one region resolved when the fence was last drawn gets the second of those wrong as soon as the row is moved rather than redrawn -- which is every scroll frame. The child holds one ref on its parent's slot, released where the child's own slot is, so a chain cannot outlive what it points at. The old assert survives as the case that is still wrong: the same widget setting two masks, which since a mask now chains would be a clip loop. Also `Painter::draw_again`, for a layout that can only discover a correction to itself by laying out once -- `List::clamp_to_content`, in the commit after this -- and `Painter::is_masked`, which is how a widget that draws outside its own box can require something to be clipping it. --- iris/core/src/render/data.rs | 13 +++++++ iris/core/src/render/shader.wgsl | 16 +++++++-- iris/core/src/ui/painter.rs | 56 +++++++++++++++++++++++++++--- iris/core/src/ui/render_state.rs | 16 ++++++--- iris/src/layout_tests.rs | 4 +-- iris/transcript-ui/src/composer.rs | 11 +++--- 6 files changed, 98 insertions(+), 18 deletions(-) diff --git a/iris/core/src/render/data.rs b/iris/core/src/render/data.rs index 2133445..c5c7bdf 100644 --- a/iris/core/src/render/data.rs +++ b/iris/core/src/render/data.rs @@ -56,6 +56,19 @@ pub struct Mask { /// primitive's own corners, so a mask and the content clipped by it /// can move independently. See LAYOUT.md section 2b. pub move_idx: MoveIdx, + /// The mask this one was set *inside* (`MaskIdx::NONE` at the top), so + /// clipping nests: the fragment stage walks the chain and a pixel has + /// to be inside every mask on it. Chained rather than intersected on + /// the CPU because each mask moves with its own widget -- a code fence + /// inside a transcript row carries the row's scroll, the list's own + /// box does not, and one region resolved when the fence was last drawn + /// gets the second of those wrong as soon as the row moves. + /// + /// A child holds one ref on its parent's slot (`Painter::set_mask`), + /// released when the child's own slot goes + /// (`UiRenderState::remove`), so the chain cannot outlive what it + /// points at. + pub parent: MaskIdx, } /// One widget's cumulative on-screen translation, and the slot of the diff --git a/iris/core/src/render/shader.wgsl b/iris/core/src/render/shader.wgsl index 99afb2c..d658716 100644 --- a/iris/core/src/render/shader.wgsl +++ b/iris/core/src/render/shader.wgsl @@ -34,6 +34,10 @@ struct Mask { x: UiSpan, y: UiSpan, move_idx: u32, + /// The mask this one is nested inside, or `4294967295u`. Mirrors + /// `Mask::parent` in data.rs; walked below with the same bound the + /// move chain uses. + parent: u32, } /// One widget's cumulative on-screen translation and the slot of the @@ -196,8 +200,15 @@ fn fs_main( color = vec4(1.0, 0.0, 1.0, 1.0); } } - if in.mask_idx != 4294967295u { - let mask = masks[in.mask_idx]; + // Every mask on the chain, not just the innermost: a widget that set + // its own mask inside another is clipped by both, and each carries its + // own move slot (`Mask::parent` in data.rs). + var mask_idx = in.mask_idx; + for (var step = 0u; step < MOVE_CHAIN_LIMIT; step++) { + if mask_idx == 4294967295u { + break; + } + let mask = masks[mask_idx]; let mask_delta = resolve_move(mask.move_idx); let tl = UiVec2(vec2(mask.x.start.rel, mask.y.start.rel), vec2(mask.x.start.abs, mask.y.start.abs)); let br = UiVec2(vec2(mask.x.end.rel, mask.y.end.rel), vec2(mask.x.end.abs, mask.y.end.abs)); @@ -207,6 +218,7 @@ fn fs_main( if pos.x < top_left.x || pos.x > bot_right.x || pos.y < top_left.y || pos.y > bot_right.y { color *= 0.0; } + mask_idx = mask.parent; } return color; } diff --git a/iris/core/src/ui/painter.rs b/iris/core/src/ui/painter.rs index 4b7e6aa..fbe8b44 100644 --- a/iris/core/src/ui/painter.rs +++ b/iris/core/src/ui/painter.rs @@ -53,8 +53,11 @@ impl<'a> Painter<'a> { } /// Clip everything this widget draws, itself and its descendants, to - /// `region`. One per widget: a second call would need the two to be - /// intersected, which nothing here does. + /// `region`. One call per widget; a widget drawn inside another + /// widget's mask nests instead -- the new mask chains to the inherited + /// one (`Mask::parent`) and the fragment stage requires a pixel to be + /// inside both, which is what lets a transcript row's code fence clip + /// to itself *and* to the list it scrolls inside. /// /// The slot is allocated once and **rewritten in place** on every /// later draw rather than pushed again, because a descendant whose own @@ -62,24 +65,69 @@ impl<'a> Painter<'a> { /// so keeps pointing at whichever slot it was drawn under. See /// `ActiveData::own_mask` for what pushing a fresh one cost. pub fn set_mask(&mut self, region: UiRegion) { - assert!(self.mask == MaskIdx::NONE); + debug_assert!( + self.own_mask == MaskIdx::NONE || self.mask != self.own_mask, + "set_mask called twice while drawing one widget: the second would replace the first \ + rather than nest inside it", + ); + let parent = self.mask; let mask = Mask { region, move_idx: self.move_slot, + parent, }; - if self.own_mask == MaskIdx::NONE { + let old_parent = if self.own_mask == MaskIdx::NONE { let slot = self.rsc.ui_mut().masks.push(mask); // The one ref this widget holds on its own slot, so the slot // outlives any single frame's primitives; released in // `UiRenderState::remove`'s `undraw` branch. self.rsc.ui_mut().masks.push_ref(slot); self.own_mask = slot; + MaskIdx::NONE } else { + let old = self.rsc.ui().masks[self.own_mask.idx()].parent; *self.rsc.ui_mut().masks.get_mut(self.own_mask) = mask; + old + }; + // The chain link's own ref, taken before the old one is dropped so + // that re-chaining to the same slot cannot free it in between. + // Released here when the link changes, and in + // `UiRenderState::remove` when this widget's slot goes. + if old_parent != parent { + if parent != MaskIdx::NONE { + self.rsc.ui_mut().masks.push_ref(parent); + } + if old_parent != MaskIdx::NONE { + self.rsc.ui_mut().masks.remove(old_parent); + } } self.mask = self.own_mask; } + /// Ask for this widget to be drawn again on the next frame, from + /// inside its own `draw` -- for a layout that can only discover a + /// correction to itself by laying out once (`List::clamp_to_content`, + /// which learns how far past its content the list is from the walk it + /// has just done). The mark is the same one `Widgets::get_dyn_mut` + /// sets, so `UiRenderState::update` picks it up exactly as it does any + /// other dirty widget; it does **not** by itself ask the platform for + /// a frame, which is the caller's own `RequestRedraw` handle. + /// + /// The correction it asks for must converge, or this is a widget that + /// redraws forever. + pub fn draw_again(&mut self) { + self.rsc.widgets_mut().needs_redraw.insert(self.id); + } + + /// Whether anything is clipping what this widget draws -- its own + /// [`Self::set_mask`], or one an ancestor set that it inherited. What + /// a widget whose contents may legitimately extend past its own box + /// (`iris::widget::List`, which draws a row straddling an edge in + /// full) asserts before relying on being cut off there. + pub fn is_masked(&self) -> bool { + self.mask != MaskIdx::NONE + } + /// Draws a widget within this widget's region, returning the size it /// reported using. pub fn widget(&mut self, id: &StrongWidget) -> Size { diff --git a/iris/core/src/ui/render_state.rs b/iris/core/src/ui/render_state.rs index 32627f3..dd08e95 100644 --- a/iris/core/src/ui/render_state.rs +++ b/iris/core/src/ui/render_state.rs @@ -324,10 +324,9 @@ impl UiRenderState { // own new one -- and `ActiveData::mask`'s only consumer is // `redraw`, which feeds it back in as the *inherited* mask. Storing // the set one instead handed a `Masked` its own mask on every - // targeted redraw, tripping `set_mask`'s nested-mask assert: - // `assertion failed: self.mask == MaskIdx::NONE`, an abort the - // first time the composer's scroll area was redrawn on the - // emulator. + // targeted redraw -- an abort the first time the composer's scroll + // area was redrawn on the emulator, and now (masks nest) a mask + // whose parent is itself, which `set_mask`'s own assert names. let inherited_mask = mask; let mut painter = Painter { state: self, @@ -514,8 +513,15 @@ impl UiRenderState { // section 2's lifecycle note). if active.own_mask != MaskIdx::NONE { // The self-ownership ref `Painter::set_mask` took when - // it allocated this widget's own mask slot. + // it allocated this widget's own mask slot, and the + // chain link's ref on the mask this one nests inside + // -- read from the arena entry, for the same reason + // the move slot's parent is. + let outer = rsc.ui().masks[active.own_mask.idx()].parent; rsc.ui_mut().masks.remove(active.own_mask); + if outer != MaskIdx::NONE { + rsc.ui_mut().masks.remove(outer); + } } let parent_slot = rsc.ui_mut().move_offsets[active.move_slot.idx()].parent; rsc.ui_mut().move_offsets.remove(active.move_slot); diff --git a/iris/src/layout_tests.rs b/iris/src/layout_tests.rs index be2ef9c..baa67e3 100644 --- a/iris/src/layout_tests.rs +++ b/iris/src/layout_tests.rs @@ -150,8 +150,8 @@ fn hit_testing_follows_a_scrolled_widget() { /// `ActiveData::mask` is the mask a widget was drawn **under**, not the one /// it set for itself -- `redraw` feeds it straight back in as the inherited /// mask, so storing the set one hands a `Masked` its own mask the second -/// time round and trips `Painter::set_mask`'s nested-mask assert. That was -/// an abort (`assertion failed: self.mask == MaskIdx::NONE`) the first time +/// time round -- which `Painter::set_mask` asserts against, since a mask +/// that chains to itself is a clip loop. That was an abort the first time /// the composer's new scroll area was redrawn on the emulator; a targeted /// redraw of a `Masked` is what any real screen does whenever anything /// inside it changes. diff --git a/iris/transcript-ui/src/composer.rs b/iris/transcript-ui/src/composer.rs index c0ebf7a..0b0d352 100644 --- a/iris/transcript-ui/src/composer.rs +++ b/iris/transcript-ui/src/composer.rs @@ -90,11 +90,12 @@ where // where the bar actually was. // `.scrollable().masked()`: the finger pan (`Scroll::drag`) plus the // clip that keeps six lines' worth of a longer message inside the - // bar. The mask is the caller's job rather than `Scroll`'s own, - // because `Painter::set_mask` allows exactly one mask per widget and - // a `Scroll` nested under another masked area would abort on the - // second -- `.masked()` is the one mechanism for clipping and this is - // one more use of it (tabs-ui's message area is the other). + // bar. The mask is the caller's job rather than `Scroll`'s own: + // `.masked()` is the one mechanism for clipping and this is one more + // use of it (tabs-ui's message area is the other). A `Scroll` nested + // under another masked area used to abort here; since 2026-09-07 the + // inner mask chains to the outer one (`Mask::parent`) and the content + // is clipped by both. // Without it the overflow paints *above* the bar, over the // transcript: measured before this change at 58px of stray text for a // 475px message in a 417px box.