kvark / obj

Basic Wavefront OBJ loader
Apache License 2.0
29 stars 12 forks source link

Decouple obj/mtl parsing from IO #25

Open cloneable opened 3 years ago

cloneable commented 3 years ago

Please consider decoupling of parsing from IO operations. I wanted to use your crate, but noticed that it doesn't provide loading of obj/mtl from &str or byte array. For my current project I cannot use regular file IO.

Maybe in addition to the load_X functions there could be read_X functions that are free of IO.

elrnv commented 3 years ago

How about the ObjData::load_buf function? it accepts any Read type. As for mtls I think it should be possible by instantiating Mtl with a dummy filename and use reload directly, though I haven't tried.

Is there something else that might be missing? Perhaps this is just a question of documenting this use case?

cloneable commented 3 years ago

Yes, thanks. ObjData::load_buf + Mtl::reload_with could do this, haven't checked yet. Yes, I guess I never bothered looking for loading functions in ObjData. If it's mentioned somewhere in the docs I didn't see it, sorry.

I understand it's not easy to decouple parsing from file IO when filenames are embedded inside the obj file, but in my opinion it would make the crate more useful when file IO is mostly provided for convenience, if at all. Ideally, Obj/Mtl could just provide functions taking io::Read impls and mapping functions for mtl filenames.

Fwiw, I ended up using a proc-macro to embed static mesh data in my binary and inside the proc-macro I can do regular file IO. (Since I'm targeting wasm I try to reduce the number of http requests.) Your crate works great, btw! Thanks!

elrnv commented 3 years ago

I understand it's not easy to decouple parsing from file IO when filenames are embedded inside the obj file, but in my opinion it would make the crate more useful when file IO is mostly provided for convenience, if at all. Ideally, Obj/Mtl could just provide functions taking io::Read impls and mapping functions for mtl filenames.

I agree with this sentiment. Perhaps we should rename Obj -> ObjFile and ObjData -> Obj. Then we can load the embedded mtls from Obj by requesting a parent directory explicitly in the API or simply have a getter for the mtl paths from Obj and have a method on Mtl to load said data. This would make this functionality more easily discoverable I guess.

The other action item here I think is to update the docs with an example of loading from a file and loading from a string or byte array.

cloneable commented 3 years ago

Sounds great! Thanks! Please also consider making it possible to remove any file IO via a feature, so that the crate can be used for targets like wasm. (I believe going all no_std doesn't seem viable for this crate due to use of Vecs in a lot of places. I could be wrong.)