diff --git a/CHANGELOG.md b/CHANGELOG.md index f0c8dc8b0..3a5c3e37c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ - `math::FaceMap::repeat()` has been renamed to `splat()`, for consistency with the same concept in the `euclid` vector types which we use. * `math::Geometry` is now `math::Wireframe`, and its `translate()` method has been replaced with inherent methods on its implementors. - `math::GridAab::expand()` now takes unsigned values; use `GridAab::shrink()` instead of negative ones. This allows both versions to never panic. + - `math::Vol::subdivide()` now returns an array instead of a tuple, and the `Vol<&mut [_]>` version takes a filter function. + The filter should make it easier to use in cases where the mutable subdivisions need to meet some size condition. - Renamed `behavior::BehaviorContext` to `behavior::Context`. - Renamed `behavior::BehaviorHost` to `behavior::Host`. diff --git a/all-is-cubes-base/src/math/vol.rs b/all-is-cubes-base/src/math/vol.rs index 5d042cd9f..2dbffb1c8 100644 --- a/all-is-cubes-base/src/math/vol.rs +++ b/all-is-cubes-base/src/math/vol.rs @@ -103,13 +103,11 @@ impl Vol<(), ZMaj> { /// it is also implemented for immutable and mutable references. /// These are intended to be useful in executing parallel algorithms on volume data. #[allow(clippy::missing_inline_in_public_items)] - pub fn subdivide(self) -> Option<(Self, Self)> { - let (lower_half, upper_half, _) = find_zmaj_subdivision(self.bounds)?; - - Some(( - Vol::new_dataless(lower_half, self.ordering).unwrap_or_else(unreachable_wrong_size), - Vol::new_dataless(upper_half, self.ordering).unwrap_or_else(unreachable_wrong_size), - )) + pub fn subdivide(self) -> Result<[Self; 2], Self> { + match find_zmaj_subdivision(self) { + Some((lower_half, upper_half, _)) => Ok([lower_half, upper_half]), + _ => Err(self), + } } } @@ -440,14 +438,22 @@ impl<'a, V> Vol<&'a [V], ZMaj> { /// it is also implemented for mutable references and `()`. /// These are intended to be useful in executing parallel algorithms on volume data. #[allow(clippy::missing_inline_in_public_items)] - pub fn subdivide(self) -> Option<(Self, Self)> { - let (lower_half, upper_half, lower_half_len) = find_zmaj_subdivision(self.bounds)?; - let (lower_contents, upper_contents) = self.contents.split_at(lower_half_len); - - Some(( - Vol::from_elements(lower_half, lower_contents).unwrap_or_else(unreachable_wrong_size), - Vol::from_elements(upper_half, upper_contents).unwrap_or_else(unreachable_wrong_size), - )) + pub fn subdivide(self) -> Result<[Self; 2], Self> { + match find_zmaj_subdivision(self.without_elements()) { + Some((lower_half, upper_half, lower_half_len)) => { + let (lower_contents, upper_contents) = self.contents.split_at(lower_half_len); + + Ok([ + lower_half + .with_elements(lower_contents) + .unwrap_or_else(unreachable_wrong_size), + upper_half + .with_elements(upper_contents) + .unwrap_or_else(unreachable_wrong_size), + ]) + } + _ => Err(self), + } } } @@ -455,20 +461,36 @@ impl<'a, V> Vol<&'a mut [V], ZMaj> { /// Divide `self` into two approximately equal-sized parts. /// each of which refers to the appropriate sub-slice of elements. /// - /// Returns [`None`] if `self` does not have at least two cubes. + /// `filter` may be used to reject subdivisions that are too small or otherwise unsuitable; + /// it is passed the bounds of the two slices that would be returned. + /// + /// Returns [`Err`] containing `self` if `self` does not have at least two cubes, + /// or if `filter` returns `false`. /// /// Note that this is one of several `subdivide()` methods for different container types; /// it is also implemented for immutable references and `()`. /// These are intended to be useful in executing parallel algorithms on volume data. + //--- + // Design note: This has the `filter` parameter, where other `subdivide()`s do not, + // because otherwise the caller may have trouble with conditional returns of mutable borrows + // (NLL Problem Case #3); the filter allows cancelling the split rather than discarding it. #[allow(clippy::missing_inline_in_public_items)] - pub fn subdivide(self) -> Option<(Self, Self)> { - let (lower_half, upper_half, lower_half_len) = find_zmaj_subdivision(self.bounds)?; - let (lower_contents, upper_contents) = self.contents.split_at_mut(lower_half_len); - - Some(( - Vol::from_elements(lower_half, lower_contents).unwrap_or_else(unreachable_wrong_size), - Vol::from_elements(upper_half, upper_contents).unwrap_or_else(unreachable_wrong_size), - )) + pub fn subdivide(self, filter: impl FnOnce([Vol<()>; 2]) -> bool) -> Result<[Self; 2], Self> { + match find_zmaj_subdivision(self.without_elements()) { + Some((lower_half, upper_half, lower_half_len)) if filter([lower_half, upper_half]) => { + let (lower_contents, upper_contents) = self.contents.split_at_mut(lower_half_len); + + Ok([ + lower_half + .with_elements(lower_contents) + .unwrap_or_else(unreachable_wrong_size), + upper_half + .with_elements(upper_contents) + .unwrap_or_else(unreachable_wrong_size), + ]) + } + _ => Err(self), + } } } @@ -780,9 +802,10 @@ impl fmt::Display for VolLengthError { /// Find a way to split the bounds of a `Vol` which results in two adjacent volumes /// whose linear elements are also adjacent. /// Returns the two boxes and the linear split point. -fn find_zmaj_subdivision(bounds: GridAab) -> Option<(GridAab, GridAab, usize)> { +fn find_zmaj_subdivision(vol: Vol<()>) -> Option<(Vol<()>, Vol<()>, usize)> { // The order of these tests must reflect the ordering in use // for the result to be valid. + let bounds = vol.bounds(); for axis in [Axis::X, Axis::Y, Axis::Z] { let axis_range = bounds.axis_range(axis); let size: u32 = bounds.size()[axis]; @@ -792,36 +815,25 @@ fn find_zmaj_subdivision(bounds: GridAab) -> Option<(GridAab, GridAab, usize)> { let mut lower_half_ub = bounds.upper_bounds(); lower_half_ub[axis] = split_coordinate; - let lower_half = GridAab::from_lower_upper(bounds.lower_bounds(), lower_half_ub); + let lower_half = GridAab::from_lower_upper(bounds.lower_bounds(), lower_half_ub) + .to_vol() + .unwrap_or_else(unreachable_wrong_size); let mut upper_half_lb = bounds.lower_bounds(); upper_half_lb[axis] = split_coordinate; - let upper_half = GridAab::from_lower_upper(upper_half_lb, bounds.upper_bounds()); - - let lower_half_volume = lower_half - .volume() - .unwrap_or_else(unreachable_volume_overflow); - debug_assert_eq!( - lower_half_volume - + upper_half - .volume() - .unwrap_or_else(unreachable_volume_overflow), - bounds.volume().unwrap_or_else(unreachable_volume_overflow) - ); + let upper_half = GridAab::from_lower_upper(upper_half_lb, bounds.upper_bounds()) + .to_vol() + .unwrap_or_else(unreachable_wrong_size); + + let lower_half_volume = lower_half.volume(); + debug_assert_eq!(lower_half_volume + upper_half.volume(), vol.volume()); return Some((lower_half, upper_half, lower_half_volume)); } } None } -/// Function for `.volume().unwrap_or_else()`s inside subdivision operations. -/// The advantage of this over many `unwrap()`s is generating fewer distinct panic sites for -/// these cases which are impossible. -#[cold] -fn unreachable_volume_overflow() -> T { - panic!("impossible volume overflow") -} -/// Function for `.unwrap_or_else()`s inside slicing operations. +/// Function for `.unwrap_or_else()`s inside subdivision operations. /// The advantage of this over many `unwrap()`s is generating fewer distinct panic sites for /// these cases which are impossible. #[cold] @@ -998,6 +1010,10 @@ mod tests { /// Test the properties of the `subdivide()` operations, starting from this example. #[inline(never)] fn check_subdivide_case(vol: Vol<&mut [Cube]>) { + fn filter(_: [Vol<()>; 2]) -> bool { + true + } + eprintln!("Checking {:?}", vol.bounds()); // Check the elements are as expected @@ -1006,23 +1022,23 @@ mod tests { } if vol.volume() < 2 { - // Never subdivide a cube or empty - assert_eq!(vol.without_elements().subdivide(), None); - assert_eq!(vol.as_ref().subdivide(), None); - assert_eq!(vol.subdivide(), None); + // Never subdivide a cube or empty. + assert!(vol.without_elements().subdivide().is_err()); // () version + assert!(vol.as_ref().subdivide().is_err()); // & version + assert!(vol.subdivide(filter).is_err()); // &mut version } else { - let Some((a, b)) = vol.without_elements().subdivide() else { + let Ok([a, b]) = vol.without_elements().subdivide() else { panic!("{vol:?} failed to subdivide"); }; assert_ne!(a.volume(), 0); assert_ne!(b.volume(), 0); // Compare immutable slice subdivide - let (aref, bref) = vol.as_ref().subdivide().unwrap(); + let [aref, bref] = vol.as_ref().subdivide().unwrap(); assert_eq!((a, b), (aref.without_elements(), bref.without_elements())); // Compare mutable slice subdivide - let (amut, bmut) = vol.subdivide().unwrap(); + let [amut, bmut] = vol.subdivide(filter).unwrap(); assert_eq!((a, b), (amut.without_elements(), bmut.without_elements())); // Recurse