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 <noreply@anthropic.com>
This commit is contained in:
1 parent
95fb4f962c
commit
97cc8b32ed
5 files changed
+73
-20
No files matched your search
@@ -40,6 +40,8 @@ pub(crate) enum Counter {
|
|||||||
ReuseWrongParent,
|
ReuseWrongParent,
|
||||||
ReuseRemapped,
|
ReuseRemapped,
|
||||||
ReuseOutside,
|
ReuseOutside,
|
||||||
|
ReuseWrongLayer,
|
||||||
|
ReuseWrongNode,
|
||||||
PlaceRedraws,
|
PlaceRedraws,
|
||||||
QueuePops,
|
QueuePops,
|
||||||
DepthReads,
|
DepthReads,
|
||||||
@@ -73,6 +75,8 @@ impl Counter {
|
|||||||
"reuse: wrong parent",
|
"reuse: wrong parent",
|
||||||
"reuse remapped",
|
"reuse remapped",
|
||||||
"reuse: outside what it holds for",
|
"reuse: outside what it holds for",
|
||||||
|
"reuse: another layer",
|
||||||
|
"reuse: region-node choice changed",
|
||||||
"placed by redrawing",
|
"placed by redrawing",
|
||||||
"redraw queue pops",
|
"redraw queue pops",
|
||||||
"depth reads",
|
"depth reads",
|
||||||
|
|||||||
@@ -42,6 +42,9 @@ pub struct Painter<'a> {
|
|||||||
/// its own when opted in, otherwise the nearest ancestor's.
|
/// its own when opted in, otherwise the nearest ancestor's.
|
||||||
pub(super) move_idx: MoveIdx,
|
pub(super) move_idx: MoveIdx,
|
||||||
pub layer: usize,
|
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) depth: usize,
|
||||||
pub(super) id: WidgetId,
|
pub(super) id: WidgetId,
|
||||||
}
|
}
|
||||||
@@ -420,6 +423,18 @@ impl<'a> Painter<'a> {
|
|||||||
self.layer = self.state.layers.child(self.layer);
|
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) {
|
pub fn next_layer(&mut self) {
|
||||||
self.layer = self.state.layers.next(self.layer);
|
self.layer = self.state.layers.next(self.layer);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -307,6 +307,7 @@ impl UiRenderState {
|
|||||||
region: local,
|
region: local,
|
||||||
mask: info.mask,
|
mask: info.mask,
|
||||||
layer: info.layer,
|
layer: info.layer,
|
||||||
|
own_layer: info.layer,
|
||||||
id,
|
id,
|
||||||
textures: Vec::new(),
|
textures: Vec::new(),
|
||||||
primitives: Vec::new(),
|
primitives: Vec::new(),
|
||||||
@@ -349,6 +350,7 @@ impl UiRenderState {
|
|||||||
under,
|
under,
|
||||||
move_idx,
|
move_idx,
|
||||||
layer,
|
layer,
|
||||||
|
own_layer: _,
|
||||||
depth: _,
|
depth: _,
|
||||||
id,
|
id,
|
||||||
} = painter;
|
} = painter;
|
||||||
@@ -586,6 +588,8 @@ impl UiRenderState {
|
|||||||
}
|
}
|
||||||
let has_region_node = active.move_idx != active.parent_move;
|
let has_region_node = active.move_idx != active.parent_move;
|
||||||
if has_region_node != info.region_node {
|
if has_region_node != info.region_node {
|
||||||
|
#[cfg(feature = "layout-diagnostics")]
|
||||||
|
diag::bump(Counter::ReuseWrongNode);
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
// Drawn on another layer: the drawing sits in that layer's list and
|
// 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.
|
// on a layer the first answer is not good for.
|
||||||
if active.layer != info.layer {
|
if active.layer != info.layer {
|
||||||
#[cfg(feature = "layout-diagnostics")]
|
#[cfg(feature = "layout-diagnostics")]
|
||||||
diag::reuse(id, ReuseOutcome::WrongLayer);
|
{
|
||||||
|
diag::bump(Counter::ReuseWrongLayer);
|
||||||
|
diag::reuse(id, ReuseOutcome::WrongLayer);
|
||||||
|
}
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
// Drawn somewhere else in the tree: its box is in coordinates it no
|
// Drawn somewhere else in the tree: its box is in coordinates it no
|
||||||
|
|||||||
@@ -16,16 +16,19 @@ impl Widget for Stack {
|
|||||||
// Whichever child sizes the stack decides the box every child gets.
|
// Whichever child sizes the stack decides the box every child gets.
|
||||||
// The stack reports that size, so a child given a longer box would
|
// The stack reports that size, so a child given a longer box would
|
||||||
// draw outside what the stack says it occupies.
|
// draw outside what the stack says it occupies.
|
||||||
let size = match sizing.and_then(|i| self.children.get(i)) {
|
let size = match sizing.and_then(|i| self.children.get(i).map(|c| (i, c))) {
|
||||||
Some(child) => painter.widget(child).size(),
|
// 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,
|
None => Size::LEFTOVER,
|
||||||
};
|
};
|
||||||
let region = painter.box_of(size);
|
let region = painter.box_of(size);
|
||||||
for (i, child) in self.children.iter().enumerate() {
|
for (i, child) in self.children.iter().enumerate() {
|
||||||
match i {
|
painter.child_layer_at(i);
|
||||||
0 => painter.child_layer(),
|
|
||||||
_ => painter.next_layer(),
|
|
||||||
}
|
|
||||||
painter.widget_aligned(child, region, RegionAlign::NEAR);
|
painter.widget_aligned(child, region, RegionAlign::NEAR);
|
||||||
}
|
}
|
||||||
size
|
size
|
||||||
|
|||||||
+37
-13
@@ -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));
|
assert_corners!(h, fixed, (200, 0), (280, 200));
|
||||||
}
|
}
|
||||||
|
|
||||||
/// `Stack` measures the child that sizes it by drawing it, then draws it
|
/// A retained drawing belongs to the layer it was made on: asked for again
|
||||||
/// again above the background it stacks over. The second ask is for the same
|
/// on another one it has to be drawn there, since nothing about its geometry
|
||||||
/// box, so nothing geometric says the answer has gone stale, and reusing it
|
/// says it is in a list that paints at a different moment.
|
||||||
/// leaves the drawing under the background. The same tree got away with it
|
|
||||||
/// while the two asks differed by a rounding.
|
|
||||||
#[test]
|
#[test]
|
||||||
fn a_widget_asked_again_on_another_layer_is_drawn_there() {
|
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 mut h = Harness::new((400, 200));
|
||||||
let background = rect(Color::RED).add(&mut h.rsc);
|
let background = rect(Color::RED).add(&mut h.rsc);
|
||||||
let (front, draws) = counted(&mut h, Size::from((100, 50)), false);
|
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();
|
h.frame();
|
||||||
|
|
||||||
let layer = |id| h.render.active[&id].layer;
|
let layer = |id| h.render.active[&id].layer;
|
||||||
assert_ne!(
|
assert_ne!(layer(front.id()), layer(stack.id()));
|
||||||
layer(front.id()),
|
|
||||||
layer(stack.id()),
|
|
||||||
"the measured drawing was left on the stack's own layer"
|
|
||||||
);
|
|
||||||
assert_ne!(layer(front.id()), layer(background.id()));
|
assert_ne!(layer(front.id()), layer(background.id()));
|
||||||
// Measured once for the size and once where it goes, which is what the
|
assert_eq!(draws.get(), 1);
|
||||||
// stack costs and not something this test is asserting a number for.
|
|
||||||
assert_eq!(draws.get(), 2);
|
|
||||||
}
|
}
|
||||||
Reference in new issue
Block a user