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.
This commit is contained in:
1 parent
9ed01e2812
commit
d507ae4c96
6 files changed
+98
-18
No files matched your search
@@ -56,6 +56,19 @@ pub struct Mask {
|
|||||||
/// primitive's own corners, so a mask and the content clipped by it
|
/// primitive's own corners, so a mask and the content clipped by it
|
||||||
/// can move independently. See LAYOUT.md section 2b.
|
/// can move independently. See LAYOUT.md section 2b.
|
||||||
pub move_idx: MoveIdx,
|
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
|
/// One widget's cumulative on-screen translation, and the slot of the
|
||||||
|
|||||||
@@ -34,6 +34,10 @@ struct Mask {
|
|||||||
x: UiSpan,
|
x: UiSpan,
|
||||||
y: UiSpan,
|
y: UiSpan,
|
||||||
move_idx: u32,
|
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
|
/// 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);
|
color = vec4(1.0, 0.0, 1.0, 1.0);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if in.mask_idx != 4294967295u {
|
// Every mask on the chain, not just the innermost: a widget that set
|
||||||
let mask = masks[in.mask_idx];
|
// 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 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 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));
|
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 {
|
if pos.x < top_left.x || pos.x > bot_right.x || pos.y < top_left.y || pos.y > bot_right.y {
|
||||||
color *= 0.0;
|
color *= 0.0;
|
||||||
}
|
}
|
||||||
|
mask_idx = mask.parent;
|
||||||
}
|
}
|
||||||
return color;
|
return color;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -53,8 +53,11 @@ impl<'a> Painter<'a> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Clip everything this widget draws, itself and its descendants, to
|
/// Clip everything this widget draws, itself and its descendants, to
|
||||||
/// `region`. One per widget: a second call would need the two to be
|
/// `region`. One call per widget; a widget drawn inside another
|
||||||
/// intersected, which nothing here does.
|
/// 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
|
/// The slot is allocated once and **rewritten in place** on every
|
||||||
/// later draw rather than pushed again, because a descendant whose own
|
/// 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
|
/// so keeps pointing at whichever slot it was drawn under. See
|
||||||
/// `ActiveData::own_mask` for what pushing a fresh one cost.
|
/// `ActiveData::own_mask` for what pushing a fresh one cost.
|
||||||
pub fn set_mask(&mut self, region: UiRegion) {
|
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 {
|
let mask = Mask {
|
||||||
region,
|
region,
|
||||||
move_idx: self.move_slot,
|
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);
|
let slot = self.rsc.ui_mut().masks.push(mask);
|
||||||
// The one ref this widget holds on its own slot, so the slot
|
// The one ref this widget holds on its own slot, so the slot
|
||||||
// outlives any single frame's primitives; released in
|
// outlives any single frame's primitives; released in
|
||||||
// `UiRenderState::remove`'s `undraw` branch.
|
// `UiRenderState::remove`'s `undraw` branch.
|
||||||
self.rsc.ui_mut().masks.push_ref(slot);
|
self.rsc.ui_mut().masks.push_ref(slot);
|
||||||
self.own_mask = slot;
|
self.own_mask = slot;
|
||||||
|
MaskIdx::NONE
|
||||||
} else {
|
} else {
|
||||||
|
let old = self.rsc.ui().masks[self.own_mask.idx()].parent;
|
||||||
*self.rsc.ui_mut().masks.get_mut(self.own_mask) = mask;
|
*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;
|
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
|
/// Draws a widget within this widget's region, returning the size it
|
||||||
/// reported using.
|
/// reported using.
|
||||||
pub fn widget<W: ?Sized>(&mut self, id: &StrongWidget<W>) -> Size {
|
pub fn widget<W: ?Sized>(&mut self, id: &StrongWidget<W>) -> Size {
|
||||||
|
|||||||
@@ -324,10 +324,9 @@ impl UiRenderState {
|
|||||||
// own new one -- and `ActiveData::mask`'s only consumer is
|
// own new one -- and `ActiveData::mask`'s only consumer is
|
||||||
// `redraw`, which feeds it back in as the *inherited* mask. Storing
|
// `redraw`, which feeds it back in as the *inherited* mask. Storing
|
||||||
// the set one instead handed a `Masked` its own mask on every
|
// the set one instead handed a `Masked` its own mask on every
|
||||||
// targeted redraw, tripping `set_mask`'s nested-mask assert:
|
// targeted redraw -- an abort the first time the composer's scroll
|
||||||
// `assertion failed: self.mask == MaskIdx::NONE`, an abort the
|
// area was redrawn on the emulator, and now (masks nest) a mask
|
||||||
// first time the composer's scroll area was redrawn on the
|
// whose parent is itself, which `set_mask`'s own assert names.
|
||||||
// emulator.
|
|
||||||
let inherited_mask = mask;
|
let inherited_mask = mask;
|
||||||
let mut painter = Painter {
|
let mut painter = Painter {
|
||||||
state: self,
|
state: self,
|
||||||
@@ -514,8 +513,15 @@ impl UiRenderState {
|
|||||||
// section 2's lifecycle note).
|
// section 2's lifecycle note).
|
||||||
if active.own_mask != MaskIdx::NONE {
|
if active.own_mask != MaskIdx::NONE {
|
||||||
// The self-ownership ref `Painter::set_mask` took when
|
// 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);
|
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;
|
let parent_slot = rsc.ui_mut().move_offsets[active.move_slot.idx()].parent;
|
||||||
rsc.ui_mut().move_offsets.remove(active.move_slot);
|
rsc.ui_mut().move_offsets.remove(active.move_slot);
|
||||||
|
|||||||
@@ -150,8 +150,8 @@ fn hit_testing_follows_a_scrolled_widget() {
|
|||||||
/// `ActiveData::mask` is the mask a widget was drawn **under**, not the one
|
/// `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
|
/// 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
|
/// 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
|
/// time round -- which `Painter::set_mask` asserts against, since a mask
|
||||||
/// an abort (`assertion failed: self.mask == MaskIdx::NONE`) the first time
|
/// 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
|
/// 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
|
/// redraw of a `Masked` is what any real screen does whenever anything
|
||||||
/// inside it changes.
|
/// inside it changes.
|
||||||
|
|||||||
@@ -90,11 +90,12 @@ where
|
|||||||
// where the bar actually was.
|
// where the bar actually was.
|
||||||
// `.scrollable().masked()`: the finger pan (`Scroll::drag`) plus the
|
// `.scrollable().masked()`: the finger pan (`Scroll::drag`) plus the
|
||||||
// clip that keeps six lines' worth of a longer message inside 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,
|
// bar. The mask is the caller's job rather than `Scroll`'s own:
|
||||||
// because `Painter::set_mask` allows exactly one mask per widget and
|
// `.masked()` is the one mechanism for clipping and this is one more
|
||||||
// a `Scroll` nested under another masked area would abort on the
|
// use of it (tabs-ui's message area is the other). A `Scroll` nested
|
||||||
// second -- `.masked()` is the one mechanism for clipping and this is
|
// under another masked area used to abort here; since 2026-09-07 the
|
||||||
// one more use of it (tabs-ui's message area is the other).
|
// 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
|
// Without it the overflow paints *above* the bar, over the
|
||||||
// transcript: measured before this change at 58px of stray text for a
|
// transcript: measured before this change at 58px of stray text for a
|
||||||
// 475px message in a 417px box.
|
// 475px message in a 417px box.
|
||||||
|
|||||||
Reference in new issue
Block a user