Hey kvark, I swear you're everywhere 😄

I am looking at using this library to convert from OBJ to another silly model format, and I need to deal with invalid inputs because users are handing me this directly. It seems like everything in the library returns results with the exception of the unexpected token branches of the mtl and the obj parser.

Would you be open to a patch that turns those panics into a Err? There are a couple ways I could try to do this depending how much you care about backwards compatibility. Some ideas off the top of my head:

• Add a new method load_err/load_buf_err, have it be the main implementation, and have the current method call it's error friend then call unwrap.
• Modify the current methods to return Result<Obj, ObjError> where ObjError is an enum containing IO Error/Parsing Error, possibly with an internal parsing error kind enum.

Let me know if you've had any thoughts about this in the past and what requirements you would want for something like this.

This ticket doesn't require action, its just for your information.

Testing indicates that 0.10.2 changes the effective MSRV from 1.37.0 to 1.40.0

https://kentfredric.github.io/rust-vmatrix/crates-o/ob/obj/

The failure on Rust <1.40.0 is:

error[E0658]: use of unstable library feature 'option_flattening'
   --> /tmp/7kHsF5P36b/vcheck_pl/cargo_1/.cargo/registry/src/github.com-1ecc6299db9ec823/obj-0.10.2/src/obj.rs:623:61
    |
623 |                 t.map(|t| normalize(t, self.texture.len())).flatten(),
    |                                                             ^^^^^^^
    |
    = note: for more information, see https://github.com/rust-lang/rust/issues/60258

error[E0658]: use of unstable library feature 'option_flattening'
   --> /tmp/7kHsF5P36b/vcheck_pl/cargo_1/.cargo/registry/src/github.com-1ecc6299db9ec823/obj-0.10.2/src/obj.rs:624:60
    |
624 |                 n.map(|n| normalize(n, self.normal.len())).flatten(),
    |                                                            ^^^^^^^
    |
    = note: for more information, see https://github.com/rust-lang/rust/issues/60258

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0658`.
error: could not compile `obj`.

And it seems Option.flatten() was stabilized in rust 1.40.0, and the commit that introduced this regression may have been 02d2c60#diff-3d0e4f350a2eb55b7d8d4c76ff6985e72306fab4046d2bf88911202e50eb95eeR625

I believe there is sometimes some value in incrementing semver when changing minimum supported rust.

Though documenting the minimum supported rust somewhere top-level (README) also goes a long way.

Setting up a CI server to check your documented MSRV sticks is also quite welcome.

In the event this issue is not helpful, just let me know and I'll try to avoid filing similar issues on this crate in future.

NB: I don't directly use obj, I'm just doing build testing across the ecosystem.

Current parsing is rather crude, and we could do better with using pom, nom, combine, and such.

cc @csherratt @vitvakatu

It would be handy to be able to export to an obj file too. This should be much simpler then writing the importer.

https://download.copr.fedorainfracloud.org/results/remilauzier/rust-simp/fedora-rawhide-x86_64/02251171-rust-obj/builder-live.log.gz

Some test failed in copr.

I'm currently using a git override for this, would it be possible to get a point release with #24 included?

Using CornellBox-Original.obj from https://casual-effects.com/data/ the following snippet:

        let meshes = obj
            .data
            .objects
            .iter()
            .flat_map(|o| &o.groups)
            .map(|g| {
                println!("{}: {}", g.name, g.polys.len());
                let mut vertices = vec![];
                let mut indexer = LruIndexer::new(16, |_, t: IndexTuple| {
                    let pos = obj.data.position[t.0];
                    let vtx = Vertex::new(pos.into());
                    vertices.push(vtx)
                });
                let indices = g
                    .polys
                    .iter()
                    .cloned()
                    .map(|p| p.into_genmesh())
                    .triangulate()
                    .vertices()
                    .map(|v| indexer.index(v) as _)
                    .collect::<Vec<_>>();

                Mesh { vertices, indices }
            })
            .collect::<Vec<_>>();

shows:

floor: 1
ceiling: 1
backWall: 1
rightWall: 1
leftWall: 1
leftWall: 6
shortBox: 0
shortBox: 6
tallBox: 0
light: 1

Similar to genmesh, I believe this library should be exposing types via https://github.com/kvark/mint

Can we release a patch version of obj with the last two PRs?

Greetings! So as I understand, the workflow to load materials data is: you go through the list of objects, then through the list of groups in the object, and if the group has the material - you load its data (textures, colours, etc). So if multiple groups link to the same material you end up duplicating all its data including textures, is it correct?

Is it possible to avoid this duplication?

For example tobj stores materials in a separate Vec and each mesh (group) than just contains material index as Option<usize>, so you can go through this flat list of materials, load all their textures etc, and then if multiple meshes link to the same material you can reuse their loaded data. tobj doesn't look as complete as this crate though.

Anything else we want to do before publishing a v0.10 with the new breaking changes?

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.

Travis signalling seems to be wonky these days