Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formalize expected numerical thresholds #33

Open
lexaknyazev opened this issue Nov 6, 2017 · 16 comments
Open

Formalize expected numerical thresholds #33

lexaknyazev opened this issue Nov 6, 2017 · 16 comments

Comments

@lexaknyazev
Copy link
Member

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)

  • Validator treats those JSON numbers as single-precision floats.
  • Current rule for triggering error is abs(1.0 - sqrt(x * x + y * y + z * z + w * w)) > 0.000005.

Attribute data with NORMAL and TANGENT semantics; animated rotation quaternions

  • Current rule for triggering error is abs(1.0 - (x * x + y * y + z * z)) > 0.0005.

Questions

  • What threshold should we have for unit vectors?
  • Should it be the same for Vec3 and Quaternions?
  • Could we rely on length ^ 2 to avoid sqrt?
  • Should we have two thresholds with Error and Warning validation severities?

Matrices decomposability

Checks

TRS Matrices stored in JSON (node.transform)

  • Validator treats those JSON numbers as single-precision floats.
  • See validation process below.

IBM Matrices (skin.inverseBindMatrices)

  • See validation process below.

Procedures

Matrix validation process

  1. Decompose matrix M into Vector3 translation, Quaternion rotation, and Vector3 scale (see below).
  2. Compose transformation matrix M' from translation, rotation, and scale (see below).
  3. Let N and N' be infinity norms of M and M' respectively.
  4. If abs(N - N') < 0.00005, matrix M is valid.

Matrix decompose process

  1. Let M[16] be input matrix written in column-major order.
  2. If M[3] != 0.0 || M[7] != 0.0 || M[11] != 0.0 || M[15] != 1.0,
    • matrix is indecomposable, exit.
  3. If determinant(M) equals 0.0,
    • matrix is indecomposable, exit.
  4. Let translation be Vector3(M[12], M[13], M[14]).
  5. Let
    • 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]).
  6. If determinant(M) < 0
    • sx = -sx.
  7. Let scale be Vector3(sx, sy, sz).
  8. Let:
    • invSx = 1.0 / sx;
    • invSy = 1.0 / sy;
    • invSz = 1.0 / sz.
  9. Let 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.
  10. Let rotation be quaternion with rotation from matrix r.

Matrix compose process

  1. Let Vector3 translation, Quaternion rotation, and Vector3 scale be input transforms, and M[16] be output composed matrix written in column-major order.
  2. Let
    • 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;
  3. Let
    • 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

  • Should we separate obviously broken matrices (invalid last row or zero determinant) into a separate validation error?
  • Is infinity norm an appropriate metric to compare matrices for this use case?
  • What threshold should we have for it?

/cc @pjcozzi @javagl @bghgary @zellski @emackey @donmccurdy

@emackey
Copy link
Member

emackey commented Nov 6, 2017

I don't know the answers to most of these, but:

Should we have two thresholds with Error and Warning 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.

@emackey
Copy link
Member

emackey commented Nov 6, 2017

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.

@emackey
Copy link
Member

emackey commented Nov 7, 2017

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.

@lexaknyazev
Copy link
Member Author

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.

@emackey
Copy link
Member

emackey commented Nov 7, 2017

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.

@SirFizX
Copy link

SirFizX commented Nov 8, 2017

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.

@javagl
Copy link

javagl commented Nov 11, 2017

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...

@lexaknyazev
Copy link
Member Author

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.

@AurL
Copy link

AurL commented Mar 6, 2018

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

@donmccurdy
Copy link
Contributor

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.

@zellski
Copy link
Contributor

zellski commented Aug 19, 2018

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.

@lexaknyazev
Copy link
Member Author

@zellski Thanks for the link, I'll return to this issue next week.

@scurest
Copy link

scurest commented Oct 13, 2018

Attribute data with NORMAL and TANGENT semantics; animated rotation quaternions
Current rule for triggering error is abs(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 avoid sqrt?

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.

@fire
Copy link

fire commented Feb 14, 2020

Will this issue be address in the near future?

@lexaknyazev
Copy link
Member Author

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.

@fire
Copy link

fire commented Feb 14, 2020

I am having trouble with bone weight normalization expecting the exact json serialization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants