Fix normalize weights

[Bolloxim]

Fixed an issue with badly weighted GLTF models. (see attached tree) On reviewing the weights it was pretty obvious the weights were all off and even had some set to 0,0,0,0 which is pretty bad data. however on correcting for this by renormalizing the mesh (did this in a playground test, script attached). So this has been added now to babylon.mesh.ts. I noticed there was a cleanMatrixWeights I test this but it did not clean up the model. However I added functions after this. It should handle extra weights fine but I've no test data to attempt it. I split the functionality into two private functions so that its much cleaner if something goes awry its self contained .
Tougher implementation was adding this call to the GLTFLoader I put this in a promise right after we bind the gometry to the mesh and then I process renormalization. It works in sandbox and playground. modes I tested both. I had to update manually declaration modules in preview release to get loaders to rebuild. This likely needs some review but it does clean up nicely. i will open an issue for supporting weight / influence debugging and optimizations to reduce large influence counts, which I will like implement at somepoint.

tree-fix.js.txt
Tree1.gltf.zip

[deltakosh]

I think this should be being a boolean to make sure we do not slow loading down for users having correct gltf
Adding @bghgary for final review

Only one question: what was wrong with the current cleanMatricesWeights function?

[bghgary]

I think this should be being a boolean to make sure we do not slow loading down for users having correct gltf

I would prefer if this kind of fix up stays out of the loader completely as it is a requirement that the weights in the glTF asset are normalized.

At the very bottom of the skins section in the spec:

The joint weights for each vertex must be >= 0, and normalized to have a linear sum of one. No joint may have more than one non-zero weight for a given vertex.

Users can call these fix up functions after the load is complete if necessary.

[Bolloxim]

Agreed on what the khronos spec says, looks like all exporters have not adhered to the spec. I read through them yesterday looking to make sure.

So no boolean in the loader. I'll rework it so that there is a mesh validator in mesh that can 'test' geometry and flag it as broken rather than just changing the data. That way it can flag it for re-export and normalization. Plus we only patch data then for those that need it. sound good ?

as for cleanMatrixCode caused geometry to map to another point not within the tree but off to the edge and all broken geometry kind of morphed into that point. I did not review the code though as I thought it did a bit more than normalization, I came across it in the mesh when looking for a suitable place to move it too.

It is also worth noting that a diagnostics pass on the weights could lend to using optimized shaders for 2 and 3 influences. We could spit that out in the validator phase.

lastly should we enable mesh validation and patching in the sandbox viewer by default ? That way assets look clean and the view can give artists immediate feedback on an exported model ?

[Bolloxim]

removed the GLTFloader changes. Created a new validateSkinning function that does the diagnostics of weight counts influences etc..should be useful for the viewer.

example of how I used it in the playground code.

      var skinReporting = tree.validateSkinning();
      if (skinReporting.valid==false) tree.normalizeSkinWeights();
      console.log("Skin Validation Report:\n" + skinReporting.report);

Here is the output from running it, I check for sorted which is useful for dynamic skinning especially at distance dropping to two influences for example. They should be sorted output, which was the case in the broken model. it also validates the bone indices are in range. If bone indices are out of range it generates an invalid report

Skin Validation Report:
Number of Weights = 3566
Maximum influences = 4
Missing Weights = 246
Not Sorted = 0
Not Normalized = 3241
WeightCounts = [246,1984,438,633,265]
Number of bones = 8
Bad Bone Indices = 0

[Bolloxim]

I figured out the reason that cleanMatrixWeights did not work on this model. The noBoneInfluece index is used to set for models with no weights therefore although it has a weighted value of 1-epsilon it is using the base transform node for this model which is translated off the origin basically stretched all those vertices. Where as the normalizeSkinWeights uses the root(0) index and that had the benefit of keeping the geometry relative to the root node and does not modify the indices.

[deltakosh]

So I think your technique is better. Would you mind replacing your code into the cleanMatricesWeights function? So we don't end up with two functions doing the same thing

[Bolloxim]

Updated the whats new doc to reflect the modification to cleanMatrixWeights. Seperated out the validate skin line. I am looking to update the validator in viewer for GLTF to use this. In the viewer on error detection shall we auto correct or add a clean button ? or both ? validator might have a json output option just to make merging in to the gltf validator simple.

[deltakosh]

Just some tiny changes

[deltakosh]

Regarding autocorrect I'm not sure we want to pay the validation cost each time we load a mesh. This has to be a developer decision.
But this is an excellent candidate to go into the inspector. When a mesh is selected we could add a validate/fix bone weights button in the ui

[bghgary]

For glTF specifically, the glTF validator will eventually have support for validating skins and the sandbox will automatically pick up these errors when loading a glTF file.

[Bolloxim]

David, I was intending to mean as part of validator when it validates the skin. As specified before auto patching should not be required per mesh but the viewer tool should be able to resolve errors I believe automatically without adding a general overhead to the systems. I reviewed the inspector code last night. Couldnt quite figure out the correct location to make a call validate mesh that is only in the validator callback and not in the mesh loader. I will be looking at that more today. Feel free to push me in the right direction here :)

[deltakosh]

Will merge as soon as I arrive at work. Thanks a lot
For the viewer that could be definitely an option ( off by default)

[deltakosh]

Reciprocal