From 8223a55cfb8cc6952045e04467ad69f56156a494 Mon Sep 17 00:00:00 2001 From: iris-ai <4+iris-ai@noreply.localhost> Date: Mon, 14 Sep 2026 03:15:17 -0400 Subject: [PATCH] Move a subtree by writing one slot `try_reuse`'s pure-translation case now writes the widget's move slot instead of remapping every primitive in its subtree. Counted on a span of 20 rows, each five primitives deep, when the row above them changes height: before 100 primitive region writes now 0, and 20 slot writes -- one per row the span re-placed `window_region` walks the same chain on the CPU, so hit testing and anyone asking in window pixels see a widget where the shader draws it. `Moves::resolve` stops at `CHAIN_LIMIT` like the shader, and asserts in debug that it got to the end rather than running out. Two things fall out of it. Rewriting a region is the one thing a slot cannot express, so `mov` zeroes the slots of everything it rewrites: a region is what its slot was a delta from. And `try_reuse` loses its `old == region` shortcut, which was wrong once a slot exists -- a widget offered exactly the box it drew against has to have its delta cleared, not skipped. `Moves` lives on `UiRenderState` rather than `UiData`, because the draw is what produces it and `window_region` should not need the ui's resources to answer where something is. The renderer already takes both. A slot is retired in `remove_rec`, after the descendants whose slots name it as their parent. Either order is correct here -- nothing can claim a freed index while a subtree is coming down, since `on_undraw` cannot reach the slots -- but this way `remove`'s `undraw` flag only notifies rather than also deciding slot lifetime, and the retirement sits beside the recursion it follows. Checked: fmt, clippy and 41 tests. `tabs` (with the image replay), `view`, `minimal` and `text` all still render byte-identical, and the live sway resize round trip -- which re-places most of the tree at the same size, so it is the slot path throughout -- matches a cold start at each size. --- core/src/render/mod.rs | 6 ++-- core/src/ui/mod.rs | 13 +++++--- core/src/ui/render_state.rs | 64 +++++++++++++++++++++++-------------- tests/layout.rs | 17 ++++++++++ 4 files changed, 68 insertions(+), 32 deletions(-) diff --git a/core/src/render/mod.rs b/core/src/render/mod.rs index d898d0f..d368ffb 100644 --- a/core/src/render/mod.rs +++ b/core/src/render/mod.rs @@ -133,9 +133,9 @@ impl UiRenderNode { ui.masks.changed = false; regroup |= self.masks.update(device, queue, &ui.masks[..]); } - if ui.moves.changed { - ui.moves.changed = false; - regroup |= self.moves.update(device, queue, ui.moves.entries()); + if ui_render.moves.changed { + ui_render.moves.changed = false; + regroup |= self.moves.update(device, queue, ui_render.moves.entries()); } if regroup { self.shared_group = Self::shared_group( diff --git a/core/src/ui/mod.rs b/core/src/ui/mod.rs index 69b3503..5bdd6cb 100644 --- a/core/src/ui/mod.rs +++ b/core/src/ui/mod.rs @@ -25,12 +25,10 @@ pub struct UiData { pub textures: Textures, pub text: TextData, pub masks: TrackedArena, - /// Where each widget's drawing sits relative to its parent's slot, so - /// moving a subtree writes one entry rather than every descendant's - /// primitives. - pub moves: Moves, } +/// Where each widget's drawing sits relative to its parent's slot, so moving +/// a subtree writes one entry rather than every descendant's primitives. #[derive(Default)] pub struct Moves { arena: Arena, @@ -64,12 +62,17 @@ impl Moves { let mut at = idx; for _ in 0..CHAIN_LIMIT { if at == MoveIdx::NONE { - break; + return total; } let entry = self.arena[at.idx()]; total += entry.delta; at = entry.parent; } + debug_assert!( + at == MoveIdx::NONE, + "a move chain longer than {CHAIN_LIMIT} resolves to the wrong place, \ + and the shader stops at the same depth" + ); total } diff --git a/core/src/ui/render_state.rs b/core/src/ui/render_state.rs index cd2333b..37c9203 100644 --- a/core/src/ui/render_state.rs +++ b/core/src/ui/render_state.rs @@ -1,6 +1,6 @@ use crate::{ - ActiveData, Axis, DrawLayers, IdLike, MaskIdx, MoveIdx, OnResize, Painter, PixelRegion, Remap, - Size, StrongWidget, UiRegion, UiRsc, WidgetId, Widgets, + ActiveData, Axis, DrawLayers, IdLike, MaskIdx, MoveIdx, Moves, OnResize, Painter, PixelRegion, + Remap, Size, StrongWidget, UiRegion, UiRsc, WidgetId, Widgets, util::{HashMap, HashSet, Vec2, forget_ref}, }; @@ -14,7 +14,8 @@ pub struct UiRenderState { draw_started: HashSet, /// A widget's move slot, which outlives any one `ActiveData`: a redraw /// replaces that while its children go on pointing at the slot. - moves: HashMap, + slots: HashMap, + pub moves: Moves, } impl UiRenderState { @@ -26,6 +27,7 @@ impl UiRenderState { old_root: None, resized: false, draw_started: Default::default(), + slots: Default::default(), moves: Default::default(), } } @@ -107,7 +109,8 @@ impl UiRenderState { } // draw widget - let move_idx = self.move_slot(id, parent, rsc); + let move_idx = self.move_slot(id, parent); + self.moves.set(move_idx, Vec2::ZERO); rsc.widgets_mut().needs_redraw.remove(&id); self.draw_started.insert(id); @@ -181,20 +184,15 @@ impl UiRenderState { /// The slot a widget's drawing is positioned through, made on its first /// draw and kept until it stops being drawn. - fn move_slot( - &mut self, - id: WidgetId, - parent: Option, - rsc: &mut dyn UiRsc, - ) -> MoveIdx { - if let Some(&idx) = self.moves.get(&id) { + fn move_slot(&mut self, id: WidgetId, parent: Option) -> MoveIdx { + if let Some(&idx) = self.slots.get(&id) { return idx; } let parent = parent - .and_then(|p| self.moves.get(&p).copied()) + .and_then(|p| self.slots.get(&p).copied()) .unwrap_or(MoveIdx::NONE); - let idx = rsc.ui_mut().moves.push(parent); - self.moves.insert(id, idx); + let idx = self.moves.push(parent); + self.slots.insert(id, idx); idx } @@ -205,12 +203,18 @@ impl UiRenderState { return None; } let active = self.active.get(&id)?; - let (size, old) = (active.size, active.region); - if old == region { + let (size, old, slot) = (active.size, active.region, active.move_idx); + // TODO: epsilon? + if old.size() == region.size() { + // The right shape and only somewhere else, which is one slot to + // write however much is under it. Both boxes are in the + // coordinates its parent drew, so the chain above applies alike. + let moved = + region.to_px(self.output_size).top_left - old.to_px(self.output_size).top_left; + self.moves.set(slot, moved); return Some(size); } - // TODO: epsilon? - if old.size() != region.size() && !self.reusable(id, region, rsc) { + if !self.reusable(id, region, rsc) { return None; } // Its drawing stands, if the new box can be reached from the old one. @@ -255,6 +259,9 @@ impl UiRenderState { }) } + /// Rewrites a subtree's regions into a new box, for a change a slot + /// cannot express. Every region it rewrites is a region some slot was a + /// delta from, so those go back to zero. fn mov(&mut self, id: WidgetId, remap: &Remap) { let active = self.active.get_mut(&id).unwrap(); for h in &active.primitives { @@ -262,6 +269,7 @@ impl UiRenderState { *region = remap.apply(*region); } active.region = remap.apply(active.region); + self.moves.set(active.move_idx, Vec2::ZERO); // SAFETY: children cannot be recursive let children = unsafe { forget_ref(&active.children) }; for child in children { @@ -282,9 +290,6 @@ impl UiRenderState { active.textures.clear(); rsc.ui_mut().textures.free(); if undraw { - if let Some(idx) = self.moves.remove(&id) { - rsc.ui_mut().moves.remove(idx); - } rsc.on_undraw(active); } } @@ -298,6 +303,10 @@ impl UiRenderState { self.remove_rec(*c, rsc); } } + // After the descendants, whose slots name this one as their parent. + if let Some(idx) = self.slots.remove(&id) { + self.moves.remove(idx); + } inst } @@ -305,8 +314,8 @@ impl UiRenderState { for (_, active) in self.active.drain() { rsc.on_undraw(&active); } + self.slots.clear(); self.moves.clear(); - rsc.ui_mut().moves.clear(); self.layers.clear(); rsc.widgets_mut().needs_redraw.clear(); rsc.free(); @@ -354,9 +363,16 @@ impl UiRenderState { } } + /// Where a widget is on screen: the box it drew against plus whatever the + /// chain above has moved since, which is the walk the vertex shader does. pub fn window_region(&self, id: &impl IdLike) -> Option { - let region = self.active.get(&id.id())?.region; - Some(region.to_px(self.output_size)) + let active = self.active.get(&id.id())?; + let moved = self.moves.resolve(active.move_idx); + let region = active.region.to_px(self.output_size); + Some(PixelRegion { + top_left: region.top_left + moved, + bot_right: region.bot_right + moved, + }) } /// redraws a widget that's currently active (drawn) diff --git a/tests/layout.rs b/tests/layout.rs index 85ee631..dddb0f1 100644 --- a/tests/layout.rs +++ b/tests/layout.rs @@ -109,3 +109,20 @@ fn a_fixed_box_is_drawn_again_rather_than_stretched() { assert_corners!(h, panel, (0, 0), (400, 250)); } + +#[test] +fn a_moved_subtree_takes_its_children_with_it() { + let mut h = Harness::new((400, 400)); + let first = rect(Color::RED).height(40).add(&mut h.rsc); + let inner = rect(Color::BLUE).add(&mut h.rsc); + let row = inner.pad(10).height(40).add(&mut h.rsc); + h.set_root((first, row).span(Dir::DOWN)); + assert_corners!(h, inner, (10, 50), (390, 70)); + + h.rsc[first].y = Some(Len::abs(80)); + h.frame(); + + // The row is the same shape somewhere else, so one slot moved it and + // `inner`'s own region was never rewritten. + assert_corners!(h, inner, (10, 90), (390, 110)); +}