diff --git a/all-is-cubes/src/block.rs b/all-is-cubes/src/block.rs index db761dc92..e5d0d86a8 100644 --- a/all-is-cubes/src/block.rs +++ b/all-is-cubes/src/block.rs @@ -457,24 +457,20 @@ 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 { - match self.evaluate_impl(filter) { - Ok(ev) => Ok(ev.into()), - Err(err) => Err(err.into_eval_error()), - } + finish_evaluation(filter.budget.get(), self.evaluate_impl(filter), filter) } /// Equivalent to `Evoxel::from_block(block.evaluate2(filter))` except for the error type. /// For use when blocks contain other blocks as voxels. fn evaluate_to_evoxel_internal(&self, filter: &EvalFilter) -> Result { - match self.evaluate_impl(filter) { - // TODO: Make this more efficient by not building the full `EvaluatedBlock` - Ok(ev) => Ok(Evoxel::from_block(&ev.into())), - Err(err) => Err(err), - } + // TODO: Make this more efficient by not building the full `EvaluatedBlock` + self.evaluate_impl(filter) + .map(|minev| Evoxel::from_block(&minev.finish(Cost::ZERO /* ignored */))) } #[inline] fn evaluate_impl(&self, filter: &EvalFilter) -> Result { + // The block's primitive counts as 1 component. Budget::decrement_components(&filter.budget)?; let mut value: MinEval = match *self.primitive() { diff --git a/all-is-cubes/src/block/block_def.rs b/all-is-cubes/src/block/block_def.rs index 2929aeb67..b6cec1b9c 100644 --- a/all-is-cubes/src/block/block_def.rs +++ b/all-is-cubes/src/block/block_def.rs @@ -87,10 +87,18 @@ impl BlockDef { /// This returns the same success or error as `Block::from(ref_to_self).evaluate()` would, not /// the same as `.block().evaluate()` would. pub fn evaluate(&self) -> Result { - match self.evaluate_impl(&block::EvalFilter::default()) { - Ok(t) => Ok(block::EvaluatedBlock::from(t)), - Err(e) => Err(e.into_eval_error()), - } + let filter = block::EvalFilter::default(); + block::finish_evaluation( + filter.budget.get(), + { + // This decrement makes the cost consistent with evaluating a + // block with Primitive::Indirect. + block::Budget::decrement_components(&filter.budget).unwrap(); + + self.evaluate_impl(&filter) + }, + &filter, + ) } /// Implementation of block evaluation used by a [`Primitive::Indirect`] pointing to this. @@ -460,10 +468,10 @@ impl manyfmt::Fmt for BlockDefStepInfo { #[cfg(test)] mod tests { + use super::*; use crate::math::Rgba; use crate::universe::Universe; - - use super::*; + use pretty_assertions::assert_eq; /// Quick more-than-nothing test for [`BlockDef::evaluate()`] being the same as more usual /// options. @@ -476,13 +484,23 @@ mod tests { .color(Rgba::new(1.0, 0.0, 0.0, 1.0)) .build(); - let eval_bare = block.evaluate(); + let eval_bare = block.evaluate().unwrap(); let block_def = BlockDef::new(block); - let eval_def = block_def.evaluate(); + let eval_def = block_def.evaluate().unwrap(); let block_def_ref = universe.insert_anonymous(block_def); - let eval_indirect = Block::from(block_def_ref).evaluate(); - - assert_eq!(eval_bare, eval_def); - assert_eq!(eval_bare, eval_indirect); + let eval_indirect = Block::from(block_def_ref).evaluate().unwrap(); + + assert_eq!( + eval_def, eval_indirect, + "BlockDef::evaluate() same as Primitive::Indirect" + ); + assert_eq!( + block::EvaluatedBlock { + cost: eval_bare.cost, + ..eval_def + }, + eval_bare, + "BlockDef::evaluate() same except for cost as the def block" + ); } } diff --git a/all-is-cubes/src/block/evaluated.rs b/all-is-cubes/src/block/evaluated.rs index db14c83b4..87a864ab2 100644 --- a/all-is-cubes/src/block/evaluated.rs +++ b/all-is-cubes/src/block/evaluated.rs @@ -7,7 +7,7 @@ use euclid::Vector3D; use ordered_float::NotNan; use crate::block::{ - self, BlockAttributes, BlockCollision, + self, BlockAttributes, BlockCollision, Cost, Resolution::{self, R1}, }; use crate::math::{ @@ -79,6 +79,9 @@ pub struct EvaluatedBlock { /// It doesn't harm normal operation because the point of having this is to compare /// block shapes, which is trivial if the block is invisible.) pub voxel_opacity_mask: Option>>, + + /// Cost of performing the evaluation. + pub(crate) cost: Cost, } impl fmt::Debug for EvaluatedBlock { @@ -93,6 +96,7 @@ impl fmt::Debug for EvaluatedBlock { visible, uniform_collision, voxel_opacity_mask, + cost, } = self; let mut ds = fmt.debug_struct("EvaluatedBlock"); if *attributes != BlockAttributes::default() { @@ -123,6 +127,7 @@ impl fmt::Debug for EvaluatedBlock { "voxel_opacity_mask", &format_args!("{:?}", voxel_opacity_mask.as_ref().map(Vol::bounds)), ); + ds.field("cost", cost); ds.finish() } } @@ -132,8 +137,12 @@ impl EvaluatedBlock { /// Computes the derived values of a voxel block. /// - /// This is also available as `impl From for EvaluatedBlock`. - pub(crate) fn from_voxels(attributes: BlockAttributes, voxels: Evoxels) -> EvaluatedBlock { + /// This is also available as [`MinEval::finish()`]. + pub(crate) fn from_voxels( + attributes: BlockAttributes, + voxels: Evoxels, + cost: Cost, + ) -> EvaluatedBlock { // Optimization for single voxels: // don't allocate any `Vol`s or perform any generalized scans. if let Some(Evoxel { @@ -163,6 +172,7 @@ impl EvaluatedBlock { } else { Some(Vol::from_element(color.opacity_category())) }, + cost, }; } @@ -277,6 +287,7 @@ impl EvaluatedBlock { uniform_collision, voxel_opacity_mask, voxels, + cost, } } @@ -340,7 +351,8 @@ impl EvaluatedBlock { #[doc(hidden)] #[track_caller] pub fn consistency_check(&self) { - let regenerated = EvaluatedBlock::from_voxels(self.attributes.clone(), self.voxels.clone()); + let regenerated = + EvaluatedBlock::from_voxels(self.attributes.clone(), self.voxels.clone(), self.cost); assert_eq!(self, ®enerated); } } @@ -668,6 +680,11 @@ pub const AIR_EVALUATED: EvaluatedBlock = EvaluatedBlock { visible: false, uniform_collision: Some(BlockCollision::None), voxel_opacity_mask: None, + cost: Cost { + components: 1, + voxels: 0, + recursion: 0, + }, }; /// This separate item is needed to convince the compiler that `AIR_ATTRIBUTES.display_name` @@ -701,14 +718,6 @@ pub(crate) struct MinEval { pub(crate) voxels: Evoxels, } -impl From for EvaluatedBlock { - fn from(value: MinEval) -> Self { - let MinEval { attributes, voxels } = value; - // TODO: EvaluatedBlock::from* should probably be entirely replaced with this - EvaluatedBlock::from_voxels(attributes, voxels) - } -} - impl From<&EvaluatedBlock> for MinEval { fn from(value: &EvaluatedBlock) -> Self { Self { @@ -727,6 +736,13 @@ impl From for MinEval { } impl MinEval { + /// Converts this into an [`EvaluatedBlock`], computing the derived values. + pub(crate) fn finish(self, cost: Cost) -> EvaluatedBlock { + let MinEval { attributes, voxels } = self; + // TODO: EvaluatedBlock::from* should probably be entirely replaced with this + EvaluatedBlock::from_voxels(attributes, voxels, cost) + } + pub fn resolution(&self) -> Resolution { self.voxels.resolution() } @@ -774,6 +790,11 @@ mod tests { collision: Hard, }, voxel_opacity_mask: Some(GridAab(0..1, 0..1, 0..1)), + cost: Cost { + components: 1, + voxels: 0, + recursion: 0, + }, } "} ); @@ -815,6 +836,11 @@ mod tests { resolution: 2, voxels: GridAab(0..2, 0..2, 0..2), voxel_opacity_mask: Some(GridAab(0..2, 0..2, 0..2)), + cost: Cost { + components: 1, + voxels: 8, + recursion: 0, + }, } "} ); @@ -854,7 +880,8 @@ mod tests { assert_eq!( EvaluatedBlock::from_voxels( attributes.clone(), - Evoxels::Many(resolution, Vol::from_fn(bounds, |_| unreachable!())) + Evoxels::Many(resolution, Vol::from_fn(bounds, |_| unreachable!())), + Cost::ZERO ), EvaluatedBlock { attributes, @@ -865,7 +892,8 @@ mod tests { opaque: FaceMap::repeat(false), visible: false, uniform_collision: Some(BlockCollision::None), - voxel_opacity_mask: None + voxel_opacity_mask: None, + cost: Cost::ZERO, // TODO wrong } ); } @@ -882,10 +910,12 @@ mod tests { Rgba::new(0.0, 0.5, 1.0, 0.5), ] { let voxel = Evoxel::from_color(color); - let ev_one = EvaluatedBlock::from_voxels(attributes.clone(), Evoxels::One(voxel)); + let ev_one = + EvaluatedBlock::from_voxels(attributes.clone(), Evoxels::One(voxel), Cost::ZERO); let ev_many = EvaluatedBlock::from_voxels( attributes.clone(), Evoxels::Many(R2, Vol::from_fn(GridAab::for_block(R2), |_| voxel)), + Cost::ZERO, ); // Check that they are identical except for the voxel data @@ -925,7 +955,7 @@ mod tests { ); // The inner_color should be ignored because it is not visible. - let ev = EvaluatedBlock::from_voxels(BlockAttributes::default(), voxels); + let ev = EvaluatedBlock::from_voxels(BlockAttributes::default(), voxels, Cost::ZERO); assert_eq!(ev.color, outer_color); } diff --git a/all-is-cubes/src/block/evaluation.rs b/all-is-cubes/src/block/evaluation.rs index dccaac3b1..8613c8c5b 100644 --- a/all-is-cubes/src/block/evaluation.rs +++ b/all-is-cubes/src/block/evaluation.rs @@ -5,7 +5,9 @@ use core::cell::Cell; #[cfg(doc)] use crate::block::Block; -use crate::block::{BlockAttributes, BlockChange, EvaluatedBlock, Evoxel, Evoxels, Resolution}; +use crate::block::{ + self, BlockAttributes, BlockChange, EvaluatedBlock, Evoxel, Evoxels, Resolution, +}; use crate::content::palette; use crate::listen; use crate::math::{GridAab, Rgba, Vol}; @@ -121,6 +123,15 @@ impl Budget { cell.set(recursed); Ok(BudgetRecurseGuard { cell }) } + + /// Express a budget as a [`Cost`] value, for public consumption. + pub(in crate::block) fn to_cost(self) -> Cost { + Cost { + components: self.components, + voxels: self.voxels, + recursion: self.recursion_used, + } + } } impl Default for Budget { @@ -157,18 +168,18 @@ impl Drop for BudgetRecurseGuard<'_> { #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct Cost { /// Number of [`Primitive`]s and [`Modifier`]s evaluated. - components: u32, + pub(crate) components: u32, /// Number of individual voxels produced (e.g. by a `Primitive::Recur`]) or altered /// (e.g. by a [`Modifier::Composite`]). - voxels: u32, + pub(crate) voxels: u32, /// Number of recursion levels used by the evaluation. /// /// Recursion occurs when a primitive or modifier which itself contains [`Block`] is /// evaluated; currently, these are [`Primitive::Text`] and `Modifier::Composite`]. /// If there are none of those, then this will be zero. - recursion: u8, + pub(crate) recursion: u8, } impl Cost { @@ -182,21 +193,21 @@ impl Cost { }; /// Compute a cost from change in budget. - pub(crate) fn new(original_budget: Budget, final_budget: Budget) -> Self { - Self { - components: final_budget - .components - .checked_sub(original_budget.components) - .unwrap(), - voxels: final_budget - .voxels - .checked_sub(original_budget.voxels) - .unwrap(), - recursion: final_budget - .recursion_used - .checked_sub(original_budget.recursion_used) - .unwrap(), - } + pub(crate) fn from_difference(original_budget: Budget, final_budget: Budget) -> Self { + let Some(new_self) = (|| { + Some(Self { + components: original_budget + .components + .checked_sub(final_budget.components)?, + voxels: original_budget.voxels.checked_sub(final_budget.voxels)?, + recursion: original_budget + .recursion_used + .checked_sub(final_budget.recursion_used)?, + }) + })() else { + panic!("overflow computing budget difference: {final_budget:#?} - {original_budget:#?}") + }; + new_self } } @@ -204,12 +215,25 @@ impl Cost { #[derive(Clone, Debug, Eq, Hash, PartialEq, displaydoc::Display)] #[non_exhaustive] pub enum EvalBlockError { - /// The block definition contained recursion that exceeded the evaluation limit. - //--- - // TODO: This is misnamed, but we're going to be changing it shortly to have - // some details, so rename it then. - #[displaydoc("block definition contains too much recursion")] - StackOverflow, + /// The evaluation budget was exceeded. + #[displaydoc("block definition exceeded evaluation budget; used {used:?} so far and only {budget:?} available")] + BudgetExceeded { + /// Budget that was available for the evaluation. + budget: Cost, + /// Computation steps actually used before failure. + used: Cost, + }, + + /// The evaluation budget was exceeded, in a previous cached evaluation. + /// rather than the current one (so the current evaluation's budget + /// could not have affected the outcome). + #[displaydoc("cached block definition exceeded evaluation budget; used {used:?} so far and only {budget:?} available")] + PriorBudgetExceeded { + /// Budget that was available for the evaluation. + budget: Cost, + /// Computation steps actually used before failure. + used: Cost, + }, /// Data referenced by the block definition was not available to read. /// @@ -223,6 +247,7 @@ pub enum EvalBlockError { #[derive(Debug)] pub(in crate::block) enum InEvalError { BudgetExceeded, + PriorBudgetExceeded { budget: Cost, used: Cost }, DataRefIs(RefError), } @@ -230,7 +255,8 @@ pub(in crate::block) enum InEvalError { impl std::error::Error for EvalBlockError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { - EvalBlockError::StackOverflow => None, + EvalBlockError::BudgetExceeded { .. } => None, + EvalBlockError::PriorBudgetExceeded { .. } => None, EvalBlockError::DataRefIs(e) => Some(e), } } @@ -243,9 +269,12 @@ impl From for InEvalError { } impl InEvalError { - pub(in crate::block) fn into_eval_error(self) -> EvalBlockError { + pub(in crate::block) fn into_eval_error(self, budget: Cost, used: Cost) -> EvalBlockError { match self { - InEvalError::BudgetExceeded => EvalBlockError::StackOverflow, + InEvalError::BudgetExceeded => EvalBlockError::BudgetExceeded { budget, used }, + InEvalError::PriorBudgetExceeded { budget, used } => { + EvalBlockError::PriorBudgetExceeded { budget, used } + } InEvalError::DataRefIs(e) => EvalBlockError::DataRefIs(e), } } @@ -254,7 +283,10 @@ impl InEvalError { impl EvalBlockError { pub(in crate::block) fn into_internal_error_for_block_def(self) -> InEvalError { match self { - EvalBlockError::StackOverflow => InEvalError::BudgetExceeded, + EvalBlockError::PriorBudgetExceeded { budget, used } => { + InEvalError::PriorBudgetExceeded { budget, used } + } + EvalBlockError::BudgetExceeded { .. } => InEvalError::BudgetExceeded, EvalBlockError::DataRefIs(e) => InEvalError::DataRefIs(e), } } @@ -291,6 +323,28 @@ impl EvalBlockError { pattern[((cube.x + cube.y + cube.z).rem_euclid(2)) as usize] }), ), + match *self { + EvalBlockError::BudgetExceeded { used, .. } => used, + EvalBlockError::PriorBudgetExceeded { .. } => Cost::ZERO, + EvalBlockError::DataRefIs(_) => Cost::ZERO, + }, ) } } + +/// Convert intermediate evaluation result to final evaluation result, +/// including calculating the evaluation cost. +/// +/// Note that the order of operands is such that the original budget may +/// be passed in the form `filter.budget.get()` without a temporary variable. +pub(in crate::block) fn finish_evaluation( + original_budget: Budget, + result: Result, + filter: &EvalFilter, +) -> Result { + let cost = Cost::from_difference(original_budget, filter.budget.get()); + match result { + Ok(ev) => Ok(ev.finish(cost)), + Err(err) => Err(err.into_eval_error(original_budget.to_cost(), cost)), + } +} diff --git a/all-is-cubes/src/block/modifier.rs b/all-is-cubes/src/block/modifier.rs index 6309e6221..c19c53d8f 100644 --- a/all-is-cubes/src/block/modifier.rs +++ b/all-is-cubes/src/block/modifier.rs @@ -265,6 +265,11 @@ mod tests { rotation_rule: block::RotationPlacementRule::Attach { by: Face6::PY }, ..BlockAttributes::default() }, + cost: block::Cost { + components: 2, + voxels: 2u32.pow(3) * 2, // original + rotation + recursion: 0 + } } ); } @@ -288,9 +293,15 @@ mod tests { .extend([Modifier::Rotate(rotation_1), Modifier::Rotate(rotation_2)]); assert_ne!(rotated_twice, two_rotations, "Oops; test is ineffective"); + let ev_rotated_twice = rotated_twice.evaluate().unwrap(); + let ev_two_rotations = two_rotations.evaluate().unwrap(); + assert_eq!( - rotated_twice.evaluate().unwrap(), - two_rotations.evaluate().unwrap() + EvaluatedBlock { + cost: ev_rotated_twice.cost, + ..ev_two_rotations + }, + ev_rotated_twice, ); } diff --git a/all-is-cubes/src/block/modifier/move.rs b/all-is-cubes/src/block/modifier/move.rs index e4cd8a07e..986e7bae2 100644 --- a/all-is-cubes/src/block/modifier/move.rs +++ b/all-is-cubes/src/block/modifier/move.rs @@ -201,6 +201,7 @@ mod tests { use crate::time; use crate::universe::Universe; use ordered_float::NotNan; + use pretty_assertions::assert_eq; #[test] fn move_atom_block_evaluation() { @@ -237,6 +238,11 @@ mod tests { visible: true, uniform_collision: None, voxel_opacity_mask: Some(Vol::repeat(expected_bounds, OpacityCategory::Opaque)), + cost: block::Cost { + components: ev_original.cost.components + 1, + voxels: expected_bounds.volume_f64() as u32, + recursion: 0 + } } ); } @@ -282,6 +288,11 @@ mod tests { visible: true, uniform_collision: None, voxel_opacity_mask: Some(Vol::repeat(expected_bounds, OpacityCategory::Opaque)), + cost: block::Cost { + components: ev_original.cost.components + 1, + voxels: 2u32.pow(3) * 3 / 2, // original recur + 1/2 block of Move + recursion: 0 + } } ); } diff --git a/all-is-cubes/src/block/modifier/zoom.rs b/all-is-cubes/src/block/modifier/zoom.rs index c2e283c84..b3fa66763 100644 --- a/all-is-cubes/src/block/modifier/zoom.rs +++ b/all-is-cubes/src/block/modifier/zoom.rs @@ -207,6 +207,11 @@ mod tests { EvaluatedBlock::from_voxels( ev_original.attributes.clone(), Evoxels::One(Evoxel::from_color(Rgba::TRANSPARENT)), + block::Cost { + components: 2, + voxels: 16u32.pow(3), // counts evaluation of Recur + recursion: 0, + }, ) } else { EvaluatedBlock::from_voxels( @@ -221,6 +226,11 @@ mod tests { )] }), ), + block::Cost { + components: 2, + voxels: 16u32.pow(3) + 8u32.pow(3), // Recur + Zoom + recursion: 0, + }, ) } ); diff --git a/all-is-cubes/src/block/tests.rs b/all-is-cubes/src/block/tests.rs index 41d9fd1ed..84cc08b4b 100644 --- a/all-is-cubes/src/block/tests.rs +++ b/all-is-cubes/src/block/tests.rs @@ -86,6 +86,8 @@ fn block_debug_with_modifiers() { } mod eval { + use crate::block::{Cost, EvaluatedBlock}; + use super::{assert_eq, *}; #[test] @@ -243,7 +245,15 @@ mod eval { GridAab::for_block(resolution), OpacityCategory::Opaque, )) - ) + ); + assert_eq!( + e.cost, + Cost { + components: 1, + voxels: 8, + recursion: 0 + } + ); } #[test] @@ -503,10 +513,26 @@ mod eval { let block = Block::builder() .voxels_ref(resolution, space_ref.clone()) .build(); - let eval_bare = block.evaluate(); + + let eval_bare = block.evaluate().unwrap(); let block_def_ref = universe.insert_anonymous(BlockDef::new(block)); - let eval_def = Block::from(block_def_ref).evaluate(); - assert_eq!(eval_bare, eval_def); + let eval_def = Block::from(block_def_ref).evaluate().unwrap(); + + assert_eq!( + eval_bare, + EvaluatedBlock { + cost: eval_bare.cost, + ..eval_def.clone() + } + ); + assert_eq!( + eval_def.cost, + Cost { + components: 1, + voxels: 0, // zero because the voxels were _already_ evaluated + recursion: 0 + } + ) } /// Fuzz-discovered test case for panic during evaluation, @@ -642,7 +668,17 @@ fn overflow_evaluate() { block .modifiers_mut() .extend((0..100).map(|_| Modifier::Rotate(GridRotation::CLOCKWISE))); - assert_eq!(block.evaluate(), Err(EvalBlockError::StackOverflow)); + assert_eq!( + block.evaluate(), + Err(EvalBlockError::BudgetExceeded { + budget: block::Budget::default().to_cost(), + used: block::Cost { + components: 2, + voxels: 8, + recursion: 0 + } + }) + ); } #[test] diff --git a/all-is-cubes/src/save/tests.rs b/all-is-cubes/src/save/tests.rs index 3af6a98dd..bd03beae7 100644 --- a/all-is-cubes/src/save/tests.rs +++ b/all-is-cubes/src/save/tests.rs @@ -878,7 +878,7 @@ fn universe_success() { let a_character = deserialized_universe .get::(&"a_character".into()) .unwrap() - .read() + . read() .unwrap(); let a_block_ev = a_block.evaluate().unwrap(); diff --git a/all-is-cubes/src/space/palette.rs b/all-is-cubes/src/space/palette.rs index 3e7ffffc8..6a5da4cc1 100644 --- a/all-is-cubes/src/space/palette.rs +++ b/all-is-cubes/src/space/palette.rs @@ -1,5 +1,6 @@ use alloc::sync::{Arc, Weak}; use alloc::vec::Vec; +use core::cell::Cell; use core::fmt; use itertools::Itertools as _; @@ -458,11 +459,14 @@ impl SpaceBlockData { let (gate, block_listener) = listener.gate(); let block_listener = block_listener.erased(); - let evaluated = match block.evaluate2(&block::EvalFilter { + + let original_budget = block::Budget::default(); + let filter = block::EvalFilter { skip_eval: false, listener: Some(block_listener.clone()), - budget: Default::default(), - }) { + budget: Cell::new(original_budget), + }; + let evaluated = match block.evaluate2(&filter) { Ok(ev) => ev, Err(err) => { // Trigger retrying evaluation at next step.