Adjust Object Model for morph targets

[lexaknyazev]

Fixes #2549.

/cc @aaronfranke @hybridherbst @robertdorn83

[javagl]

Looks fine for me in view of the discussion that happened in the linked issue (several people seemed to agree that this is the way to go). I'm not deeply involved in implementations of the object model, so while I can 'Approve' this, it could be good to wait for a second pair of eyes before actually merging.

[aaronfranke]

I'm curious about the intended use cases for weights on the mesh objects themselves. I know the spec says "These weights MUST be used when node.weights is undefined", but are these default values expected to be copied, or are these intended to keep applying if the mesh weights changed somehow? Regardless of the answer to this question, in practice it would be tricky for implementations to deal with mesh weights, so I am in favor of requiring that the object model only allows node weights.

[lexaknyazev]

but are these default values expected to be copied, or are these intended to keep applying if the mesh weights changed somehow?

With this PR, the mesh.weights values are no longer accessible let alone mutable so they should not ever change. Their intended use cases are not very strong though:

• More explicit signalling of a mesh's default state, e.g., when all-zero weights are not the intended default look.
• Potential JSON deduplication if a mesh is used multiple times with the same weights and they are not animated.

[bghgary]

I can't say I've look at the issues enough to understand the nuances, but we did have issues implementing this in Babylon.js that are still not resolved, so making this simpler is good. Do we have samples/examples that we can use to test this?

[lexaknyazev]

Do we have samples/examples that we can use to test this?

I don't think we ever had sample assets with /meshes/{}/weights pointers. That said we need to have the following test assets (they shouldn't block the PR though).

For KHR_animation_pointer

All these assets must behave the same when the animation is active:

• An asset that uses a /nodes/{}/weights animation pointer on a node with a mesh that has morph targets and does not have any static weights.
• An asset that uses a /nodes/{}/weights animation pointer on a node with a mesh that has morph targets and only mesh.weights static weights.
• An asset that uses a /nodes/{}/weights animation pointer on a node with a mesh that has morph targets and only node.weights static weights.
• An asset that uses a /nodes/{}/weights animation pointer on a node with a mesh that has morph targets and both mesh.weights and node.weights static weights.

For KHR_interactivity

This list is not exhaustive but it's a good start.

1. A graph that reads /nodes/{}/weights.length from a node that has no mesh to confirm that the pointer is accepted and returns 0.
2. A graph that reads /nodes/{}/weights.length from a node that has a mesh with no morph targets to confirm that the pointer is accepted and returns 0.
3. A graph that reads /nodes/{}/weights.length from a node that has a mesh with morph targets and no static weights to confirm that the pointer is accepted and returns the number of morph targets.
4. A graph that reads /nodes/{}/weights/{} from a node that has no mesh to confirm that the pointer is rejected.
5. A graph that reads /nodes/{}/weights/{} from a node that has a mesh with no morph targets to confirm that the pointer is rejected.
6. A graph that reads /nodes/{}/weights/{} from a node that has a mesh with morph targets and no static weights to confirm that the pointer is accepted and returns all zeros.
7. A graph that reads /nodes/{}/weights/{} from a node that has a mesh with morph targets and static weights defined only in mesh.weights to confirm that the pointer is accepted and returns the weights specified.
8. A graph that reads /nodes/{}/weights/{} from a node that has a mesh with morph targets and static weights defined only in node.weights to confirm that the pointer is accepted and returns the weights specified.
9. A graph that reads /nodes/{}/weights/{} from a node that has a mesh with morph targets and static weights defined both in mesh.weights and node.weights to confirm that the pointer is accepted and returns the weights specified in node.weights.

[bghgary]

I don't think we ever had sample assets with /meshes/{}/weights pointers. That said we need to have the following test assets (they shouldn't block the PR though).

I think it's harder to review the changes without actually trying to implement the changes. We should have some samples to be confident with the implementation.

[lexaknyazev]

This PR removes an under-defined and previously untested part of the Object Model scope.

[bghgary]

That's fair.

[robertdorn83]

@lexaknyazev I have some interactivity tests created based on your list.
Can be found in the interactivity test repo: Weight Tests

[lexaknyazev]

@robertdorn83 I think the first test is incorrect: /nodes/{}/weights.length should be valid even without a mesh because the spec says:

All pointers named *.length return zero if the corresponding array is not defined.

So /nodes/{}/weights.length may be invalid only if the node index does not exist.

[robertdorn83]

@robertdorn83 I think the first test is incorrect: /nodes/{}/weights.length should be valid even without a mesh because the spec says:

All pointers named *.length return zero if the corresponding array is not defined.

So /nodes/{}/weights.length may be invalid only if the node index does not exist.

have changed it!