EXT_mesh_gpu_instancing: Clarify mesh restriction

[zeux]

We now call out mesh requirement more explicitly , and additionally disallow skin specification, as skinning doesn't depend on the mesh transform and instancing doesn't allow to override joint transforms per index.

[lexaknyazev]

Normative language changes after ratification are generally discouraged. We should get some feedback from implementors before merging this.

/cc @DRx3D

[zeux]

I can replace the second (or both?) with something weaker like should, I mostly want to make sure the incompatibility with skinning is explicitly called out.

[lexaknyazev]

That incompatibility was undefined before therefore making it explicit is a potentially breaking change. We should check what current implementations do.

[donmccurdy]

For one implementation — three.js does not, as yet, support GPU instancing in combination with skinning. We assume that the node with KHR_mesh_gpu_instancing has a mesh, so no concerns here.

Similarly, these changes are compatible with the implementation in glTF Transform.

[zeux]

I updated the skinning clause for now to say "should not". While I think there's no consistent implementation possible, I don't need this to be enforced strictly. But regardless of existing implementations, it's a logical contradiction so I assume every existing implementation either just happens to not actually implement skinning, or renders every instance at the same location.

[zeux]

Please let me know if any further changes are needed on this PR (or if there are no plans to merge it).

[emackey]

For what it's worth, skinning does appear to work in BabylonJS and CesiumJS, and in my company's software. The general consensus appears to be that the node transforms are ignored per usual glTF behavior, but the instance transforms are applied after the skinning. So, copies of an animated skinned mesh can appear in different locations and orientations.

Here's a sample with instances of the animated Fox model: FoxInstances.zip

I suppose this could be disallowed, but it seems a nice feature to have. One could use it for large animated crowds, for example.

[Off-topic: I really wish COLOR_0 could appear in the instance attributes, to give each instance its own color multiplier. Alas.]

[zeux]

Ah, interesting. I was assuming that it would be logical to ignore the instance transform since it’s structurally equivalent to node transform but if you assume skinning only ignores node transform and object transform is independent then I could see this combination being sensible and working. This is a point of ambiguity that needs to be clarified though…

[zeux]

FWIW the attached example doesn't render correctly in three.js: it uses separate object types for skinned meshes & instanced meshes, and in this instance it selects InstancedMesh so the skinning deformation is lost and the scene renders like this:

Compared to this from Babylon.JS:

But if we agree that Babylon's interpretation is correct we could maybe classify three's behavior as a bug? Since the behavior wasn't specified either way and the extension is marked as complete & ratified I'm not sure what the desirable path forward is here -- my initial inclination was to disallow/discourage this per the edit in this PR but maybe other options are preferable.

[emackey]

Since the behavior wasn't specified either way and the extension is marked as complete & ratified I'm not sure what the desirable path forward is here

If we add new "intellectual property" to it as part of a fix, we would need to re-ratify. But I don't think any new IP is needed to clarify the skinned behavior, we either discourage it or we specify that the instance transforms apply after skinning.

My inclination is biased towards including skinning, but that may be because I went to the trouble of figuring out how to make it work in my renderer. Basically the vertex shader just does the normal skinning thing before applying the instance transform. I wonder if that ThreeJS screenshot is showing the opposite order, where an instance transform has been applied first and broken the binding to the joints.

The properly-instanced foxes screenshot gives a hint of some possibilities here. I could imagine a parade of folks marching in uniform, or a crowd of people with different orientations walking different directions. There are serious limitations on variability of course, but one could mix together a handful of instanced groups.

[zeux]

I wonder if that ThreeJS screenshot is showing the opposite order, where an instance transform has been applied first and broken the binding to the joints.

The skinning is just disabled, but the root joint here has a 0.1 scale so if you don't include that, the foxes overlap significantly.

I should mention that I need to look further into what Babylon.JS is doing here but I think it might be getting the order wrong somewhere. This is based on me trying glTF-Transform and gltfpack quantization on the example above, and that currently changes the rendered result in ways that makes me suspect the code there isn't doing the right thing. (also on the original unquantized scene the bounding box for the example isn't quite correct either).

I would feel better arguing for clarifying this in the direction of enabling this, as it does seem useful, if I saw multiple implementations aligned on the behavior. I haven't checked Cesium yet or understood the deviation that I'm seeing so I will update this once I do.

Ok I double checked and I think Babylon.JS and Cesium are in fact consistent with each other and glTF-Transform does work with quantization, the only thing that seems to change is the bounding box because presumably it's not computed correctly for this combination, and that changes the view framing.

[zeux]

Wrt ratification I meant compatibility moreso than ratification requirements: even though the original extension should have made this clear and didn't, updating it to specify the skinning to be compatible would retroactively make existing (arguably conformant?) implementations like Three non-conformant; but this comes at a cost of rejecting a useful behavior. By comparison, updating the extension to specify skinning to be incompatible potentially makes existing assets that worked in some renderers invalid. I thought these don't exist in the wild but am now not sure (clearly this thread has at least one!), so I can see both sides.

After thinking about this I think it would make sense to tackle skinning as a separate PR. I was originally intending to clarify both mesh & skin restrictions in one PR because I assumed (incorrectly) that skinning is fundamentally incompatible due to the instance transform being "part of" the node hierarchy, and the combination thus making no sense for any renderer, but since there's already renderers that implement the combination in the most useful way, but others don't, I think it would make sense to decide that in a separate issue and then make a PR with the results of the decision.

I updated the PR accordingly to just leave the correct language for the required presence of the mesh node.

[lexaknyazev]

The new language is more restrictive that the original. Do current engines reject assets that use the extension on mesh-less nodes? If not, it would be safer to standardize on ignoring the extension in such cases.

[zeux]

I think three/babylon ignore the nodes without a mesh, but that combination is non-sensical. I'm not interested in standardizing something like this, and just in general I stopped understanding the policies of extension updates when the original extension is underspecified, so I'll just close this and let this rest.