Carry a widget's depth down the draw instead of walking up for it
Choosing which dirty widget to settle next asked every one of them how deep it was, and answering meant walking its ancestry to the root. At 130 of 260 widgets dirty that was 25.8% of the frame -- more than laying out or rendering. A widget's depth is known where it is drawn: its parent's plus one. So `Painter` carries it and `ActiveData` keeps it, and the choice reads a field. Being reused counts as being visited, so the two reuse paths keep it current too; only a subtree nothing looked at can hold an old one, and nothing under an unvisited subtree is being ordered. The order is unchanged, so nothing about the layout is: the five reference renders and the resize render are byte-identical. What the carried depth might get wrong is itself, so `depth` asserts it against the ancestry in debug builds, and the hundred-seed sweep passes with those assertions on -- including the reshuffles, which are what move a widget to another parent. Same load, 1000 frames, 130 of 260 dirty: 8.16M instructions per frame to 7.14M, median 0.813 ms to 0.639, and the choosing from 25.8% of the frame to 4.7%. What is left of it is iterating the dirty set itself, which a `HashSet` walks by capacity rather than by length. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
bf9438087a
commit
3f7cd8251b
4 files changed
+40
-5
No files matched your search
@@ -45,7 +45,7 @@ pub(crate) enum Counter {
|
|||||||
ResizeChecks,
|
ResizeChecks,
|
||||||
ResizeCheckChildren,
|
ResizeCheckChildren,
|
||||||
QueuePops,
|
QueuePops,
|
||||||
DepthSteps,
|
DepthReads,
|
||||||
EagerReaderRedraws,
|
EagerReaderRedraws,
|
||||||
LocalRedraws,
|
LocalRedraws,
|
||||||
SizeChanges,
|
SizeChanges,
|
||||||
@@ -82,7 +82,7 @@ impl Counter {
|
|||||||
"resize checks",
|
"resize checks",
|
||||||
"resize children checked",
|
"resize children checked",
|
||||||
"redraw queue pops",
|
"redraw queue pops",
|
||||||
"depth steps",
|
"depth reads",
|
||||||
"eager reader redraws",
|
"eager reader redraws",
|
||||||
"local redraws",
|
"local redraws",
|
||||||
"size changes",
|
"size changes",
|
||||||
|
|||||||
@@ -14,6 +14,10 @@ pub struct ActiveData {
|
|||||||
/// has since changed is a different number of pixels.
|
/// has since changed is a different number of pixels.
|
||||||
pub px: Vec2,
|
pub px: Vec2,
|
||||||
pub parent: Option<WidgetId>,
|
pub parent: Option<WidgetId>,
|
||||||
|
/// How far down the tree it was drawn, the root being 1. Carried down a
|
||||||
|
/// draw rather than worked out by walking up, so it is right for every
|
||||||
|
/// widget a frame visits and cannot drift while one is being drawn.
|
||||||
|
pub depth: usize,
|
||||||
pub textures: Vec<TextureHandle>,
|
pub textures: Vec<TextureHandle>,
|
||||||
pub primitives: Vec<PrimitiveHandle>,
|
pub primitives: Vec<PrimitiveHandle>,
|
||||||
pub children: Vec<WidgetId>,
|
pub children: Vec<WidgetId>,
|
||||||
|
|||||||
@@ -31,6 +31,7 @@ pub struct Painter<'a> {
|
|||||||
/// its parent placed it, otherwise the nearest ancestor that has one.
|
/// its parent placed it, otherwise the nearest ancestor that has one.
|
||||||
pub(super) move_idx: MoveIdx,
|
pub(super) move_idx: MoveIdx,
|
||||||
pub layer: usize,
|
pub layer: usize,
|
||||||
|
pub(super) depth: usize,
|
||||||
pub(super) id: WidgetId,
|
pub(super) id: WidgetId,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -134,6 +135,7 @@ impl<'a> Painter<'a> {
|
|||||||
id.id(),
|
id.id(),
|
||||||
region,
|
region,
|
||||||
Some(self.id),
|
Some(self.id),
|
||||||
|
self.depth + 1,
|
||||||
self.move_idx,
|
self.move_idx,
|
||||||
slotted,
|
slotted,
|
||||||
self.mask,
|
self.mask,
|
||||||
|
|||||||
@@ -155,6 +155,7 @@ impl UiRenderState {
|
|||||||
id.id(),
|
id.id(),
|
||||||
UiRegion::FULL,
|
UiRegion::FULL,
|
||||||
None,
|
None,
|
||||||
|
1,
|
||||||
MoveIdx::NONE,
|
MoveIdx::NONE,
|
||||||
false,
|
false,
|
||||||
MaskIdx::NONE,
|
MaskIdx::NONE,
|
||||||
@@ -172,6 +173,7 @@ impl UiRenderState {
|
|||||||
id: WidgetId,
|
id: WidgetId,
|
||||||
region: UiRegion,
|
region: UiRegion,
|
||||||
parent: Option<WidgetId>,
|
parent: Option<WidgetId>,
|
||||||
|
depth: usize,
|
||||||
parent_move: MoveIdx,
|
parent_move: MoveIdx,
|
||||||
slotted: bool,
|
slotted: bool,
|
||||||
mask: MaskIdx,
|
mask: MaskIdx,
|
||||||
@@ -185,7 +187,7 @@ impl UiRenderState {
|
|||||||
}
|
}
|
||||||
let mut old_children = old_children.unwrap_or_default();
|
let mut old_children = old_children.unwrap_or_default();
|
||||||
if self.active.contains_key(&id) {
|
if self.active.contains_key(&id) {
|
||||||
if let Some(size) = self.try_reuse(id, region, parent_move, rsc) {
|
if let Some(size) = self.try_reuse(id, region, depth, parent_move, rsc) {
|
||||||
return size;
|
return size;
|
||||||
}
|
}
|
||||||
// if not, then maintain resize and track old children to remove unneeded
|
// if not, then maintain resize and track old children to remove unneeded
|
||||||
@@ -217,6 +219,7 @@ impl UiRenderState {
|
|||||||
primitives: Vec::new(),
|
primitives: Vec::new(),
|
||||||
children: Vec::new(),
|
children: Vec::new(),
|
||||||
size_deps: Vec::new(),
|
size_deps: Vec::new(),
|
||||||
|
depth,
|
||||||
size_box_inputs: [false; 2],
|
size_box_inputs: [false; 2],
|
||||||
size_output_inputs: [false; 2],
|
size_output_inputs: [false; 2],
|
||||||
reads_output: [false; 2],
|
reads_output: [false; 2],
|
||||||
@@ -249,6 +252,7 @@ impl UiRenderState {
|
|||||||
reads_output,
|
reads_output,
|
||||||
move_idx,
|
move_idx,
|
||||||
layer,
|
layer,
|
||||||
|
depth: _,
|
||||||
id,
|
id,
|
||||||
} = painter;
|
} = painter;
|
||||||
|
|
||||||
@@ -265,6 +269,7 @@ impl UiRenderState {
|
|||||||
size,
|
size,
|
||||||
px,
|
px,
|
||||||
parent,
|
parent,
|
||||||
|
depth,
|
||||||
textures,
|
textures,
|
||||||
primitives,
|
primitives,
|
||||||
children,
|
children,
|
||||||
@@ -383,6 +388,7 @@ impl UiRenderState {
|
|||||||
&mut self,
|
&mut self,
|
||||||
id: WidgetId,
|
id: WidgetId,
|
||||||
region: UiRegion,
|
region: UiRegion,
|
||||||
|
depth: usize,
|
||||||
parent_move: MoveIdx,
|
parent_move: MoveIdx,
|
||||||
rsc: &mut dyn UiRsc,
|
rsc: &mut dyn UiRsc,
|
||||||
) -> Option<Size> {
|
) -> Option<Size> {
|
||||||
@@ -429,6 +435,7 @@ impl UiRenderState {
|
|||||||
diag::bump(Counter::ReuseExact);
|
diag::bump(Counter::ReuseExact);
|
||||||
diag::reuse(id, ReuseOutcome::Exact);
|
diag::reuse(id, ReuseOutcome::Exact);
|
||||||
}
|
}
|
||||||
|
self.keep_depth(id, depth);
|
||||||
return Some(size);
|
return Some(size);
|
||||||
}
|
}
|
||||||
// Only a placed widget can be given a different box without drawing
|
// Only a placed widget can be given a different box without drawing
|
||||||
@@ -469,6 +476,7 @@ impl UiRenderState {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
self.moves.set(slot, region);
|
self.moves.set(slot, region);
|
||||||
|
self.keep_depth(id, depth);
|
||||||
let active = self.active.get_mut(&id).unwrap();
|
let active = self.active.get_mut(&id).unwrap();
|
||||||
active.region = region;
|
active.region = region;
|
||||||
#[cfg(feature = "layout-diagnostics")]
|
#[cfg(feature = "layout-diagnostics")]
|
||||||
@@ -614,12 +622,32 @@ impl UiRenderState {
|
|||||||
rsc.free();
|
rsc.free();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Keeps a reused widget's depth current, since being reused is being
|
||||||
|
/// visited: only a subtree nobody looked at can hold a stale one.
|
||||||
|
fn keep_depth(&mut self, id: WidgetId, depth: usize) {
|
||||||
|
if let Some(active) = self.active.get_mut(&id) {
|
||||||
|
active.depth = depth;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn depth(&self, id: WidgetId) -> usize {
|
fn depth(&self, id: WidgetId) -> usize {
|
||||||
|
#[cfg(feature = "layout-diagnostics")]
|
||||||
|
diag::bump(Counter::DepthReads);
|
||||||
|
let depth = self.active.get(&id).map_or(1, |active| active.depth);
|
||||||
|
debug_assert_eq!(
|
||||||
|
depth,
|
||||||
|
self.walked_depth(id),
|
||||||
|
"a widget's kept depth is not the one its ancestry says"
|
||||||
|
);
|
||||||
|
depth
|
||||||
|
}
|
||||||
|
|
||||||
|
/// What the kept depth is checked against, and the only thing that reads
|
||||||
|
/// the ancestry to find one.
|
||||||
|
fn walked_depth(&self, id: WidgetId) -> usize {
|
||||||
let mut depth = 0;
|
let mut depth = 0;
|
||||||
let mut at = Some(id);
|
let mut at = Some(id);
|
||||||
while let Some(id) = at {
|
while let Some(id) = at {
|
||||||
#[cfg(feature = "layout-diagnostics")]
|
|
||||||
diag::bump(Counter::DepthSteps);
|
|
||||||
at = self.active.get(&id).and_then(|active| active.parent);
|
at = self.active.get(&id).and_then(|active| active.parent);
|
||||||
depth += 1;
|
depth += 1;
|
||||||
}
|
}
|
||||||
@@ -714,6 +742,7 @@ impl UiRenderState {
|
|||||||
id,
|
id,
|
||||||
active.region,
|
active.region,
|
||||||
active.parent,
|
active.parent,
|
||||||
|
active.depth,
|
||||||
active.parent_move,
|
active.parent_move,
|
||||||
active.move_idx != active.parent_move,
|
active.move_idx != active.parent_move,
|
||||||
active.mask,
|
active.mask,
|
||||||
|
|||||||
Reference in new issue
Block a user