From add6774980679f8c7dc69f6da7f23efedc39a0b0 Mon Sep 17 00:00:00 2001 From: iris-ai <4+iris-ai@noreply.localhost> Date: Sat, 19 Sep 2026 13:43:11 -0400 Subject: [PATCH] Keep retained masks and reparented drawings alive, and advance collapsed slots --- core/src/ui/active.rs | 1 + core/src/ui/painter.rs | 20 ++++- core/src/ui/render_state.rs | 28 +++++-- src/widget/position/span.rs | 1 + tests/cases/layout.rs | 37 +++++++++ tests/cases/retained.rs | 151 ++++++++++++++++++++++++++++++++++++ 6 files changed, 229 insertions(+), 9 deletions(-) diff --git a/core/src/ui/active.rs b/core/src/ui/active.rs index 68204d6..f787e01 100644 --- a/core/src/ui/active.rs +++ b/core/src/ui/active.rs @@ -53,6 +53,7 @@ pub struct ActiveData { /// Its primitives, each keeping the box it was written in -- in this /// widget's extent coordinates, which is what a move recomposes from. pub primitives: Vec, + /// An owned mask holds one reference independently of its primitives. pub mask_region: Option, pub children: Vec, /// The children whose size this widget read while drawing. diff --git a/core/src/ui/painter.rs b/core/src/ui/painter.rs index 0531147..6d4e9e1 100644 --- a/core/src/ui/painter.rs +++ b/core/src/ui/painter.rs @@ -35,6 +35,8 @@ pub struct Painter<'a> { pub(super) textures: Vec, pub(super) primitives: Vec, pub(super) mask_region: Option, + /// The previous drawing's owned mask, available for this draw to reclaim. + pub(super) mask_slot: Option, /// Only children whose answers were read constrain this widget's answer. pub(super) answer_under: LayoutHolds, pub(super) children: Vec, @@ -104,7 +106,6 @@ impl<'a> Painter<'a> { fn push_primitive(&mut self, h: RetainedPrimitive) { if self.mask != MaskIdx::NONE { - // TODO: I have no clue if this works at all :joy: self.rsc.ui_mut().masks.push_ref(self.mask); } self.primitives.push(h); @@ -129,10 +130,23 @@ impl<'a> Painter<'a> { assert!(self.mask == MaskIdx::NONE); let resolved = self.resolve(region); let move_idx = self.move_idx; - self.mask = self.rsc.ui_mut().masks.push(Mask { + let mask = Mask { region: resolved, move_idx, - }); + }; + let masks = &mut self.rsc.ui_mut().masks; + self.mask = match self.mask_slot.take() { + Some(idx) => { + *masks.get_mut(idx) = mask; + idx + } + None => { + let idx = masks.push(mask); + // The owner keeps the slot alive even with no primitives. + masks.push_ref(idx); + idx + } + }; } /// Draws a widget in the whole of this widget's own box, with the frame diff --git a/core/src/ui/render_state.rs b/core/src/ui/render_state.rs index 2f8e5b7..74f2eb5 100644 --- a/core/src/ui/render_state.rs +++ b/core/src/ui/render_state.rs @@ -223,6 +223,10 @@ impl UiRenderState { mut old: Option, rsc: &mut dyn UiRsc, ) -> (Size, LayoutHolds, LayoutHolds) { + let old_parent = old + .as_ref() + .or_else(|| self.active.get(&id)) + .and_then(|a| a.parent); let part = info.part; #[cfg(feature = "layout-diagnostics")] { @@ -275,12 +279,9 @@ impl UiRenderState { active.part = part; active.placed = info.placed; active.own_align = align; - // A subtree can be reused whole under a different parent -- same box, - // same layer, same region node -- and nothing in the drawing says it - // changed hands. Two things read who its parent is: a deferral, which - // marks whoever has it to draw, and the old parent's list of children, - // which its next draw undraws whatever is missing from. - let old_parent = std::mem::replace(&mut active.parent, info.parent); + // The previous parent must stop owning the subtree before it can + // undraw it, whether changing hands reused the drawing or replaced it. + active.parent = info.parent; if old_parent != info.parent && let Some(old_parent) = old_parent && let Some(old_parent) = self.active.get_mut(&old_parent) @@ -312,6 +313,9 @@ impl UiRenderState { // Reusing its index sooner could make an old parent look current. false => (info.parent_move, extent, self.slots.remove(&id)), }; + let mask_slot = old + .as_ref() + .and_then(|old| old.mask_region.map(|_| old.mask)); let old_children = old.map_or_else(Vec::new, |old| old.children); rsc.widgets_mut().needs_redraw.remove(&id); let px = info.px; @@ -330,6 +334,7 @@ impl UiRenderState { textures: Vec::new(), primitives: Vec::new(), mask_region: None, + mask_slot, children: Vec::new(), size_deps: Vec::new(), window_own: [Holds::ANY; 2], @@ -363,6 +368,7 @@ impl UiRenderState { textures, primitives, mask_region, + mask_slot, extent_own, extent_len, answer_under, @@ -421,6 +427,9 @@ impl UiRenderState { self.undraw_rec(*c, rsc); } } + if let Some(idx) = mask_slot { + rsc.ui_mut().masks.remove(idx); + } if let Some(idx) = retired_move { self.moves.remove(idx); } @@ -605,6 +614,9 @@ impl UiRenderState { } return None; } + if active.parent_mask != info.mask { + return None; + } // Drawn somewhere else in the tree: its box is in coordinates it no // longer sits in, and its slot names the wrong parent. if active.parent_move != info.parent_move { @@ -826,6 +838,9 @@ impl UiRenderState { rsc.ui_mut().masks.remove(mask); } } + if undraw && active.mask_region.take().is_some() { + rsc.ui_mut().masks.remove(active.mask); + } active.primitives.clear(); active.textures.clear(); rsc.ui_mut().textures.free(); @@ -913,6 +928,7 @@ impl UiRenderState { self.slots.clear(); self.moves.clear(); self.layers.clear(); + rsc.ui_mut().masks = Default::default(); rsc.widgets_mut().needs_redraw.clear(); self.free(rsc); } diff --git a/src/widget/position/span.rs b/src/widget/position/span.rs index ae7522f..2030fa7 100644 --- a/src/widget/position/span.rs +++ b/src/widget/position/span.rs @@ -108,6 +108,7 @@ impl Widget for Span { { painter.undraw(child); fixed.px += self.gap; + start = shared(fixed, taken, total.leftover, room); continue; } let from = start; diff --git a/tests/cases/layout.rs b/tests/cases/layout.rs index 917e799..19b489f 100644 --- a/tests/cases/layout.rs +++ b/tests/cases/layout.rs @@ -821,3 +821,40 @@ fn a_root_with_a_fraction_rule_is_that_fraction_of_the_window() { h.set_root(root); assert_eq!(h.region(&root).unwrap().size().x, Px::from_int(450)); } + +#[test] +fn a_collapsed_share_keeps_the_gaps_before_the_next_slot() { + for dir in [Dir::RIGHT, Dir::LEFT, Dir::DOWN, Dir::UP] { + for collapsed in [1, 2] { + let mut h = Harness::new((400, 400)); + let head = rect(Color::RED).add(&mut h.rsc); + h.set_len(head, dir.axis, 200); + let tail = rect(Color::BLUE).add(&mut h.rsc); + let tail_len = 200 - 10 * (collapsed + 1); + h.set_len(tail, dir.axis, tail_len); + let mut children: Vec = vec![head.add_strong(&mut h.rsc)]; + let mut shares = Vec::new(); + for _ in 0..collapsed { + let share = rect(Color::GREEN).add(&mut h.rsc); + shares.push(share); + children.push(share.add_strong(&mut h.rsc)); + } + children.push(tail.add_strong(&mut h.rsc)); + h.set_root(Span { + children, + dir, + gap: Px::from_int(10), + }); + for share in shares { + assert!(h.region(&share).is_none()); + } + let region = h.region(&tail).unwrap(); + let (from, to) = match dir.sign { + Sign::Pos => (400 - tail_len, 400), + Sign::Neg => (0, tail_len), + }; + assert_eq!(region.top_left.axis(dir.axis), Px::from_int(from)); + assert_eq!(region.bot_right.axis(dir.axis), Px::from_int(to)); + } + } +} diff --git a/tests/cases/retained.rs b/tests/cases/retained.rs index 3bfee27..2faf18b 100644 --- a/tests/cases/retained.rs +++ b/tests/cases/retained.rs @@ -1342,3 +1342,154 @@ fn extent_frames_keep_fractional_reports_and_numeric_dependencies_valid() { } } } + +struct OptionalMask { + inner: StrongWidget, + enabled: bool, +} + +impl Widget for OptionalMask { + fn draw(&mut self, painter: &mut Painter) -> Size { + if self.enabled { + painter.set_mask(UiRegion::FULL); + } + painter.widget(&self.inner); + Size::LEFTOVER + } +} + +fn primitive_masks(h: &Harness, id: WidgetId) -> Vec { + h.render.active[&id] + .primitives + .iter() + .map(|primitive| { + let handle = &primitive.handle; + h.render.layers[handle.layer].primitives()[handle.kind as usize] + .as_ref() + .unwrap() + .instances()[handle.inst_idx] + .mask_idx + }) + .collect() +} + +#[test] +fn a_redrawn_mask_keeps_reused_primitives_clipped_when_it_moves() { + for node in [false, true] { + let mut h = Harness::new((400, 200)); + let first = rect(Color::RED).height(50).add(&mut h.rsc); + let inner = rect(Color::BLUE).add(&mut h.rsc); + let draws = Rc::new(Cell::new(0)); + let child = Stretchy { + inner: inner.add_strong(&mut h.rsc), + draws: draws.clone(), + } + .add(&mut h.rsc); + let masked = child.masked().add(&mut h.rsc); + h.rsc.widgets_mut().set_region_node(masked, node); + h.set_root((first, masked).span(Dir::DOWN)); + let mask = h.render.active[&masked.id()].mask; + let settled = draws.get(); + h.rsc.widgets_mut().get_dyn_mut(masked.id()); + h.frame(); + assert_eq!(primitive_masks(&h, inner.id()), vec![mask]); + assert_eq!(draws.get(), settled, "a mask repaint must reuse its child"); + assert_eq!(h.render.active[&masked.id()].mask, mask); + h.set_len(first, Axis::Y, 10); + h.frame(); + let clip = h.rsc.ui().masks[mask.idx()]; + let clip = h + .render + .moves + .resolve(clip.move_idx, clip.region) + .to_px(h.render.output_size()); + assert_eq!(clip, h.region(&masked).unwrap()); + assert_corners!(h, inner, (0, 10), (400, 200)); + } +} + +#[test] +fn adding_and_removing_a_mask_updates_existing_primitives() { + let mut h = Harness::new((400, 200)); + let inner = rect(Color::BLUE).add(&mut h.rsc); + let masked = OptionalMask { + inner: inner.add_strong(&mut h.rsc), + enabled: false, + } + .add(&mut h.rsc); + h.set_root(masked); + for enabled in [true, false, true, false] { + h.rsc[masked].enabled = enabled; + h.frame(); + let mask = h.render.active[&masked.id()].mask; + assert_eq!(mask == MaskIdx::NONE, !enabled); + assert_eq!(primitive_masks(&h, inner.id()), vec![mask]); + } + assert_eq!(h.rsc.ui().masks.len(), 1, "retired slots must be reusable"); +} + +#[test] +fn an_empty_masks_slot_is_released_when_the_mask_is_removed_or_undrawn() { + let mut h = Harness::new((400, 200)); + let (inner, _) = counted(&mut h, Size::LEFTOVER, false); + let masked = OptionalMask { + inner: inner.add_strong(&mut h.rsc), + enabled: true, + } + .add(&mut h.rsc); + let row = (masked,).span(Dir::DOWN).add(&mut h.rsc); + h.set_root(row); + for _ in 0..3 { + h.rsc[masked].enabled = false; + h.frame(); + h.rsc[masked].enabled = true; + h.frame(); + let child = h.rsc[row].pop().unwrap(); + h.frame(); + h.rsc[row].push(child); + h.frame(); + } + assert_eq!(h.rsc.ui().masks.len(), 1); +} + +struct SharedChild(Rc); + +impl Widget for SharedChild { + fn draw(&mut self, painter: &mut Painter) -> Size { + painter.widget(self.0.as_ref()).size() + } +} + +struct SwitchParent { + choices: [StrongWidget; 2], + choice: usize, +} + +impl Widget for SwitchParent { + fn draw(&mut self, painter: &mut Painter) -> Size { + painter.widget(&self.choices[self.choice]).size() + } +} + +#[test] +fn a_redrawn_subtree_is_not_undrawn_by_the_parent_it_left() { + for node in [false, true] { + let mut h = Harness::new((400, 200)); + let leaf = rect(Color::RED).width(40).add(&mut h.rsc); + let held: StrongWidget = leaf.add_strong(&mut h.rsc); + let shared = Rc::new(held); + let first = SharedChild(shared.clone()).add_strong(&mut h.rsc); + let second = SharedChild(shared).add_strong(&mut h.rsc); + h.rsc.widgets_mut().set_region_node(&second, node); + let root = SwitchParent { + choices: [first, second], + choice: 0, + } + .add(&mut h.rsc); + h.set_root(root); + let before = h.region(&leaf); + h.rsc[root].choice = 1; + h.frame(); + assert_eq!(h.region(&leaf), before); + } +}