playcanvas / engine

JavaScript game engine built on WebGL, WebGPU, WebXR and glTF
https://playcanvas.com
MIT License
9.57k stars 1.34k forks source link

Architectural Issue: The skin is defined within the mesh. #5680

Open querielo opened 12 months ago

querielo commented 12 months ago

Currently, it's not possible to instantiate multiple skins on one mesh. It seems that the Mesh shouldn't contain the skin (Mesh.skin)

I believe that the Mesh and Skin should be in a many-to-many relationship (Mesh has many data, Skin has many data) and one shouldn't be stored inside the other to avoid excessive resource allocation when we need to copy or reuse one without other. Note that the information about Skin in MeshInstance is delegated to SkinInstance. This means, through skinInstance and mesh in MeshInstance, we can ideally maintain a many-to-many relationship, rather than storing Skin in Mesh.

What problems does this cause?

  1. The current implementation requires that if we have two nodes with the same mesh and these nodes have unrelated skeletons, we need to create two Meshes. Otherwise, the skinning/animation of the mesh on the second node will be incorrect. Cloning a mesh is resource-intensive, and it requires tracking more allocated resources. Note: the engine does not provide API to clone a mesh.
  2. I believe the PlayCanvas architecture is very compatible with glTF. According to its specification glTF 2.0 Specification

    A skin is instantiated within a node using a combination of the node’s mesh and skin properties. The mesh for a skin instance is defined in the mesh property. The skin property contains the index of the skin to instance.

This means that the mesh and skin can be instantiated in different combinations in a node, which in the current architecture can only be implemented by cloning the mesh (rather than reusing it). Here is the issue place in glb-parser.js

querielo commented 12 months ago

possibly related to https://github.com/playcanvas/engine/issues/5329

mvaligursky commented 12 months ago

I'm not sure I understand this, but I'm not sure it is correct what it is saying.

You can definitely have a single mesh, and have it twice in the scene, each being animated by a separate skeleton, Is that what you need?

querielo commented 12 months ago

@mvaligursky Yes.

Example: https://playcanvas.com/model-viewer?load=https://necrosipov.b-cdn.net/playcanvas/SampleScene.glb

https://github.com/playcanvas/engine/assets/104348270/454823d5-cc37-4175-be33-7e23dabac10d

querielo commented 12 months ago

Expected:

https://github.com/playcanvas/engine/assets/104348270/223e1601-1723-42c2-a50f-f5bee788326d

querielo commented 12 months ago

NOTE: tried to disable https://github.com/playcanvas/engine/blob/e5fa96b2411bc345478d873914f9dca3413a85c8/src/framework/parsers/glb-parser.js#L665 It doesn't help.

mvaligursky commented 12 months ago

Have you tried a single skinned character in the glb. Import it to the Editor, and place the template twice in the scene. Attach the anim component to both hierarchies, set up rootBone of the render component to point to those skeletons, and play two different animations - that should work fine. There will be a single mesh and a single skin in memory, and two mesh instances and two skin instances.

querielo commented 12 months ago

The main problem is that at the stage of the glb-parser we lose information about actual bones of skinned meshes. This is because at least the parser assumes that each mesh is in a one-to-one/many-to-one relationship with a skin.

Thanks for your suggestion. In this case there will be different skin instances. But "Import it to the Editor" contradicts with our assets preparation pipeline.

Our current workaround is:

Screenshot 2023-09-27 at 00 47 05

implemented after a .glb-file is parsed. The idea is to call _clearSkinInstances and _cloneSkinInstances with correct rootBone. But in general it is not correct.

It won't work for all possible cases but works for us.