New block evaluation Budget system replacing depth.

The old idea of just capping recursion depth was too weak, because
it's easy to make a very expensive block without having very much
recursion. So, instead, we have counters that are only decremented
(while still also having a recursion guard).

By putting it inside the `EvalFilter`, we avoid having multiple
recursively passed parameters, and allow the original caller to both
choose the budget and find out how much of it was used.

Future work:
Expose evaluation costs in `EvaluatedBlock` for diagnostic purposes;
specifically so users can avoid running into them unexpectedly, and
so we can write tests of this new logic.

all-is-cubes/src/block.rs

@@ -9,6 +9,7 @@ use alloc::boxed::Box;
 use alloc::collections::VecDeque;
 use alloc::sync::Arc;
 use alloc::vec::Vec;
+use core::cell::Cell;
 use core::fmt;
 
 use crate::listen::{self, Listen, Listener};
@@ -421,6 +422,7 @@ impl Block {
         self.evaluate2(&EvalFilter {
             skip_eval: false,
             listener: None,
+            budget: Default::default(),
         })
     }
 
@@ -444,6 +446,7 @@ impl Block {
         self.evaluate2(&EvalFilter {
             skip_eval: false,
             listener: Some(listener.erased()),
+            budget: Default::default(),
         })
     }
 
@@ -452,11 +455,13 @@ impl Block {
     /// TODO: Placeholder name. At some point we may expose `EvalFilter` directly and make
     /// this be just `evaluate()`.
     pub(crate) fn evaluate2(&self, filter: &EvalFilter) -> Result<EvaluatedBlock, EvalBlockError> {
-        Ok(EvaluatedBlock::from(self.evaluate_impl(0, filter)?))
+        Ok(EvaluatedBlock::from(self.evaluate_impl(filter)?))
     }
 
     #[inline]
-    fn evaluate_impl(&self, depth: u8, filter: &EvalFilter) -> Result<MinEval, EvalBlockError> {
+    fn evaluate_impl(&self, filter: &EvalFilter) -> Result<MinEval, EvalBlockError> {
+        Budget::decrement_components(&filter.budget)?;
+
         let mut value: MinEval = match *self.primitive() {
             Primitive::Indirect(ref def_ref) => def_ref.read()?.evaluate_impl(filter)?,
 
@@ -520,17 +525,24 @@ impl Block {
                     .intersection(block_space.bounds())
                     .filter(|_| !filter.skip_eval)
                 {
-                    Some(occupied_bounds) => block_space
-                        .extract(
-                            occupied_bounds,
-                            #[inline(always)]
-                            |extract| {
-                                let ev = extract.block_data().evaluated();
-                                voxels_animation_hint |= ev.attributes.animation_hint;
-                                Evoxel::from_block(ev)
-                            },
-                        )
-                        .translate(-offset.to_vector()),
+                    Some(occupied_bounds) => {
+                        Budget::decrement_voxels(
+                            &filter.budget,
+                            occupied_bounds.volume().unwrap(),
+                        )?;
+
+                        block_space
+                            .extract(
+                                occupied_bounds,
+                                #[inline(always)]
+                                |extract| {
+                                    let ev = extract.block_data().evaluated();
+                                    voxels_animation_hint |= ev.attributes.animation_hint;
+                                    Evoxel::from_block(ev)
+                                },
+                            )
+                            .translate(-offset.to_vector())
+                    }
                     None => {
                         // If there is no intersection, then return an empty voxel array,
                         // with an arbitrary position.
@@ -556,15 +568,14 @@ impl Block {
                 }
             }
 
-            Primitive::Text { ref text, offset } => text.evaluate(offset, depth, filter)?,
+            Primitive::Text { ref text, offset } => text.evaluate(offset, filter)?,
         };
 
         #[cfg(debug_assertions)]
         value.consistency_check();
 
         for (index, modifier) in self.modifiers().iter().enumerate() {
-            // TODO: Extend recursion depth model to catch stacking up lots of modifiers
-            value = modifier.evaluate(self, index, value, depth, filter)?;
+            value = modifier.evaluate(self, index, value, filter)?;
 
             #[cfg(debug_assertions)]
             value.consistency_check();
@@ -607,6 +618,13 @@ pub(crate) struct EvalFilter {
     /// such as the [`Space`] referred to by a [`Primitive::Recur`] or the [`BlockDef`]
     /// referred to by a [`Primitive::Indirect`].
     pub listener: Option<listen::DynListener<BlockChange>>,
+
+    /// How much computation may be spent on performing the evaluation.
+    ///
+    /// If the budget is exhausted, evaluation returns [`EvalBlockError::StackOverflow`].
+    ///
+    /// Outside of special circumstances, use [`Budget::default()`] here.
+    pub budget: Cell<Budget>,
 }
 
 impl Default for EvalFilter {
@@ -616,19 +634,11 @@ impl Default for EvalFilter {
         Self {
             skip_eval: Default::default(),
             listener: Default::default(),
+            budget: Default::default(),
         }
     }
 }
 
-/// Recursion limiter helper for evaluate.
-fn next_depth(depth: u8) -> Result<u8, EvalBlockError> {
-    if depth > 32 {
-        Err(EvalBlockError::StackOverflow)
-    } else {
-        Ok(depth + 1)
-    }
-}
-
 // Manual implementations of Eq and Hash ensure that the [`BlockPtr`] storage
 // choices do not affect equality.
 impl PartialEq for Block {
@@ -941,6 +951,100 @@ mod conversions_for_indirect {
     }
 }
 
+/// Computation budget for block evaluations.
+///
+/// This is used inside an [`EvalFilter`].
+///
+/// In principle, what we want is a time budget, but in order to offer determinism and
+/// comprehensibility, it is instead made up of multiple discrete quantities.
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub(crate) struct Budget {
+    /// Number of [`Primitive`]s and [`Modifier`]s.
+    components: u32,
+
+    /// Number of individual voxels produced (e.g. by a [`Primitive::Recur`]) or altered
+    /// (e.g. by a [`Modifier::Composite`]).
+    voxels: u32,
+
+    /// Number of levels of evaluation recursion permitted.
+    ///
+    /// Recursion occurs when a primitive or modifier which itself contains a [`Block`] is
+    /// evaluated; currently, these are [`Primitive::Text`] and [`Modifier::Composite`].
+    ///
+    /// This must be set low enough to avoid Rust stack overflows which cannot be recovered from.
+    /// Unlike the other budget parameters, this is not cumulative over the entire evaluation.
+    recursion: u8,
+
+    /// Number of recursion levels actually used by the evaluation.
+    /// This is tracked separately so that it can be reported afterward,
+    /// whereas the other counters are only ever decremented and so a subtraction suffices.
+    recursion_used: u8,
+}
+
+impl Budget {
+    pub(crate) fn decrement_components(cell: &Cell<Budget>) -> Result<(), EvalBlockError> {
+        let mut budget = cell.get();
+        match budget.components.checked_sub(1) {
+            Some(updated) => budget.components = updated,
+            None => return Err(EvalBlockError::StackOverflow),
+        }
+        cell.set(budget);
+        Ok(())
+    }
+
+    pub(crate) fn decrement_voxels(
+        cell: &Cell<Budget>,
+        amount: usize,
+    ) -> Result<(), EvalBlockError> {
+        let mut budget = cell.get();
+        match u32::try_from(amount)
+            .ok()
+            .and_then(|amount| budget.voxels.checked_sub(amount))
+        {
+            Some(updated) => budget.voxels = updated,
+            None => return Err(EvalBlockError::StackOverflow),
+        }
+        cell.set(budget);
+        Ok(())
+    }
+
+    pub(crate) fn recurse(cell: &Cell<Budget>) -> Result<BudgetRecurseGuard<'_>, EvalBlockError> {
+        let current = cell.get();
+        let mut recursed = current;
+        match recursed.recursion.checked_sub(1) {
+            Some(updated) => recursed.recursion = updated,
+            None => return Err(EvalBlockError::StackOverflow),
+        }
+        cell.set(recursed);
+        Ok(BudgetRecurseGuard { cell })
+    }
+}
+
+impl Default for Budget {
+    /// Returns the standard budget for starting any evaluation.
+    fn default() -> Self {
+        Self {
+            components: 1000,
+            voxels: 64 * 64 * 128,
+            recursion: 30,
+            recursion_used: 0,
+        }
+    }
+}
+
+#[must_use]
+pub(crate) struct BudgetRecurseGuard<'a> {
+    cell: &'a Cell<Budget>,
+}
+
+impl Drop for BudgetRecurseGuard<'_> {
+    fn drop(&mut self) {
+        let mut budget = self.cell.get();
+        budget.recursion = budget.recursion.checked_add(1).unwrap();
+        self.cell.set(budget);
+    }
+}
+
 /// Notification when an [`EvaluatedBlock`] result changes.
 #[derive(Clone, Debug, Eq, Hash, PartialEq)]
 #[non_exhaustive]

all-is-cubes/src/block/block_def.rs

@@ -99,6 +99,7 @@ impl BlockDef {
         let &block::EvalFilter {
             skip_eval,
             ref listener,
+            budget: _, // already accounted in the caller
         } = filter;
 
         if let Some(listener) = listener {
@@ -132,6 +133,7 @@ impl BlockDefState {
             .evaluate2(&block::EvalFilter {
                 skip_eval: false,
                 listener: Some(block_listener.erased()),
+                budget: Default::default(),
             })
             .map(MinEval::from);
 
@@ -165,6 +167,7 @@ impl BlockDefState {
                 .evaluate2(&block::EvalFilter {
                     skip_eval: false,
                     listener: None, // we already have a listener installed
+                    budget: Default::default(),
                 })
                 .map(MinEval::from);
 

all-is-cubes/src/block/evaluated.rs

@@ -602,6 +602,14 @@ impl Evoxels {
         }
     }
 
+    /// Returns the count of voxels, aka [`Vol::volume()`] at the resolution.
+    pub fn count(&self) -> usize {
+        match self {
+            Evoxels::One(_) => 1,
+            Evoxels::Many(_, voxels) => voxels.volume(),
+        }
+    }
+
     /// If this has a resolution of 1, then return that single voxel.
     #[inline]
     pub fn single_voxel(&self) -> Option<Evoxel> {

all-is-cubes/src/block/modifier.rs

@@ -79,20 +79,23 @@ impl Modifier {
     /// * `this_modifier_index` is the index in `block.modifiers()` of `self`.
     /// * `value` is the output of the preceding modifier or primitive, which is what the
     ///   current modifier should be applied to.
-    /// * `depth` is the current block evaluation recursion depth (which is *not*
-    ///   incremented by modifiers; TODO: define a computation limit strategy).
+    /// * `filter` controls evaluation options and listening, and its budget is decremented by
+    ///   1 component (the modifier itself) plus as many voxels and additional components as the
+    ///   particular modifier needs to calculate.
     pub(crate) fn evaluate(
         &self,
         block: &Block,
         this_modifier_index: usize,
         mut value: MinEval,
-        depth: u8,
         filter: &block::EvalFilter,
     ) -> Result<MinEval, block::EvalBlockError> {
+        block::Budget::decrement_components(&filter.budget)?;
+
         Ok(match *self {
             Modifier::Quote(Quote { suppress_ambient }) => {
                 value.attributes.tick_action = None;
                 if suppress_ambient {
+                    block::Budget::decrement_voxels(&filter.budget, value.voxels.count())?;
                     for voxel in value.voxels.as_vol_mut().as_linear_mut().iter_mut() {
                         voxel.emission = Rgb::ZERO;
                     }
@@ -105,6 +108,7 @@ impl Modifier {
                     // Skip computation of transforms
                     value
                 } else {
+                    block::Budget::decrement_voxels(&filter.budget, value.voxels.count())?;
                     // TODO: Add a shuffle-in-place rotation operation to Vol and try implementing this using that, which should have less arithmetic involved than these matrix ops
                     let resolution = value.resolution();
                     let inner_to_outer = rotation.to_positive_octant_transform(resolution.into());
@@ -130,13 +134,11 @@ impl Modifier {
                 }
             }
 
-            Modifier::Composite(ref c) => c.evaluate(value, depth, filter)?,
+            Modifier::Composite(ref c) => c.evaluate(value, filter)?,
 
-            Modifier::Zoom(ref z) => z.evaluate(value)?,
+            Modifier::Zoom(ref z) => z.evaluate(value, filter)?,
 
-            Modifier::Move(ref m) => {
-                m.evaluate(block, this_modifier_index, value, depth, filter)?
-            }
+            Modifier::Move(ref m) => m.evaluate(block, this_modifier_index, value, filter)?,
         })
     }
 

all-is-cubes/src/block/modifier/composite.rs

@@ -118,7 +118,6 @@ impl Composite {
     pub(super) fn evaluate(
         &self,
         mut dst_evaluated: MinEval,
-        depth: u8,
         filter: &block::EvalFilter,
     ) -> Result<MinEval, block::EvalBlockError> {
         let Composite {
@@ -130,7 +129,10 @@ impl Composite {
 
         // The destination block is already evaluated (it is the input to this
         // modifier), but we need to evaluate the source block.
-        let mut src_evaluated = source.evaluate_impl(block::next_depth(depth)?, filter)?;
+        let mut src_evaluated = {
+            let _recursion_scope = block::Budget::recurse(&filter.budget)?;
+            source.evaluate_impl(filter)?
+        };
 
         if filter.skip_eval {
             return Ok(dst_evaluated);
@@ -163,6 +165,8 @@ impl Composite {
 
         let output_bounds = operator.bounds(src_bounds_scaled, dst_bounds_scaled);
 
+        block::Budget::decrement_voxels(&filter.budget, output_bounds.volume().unwrap())?;
+
         let attributes = block::BlockAttributes {
             display_name: dst_att.display_name, // TODO merge
             selectable: src_att.selectable | dst_att.selectable,

all-is-cubes/src/block/modifier/move.rs

@@ -68,7 +68,6 @@ impl Move {
         block: &Block,
         this_modifier_index: usize,
         mut input: MinEval,
-        depth: u8,
         filter: &block::EvalFilter,
     ) -> Result<MinEval, block::EvalBlockError> {
         let Move {
@@ -81,11 +80,11 @@ impl Move {
         // don't interfere with movement or cause duplication.
         // (In the future we may want a more nuanced policy that allows internal changes,
         // but that will probably involve refining tick_action processing.)
+        // TODO: This can be done more cleanly by having an evaluate function on Quote itself.
         input = Modifier::from(block::Quote::default()).evaluate(
             block,
             this_modifier_index,
             input,
-            depth,
             filter,
         )?;
 
@@ -148,6 +147,11 @@ impl Move {
 
         Ok(match displaced_bounds {
             Some(displaced_bounds) => {
+                block::Budget::decrement_voxels(
+                    &filter.budget,
+                    displaced_bounds.volume().unwrap(),
+                )?;
+
                 let displaced_voxels = match &input.voxels {
                     Evoxels::Many(_, voxels) => Evoxels::Many(
                         effective_resolution,