Voxel trait inflexible

[dcz-self]

visible_block_faces accepts a pre-allocated buffer of V: Voxel. This is a limitation that becomes annoying when trying to render the same data in multiple ways.

As an example, I have a voxel scene editor, and I want to have several renderings of the same data Chunk by different properties: all voxels, walls only, conduits only. If I use visible_block_faces, I need to create different types of voxels for rendering (VAll, VWall, VConduit), with 3 different Voxel implementations, one for each class of visibility. visible_block_faces needs an allocated slice of the rendering voxels, so I can't use a slice of Voxel directly, I need to convert it. But I can't feed it an iter like this: (&[V]).map(|v| VWall::from(v). I have to ::collect() any iterator before I can use it as a slice and feed it to visible_block_faces.

I think that's unnecessarily complicated, and comes with a performance hit for a relatively simple operation.

The same problem with MergeVoxel has been solved by greedy_quads_with_merge_strategy by adding a MergeStrategy trait (although that trait is overly complicated for this use case).

1. I can see a simpler solution reusing the Voxel trait:

pub fn visible_block_faces<T, S, V>(
	    voxels: &[T],
	    voxels_shape: &S,
	    min: [u32; 3],
	    max: [u32; 3],
	    faces: &[OrientedBlockFace; 6],
	    output: &mut UnitQuadBuffer,
	) where
	    S: Shape<u32, 3>,
            V: Voxel + From<T>,
	{
// processing example: just convert on the fly
    for t in voxels:
        let v: V = t.into();
        if v.is_opaque(): ...

2. Another possibility would be to drop the &[T] slice in favour of a lazy Map<T, F> type, which translates on the fly.

EDIT: 3. Accept A: Index<usize, Output=T>.

In either of those cases, using a newtype would end up being a no-op, leading to (presumably) no performance loss.

I'm ready to implement this, just need to consult which solution makes sense.

[bonsairobo]

I like the idea of using voxels: Index<usize, Output=V> instead of a slice the best.

[bonsairobo]

I think it might need to know the length of the buffer as well though.

[dcz-self]

As far as I can tell, there's no "Length" trait, so one would have to be added and implemented for a couple builtin types. That's not the end of the world, but somewhat messy.

[bonsairobo]

Maybe I change my mind. I guess rather than trying to build the length into the trait, we could go slightly less generic and do your first idea with the newtypes. But I'd also like to have a less generic variant for when T=V. So like:

fn visible_block_faces_generic<T, V>(voxels: &[T]) where V: From<T> + Voxel { ... }

fn visible_block_faces<V>(voxels: &[V]) where V: Voxel { ... }

The fact that the algorithm acts on a slice is one of the important ways it gets good performance, so I don't think it makes sense to abstract that away.

[dcz-self]

The less generic variant can be achieved be defining an "Identity" newtype.

Thinking about slices, I don't think abstracting access is a bad thing, as long as there is still a fast way to achieve the same. But I follow the philosophy of "make the code easy to use" above "make the code fast", which may not match yours.

Either way, one step at a time, and I'm going to follow your choice.

[dcz-self]

Implemented in #4 , for the simple mesher only.

As I wrote there, it's not so great.

I think the Index approach might work better. We already have a length to work with: it's the Shape, so the only change necessary would be to provide an unchecked indexing trait.