Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loading reorders vertices even though I did not use the reorder flag #61

Open
w1th0utnam3 opened this issue Jul 7, 2023 · 2 comments
Open

Comments

@w1th0utnam3
Copy link

w1th0utnam3 commented Jul 7, 2023

I exported the default cube from Blender:

cube.obj
# Blender 3.5.0
# www.blender.org
mtllib cube.mtl
o Cube
v 1.000000 1.000000 -1.000000
v 1.000000 -1.000000 -1.000000
v 1.000000 1.000000 1.000000
v 1.000000 -1.000000 1.000000
v -1.000000 1.000000 -1.000000
v -1.000000 -1.000000 -1.000000
v -1.000000 1.000000 1.000000
v -1.000000 -1.000000 1.000000
vn -0.0000 1.0000 -0.0000
vn -0.0000 -0.0000 1.0000
vn -1.0000 -0.0000 -0.0000
vn -0.0000 -1.0000 -0.0000
vn 1.0000 -0.0000 -0.0000
vn -0.0000 -0.0000 -1.0000
vt 0.625000 0.500000
vt 0.375000 0.500000
vt 0.625000 0.750000
vt 0.375000 0.750000
vt 0.875000 0.500000
vt 0.625000 0.250000
vt 0.125000 0.500000
vt 0.375000 0.250000
vt 0.875000 0.750000
vt 0.625000 1.000000
vt 0.625000 0.000000
vt 0.375000 1.000000
vt 0.375000 0.000000
vt 0.125000 0.750000
s 0
usemtl Material
f 1/1/1 5/5/1 7/9/1 3/3/1
f 4/4/2 3/3/2 7/10/2 8/12/2
f 8/13/3 7/11/3 5/6/3 6/8/3
f 6/7/4 2/2/4 4/4/4 8/14/4
f 2/2/5 1/1/5 3/3/5 4/4/5
f 6/8/6 5/6/6 1/1/6 2/2/6

And load it with tobj:

let model = tobj::load_obj("assets/cube.obj", &tobj::LoadOptions::default())
        .context("failed to load mesh from obj file")
        // Discard material information
        .map(|(meshes, _)| meshes)?
        // Get first model only
        .into_iter()
        .next()
        .ok_or(eyre!("obj file does not contain a model"))?;
dbg!(&model.mesh.positions);

However, I get the following output:

dbg!(&model.mesh.positions);
[src/bin/friction_twist.rs:79] &model.mesh.positions = [
    1.0,
    1.0,
    -1.0,
    -1.0,
    1.0,
    -1.0,
    -1.0,
    1.0,
    1.0,
    1.0,
    1.0,
    1.0,
    1.0,
    -1.0,
    1.0,
    -1.0,
    -1.0,
    1.0,
    -1.0,
    -1.0,
    -1.0,
    1.0,
    -1.0,
    -1.0,
]

Note how the second vertex in this output does not match the second vertex from the OBJ file.

The vertices (and all other data) is re-ordered according to the order they are used by the list of faces even though I did not specify any reordering. To me this is very unexpected and it leads to problems if I e.g. re-export the obj after some manipulation and an outside application expects the order to stay constant - or if you want to debug something and want to use vertex ids to identify them in an outside application.

Am I using the library wrong or is this undocumented intentional behavior?

@Twinklebear
Copy link
Owner

Hi @w1th0utnam3 , thanks for opening this issue! Yes, this re-ordering behavior is expected, and is different from what the library "no reordering" means. So, maybe we should update the docs about this.

When you load an OBJ file, each mesh in the file gets its own set of compact positions, indices, etc., because they are built individually when we encounter the o or g tag: https://github.com/Twinklebear/tobj/blob/master/src/lib.rs#L1708-L1737 . This means that positions/normals/etc in a mesh will be exported in the order that they appear in the mesh's face list, not in the order that they appear in the file.

The "no reordering" flag only effects this step, that it should export the faces without duplicating or shuffling vertices to produce vertices that use a single index to refer to all attributes.

It sounds like the behavior you're looking for is to load the vertex/index/etc data into a single buffer that is referenced by all meshes, essentially a 1:1 mapping of the OBJ file content to the loaded data. Since I wrote this library tinyobjloader actually changed to provide an API like that: https://github.com/tinyobjloader/tinyobjloader#example-code-new-object-oriented-api instead; however there's a lot of convenience in having the mesh re-indexing to single index built into the library too (e.g., now I have to implement it myself for tinyobjloader projects: https://github.com/Twinklebear/ChameleonRT/blob/master/util/scene.cpp#L116-L182 ).

It may be possible to rework things in tobj a bit to provide this API, along with the same load_obj APIs that do reindex for convenience on top of this.

@w1th0utnam3
Copy link
Author

Thanks for the reply. Your explanation makes sense, I was just not expecting this (because I didn't consider using an OBJ file for multiple meshes). So maybe in the short term a more explicit explanation of this behavior in the docs would help.

For now I'm using the obj crate to load files as it preserves the indices. I guess you have to decide whether the additional maintenance is worth adding such an interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants