guybrush77 / rapidobj

A fast, header-only, C++17 library for parsing Wavefront .obj files.
MIT License
178 stars 16 forks source link

Material Id == -1? #17

Closed blitz-research closed 1 year ago

blitz-research commented 1 year ago

Hi,

I am trying to get a model loading, but all the material id's are returned as -1.

The model has an .mtl file which appears to be loading OK as there is an array of 2 materials in the loaded result and 2 materials in the mtl file.

The model works OK if I 'force' material id to 1 (the last material in the .mtl file) but just thought this might be a bug and, if not, what should I be doing with material id's of -1?

Of course the model could be corrupt up - it's a model of a vive VR controller that comes with steam vr and if you have steamvr you can find it in there somewhere, if not I'd be happy to send it to you.

Bye! Mark

guybrush77 commented 1 year ago

Hello Mark,

Thank you for reporting the issue. I do not have this model, could you please upload it, if it is not too large? I will take a look.

blitz-research commented 1 year ago

Hi,

I've uploaded the model and textures here:

https://murena.io/s/Z6MSrKkCf2Mr72Y

Also, in case it's something I'm doing wrong (wont be the first time!) here's the loop in my code that reads in vertices/triangles where the problem is occuring. The Model:: namespace is my own class, but it should be pretty self explanatory how it works:

    Map<Model::Vertex, uint32_t> vertices; // just a std::map
    Vector<Vector<Model::Triangle>> triangles;  // just a std::vector
    triangles.resize(result.materials.size());

    for (auto& shape : result.shapes) {

        auto& mesh = shape.mesh;
        auto num_faces = mesh.num_face_vertices.size();
        if (!num_faces) continue;

        auto materialIt = mesh.material_ids.begin();
        auto indexIt = mesh.indices.begin();

        for (size_t face = 0; face < num_faces; ++face) {
            assert(mesh.num_face_vertices[face] == 3);

            // >>>>> Here is where I'm getting materialId of -1 <<<<< //
            auto materialId = *materialIt++;
            // assert(materialId >= 0 && materialId < result.materials.size());
            materialId = 1;

            Model::Triangle tri;

            for (int i = 0; i < 3; ++i) {

                auto& index = *indexIt++;
                Model::Vertex v;

                float* position = &result.attributes.positions[index.position_index * 3];
                v.position = {position[0], position[1], position[2]};

                if (index.normal_index != -1) {
                    float* normal = &result.attributes.normals[index.normal_index * 3];
                    v.normal = normalize(Vec3f{normal[0], normal[1], normal[2]});
                }
                if (index.texcoord_index != -1) {
                    float* texcoord = &result.attributes.texcoords[index.texcoord_index * 2];
                    v.texcoord = {texcoord[0], texcoord[1]};
                }

                auto it = vertices.find(v);
                if (it == vertices.end()) {
                    it = vertices.insert(std::make_pair(v, model->vertices.size())).first;
                    model->vertices.push_back(v);
                }
                tri.vertices[i] = it->second;
            }
            triangles[materialId].push_back(tri);
        }
    }

Does that look OK? Am I correct in thinking I need something like the map above if I want to remove duplicate vertices?

Nice little loader by the way, I found it very pleasant to work with, excellent docs too.

Bye, Mark

guybrush77 commented 1 year ago

Thanks! There is an issue with this .obj file. To apply a material to a face, it is necessary to use the usemtl command. All faces that follow will be assigned the id of that material until the next usemtl command is encountered. See here: https://people.computing.clemson.edu/~dhouse/courses/405/docs/brief-obj-file-format.html.

So, for example, you could modify the file by simply adding a usemtl after the group definition on line 30344:

g whole_model group1
usemtl lambert4SG

Does this work for you?

And, yes, using a map to de-duplicate is fine.

blitz-research commented 1 year ago

Hi,

Thanks for looking into that, it's supposed to be a generic loader so I need to come up with a way to choose a 'default' material I guess, my problem definitely not yours though.

I found something else a bit weird in mtl file today, the lambert4SG material has these diffuse components:

Kd 0.00 0.00 0.00 map_Kd onepointfive_texture.png

I usually multiply diffuse color by diffuse texture, but in this case it'd just result in black everywhere! Does obj file format handle diffuse differently, or is it just the file being crap again?

Bye, Mark

guybrush77 commented 1 year ago

The .obj file format, including handling materials, is not so well defined but what you are doing makes sense to me. I think you are right, the exporter is not doing the best job.

guybrush77 commented 1 year ago

Closing issue.