Only create aoMetRoughTex when metalnesss or roughness texture provided

[mtostenson]

This change prevents the creation of the combined metalness and roughness texture for PBR FBX materials if neither a metalness or roughness map has been provided.

The motivation for this change was to prevent overriding the provided constant values for metalness and roughness. In our situation, we are not using metalness or roughness maps and provide constants for both values, but they were being lost because the combined map was being created anyway. In the file MaterialData.cpp the to_json() method forces the roughness and metalness values to 1 if the metRoughTexture exists, so it should follow that the texture should only be created if we provide either a metalness or roughness map from the beginning.

[zellski]

Oh good lord, it looks like we always did the wrong thing here. Many thanks. I'll run it through a couple of test suites tonight & then happily merge.

[zellski]

I merged this –– thanks! After closer perusal of the section in general, I also took it a step further in https://github.com/facebookincubator/FBX2glTF/commit/d2f7d0e270156509f18390244fa17fae65d92712. Would you be able to run this against a couple of models with occlusion maps –– ideally with and without metallic/roughness maps? I realise I don't have any in my test suite (must fix).

[mtostenson]

I merged this –– thanks! After closer perusal of the section in general, I also took it a step further in d2f7d0e. Would you be able to run this against a couple of models with occlusion maps –– ideally with and without metallic/roughness maps? I realise I don't have any in my test suite (must fix).

I tested this on one of our models that provide a constant for metalness and roughness, and it looks like they got overridden. Here's the material object from the glTF:

        {
            "name": "mat_metal",
            "alphaMode": "OPAQUE",
            "extras": {
                "fromFBX": {
                    "shadingModel": "Metallic/Roughness",
                    "isTruePBR": true
                }
            },
            "occlusionTexture": {
                "index": 1,
                "texCoord": 0
            },
            "pbrMetallicRoughness": {
                "baseColorTexture": {
                    "index": 2,
                    "texCoord": 0
                },
                "baseColorFactor": [
                    0.5,
                    0.5,
                    0.5,
                    1.0
                ],
                "metallicRoughnessTexture": {
                    "index": 1,
                    "texCoord": 0
                },
                "roughnessFactor": 1.0,
                "metallicFactor": 1.0
            }

It looks like it's referencing the correct AO map though, just using the provided one.

"images": [
        {
            "name": "180971&mat_aerator&AO.jpg",
            "uri": "180971&mat_aerator&AO.jpg"
        },
        {
            "name": "180971&mat_metal&AO.jpg",
            "uri": "180971&mat_metal&AO.jpg"
        },
        {
            "name": "180971&mat_metal_1907278&diffuse.jpg",
            "uri": "180971&mat_metal_1907278&diffuse.jpg"
        }
    ],

[zellski]

Ah, crap, yes. More logic errors on my part. Will take another pass in a while.

(If you hadn't guessed, our regression suite is still very poor.)

I don't suppose, for sake of expedience, that you have a FBX file available that you're at liberty to send me?

[mtostenson]

Ah, crap, yes. More logic errors on my part. Will take another pass in a while.

(If you hadn't guessed, our regression suite is still very poor.)

I don't suppose, for sake of expedience, that you have a FBX file available that you're at liberty to send me?

@LaurenPozzi do you have something?

[LaurenPozzi]

Attached. This includes AO maps and has values plugged in for color, roughness, and metalness. PBR toilet fbx.zip

[zellski]

Fantastic! Thank you. Will sic code at it shortly.

[mtostenson]

@zellski Any chance you could kick off a windows build so my artists can get the latest?

[zellski]

@mtostenson apologies for the delay –– kicked off builds here: https://dev.azure.com/parwinzell/FBX2glTF/_build/results?buildId=45