SkinnedMesh: Fix a vertex warping issue under negative weight

[0b5vr]

I'm curretnly working on glTF stuff using THREE.GLTFLoader.
Sometimes SkinnedMesh accidentally has negative bone weights, and such models does weird behavior on Three.js, while most of other environments are not.
It's since Three.js is using 1.0 / Vector4.manhattanLength() when normalizing these weights.
I know negative weights are generally invalid in most of situation, but it seems most of applications has different procedure from calculating manhattan length: just using sum of all weights.

• Reference, how other environment (Babylon.JS) deals with skinning weight:
  https://github.com/BabylonJS/Babylon.js/blob/405068971daa526cf42d19eb80b7c8e3cb2dd8a4/src/Mesh/babylon.mesh.ts#L1593

• Here's a discussion about skin weights that happened in glTF spec repo
  Are skin weights normalized? KhronosGroup/glTF#1213

[donmccurdy]

Negative weights in glTF are definitely invalid — from the conclusion of that thread, and the spec:

The joint weights for each vertex must be >= 0, and normalized to have a linear sum of one.

If there is a tool creating negative weights, I'd recommend filing a bug about that.

Are you trying to get a certain behavior here? With or without this change, it doesn't seem like the result will be particularly meaningful.

[0b5vr]

If there is a tool creating negative weights, I'd recommend filing a bug about that.

Yeah definitely, I know (and honestly it tried to prevent me from issue this pr), but in either way the current way how Three.js deals with such situation doesn't match to other "standard" idea. 😥

[Mugen87]

The formula for the manhatten length is correct since it's the sum of the absolute differences of their coordinates and I think it's valid to use it in context of skin weights.

I do not vote to merge this PR.

[0b5vr]

The formula for the manhatten length is correct

No, there are no correct way to deal with negative weights since negative weights are invalid

In anyway I don't get the current way how we're dealing with negative bones, no validations AND different result from other environments, I got really confused by this behavior yesterday (I took a full day to solve this).

...I'm beginning to be embarrassed about starting this discussion as a pr 😅

[Mugen87]

No, there are no correct way to deal with negative weights since negative weights are invalid

I've referred to the manhatten length. It's formula is definitely correct (see https://en.wikipedia.org/wiki/Taxicab_geometry).

[donmccurdy]

@fms-cat No need to feel embarrassed, it's a good question and we've had to clarify the glTF spec on this before. I think your point that there is no correct way to deal with negative weights is right. I prefer the simplicity of using the existing manhattanLength method, since what other tools do here is no more correct than our solution. Arguably the most "correct" thing to do would be to throw an error.

When you test the model on https://gltf-viewer.donmccurdy.com/ or http://github.khronos.org/glTF-Validator/, do you see a warning about the negative weights? If not, I think the best step would be to ensure these tools show a warning.

[donmccurdy]

Thanks for finding the issue for the PR, and sorry it was more complicated than expected. :)

[0b5vr]

@donmccurdy Thanks too! As you said it's more than I expected 😖