search

Twinklebear / tobj

Tiny OBJ Loader in Rust
MIT License
233 stars 47 forks source link

Add Support For Async Material Loader on tobj::load_obj_buf #45

Closed hwoodiwiss closed 3 years ago

hwoodiwiss commented 3 years ago

tobj::load_obj_buf does not currently support async material loader functions.

It would be useful if it could for using the library in a WebAssembly context, where no IO can be blocking.

An alternative could be a separate async version, i.e. tobj::load_obj_buf_async

If this is something that would be likely to be accepted, I would be happy to have a go at implementing this as well.

Twinklebear commented 3 years ago

Yeah, this sounds like it'd be a useful addition! If there's a way to do it through some trait to have both blocking and non-blocking go through tobj::load_obj_buf without breaking the API, then it'd be fine to do it through that API. Otherwise a separate tobj::load_obj_buf_async would also be great.

I'd be happy to give feedback and accept a PR if you have time to work on this!

hwoodiwiss commented 3 years ago

I was thinking potentially gate it behind a non-default feature, which would allow the original implementaion to stand as is, but be overloaded with the async version when the async feature is used?

Twinklebear commented 3 years ago

Sounds good! I'm not up to speed with the async stuff, would that require a nightly version of Rust?

hwoodiwiss commented 3 years ago

I don't think it needs anything that hasn't already been stabilised and release.

hwoodiwiss commented 3 years ago

One issue I am coming up against is wrangling the lifetime for the &Path parameter for the closure.

Would it be acceptable to try to copy the path to an owned String, and pass that instead?

Twinklebear commented 3 years ago

Oh right I see for the async one the lifetime there will be an issue. I think copying it to an owned string (or Path) should be fine

hwoodiwiss commented 3 years ago

I've opened a PR for you to have a look at #46

I've ended up going with just a separate tobj::load_obj_buf_async as changing the signature for tobj::load_obj_buf would require making tobj::load_obj asynchronous as well.

I've still gated the async version behind a feature, just for cleanliness, and am happy to support this feature going forward.

hwoodiwiss commented 3 years ago

Hey, sorry to bump, would it be possible for you to have a look at #46?

Twinklebear commented 3 years ago

Sorry for the delay on my side, been busy with some other stuff. I'll check it out!

hwoodiwiss commented 3 years ago

Thanks, I get how it can be!