FBXLoader: Check null textures before assignment

[camnewnham]

Description

We've found that a variety of FBX exporters from CAD software fail to correctly create/assign textures. As a result, scope.getTexture returns null/undefined, but the code fails when attempting to assign an encoding or other property immediately after.

As such, the preferred behaviour would be to load what is possible from the model (sans textures if they are not able to load) rather than throw and exception.

[looeee]

This looks good, thanks! 👍

[camnewnham]

I've updated this some more to not assign the material.map field if the texture (material.map.image) is missing. This is because exporters (GLTFExporter in my tests) check for material.map and assume that means an image is assigned. Would it be appropriate to console.warn here to notify the user/developer that the textures are missing?

[Mugen87]

I suggest you undo a1999fe. The previous commit a221988 already looked fine.

[camnewnham]

I suggest you undo a1999fe. The previous commit a221988 already looked fine.

Without this, the imported material has map defined but map.image undefined - this causes issues with GLTFExporter (and perhaps others) which checks for map !== undefined and then uses map.image.

Would you prefer I reversed this commit and changed GLTFExporter to check for this instead?

[Mugen87]

No, it's better to fix this in FBXLoader.

Thanks for the additional explanation. Wouldn't it be better if getTexture() does not return an incomplete texture? In this way, the check for an undefined image has to be implemented only once.

Besides, if (bumpMap?.image) { does not look like valid JS syntax.

[mrdoob]

It is valid actually:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining

Maybe we can start adopting it?
https://caniuse.com/mdn-javascript_operators_optional_chaining

[mrdoob]

But yes, adding the logic to getTexture() seems cleaner indeed.

[Mugen87]

Interesting! Support looks sufficiently good.

[camnewnham]

Thanks for the feedback. I've updated this to return undefined from getTexture() when the image is undefined.

[Mugen87]

Cleaned up the code style a bit...

[mrdoob]

Thanks!