iris: a measurement is a mode on the painter, not a discarded draw
Painter::draw_twice(child, first, |used| second) becomes Painter::measure plus an ordinary draw. Iris's objection was the shape it forced on the caller rather than the two draws themselves: the arithmetic that picks the real region had to happen inside a closure, and anything it wanted to keep came back out through a captured &mut. LazySpan::place was the only caller, and it now reads as the three statements it is. DrawMode::Measure is that draw with everything it writes switched off -- no arena slot, no mask, no move slot, nothing left in `active`, nothing marked dirty. Only the returned Size survives, and the widget is left exactly as it was, so the real draw that follows is an ordinary first draw or redraw. That last part is load-bearing: a measurement that left an ActiveData behind would let the following draw hit draw_inner's "already at this region" fast path and return having drawn nothing. A measurement also does not consume a redraw mark, since it is not the redraw the mark asked for, and it takes none of the fast paths, since "already drawn here" cannot report a size. Every Painter method that writes now returns early on the mode -- a widget's own draw never checks, which is the point. A debug_assert at the end of draw_inner catches one that forgot, because the failure otherwise is a single leaked primitive per measured widget per frame, which a screen redrawn every frame turns into an arena that grows without bound. What this is worth, and what it is not. The amplification it applies to, measured on a streamed frame: 1,083 Widget::draw calls over 113 distinct widgets, with the worst drawn 11 times at nesting depth 7-8 -- it is not two draws but two to the power of how many measuring ancestors a widget has. Only the writes go away; the walk and the region arithmetic still happen 11 times, and removing those needs a size answerable without a draw, which LAYOUT.md section 5 rules out. Streamed frame p50 1.39ms -> 1.22ms, p99 4.75ms -> 3.58ms. The upload numbers do not move, because slot recycling had already made the discarded writes free in arena terms. Also extracts move_slot_for from draw_inner, since measuring must not allocate one and the reuse-in-place rule wanted saying once. Verified: run-tests.sh, iris's suite, clippy and rustfmt clean, and the headless phone render is byte-identical to the previous commit's on the real GPU (Venus, RX 7900 XT -- checked, not llvmpipe).
This commit is contained in:
1 parent
4fb369fdd0
commit
2540f6517c
4 files changed
+204
-75
No files matched your search
+108
-33
@@ -2,7 +2,7 @@ use std::sync::Mutex;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use crate::{
|
||||
ActiveData, IdLike, MaskIdx, MoveIdx, Painter, PixelRegion, PrimitiveLayers, RegionAlign,
|
||||
ActiveData, IdLike, MaskIdx, MoveIdx, Painter, PixelRegion, PrimitiveLayers, RegionAlign, Size,
|
||||
StrongWidget, UiRegion, UiRsc, UiVec2, WidgetId, Widgets,
|
||||
render::{
|
||||
Drawn, MoveOffset, NOT_DRAWN, Primitive, PrimitiveHandle, PrimitiveInst, Primitives,
|
||||
@@ -94,6 +94,37 @@ pub struct UiRenderState {
|
||||
last_input_at: Mutex<Option<Instant>>,
|
||||
}
|
||||
|
||||
/// Whether a draw is producing what goes on screen, or only asking a
|
||||
/// widget how big it would be.
|
||||
///
|
||||
/// **There is no size query without a draw** (LAYOUT.md section 5):
|
||||
/// `Widget::draw` reports the size it used, and nothing else can answer
|
||||
/// it. A container that cannot choose what to offer a child without
|
||||
/// already knowing the child's size therefore has to draw it -- so
|
||||
/// [`Self::Measure`] is that draw with everything it *writes* switched
|
||||
/// off. It allocates no arena slot, no mask, no move slot, leaves nothing
|
||||
/// in `active` and marks nothing dirty; the widget is walked and its text
|
||||
/// is shaped (which is memoized, and is the expensive half anyway), and
|
||||
/// only the returned `Size` survives.
|
||||
///
|
||||
/// Because it leaves no trace, the real draw that follows is an ordinary
|
||||
/// first draw or redraw and cannot be short-circuited by the measurement
|
||||
/// having "already drawn" the widget at that region -- which is the trap
|
||||
/// the discarded-draw approach it replaced had to work around.
|
||||
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
|
||||
pub enum DrawMode {
|
||||
/// Write primitives, keep the result in `active`.
|
||||
Draw,
|
||||
/// Report a size and write nothing.
|
||||
Measure,
|
||||
}
|
||||
|
||||
impl DrawMode {
|
||||
fn measuring(self) -> bool {
|
||||
self == Self::Measure
|
||||
}
|
||||
}
|
||||
|
||||
/// What a widget being redrawn keeps from the draw it is replacing.
|
||||
///
|
||||
/// These four always travel together -- they are read off one
|
||||
@@ -102,7 +133,7 @@ pub struct UiRenderState {
|
||||
/// parameters of [`UiRenderState::draw_inner`] until 2026-09-09, next to
|
||||
/// six others. [`Default`] is the "nothing to keep" case: a widget drawn
|
||||
/// for the first time, and the root of a full relayout.
|
||||
pub(super) struct Retained {
|
||||
pub(crate) struct Retained {
|
||||
/// So children this draw does not draw again can be retired.
|
||||
pub children: Vec<WidgetId>,
|
||||
/// Reused in place with its delta reset, never reallocated: a
|
||||
@@ -422,6 +453,7 @@ impl UiRenderState {
|
||||
None,
|
||||
MoveOffset::NONE_PARENT,
|
||||
MaskIdx::NONE,
|
||||
DrawMode::Draw,
|
||||
Retained::default(),
|
||||
rsc,
|
||||
);
|
||||
@@ -455,9 +487,10 @@ impl UiRenderState {
|
||||
parent: Option<WidgetId>,
|
||||
parent_move_slot: u32,
|
||||
mask: MaskIdx,
|
||||
mode: DrawMode,
|
||||
retained: Retained,
|
||||
rsc: &mut dyn UiRsc,
|
||||
) {
|
||||
) -> Size {
|
||||
let Retained {
|
||||
children: mut old_children,
|
||||
move_slot: mut old_move_slot,
|
||||
@@ -466,7 +499,7 @@ impl UiRenderState {
|
||||
} = retained;
|
||||
// Consumed here, not merely read: this call *is* the redraw the mark
|
||||
// asked for, and leaving the mark set is what stranded a widget's
|
||||
// primitives. `Painter::draw_twice` calls this twice for the same id
|
||||
// primitives. A measure-then-draw reaches this twice for the same id
|
||||
// in one frame (`LazySpan::place`'s measurement pass), and on the second
|
||||
// call the still-set mark took the whole `if let` below -- including
|
||||
// the `remove` that frees the first draw's primitives -- out of play,
|
||||
@@ -476,18 +509,24 @@ impl UiRenderState {
|
||||
// and, with `LazySpan` setting no mask, outside the list's own bounds:
|
||||
// the doubled `Compacted:` row in docs/bench/iris-phone-v2-2026-09-06.md.
|
||||
// The same shape reaches any dirty widget an ancestor redraws first.
|
||||
let dirty = rsc.widgets_mut().needs_redraw.remove(&id);
|
||||
// A measurement consumes no redraw mark and takes none of the
|
||||
// fast paths: it is not the redraw the mark asked for, and
|
||||
// "already drawn at this region" would make it return without
|
||||
// reporting a size at all.
|
||||
let dirty = !mode.measuring() && rsc.widgets_mut().needs_redraw.remove(&id);
|
||||
if let Some(active) = self.active.get_mut(&id)
|
||||
&& !dirty
|
||||
&& !mode.measuring()
|
||||
{
|
||||
// check to see if we can skip drawing first
|
||||
if active.region == region {
|
||||
return;
|
||||
return active.size;
|
||||
} else if active.region.size() == region.size() {
|
||||
// TODO: epsilon?
|
||||
let from = active.region;
|
||||
let size = active.size;
|
||||
self.mov(id, from, region, rsc);
|
||||
return;
|
||||
return size;
|
||||
} else if rsc
|
||||
.widgets()
|
||||
.get_dyn(id)
|
||||
@@ -515,7 +554,7 @@ impl UiRenderState {
|
||||
// exactly this step. See `ActiveData::move_applied`, and
|
||||
// `a_size_independent_widget_moved_by_its_parent_has_the_hit_box_it_is_drawn_at`.
|
||||
active.region = region;
|
||||
return;
|
||||
return active.size;
|
||||
}
|
||||
// if not, then maintain resize and track old children to remove unneeded
|
||||
let active = self.remove(id, false, true, rsc).unwrap();
|
||||
@@ -524,6 +563,7 @@ impl UiRenderState {
|
||||
own_mask = active.own_mask;
|
||||
recycle = active.primitives;
|
||||
} else if dirty && self.active.contains_key(&id) {
|
||||
debug_assert!(!mode.measuring());
|
||||
// Dirty and already drawn: none of the fast paths above may be
|
||||
// taken (the widget's own content changed, so its old primitives
|
||||
// say nothing about its new ones), but they are also the only
|
||||
@@ -543,31 +583,12 @@ impl UiRenderState {
|
||||
the second draw's primitives would orphan the first's"
|
||||
);
|
||||
|
||||
let move_slot = match old_move_slot {
|
||||
// Reused across a real redraw of the same id: the fresh
|
||||
// geometry this draw is about to write is placed at its
|
||||
// correct absolute position by `region` itself, so any delta
|
||||
// accumulated before this redraw is now stale and would
|
||||
// double-offset it if left in place. The chain link (`parent`)
|
||||
// is untouched -- the logical parent has not changed.
|
||||
Some(slot) => {
|
||||
let entry = rsc.ui_mut().move_offsets.get_mut(slot);
|
||||
entry.delta = [0.0, 0.0];
|
||||
slot
|
||||
}
|
||||
None => {
|
||||
let slot = rsc
|
||||
.ui_mut()
|
||||
.move_offsets
|
||||
.push(MoveOffset::new([0.0, 0.0], parent_move_slot));
|
||||
rsc.ui_mut().move_offsets.push_ref(slot);
|
||||
if parent_move_slot != MoveOffset::NONE_PARENT {
|
||||
rsc.ui_mut()
|
||||
.move_offsets
|
||||
.push_ref(Id::preset(parent_move_slot));
|
||||
}
|
||||
slot
|
||||
}
|
||||
// A measurement writes no primitive, so nothing ever reads this
|
||||
// -- and allocating one would leak a slot per measured widget per
|
||||
// frame, since `move_offsets` only frees on a widget's removal.
|
||||
let move_slot = match mode {
|
||||
DrawMode::Measure => Id::preset(MoveOffset::NONE_PARENT),
|
||||
DrawMode::Draw => Self::move_slot_for(old_move_slot, parent_move_slot, rsc),
|
||||
};
|
||||
|
||||
// The mask this widget was drawn *under*, kept aside because
|
||||
@@ -591,6 +612,7 @@ impl UiRenderState {
|
||||
primitives: Vec::new(),
|
||||
recycle: recycle.into_iter().peekable(),
|
||||
children: Vec::new(),
|
||||
mode,
|
||||
rsc,
|
||||
};
|
||||
|
||||
@@ -623,8 +645,27 @@ impl UiRenderState {
|
||||
children,
|
||||
layer,
|
||||
id,
|
||||
mode: _,
|
||||
} = painter;
|
||||
|
||||
if mode.measuring() {
|
||||
// Nothing to unwind: a measurement allocates no slot, no
|
||||
// mask, no move offset and no `ActiveData`, so the size is
|
||||
// the whole of what it produced. Asserted rather than
|
||||
// assumed, because a `Painter` method that forgot to check
|
||||
// the mode would otherwise leak silently -- one primitive per
|
||||
// measured widget per frame, which a screen redrawn every
|
||||
// frame turns into an arena that grows without bound.
|
||||
debug_assert!(
|
||||
primitives.is_empty() && textures.is_empty(),
|
||||
"measuring {id:?} wrote {} primitive(s) and {} texture(s); \
|
||||
every `Painter` write must check `Painter::measuring`",
|
||||
primitives.len(),
|
||||
textures.len(),
|
||||
);
|
||||
return size;
|
||||
}
|
||||
|
||||
// Whatever the draw did not claim is genuinely gone: this draw
|
||||
// wrote fewer primitives than the last one, or stopped matching
|
||||
// part way. Freeing it here rather than in `remove` is what lets
|
||||
@@ -660,6 +701,39 @@ impl UiRenderState {
|
||||
|
||||
rsc.on_draw(&active);
|
||||
self.active.insert(id, active);
|
||||
size
|
||||
}
|
||||
|
||||
/// This widget's slot in `move_offsets`: the one it already had if it
|
||||
/// is being redrawn, or a fresh one linked to its parent's.
|
||||
///
|
||||
/// A redraw **reuses the slot in place with its delta reset**, never
|
||||
/// reallocates: the geometry this draw is about to write is already
|
||||
/// at its correct absolute position, so a delta accumulated before it
|
||||
/// would double-offset it -- while the chain link (`parent`) is left
|
||||
/// alone, because the logical parent has not changed and a descendant
|
||||
/// that is not itself redrawn still points here. See LAYOUT.md
|
||||
/// section 2.
|
||||
fn move_slot_for(old: Option<MoveIdx>, parent_move_slot: u32, rsc: &mut dyn UiRsc) -> MoveIdx {
|
||||
match old {
|
||||
Some(slot) => {
|
||||
rsc.ui_mut().move_offsets.get_mut(slot).delta = [0.0, 0.0];
|
||||
slot
|
||||
}
|
||||
None => {
|
||||
let slot = rsc
|
||||
.ui_mut()
|
||||
.move_offsets
|
||||
.push(MoveOffset::new([0.0, 0.0], parent_move_slot));
|
||||
rsc.ui_mut().move_offsets.push_ref(slot);
|
||||
if parent_move_slot != MoveOffset::NONE_PARENT {
|
||||
rsc.ui_mut()
|
||||
.move_offsets
|
||||
.push_ref(Id::preset(parent_move_slot));
|
||||
}
|
||||
slot
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// O(1): write the delta for this widget's own slot in
|
||||
@@ -1217,6 +1291,7 @@ impl UiRenderState {
|
||||
parent,
|
||||
parent_move_slot,
|
||||
active.mask,
|
||||
DrawMode::Draw,
|
||||
Retained {
|
||||
children: active.children,
|
||||
move_slot: Some(active.move_slot),
|
||||
|
||||
Reference in new issue
Block a user