mmikk / mikktspace Goto Github PK

Hello,

I noticed that there seems to be a couple of off by one errors in the edge sorting code. The condition checks here and here need to be <= as opposed to <. It seems that the last element of the edges array is not being sorted into the proper position without this change.

An example array tail before the change:

[ ...
  unnamed { i0: 84, i1: 94, f: 21 }
  unnamed { i0: 89, i1: 94, f: 23 }
  unnamed { i0: 89, i1: 94, f: 22 }
]

Thank you.

Hi!

We've recently started transitioning our engine (Northlight) to support per-pixel MikkTSpace. It has been mostly smooth sailing! We did run into a small issue when using the library in this repository though (which I think is the same one as http://mikktspace.com/).

We have the option to either use DCC exported tangents (e.g. from Blender) or generate them in our conversion pipeline. The ones coming from Blender are perfect, but the ones going through our conversion pipeline (which use this code) exhibit a warping effect. The reason why they are warped is because the vertex tangents that are being produced by our conversion pipeline don't match the ones coming out of Blender. See the screenshots below. On the left, you have the tangents from Blender, and on the right the ones generated from our conversion pipeline. Note that a few of them are fine, but some of them actually have two tangents produced per corner, when they should only have a single one. This is of course because of welding, but it's a bit surprising that these are not the same between Blender and our conversion pipeline. Our welding is just a hash of the vertex attributes, so nothing super fancy.

Both of these use the same normal map, baked from Substance (if I'm not wrong). We did a bit of investigation, and we came to the conclusion that it's somehow related to UV shells. If you use multiple UV shells (like the one on the left) our converter and Blender's agree completely, and we don't get any warping (we do of course get multiple tangents per corner in this case). However, if we use a single UV shell (like the one on the right) that's when we get incompatible tangents generated from this library and Blender. We've also tried exporting from Modo, and it gives us the same results as Blender. I'm not sure if this type of setup is an edge-case or not?

Since Blender uses the "same" mikktspace.c implementation, I downloaded the source code and stepped through the code to see where things were different. This isn't the root of the problem, but it's the first location where I could see that something seemed fishy. In BuildNeighborsFast the pEdges which are produced are completely different between this implementation (on the right) and Blender (on the left). Actually, the ones coming out from there don't seem to make much sense, they are edges connected via the same vertex! So something with building the neighborhood connectivity doesn't seem quite right, which would explain why it can't weld things properly, since it doesn't know which vertices are the actual neighbors! When looking closer at the Blender code, some functions have been re-written. My hunch is that the welding code in GenerateSharedVerticesIndexList was fixed in Blender.

This might seem a bit anticlimactic, but we simply switched over to Blender's mikktspace.c and that seemed to fix our problems. I would've liked to figure out what was the actual problem and submit a patch, but I couldn't find anything obviously out of place in here. So my question is if there could be some sort of edge-case in this implementation? Or rather, if this type of weird "single UV shell setup" was even supposed to work properly in MikkTSpace? This seems to be a pretty extreme test, so I'm not sure if any real content would ever be built like this. Below is the screenshot of our test scene, where we've swapped to Blender's implementation.

Here are the test assets we've used: mikkt_tests.zip. We assign a perfect mirror material and add a environment probe to the scene.

I think it would be worth fixing this edge-case, or integrating the changes from Blender. I will try to take another stab at this at some point, but in the meantime, is anybody else able to reproduce these issues, or is it only us? Are there any hunches what it could be?

Cheers!

Is it a fair expectation that generated tangents should be different for normals?
Is there something I need to do to make it so?

I needed tangents for anisotropic light reflection and tried to use MikkTSpace for this.

I run into a problem where generated tangent has same direction as normal. This will cause lighting calculations to fail.

This happens with my cylinder primitive geometry. It generates X-axis aligned cylinder, so that 'bottom' end has normal (-1, 0, 0) and 'top' end has normal (1, 0, 0).

Here, for simplicity, I approximate the cylinder with minimum number of vertices. this makes both bottom and top just triangles.

I've added logging to my code so that I can get a trace of SMikkTSpaceInterface usage.

A minimal trace where I have isolated the geometry only to have one triangle, which is the top:

getNumFaces() num_faces = 1
getNumVerticesOfFace(iFace = 0) = 3
getNumFaces() num_faces = 1
getNumVerticesOfFace(iFace = 0) = 3
getNumFaces() num_faces = 1
getPosition(iFace = 0, iVert = 0) = 1, 0
getPosition(iFace = 0, iVert = 1) = 1, 0.8660254
getPosition(iFace = 0, iVert = 2) = 1, -0.8660254
getPosition(iFace = 0, iVert = 0) = 1, 0
getPosition(iFace = 0, iVert = 1) = 1, 0.8660254
getPosition(iFace = 0, iVert = 2) = 1, -0.8660254
getPosition(iFace = 0, iVert = 0) = 1, 0
getPosition(iFace = 0, iVert = 1) = 1, 0.8660254
getPosition(iFace = 0, iVert = 2) = 1, -0.8660254
getPosition(iFace = 0, iVert = 0) = 1, 0
getPosition(iFace = 0, iVert = 1) = 1, 0.8660254
getPosition(iFace = 0, iVert = 2) = 1, -0.8660254
getPosition(iFace = 0, iVert = 0) = 1, 0
getPosition(iFace = 0, iVert = 1) = 1, 0.8660254
getPosition(iFace = 0, iVert = 2) = 1, -0.8660254
getTexCoord(face = 0, vert = 0) = 0, 1
getTexCoord(face = 0, vert = 1) = 0.33333334, 1
getTexCoord(face = 0, vert = 2) = 0.6666667, 1
getNumVerticesOfFace(iFace = 0) = 3
setTSpace(fvTangent = 1, 0, 0, fvBiTangent = 0, 1, 0, fMagS = 1, fMagT = 1, bIsOrientationPreserving = 0, iFace = 0, iVert = 0) corner_id = 0
setTSpaceBasic(fvTangent = 1, 0, 0, fSign = -1, iFace = 0, iVert = 0) corner_id = 0
setTSpace(fvTangent = 1, 0, 0, fvBiTangent = 0, 1, 0, fMagS = 1, fMagT = 1, bIsOrientationPreserving = 0, iFace = 0, iVert = 1) corner_id = 1
setTSpaceBasic(fvTangent = 1, 0, 0, fSign = -1, iFace = 0, iVert = 1) corner_id = 1
setTSpace(fvTangent = 1, 0, 0, fvBiTangent = 0, 1, 0, fMagS = 1, fMagT = 1, bIsOrientationPreserving = 0, iFace = 0, iVert = 2) corner_id = 2
setTSpaceBasic(fvTangent = 1, 0, 0, fSign = -1, iFace = 0, iVert = 2) corner_id = 2

Interestingly, normals are not queried at all.

Attached log has trace for full cylinder geometry. It shows that normals are now queried, but face 7 still gets tangent == normal.

You can see the full geometry in the screenshot below. Face numbers are yellow, points are green. I do not currently have visualization for face corners, so the points probably do not have much value as is. Faces are filled with interpolated texture coordinates (scaled by 0.5).

cylinder_trace.txt

In case it helps:

• my full code is at https://github.com/tksuoran/erhe.
• use of MikkTSpace is in https://github.com/tksuoran/erhe/blob/master/src/erhe/geometry/geometry.cpp#L669

Thanks!

Thanks for making your library open source. I'm looking to package it for Fedora, and its recommended that packages include a separate file which has the license text. The project license is pretty clear, but it would be helpful for documentation purposes to include this in the root of the repository.