jkvargas / russimp

Assimp bindings for Rust
Other
84 stars 27 forks source link

russimp: Introduce FileSystem loading #56

Closed waych closed 9 months ago

waych commented 11 months ago

When loading resources from memory buffers, sometimes the resources in question link to secondary files that must also be loaded. An example of this is the loading of .obj files, which store their materials in a secondary .mtl file. This file can't be passed in using the from_buffer() loading path, resulting in the need to load the files from the host native filesystem using the from_file() loading path.

This change implements a rust safe loading path to allowing applications to implement their own virtual filesystem interface with assimp. This is convenient for use in combination with an application specific resource loader.

To use, callers simply implement the fs::FileSystem trait, which neccesitates the implementation of open() which returns their own type as a trait object implementing fs::FileOperations.

This new fs module handles all the unsafe glue required to interface this safe trait-based interface with the underlying unsafe aiFileIO parts.

waych commented 11 months ago

Build is failing due to lack of aiFileIO, which isn't exposed in the depended-upon version of russimp-sys. I had to add "cfileio.h" to my local copy of russimp-sys's wrapper.h to have the bindings generated for the type.

I see that cfileio.h is enabled in the latest russimp-sys main branch, but I haven't been able to switch to the new build there as of yet (missing "assimp/cimport.h"?). Haven't dug into debugging why it fails for me yet.. I see that this is blocked on moving russimp itself over to the russimp-sys 2.0. I guess this can sit until #50 is resolved, or another 1.0 release is pushed with cfileio.h included in wrapper.h.

jkvargas commented 9 months ago

hey mate, thanks so much for your contribution. I am not having a lot of time to work on russimp lately. but I am accepting PRs.

Are you up to update russimp to use russimp-sys 2.0? Thanks for the patience, man! =)

waych commented 9 months ago

I have russimp-sys 2.0 working locally, so this shouldn't be too much of a problem. I think I'm not hitting the issue mentioned in #50 however as russimp-sys is coming into my build as a git submodule and therefore am also pulling the nested assimp codebase.

Is the issue blocking #50 just that a copy of the assimp.h header is required in the crate distribution e.g. for when not using the build-assimp feature?

jkvargas commented 9 months ago

I am going to merge it and fix possible issues. =)

Thanks for your contribution again @waych

waych commented 9 months ago

Thanks for the pull!

I'll admit that I tried to kick the tires here to make sure they still fit, but I kept getting hung up on the fact that the new russimp-sys doesn't include sources, only headers. This doesn't affect me for my local project as I'm providing the library in elsewhere in my project, but for russimp stand alone, it would seem feature="static-link" cannot be relied upon to the build locally anymore when pulling the crate from crates.io?

jkvargas commented 8 months ago

@waych hey man, I believe that makes sense... I wouldn't mind if you want to put your hands on that as well =)