Library structure #13
Comments
|
I think the main import function could use a variant that takes a
|
|
This is not a bad idea, but there is one caveat, namely the dependence on the directory path for buffer and image data sources. If there is a reasonable workaround for this then I'm 100% with you. Note that:
|
|
I suppose we could provide a |
|
I understood the section about streaming to be more about starting rendering before everything is loaded rather then whether files are loaded directly from disk.
suggests that Maybe we could abstract over URIs and have (user defined) implementations for different schemes that handle the actual loading. Another benefit of an API that supports this would be that even local buffer/image references could be loaded in parallel for maximum performance. |
That would make a lot more sense, I misunderstood the meaning of streaming,
The scope of the crate is growing larger than I originally planned. When you say My first thoughts were a simple callback with the URI passed as a parameter. The My second thoughts were some kind of trait, for example: trait Reader {
type Error;
fn read_buffer(&mut self, buffer: &Buffer) -> Result<Vec<u8>, Self::Error>;
fn read_image(&mut self, image: &Image) -> Result<Vec<u8>, Self::Error>;
}The user would then write something like this: struct Reader;
impl Reader for Reader {
type Error = ...;
fn read_buffer(&mut self, buffer: &Buffer) -> Result<Vec<u8>, Self::Error> {
if buffer.uri().starts_with("file://") {
// Read from file system
} else if buffer.uri().starts_with("http://") {
// Download from internet
} else {
// Unsupported scheme
Err(...)
}
}
fn read_image(&mut self, image: &Image) -> Result<Vec<u8>, Self::Error> {
// Similar to above
}
}
fn main() {
let gltf = gltf::v2::import("Foo.gltf", Reader);
}This seems like a lot of work on the user's behalf. Perhaps we could provide a fn main() {
let gltf = gltf::v2::import("Foo.gltf", gltf::v2::reader::Simple("./dir"));
} |
|
Third thoughts: The above is no good since the original discussion was about /// A glTF source. Calling `Read::*()` means reading the .gltf file.
/// `read_buffer()` and `read_image()` are extra required methods needed to read
/// the data referenced by the .gltf file.
trait Source: Read {
type Error;
fn read_buffer(&mut self, buffer: &Buffer) -> Result<Vec<u8>, Self::Error>;
fn read_image(&mut self, image: &Image) -> Result<Vec<u8>, Self::Error>;
}
enum Reader {
/// glTF hosted on the file system.
File(PathBuf),
/// glTF hosted on the world wide web.
Http(Url),
}
impl Read for Reader {
...
}
impl Source for Reader {
type Error = ...;
fn read_buffer(&mut self, buffer: &Buffer) -> Result<Vec<u8>, Self::Error> {
if buffer.uri().starts_with("file://") {
// Read from FS
} else if buffer.uri().starts_with("http://") {
// Download from WWW
} else {
// Unsupported scheme
Err(...)
}
}
fn read_image(&mut self, image: &image) -> Result<Vec<u8>, Self::Error> {
// Similar to above
}
}
fn main() {
let gltf_from_fs = gltf::v2::import(Reader::File("./dir/Foo.gltf"));
let gltf_from_www = gltf::v2::import(Reader::Http("example.com/Foo.gltf"));
}The pub fn import(source: &mut Source) -> ImportResult {
let root: Root = {
let mut data = Vec::new();
source.read_to_end(&mut data)?;
serde_json::from_slice(&data)?
};
root.source = Some(source);
Ok(root)
}Note that we would want dynamic dispatch here or else compile times would be insane. |
|
That's pretty much what I had in mind, though I hadn't thought it out this far. To avoid the if-else cascade in I wonder though, who would actually call Also, where would the (down)loaded data be saved? Extra fields on About the compile times: I was already surprised how slow |
I've been planning a 'rustic' wrapper for the crate for a while. You can browse the ideas in this new
I'd say the
This has been a pain point in the wrapper idea because the Rust ownership rules start to get in the way. The current way the wrapper handles this is to preload everything upon creation. This seems like a bad idea though because of blocking I/O, denial of service, et cetera. External data should ideally be lazily loaded but as far as I'm aware this means the user would have to make a place to store the data and pass
Maybe. I'd suggest using |
Sounds like a good default, but I think the
Might On the other hand, perhaps it would be better to leave some of the details to the user and just provide basic helper functions and structs. For example, lazy loading could mean different things:
Even better is the RLS (though it was broken in nightly unfortunately when I was working on gltf...) |
This is a very good point. If an
Let's not go there if possible. It would solve some problems but also create new problems, for example making sure there is only one writer at runtime.
Thanks, I'll check that out. |
|
Some more points:
There are likely to be many nodes, and if they're all reading little segments from say the file system then I doubt that would perform well, but that's only speculation.
There is a further point to be raised here: Since glTF can contain multiple scenes then perhaps reading the .gltf / .glb file and loading the referenced data should be two separate operations. For example: // Calls `serde_json::from_whatever` and performs some validation
fn import(path: Path) -> Gltf { ... }
impl Scene {
// Loads the scene data
//
// N.B. taking 'out' parameters is not recommended by the Rust API guidelines
// but `std::io::Read` does it for reasons similar to one's were facing
fn load(dest: &mut SceneData) -> LoadResult<()> { ... }
}
I used to have this mindset but then I began to think if the user has this burden then they are unlikely (at least in the short term) to adopt glTF into their project. The wrapper exists alongside the 'raw' data structures to allow any user to do their own thing if they really want to.
That sounds like a optimisation that's out of scope for the |
I'd expect that if there's only one .bin file as in all the examples I've seen so far, only the first node accessed would read from the file system, later ones would get the data from memory. Though I also like the separate scene loading interface. |
|
Here's a draft of the new import strategy. I'm still not sure how to handle async yet. |
|
Looks/works good so far. Two comments:
|
|
Both the mesh iterator on I'm pleased to hear the wrapper is working well. |
|
Finally got to a first successful render of
I'll try writing a network-based |
|
Another thing: loading Perhaps we could track somewhere what already works/what is missing? |
That's wonderful news! It's great to see the crate in action.
They are pretty nifty if I may say so myself, and especially so if used with
The only reason why it's named this way is because then it matches the attribute name in the specification. I completely agree that they're confusing, so let's change them to plurals.
It's because the URI contains embedded base64 data, which is at the top of my 'to do' list to implement.
That's a good idea. I think it would be best to do this using GitHub issues with custom tags. Many Rust projects seem to do this successfully.
If you have the time, feedback would be appreciated. |
Interesting alternative, I wanted to use |
|
By the way there's a new version of the wrapper interface. GLB is supported now - the implementation was much more difficult to write than I'd expected. |
My first Idea was a single issue with a few checkboxes.
Nice! |
How about an issue with a list of all the things that are in a working state? The rest can have individual issues so that we can discuss the details easily. |
|
Found another surprising thing: I wanted to return a |
|
This is a good point. Until now I've believed that simply returning |
|
Bad news I was wrong about returning impl Owner {
fn borrow<'a>(&'a self) -> Borrowed<'a> {
// self is borrowed even if 'a is the 'static lifetime
}
}Good news This problem was easily solved by instead having two flavours of the importer - I called them You should be able to return a |
|
Also, |
This addresses comment https://github.com/alteous/gltf/issues/13#issuecomment-310899934.
|
With the increasing number of possible options, it might be worth considering the builder pattern. Something along the lines of: let importer = ImporterBuilder::new()
.source(NetworkSource::new("http://example.com/foo.gltf")
.load_binary_data(true), // with `false`, only load the json, leave the rest to the user
.mode(Mode::Async) // or ZeroCopy
.lazy_loading(Lazy::Off) // or Scene/All
.validation(Validation::Complete) // or Minimal
.generate_missing(Semantic::Normals) // can be repeated with Semantic::Tangents
.build();This might also make it easier to add new options later without breaking the API. |
|
Issues #21 and #22 would be easier with a math library. Could we (optionally?) add Another idea: the |
|
Could |
|
Did you try |
|
We could make functions more generic, maybe? impl Node {
fn matrix<T>(&self) -> T
where T: From<[f32; 4]>
{
self.json.matrix.into()
}
} |
It could be if it was actually integrated with the libraries - kvark/mint#11 suggests that it's not very successfull so far. I also have the feeling that most people now use
Yes, same message as for |
|
It seems there is an |
|
The crate received some valuable criticism recently on the #rust-gamedev IRC channel. Below is a summary of the comments received.
To address these criticisms, I propose the following.
I encourage further discussion. |
|
@alteous Btw, |
|
@vitvakatu Thanks for your interest!
Of course. In case I've misunderstood, would the intention be to replace vector types with |
I can't speak for everyone, but I am very happy with nalgebra.
I'm not deeply familiar with the current API (or glTF itself for that matter) but (as alluded to in IRC) it's not at all obvious to me that the root If you want to continue to provide friendly accessors like |
|
About splitting into sub-crates: Apart from that, I agree with the general direction. I personally like the convenience the crate provides beyond just parsing, but I have no problem moving some of that to my viewer/engine. |
As do I. I'd like to redesign the way the wrapper library receives data so that we still have these conveniences. One idea I had this morning was to introduce a trait Source {
/// Customisable error type - reference implementation provided by `gltf-importer`.
type Error;
/// Returns `Ok(())` if the data is prepared. The data should be loaded by the `Source`
/// implementation if unavailable, returning `Err(...)` if the data cannot be sourced.
///
/// The user is expected to persist the buffer for the lifetime of the loaded data.
/// Hopefully the borrow checker can guarantee this for us.
fn prepare(&mut self, buffer: usize) -> Result<(), Self::Error>;
/// For the wrapper library to retrieve buffer data without taking `&mut`.
fn retrieve<'a>(&'a self, buffer: usize) -> &'a [u8];
}
/// Internal trait for converting `Unloaded<T>` to `Loaded<T>`.
trait Load {
/// The loaded result type.
type Loaded;
/// Recurses down the hierarchy, calling `prepare()` on every item that will need to be
/// ready for the wrapper.
fn load<S: Source>(self, source: &mut S) -> Result<Self::Loaded, S::Error>;
}
/// Wrapper type for items that may not have their data ready.
struct Unloaded<T: Load> {
item: T,
}
impl<T: Clone + Load> Load for Unloaded<T> {
type Loaded = T::Loaded;
fn load<S: Source>(self, source: &mut S) -> Result<Self::Loaded, S::Error> {
/// Recurse down the hierarchy, loading everything that's needed from hereon.
self.item.clone().load(source)
}
} |
|
The advantage of this idea is that you can call |
Unfortunately it has to be in its own crate because it's a |
|
Actually, I think this idea is overly complex. A much simpler approach might be to allow the user to 'query' what data is needed on individual objects, let the user source the data themselves, and provide the data back to the object. This pseudo-code should briefly explain the idea: /// A source of buffer data.
trait Source {
/// N.B. this *must not* fail - the user is expected to
/// ensure that all the data is available.
fn source_buffer<'a>(&'a self, buffer: Buffer<'a>) -> &[u8];
}
/// New: Wrapper type for a glTF object with loaded data.
struct Loaded<'a, T> {
/// The wrapper item - `Mesh` in this example.
item: T,
/// User data that (hopefully) contains all the data needed by the item
/// and all of its children.
source: &'a Source,
};
/// Same as before.
struct Mesh<'a> {
gltf: &'a Gltf,
json: &'a json::Mesh,
index: usize,
}
impl Mesh {
/// Returns an `Iterator` over the mesh primtives, none
/// of which have any data.
fn primitives(&self) -> Primitives<'a> {
...
}
/// Returns a `Vec` of all the buffers required by the
/// user to be loaded.
fn query(&self) ->Vec<Buffer> {
vec![Buffer(3), Buffer(1), Buffer(4)]
}
}
impl Loaded<Mesh> {
/// Returns an `Iterator` over the mesh primitives, *all* of
/// which have their data, provided the user initialised the
/// buffer source correctly.
fn primitives(&self) -> LoadedPrimitives<'a> {
LoadedPrimitives {
iter: self.json.iter(),
// All the child objects of `Loaded<Mesh>` get their data too!
// Imagine how flexible this could be with say, individual nodes.
source: self.source,
}
}
}
struct UserSourceImplementation {
/// For example.
buffers: Vec<Vec<u8>>,
}
impl Source for UserSourceImplementation {
fn source_buffer<'a>(&'a self, buffer: Buffer<'a>) -> &[u8] {
&self.buffers[buffer.index()]
}
}
enum Gltf {
Standard(Gltf),
Binary(Gltf, Vec<(u32, Vec<u8>)>),
}
fn main() {
let mut source = UserSourceImplementation;
let gltf = match gltf::parse(include_bytes!("example.glb")) {
Gltf::Standard(gltf) => gltf,
Gltf::Binary(gltf, embedded_buffers) => {
source.store(embedded_buffers);
gltf
},
};
let unloaded_mesh = gltf.meshes().nth(0).unwrap();
source.load_buffers(unloaded_mesh.query());
let mesh = unloaded_mesh.load(&source);
for loaded_primitive in mesh.primitives() {
let indices = loaded_primitive.indices_u32().unwrap().collect();
// you get the picture...
}
} |
|
Also, we can make |
|
That looks similar to what I had in mind. However, I'd still much rather do away with the |
Good news: this can be done in one line! Contents of example: struct Loader {
data: Vec<u8>,
}
#[derive(Debug)]
struct LoadError;
trait Source {
fn source(&self, buffer: usize) -> &[u8];
}
trait Query {
fn query(&self) -> Vec<usize>;
}
struct Item {
required_buffer: usize,
}
impl Query for Item {
fn query(&self) -> Vec<usize> {
vec![self.required_buffer]
}
}
struct Loaded<'a, T> {
item: T,
source: &'a Source,
}
impl<'a> Loaded<'a, Item> {
fn print(&self) {
println!("{:?}", self.source.source(self.item.required_buffer));
}
}
impl Loader {
fn load<T: Query>(&mut self, item: T) -> Result<Loaded<T>, LoadError> {
for buffer in item.query() {
self.data.push(buffer as u8);
}
Ok(Loaded {
item,
source: self,
})
}
}
impl Source for Loader {
fn source(&self, _buffer: usize) -> &[u8] {
self.data.as_slice()
}
}
fn main() {
let mut loader = Loader { data: vec![] };
let unloaded_item = Item { required_buffer: 123 };
let item = loader.load(unloaded_item).expect("loading error");
item.print();
} |
|
The above decouples buffer loading from tree traversal. The |
glTF data can come from many buffers. For example, a
Extensions have always been opt-in via cargo features, so one pays for only what is used. There are no extensions implemented at the moment.
Forgive me for being nosy, but what is it you're trying to achieve? glTF is intended to be the 'jpeg of 3D', with the further intention of replacing proprietary asset formats with its own extensible, open format. Hence, supporting conversion from glTF to proprietary formats is not something the crate was ever intended to achieve. |
Ah, that clarifies the reasoning behind the direction you're taking things. I especially like the The
How are non-standard extensions (which nearly every standard extension starts out as) going to be supported, then? I'm not a big fan of forcing people to fork the library.
It was intended as a vague example. A more concrete example might be a tool to extract individual nodes, or a visualizer (or even game engine!) that only loads buffers associated with data that actually needs to be displayed, which, given the "asset library" usecase glTF explicitly supports, should frequently not be "all of them." Even if I am using all data in a glTF file, I'd prefer to be able to stream it in continuously (ordered by visibility, say) rather than as a single monolithic blob.
I do, however, fully intend to convert from glTF to a format of my own. Not because I have some weird political requirement to use proprietary formats, but because my internal format has a fundamentally different architecture than glTF does--it's a fine-grained content-addressed store, not to mention that it encodes a variety of application-specific data in addition to geometry and textures. I'm a fan of glTF's objectives and hope to support it as fully as possible, but it should be no surprise that it's not the perfect terminal format for literally every application. |
|
I've abandoned the idea of the |
|
Updated the OP. |
|
The names of some animation data structures are quite verbose. I suggest we rename |
|
KhronosGroup/glTF#1100 suggests we shouldn't be returning |
|
Moving discussion over to #12 |
This thread exists as a place to discuss the modules, functions, data structures, and their names included in the library. In version
1.0.0names must be agreed upon and remain immutable for backward compatibility. Until then, we should experiment as much as possible and find out what works best.Current structure
gltf
gltf-json
gltf.gltf.semvermatches that ofgltffor convenience but is otherwise unstable.gltf-importer
gltf-utils
gltfcrate.May remain unstable postgltf 1.0.The text was updated successfully, but these errors were encountered: