commit 3d6abbc0db47ef3d860726ef29f2c05582c01f7c Author: William Casarin Date: Mon Sep 21 13:18:48 2026 -0700 core: budgets that are enumerated rather than named Every storage primitive already carried the measurement pillar 2 asks for — a name, occupancy, a capacity, peak() and overflows(). What none of them could do was be found. The consumer had to name each one, and the shell's per-second report showed what that costs: five accessors off three primitives in one format string, two of them tested for overflow in one warning branch, the third in another worded differently, and reset_stats called on each in turn. Adding storage meant editing four places in one function, and missing one produced the worst failure available, because an unreported budget is indistinguishable from a budget that is fine. So invert it. BudgetReport is six plain numbers, a Budgeted thing hands over one or more, and visit_all walks a borrowed slice. The shell now holds &[&dyn Budgeted] and formats reports; one line in the tree knows what storage exists. Nothing registers and the registry owns nothing: a report is built on demand from the live primitive, so there is no second copy of the truth to go stale and no registration call to forget. The registry is a slice literal, sized by the compiler, so there is no registry capacity to pick — which matters, because a capacity chosen before anything measures one is the guess BUDGETS.md exists to replace. A visitor rather than an iterator, and the reason is object safety. Vec allocates, which is denied here. impl Iterator does not, but it is not object-safe, so &dyn Budgeted could not exist and neither could the slice; aggregates make it worse, since Graph holds two budgets and a world will hold an aggregate of aggregates, chaining another iterator type per level of nesting. A visitor composes across that tree for free. The cost is that a consumer cannot take two reports and stop, which nothing wants to do. Graph reports its pool and its Order as two lines although they share a name and fill together: an ordering that refused a push while the pool still had slots is a bug in that module, and one folded line is a line in which that bug is invisible. The card's other half turned out to be already true — Exhausted, Overflow and Attach each carry the resource and the op — so what it wanted was a check, not code. tests/budgets.rs drives each primitive to capacity and reads back what it says, asserts a refusal leaves occupancy and the high-water mark exactly as they were, and asserts the overflow count survives the frame boundary that clears the contents. DrawList::push is the one deliberate exception: it is infallible so that rendering failures do not reach every gameplay call site, and the guarantee it owes is carried by its report instead, which is now a test rather than a comment. Two things fell out. DrawList had no name, alone among the primitives, and needed one to be reportable at all. And the shell's instance buffer is a budget that was being hand-reported: Batcher grew a name and a peak and implements the core's trait, which is the case worth having — a budget the core has never heard of, in the same enumeration, indistinguishable to a consumer. RenderStats.dropped went with it, being the second copy the enumeration now makes unnecessary. Unresolved handles deliberately stay out: a command naming a mesh that is not loaded would have been dropped by a buffer with room to spare, so reading it as capacity pressure sends whoever is looking to the wrong place. Not verified: the shell's printed report. There is no display in this session, so the window never opens. The enumeration itself is covered by tests on both sides of the boundary; the format string is not. 189 -> 212 tests. fmt, clippy (denies) and the suite pass. No unsafe changed, so miri stays out of the loop per AGENTS.md. Changelog-Added: Budget reports — every bounded resource can be enumerated without being named Headway: headway:gamedev/deny-cool-license Agentium: agentium:acid-absent-glory Co-Authored-By: Claude Opus 5 diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index abd814248571..c8717bff739b 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1029,6 +1029,73 @@ pool (`headway:gamedev/flat-actor-rotate`), because a capacity chosen before anything has measured one would be a guess, and `BUDGETS.md` takes numbers from a running machine. +## Budgets are enumerated, not named + +Pillar 2 asks for "simple visibility into resource use from the beginning". Every +storage primitive already carried the measurement — a name, occupancy, a +capacity, `peak()` and `overflows()`. What none of them could do was be *found*. + +The consumer had to name each one. The shell's per-second report read five +accessors off three primitives into one format string, tested two of them for +overflow in one warning branch, tested the third in another worded differently, +and called `reset_stats` on each in turn. Adding storage meant editing four +places in one function, and the failure when you missed one was the worst +available: an unreported budget is indistinguishable from a budget that is fine. + +So `budget.rs` inverts it. A `BudgetReport` is six plain numbers; a `Budgeted` +thing hands over one or more; `visit_all` walks a borrowed slice of them. The +shell now holds `&[&dyn Budgeted]` and formats reports, and there is exactly one +line in the codebase that knows what storage exists. + +**The registry owns nothing and nothing registers with it.** A report is built +on demand by reading the live primitive, so there is no second copy of the truth +to go stale and no registration call anybody can forget. The "registry" is a +slice literal at the call site — sized by the compiler, so there is no registry +capacity to pick, which matters because a capacity chosen before anything +measures one is exactly the guess `BUDGETS.md` exists to replace. + +**Why a visitor and not an iterator.** `Vec` allocates, which is +denied in the core. `impl Iterator` does not allocate but is not object-safe, so +`&dyn Budgeted` and therefore the slice above could not exist — and aggregates +make that worse, since a `Graph` holds two budgets and a world will hold an +aggregate of aggregates, so each level of nesting would chain another iterator +type through the signature. A visitor composes across that tree for free, stays +object-safe, and allocates nothing. The cost is that a consumer cannot take the +first two reports and stop, which nothing wants to do. + +**An aggregate reports its parts separately.** `Graph` visits its pool and its +`Order` as two lines although they share a name and fill together, because they +are separate storage that can be separately wrong: an ordering that refused a +push while the pool still had slots is a bug in that module, and a report that +had folded them into one line is a report in which that bug is invisible. + +**The counts are `u32` and convert by saturating.** Wrapping would turn a 5 GiB +arena into a plausible small number and make the report quietly false; +saturating produces `4294967295`, which is not a number any budget here has and +reads as "go and look". Neither case is expected — this engine measures its +budgets in tens of kilobytes — so the only question is which failure is legible. + +### A refusal names the resource and the operation + +The other half of pillar 2, and it was already true: `Exhausted` carries the +arena and the `op`, `Overflow` the pool, the `op` and the rejected value, and +`Attach` the graph, the `op` and the reason. What was missing was a check, so +`tests/budgets.rs` drives each primitive to capacity and reads back what it +says. It also asserts that a refused operation leaves occupancy and the +high-water mark byte for byte as they were, and that the overflow count survives +the frame boundary that clears the contents. + +**`DrawList::push` is the one deliberate exception.** It is infallible, because +a caller that had to handle a rendering failure at every call site would push +renderer concerns back into gameplay. The guarantee it owes is therefore carried +by the budget report instead — the resource is named there and the refusal is +counted there — and that substitution is a test rather than a comment. + +**Unresolved handles are not a budget.** A command whose mesh names nothing +would have been dropped by an instance buffer with room to spare, so counting it +as capacity pressure would send whoever is reading to the wrong place. It keeps +its own counter and its own warning line, outside the report. + ## World limits are not implementation limits The rule, restated because it is the easiest thing here to erode: **raising a diff --git a/crates/microverse_core/src/arena.rs b/crates/microverse_core/src/arena.rs index 9d58ac40f148..01b069a98da7 100644 --- a/crates/microverse_core/src/arena.rs +++ b/crates/microverse_core/src/arena.rs @@ -95,6 +95,7 @@ use core::cell::{Cell, UnsafeCell}; use core::fmt; use core::mem::{MaybeUninit, align_of, needs_drop, size_of}; +use crate::budget::{BudgetKind, BudgetReport, Budgeted}; use crate::snapshot::{Pod, ShapeMismatch}; /// The strictest alignment the arena guarantees, and the granularity its buffer @@ -527,6 +528,22 @@ impl Arena { self.peak.set(self.head.get()); } + /// What this arena is holding, for a budget report. + /// + /// Bytes, not items: an arena's occupancy includes the alignment padding + /// between allocations, so a count of allocations would understate what the + /// budget actually has to cover. + pub fn budget(&self) -> BudgetReport { + BudgetReport::from_usize_counts( + self.name, + self.head.get(), + self.capacity(), + self.peak.get(), + self.overflows.get(), + BudgetKind::Bytes, + ) + } + /// Reserve `size` bytes at `align`, or refuse and change nothing. /// /// Returns the offset of the reserved region. Every arithmetic step here is @@ -703,6 +720,12 @@ impl fmt::Debug for ArenaSnapshot { } } +impl Budgeted for Arena { + fn budgets(&self, visit: &mut dyn FnMut(BudgetReport)) { + visit(self.budget()); + } +} + impl fmt::Debug for Arena { /// The budget line, not the bytes. fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { diff --git a/crates/microverse_core/src/budget.rs b/crates/microverse_core/src/budget.rs new file mode 100644 index 000000000000..38ca9a144719 --- /dev/null +++ b/crates/microverse_core/src/budget.rs @@ -0,0 +1,366 @@ +//! One place to ask what the machine is holding. +//! +//! Every storage primitive already knows its own occupancy, its high-water mark +//! and how many times it refused. What it cannot do by itself is be *found*: a +//! caller that wants the whole picture has to name each one, read five +//! accessors off it, and know whether the numbers are bytes or items. That +//! knowledge spreads — into the shell's report line, into a debug overlay, into +//! whatever records baselines — and every copy of it is a place a budget can be +//! added without being reported, which is the one failure a budget report must +//! not have. +//! +//! So the shape here is deliberately small: a [`BudgetReport`] is plain data, a +//! [`Budgeted`] thing hands over one or more of them, and [`visit_all`] walks a +//! borrowed list of them. A consumer sees reports and never sees an arena, a +//! pool, or a draw list. +//! +//! # The registry does not own anything +//! +//! There is no registration step and nothing to keep in sync. A report is built +//! on demand by reading the live primitive, so there is no second copy of the +//! truth that can go stale, and no way to hold a report *and* the thing it +//! describes and have them disagree about anything except time. +//! +//! The "registry" is therefore a borrowed slice at the call site: +//! +//! ``` +//! use microverse_core::budget::{self, BudgetReport}; +//! use microverse_core::{Arena, DrawList}; +//! +//! let level = Arena::with_capacity("level", 4096); +//! let scratch = Arena::with_capacity("frame", 1024); +//! let draws = DrawList::with_capacity("draws", 512); +//! +//! let mut lines = 0; +//! budget::visit_all(&[&level, &scratch, &draws], &mut |b: BudgetReport| { +//! assert!(b.used <= b.cap); +//! lines += 1; +//! }); +//! assert_eq!(lines, 3); +//! ``` +//! +//! That array is a stack literal sized by the compiler, so adding a budget is +//! one word at one site and there is no registry capacity to pick — which +//! matters, because a capacity picked before anything measures one is exactly +//! the guess `BUDGETS.md` exists to replace. +//! +//! # Why a visitor and not an iterator +//! +//! Three shapes were available and two of them cost something the crate is not +//! willing to pay. +//! +//! Returning `Vec` allocates, which is denied here and would put +//! the report out of reach of anything on the frame path. +//! +//! Returning `impl Iterator` does not allocate, but it is +//! not object-safe, so `&dyn Budgeted` would be impossible and the borrowed +//! slice above could not exist. Aggregates make it worse: a [`Graph`] holds two +//! budgets, and a world will hold an aggregate of aggregates, so every level of +//! nesting would chain another iterator type through the signature. +//! +//! A visitor composes across that tree for free — an aggregate calls its +//! children's `budgets` with the same closure — stays object-safe, and +//! allocates nothing. The cost is that a caller cannot lazily take the first +//! two and stop, which nothing wants to do with a budget report. +//! +//! [`Graph`]: crate::transform::Graph +//! +//! # Order is stable +//! +//! Reports arrive in the order the implementations visit them, which is source +//! order and therefore fixed. That is a requirement rather than an +//! accident: a baseline that is recorded and diffed +//! (headway:gamedev/ugly-elephant-slide) compares runs line by line, and a set +//! that reordered itself between runs would read as every budget changing at +//! once. + +use core::fmt; + +/// What a budget is counted in. +/// +/// The unit is carried rather than inferred from the name because a consumer +/// formatting `84/4096` has no other way to know whether to write `B` or +/// `items`, and guessing from a name like `"level"` is the kind of thing that +/// works until somebody adds a pool called `"bytes"`. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum BudgetKind { + /// Slots, nodes, commands — anything counted one at a time. + Items, + /// Bytes of a byte buffer, as an [`Arena`] counts them. + /// + /// [`Arena`]: crate::arena::Arena + Bytes, +} + +impl BudgetKind { + /// The unit, short enough to sit after a number in a report line. + pub const fn unit(self) -> &'static str { + match self { + BudgetKind::Items => "items", + BudgetKind::Bytes => "B", + } + } +} + +/// What one bounded resource is holding, and what it has held. +/// +/// Plain `Copy` data with public fields. It is a *reading*, not a handle: it +/// borrows nothing, so it can be stored, compared against a previous run, or +/// sent somewhere, and it says nothing about the primitive it came from beyond +/// these six numbers. +/// +/// The counts are `u32` rather than `usize` so that a report means the same +/// thing on every target and can be compared against one recorded elsewhere. +/// See [`BudgetReport::from_usize_counts`] for what happens to a count that +/// does not fit. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct BudgetReport { + /// What the resource calls itself — the name it was constructed with. + pub name: &'static str, + /// Occupancy right now. + pub used: u32, + /// The fixed capacity it was built with. + pub cap: u32, + /// The high-water mark since the last `reset_stats`, which is the number a + /// budget should be set from. + pub peak: u32, + /// How many operations were refused for want of capacity since the last + /// `reset_stats`. + pub overflows: u32, + /// Whether the counts are items or bytes. + pub kind: BudgetKind, +} + +impl BudgetReport { + /// Build a report from counts a primitive keeps as `usize`. + /// + /// Counts that exceed [`u32::MAX`] **saturate**, and that choice is the + /// interesting part. Wrapping would turn a 5 GiB arena into a plausible + /// small number and quietly make the report a lie; saturating produces + /// `4294967295`, which is not a number any budget in this engine has and + /// therefore reads as "go and look" rather than as data. Neither case is + /// expected — the engine measures its budgets in tens of kilobytes — so the + /// question is only which failure is legible, and an absurd number is. + pub const fn from_usize_counts( + name: &'static str, + used: usize, + cap: usize, + peak: usize, + overflows: u32, + kind: BudgetKind, + ) -> Self { + Self { + name, + used: saturate(used), + cap: saturate(cap), + peak: saturate(peak), + overflows, + kind, + } + } + + /// Occupancy as a fraction of capacity, for a bar an overlay can draw. + /// + /// A zero-capacity resource reads as `1.0`. It holds nothing and never can, + /// so every operation on it overflows — "full" is the honest reading, and + /// it is the one that keeps `0 / 0` from putting a `NaN` into a layout. + pub fn fraction(&self) -> f32 { + if self.cap == 0 { + return 1.0; + } + self.used as f32 / self.cap as f32 + } + + /// The high-water mark as a fraction of capacity. See [`Self::fraction`] + /// for the zero-capacity case, which is the same. + pub fn peak_fraction(&self) -> f32 { + if self.cap == 0 { + return 1.0; + } + self.peak as f32 / self.cap as f32 + } + + /// Whether anything was refused for want of capacity. + /// + /// The question a report line is usually being scanned for, and a name for + /// it here keeps `> 0` from being written at every call site. + pub const fn overflowed(&self) -> bool { + self.overflows > 0 + } +} + +impl fmt::Display for BudgetReport { + /// One line, in the order a reader asks the questions: what is it, how full + /// is it, how full has it been, did it refuse anything. + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "{} {}/{} {} peak {} overflows {}", + self.name, + self.used, + self.cap, + self.kind.unit(), + self.peak, + self.overflows, + ) + } +} + +/// Something that can say what it is holding. +/// +/// Implemented by each storage primitive for itself, and by anything that owns +/// several so that a consumer never has to know the difference. A [`Graph`] +/// reports two budgets, a [`Pool`] reports one, and both are one call. +/// +/// [`Graph`]: crate::transform::Graph +/// [`Pool`]: crate::pool::Pool +/// +/// # Implementing it +/// +/// Visit every budget you own, in a fixed order, and visit nothing else. An +/// aggregate forwards the same closure to its children: +/// +/// ``` +/// use microverse_core::budget::{BudgetReport, Budgeted}; +/// use microverse_core::{Arena, DrawList}; +/// +/// struct Machine { +/// scratch: Arena, +/// draws: DrawList, +/// } +/// +/// impl Budgeted for Machine { +/// fn budgets(&self, visit: &mut dyn FnMut(BudgetReport)) { +/// self.scratch.budgets(visit); +/// self.draws.budgets(visit); +/// } +/// } +/// +/// let m = Machine { +/// scratch: Arena::with_capacity("frame", 64), +/// draws: DrawList::with_capacity("draws", 8), +/// }; +/// +/// let mut names = ["", ""]; +/// let mut n = 0; +/// m.budgets(&mut |b| { +/// names[n] = b.name; +/// n += 1; +/// }); +/// assert_eq!(names, ["frame", "draws"]); +/// ``` +/// +/// The closure is `&mut dyn FnMut` rather than a generic parameter so that the +/// trait stays object-safe. That is what makes `&dyn Budgeted` — and therefore +/// [`visit_all`] — possible, and it costs one indirect call per budget on a +/// path that runs once a second at most. +pub trait Budgeted { + /// Hand every budget this owns to `visit`, in a stable order. + fn budgets(&self, visit: &mut dyn FnMut(BudgetReport)); +} + +/// Walk several budgeted things as though they were one. +/// +/// The whole registry: a borrowed slice, read in order, owning nothing. The +/// shell builds the slice at the one site that knows what exists, and every +/// consumer downstream of this call sees only reports. +/// +/// ``` +/// use microverse_core::budget; +/// use microverse_core::{Arena, DrawList}; +/// +/// let level = Arena::with_capacity("level", 128); +/// let draws = DrawList::with_capacity("draws", 4); +/// +/// let mut worst = 0.0f32; +/// budget::visit_all(&[&level, &draws], &mut |b| worst = worst.max(b.fraction())); +/// ``` +pub fn visit_all(sources: &[&dyn Budgeted], visit: &mut dyn FnMut(BudgetReport)) { + for source in sources { + source.budgets(visit); + } +} + +/// A `usize` count as a `u32`, saturating rather than wrapping. See +/// [`BudgetReport::from_usize_counts`] for why. +const fn saturate(n: usize) -> u32 { + if n > u32::MAX as usize { u32::MAX } else { n as u32 } +} + +#[cfg(test)] +// Test code allocates freely. The rule is about the frame path, and a test that +// had to assemble its expectations in a fixed-capacity buffer would be testing +// the buffer. What allocation the frame path actually does is measured in +// `tests/no_alloc.rs`, not guessed at from a lint. +#[allow( + clippy::disallowed_methods, + clippy::disallowed_macros, + reason = "test code is not the frame path" +)] +mod tests { + use super::*; + + fn report(used: u32, cap: u32) -> BudgetReport { + BudgetReport { name: "b", used, cap, peak: used, overflows: 0, kind: BudgetKind::Items } + } + + #[test] + fn a_report_reads_as_one_line_with_its_unit() { + let bytes = BudgetReport { + name: "level", + used: 84, + cap: 4096, + peak: 84, + overflows: 0, + kind: BudgetKind::Bytes, + }; + assert_eq!(bytes.to_string(), "level 84/4096 B peak 84 overflows 0"); + + let items = BudgetReport { + name: "draws", + used: 441, + cap: 4096, + peak: 441, + overflows: 2, + kind: BudgetKind::Items, + }; + assert_eq!(items.to_string(), "draws 441/4096 items peak 441 overflows 2"); + } + + #[test] + fn occupancy_is_a_fraction_and_an_empty_budget_is_not_full() { + assert_eq!(report(0, 4).fraction(), 0.0); + assert_eq!(report(2, 4).fraction(), 0.5); + assert_eq!(report(4, 4).fraction(), 1.0); + } + + /// A zero-capacity budget is a real, tested configuration — a draw list + /// that may submit nothing — and the arithmetic must not hand a `NaN` to + /// whatever is laying out a bar. + #[test] + fn a_budget_with_no_capacity_reads_as_full_rather_than_as_nan() { + let empty = report(0, 0); + assert_eq!(empty.fraction(), 1.0); + assert_eq!(empty.peak_fraction(), 1.0); + assert!(!empty.fraction().is_nan()); + } + + /// The failure that matters is a count that *wraps* into a plausible small + /// number. Saturation is chosen so an impossible count reads as impossible. + #[test] + fn a_count_too_large_for_the_report_saturates_instead_of_wrapping() { + let huge = u32::MAX as usize + 1; + let b = BudgetReport::from_usize_counts("vast", huge, huge, huge, 0, BudgetKind::Bytes); + assert_eq!(b.used, u32::MAX, "a wrap here would have read as 0"); + assert_eq!(b.cap, u32::MAX); + assert_eq!(b.peak, u32::MAX); + } + + #[test] + fn overflowed_is_the_question_a_report_line_is_scanned_for() { + assert!(!report(1, 4).overflowed()); + let mut refused = report(4, 4); + refused.overflows = 1; + assert!(refused.overflowed()); + } +} diff --git a/crates/microverse_core/src/draw.rs b/crates/microverse_core/src/draw.rs index f88186d61feb..feb8546d88f3 100644 --- a/crates/microverse_core/src/draw.rs +++ b/crates/microverse_core/src/draw.rs @@ -10,6 +10,7 @@ //! This is a **renderer** budget, not a world budget. Raising [`DrawList`]'s //! capacity must never change what the game permits. See `ARCHITECTURE.md`. +use crate::budget::{BudgetKind, BudgetReport, Budgeted}; use crate::pool::Handle; /// A mesh, named but not held. @@ -60,6 +61,8 @@ pub struct DrawCmd { /// succession without threading a lifetime through everything that holds one. pub struct DrawList { cmds: Box<[DrawCmd]>, + /// What this list calls itself in a budget report and in a warning. + name: &'static str, len: usize, /// Commands dropped for want of capacity, since the last [`Self::reset_stats`]. dropped: u32, @@ -69,7 +72,15 @@ pub struct DrawList { impl DrawList { /// Allocate the command buffer. This is the only allocation it ever does. - pub fn with_capacity(cap: usize) -> Self { + /// + /// `name` is what this list is called in a [`BudgetReport`], for the same + /// reason every other storage primitive takes one: a report that enumerates + /// budgets without knowing what any of them are has nothing else to print, + /// and two lists — a shadow pass and a main pass — would otherwise be + /// indistinguishable in the one place that exists to tell them apart. + /// + /// [`BudgetReport`]: crate::budget::BudgetReport + pub fn with_capacity(name: &'static str, cap: usize) -> Self { // Filler for the slots past `len`, which `commands()` never exposes. A // deliberately absurd handle rather than a plausible one, so that a bug // that did expose it reads as garbage instead of as mesh zero. @@ -84,7 +95,12 @@ impl DrawList { // storage declaring itself, which is what `tests/storage.rs` checks. #[allow(clippy::disallowed_macros, reason = "startup: the draw list's one allocation")] let cmds = vec![empty; cap].into_boxed_slice(); - Self { cmds, len: 0, dropped: 0, peak: 0 } + Self { cmds, name, len: 0, dropped: 0, peak: 0 } + } + + /// What this list is called in a budget report. + pub fn name(&self) -> &'static str { + self.name } /// Submit a command. Dropped and counted if the buffer is full. @@ -139,6 +155,29 @@ impl DrawList { self.dropped = 0; self.peak = self.len; } + + /// What this list is holding, for a budget report. + /// + /// `dropped` is the overflow count. The two words describe the same event + /// from different ends — the list drops a command, the budget records a + /// refusal — and the report uses the budget's word so that a consumer + /// comparing an arena against a draw list is comparing the same thing. + pub fn budget(&self) -> BudgetReport { + BudgetReport::from_usize_counts( + self.name, + self.len, + self.cmds.len(), + self.peak, + self.dropped, + BudgetKind::Items, + ) + } +} + +impl Budgeted for DrawList { + fn budgets(&self, visit: &mut dyn FnMut(BudgetReport)) { + visit(self.budget()); + } } #[cfg(test)] @@ -156,7 +195,7 @@ mod tests { #[test] fn overflow_drops_and_counts_without_growing() { - let mut list = DrawList::with_capacity(2); + let mut list = DrawList::with_capacity("draws", 2); for i in 0..5 { list.push(cmd(i)); } @@ -173,7 +212,7 @@ mod tests { #[test] fn clear_keeps_capacity_and_peak() { - let mut list = DrawList::with_capacity(4); + let mut list = DrawList::with_capacity("draws", 4); list.push(cmd(0)); list.push(cmd(1)); list.clear(); @@ -189,7 +228,7 @@ mod tests { // the count is only worth anything if it is still there when somebody // prints the budget report at the end of the second. `clear` runs every // frame; `reset_stats` runs when a report has been taken. - let mut list = DrawList::with_capacity(1); + let mut list = DrawList::with_capacity("draws", 1); for i in 0..4 { list.push(cmd(i)); } diff --git a/crates/microverse_core/src/lib.rs b/crates/microverse_core/src/lib.rs index 07c884322afe..a63e7896b112 100644 --- a/crates/microverse_core/src/lib.rs +++ b/crates/microverse_core/src/lib.rs @@ -9,6 +9,7 @@ //! at startup and at area-load time is expected. See `ARCHITECTURE.md`. pub mod arena; +pub mod budget; pub mod draw; pub mod input; pub mod order; @@ -18,6 +19,7 @@ pub mod transform; pub mod world; pub use arena::{Arena, ArenaSnapshot, Exhausted}; +pub use budget::{BudgetKind, BudgetReport, Budgeted}; pub use draw::{DrawCmd, DrawList, Material, Mesh}; pub use input::{Axis, Edge, Input, Key, MouseButton}; pub use order::Order; @@ -51,7 +53,7 @@ pub use world::World; /// let mut scratch = Arena::with_capacity("frame", 4096); /// let mut world = World::load(&level); /// # let input = Input::new(); -/// # let mut draws = DrawList::with_capacity(1024); +/// # let mut draws = DrawList::with_capacity("draws", 1024); /// for _ in 0..3 { /// scratch.reset(); // &mut self — legal here, nothing borrows it /// frame(&mut world, &scratch, &input, &mut draws, 1.0 / 60.0); @@ -71,7 +73,7 @@ pub use world::World; /// let mut scratch = Arena::with_capacity("frame", 4096); /// # let mut world = World::load(&level); /// # let input = Input::new(); -/// # let mut draws = DrawList::with_capacity(1024); +/// # let mut draws = DrawList::with_capacity("draws", 1024); /// let kept = scratch.alloc(1u32).unwrap(); /// scratch.reset(); // the reset the loop does every frame /// *kept = 2; // and the reference that did not survive it @@ -271,7 +273,7 @@ mod tests { let scratch = Arena::with_capacity("frame", 1024); let mut world = World::load(&level); let input = Input::new(); - let mut out = DrawList::with_capacity(4096); + let mut out = DrawList::with_capacity("draws", 4096); frame(&mut world, &scratch, &input, &mut out, 1.0 / 60.0); @@ -298,7 +300,7 @@ mod tests { let scratch = Arena::with_capacity("frame", 1024); let mut world = World::load(&level); let input = Input::new(); - let mut out = DrawList::with_capacity(4096); + let mut out = DrawList::with_capacity("draws", 4096); frame(&mut world, &scratch, &input, &mut out, 1.0 / 60.0); let runs = out @@ -315,7 +317,7 @@ mod tests { let mut scratch = Arena::with_capacity("frame", 1024); let mut world = World::load(&level); let mut input = Input::new(); - let mut out = DrawList::with_capacity(4096); + let mut out = DrawList::with_capacity("draws", 4096); frame(&mut world, &scratch, &input, &mut out, 1.0 / 60.0); let still = out.commands()[0].xform; @@ -343,8 +345,8 @@ mod tests { let mut a = World::load(&level); let mut b = World::load(&level); let input = Input::new(); - let mut hoisted = DrawList::with_capacity(4096); - let mut inline = DrawList::with_capacity(4096); + let mut hoisted = DrawList::with_capacity("draws", 4096); + let mut inline = DrawList::with_capacity("draws", 4096); frame(&mut a, &roomy, &input, &mut hoisted, 1.0 / 60.0); frame(&mut b, &cramped, &input, &mut inline, 1.0 / 60.0); @@ -383,7 +385,7 @@ mod tests { let mut scratch = Arena::with_capacity("frame", 1024); let mut world = World::load(&level); let input = Input::new(); - let mut out = DrawList::with_capacity(4096); + let mut out = DrawList::with_capacity("draws", 4096); for _ in 0..100 { scratch.reset(); diff --git a/crates/microverse_core/src/order.rs b/crates/microverse_core/src/order.rs index 846f09c36aa9..e9c7d1df89d7 100644 --- a/crates/microverse_core/src/order.rs +++ b/crates/microverse_core/src/order.rs @@ -76,6 +76,7 @@ use core::fmt; +use crate::budget::{BudgetKind, BudgetReport, Budgeted}; use crate::pool::{Handle, Overflow, Pool}; /// A fixed-capacity ordering of handles into a pool. @@ -232,6 +233,29 @@ impl Order { self.peak = self.len; self.overflows = 0; } + + /// What this ordering is holding, for a budget report. + /// + /// It reports under the pool's name, because that is the name it was built + /// with and the two capacities are the same number by construction. A + /// reader seeing the name twice is seeing something true: the pool's slots + /// and this ordering of them are two budgets that fill together. + pub fn budget(&self) -> BudgetReport { + BudgetReport { + name: self.name, + used: self.len, + cap: self.capacity(), + peak: self.peak, + overflows: self.overflows, + kind: BudgetKind::Items, + } + } +} + +impl Budgeted for Order { + fn budgets(&self, visit: &mut dyn FnMut(BudgetReport)) { + visit(self.budget()); + } } impl fmt::Debug for Order { diff --git a/crates/microverse_core/src/pool.rs b/crates/microverse_core/src/pool.rs index 7c35f7063efe..bf7f5c56ff18 100644 --- a/crates/microverse_core/src/pool.rs +++ b/crates/microverse_core/src/pool.rs @@ -55,6 +55,7 @@ use core::marker::PhantomData; use core::num::NonZeroU32; use core::ops::{Index, IndexMut}; +use crate::budget::{BudgetKind, BudgetReport, Budgeted}; use crate::snapshot::{Pod, ShapeMismatch}; /// The generation every slot starts at. Handles never carry zero, which is what @@ -632,6 +633,24 @@ impl Pool { self.peak = self.len; } + /// What this pool is holding, for a budget report. + /// + /// `retired` is deliberately not in the report. It is lost capacity rather + /// than occupancy, it does not belong to the interval `reset_stats` bounds, + /// and folding it into any of the six fields would make one of them mean + /// two things. A pool that has retired a slot needs its own line, and does + /// not have one yet. + pub fn budget(&self) -> BudgetReport { + BudgetReport { + name: self.name, + used: self.len, + cap: self.capacity_u32(), + peak: self.peak, + overflows: self.overflows, + kind: BudgetKind::Items, + } + } + fn capacity_u32(&self) -> u32 { self.slots.len() as u32 } @@ -931,6 +950,12 @@ impl IndexMut> for Pool { } } +impl Budgeted for Pool { + fn budgets(&self, visit: &mut dyn FnMut(BudgetReport)) { + visit(self.budget()); + } +} + impl fmt::Debug for Pool { /// The budget line, not the contents. A pool's contents are the game; its /// occupancy is the thing you want when something is wrong. diff --git a/crates/microverse_core/src/transform.rs b/crates/microverse_core/src/transform.rs index d53c12b090da..4a0b59075fc3 100644 --- a/crates/microverse_core/src/transform.rs +++ b/crates/microverse_core/src/transform.rs @@ -135,6 +135,7 @@ use core::fmt; +use crate::budget::{BudgetReport, Budgeted}; use crate::order::Order; use crate::pool::{AtCapacity, Handle, Pool}; @@ -810,6 +811,20 @@ impl Graph { } } +impl Budgeted for Graph { + /// Two budgets, not one: the pool's slots and the ordering of them. + /// + /// They are reported separately although they fill together and share a + /// name, because they are separate storage that can be separately wrong. An + /// ordering that refused a push while the pool still had slots is a bug in + /// this module, and a report that had folded the two into one line is a + /// report in which that bug is invisible. + fn budgets(&self, visit: &mut dyn FnMut(BudgetReport)) { + self.nodes.budgets(visit); + self.order.budgets(visit); + } +} + impl fmt::Debug for Graph { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Graph") diff --git a/crates/microverse_core/src/world.rs b/crates/microverse_core/src/world.rs index c8a4321cee51..0f9ffaab1d13 100644 --- a/crates/microverse_core/src/world.rs +++ b/crates/microverse_core/src/world.rs @@ -50,6 +50,7 @@ //! [`Handle`]: crate::pool::Handle use crate::arena::Arena; +use crate::budget::{BudgetReport, Budgeted}; /// Everything the simulation owns, for the length of one area. /// @@ -123,3 +124,19 @@ impl<'a> World<'a> { self.level } } + +impl Budgeted for World<'_> { + /// The level arena, and for now nothing else. + /// + /// This is the site the card's "every pool and arena registers" actually + /// lands on: a world that grows an actor pool grows a line here, and a + /// consumer enumerating budgets picks it up without being told. Frame + /// scratch and the draw list are absent because the world does not own + /// them — they are the shell's, and the shell puts them in the slice it + /// passes to [`visit_all`]. + /// + /// [`visit_all`]: crate::budget::visit_all + fn budgets(&self, visit: &mut dyn FnMut(BudgetReport)) { + self.level.budgets(visit); + } +} diff --git a/crates/microverse_core/tests/budgets.rs b/crates/microverse_core/tests/budgets.rs new file mode 100644 index 000000000000..f6206de3b7b8 --- /dev/null +++ b/crates/microverse_core/tests/budgets.rs @@ -0,0 +1,270 @@ +//! What a budget report has to be true of, checked rather than trusted. +//! +//! Two properties, which are the two halves of +//! `headway:gamedev/deny-cool-license` and the two halves of pillar 2. +//! +//! **Every bounded resource can be found without being named.** A consumer +//! holds `&[&dyn Budgeted]` and gets [`BudgetReport`]s; it never learns that an +//! arena or a pool exists. The failure this guards against is a budget that is +//! added and then silently not reported, which is the one failure a budget +//! report must not have — an unreported budget is indistinguishable from a +//! budget that is fine. +//! +//! **A refusal names the exhausted resource and the operation that wanted +//! it.** Not `None`, not a bare `false`. The test drives each primitive to +//! capacity and reads what comes back. +//! +//! # What it cannot see +//! +//! A primitive added to the crate and never put in a slice anywhere. This file +//! enumerates what exists today, so it checks the contract each primitive +//! honours rather than that the set is complete; `tests/storage.rs` is what +//! notices a new storage kind appearing. + +// Reading a refusal back means rendering it, which allocates a String. Outside +// the rule, like every other test: the frame path is `frame()`, measured in +// `no_alloc.rs`. +#![allow(clippy::disallowed_methods, reason = "reading an error message is not the frame path")] + +use microverse_core::budget::{self, BudgetKind, BudgetReport, Budgeted}; +use microverse_core::{Arena, AtCapacity, DrawList, Graph, Order, Pool, Transform, World}; + +/// A consumer that knows nothing about what it is reading. +/// +/// The signature is the point: `&dyn Budgeted` in, counts out. If this function +/// compiles and runs, a debug overlay or a baseline recorder can be written the +/// same way. +fn summarise(sources: &[&dyn Budgeted]) -> (usize, u32) { + let mut lines = 0; + let mut refusals = 0; + budget::visit_all(sources, &mut |b: BudgetReport| { + lines += 1; + refusals += b.overflows; + }); + (lines, refusals) +} + +#[test] +fn a_consumer_enumerates_every_budget_without_naming_a_single_type() { + let level = Arena::with_capacity("level", 256); + let scratch = Arena::with_capacity("frame", 64); + let draws = DrawList::with_capacity("draws", 8); + let pool: Pool = Pool::with_capacity("things", 4, AtCapacity::Reject); + + let (lines, refusals) = summarise(&[&level, &scratch, &draws, &pool]); + + assert_eq!(lines, 4, "one line per budget"); + assert_eq!(refusals, 0, "nothing has been asked for yet"); +} + +/// The aggregate case, which is why this is a visitor and not an accessor: a +/// graph owns two budgets and the caller does not have to know that. +#[test] +fn an_aggregate_reports_every_budget_it_owns_and_the_caller_cannot_tell() { + let graph = Graph::with_capacity("transform nodes", 16); + + let mut names = ["", "", "", ""]; + let mut n = 0; + graph.budgets(&mut |b| { + names[n] = b.name; + n += 1; + }); + + assert_eq!(n, 2, "the pool's slots and the ordering of them"); + assert_eq!(names[0], "transform nodes"); + assert_eq!(names[1], "transform nodes", "the ordering is named for its pool"); +} + +/// A world is an aggregate too, and it is the site a later card grows: an actor +/// pool added to `World` has to appear here without this test being edited. +#[test] +fn a_world_reports_the_storage_it_owns() { + let level = Arena::with_capacity("level", 128); + let world = World::load(&level); + + let (lines, _) = summarise(&[&world]); + assert_eq!(lines, 1, "the level arena, until the world owns more"); +} + +/// A baseline that is recorded and diffed compares runs line by line +/// (headway:gamedev/ugly-elephant-slide). A set that reordered itself between +/// two calls would read as every budget changing at once. +#[test] +fn the_order_of_the_reports_is_stable_between_calls() { + let level = Arena::with_capacity("level", 64); + let graph = Graph::with_capacity("nodes", 4); + let draws = DrawList::with_capacity("draws", 4); + let sources: &[&dyn Budgeted] = &[&level, &graph, &draws]; + + let mut first = [("", 0u32); 8]; + let mut i = 0; + budget::visit_all(sources, &mut |b| { + first[i] = (b.name, b.cap); + i += 1; + }); + + let mut second = [("", 0u32); 8]; + let mut j = 0; + budget::visit_all(sources, &mut |b| { + second[j] = (b.name, b.cap); + j += 1; + }); + + assert_eq!(i, 4, "level, the graph's two, draws"); + assert_eq!(first, second); +} + +/// Invariants every report has to satisfy whatever produced it. An overlay +/// drawing a bar and a baseline computing a delta both assume these. +#[test] +fn every_report_is_internally_consistent() { + let arena = Arena::with_capacity("level", 128); + arena.alloc(7u32).expect("room for one u32"); + let mut pool: Pool = Pool::with_capacity("things", 4, AtCapacity::Reject); + pool.insert(1).expect("room for one"); + let mut draws = DrawList::with_capacity("draws", 4); + draws.clear(); + + budget::visit_all(&[&arena, &pool, &draws], &mut |b| { + assert!(b.used <= b.cap, "{} holds more than it has: {b}", b.name); + assert!(b.peak >= b.used, "{} peaked below its occupancy: {b}", b.name); + assert!(b.peak <= b.cap, "{} peaked above its capacity: {b}", b.name); + assert!(!b.name.is_empty(), "a budget with no name cannot be reported"); + assert!(b.fraction().is_finite(), "{} produced a non-finite bar: {b}", b.name); + }); +} + +#[test] +fn an_arena_counts_bytes_and_everything_else_counts_items() { + let arena = Arena::with_capacity("level", 128); + let pool: Pool = Pool::with_capacity("things", 4, AtCapacity::Reject); + let draws = DrawList::with_capacity("draws", 4); + + assert_eq!(arena.budget().kind, BudgetKind::Bytes); + assert_eq!(pool.budget().kind, BudgetKind::Items); + assert_eq!(draws.budget().kind, BudgetKind::Items); +} + +// ---- A refusal names the resource and the operation ----------------------- + +#[test] +fn an_arena_refusal_names_the_arena_and_the_allocation_that_asked() { + let arena = Arena::with_capacity("level", 8); + let err = arena.alloc_slice_fill(64usize, 0u32).expect_err("64 u32 do not fit in 8 bytes"); + + assert_eq!(err.arena, "level"); + assert_eq!(err.op, "alloc_slice_fill"); + let shown = err.to_string(); + assert!(shown.contains("level"), "the resource is not named: {shown}"); + assert!(shown.contains("alloc_slice_fill"), "the operation is not named: {shown}"); +} + +#[test] +fn a_pool_refusal_names_the_pool_and_the_insert_that_asked() { + let mut pool: Pool = Pool::with_capacity("things", 1, AtCapacity::Reject); + pool.insert(1).expect("the first fits"); + let err = pool.insert(2).expect_err("the second does not"); + + assert_eq!(err.pool, "things"); + assert_eq!(err.op, "insert"); + assert_eq!(err.into_value(), 2, "and the value is handed back, not dropped"); +} + +#[test] +fn an_ordering_refusal_names_the_pool_it_orders_and_the_push_that_asked() { + let mut pool: Pool = Pool::with_capacity("things", 1, AtCapacity::Reject); + let a = pool.insert(1).expect("the first fits"); + let mut order = Order::for_pool(&pool); + order.push(a).expect("the first fits here too"); + + let err = order.push(a).expect_err("the ordering is one slot wide"); + assert_eq!(err.pool, "things"); + assert_eq!(err.op, "push"); +} + +#[test] +fn a_graph_refusal_names_the_graph_and_the_attachment_that_asked() { + let mut graph = Graph::with_capacity("transform nodes", 1); + let root = graph.insert(Transform::default()).expect("the first fits"); + + let err = graph.insert_child(root, Transform::default()).expect_err("the graph is full"); + assert_eq!(err.graph, "transform nodes"); + assert_eq!(err.op, "insert_child"); + let shown = err.to_string(); + assert!(shown.contains("transform nodes"), "the resource is not named: {shown}"); + assert!(shown.contains("insert_child"), "the operation is not named: {shown}"); +} + +/// The one refusal that is not an error, and why that is still honest. +/// +/// [`DrawList::push`] is infallible on purpose — a caller that had to handle a +/// rendering failure at every call site would push renderer concerns back into +/// gameplay. So the guarantee it owes is carried by the budget report instead: +/// the resource is named there, and the refusal is counted there. This test is +/// what makes that substitution a checked claim rather than a comment. +#[test] +fn the_draw_lists_silent_refusal_is_named_and_counted_in_its_report() { + let mut draws = DrawList::with_capacity("draws", 1); + draws.push(cmd()); + draws.push(cmd()); + draws.push(cmd()); + + let b = draws.budget(); + assert_eq!(b.name, "draws", "the resource is named where the error would have been"); + assert_eq!(b.overflows, 2, "and both refusals are counted, not hidden"); + assert!(b.overflowed()); +} + +// ---- A refusal changes nothing, and is still visible a frame later -------- + +/// Pillar 2: exhaustion never corrupts state. A refused operation leaves the +/// storage as it was — not a wrap, not a partial write, not a head that moved. +#[test] +fn a_refused_operation_leaves_the_budget_exactly_as_it_was() { + let arena = Arena::with_capacity("level", 16); + arena.alloc(1u32).expect("room for one u32"); + let before = arena.budget(); + + arena.alloc_slice_fill(64usize, 0u32).expect_err("this cannot fit"); + let after = arena.budget(); + + assert_eq!(before.used, after.used, "the head moved on a refused allocation"); + assert_eq!(before.peak, after.peak, "a refusal must not raise the high-water mark"); + assert_eq!(after.overflows, before.overflows + 1, "but it is counted"); +} + +/// The card's "done when": an overflow increments a counter that is still +/// visible a frame later. The frame boundary is `clear()`, which drops the +/// commands and keeps the measurements. +#[test] +fn an_overflow_is_still_visible_a_frame_after_the_frame_that_caused_it() { + let mut draws = DrawList::with_capacity("draws", 1); + draws.push(cmd()); + draws.push(cmd()); + assert_eq!(draws.budget().overflows, 1); + + // The frame boundary. A budget report taken now is describing the frame + // that just ended, which is the only time anything gets to look at it. + draws.clear(); + assert_eq!(draws.budget().overflows, 1, "the refusal did not survive the frame"); + assert_eq!(draws.budget().peak, 1, "nor did the high-water mark"); + assert_eq!(draws.budget().used, 0, "though the commands did not survive it"); + + // And it is still there on the next frame's first look, until something + // explicitly clears it. + draws.push(cmd()); + assert_eq!(draws.budget().overflows, 1); + + draws.reset_stats(); + assert_eq!(draws.budget().overflows, 0, "cleared only when asked"); +} + +fn cmd() -> microverse_core::DrawCmd { + use microverse_core::Handle; + let g = core::num::NonZeroU32::MIN; + microverse_core::DrawCmd { + mesh: Handle::from_raw(0, g), + material: Handle::from_raw(0, g), + xform: [0.0; 16], + } +} diff --git a/crates/microverse_core/tests/no_alloc.rs b/crates/microverse_core/tests/no_alloc.rs index c83c2fcdee38..529bcc506a6a 100644 --- a/crates/microverse_core/tests/no_alloc.rs +++ b/crates/microverse_core/tests/no_alloc.rs @@ -305,7 +305,7 @@ fn the_frame_path_allocates_nothing() { let mut scratch = Arena::with_capacity("frame", FRAME_BYTES); let mut world = populated_world(&level); let mut input = Input::new(); - let mut draws = DrawList::with_capacity(MAX_DRAW_COMMANDS); + let mut draws = DrawList::with_capacity("draws", MAX_DRAW_COMMANDS); // Frame numbers start at one: zero is `Edge`'s "never happened" sentinel. for n in 1..=FRAMES { @@ -350,7 +350,7 @@ fn the_draw_list_overflow_path_allocates_nothing_either() { let mut scratch = Arena::with_capacity("frame", FRAME_BYTES); let mut world = populated_world(&level); let mut input = Input::new(); - let mut draws = DrawList::with_capacity(0); + let mut draws = DrawList::with_capacity("draws", 0); for n in 1..=8 { drive_input(&mut input, n); diff --git a/crates/microverse_shell/src/batch.rs b/crates/microverse_shell/src/batch.rs index afd03eb3308c..e500323cd5d2 100644 --- a/crates/microverse_shell/src/batch.rs +++ b/crates/microverse_shell/src/batch.rs @@ -52,6 +52,7 @@ //! or not anyone expects it to move. use bytemuck::{Pod, Zeroable}; +use microverse_core::budget::{BudgetKind, BudgetReport, Budgeted}; use microverse_core::{DrawCmd, Handle, Material, Mesh}; /// One instance, as the vertex shader reads it: a column-major model matrix. @@ -113,13 +114,21 @@ pub struct Batcher { instances: Vec, batches: Vec, capacity: usize, + /// What the instance buffer calls itself in a budget report. + name: &'static str, + /// High-water instance count, so the renderer's budget is measured rather + /// than guessed like every other budget in the engine. + peak: usize, dropped: u32, unresolved: u32, } impl Batcher { /// Reserve room for `capacity` instances. Startup only. - pub fn with_capacity(capacity: usize) -> Self { + /// + /// `name` is what this appears as in a budget report, for the same reason + /// every storage primitive in the core takes one. + pub fn with_capacity(name: &'static str, capacity: usize) -> Self { Self { keyed: Vec::with_capacity(capacity), instances: Vec::with_capacity(capacity), @@ -129,6 +138,8 @@ impl Batcher { // it means the pathological frame is slow and not also allocating. batches: Vec::with_capacity(capacity), capacity, + name, + peak: 0, dropped: 0, unresolved: 0, } @@ -184,6 +195,10 @@ impl Batcher { } self.instances.push(Instance { model: cmd.xform }); } + + if self.instances.len() > self.peak { + self.peak = self.instances.len(); + } } /// This frame's instance data, in the order the batches index it. @@ -201,12 +216,6 @@ impl Batcher { self.instances.len() as u32 } - /// Instances refused for want of room in the instance buffer, since the - /// last [`Self::reset_stats`]. - pub fn dropped(&self) -> u32 { - self.dropped - } - /// Commands whose mesh or material named nothing, since the last /// [`Self::reset_stats`]. pub fn unresolved(&self) -> u32 { @@ -221,6 +230,31 @@ impl Batcher { pub fn reset_stats(&mut self) { self.dropped = 0; self.unresolved = 0; + self.peak = self.instances.len(); + } + + /// What the instance buffer is holding, for a budget report. + /// + /// `unresolved` is deliberately not the overflow count. A command whose + /// mesh names nothing is a bad handle, not a full buffer — it would still + /// have been dropped in a buffer with room to spare — and folding it in + /// would make a budget look too small when the actual fault is upstream of + /// any capacity. It keeps its own counter and its own warning line. + pub fn budget(&self) -> BudgetReport { + BudgetReport::from_usize_counts( + self.name, + self.instances.len(), + self.capacity, + self.peak, + self.dropped, + BudgetKind::Items, + ) + } +} + +impl Budgeted for Batcher { + fn budgets(&self, visit: &mut dyn FnMut(BudgetReport)) { + visit(self.budget()); } } @@ -234,6 +268,8 @@ mod tests { use super::*; use core::num::NonZeroU32; + use microverse_core::budget::BudgetKind; + const G: NonZeroU32 = NonZeroU32::MIN; fn cmd(mesh: u32, material: u32) -> DrawCmd { @@ -253,14 +289,14 @@ mod tests { fn a_shuffled_frame_comes_out_as_one_draw_per_distinct_pair() { // The card's done-condition, in the form a test can hold: several // hundred instances of a few meshes in a handful of draw calls. - let mut batcher = Batcher::with_capacity(4096); + let mut batcher = Batcher::with_capacity("instances", 4096); let cmds: Vec = (0..441).map(|n| cmd(n % 3, (n / 3) % 3)).collect(); batcher.plan(&cmds, everything_resolves); assert_eq!(batcher.batches().len(), 9, "3 meshes x 3 materials"); assert_eq!(batcher.instance_count(), 441); - assert_eq!(batcher.dropped(), 0); + assert_eq!(batcher.budget().overflows, 0); assert_eq!(batcher.unresolved(), 0); } @@ -269,7 +305,7 @@ mod tests { // A batching bug that loses or repeats instances still renders a // plausible picture, so counting is not enough: the batches have to // tile the instance buffer exactly. - let mut batcher = Batcher::with_capacity(4096); + let mut batcher = Batcher::with_capacity("instances", 4096); let cmds: Vec = (0..200).map(|n| cmd(n % 4, (n / 4) % 5)).collect(); batcher.plan(&cmds, everything_resolves); @@ -294,7 +330,7 @@ mod tests { // Which way round the key packs is invisible in a draw-call count and // very visible in a frame capture. Material above mesh means all of one // material's meshes are drawn before the next material is bound. - let mut batcher = Batcher::with_capacity(64); + let mut batcher = Batcher::with_capacity("instances", 64); let cmds = [cmd(1, 1), cmd(0, 0), cmd(1, 0), cmd(0, 1)]; batcher.plan(&cmds, everything_resolves); @@ -309,8 +345,8 @@ mod tests { let forwards: Vec = (0..120).map(|n| cmd(n % 3, (n / 3) % 3)).collect(); let backwards: Vec = forwards.iter().rev().copied().collect(); - let mut a = Batcher::with_capacity(4096); - let mut b = Batcher::with_capacity(4096); + let mut a = Batcher::with_capacity("instances", 4096); + let mut b = Batcher::with_capacity("instances", 4096); a.plan(&forwards, everything_resolves); b.plan(&backwards, everything_resolves); @@ -319,13 +355,13 @@ mod tests { #[test] fn a_full_instance_buffer_drops_and_counts_rather_than_growing() { - let mut batcher = Batcher::with_capacity(10); + let mut batcher = Batcher::with_capacity("instances", 10); let cmds: Vec = (0..25).map(|n| cmd(n % 3, 0)).collect(); batcher.plan(&cmds, everything_resolves); assert_eq!(batcher.instance_count(), 10); - assert_eq!(batcher.dropped(), 15); + assert_eq!(batcher.budget().overflows, 15); assert_eq!(batcher.capacity(), 10, "capacity must never grow"); // The staging vector is what feeds `write_buffer`; if it grew past the // GPU buffer's size the upload would be the thing that failed, some @@ -333,24 +369,67 @@ mod tests { assert!(batcher.instances().len() <= batcher.capacity()); } + /// The instance buffer joins the same enumeration the core's storage does, + /// which is the point of the trait living in the core: the shell holds a + /// budget the core has never heard of, and a consumer reading budgets + /// cannot tell which side of the boundary any of them came from. + #[test] + fn the_instance_buffer_reports_itself_as_a_budget() { + let mut batcher = Batcher::with_capacity("instances", 64); + let cmds: Vec = (0..40).map(|n| cmd(n % 3, 0)).collect(); + batcher.plan(&cmds, everything_resolves); + + let mut seen = 0; + batcher.budgets(&mut |b| { + assert_eq!(b.name, "instances"); + assert_eq!(b.used, 40); + assert_eq!(b.cap, 64); + assert_eq!(b.peak, 40); + assert_eq!(b.overflows, 0); + assert_eq!(b.kind, BudgetKind::Items); + seen += 1; + }); + assert_eq!(seen, 1, "one buffer, one budget"); + } + + /// The high-water mark has to outlive the frame that set it, or it is + /// measuring the last frame rather than the worst one — and the worst one + /// is the number a budget gets set from. + #[test] + fn the_instance_peak_survives_a_lighter_frame() { + let mut batcher = Batcher::with_capacity("instances", 64); + + let busy: Vec = (0..50).map(|n| cmd(n % 3, 0)).collect(); + batcher.plan(&busy, everything_resolves); + assert_eq!(batcher.budget().peak, 50); + + let quiet: Vec = (0..2).map(|n| cmd(n % 3, 0)).collect(); + batcher.plan(&quiet, everything_resolves); + assert_eq!(batcher.budget().used, 2, "this frame"); + assert_eq!(batcher.budget().peak, 50, "and the worst one so far"); + + batcher.reset_stats(); + assert_eq!(batcher.budget().peak, 2, "restarted from what is in hand"); + } + #[test] fn a_handle_that_names_nothing_is_dropped_and_counted_separately() { // Separately from a capacity drop, because they send you to different // places: one means the budget is too small, the other means something // handed the core a handle to a mesh that is not loaded. - let mut batcher = Batcher::with_capacity(64); + let mut batcher = Batcher::with_capacity("instances", 64); let cmds = [cmd(0, 0), cmd(9, 0), cmd(1, 0), cmd(0, 9)]; batcher.plan(&cmds, |c| c.mesh.index() < 3 && c.material.index() < 3); assert_eq!(batcher.instance_count(), 2); assert_eq!(batcher.unresolved(), 2); - assert_eq!(batcher.dropped(), 0, "a dangling handle is not a budget problem"); + assert_eq!(batcher.budget().overflows, 0, "a dangling handle is not a budget problem"); } #[test] fn a_frame_with_nothing_in_it_produces_no_draw_calls() { - let mut batcher = Batcher::with_capacity(64); + let mut batcher = Batcher::with_capacity("instances", 64); batcher.plan(&[], everything_resolves); assert!(batcher.batches().is_empty()); assert_eq!(batcher.instance_count(), 0); @@ -358,7 +437,7 @@ mod tests { #[test] fn planning_again_forgets_the_previous_frame_but_not_the_counters() { - let mut batcher = Batcher::with_capacity(4); + let mut batcher = Batcher::with_capacity("instances", 4); let many: Vec = (0..9).map(|n| cmd(n % 3, 0)).collect(); batcher.plan(&many, everything_resolves); @@ -366,10 +445,10 @@ mod tests { assert_eq!(batcher.batches().len(), 1, "last frame's batches are gone"); assert_eq!(batcher.instance_count(), 1); - assert_eq!(batcher.dropped(), 5, "but what it lost is still reportable"); + assert_eq!(batcher.budget().overflows, 5, "but what it lost is still reportable"); batcher.reset_stats(); - assert_eq!(batcher.dropped(), 0); + assert_eq!(batcher.budget().overflows, 0); assert_eq!(batcher.unresolved(), 0); } diff --git a/crates/microverse_shell/src/main.rs b/crates/microverse_shell/src/main.rs index 8c1161ee6188..368411a0d22f 100644 --- a/crates/microverse_shell/src/main.rs +++ b/crates/microverse_shell/src/main.rs @@ -19,6 +19,7 @@ mod window; use std::error::Error; use std::process::ExitCode; +use microverse_core::budget::{self, Budgeted}; use microverse_core::{Arena, DrawList, Input, Key, World}; use crate::clock::Clock; @@ -104,7 +105,7 @@ fn run() -> Result<(), Box> { let mut window = Window::open(WINDOW_TITLE, WINDOW_SIZE.0, WINDOW_SIZE.1)?; let mut input = Input::new(); - let mut draws = DrawList::with_capacity(draw_budget()); + let mut draws = DrawList::with_capacity("draws", draw_budget()); // The two arenas, in the order their lifetimes nest. `level` is declared // before `world` because `world` borrows it, and the shell owns both so @@ -214,52 +215,51 @@ fn run() -> Result<(), Box> { let stats = renderer.stats(); let axis = input.move_axis(); println!( - "frame {frame}: {:.0} fps dt {:.2}ms draws {}/{} (peak {}) \ - {} instances in {} calls arena L {}/{} F {}/{} (peak {}) \ + "frame {frame}: {:.0} fps dt {:.2}ms {} instances in {} calls \ move [{:+.2} {:+.2}] {}x{}", frames_since_report as f32 / secs_since_report, dt * 1000.0, - draws.len(), - draws.capacity(), - draws.peak(), stats.instances, stats.draw_calls, - level.len(), - level.capacity(), - scratch.len(), - scratch.capacity(), - scratch.peak(), axis[0], axis[1], viewport.0, viewport.1, ); + // Every budget, enumerated rather than named. This is the one site + // that knows what storage exists, and it is a slice literal, so + // adding a pool is one word here and a line in the output — not an + // edit to a format string, a warning branch and a `reset_stats` + // call, which is what it used to cost and is how a budget ends up + // added but never reported. + // + // `world` stands in for the level arena it owns: the shell does not + // name the world's storage, and the day the world holds an actor + // pool this line does not change. + let budgeted: &[&dyn Budgeted] = &[&world, &scratch, &draws, &renderer]; + print!("frame {frame}: budgets"); + budget::visit_all(budgeted, &mut |b| { + print!(" {} {}/{}{} (peak {})", b.name, b.used, b.cap, b.kind.unit(), b.peak); + }); + println!(); + // Overflow gets its own line rather than a column, because a column // that reads zero every second for a month is a column nobody sees - // any more. These three are all "the frame did less than it was - // asked to", and each names a different place to go look. - if draws.dropped() > 0 || stats.dropped > 0 || stats.unresolved > 0 { - println!( - "frame {frame}: warning: dropped {} draw commands (buffer full), \ - {} instances (instance buffer full), {} unresolved handles", - draws.dropped(), - stats.dropped, - stats.unresolved, - ); - } - - // An arena that refused is a different failure from a buffer that - // dropped: nothing was lost or truncated, an allocation simply did - // not happen and the caller took its other path. Named separately - // so the two are not read as the same event. - if level.overflows() > 0 || scratch.overflows() > 0 { - println!( - "frame {frame}: warning: {} level and {} frame allocations refused \ - (arena full)", - level.overflows(), - scratch.overflows(), - ); + // any more. Driven off the same enumeration, so a budget cannot be + // reported in one place and forgotten in the other. + budget::visit_all(budgeted, &mut |b| { + if b.overflowed() { + println!("frame {frame}: warning: {} refused {} times", b.name, b.overflows); + } + }); + + // Not a budget, and kept separate on purpose: a command whose mesh + // or material named nothing would have been dropped by a buffer + // with room to spare, so reading it as capacity pressure would send + // whoever is looking to the wrong place entirely. + if stats.unresolved > 0 { + println!("frame {frame}: warning: {} unresolved handles", stats.unresolved); } // Cleared so the next line reads "since the last report" rather diff --git a/crates/microverse_shell/src/render.rs b/crates/microverse_shell/src/render.rs index 76a29fd84d57..9e178c0af6b5 100644 --- a/crates/microverse_shell/src/render.rs +++ b/crates/microverse_shell/src/render.rs @@ -60,6 +60,8 @@ use std::fmt; use bytemuck::{Pod, Zeroable}; use microverse_core::{DrawCmd, Handle, Material}; +use microverse_core::budget::{BudgetReport, Budgeted}; + use crate::batch::{Batcher, Instance}; use crate::camera; use crate::material::MaterialRegistry; @@ -151,9 +153,14 @@ pub struct RenderStats { pub draw_calls: u32, /// Instances those calls drew between them. pub instances: u32, - /// Instances refused for want of room in the instance buffer, cumulative. - pub dropped: u32, /// Commands whose mesh or material named nothing, cumulative. + /// + /// Not a capacity failure, which is why it is here and not in the budget + /// report: a command naming a mesh that does not exist would have been + /// dropped by an instance buffer with room to spare. Instances refused for + /// *want of room* are the instance buffer's budget and are reported through + /// [`Budgeted`], so that this struct is not a second copy of a number the + /// enumeration already carries. pub unresolved: u32, } @@ -454,7 +461,7 @@ impl Renderer { uniform, bind_group, instances, - batcher: Batcher::with_capacity(max_instances), + batcher: Batcher::with_capacity("instances", max_instances), meshes, materials, stats: RenderStats::default(), @@ -659,7 +666,6 @@ impl Renderer { self.stats = RenderStats { draw_calls, instances: self.batcher.instance_count(), - dropped: self.batcher.dropped(), unresolved: self.batcher.unresolved(), }; @@ -826,6 +832,20 @@ fn create_depth(device: &wgpu::Device, width: u32, height: u32) -> wgpu::Texture texture.create_view(&wgpu::TextureViewDescriptor::default()) } +impl Budgeted for Renderer { + /// The instance buffer, which is a **renderer** budget. + /// + /// It is in the same enumeration as the world's storage and it must never + /// be read as the same kind of thing: raising it lets a frame draw more, + /// and changes nothing about what the game permits. See "World limits are + /// not implementation limits" in `ARCHITECTURE.md`. What the shared + /// enumeration buys is that a reader sees both without the shell having to + /// format either. + fn budgets(&self, visit: &mut dyn FnMut(BudgetReport)) { + self.batcher.budgets(visit); + } +} + #[cfg(test)] mod tests { use super::*;