search

bonsairobo / building-blocks

A voxel library for real-time applications.
MIT License
369 stars 30 forks source link

WIP: Integration for reading VOX files with vox-format crate #36

Closed jgraef closed 3 years ago

jgraef commented 3 years ago

I marked this PR as work-in-progress, because I haven't written any tests yet.

This PR contains integration for the vox-format crate. The integration is behing the feature vox-format, available for building-blocks and building_blocks_storage. VOX files can be loaded as Array3x1<Color> or Array3x1<ColorIndex>:

use buildings_blocks::storage::vox_format::from_file;

let voxels: Array3x1<ColorIndex> = from_file("test.vox").unwrap();

I'm not very happy that I can't implement the conversion traits in building_blocks_storage, because users might want to implement their own VoxBuffer or VoxModelBuffer, and these conversions would help with it.

I tried creating an example, but I'm not very familiar with bevy. And if I write tests, where would I put a test VOX file?

bonsairobo commented 3 years ago

Hey thanks for contributing!

I tried creating an example, but I'm not very familiar with bevy. And if I write tests, where would I put a test VOX file?

Probably use the sdf_mesh example as a starting point. And you can put any VOX files in the examples/assets directory. Maybe we should just move that out of examples/, since it's now for tests as well?

jgraef commented 3 years ago

The new commit addresses the previous points, except that I now don't want to require VoxChannel: Clone, and thus Array::fill_with is used. But I'm open to suggestions.

The VoxChannel trait makes it easier to load .VOX files into Arrays with your own channel types. It also avoids the orphan rules, as it's not possible for a user to implement VoxModelBuffer for Array3x1<C>. But now I notice that I impose to use Array3x1. I think it would be better to allow loading for any Array3, but I'm not very familiar with how channels work. @bonsairobo Maybe you can give me a hint, what I need to change.

I implemented a total of 1 tests :laughing:. The vox-format crate comes with tests, so IMHO it's enough to ensure the conversion works.

I also added docs with examples, that run as doc-tests.

bonsairobo commented 3 years ago

I now don't want to require VoxChannel: Clone, and thus Array::fill_with is used. But I'm open to suggestions.

I just added a Channel::fill_with that doesn't require T: Clone. You should be able to use that like:

let channel = Channel::fill_with(extent.num_points(), ColorIndex::default);
Array3x1::new(extent, channel)

I think it would be better to allow loading for any Array3, but I'm not very familiar with how channels work. @bonsairobo Maybe you can give me a hint, what I need to change.

Channels are either a Channel<T> or a tuple where all elements are channels, i.e. they implement whatever channel traits you need. E.g. (Channel<A>, Channel<B>) implements the same channel traits as Channel<A>. So if you want to support loading arrays with arbitrary channels, you would need Array<Chan> where Chan: GetMut + FillChannels. Right now the FillChannels impl does require Chan::Data: Clone, which won't work for you. I could make a new trait like FillChannelsWith that would make it possible.

I'd also be happy to accept this PR and make it more generic for you.

jgraef commented 3 years ago

I'd also be happy to accept this PR and make it more generic for you.

Perfect. Please go ahead :)