From 97fca7610864135bf639970f984084d7a7bf9384 Mon Sep 17 00:00:00 2001 From: iris-ai <4+iris-ai@noreply.localhost> Date: Sun, 20 Sep 2026 23:56:12 -0400 Subject: [PATCH] Answer "have I asked this child?" in one read A container's draw asked it once per child by searching the list of children it had added so far, and four other per-child steps searched a list too, so one draw cost the square of its children: 70% of a 1,600-child redraw was those searches. A draw takes a DrawId and leaves it on every widget it asks about; one note per widget is enough because the handle a container holds a child by cannot be cloned. tests/children_cost.rs is the rig that shows it, and it is the only one here that varies width: 3.680 ms to 0.811 ms at 1,600 children, and flat per child. Beside it, the rest of the fourteenth sweep of #19: a mask's rectangle resolved once per fragment instead of once per instance, which takes the storage buffers out of the fragment stage and is 8.8x on a screenful of deeply nested clips; TextBuffer::shape copying its attrs before the check that would not need them, which allocated once per named-family text per frame; a should_panic test on a debug assertion that made cargo test --release fail; Fixed::div, reached only by its own test; Moves::remove re-uploading an array it cannot have changed; and two comments the branch itself falsified. docs/LAYOUT_LOG.md has all eight with their measurements, the five things looked at and left, and what was verified. --- core/src/fixed.rs | 38 +------ core/src/primitive/text.rs | 66 ++++++------ core/src/render/mod.rs | 8 +- core/src/render/shader/prelude.wgsl | 48 +++++---- core/src/ui/active.rs | 23 ++++ core/src/ui/mod.rs | 5 +- core/src/ui/painter.rs | 50 +++++++-- core/src/ui/render_state.rs | 42 +++++++- src/widget/mask.rs | 13 ++- src/widget/trait_fns.rs | 6 +- tests/allocation_cost.rs | 44 ++++++++ tests/cases/retained.rs | 27 +++++ tests/cases/scroll.rs | 5 + tests/chain_cost.rs | 111 +++++++++++++------ tests/children_cost.rs | 59 ++++++++++ tests/gpu/mod.rs | 30 +++++- tests/mask_clip.rs | 162 ++++++++++++++++++++++++++++ 17 files changed, 588 insertions(+), 149 deletions(-) create mode 100644 tests/children_cost.rs create mode 100644 tests/mask_clip.rs diff --git a/core/src/fixed.rs b/core/src/fixed.rs index d5aad3b..7edd55c 100644 --- a/core/src/fixed.rs +++ b/core/src/fixed.rs @@ -1,7 +1,7 @@ use crate::{UiNum, util::Vec2}; use std::{ fmt::{Debug, Display, Formatter}, - ops::{Add, AddAssign, Div, Mul, Neg, Sub, SubAssign}, + ops::{Add, AddAssign, Mul, Neg, Sub, SubAssign}, }; /// A number held as a whole count of `1 / 2^SHIFT`. @@ -175,21 +175,6 @@ impl Fixed { Self(div_round(self.0 as i64, by as i64) as i32) } - /// Divided by a number on any grid. A zero divisor is a caller bug -- a - /// box of no length has no fraction of itself -- and answers with the end - /// of the range so that a release build lays out something absurd rather - /// than dying. - pub const fn div(self, by: Fixed) -> Self { - debug_assert!(by.0 != 0, "dividing by a length of zero"); - if by.0 == 0 { - return match self.0 < 0 { - true => Self::MIN, - false => Self::MAX, - }; - } - Self(div_round((self.0 as i64) << BY, by.0 as i64) as i32) - } - /// `num / den` on *this* grid rather than on theirs, for weights coarser /// than the share they divide. pub const fn ratio(num: Fixed, den: Fixed) -> Self { @@ -320,14 +305,6 @@ const impl Mul> for Fixed { } } -const impl Div> for Fixed { - type Output = Self; - - fn div(self, rhs: Fixed) -> Self { - Fixed::div(self, rhs) - } -} - impl Display for Fixed { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { Display::fmt(&self.to_f32(), f) @@ -478,19 +455,6 @@ mod tests { assert_eq!(Px::ONE.neg() * step_and_a_half, Px::from_raw(-2)); } - /// A division rounds to the nearest step, so it cannot put back the - /// steps a truncating multiply dropped: a round trip comes back short, - /// never long, and by the few steps the two operations gave up. - #[test] - fn dividing_by_a_fraction_cannot_undo_a_truncating_multiply() { - let third = Rel::ONE / Rel::from_int(3); - let len = Px::from_int(300); - let back = len * third / third; - assert!(back <= len, "{back:?} is longer than {len:?}"); - assert!(len - back <= Px::from_raw(3), "{back:?} against {len:?}"); - assert_eq!(Px::from_int(100) / Rel::from_f32(0.5), Px::from_int(200)); - } - /// The bound a greedy line break needs: the width it was measured at is /// not on the grid, and the narrowest box the break still holds for is /// the step at or above it, never the one below. diff --git a/core/src/primitive/text.rs b/core/src/primitive/text.rs index b808435..05b99f5 100644 --- a/core/src/primitive/text.rs +++ b/core/src/primitive/text.rs @@ -194,41 +194,25 @@ impl TextBuffer { } pub fn shape(&mut self, data: &mut TextData, attrs: &TextAttrs, width: Option) { - let layout_key = LayoutKey { - attrs: attrs.clone(), - max_width: width, - }; - if self.layout_key.as_ref() == Some(&layout_key) { - #[cfg(feature = "layout-diagnostics")] - diag::bump(Counter::TextShapeHits); - return; - } - // A greedy break at one width is the same break at every width down - // to the longest line it produced: each line still fits, and none can - // take a word that would not fit in the wider box. So the layout in - // hand already answers, and re-breaking would only be work. - // - // At the longest line exactly, with no margin below it. A narrower - // width really does break differently, so answering one from the - // break in hand is how a warm tree keeps lines a cold tree would - // never produce. The margin was here because a text reports the - // width it used and a parent hands that back; the report is the step - // at or above its longest line now, so what comes back fits. - if let Some(key) = &self.layout_key - && key.attrs == *attrs - && let (Some(broke_at), Some(want)) = (key.max_width, width) - && want <= broke_at - && want >= self.layout.width() + // Asked of the attrs it was given rather than of a copy: copying one + // allocates wherever its family is named, and the hit below is what + // this cache is for. + let same_shaping = self + .layout_key + .as_ref() + .is_some_and(|key| key.attrs == *attrs); + if same_shaping + && let Some(key) = &self.layout_key + && self.breaks_the_same(key.max_width, width) { #[cfg(feature = "layout-diagnostics")] diag::bump(Counter::TextShapeHits); return; } - let same_shaping = self - .layout_key - .as_ref() - .is_some_and(|key| key.attrs == *attrs); - let old_key = self.layout_key.replace(layout_key); + let old_key = self.layout_key.replace(LayoutKey { + attrs: attrs.clone(), + max_width: width, + }); // The glyphs it holds are of the width it held, which the layout may // well come back to. if let Some(key) = old_key @@ -268,6 +252,28 @@ impl TextBuffer { self.break_lines(width); } + /// Whether the break in hand is the break `want` would make. The attrs + /// are the caller's to compare; this is about the width alone. + /// + /// A greedy break at one width is the same break at every width down to + /// the longest line it produced: each line still fits, and none can take a + /// word that would not fit in the wider box. So the layout in hand already + /// answers, and re-breaking would only be work. + /// + /// At the longest line exactly, with no margin below it. A narrower width + /// really does break differently, so answering one from the break in hand + /// is how a warm tree keeps lines a cold tree would never produce. The + /// margin was here because a text reports the width it used and a parent + /// hands that back; the report is the step at or above its longest line + /// now, so what comes back fits. + fn breaks_the_same(&self, broke_at: Option, want: Option) -> bool { + match (broke_at, want) { + (broke_at, want) if broke_at == want => true, + (Some(broke_at), Some(want)) => want <= broke_at && want >= self.layout.width(), + _ => false, + } + } + fn break_lines(&mut self, width: Option) { self.layout.break_all_lines(width); self.layout diff --git a/core/src/render/mod.rs b/core/src/render/mod.rs index 4dd265a..571f279 100644 --- a/core/src/render/mod.rs +++ b/core/src/render/mod.rs @@ -285,7 +285,9 @@ impl UiRenderNode { } /// What every draw in the ui is given: the window, the masks and the - /// move chain every position is resolved through. + /// move chain every position is resolved through. The last two are the + /// vertex stage's alone -- a mask's rectangle is the same for every + /// fragment of one instance, so it is resolved once and handed on. fn shared_layout(device: &Device) -> BindGroupLayout { device.create_bind_group_layout(&BindGroupLayoutDescriptor { entries: &[ @@ -301,7 +303,7 @@ impl UiRenderNode { }, BindGroupLayoutEntry { binding: 1, - visibility: ShaderStages::FRAGMENT, + visibility: ShaderStages::VERTEX, ty: BindingType::Buffer { ty: BufferBindingType::Storage { read_only: true }, has_dynamic_offset: false, @@ -311,7 +313,7 @@ impl UiRenderNode { }, BindGroupLayoutEntry { binding: 2, - visibility: ShaderStages::VERTEX | ShaderStages::FRAGMENT, + visibility: ShaderStages::VERTEX, ty: BindingType::Buffer { ty: BufferBindingType::Storage { read_only: true }, has_dynamic_offset: false, diff --git a/core/src/render/shader/prelude.wgsl b/core/src/render/shader/prelude.wgsl index 510f1c3..3b48c5f 100644 --- a/core/src/render/shader/prelude.wgsl +++ b/core/src/render/shader/prelude.wgsl @@ -126,9 +126,25 @@ struct VertexOutput { @location(2) uv: vec2, @location(3) @interpolate(flat) mask_idx: u32, @location(4) @interpolate(flat) idx: u32, + // The mask's rectangle in output pixels, resolved here because it is the + // same rectangle for every fragment of one instance and resolving it is a + // walk up a chain. Its own chain, not the drawn primitive's, so a + // stationary viewport clips content that moves inside it. + @location(5) @interpolate(flat) mask_top_left: vec2, + @location(6) @interpolate(flat) mask_bot_right: vec2, @builtin(position) clip_position: vec4, }; +// The pixel corners of a region, which is where every coordinate the CPU +// decided becomes one. +fn corners(r: Region) -> mat2x2 { + let top_left = snap_floor(vec2(r.x.start.rel, r.y.start.rel) * window.dim + + vec2(r.x.start.px, r.y.start.px)); + let bot_right = snap_floor(vec2(r.x.end.rel, r.y.end.rel) * window.dim + + vec2(r.x.end.px, r.y.end.px)); + return mat2x2(top_left, bot_right); +} + @vertex fn vs_main( @builtin(vertex_index) vi: u32, @@ -141,16 +157,17 @@ fn vs_main( UiSpan(scalar_of_pair(in.x_start), scalar_of_pair(in.x_end)), UiSpan(scalar_of_pair(in.y_start), scalar_of_pair(in.y_end)), ); - let r = resolve_move(in.move_idx, local); - let top_left_rel = vec2(r.x.start.rel, r.y.start.rel); - let top_left_px = vec2(r.x.start.px, r.y.start.px); - let bot_right_rel = vec2(r.x.end.rel, r.y.end.rel); - let bot_right_px = vec2(r.x.end.px, r.y.end.px); - - let top_left = snap_floor(top_left_rel * window.dim + top_left_px); - let bot_right = snap_floor(bot_right_rel * window.dim + bot_right_px); + let own = corners(resolve_move(in.move_idx, local)); + let top_left = own[0]; + let bot_right = own[1]; let size = bot_right - top_left; + var mask = mat2x2(vec2(0.0), vec2(0.0)); + if in.mask_idx != MASK_NONE { + let m = masks[in.mask_idx]; + mask = corners(resolve_move(m.move_idx, Region(span_of(m.x), span_of(m.y)))); + } + let uv = vec2( f32(vi % 2u), f32(vi / 2u) @@ -161,6 +178,8 @@ fn vs_main( out.top_left = top_left; out.bot_right = bot_right; out.mask_idx = in.mask_idx; + out.mask_top_left = mask[0]; + out.mask_bot_right = mask[1]; out.idx = ii; return out; @@ -170,17 +189,8 @@ fn masked(in: VertexOutput, color: vec4) -> vec4 { if in.mask_idx == MASK_NONE { return color; } - let mask = masks[in.mask_idx]; - // Its own chain, not the drawn primitive's, so a stationary viewport - // clips content that moves inside it. - let m = resolve_move(mask.move_idx, Region(span_of(mask.x), span_of(mask.y))); - let tl = vec2(m.x.start.rel, m.y.start.rel); - let tl_px = vec2(m.x.start.px, m.y.start.px); - let br = vec2(m.x.end.rel, m.y.end.rel); - let br_px = vec2(m.x.end.px, m.y.end.px); - - let top_left = snap_floor(tl * window.dim + tl_px); - let bot_right = snap_floor(br * window.dim + br_px); + let top_left = in.mask_top_left; + let bot_right = in.mask_bot_right; let pos = in.clip_position.xy; if pos.x < top_left.x || pos.x > bot_right.x || pos.y < top_left.y || pos.y > bot_right.y { return color * 0.0; diff --git a/core/src/ui/active.rs b/core/src/ui/active.rs index af2babf..e0f220f 100644 --- a/core/src/ui/active.rs +++ b/core/src/ui/active.rs @@ -3,6 +3,23 @@ use crate::{ RetainedPrimitive, Size, TextureHandle, UiRegion, UiVec2, WidgetId, }; +/// One draw of one widget, so that a widget it asked about can carry which +/// draw that was. Its own type beside the indices here because it names an +/// occasion rather than a slot: nothing is stored per draw. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct DrawId(u64); + +impl DrawId { + /// No draw at all, which is what a widget nothing has asked about carries. + pub const NONE: Self = Self(0); + + /// The next one after this. Handed out in order and never reused, so a + /// note left by an earlier draw can never be read as this one's. + pub(crate) fn next(self) -> Self { + Self(self.0 + 1) + } +} + /// What is kept of a widget its parent has asked about. `drawn` says whether /// it currently draws; one that does not is kept so that a change to it, or /// under it, still reaches whoever asked. @@ -28,6 +45,12 @@ pub struct ActiveData { /// The measured answer and its dependencies. A hint-only dependency or /// a widget first encountered during placement has no measurement yet. pub answer: Option, + /// The draw that last asked about this widget, which is what says whether + /// the draw now running has already asked -- a question the child list can + /// only answer by a search, and so in the children a container has rather + /// than in one read. Written by whoever asked, so a draw of this widget + /// itself carries it across rather than setting it. + pub(crate) asked_by: DrawId, /// Asked more than once in its parent's last draw -- measured in one box /// and then asked in the one the parent decided. The parent's layout /// rests on the first answer and its drawing on the last, so only the diff --git a/core/src/ui/mod.rs b/core/src/ui/mod.rs index 7ff905d..e4944c2 100644 --- a/core/src/ui/mod.rs +++ b/core/src/ui/mod.rs @@ -57,8 +57,11 @@ impl Moves { } } + /// Frees a slot. Not a change to the entries: the slot keeps the bytes it + /// had, nothing names it until it is handed out again, and whoever is + /// handed it writes it then -- so re-uploading the array here would send + /// the GPU what it already has. pub fn remove(&mut self, idx: MoveIdx) { - self.changed = true; self.arena.remove(Id::preset(idx.idx() as u32)); } diff --git a/core/src/ui/painter.rs b/core/src/ui/painter.rs index 8aa0a69..58a35a8 100644 --- a/core/src/ui/painter.rs +++ b/core/src/ui/painter.rs @@ -1,8 +1,8 @@ #[cfg(feature = "layout-diagnostics")] use crate::layout_diagnostics::{self as diag, Counter}; use crate::{ - Axis, Bound, Bounds, Declared, DrawScratch, Holds, LayoutHolds, LayoutLen, Len, PlaceDesc, - PlaceFit, Px, PxVec2, RegionAlign, Rel, RenderedText, RequestArena, RequestedLen, + Axis, Bound, Bounds, Declared, DrawId, DrawScratch, Holds, LayoutHolds, LayoutLen, Len, + PlaceDesc, PlaceFit, Px, PxVec2, RegionAlign, Rel, RenderedText, RequestArena, RequestedLen, RetainedPrimitive, Size, SizeRequests, StrongWidget, TextAttrs, TextBuffer, TextureHandle, UiRegion, UiRenderState, UiRsc, UiVec2, Weight, WidgetId, Widgets, render::{ @@ -61,6 +61,8 @@ pub struct Painter<'a> { /// counted from however far `layer` has walked. pub(super) own_layer: usize, pub(super) depth: usize, + /// This draw's own id, which it leaves on every widget it asks about. + pub(super) draw: DrawId, pub(super) id: WidgetId, } @@ -339,7 +341,12 @@ impl<'a> Painter<'a> { diag::region_node(id.id(), self.id, region); } // A child listed twice would be moved twice. - let re_asked = self.children.contains(&id.id()); + let re_asked = self.state.asked_in(id.id(), self.draw); + debug_assert_eq!( + re_asked, + self.children.contains(&id.id()), + "the note on a child disagrees with the list it says it is in", + ); if !re_asked { self.children.push(id.id()); } @@ -364,11 +371,27 @@ impl<'a> Painter<'a> { None, self.rsc, ); + // Written now the draw has happened, since a widget drawn here has no + // record of its own until it has. + self.state + .active + .get_mut(&id.id()) + .expect("a widget that was drawn has a record") + .asked_by = self.draw; let holds = self.in_parent(drawn.drawing_holds, region, place, declared); let answer_holds = self.in_parent(drawn.answer.holds, region, place, declared); - match self.under.iter_mut().find(|(child, _)| *child == id.id()) { - Some((_, kept)) => *kept = holds, - None => self.under.push((id.id(), holds)), + // Added in step with the child list, so the one search here is the + // rare case of a child asked about twice. + match re_asked { + true => { + let kept = self + .under + .iter_mut() + .find(|(child, _)| *child == id.id()) + .expect("a child asked about twice was added the first time"); + kept.1 = holds; + } + false => self.under.push((id.id(), holds)), } DrawResult { child: id, @@ -385,6 +408,11 @@ impl<'a> Painter<'a> { self.children.retain(|child| *child != id.id()); self.under.retain(|(child, _)| *child != id.id()); self.state.undraw_rec(id.id(), self.rsc); + // Taken out of the child list, so the note saying it is in there goes + // with it: placing it again is asking again. + if let Some(active) = self.state.active.get_mut(&id.id()) { + active.asked_by = DrawId::NONE; + } } /// Puts a child in `place` of this widget's box, where that box is the @@ -406,7 +434,7 @@ impl<'a> Painter<'a> { let states_rel_base = Axis::BOTH .iter() .any(|&axis| matches!(place[axis].rel_base, RelBase::Len(_))); - if states_rel_base || !self.children.contains(&id.id()) { + if states_rel_base || !self.state.asked_in(id.id(), self.draw) { return self.widget_at(id, place); } let at = self.placing(); @@ -470,10 +498,12 @@ impl<'a> Painter<'a> { resolved } + /// Records that this draw read a child's length. Listed once per read + /// rather than once per child: a child that answered with a hint may have + /// no record to note it on until this draw ends, and what the list drives + /// asks the same of a widget twice as of it once. fn depend_on(&mut self, child: &StrongWidget) { - if !self.size_deps.contains(&child.id()) { - self.size_deps.push(child.id()); - } + self.size_deps.push(child.id()); } pub fn render_text<'b>( diff --git a/core/src/ui/render_state.rs b/core/src/ui/render_state.rs index e41b269..b6f2be1 100644 --- a/core/src/ui/render_state.rs +++ b/core/src/ui/render_state.rs @@ -1,9 +1,9 @@ #[cfg(feature = "layout-diagnostics")] use crate::layout_diagnostics::{self as diag, Counter, ReuseOutcome, TimerKind}; use crate::{ - ActiveData, Answer, Axis, Bounds, Declared, DrawLayers, IdLike, LayoutHolds, LayoutLen, Len, - MaskIdx, MoveIdx, Moves, Painter, PixelRegion, PlaceDesc, PxVec2, Size, StrongWidget, UiRegion, - UiRsc, UiSpan, UiVec2, Weight, WidgetId, Widgets, + ActiveData, Answer, Axis, Bounds, Declared, DrawId, DrawLayers, IdLike, LayoutHolds, LayoutLen, + Len, MaskIdx, MoveIdx, Moves, Painter, PixelRegion, PlaceDesc, PxVec2, Size, StrongWidget, + UiRegion, UiRsc, UiSpan, UiVec2, Weight, WidgetId, Widgets, ui::painter::Ask, util::{HashMap, Vec2}, }; @@ -97,6 +97,10 @@ pub struct UiRenderState { pending: std::collections::BinaryHeap<(usize, WidgetId)>, pub(super) requests: crate::RequestArena, changed: Vec, + /// The last draw id handed out. Each draw takes a fresh one and leaves it + /// on every widget it asks about, which is how it knows in one read + /// whether it has asked already. + last_draw: DrawId, request_readers: HashMap>, pub moves: Moves, } @@ -113,6 +117,7 @@ impl UiRenderState { pending: Default::default(), requests: Default::default(), changed: Vec::new(), + last_draw: DrawId::NONE, request_readers: Default::default(), moves: Default::default(), resized: false, @@ -322,6 +327,14 @@ impl UiRenderState { old: Option, rsc: &mut dyn UiRsc, ) -> Answer { + let draw = self.next_draw(); + // Whoever asked about this widget wrote this, and a draw of the widget + // itself is not that: the record is rebuilt below, so it is carried + // across rather than reset. + let asked_by = old + .as_ref() + .or_else(|| self.active.get(&id)) + .map_or(DrawId::NONE, |active| active.asked_by); let rel_base = info.rel_base; let (move_idx, region, retired_move) = match info.region_node { // A node entry is only a translation. Its local box keeps the @@ -383,6 +396,7 @@ impl UiRenderState { answer_under: LayoutHolds::ANY, depth: info.depth, move_idx, + draw, rsc, }; @@ -419,6 +433,7 @@ impl UiRenderState { layer, own_layer: _, depth: _, + draw: _, id, } = painter; @@ -507,7 +522,7 @@ impl UiRenderState { region.to_px(window), ); for c in &old_children { - if !children.contains(c) { + if !self.asked_in(*c, draw) { self.undraw_rec(*c, rsc); } } @@ -531,7 +546,7 @@ impl UiRenderState { // and a change there has to reach it. Asking answered whatever mark // it had: a hint is read live, and a drawing is not kept past one. for &dep in &size_deps { - if !children.contains(&dep) { + if !self.asked_in(dep, draw) { self.asked( dep, DrawInfo { @@ -574,6 +589,7 @@ impl UiRenderState { region, // Whoever asked writes the answer. answer: None, + asked_by, re_asked: info.re_asked, size, holds, @@ -603,6 +619,21 @@ impl UiRenderState { } } + fn next_draw(&mut self) -> DrawId { + self.last_draw = self.last_draw.next(); + self.last_draw + } + + /// Whether `draw` has asked about this widget, which is what it left on + /// the widget's own record when it did. One widget is asked about by one + /// container, since the handle a container holds a child by cannot be + /// cloned, so one note per widget is enough to answer this. + pub(super) fn asked_in(&self, id: WidgetId, draw: DrawId) -> bool { + self.active + .get(&id) + .is_some_and(|active| active.asked_by == draw) + } + /// Keeps a region node's entry across redraws because descendants retain /// its index. fn move_slot(&mut self, id: WidgetId, parent: MoveIdx, region: UiRegion) -> MoveIdx { @@ -956,6 +987,7 @@ impl UiRenderState { asked: PlaceDesc::WHOLE, region: UiRegion::FULL, answer: None, + asked_by: DrawId::NONE, re_asked: false, size, holds: LayoutHolds::ANY, diff --git a/src/widget/mask.rs b/src/widget/mask.rs index 82bb622..eee277c 100644 --- a/src/widget/mask.rs +++ b/src/widget/mask.rs @@ -8,11 +8,14 @@ impl Widget for Masked { fn draw(&mut self, painter: &mut Painter) -> Size { painter.set_mask(UiRegion::FULL); painter.widget(&self.inner); - // What it occupies is its box, on both axes, for the reason `Scroll` - // reports the same: it clips what is inside to that box, so it can - // neither take less of one nor honestly ask for more. Passing the - // inner size up instead asks to be placed at a length it does not - // draw, and the framework would place the drawing it clipped away. + // What it occupies is its box, on both axes, because it clips what is + // inside to that box: it can neither take less of one nor honestly ask + // for more. Passing the inner size up instead asks to be placed at a + // length it does not draw, and the framework would place the drawing it + // clipped away. `Scroll` reports its box too, for a reason of its own: + // it is a viewport whose content is positioned by a move rather than + // clipped, since masking is a capability a caller opts into by putting + // one of these around it. Size::LEFTOVER } diff --git a/src/widget/trait_fns.rs b/src/widget/trait_fns.rs index c9cdde2..02b14aa 100644 --- a/src/widget/trait_fns.rs +++ b/src/widget/trait_fns.rs @@ -166,9 +166,9 @@ widget_trait! { |state| self.add(state) } - // Named for the type it makes rather than as `wrapped`, which would read - // as the text setting. `widget_trait!` takes no attributes, so what it is - // for is on `Wrapper` itself. + /// This widget in a [`Wrapper`], which is how it gets a second length or + /// alignment beside the one it already carries. Named for the type it + /// makes rather than as `wrapped`, which would read as the text setting. fn wrapper(self) -> impl WidgetFn { |state| Wrapper { inner: Some(self.add_strong(state)), diff --git a/tests/allocation_cost.rs b/tests/allocation_cost.rs index 929e6f1..c3cf7ac 100644 --- a/tests/allocation_cost.rs +++ b/tests/allocation_cost.rs @@ -78,3 +78,47 @@ fn unchanged_tree_reuses_layout_storage() { assert_eq!(allocations, 0); } } + +/// A text drawn again at the width it already has places no glyphs and shapes +/// nothing, so the frame costs nothing at all -- which is what the shaping +/// cache is for, and a copy of the attrs made to ask it undid for any text +/// naming its font family. +/// +/// Only that case: a text drawn at a width it has not seen places its glyphs, +/// and placing them allocates a list to hold them. +#[test] +fn redrawing_a_text_at_one_width_allocates_nothing() { + let mut h = Harness::new((600, 200)); + let mut col = Span::empty(Dir::DOWN); + let mut texts = Vec::new(); + for _ in 0..8 { + let text = + wtext("wrapping shapes one source into as many lines as the box leaves room for") + .size(16) + // Named rather than generic, because a named one is the family + // that costs an allocation to copy. + .family(Family::Named("sans-serif".into())) + .wrap(true) + .add_strong(&mut h.rsc); + texts.push(text.id()); + col.push(text); + } + let root = col.add(&mut h.rsc); + h.set_root(root); + let redraw = |h: &mut Harness| { + for &id in &texts { + h.rsc.widgets_mut().mark_for_redraw(id); + } + h.frame(); + }; + for _ in 0..8 { + redraw(&mut h); + } + COUNT.set(Some(0)); + for _ in 0..100 { + redraw(&mut h); + } + let allocations = COUNT.replace(None).unwrap(); + println!("text: {allocations} allocations over 100 redraws of 8 texts"); + assert_eq!(allocations, 0); +} diff --git a/tests/cases/retained.rs b/tests/cases/retained.rs index 9867ecd..c5f16fa 100644 --- a/tests/cases/retained.rs +++ b/tests/cases/retained.rs @@ -1563,3 +1563,30 @@ fn a_contract_this_window_is_outside_is_not_kept() { "the leaf settled once and its parent kept what it settled" ); } + +/// A container that measures a child by drawing it, takes that drawing back, +/// and then places the child, which `place_at` answers by asking again: taking +/// a child back takes it out of the child list, and the note on the child +/// saying it is in there has to go with it, or the placement re-expresses a +/// drawing that no longer exists. +#[test] +fn a_child_taken_back_and_placed_again_is_asked_again() { + struct Retake(StrongWidget); + + impl Widget for Retake { + fn draw(&mut self, painter: &mut Painter) -> Size { + painter.widget(&self.0); + painter.undraw(&self.0); + painter.place_at(&self.0, PlaceDesc::WHOLE).size() + } + } + + let mut h = Harness::new((600, 200)); + let child = rect(Color::RED).add_strong(&mut h.rsc); + let id = child.id(); + let root = Retake(child).add(&mut h.rsc); + h.set_root(root); + h.frame(); + + assert_corners!(h, id, (0, 0), (600, 200)); +} diff --git a/tests/cases/scroll.rs b/tests/cases/scroll.rs index 30dea29..2191610 100644 --- a/tests/cases/scroll.rs +++ b/tests/cases/scroll.rs @@ -127,6 +127,11 @@ fn wrapping_content_beside_a_fixed_length_is_stable_warm_and_cold() { /// parent would place the part it cut off, and the framework would put a /// drawing longer than its box somewhere. `Masked` is the second of these /// after `Scroll`, and the assertion in `draw_at` is what says so. +// What it checks is a debug assertion, which a release build does not compile +// -- and a `should_panic` test of one fails there rather than passing +// vacuously, so it is not built either. Every measurement rig here is run in +// release, so `cargo test --release` has to pass. +#[cfg(debug_assertions)] #[test] #[should_panic = "clips to"] fn a_clipping_widget_reporting_more_than_its_box_is_caught() { diff --git a/tests/chain_cost.rs b/tests/chain_cost.rs index f7f5e13..1f66a83 100644 --- a/tests/chain_cost.rs +++ b/tests/chain_cost.rs @@ -8,13 +8,15 @@ //! submitted and waited on, so this is the GPU's cost and not the recording //! loop's -- which is what `draw_cost.rs` measures instead. //! -//! The instances are two pixels wide so that vertex work dominates; a chain -//! walk that does not show up against small quads will not show up against -//! anything. +//! Two fixtures, because the walk happens in both stages. `chain_cost_by_depth` +//! draws instances two pixels wide so that vertex work dominates; a walk that +//! does not show up against small quads will not show up against anything. +//! `mask_cost_by_depth` draws one screenful through a mask instead, which is +//! where a walk in the fragment stage would show and nowhere else. use iris::prelude::*; use iris_core::{ - Len, MaskIdx, MoveIdx, PrimitiveInst, RectPrimitive, UiData, UiRegion, UiRenderNode, + Len, Mask, MaskIdx, MoveIdx, PrimitiveInst, RectPrimitive, UiData, UiRegion, UiRenderNode, UiRenderState, UiSpan, }; use wgpu::{Color as GpuColor, *}; @@ -45,8 +47,17 @@ fn gpu() -> Option<(Device, Queue, f32)> { Some((device, queue, period)) } +/// Which stage the fill puts the work in: many small quads, where a walk per +/// vertex is what shows, or one screenful of masked rows, where a walk per +/// fragment would. +#[derive(Clone, Copy)] +enum Fixture { + Quads, + Masked, +} + /// A chain `depth` slots long, and instances that all resolve through its end. -fn fill(ui: &mut UiData, render: &mut UiRenderState, depth: usize) { +fn fill(ui: &mut UiData, render: &mut UiRenderState, depth: usize, fixture: Fixture) { let kind = ui.primitives.kind::(); let id = ui.widgets.add_strong(Rect::new(UiColor::WHITE)).id(); @@ -56,20 +67,51 @@ fn fill(ui: &mut UiData, render: &mut UiRenderState, depth: usize) { } let px = |v: f32| Len::px(v); - for i in 0..INSTANCES { - let x = (i % (SIZE as usize / 2)) as f32 * 2.0; - let y = (i / (SIZE as usize / 2)) as f32; + let rows = SIZE as usize; + let mask_idx = match fixture { + Fixture::Quads => MaskIdx::NONE, + // Its own chain as long as the instances', since a viewport sits as + // deep in the tree as the content it clips. + Fixture::Masked => { + let idx = ui.masks.push(Mask { + region: UiRegion::FULL, + move_idx: slot, + }); + // Nothing frees it here, but the owner's reference is what a real + // one is kept alive by. + ui.masks.push_ref(idx); + idx + } + }; + let instances = match fixture { + Fixture::Quads => INSTANCES, + Fixture::Masked => rows, + }; + for i in 0..instances { + let region = match fixture { + Fixture::Quads => { + let x = (i % (rows / 2)) as f32 * 2.0; + let y = (i / (rows / 2)) as f32; + UiRegion::new( + UiSpan::new(px(x), px(x + 2.0)), + UiSpan::new(px(y), px(y + 1.0)), + ) + } + // A full row each, so one screenful of fragments goes through the + // mask and the vertex stage is four corners per row. + Fixture::Masked => UiRegion::new( + UiSpan::new(px(0.0), px(SIZE as f32)), + UiSpan::new(px(i as f32), px(i as f32 + 1.0)), + ), + }; render.layers.write( 0, PrimitiveInst { kind, id, primitive: RectPrimitive::color(UiColor::WHITE), - region: UiRegion::new( - UiSpan::new(px(x), px(x + 2.0)), - UiSpan::new(px(y), px(y + 1.0)), - ), - mask_idx: MaskIdx::NONE, + region, + mask_idx, move_idx: slot, }, ); @@ -77,28 +119,15 @@ fn fill(ui: &mut UiData, render: &mut UiRenderState, depth: usize) { } /// Nanoseconds the pass took on the GPU, best of `BATCHES`. -fn pass_cost(device: &Device, queue: &Queue, period: f32, depth: usize) -> f64 { +fn pass_cost(device: &Device, queue: &Queue, period: f32, depth: usize, fixture: Fixture) -> f64 { let format = TextureFormat::Bgra8Unorm; let mut node = UiRenderNode::new(device, &gpu::config(format, SIZE)); let mut ui = UiData::default(); let mut render = UiRenderState::new(); - fill(&mut ui, &mut render, depth); + fill(&mut ui, &mut render, depth, fixture); node.update(device, queue, &mut ui, &mut render); - let target = device.create_texture(&TextureDescriptor { - label: Some("chain cost"), - size: Extent3d { - width: SIZE, - height: SIZE, - depth_or_array_layers: 1, - }, - mip_level_count: 1, - sample_count: 1, - dimension: TextureDimension::D2, - format, - usage: TextureUsages::RENDER_ATTACHMENT, - view_formats: &[], - }); + let target = gpu::target(device, format, SIZE, false); let view = target.create_view(&TextureViewDescriptor::default()); let queries = device.create_query_set(&QuerySetDescriptor { @@ -178,17 +207,15 @@ fn pass_cost(device: &Device, queue: &Queue, period: f32, depth: usize) -> f64 { best } -#[test] -#[ignore = "measurement, not a check"] -fn chain_cost_by_depth() { +fn by_depth(fixture: Fixture, instances: usize) { let Some((device, queue, period)) = gpu() else { println!("no gpu with timestamps; nothing measured"); return; }; - println!("{INSTANCES} instances, {SIZE}x{SIZE}, best of {BATCHES} batches"); + println!("{instances} instances, {SIZE}x{SIZE}, best of {BATCHES} batches"); let mut base = None; for depth in [1, 2, 4, 8, 16, 32, 64] { - let ns = pass_cost(&device, &queue, period, depth); + let ns = pass_cost(&device, &queue, period, depth, fixture); let base = *base.get_or_insert(ns); println!( "depth {depth:>3}: {:>9.1} us {:+6.1}% against depth 1", @@ -197,3 +224,19 @@ fn chain_cost_by_depth() { ); } } + +#[test] +#[ignore = "measurement, not a check"] +fn chain_cost_by_depth() { + by_depth(Fixture::Quads, INSTANCES); +} + +/// One screenful of rows, every one clipped by a mask whose own chain is that +/// deep. What this says that the quads cannot is whether a mask costs the walk +/// once per instance or once per fragment: at a screenful of fragments per +/// chain, the second is the difference between these two tables. +#[test] +#[ignore = "measurement, not a check"] +fn mask_cost_by_depth() { + by_depth(Fixture::Masked, SIZE as usize); +} diff --git a/tests/children_cost.rs b/tests/children_cost.rs new file mode 100644 index 0000000..03be24e --- /dev/null +++ b/tests/children_cost.rs @@ -0,0 +1,59 @@ +//! What one container's draw costs against the number of children it has. +//! +//! cargo test --release --test children_cost -- --ignored --nocapture +//! +//! Every other rig here varies depth, the window, or what changed between +//! frames; this one varies width, which is the dimension a container's own +//! per-child bookkeeping is counted in. A list of rows is the shape that gets +//! wide -- a transcript, a file tree -- and a cost per child that is not flat +//! down this table is a cost paid twice for every child added. +//! +//! Wall time rather than instructions, because what is being told apart here +//! is a factor rather than a few percent, and the table says which it is: a +//! flat right-hand column is linear and a rising one is not. + +mod rig; + +use iris::harness::Harness; +use iris::prelude::*; +use rig::env; +use std::time::Instant; + +/// A column of leaves each with a length of its own, so the span asks every +/// one of them and reads what each answered. +fn build(h: &mut Harness, children: usize) -> WidgetId { + let mut col = Span::empty(Dir::DOWN); + for _ in 0..children { + col.push( + rect(Color::RED) + .height(LayoutLen::px(4.0)) + .add_strong(&mut h.rsc), + ); + } + let root = col.add(&mut h.rsc); + h.set_root(root); + root.id() +} + +#[test] +#[ignore = "measurement, not a check"] +fn draw_cost_by_children() { + let frames = env("FRAMES", 40_usize); + println!("{frames} full redraws of one span, per child in the last column"); + for children in [100_usize, 200, 400, 800, 1600] { + // Tall enough that no child is collapsed for want of room. + let mut h = Harness::new((600.0, children as f32 * 8.0)); + let root = build(&mut h, children); + h.frame(); + let start = Instant::now(); + for _ in 0..frames { + h.rsc.widgets_mut().mark_for_redraw(root); + h.frame(); + } + let ms = start.elapsed().as_secs_f64() * 1000.0 / frames as f64; + println!( + "children {children:>5}: {ms:>8.3} ms per redraw, {:>7.4} ms each", + ms / children as f64 + ); + } +} diff --git a/tests/gpu/mod.rs b/tests/gpu/mod.rs index ebf42ef..ade3f48 100644 --- a/tests/gpu/mod.rs +++ b/tests/gpu/mod.rs @@ -1,5 +1,6 @@ -//! The adapter and the surface configuration the GPU measurement rigs share, -//! so the two cannot probe for a device in two different ways. +//! The adapter, the surface configuration and the target the GPU rigs share, +//! so no two of them can probe for a device or make a target in different +//! ways. use wgpu::*; @@ -38,3 +39,28 @@ pub fn config(format: TextureFormat, size: u32) -> SurfaceConfiguration { view_formats: vec![], } } + +/// A square colour target to draw a pass into. `copy` adds the usage a rig +/// that reads the pixels back needs; one that only times the pass does not. +// This module is compiled into each rig target separately, so a helper the +// ones that make no target of their own do not call is dead code there. +#[allow(dead_code)] +pub fn target(device: &Device, format: TextureFormat, size: u32, copy: bool) -> Texture { + device.create_texture(&TextureDescriptor { + label: Some("gpu rig target"), + size: Extent3d { + width: size, + height: size, + depth_or_array_layers: 1, + }, + mip_level_count: 1, + sample_count: 1, + dimension: TextureDimension::D2, + format, + usage: match copy { + true => TextureUsages::RENDER_ATTACHMENT | TextureUsages::COPY_SRC, + false => TextureUsages::RENDER_ATTACHMENT, + }, + view_formats: &[], + }) +} diff --git a/tests/mask_clip.rs b/tests/mask_clip.rs new file mode 100644 index 0000000..00c9b9a --- /dev/null +++ b/tests/mask_clip.rs @@ -0,0 +1,162 @@ +//! Which pixels a mask lets through, read back off the GPU. +//! +//! cargo test --release --test mask_clip -- --ignored --nocapture +//! +//! Ignored because it needs a device, which not every machine running the +//! suite has -- and a deliberate run on one without fails rather than passing +//! with nothing checked. Nothing else here sees a mask at all: `iris::harness` +//! draws no pixels, and a mask's rectangle is resolved through its own move +//! chain in the shader, so the CPU's idea of it is not what clips anything. + +use iris::prelude::*; +use iris_core::{ + Len, Mask, MoveIdx, PrimitiveInst, RectPrimitive, UiData, UiRegion, UiRenderNode, + UiRenderState, UiSpan, +}; +use wgpu::{Color as GpuColor, *}; + +#[path = "gpu/mod.rs"] +mod gpu; + +const SIZE: u32 = 256; + +/// The mask is a box inside a move chain two links long and the drawing +/// overflows it on both axes, so what comes back is the mask's own rectangle +/// composed through that chain -- and a clip resolved through the wrong one, +/// or not composed at all, lands somewhere else. +#[test] +#[ignore = "needs a gpu"] +fn a_mask_clips_its_own_box_composed_through_its_chain() { + let adapter = gpu::adapter().expect("no adapter to draw with"); + println!("adapter: {:?}", adapter.get_info().name); + let (device, queue) = pollster::block_on(adapter.request_device(&DeviceDescriptor::default())) + .expect("no device on that adapter"); + let format = TextureFormat::Bgra8Unorm; + let mut node = UiRenderNode::new(&device, &gpu::config(format, SIZE)); + let mut ui = UiData::default(); + let mut render = UiRenderState::new(); + let kind = ui.primitives.kind::(); + let id = ui.widgets.add_strong(Rect::new(UiColor::WHITE)).id(); + let px = Len::px; + + let outer = render.moves.push(MoveIdx::NONE, UiRegion::FULL); + let shift = (16.0, 24.0); + let inner = render.moves.push( + outer, + UiRegion::new( + UiSpan::new(px(shift.0), px(shift.0) + Len::FULL), + UiSpan::new(px(shift.1), px(shift.1) + Len::FULL), + ), + ); + let clip = (20.0, 30.0, 120.0, 90.0); + let mask = ui.masks.push(Mask { + region: UiRegion::new( + UiSpan::new(px(clip.0), px(clip.2)), + UiSpan::new(px(clip.1), px(clip.3)), + ), + move_idx: inner, + }); + // The owner's reference, which is what keeps a real one alive. + ui.masks.push_ref(mask); + render.layers.write( + 0, + PrimitiveInst { + kind, + id, + primitive: RectPrimitive::color(UiColor::WHITE), + region: UiRegion::new( + UiSpan::new(px(0.0), px(200.0)), + UiSpan::new(px(0.0), px(200.0)), + ), + mask_idx: mask, + move_idx: inner, + }, + ); + node.update(&device, &queue, &mut ui, &mut render); + + let target = gpu::target(&device, format, SIZE, true); + let view = target.create_view(&TextureViewDescriptor::default()); + let row = SIZE * 4; + let readback = device.create_buffer(&BufferDescriptor { + label: Some("mask clip"), + size: (row * SIZE) as u64, + usage: BufferUsages::MAP_READ | BufferUsages::COPY_DST, + mapped_at_creation: false, + }); + let mut encoder = device.create_command_encoder(&CommandEncoderDescriptor::default()); + { + let pass = &mut encoder.begin_render_pass(&RenderPassDescriptor { + label: None, + color_attachments: &[Some(RenderPassColorAttachment { + view: &view, + resolve_target: None, + ops: Operations { + load: LoadOp::Clear(GpuColor::BLACK), + store: StoreOp::Store, + }, + depth_slice: None, + })], + depth_stencil_attachment: None, + timestamp_writes: None, + occlusion_query_set: None, + multiview_mask: None, + }); + node.draw(pass); + } + let whole = Extent3d { + width: SIZE, + height: SIZE, + depth_or_array_layers: 1, + }; + encoder.copy_texture_to_buffer( + TexelCopyTextureInfo { + texture: &target, + mip_level: 0, + origin: Origin3d::ZERO, + aspect: TextureAspect::All, + }, + TexelCopyBufferInfo { + buffer: &readback, + layout: TexelCopyBufferLayout { + offset: 0, + bytes_per_row: Some(row), + rows_per_image: Some(SIZE), + }, + }, + whole, + ); + queue.submit(Some(encoder.finish())); + let slice = readback.slice(..); + slice.map_async(MapMode::Read, |_| {}); + device + .poll(PollType::Wait { + submission_index: None, + timeout: None, + }) + .expect("the pass did not finish"); + let pixels = slice.get_mapped_range().expect("the target did not map"); + + let mut lit = 0; + let mut bounds: Option<(u32, u32, u32, u32)> = None; + for y in 0..SIZE { + for x in 0..SIZE { + if pixels[(y * row + x * 4) as usize] == 0 { + continue; + } + lit += 1; + let (x0, y0, x1, y1) = bounds.unwrap_or((x, y, x, y)); + bounds = Some((x0.min(x), y0.min(y), x1.max(x), y1.max(y))); + } + } + // The clip shifted by the chain. Its far edge is exclusive: a fragment + // exactly on it is the first one outside. + let want = ( + (clip.0 + shift.0) as u32, + (clip.1 + shift.1) as u32, + (clip.2 + shift.0) as u32 - 1, + (clip.3 + shift.1) as u32 - 1, + ); + assert_eq!(bounds, Some(want), "{lit} pixels through the mask"); + let (x0, y0, x1, y1) = want; + assert_eq!(lit, (x1 - x0 + 1) * (y1 - y0 + 1), "the clip has a hole"); +}