zeux / meshoptimizer

Mesh optimization library that makes meshes smaller and faster to render
MIT License
5.52k stars 474 forks source link

gltfpack: Material Kd is not making it into baseColorFactor #640

Closed kvark closed 9 months ago

kvark commented 9 months ago

Following up on #594 I'm converting an OBJ+MTL into a GLB:

"c:\Program Files\gltfpack.exe" -i level3.obj -o level3.glb -si 0.1 -km

The MTL file looks like this:

newmtl t0
    Kd 0.5176471 0.5647059 0.65882355
newmtl t1
    Kd 0.5803922 0.6431373 0.47058824
newmtl t2
    Kd 0.5176471 0.4392157 0.34509805
newmtl t3
    Kd 0.3764706 0.4392157 0.3764706
newmtl t4
    Kd 0.53333336 0.4392157 0.17254902
newmtl t5
    Kd 0.8 0 0.047058824
newmtl t6
    Kd 0.7529412 0.78431374 0
newmtl t7
    Kd 0.26666668 0.3764706 0.26666668

E.g. "t5" is red.

But after converting to gltf, this data is lost:

        {
            "name": "t5",
            "pbrMetallicRoughness":
            {
                "metallicFactor": 0
            }
        },

Using v0.20 release version, which happened on Nov 2nd. I assume it includes the fix to #594, which landed in August.

zeux commented 9 months ago

This issue needs a reproducer file.

kvark commented 9 months ago

Input: test.zip Command (using 0.20 release):

gltfpack.exe -i test.obj -o test.gltf -km

The fragment of the out that I'm getting:

    "materials":
    [
        {
            "name": "t0",
            "pbrMetallicRoughness":
            {
                "metallicFactor": 0
            }
        }
    ],
kvark commented 9 months ago

Figured this out, I think. If I add mtllib test.mtl then it gets properly picked up and translated. This behavior seems reasonable, but it diverges from other tools. E.g. blender automatically loads an "mtl" file with the same name.

Looking at the spec, it's not entirely clear to me, and I could see how it could be interpreted differently. So please feel free to close this if you believe in meshoptimizer behavior being correct.

zeux commented 9 months ago

I need to double check the spec and other loaders; I’m not sure automatically loading the mtl is right, but it might make sense to at least print a warning if an unreferenced mtl with the same name is found.

zeux commented 9 months ago

In fact, the spec you linked clearly states that there's no default material library:

mtllib filename1 filename2 . . .

    Polygonal and free-form geometry statement.

    Specifies the material library file for the material definitions
    set with the usemtl statement. You can specify multiple filenames
    with mtllib. If multiple filenames are specified, the first file
    listed is searched first for the material definition, the second
    file is searched next, and so on.

    When you assign a material library using the Model program, only
    one map library per .obj file is allowed. You can assign multiple
    libraries using a text editor.

    filename is the name of the library file that defines the
    materials.  There is no default.

tinyobjloader doesn't seem to automatically load the mtl file with the same name, and neither does assimp; I suspect Blender may be unique here, or at least unusual.

mosra commented 9 months ago

Just a random note regarding Assimp -- it does load the material with the same name, but only if there's a mtllib present with a filename that isn't found. I discovered this by accident when comparing OBJ import behavior across different libraries. Relevant piece of code is here, and this is the PR where it originated, together with some reasoning why that would be done: https://github.com/assimp/assimp/pull/835

I.e., Assimp would load test.mtl if the test.obj file contained

mtllib whatever.foo

I don't think Assimp's behavior here is anything to get inspired from, though.

To have even more data points, I checked with ufbx and it doesn't have any fallback for *.mtl loading either. So Blender is the odd one out I think.