From 97cc8b32ed1fdea1761857dc828d9b8edddd7804 Mon Sep 17 00:00:00 2001 From: iris-ai <4+iris-ai@noreply.localhost> Date: Wed, 16 Sep 2026 04:04:31 -0400 Subject: [PATCH] Measure a child on the layer it draws on, not twice on two Fixed point cost 3x in layout: `many` went from 0.179 ms a frame to 0.544, and `scroll` from 0.011 to 0.030. The counters said why -- eight more "placed by redrawing" a frame -- and the reason was mine rather than the grid's. A retained drawing belongs to the layer it was made on, which `4e28f10` started enforcing, and `Stack` measures the child that sizes it by drawing it on its own layer and then draws it again on the child layer. So every stacked child redrew twice a frame, forever. `Painter::child_layer_at` addresses a child's layer rather than walking to it, and `Stack` measures on the layer that child ends up on. The second ask is then a reuse. Its glyphs are written once rather than once under the background and once over it. Measured on the same fixture: `scroll` 0.031 ms to 0.020, `many` 0.570 to 0.283, and the scroll phase's counters are back to what they were before fixed point -- 4 widget draws and 12 draw requests a frame, exactly. What is left above that baseline is not this. `ReuseOutcome` could not say "another layer" or "the region-node choice changed"; both returned without a counter, which is why the first look at this said nothing. They have counters now. Checked: fmt, clippy, 105 tests, five shrinker cases at 300 seeds, 100 generated seeds, and the examples byte-identical but for 36 pixels of `random` at one level -- edges that were being drawn twice. Co-Authored-By: Claude Opus 5 --- core/src/layout_diagnostics.rs | 4 +++ core/src/ui/painter.rs | 15 ++++++++++ core/src/ui/render_state.rs | 9 +++++- src/widget/position/stack.rs | 15 ++++++---- tests/cases/retained.rs | 50 +++++++++++++++++++++++++--------- 5 files changed, 73 insertions(+), 20 deletions(-) diff --git a/core/src/layout_diagnostics.rs b/core/src/layout_diagnostics.rs index c86ecd8..5886307 100644 --- a/core/src/layout_diagnostics.rs +++ b/core/src/layout_diagnostics.rs @@ -40,6 +40,8 @@ pub(crate) enum Counter { ReuseWrongParent, ReuseRemapped, ReuseOutside, + ReuseWrongLayer, + ReuseWrongNode, PlaceRedraws, QueuePops, DepthReads, @@ -73,6 +75,8 @@ impl Counter { "reuse: wrong parent", "reuse remapped", "reuse: outside what it holds for", + "reuse: another layer", + "reuse: region-node choice changed", "placed by redrawing", "redraw queue pops", "depth reads", diff --git a/core/src/ui/painter.rs b/core/src/ui/painter.rs index a470f17..653c36c 100644 --- a/core/src/ui/painter.rs +++ b/core/src/ui/painter.rs @@ -42,6 +42,9 @@ pub struct Painter<'a> { /// its own when opted in, otherwise the nearest ancestor's. pub(super) move_idx: MoveIdx, pub layer: usize, + /// The layer this widget was entered on, which its children's layers are + /// counted from however far `layer` has walked. + pub(super) own_layer: usize, pub(super) depth: usize, pub(super) id: WidgetId, } @@ -420,6 +423,18 @@ impl<'a> Painter<'a> { self.layer = self.state.layers.child(self.layer); } + /// The layer this widget's `n`th child draws on, addressed rather than + /// walked to. A container that measures one child by drawing it can ask + /// on the layer that child will end up on, and then the second ask is a + /// reuse rather than a second drawing on another layer. + pub fn child_layer_at(&mut self, n: usize) { + let mut at = self.state.layers.child(self.own_layer); + for _ in 0..n { + at = self.state.layers.next(at); + } + self.layer = at; + } + pub fn next_layer(&mut self) { self.layer = self.state.layers.next(self.layer); } diff --git a/core/src/ui/render_state.rs b/core/src/ui/render_state.rs index d599ee9..7b87280 100644 --- a/core/src/ui/render_state.rs +++ b/core/src/ui/render_state.rs @@ -307,6 +307,7 @@ impl UiRenderState { region: local, mask: info.mask, layer: info.layer, + own_layer: info.layer, id, textures: Vec::new(), primitives: Vec::new(), @@ -349,6 +350,7 @@ impl UiRenderState { under, move_idx, layer, + own_layer: _, depth: _, id, } = painter; @@ -586,6 +588,8 @@ impl UiRenderState { } let has_region_node = active.move_idx != active.parent_move; if has_region_node != info.region_node { + #[cfg(feature = "layout-diagnostics")] + diag::bump(Counter::ReuseWrongNode); return None; } // Drawn on another layer: the drawing sits in that layer's list and @@ -595,7 +599,10 @@ impl UiRenderState { // on a layer the first answer is not good for. if active.layer != info.layer { #[cfg(feature = "layout-diagnostics")] - diag::reuse(id, ReuseOutcome::WrongLayer); + { + diag::bump(Counter::ReuseWrongLayer); + diag::reuse(id, ReuseOutcome::WrongLayer); + } return None; } // Drawn somewhere else in the tree: its box is in coordinates it no diff --git a/src/widget/position/stack.rs b/src/widget/position/stack.rs index 82b86f6..f4c5ca7 100644 --- a/src/widget/position/stack.rs +++ b/src/widget/position/stack.rs @@ -16,16 +16,19 @@ impl Widget for Stack { // Whichever child sizes the stack decides the box every child gets. // The stack reports that size, so a child given a longer box would // draw outside what the stack says it occupies. - let size = match sizing.and_then(|i| self.children.get(i)) { - Some(child) => painter.widget(child).size(), + let size = match sizing.and_then(|i| self.children.get(i).map(|c| (i, c))) { + // On the layer that child ends up on, so the ask below is a reuse + // rather than a second drawing of it somewhere else: a retained + // drawing belongs to the layer it was made on. + Some((i, child)) => { + painter.child_layer_at(i); + painter.widget(child).size() + } None => Size::LEFTOVER, }; let region = painter.box_of(size); for (i, child) in self.children.iter().enumerate() { - match i { - 0 => painter.child_layer(), - _ => painter.next_layer(), - } + painter.child_layer_at(i); painter.widget_aligned(child, region, RegionAlign::NEAR); } size diff --git a/tests/cases/retained.rs b/tests/cases/retained.rs index 5c099d7..091c897 100644 --- a/tests/cases/retained.rs +++ b/tests/cases/retained.rs @@ -560,13 +560,43 @@ fn a_declared_length_child_is_not_redrawn_when_the_box_around_it_grows() { assert_corners!(h, fixed, (200, 0), (280, 200)); } -/// `Stack` measures the child that sizes it by drawing it, then draws it -/// again above the background it stacks over. The second ask is for the same -/// box, so nothing geometric says the answer has gone stale, and reusing it -/// leaves the drawing under the background. The same tree got away with it -/// while the two asks differed by a rounding. +/// A retained drawing belongs to the layer it was made on: asked for again +/// on another one it has to be drawn there, since nothing about its geometry +/// says it is in a list that paints at a different moment. #[test] fn a_widget_asked_again_on_another_layer_is_drawn_there() { + /// Draws its child on its own layer, then again one layer in -- which is + /// what a container measuring a child by drawing it used to do. + struct Twice(StrongWidget); + + impl Widget for Twice { + fn draw(&mut self, painter: &mut Painter) -> Size { + let size = painter.widget(&self.0).size(); + painter.child_layer(); + painter.widget(&self.0); + size + } + } + + let mut h = Harness::new((400, 200)); + let (front, draws) = counted(&mut h, Size::from((100, 50)), false); + let outer = Twice(front.add_strong(&mut h.rsc)).add(&mut h.rsc); + h.set_root(outer); + h.frame(); + + assert_ne!( + h.render.active[&front.id()].layer, + h.render.active[&outer.id()].layer, + "the first drawing was kept, on the layer it was measured on" + ); + assert_eq!(draws.get(), 2, "the second ask could not reuse the first"); +} + +/// Which is why `Stack` measures the child that sizes it on the layer that +/// child draws on: one drawing, above the background it stacks over, rather +/// than one on each layer and the wrong one kept. +#[test] +fn a_stacks_sizing_child_is_drawn_once_where_it_belongs() { let mut h = Harness::new((400, 200)); let background = rect(Color::RED).add(&mut h.rsc); let (front, draws) = counted(&mut h, Size::from((100, 50)), false); @@ -582,13 +612,7 @@ fn a_widget_asked_again_on_another_layer_is_drawn_there() { h.frame(); let layer = |id| h.render.active[&id].layer; - assert_ne!( - layer(front.id()), - layer(stack.id()), - "the measured drawing was left on the stack's own layer" - ); + assert_ne!(layer(front.id()), layer(stack.id())); assert_ne!(layer(front.id()), layer(background.id())); - // Measured once for the size and once where it goes, which is what the - // stack costs and not something this test is asserting a number for. - assert_eq!(draws.get(), 2); + assert_eq!(draws.get(), 1); }