Fix OBJ loader with empty uvs or normals

[fernandojsg]

UVs idx or normals could be !== undefined but they could still be empty, as happens with the WaltzHead.obj re: #13397

[fernandojsg]

Looking at the origin of the "problem" we see that the faces on the walthead.obj has vertex and normal index but not uv:
f 5435//5435 5436//5436 2290//2290

So when we split that line, we get "" on the UVs: https://github.com/mrdoob/three.js/blob/dev/examples/js/loaders/OBJLoader.js#L503

And then we add the face with that array of parameters:
https://github.com/mrdoob/three.js/blob/dev/examples/js/loaders/OBJLoader.js#L519-L523

So basically we could do the check directly on the addFace function as I proposed on the PR or we should do the same conditional on the addFace call to send undefined if "" is detected.

@mrdoob up to you! ;)

[mrdoob]

That's for investigating further. But then this only affects uvs, right? The normals check shouldn't be needed?

[WestLangley]

Or test ua.length ? Do we know it is a string at this point?

[fernandojsg]

@WestLangley yeah, it's a string, I could change it for if ( ua !== undefined && ua.length) I don't have an strong opinion here

[WestLangley]

I wonder why this loader passes strings around, instead of converting to numbers right away?

[fernandojsg]

@WestLangley that a good point, I could give it a try to refactor it, something like adding a:

var vertexParts = vertex.split( '/' ).map(function(value) {return parseInt(value, 10); });

before passing the data around: https://github.com/mrdoob/three.js/blob/dev/examples/js/loaders/OBJLoader.js#L503

So we could get rid of the conversions inside the methods that use these values: https://github.com/mrdoob/three.js/blob/dev/examples/js/loaders/OBJLoader.js#L189

[fernandojsg]

We'll still need to do the check ua !== undefined && !isNaN(ua)

[WestLangley]

I think a refactor in that regard would be beneficial.

[mrdoob]

@WestLangley Agreed. I'll create an issue about it.

[fernandojsg]

@mrdoob not in this model, but I believe we could find some models using the following syntax and they'll still be valid obj models:

f 5435/232/ 5436/233/ 2290/234/

In fact if using our OBJExporter we'll get that that style syntax if no normal found: https://github.com/mrdoob/three.js/blob/dev/examples/js/exporters/OBJExporter.js#L144

[mrdoob]

@mrdoob not in this model, but I believe we could find some models using the following syntax and they'll still be valid obj models:

f 5435/232/ 5436/233/ 2290/234/

I don't think that's correct. These are the possible options:

f v1 v2 v3 ....
f v1/vt1 v2/vt2 v3/vt3 ...
f v1/vt1/vn1 v2/vt2/vn2 v3/vt3/vn3 ...
f v1//vn1 v2//vn2 v3//vn3 ...

The obj format is crazy but not THAT crazy 😁

In fact if using our OBJExporter we'll get that that style syntax if no normal found: https://github.com/mrdoob/three.js/blob/dev/examples/js/exporters/OBJExporter.js#L144

We should fix that 😕

[fernandojsg]

@mrdoob not sure...

http://paulbourke.net/dataformats/obj/

Some elements, such as faces and surfaces, may have a triplet of
numbers that reference vertex data.These numbers are the reference
numbers for a geometric vertex, a texture vertex, and a vertex normal.

Each triplet of numbers specifies a geometric vertex, texture vertex,
and vertex normal. The reference numbers must be in order and must
separated by slashes (/).

o       The first reference number is the geometric vertex.
o       The second reference number is the texture vertex. It follows
	   the first slash.
o       The third reference number is the vertex normal. It follows the
	   second slash.

There is no space between numbers and the slashes. There may be more
than one series of geometric vertex/texture vertex/vertex normal
numbers on a line.

The following is a portion of a sample file for a four-sided face
element:

    f 1/1/1 2/2/2 3/3/3 4/4/4

Using v, vt, and vn to represent geometric vertices, texture vertices,
and vertex normals, the statement would read:

    f v/vt/vn v/vt/vn v/vt/vn v/vt/vn

If there are only vertices and vertex normals for a face element (no
texture vertices), you would enter two slashes (//). For example, to
specify only the vertex and vertex normal reference numbers, you would
enter:

    f 1//1 2//2 3//3 4//4

From that text I wouldn't say f 1// 2// 3// or f 1/1/ 2/2/ 3/3/ are invalid.

[fernandojsg]

@mrdoob In any case I agree that we should fix that as it looks just a waste of space :D, I did a PR for that #13409

[mrdoob]

Thanks!