diff --git a/core/src/render/mod.rs b/core/src/render/mod.rs index 9723a41..04e4242 100644 --- a/core/src/render/mod.rs +++ b/core/src/render/mod.rs @@ -242,8 +242,10 @@ impl UiRenderNode { image_tex_indices: Vec::new(), }); if order.updated { - rlayer.order.update(device, queue, order.order()); - rlayer.images.update(device, queue, order.images()); + let (entries, dirty) = order.order_for_upload(); + rlayer.order.update(device, queue, entries, dirty); + let (entries, dirty) = order.images_for_upload(); + rlayer.images.update(device, queue, entries, dirty); rlayer.image_tex_indices = order .images() .iter() @@ -252,32 +254,27 @@ impl UiRenderNode { order.updated = false; } } - let instances_resized = if ui_render.primitives.updated { - ui_render.primitives.updated = false; - let resized = self - .instances - .update(device, queue, ui_render.primitives.instances()); - self.primitives - .update(device, queue, ui_render.primitives.data()); - self.primitive_group = - Self::primitive_group(device, &self.primitive_layout, self.primitives.buffers()); + let instances_resized = if ui_render.primitives.needs_upload() { + let (entries, dirty) = ui_render.primitives.instances_for_upload(); + let resized = self.instances.update(device, queue, entries, dirty); + if self + .primitives + .update(device, queue, ui_render.primitives.data_mut()) + { + self.primitive_group = Self::primitive_group( + device, + &self.primitive_layout, + self.primitives.buffers(), + ); + } resized } else { false }; - let masks_resized = if ui.masks.changed { - ui.masks.changed = false; - self.masks.update(device, queue, &ui.masks[..]) - } else { - false - }; - let moves_resized = if ui.move_offsets.changed { - ui.move_offsets.changed = false; - self.move_offsets - .update(device, queue, &ui.move_offsets[..]) - } else { - false - }; + let (entries, dirty) = ui.masks.for_upload(); + let masks_resized = self.masks.update(device, queue, entries, dirty); + let (entries, dirty) = ui.move_offsets.for_upload(); + let moves_resized = self.move_offsets.update(device, queue, entries, dirty); if masks_resized || moves_resized || instances_resized { self.masks_group = Self::masks_group( device, diff --git a/core/src/render/primitive.rs b/core/src/render/primitive.rs index e184090..f3e0210 100644 --- a/core/src/render/primitive.rs +++ b/core/src/render/primitive.rs @@ -1,4 +1,4 @@ -use std::ops::{Deref, DerefMut}; +use std::ops::Deref; use crate::{ Color, UiRegion, WidgetId, @@ -6,7 +6,7 @@ use crate::{ ArrBuf, data::{MaskIdx, MoveIdx, PrimitiveInstance}, }, - util::HashSet, + util::{Dirty, HashSet}, }; use bytemuck::Pod; use wgpu::*; @@ -39,8 +39,25 @@ macro_rules! primitives { } impl PrimitiveBuffers { - pub fn update(&mut self, device: &Device, queue: &Queue, data: &PrimitiveData) { - $(self.$name.update(device, queue, &data.$name);)* + /// Answers whether **any** of the per-primitive buffers was + /// reallocated, which is the only thing that obliges the + /// caller to rebuild the bind group naming them. It used to + /// return nothing and the group was rebuilt on every frame + /// the arena changed -- once `ArrBuf` kept its buffer across + /// a length change, that was a bind group per frame for a + /// buffer identity that had not moved. + pub fn update( + &mut self, + device: &Device, + queue: &Queue, + data: &mut PrimitiveData, + ) -> bool { + let mut reallocated = false; + $( + let (entries, dirty) = data.$name.for_upload(); + reallocated |= self.$name.update(device, queue, entries, dirty); + )* + reallocated } } @@ -71,6 +88,9 @@ macro_rules! primitives { } impl PrimitiveData { + pub fn needs_upload(&self) -> bool { + $(!self.$name.dirty.is_clean() ||)* false + } pub fn clear(&mut self) { $(self.$name.clear();)* } @@ -148,10 +168,11 @@ pub struct Primitives { /// hands out. reusable: Vec, data: PrimitiveData, - /// Whether the instance arena or the per-primitive data changed since - /// the last upload -- one flag for both, since they are uploaded - /// together. - pub updated: bool, + /// Which instance slots have changed since the last upload. Was a + /// single `bool` covering the instances **and** the per-primitive + /// data until 2026-09-09, so rewriting one rect's region re-uploaded + /// every glyph as well; each array carries its own now. + pub dirty: Dirty, } impl Default for Primitives { @@ -163,7 +184,7 @@ impl Default for Primitives { freed: Vec::new(), reusable: Vec::new(), data: Default::default(), - updated: true, + dirty: Dirty::new_all(), } } } @@ -228,7 +249,6 @@ impl Primitives { } fn push(&mut self, inst: PrimitiveInstance, id: WidgetId) -> u32 { - self.updated = true; let slot = if let Some(i) = self.reusable.pop() { self.instances[i] = inst; self.assoc[i] = id; @@ -240,14 +260,109 @@ impl Primitives { self.handle_idx.push(Self::NO_HANDLE); self.instances.len() - 1 }; + self.dirty.mark(slot); slot as u32 } + /// Rewrites a slot this widget already owns, instead of freeing it + /// and allocating another -- the recycle path + /// (`Painter::write_primitive`). + /// + /// **Why a redraw must be able to do this.** Freed slots do not + /// become reusable until the end of the frame (`freed`), so a widget + /// that frees its primitives and immediately draws again takes fresh + /// slots every time. Since `Painter::draw_twice` is how a container + /// learns a child's size, and containers nest, that made the arena's + /// high-water the *transient* push count rather than the live one: + /// measured over the bench fixture's 401 streamed deltas + /// (`scripts/rigs/ui-profile`'s `arena_churn`) at 17 million pushes + /// and 127,443 slots for 11,569 live primitives, growing linearly + /// with the transcript. + /// + /// The caller has already checked that `h` is the same kind of + /// primitive in the same layer, which is what makes the slot, its + /// entry in the per-primitive data, and its position in the layer's + /// draw order all still the right ones -- so nothing here touches + /// `freed`, `reusable` or `LayerOrder`, and no renumbering follows. + pub fn recycle( + &mut self, + h: &PrimitiveHandle, + PrimitiveInst { + id, + primitive, + region, + mask_idx, + move_idx, + }: PrimitiveInst

, + ) { + debug_assert_eq!( + h.binding, + P::BINDING, + "recycling slot {} as a different kind of primitive than it holds", + h.slot, + ); + P::vec(&mut self.data).set(h.data_idx, primitive); + self.set_instance( + h.slot, + PrimitiveInstance { + region, + idx: h.data_idx as u32, + mask_idx, + move_idx, + binding: P::BINDING, + }, + id, + ); + } + + /// The image half of [`Self::recycle`] -- no `PrimitiveData` entry, so + /// `texture_idx` rides in `idx` exactly as [`Self::alloc_image`] puts + /// it there. + pub fn recycle_image( + &mut self, + h: &PrimitiveHandle, + id: WidgetId, + texture_idx: u32, + region: UiRegion, + mask_idx: MaskIdx, + move_idx: MoveIdx, + ) { + debug_assert_eq!( + h.binding, IMAGE_BINDING, + "recycling slot {} as an image when it holds a primitive", + h.slot, + ); + self.set_instance( + h.slot, + PrimitiveInstance { + region, + idx: texture_idx, + mask_idx, + move_idx, + binding: IMAGE_BINDING, + }, + id, + ); + } + + /// Writes an instance into a slot that already holds one, marking it + /// dirty only if it differs -- the same rule as + /// [`PrimitiveVec::set`], for the same reason. `assoc` is not part of + /// the comparison because it is never uploaded. + fn set_instance(&mut self, slot: u32, inst: PrimitiveInstance, id: WidgetId) { + let slot = slot as usize; + self.assoc[slot] = id; + if bytemuck::bytes_of(&self.instances[slot]) == bytemuck::bytes_of(&inst) { + return; + } + self.instances[slot] = inst; + self.dirty.mark(slot); + } + /// Retires a slot, answering the mask it was drawn under so the caller /// can drop that mask's ref. The slot itself only becomes reusable at /// the next [`Self::apply_free`] -- see `freed`. pub fn free(&mut self, h: &PrimitiveHandle) -> MaskIdx { - self.updated = true; let slot = h.slot as usize; if h.binding != IMAGE_BINDING { self.data.free(h.binding, h.data_idx); @@ -288,7 +403,7 @@ impl Primitives { } pub fn clear(&mut self) { - self.updated = true; + self.dirty.mark_all(); self.instances.clear(); self.assoc.clear(); self.handle_idx.clear(); @@ -330,6 +445,24 @@ impl Primitives { &self.instances } + /// The instance arena and its dirty set together -- see + /// [`PrimitiveVec::for_upload`]. + pub fn instances_for_upload(&mut self) -> (&[PrimitiveInstance], &mut Dirty) { + (&self.instances, &mut self.dirty) + } + + /// The per-primitive data, mutably, for the one caller that uploads it + /// (`UiRenderNode::update`) and so has to clear its dirty sets. + pub fn data_mut(&mut self) -> &mut PrimitiveData { + &mut self.data + } + + /// Whether anything at all needs uploading -- the instances or any of + /// the per-primitive arrays. + pub fn needs_upload(&self) -> bool { + !self.dirty.is_clean() || self.data.needs_upload() + } + pub fn instance(&self, slot: u32) -> &PrimitiveInstance { &self.instances[slot as usize] } @@ -344,7 +477,7 @@ impl Primitives { } pub fn region_mut(&mut self, h: &PrimitiveHandle) -> &mut UiRegion { - self.updated = true; + self.dirty.mark(h.slot as usize); &mut self.instances[h.slot as usize].region } } @@ -358,6 +491,8 @@ impl Primitives { #[derive(Default)] pub struct LayerOrder { order: Vec, + order_dirty: Dirty, + images_dirty: Dirty, /// Standalone images, kept apart because each draws with its own bind /// group rather than sharing the layer's one instanced draw -- see /// `UiRenderNode::draw`. @@ -370,12 +505,13 @@ pub struct LayerOrder { impl LayerOrder { pub fn push(&mut self, slot: u32, is_image: bool) -> usize { self.updated = true; - let list = if is_image { - &mut self.images + let (list, dirty) = if is_image { + (&mut self.images, &mut self.images_dirty) } else { - &mut self.order + (&mut self.order, &mut self.order_dirty) }; list.push(slot); + dirty.mark(list.len() - 1); list.len() - 1 } @@ -394,18 +530,35 @@ impl LayerOrder { /// Compacts both lists, answering every primitive whose position /// moved so its handle can be corrected. pub fn apply_free(&mut self) -> Vec { - let mut changes = Self::apply_free_list(&mut self.free, &mut self.order, false); + let mut changes = Self::apply_free_list( + &mut self.free, + &mut self.order, + &mut self.order_dirty, + false, + ); changes.extend(Self::apply_free_list( &mut self.image_free, &mut self.images, + &mut self.images_dirty, true, )); changes } + /// The draw order and its dirty set together -- see + /// [`PrimitiveVec::for_upload`]. + pub fn order_for_upload(&mut self) -> (&[u32], &mut Dirty) { + (&self.order, &mut self.order_dirty) + } + + pub fn images_for_upload(&mut self) -> (&[u32], &mut Dirty) { + (&self.images, &mut self.images_dirty) + } + fn apply_free_list( free: &mut Vec, list: &mut Vec, + dirty: &mut Dirty, is_image: bool, ) -> Vec { // Descending, so removing a contiguous tail costs no renumbering @@ -417,6 +570,10 @@ impl LayerOrder { if pos == list.len() { return None; } + // `swap_remove` moved the tail entry here; nothing else in + // the list changed, which is why compacting an order is + // two dirty entries rather than the whole buffer. + dirty.mark(pos); Some(OrderChange { slot: list[pos], is_image, @@ -559,6 +716,11 @@ impl GlyphPrimitive { pub struct PrimitiveVec { vec: Vec, free: Vec, + /// Which entries have changed since the last upload. Every way to + /// write one goes through [`Self::add`] or [`Self::set`], which is + /// what keeps this in step -- there is deliberately no `DerefMut`, + /// because an entry written through one would never be uploaded. + pub dirty: Dirty, } impl PrimitiveVec { @@ -566,24 +728,60 @@ impl PrimitiveVec { Self { vec: Vec::new(), free: Vec::new(), + dirty: Dirty::new_all(), } } pub fn add(&mut self, t: T) -> usize { - if let Some(i) = self.free.pop() { - self.vec[i] = t; - i - } else { - let i = self.vec.len(); - self.vec.push(t); - i + let i = match self.free.pop() { + Some(i) => { + self.vec[i] = t; + i + } + None => { + self.vec.push(t); + self.vec.len() - 1 + } + }; + self.dirty.mark(i); + i + } + /// Overwrites an entry already allocated -- the recycle path + /// ([`Primitives::recycle`]) -- and marks it dirty **only if the + /// value actually differs**. + /// + /// That check is not an optimisation of the comparison; it is what + /// makes the dirty set mean "changed" rather than "written". A row + /// that moves, or is re-laid-out at a new width, rewrites every glyph + /// it owns with the same `uv`, `layer`, `colour` and `flags` -- what + /// moved is the *instance's* region, which is a different array. Over + /// the bench fixture's streamed reply the glyph array was being + /// marked at 73% per frame against 0.6% genuinely changed, a 122x + /// over-upload, entirely from this (`scripts/rigs/ui-profile`'s + /// `arena_churn`, which prints both numbers side by side so the gap + /// cannot reopen unnoticed). + pub fn set(&mut self, i: usize, t: T) + where + T: Pod, + { + if bytemuck::bytes_of(&self.vec[i]) == bytemuck::bytes_of(&t) { + return; } + self.vec[i] = t; + self.dirty.mark(i); } pub fn free(&mut self, i: usize) { self.free.push(i); } + /// The entries and the dirty set together, so an uploader can read one + /// while clearing the other -- they are different fields, but a + /// caller reaching for both through `Deref` cannot say so. + pub fn for_upload(&mut self) -> (&[T], &mut Dirty) { + (&self.vec, &mut self.dirty) + } pub fn clear(&mut self) { self.free.clear(); self.vec.clear(); + self.dirty.mark_all(); } } @@ -600,9 +798,3 @@ impl Deref for PrimitiveVec { &self.vec } } - -impl DerefMut for PrimitiveVec { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.vec - } -} diff --git a/core/src/render/util/mod.rs b/core/src/render/util/mod.rs index d4faf54..18e289a 100644 --- a/core/src/render/util/mod.rs +++ b/core/src/render/util/mod.rs @@ -1,44 +1,85 @@ use std::marker::PhantomData; +use crate::util::Dirty; use bytemuck::Pod; use wgpu::*; +/// A GPU array whose `Buffer` outlives the data in it. +/// +/// **The buffer has a capacity, and shrinking never reallocates.** That +/// is not only about allocation cost: a fresh `Buffer`'s contents are +/// undefined, so a reallocation is the one event after which a *partial* +/// upload is not correct. Keeping the buffer alive across a length change +/// is therefore the precondition for uploading only what changed, and +/// [`Self::update`] says which of the two happened so a caller can force +/// the whole range dirty. +/// +/// It reallocated on every length change until 2026-09-09, which made the +/// streaming path pay a full rewrite of every arena on nearly every +/// frame -- adding one glyph changes a length. Measured over the bench +/// fixture's 401 streamed deltas (`scripts/rigs/ui-profile`'s +/// `arena_churn`): the glyph buffer's *changed* bytes were 3.0% of its +/// size, but 95% of it had to be re-uploaded anyway because the buffer +/// underneath had just been replaced. pub struct ArrBuf { label: &'static str, usage: BufferUsages, pub buffer: Buffer, + /// Entries the caller last wrote -- what a draw call reads. len: usize, + /// Entries the buffer has room for. Grows geometrically and never + /// shrinks, so a list that oscillates in length (every frame of a + /// fling adds and drops rows) settles on one allocation. + capacity: usize, _pd: PhantomData, } +/// The smallest allocation worth making, in entries. A buffer that starts +/// at the exact first length reallocates on the second frame of anything; +/// this is small enough to be free and large enough that a handful of +/// masks or move offsets never grows at all. +const MIN_CAPACITY: usize = 64; + impl ArrBuf { pub fn new(device: &Device, usage: BufferUsages, label: &'static str) -> Self { Self { label, usage, - buffer: Self::init_buf(device, 0, usage, label), + buffer: Self::init_buf(device, MIN_CAPACITY, usage, label), len: 0, + capacity: MIN_CAPACITY, _pd: PhantomData, } } - /// Returns whether the underlying `Buffer` was recreated -- a caller that - /// cached a `BindGroup` referencing it (as `GpuTextures` does for the - /// masks buffer) needs to know to rebuild that too. - pub fn update(&mut self, device: &Device, queue: &Queue, data: &[T]) -> bool { - let resized = self.len != data.len(); - if resized { - self.len = data.len(); - self.buffer = - Self::init_buf(device, std::mem::size_of_val(data), self.usage, self.label); + + /// Grows to hold `len` entries if it does not already, answering + /// whether that meant a new `Buffer`. Doubling rather than exact, so a + /// buffer that grows by one entry per frame -- which is what a + /// streamed reply does to the glyph arena -- reallocates a logarithmic + /// number of times rather than every frame. + pub fn reserve(&mut self, device: &Device, len: usize) -> bool { + if len <= self.capacity { + return false; } - queue.write_buffer(&self.buffer, 0, bytemuck::cast_slice(data)); - resized + let mut capacity = self.capacity.max(MIN_CAPACITY); + while capacity < len { + capacity *= 2; + } + self.capacity = capacity; + self.buffer = Self::init_buf(device, capacity, self.usage, self.label); + true } - fn init_buf(device: &Device, size: usize, usage: BufferUsages, label: &'static str) -> Buffer { - let mut size = size as u64; - if usage.contains(BufferUsages::STORAGE) { - size = size.max(std::mem::size_of::() as u64); - } + + fn init_buf( + device: &Device, + entries: usize, + usage: BufferUsages, + label: &'static str, + ) -> Buffer { + // A storage binding of size 0 is a validation error, and an empty + // arena is the ordinary state of a buffer nothing has drawn into + // yet. + let size = (entries.max(1) * std::mem::size_of::()) as u64; device.create_buffer(&BufferDescriptor { label: Some(label), size, @@ -46,6 +87,48 @@ impl ArrBuf { usage, }) } + + /// Writes the entries `dirty` names and clears it, answering whether + /// the underlying `Buffer` was **recreated** -- which a caller holding + /// a `BindGroup` over it must know, since it has to rebuild that group. + /// + /// Correct only because the buffer outlives the data in it: a + /// reallocation leaves the rest of the buffer undefined, which is why + /// one forces the whole range dirty here rather than leaving the + /// caller to remember. Measured over the bench fixture + /// (`scripts/rigs/ui-profile`'s `arena_churn`): a fling writes 3.3% of + /// what the whole-array path wrote, and the median frame writes + /// nothing at all. + pub fn update( + &mut self, + device: &Device, + queue: &Queue, + data: &[T], + dirty: &mut Dirty, + ) -> bool { + let reallocated = self.reserve(device, data.len()); + if reallocated { + dirty.mark_all(); + } + self.len = data.len(); + let stride = std::mem::size_of::() as BufferAddress; + for range in dirty.ranges(data.len(), Self::MERGE_GAP) { + queue.write_buffer( + &self.buffer, + range.start as BufferAddress * stride, + bytemuck::cast_slice(&data[range]), + ); + } + dirty.clear(); + reallocated + } + + /// How far apart two dirty runs may be and still be uploaded as one + /// -- in entries, so a wider entry merges across fewer of them and + /// the *byte* cost of merging is the same either way. See + /// [`Dirty::ranges`] for the measurement behind 1 KiB. + const MERGE_GAP: usize = 1024 / std::mem::size_of::(); + #[allow(clippy::len_without_is_empty)] pub fn len(&self) -> usize { self.len diff --git a/core/src/ui/painter.rs b/core/src/ui/painter.rs index eb1f977..5b54b6a 100644 --- a/core/src/ui/painter.rs +++ b/core/src/ui/painter.rs @@ -2,8 +2,8 @@ use crate::{ Color, RenderedText, Size, StrongWidget, TextAttrs, TextBuffer, TextData, TextureHandle, UiRegion, UiRenderState, UiRsc, UiScalar, UiVec2, WidgetId, render::{ - Drawn, GlyphPrimitive, Mask, MaskIdx, MoveIdx, Primitive, PrimitiveHandle, PrimitiveInst, - RectPrimitive, + Drawn, GlyphPrimitive, IMAGE_BINDING, Mask, MaskIdx, MoveIdx, NOT_DRAWN, Primitive, + PrimitiveHandle, PrimitiveInst, RectPrimitive, }, util::Vec2, }; @@ -22,6 +22,17 @@ pub struct Painter<'a> { pub(super) own_mask: MaskIdx, pub(super) textures: Vec, pub(super) primitives: Vec, + /// The handles this widget owned before *this* draw, offered back to + /// it in the order it wrote them last time -- see + /// [`Self::take_recycled`]. Empty for a widget being drawn for the + /// first time. Whatever is left when the draw ends is genuinely gone, + /// and `UiRenderState::draw_inner` frees the remainder. + /// + /// An iterator rather than a vec and a cursor because a + /// `PrimitiveHandle` is an ownership token and deliberately not + /// `Clone`: `peek` asks whether the next one fits without taking it, + /// `next` takes it, and what is left is exactly what nothing claimed. + pub(super) recycle: std::iter::Peekable>, pub(super) children: Vec, pub layer: usize, pub(super) id: WidgetId, @@ -32,6 +43,29 @@ impl<'a> Painter<'a> { self.write_primitive(primitive, region, Drawn::Yes); } + /// The next handle from the previous draw, if it can hold what is + /// about to be written: same kind of primitive, same layer, and the + /// same answer to "does a layer's draw order name it". + /// + /// **Consumed strictly in order, and one mismatch ends recycling for + /// the rest of the draw.** A widget's `draw` is a function of its own + /// state, so a redraw writes the same sequence of primitives in the + /// same order in the overwhelmingly common case; searching the + /// remainder for a match would turn an O(1) step into an O(primitives) + /// one to rescue a case that means the widget's content changed shape + /// anyway. Stopping is also what keeps the invariant simple: every + /// handle from `recycled` on is untouched and gets freed together. + fn take_recycled(&mut self, binding: u32, drawn: Drawn) -> Option { + let h = self.recycle.peek()?; + let drawn_matches = (h.pos == NOT_DRAWN) == (drawn == Drawn::No); + if h.binding != binding || h.layer != self.layer || !drawn_matches { + // Left un-taken deliberately: this handle and everything after + // it is freed together when the draw ends. + return None; + } + self.recycle.next() + } + /// The one path every primitive this widget owns goes through -- /// drawn or, for a mask's shape, only referenced. fn write_primitive( @@ -40,17 +74,20 @@ impl<'a> Painter<'a> { region: UiRegion, drawn: Drawn, ) -> u32 { - let h = self.state.write_primitive( - self.layer, - drawn, - PrimitiveInst { - id: self.id, - primitive, - region, - mask_idx: self.mask, - move_idx: self.move_slot, - }, - ); + let inst = PrimitiveInst { + id: self.id, + primitive, + region, + mask_idx: self.mask, + move_idx: self.move_slot, + }; + let h = match self.take_recycled(P::BINDING, drawn) { + Some(h) => { + self.state.primitives.recycle(&h, inst); + h + } + None => self.state.write_primitive(self.layer, drawn, inst), + }; if self.mask != MaskIdx::NONE { // TODO: I have no clue if this works at all :joy: self.rsc.ui_mut().masks.push_ref(self.mask); @@ -209,9 +246,7 @@ impl<'a> Painter<'a> { Some(self.id), self.move_slot.idx() as u32, self.mask, - None, - None, - crate::render::MaskIdx::NONE, + Default::default(), self.rsc, ); self.state @@ -271,14 +306,27 @@ impl<'a> Painter<'a> { /// the layer's one instanced draw, so it goes through /// `Primitives::write_image` instead of `primitive_at`/`Primitive::vec`. fn write_image(&mut self, texture_idx: u32, region: UiRegion) { - let h = self.state.write_image( - self.layer, - self.id, - texture_idx, - region, - self.mask, - self.move_slot, - ); + let h = match self.take_recycled(IMAGE_BINDING, Drawn::Yes) { + Some(h) => { + self.state.primitives.recycle_image( + &h, + self.id, + texture_idx, + region, + self.mask, + self.move_slot, + ); + h + } + None => self.state.write_image( + self.layer, + self.id, + texture_idx, + region, + self.mask, + self.move_slot, + ), + }; if self.mask != MaskIdx::NONE { self.rsc.ui_mut().masks.push_ref(self.mask); } diff --git a/core/src/ui/render_state.rs b/core/src/ui/render_state.rs index a37cc80..4fe4fc0 100644 --- a/core/src/ui/render_state.rs +++ b/core/src/ui/render_state.rs @@ -94,6 +94,42 @@ pub struct UiRenderState { last_input_at: Mutex>, } +/// What a widget being redrawn keeps from the draw it is replacing. +/// +/// These four always travel together -- they are read off one +/// `ActiveData` that was just taken out of `active` and handed straight +/// to the draw that replaces it -- and they were four positional +/// 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 { + /// So children this draw does not draw again can be retired. + pub children: Vec, + /// Reused in place with its delta reset, never reallocated: a + /// descendant that is not itself redrawn still points at it. See + /// LAYOUT.md section 2. + pub move_slot: Option, + pub own_mask: MaskIdx, + /// Slots the draw may write into instead of allocating -- see + /// `Painter::take_recycled`. Anything it does not claim is freed when + /// the draw ends. + pub primitives: Vec, +} + +impl Default for Retained { + /// Nothing kept: no children to retire, no move slot to reuse, no + /// mask of its own yet, nothing to recycle. Hand-written because + /// `MaskIdx`'s zero is a real slot rather than "none". + fn default() -> Self { + Self { + children: Vec::new(), + move_slot: None, + own_mask: MaskIdx::NONE, + primitives: Vec::new(), + } + } +} + /// The bound on the parent walk -- see `resolve_move` in shader.wgsl, /// which walks the identical chain and must be kept in step with this /// constant. It exists so a cyclic `parent` link cannot hang either walk, @@ -386,9 +422,7 @@ impl UiRenderState { None, MoveOffset::NONE_PARENT, MaskIdx::NONE, - None, - None, - MaskIdx::NONE, + Retained::default(), rsc, ); } @@ -421,14 +455,15 @@ impl UiRenderState { parent: Option, parent_move_slot: u32, mask: MaskIdx, - old_children: Option>, - old_move_slot: Option, - old_own_mask: MaskIdx, + retained: Retained, rsc: &mut dyn UiRsc, ) { - let mut old_children = old_children.unwrap_or_default(); - let mut old_move_slot = old_move_slot; - let mut own_mask = old_own_mask; + let Retained { + children: mut old_children, + move_slot: mut old_move_slot, + mut own_mask, + primitives: mut recycle, + } = 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 @@ -483,19 +518,21 @@ impl UiRenderState { return; } // if not, then maintain resize and track old children to remove unneeded - let active = self.remove(id, false, rsc).unwrap(); + let active = self.remove(id, false, true, rsc).unwrap(); old_children = active.children; old_move_slot = Some(active.move_slot); own_mask = active.own_mask; + recycle = active.primitives; } else if dirty && self.active.contains_key(&id) { // 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 // thing that frees them. Same two lines, reached the other way. - let active = self.remove(id, false, rsc).unwrap(); + let active = self.remove(id, false, true, rsc).unwrap(); old_children = active.children; old_move_slot = Some(active.move_slot); own_mask = active.own_mask; + recycle = active.primitives; } // draw widget @@ -552,6 +589,7 @@ impl UiRenderState { id, textures: Vec::new(), primitives: Vec::new(), + recycle: recycle.into_iter().peekable(), children: Vec::new(), rsc, }; @@ -581,11 +619,21 @@ impl UiRenderState { own_mask, textures, primitives, + recycle, children, layer, id, } = painter; + // 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 + // the draw in between reuse the slots -- see + // `Primitives::recycle`. + for h in recycle { + self.free_primitive(&h); + } + // add to active let active = ActiveData { id, @@ -698,19 +746,42 @@ impl UiRenderState { self.mov_count += 1; } + /// Retires `id`'s primitives (unless `keep_primitives`, in which case + /// they come back in the returned `ActiveData` for the redraw about to + /// happen to recycle -- see `Painter::take_recycled`), drops the mask + /// refs they held, and takes the widget out of `active`. + /// + /// The handles stay in the returned `ActiveData` either way, freed or + /// not: `remask_shape_users` below reads them, and so does the + /// caller. **A caller that passed `keep_primitives: false` must not + /// free them again** -- they name slots that may already have been + /// handed out. + /// + /// The mask refs are dropped either way: a recycled slot is rewritten + /// with whatever mask the *new* draw is under, and that draw takes its + /// own ref (`Painter::write_primitive`). + /// /// NOTE: instance textures are cleared and self.textures freed - fn remove(&mut self, id: WidgetId, undraw: bool, rsc: &mut dyn UiRsc) -> Option { + fn remove( + &mut self, + id: WidgetId, + undraw: bool, + keep_primitives: bool, + rsc: &mut dyn UiRsc, + ) -> Option { let mut active = self.active.remove(&id); if let Some(active) = &mut active { for h in &active.primitives { - let mask = self.primitives.free(h); - if h.pos != NOT_DRAWN { - self.layers[h.layer].free(h.pos, h.is_image()); - } + let mask = self.primitives.instance(h.slot).mask_idx; if mask != MaskIdx::NONE { rsc.ui_mut().masks.remove(mask); } } + if !keep_primitives { + for h in &active.primitives { + self.free_primitive(h); + } + } Self::remask_shape_users(&self.active, id, active.own_mask, &active.primitives, rsc); active.textures.clear(); rsc.ui_mut().textures.free(); @@ -753,6 +824,18 @@ impl UiRenderState { active } + /// Retires one primitive: its arena slot and, if a layer's draw order + /// names it, its position there. The two go together -- a slot handed + /// out again while its old order entry still names it would be drawn + /// twice -- which is why this is one function rather than two lines + /// repeated at each call site. + fn free_primitive(&mut self, h: &PrimitiveHandle) { + self.primitives.free(h); + if h.pos != NOT_DRAWN { + self.layers[h.layer].free(h.pos, h.is_image()); + } + } + /// A mask whose shape primitive was just freed clips to a slot that /// now holds something else, so the widget that owns it is marked for /// redraw -- its own `set_mask` is the only thing that resolves the @@ -802,7 +885,7 @@ impl UiRenderState { } fn remove_rec(&mut self, id: WidgetId, rsc: &mut dyn UiRsc) -> Option { - let inst = self.remove(id, true, rsc); + let inst = self.remove(id, true, false, rsc); if let Some(inst) = &inst { for c in &inst.children { self.remove_rec(*c, rsc); @@ -1117,7 +1200,7 @@ impl UiRenderState { return; } - let Some(active) = self.remove(id, false, rsc) else { + let Some(active) = self.remove(id, false, true, rsc) else { return; }; let old_size = active.size; @@ -1134,9 +1217,12 @@ impl UiRenderState { parent, parent_move_slot, active.mask, - Some(active.children), - Some(active.move_slot), - active.own_mask, + Retained { + children: active.children, + move_slot: Some(active.move_slot), + own_mask: active.own_mask, + primitives: active.primitives, + }, rsc, ); // If this widget's own reported size changed, its parent's layout diff --git a/core/src/util/arena.rs b/core/src/util/arena.rs index f224634..4a9dd6b 100644 --- a/core/src/util/arena.rs +++ b/core/src/util/arena.rs @@ -1,6 +1,6 @@ use std::ops::Deref; -use crate::util::{Id, IdNum, IdTracker}; +use crate::util::{Dirty, Id, IdNum, IdTracker}; pub struct Arena { data: Vec, @@ -45,7 +45,9 @@ impl Default for Arena { pub struct TrackedArena { inner: Arena, refs: Vec, - pub changed: bool, + /// Which entries changed since the last upload. Was a `bool`, so one + /// widget getting a move offset re-uploaded every other widget's. + pub dirty: Dirty, } impl TrackedArena { @@ -53,14 +55,14 @@ impl TrackedArena { Self { inner: Arena::default(), refs: Vec::new(), - changed: true, + dirty: Dirty::new_all(), } } pub fn push(&mut self, value: T) -> Id { - self.changed = true; let id = self.inner.push(value); let i = id.idx(); + self.dirty.mark(i); if i == self.refs.len() { self.refs.push(0); } @@ -76,10 +78,16 @@ impl TrackedArena { /// rather than replaced. Marks the arena changed so the GPU copy is /// re-uploaded. pub fn get_mut(&mut self, id: Id) -> &mut T { - self.changed = true; + self.dirty.mark(id.idx()); &mut self.inner.data[id.idx()] } + /// The entries and the dirty set together -- see + /// `PrimitiveVec::for_upload`. + pub fn for_upload(&mut self) -> (&[T], &mut Dirty) { + (&self.inner.data, &mut self.dirty) + } + pub fn remove(&mut self, id: Id) -> T where T: Copy, @@ -87,7 +95,6 @@ impl TrackedArena { let i = id.idx(); self.refs[i] -= 1; if self.refs[i] == 0 { - self.changed = true; self.inner.remove(id) } else { self[i] diff --git a/core/src/util/dirty.rs b/core/src/util/dirty.rs new file mode 100644 index 0000000..2a77f06 --- /dev/null +++ b/core/src/util/dirty.rs @@ -0,0 +1,150 @@ +//! Which entries of a GPU-bound array changed since the last upload. + +use std::ops::Range; + +/// A bitset of dirty entries, coalesced into a handful of ranges when it +/// is time to upload. +/// +/// **Why a bitset** rather than the two obvious alternatives, both of +/// which were measured against the bench fixture before this was written +/// (`scripts/rigs/ui-profile`'s `arena_churn`). A `min..max` span is far +/// too coarse: a frame's changes land in 5-20 runs scattered across the +/// whole arena, so the span is very nearly the whole buffer. A `Vec` of +/// touched indices is too expensive to *write*: a streaming frame marks +/// several thousand entries, which would mean an allocation and a sort +/// per frame. Marking a bit is O(1), allocation-free and idempotent, and +/// the scan that reads it back is one word per 64 entries. +#[derive(Default)] +pub struct Dirty { + words: Vec, + /// Everything is dirty regardless of the bits -- the state after a + /// buffer reallocation, whose contents are undefined, and the state a + /// freshly built arena starts in. Kept as a flag rather than by + /// setting every bit so that it costs nothing to say and cannot be + /// half-applied as the array grows. + all: bool, +} + +impl Dirty { + /// Nothing uploaded yet, so nothing may be assumed about the buffer. + pub fn new_all() -> Self { + Self { + words: Vec::new(), + all: true, + } + } + + pub fn mark(&mut self, i: usize) { + if self.all { + return; + } + let word = i / 64; + if word >= self.words.len() { + self.words.resize(word + 1, 0); + } + self.words[word] |= 1 << (i % 64); + } + + /// Everything must be written: the buffer was reallocated (its + /// contents are undefined), or the array was cleared. + pub fn mark_all(&mut self) { + self.all = true; + self.words.clear(); + } + + pub fn is_clean(&self) -> bool { + !self.all && self.words.iter().all(|w| *w == 0) + } + + /// The ranges to upload, in ascending order, merging two runs + /// separated by a gap of fewer than `gap` entries. + /// + /// Merging trades bytes for `write_buffer` calls, and the fixture + /// says the trade is very cheap in one direction: over a fling, a + /// 1 KiB gap costs 0.1% more bytes than merging nothing at all and + /// halves the worst-case call count (23 to 13). Past that it stops + /// paying -- 4 KiB is +2% bytes for two fewer calls. + pub fn ranges(&self, len: usize, gap: usize) -> Vec> { + if self.all { + return Vec::from_iter((len > 0).then_some(0..len)); + } + let mut ranges: Vec> = Vec::new(); + for (w, word) in self.words.iter().enumerate() { + let mut bits = *word; + while bits != 0 { + let start = w * 64 + bits.trailing_zeros() as usize; + // The run of set bits starting here, within this word. + let run = (bits >> (start - w * 64)).trailing_ones() as usize; + let end = (start + run).min(len); + if start >= len { + break; + } + match ranges.last_mut() { + // `start - last.end` is the gap; equal ends means + // adjacent, which always merges. + Some(last) if start - last.end <= gap => last.end = end, + _ => ranges.push(start..end), + } + bits &= !(((1u128 << run) - 1) as u64) << (start - w * 64); + } + } + ranges + } + + pub fn clear(&mut self) { + self.all = false; + self.words.clear(); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn marked(indices: &[usize], len: usize, gap: usize) -> Vec> { + let mut d = Dirty::default(); + for &i in indices { + d.mark(i); + } + d.ranges(len, gap) + } + + #[test] + fn adjacent_entries_are_one_range() { + assert_eq!(marked(&[3, 4, 5], 64, 0), vec![3..6]); + } + + #[test] + fn a_run_that_crosses_a_word_boundary_is_one_range() { + assert_eq!(marked(&[62, 63, 64, 65], 128, 0), vec![62..66]); + } + + #[test] + fn a_gap_wider_than_the_threshold_stays_two_ranges() { + assert_eq!(marked(&[0, 10], 64, 4), vec![0..1, 10..11]); + assert_eq!(marked(&[0, 10], 64, 16), vec![0..11]); + } + + #[test] + fn ranges_stop_at_the_length() { + // Entries marked and then dropped by a shrink must not be + // uploaded past the end of what the caller is writing. + assert_eq!(marked(&[1, 2, 40], 3, 0), vec![1..3]); + } + + #[test] + fn mark_all_covers_everything_and_survives_later_marks() { + let mut d = Dirty::new_all(); + d.mark(2); + assert_eq!(d.ranges(9, 0), vec![0..9]); + assert!(!d.is_clean()); + d.clear(); + assert!(d.is_clean()); + assert!(d.ranges(9, 0).is_empty()); + } + + #[test] + fn an_empty_array_has_nothing_to_upload_even_when_all_is_set() { + assert!(Dirty::new_all().ranges(0, 0).is_empty()); + } +} diff --git a/core/src/util/mod.rs b/core/src/util/mod.rs index 69c3331..d462326 100644 --- a/core/src/util/mod.rs +++ b/core/src/util/mod.rs @@ -1,6 +1,7 @@ mod arena; mod borrow; mod change; +mod dirty; mod id; mod math; mod refcount; @@ -12,6 +13,7 @@ mod vec2; pub use arena::*; pub use borrow::*; pub use change::*; +pub use dirty::*; pub use id::*; pub use math::*; pub use refcount::*;