Open lexaknyazev opened 6 years ago
I don't know the answers to most of these, but:
Should we have two thresholds with
Error
andWarning
validation severities?
No. The glTF group should decide on a single allowable threshold, and either it's violated or it's not violated. Violating it produces a message of user-configurable severity.
Also:
Should we separate obviously broken matrices (invalid last row or zero determinant) into a separate validation error?
I'd be OK with this, if people generally agree it's good.
It may be slight overkill though, as I see the goal of the validator as being more along the lines of "Hey user, direct your attention right here, this thing is broken here." I guess it could be useful to the user to have two different ways to specify why a matrix was called out as being in need of attention.
I suggested "no" to having 2 separate numeric thresholds above, and I stand by that, but now I do need to recommend adding a special case:
In KhronosGroup/glTF-Blender-Exporter#110 we realized that an artist could legitimately place triangles that contain degenerate UV coordinates (that is, three verts in different XYZ locations, but two of those verts share the same UV coordinates, stretching the texture across the triangle). Such a configuration makes it impossible to calculate tangent vectors, and Blender's response is to simply embed zero-length tangent vectors when asked to include them.
So, I think zero-length tangents (and maybe zero-length normals) should be a special case, treated separately from other vectors that violate numeric thresholds, with their own configurable message severity. I'm tempted to say the default severity should be no higher than warning
since the artist may have intended these.
we realized that an artist could legitimately place triangles that contain degenerate UV coordinates (that is, three verts in different XYZ locations, but two of those verts share the same UV coordinates, stretching the texture across the triangle)
Are we sure that it was the case for DamagedHelmet
? Blender uses MikkTSpace for tangents generation.
Are we sure that it was the case for DamagedHelmet?
Sorry, I haven't dug into that yet. It's on my list but it's not very high priority, the model works fine without the tangents.
I am no expert, but I would consider applying a proper formalism for the propagation of error as the basis for choosing the relative thresholds between Vec3 and Quaternions. This could avoid unnecessary cleanup downstream by researchers performing rigorous analysis of simulations designed with glTF.
Although I don't have a solid basis for arguing about this:
Is the decomposing procedure that is described there some sort of "common best practice"? I think that there might be some degrees of freedom or possibly alternative implementations.
I also wonder about possible numerical instabilities and such. Scaling factors like 0.001 (millimeters to meters) will involve a determinant of 1e-9, quickly touching the limits of float
precision. Or to put it that way: Checking for det(M)==0.0
likely does not make sense, because a det(M)==1e-20
should probably already be considered as being invalid.
Additionally, I could imagine that some artists might intentionally use scaling factors of 0.0 to let objects "disappear" or to "flatten/project" them somehow. But again: These are not arguments, just thoughts...
Is the decomposing procedure that is described there some sort of "common best practice"? I think that there might be some degrees of freedom or possibly alternative implementations.
I think yes. Some may also decompose sheer/skew transforms but they aren't supported in glTF.
Checking for
det(M)==0.0
likely does not make sense
That check is meant to guard against matrices with exact zero scale (which isn't allowed by the spec).
I could imagine that some artists might intentionally use scaling factors of 0.0
In such case, they should use node.scale
property, not matrix
.
Should we separate obviously broken matrices (invalid last row or zero determinant) into a separate validation error?
I would think yes 👍
We are updating our glTF export code and until now we were ignoring the NODE_MATRIX_NON_TRS error because some of our models are raising it since the decompose-recompose step is not validated (we don't have such constraints when processing uploaded assets). The threshold update allowed us to validate more assets but the error is still raised for some of them.
Here is an example:
{
"asset": {
"version": "2.0"
},
"nodes": [
{
"matrix": [
-1.4063500165939331,
-0.0000035653529266710393,
9.4780273907080215E-11,
0,
0.0000013945755199529231,
-0.55008900165557861,
0.000074632407631725073,
0,
2.028047174640335E-11,
0.000029386374080786481,
1.4063500165939331,
0,
0.0010293800150975585,
-0.085152715444564819,
0.0020093917846679688,
1
],
"name": "Node"
}
],
"scene": 0,
"scenes": [
{
"name": "Scene",
"nodes": [
0
]
}
]
}
Another example with Sketchfab link: https://sketchfab.com/models/2c65b393c6bf4064a1c48423e08f6112
It still makes sense to detect obviously broken matrices. Do you have any news on this ? Thanks
I'm seeing these tolerance-related warnings a lot on models, especially ACCESSOR_NON_UNIT
. I think it is the most common issue I get, even after de-duplicating messages about multiple elements on the same accessor.
With the usual disclaimers that I'm not an expert and only dimly remember my mathematics background, isn't there danger in using an absolute numerical threshold like abs(N - N') < 0.00005
for a matrix that encodes both bounded values (the rotation) and the others two, which will vary arbitrarily depending on scale? A sub-millimetre model will have tiny numbers for scale and translation, whereas a model of a city perhaps 6 orders of magnitude greater.
Disclaimer repeated, isn't this where ideas like abs(N - N') / abs(N) < k * FLT_EPSILON
come in, for some small integer k
? There is more here but my brain explodes around the 70% mark of that blog post.
This may be preemptively over-contemplating things. Also, switching to relative testing might be better for scale and translation, but it could easily be much worse for the rotation part.
@zellski Thanks for the link, I'll return to this issue next week.
Attribute data with NORMAL and TANGENT semantics; animated rotation quaternions
Current rule for triggering error isabs(1.0 - (x * x + y * y + z * z)) > 0.0005
.
This is too strict for normalized vectors stored as BYTEs or UNSIGNED_BYTEs. Empirically about 90% of normalized quaternions don't satisfy this after round-tripping through BYTEs.
import math, random
def mag2(v): return sum(x*x for x in v)
def mag(v): return math.sqrt(mag2(v))
def normalized(v):
m = mag(v)
return [x/m for x in v]
def f2d(x): return round(x * b)
def d2f(x): return max(x / b, -1)
def roundtrip(v): return [d2f(f2d(x)) for x in v]
def error2(v):
v = normalized(v)
return abs(mag2(roundtrip(v)) - 1)
n = 4 # dimension of space
b = 127
trials = 100000
num_bad = 0
for i in range(0, trials):
v = [random.randint(-10000,10000) for t in range(0, n)]
if error2(v) >= 0.0005: num_bad += 1
print((num_bad/trials)*100, '%') # ≈ 91.5%
You can get a theoretically upper bound on the error in round-tripping like this (and this is only the error in round-tripping; it assumes that the original vector is perfectly normalized):
Given a normalized n-vector, if we bump every component as x → x + ε where |ε| ≤ a, then assuming all the ε are maximally bad and there is no cancellation (ie. everything is non-negative), the squared norm changes as Σ x² → Σ (x + a)² = Σ x² + 2 a (Σ x) + n a². The AM-QM inequality gives Σ x ≤ √n so the squared norm changes by at most 2 √n a + n a².
For the error introduced by round-tripping we should have a = 1/2b where b is 127 for BYTE, 255 for UNSIGNED_BYTE, etc.
For quaternions, this gives an upper bound of 2/b + 1/b², or
componentType | b | Bound |
---|---|---|
BYTE | 127 | 0.01581 |
UNSIGNED_BYTE | 255 | 0.00786 |
SHORT | 32767 | 0.00006 |
UNSIGNED_SHORT | 65535 | 0.00003 |
For 3-vectors it gives √3/b + 3/4b², or
componentType | b | Bound |
---|---|---|
BYTE | 127 | 0.01368 |
UNSIGNED_BYTE | 255 | 0.00680 |
SHORT | 32767 | 0.00005 |
UNSIGNED_SHORT | 65535 | 0.00002 |
Empirically, this bound seems to be surprisingly tight. For example, the 3-vector
[0.460878618437258, 0.7047278721433418, 0.539397372344065]
becomes [59, 90, 69] when packed as normalized BYTEs which then unpacks to a vector with squared norm ≈1.01321.
So 0.0005 should be good enough for SHORT and UNSIGNED_SHORT, but it is too small for BYTE and UNSIGNED_BYTE. If you want to use the same threshold for everything, it needs to be > 0.01368.
Could we rely on
length ^ 2
to avoidsqrt
?
To first order √(1+h) ≈ 1 + h/2, which suggests if you want the error threshold on the length to be t to use an error threshold on the squared length of 2t.
Will this issue be address in the near future?
The only unaddressed part here is matrix (only for node.matrix
) decomposability threshold. All other cases were addressed in 2.0.0-dev.3.0
release.
I am having trouble with bone weight normalization expecting the exact json serialization.
Currently, certain validation checks rely on inconsistent and arbitrary-chosen thresholds. We should harmonize them, and optionally add notes to the main spec.
Unit length of certain vectors
Rotation quaternions stored in JSON (
node.rotation
)abs(1.0 - sqrt(x * x + y * y + z * z + w * w)) > 0.000005
.Attribute data with
NORMAL
andTANGENT
semantics; animated rotation quaternionsabs(1.0 - (x * x + y * y + z * z)) > 0.0005
.Questions
length ^ 2
to avoidsqrt
?Error
andWarning
validation severities?Matrices decomposability
Checks
TRS Matrices stored in JSON (
node.transform
)IBM Matrices (
skin.inverseBindMatrices
)Procedures
Matrix validation process
M
intoVector3 translation
,Quaternion rotation
, andVector3 scale
(see below).M'
fromtranslation
,rotation
, andscale
(see below).N
andN'
be infinity norms ofM
andM'
respectively.abs(N - N') < 0.00005
, matrixM
is valid.Matrix decompose process
M[16]
be input matrix written in column-major order.M[3] != 0.0 || M[7] != 0.0 || M[11] != 0.0 || M[15] != 1.0
,determinant(M)
equals0.0
,translation
beVector3(M[12], M[13], M[14])
.sx = sqrt(M[0] * M[0] + M[1] * M[1] + M[2] * M[2])
;sy = sqrt(M[4] * M[4] + M[5] * M[5] + M[6] * M[6])
;sz = sqrt(M[8] * M[8] + M[9] * M[9] + M[10] * M[10])
.determinant(M) < 0
sx = -sx
.scale
beVector3(sx, sy, sz)
.invSx = 1.0 / sx
;invSy = 1.0 / sy
;invSz = 1.0 / sz
.r[9]
be rotation matrix:r[0] = M[0] * invSx
;r[1] = M[1] * invSx
;r[2] = M[2] * invSx
;r[3] = M[4] * invSy
;r[4] = M[5] * invSy
;r[5] = M[6] * invSy
;r[6] = M[8] * invSz
;r[7] = M[9] * invSz
;r[8] = M[10] * invSz
.rotation
be quaternion with rotation from matrixr
.Matrix compose process
Vector3 translation
,Quaternion rotation
, andVector3 scale
be input transforms, andM[16]
be output composed matrix written in column-major order.x = rotation.x
;y = rotation.y
;z = rotation.z
;w = rotation.w
;x2 = x + x
;y2 = y + y
;z2 = z + z
;xx = x * x2
;xy = x * y2
;xz = x * z2
;yy = y * y2
;yz = y * z2
;zz = z * z2
;wx = w * x2
;wy = w * y2
;wz = w * z2
;M[0] = (1.0 - (yy + zz)) * scale.x
;M[1] = (xy + wz) * scale.x
;M[2] = (xz - wy) * scale.x
;M[3] = 0.0
;M[4] = (xy - wz) * scale.y
;M[5] = (1.0 - (xx + zz)) * scale.y
;M[6] = (yz + wx) * scale.y
;M[7] = 0.0
;M[8] = (xz + wy) * scale.z
;M[9] = (yz - wx) * scale.z
;M[10] = (1.0 - (xx + yy)) * scale.z
;M[11] = 0.0
;M[12] = translation.x
;M[13] = translation.y
;M[14] = translation.z
;M[15] = 1.0
.Questions
/cc @pjcozzi @javagl @bghgary @zellski @emackey @donmccurdy