Skip to content

Commit

Permalink
mesh: Make empty texture allocations prohibited.
Browse files Browse the repository at this point in the history
This simplifies the obligations of allocator implementations because
they may assume that every existing allocation occupies at least one
distinct voxel.
  • Loading branch information
kpreid committed Nov 4, 2024
1 parent ef61541 commit 4b60391
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 9 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@
- `all-is-cubes-gpu` library:
- `in_wgpu::SurfaceRenderer::new()` requires `wgpu::Adapter` instead of `&wgpu::Adapter`.

- `all-is-cubes-mesh` library:
- `texture::Allocator` implementations are now permitted to reject zero-volume allocations.

- `all-is-cubes-port` library:
- All functionality is now conditional on feature flags, to allow omitting unneeded formats and operations.
- `ExportFormat` is now named `Format`.
Expand Down
3 changes: 3 additions & 0 deletions all-is-cubes-gpu/src/common/octree_alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ impl<A> Alloctree<A> {
/// The returned handle **does not deallocate on drop**, because this tree does not
/// implement interior mutability; it is the caller's responsibility to provide such
/// functionality if needed.
///
/// Panics if the request has zero volume.
pub fn allocate(&mut self, request: GridAab) -> Option<AlloctreeHandle<A>> {
assert!(!request.is_empty());
if !fits(request, self.size_exponent) {
// Too big, can never fit.
return None;
Expand Down
2 changes: 2 additions & 0 deletions all-is-cubes-gpu/src/in_wgpu/block_texture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ impl texture::Allocator for AtlasAllocator {
type Point = TexPoint;

fn allocate(&self, requested_bounds: GridAab, channels: Channels) -> Option<AtlasTile> {
assert!(!requested_bounds.is_empty());

let backing_arc = match channels {
Channels::Reflectance => &self.reflectance_backing,
Channels::ReflectanceEmission => &self.reflectance_and_emission_backing,
Expand Down
5 changes: 3 additions & 2 deletions all-is-cubes-mesh/src/block_mesh/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,9 @@ pub(super) fn compute_block_mesh<M: MeshTypes>(
viz.voxels(voxels);
let voxels_array = voxels.as_vol_ref();

// Exit when the voxel data is not at all in the right volume.
// This dodges some integer overflow cases on bad input.
// Exit when the voxel data is not at all in the right volume, or is empty.
// This dodges some integer overflow cases on bad input,
// and avoids asking the texture allocator for an empty allocation.
// TODO: Add a test for this case
if voxels_array
.bounds()
Expand Down
1 change: 1 addition & 0 deletions all-is-cubes-mesh/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ impl texture::Allocator for Allocator {
type Point = TexPoint;

fn allocate(&self, bounds: GridAab, channels: texture::Channels) -> Option<Self::Tile> {
assert!(!bounds.is_empty());
self.count_allocated
.fetch_update(SeqCst, SeqCst, |count| {
if count < self.capacity {
Expand Down
5 changes: 4 additions & 1 deletion all-is-cubes-mesh/src/texture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ pub trait Allocator {
/// * `bounds` specifies the desired size of the allocation;
/// its translation does not affect the size but may be used to make the resulting
/// texture coordinate transformation convenient for the caller.
/// It must not be empty (zero volume).
/// * `channels` specifies what types of data the texture should capture from the
/// [`Evoxel`]s that will be provided later to [`Tile::write()`].
/// The allocator may choose to ignore some channels if this suits the
Expand Down Expand Up @@ -212,6 +213,7 @@ pub fn validate_slice(tile_bounds: GridAab, slice_bounds: GridAab) -> Axis {
}
}

/// `voxels` must not be empty (zero volume).
pub(super) fn copy_voxels_to_new_texture<A: Allocator>(
texture_allocator: &A,
voxels: &Evoxels,
Expand Down Expand Up @@ -293,7 +295,8 @@ impl Allocator for NoTextures {
type Tile = NoTexture;
type Point = NoTexture;

fn allocate(&self, _: GridAab, _: Channels) -> Option<Self::Tile> {
fn allocate(&self, bounds: GridAab, _: Channels) -> Option<Self::Tile> {
assert!(!bounds.is_empty());
None
}
}
Expand Down
2 changes: 2 additions & 0 deletions all-is-cubes-port/src/gltf/texture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ impl texture::Allocator for GltfTextureAllocator {
type Point = GltfAtlasPoint;

fn allocate(&self, bounds: GridAab, mut channels: texture::Channels) -> Option<GltfTile> {
assert!(!bounds.is_empty());

if self.enable {
// TODO: implement more channels
if true {
Expand Down
37 changes: 31 additions & 6 deletions fuzz/fuzz_targets/fuzz_octree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ extern crate all_is_cubes;

use core::fmt;

use all_is_cubes::math::GridAab;
use all_is_cubes::math::{Face6, FaceMap, GridAab};
use all_is_cubes_gpu::octree_alloc::Alloctree;

use libfuzzer_sys::{arbitrary::Arbitrary, fuzz_target};
Expand All @@ -16,24 +16,27 @@ struct FuzzOctree {

#[derive(Arbitrary)]
enum Operation {
Allocate(GridAab),
AllocateGrow(GridAab, u8),
Allocate(NonEmptyGridAab),
AllocateGrow(NonEmptyGridAab, u8),
Free(usize),
}

#[derive(Clone, Copy, Debug)]
struct NonEmptyGridAab(GridAab);

fuzz_target!(|input: FuzzOctree| {
let mut t = Alloctree::<()>::new(clean_exponent(input.size_exponent));
let mut handles = Vec::new();

for operation in input.operations {
match operation {
Operation::Allocate(request) => {
Operation::Allocate(NonEmptyGridAab(request)) => {
let result = t.allocate(request);
if let Some(handle) = result {
handles.push(handle);
}
}
Operation::AllocateGrow(request, max_growth) => {
Operation::AllocateGrow(NonEmptyGridAab(request), max_growth) => {
let result = t.allocate_with_growth(request, max_growth);
if let Some(handle) = result {
handles.push(handle);
Expand All @@ -57,7 +60,8 @@ fn clean_exponent(input: u8) -> u8 {
/// Print operations in the form of code that can be roughly copied into an integration test.
impl fmt::Debug for Operation {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fn fmt_aab(f: &mut fmt::Formatter<'_>, aab: GridAab) -> fmt::Result {
fn fmt_aab(f: &mut fmt::Formatter<'_>, aab: NonEmptyGridAab) -> fmt::Result {
let aab = aab.0;
let lower = aab.lower_bounds().to_array();
let size = aab.size().to_array();
write!(f, "GridAab::from_lower_size({lower:?}, {size:?})")
Expand All @@ -78,3 +82,24 @@ impl fmt::Debug for Operation {
}
}
}

impl<'a> arbitrary::Arbitrary<'a> for NonEmptyGridAab {
fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<Self> {
let mut candidate = GridAab::arbitrary(u)?;
if candidate.is_empty() {
candidate = candidate.expand(FaceMap::default().with(Face6::arbitrary(u)?, 1));
}
if candidate.is_empty() {
Err(arbitrary::Error::IncorrectFormat)
} else {
Ok(NonEmptyGridAab(candidate))
}
}

fn size_hint(depth: usize) -> (usize, Option<usize>) {
arbitrary::size_hint::and(
GridAab::size_hint(depth),
arbitrary::size_hint::or((0, Some(0)), Face6::size_hint(depth)),
)
}
}

0 comments on commit 4b60391

Please sign in to comment.