Rendering fix for degenerate UVs

[elalish]

Upstreaming a bugfix from model-viewer: google/model-viewer#1078

Unfortunately the models I have that display it are not for public consumption, but it's not hard to create a repro. If you have a non-degenerate triangle with duplicate UV coordinates (which happens a lot with DRACO compression), you get NaNs in the normal which can come out as a variety of different artifacts depending on the GPU driver (I've seen all-black triangles on my macbook pro and random black noise on triangles on my Linux desktop).

I've only repro'd this with glTF models without supplied tangents.

[WestLangley]

Similar: #18103.

I think it is great that you discovered the modeling problem that can lead to the mentioned artifacts.

But personally, I am not a fan of modifying the library to accommodate invalid models. (Maybe a model validator would be appropriate?)

... On the other hand, maybe you do not consider degenerate triangles in UV space to be "invalid"... @donmccurdy do you have an opinion on that?

[elalish]

@WestLangley I had the same thought as you when I first discovered this, but I've now noticed that it is surprisingly wide-spread. A big driver is compression though quantization (which DRACO uses, but so do other tools; there's even a glTF extension for it). I believe it is a legitimately difficult math problem to quantize both positions and UVs in such a way that they always become degenerate at exactly the same instant.

Considering that standard UV mapping handles degenerate UVs without any artifacts and we only introduce them though this transform, I think this is a reasonable case to handle.

[WestLangley]

@elalish That is reasonable, I guess. :-)

Just checking, are you properly handling back/double sided? And uv mirrored-repeat wrapping?

Also, is there a performance hit because of the branching?

[elalish]

@WestLangley

Just checking, are you properly handling back/double sided? And uv mirrored-repeat wrapping?

Well, I'm using Three's GLTFLoader, so I guess I would put that question back to you? At least some of these models use only front side and non-mirrored UVs.

Also, is there a performance hit because of the branching?

Well, I suppose the answer is always yes, but it's a very short branch and most models will never hit it, so it really only costs a compare. For those that do, the branch is spatially-correlated since you'll always get the same answer across a whole triangle, so it's about as good as we can hope for.

[WestLangley]

In my testing, if all three vertices of the triangle have the same UVs, then this patch appears to work.

But if two of the vertices have the same UVs and the third vertex is different, then this patch does not appear to work for me.

@donmccurdy Does the gltf validator check for degenerate UVs (on non-degenerate triangles)?

[elalish]

@WestLangley Good find! I'll look into that. That's funny because I thought my triangles only had two points with equal UVs...

[donmccurdy]

... On the other hand, maybe you do not consider degenerate triangles in UV space to be "invalid"... @donmccurdy do you have an opinion on that?

UVs are sometimes used to store custom data, or as 1D coordinates into a color palette:

So, I don't think glTF or another format can realistically prevent this situation from happening, or prohibit degenerate triangles in UVs, although it's obviously unwanted in cases where the UVs are being used in the usual way. I'm afraid I have no idea whether it's worth an unknown performance hit to resolve an issue of unknown frequency. Storing tangents is presumably the workaround we'd prefer users took, although I'm not even sure what a best-case tangent generation method would come up with here.

[elalish]

@WestLangley I managed to reproduce your error, more or less. It looks like my approach is not valid because the UVs get rounding error through interpolation, which makes == 0.0 too inaccurate to work in general. As I think about it more I think @donmccurdy is correct that in the case of degenerate UVs, tangents are required to disambiguate. The tool that has enough information to properly generate them is the tool that is causing the degeneracies in the first place, via quantization for instance. I've given this feedback to the DRACO team.