Track glTF associations for node, material and texture

[cdata]

This change proposes keeping track of some associations between glTF dependencies and the Three.js instances that are created from them.

The use case addressed by this change is described in more detail in #19257. After discussion with @donmccurdy , it was agreed that we should start with nodes and materials. In pursuing this change, I realized that textures have the same ambiguity case as materials, so I also included textures in this change.

Thank you for considering this change!

Fixes #19257

[cdata]

Rebased to pick up some upstream changes.

[donmccurdy]

This structure looks good to me, and complements the getDependency() API and upcoming plugin API well, I think. Any concerns @takahirox?

[donmccurdy]

nit: this line seems a bit out of place in the middle of the nodeDef assignments, perhaps move it below the matrix/TRS block?

[cdata]

Done

[mrdoob]

I'll merge this for now. If @takahirox has any concerns we can address them in other PRs.

[mrdoob]

Thanks!

[mrdoob]

@donmccurdy Synced the js file: 475efec

[takahirox]

Sorry I didn't notice the review request for a while. Thanks @mrdoob for notifying me.

Unfortunately it's very difficult for me to read through the long discussion, but it sounds good to me to provide a way to get a corresponding index in glTF.

I haven't thought deeply yet but perhaps it'd be ok for the new extension system. If we see any problems while we will be moving forward the new system, let's discuss then.

One minor thing is whether we should clear the association map in parse() like we do for cache. Would it be worth to keep the map even after parsing another glTF file? (Currently we allow to parse different glTF files in one parser. Limiting one glTF file per one parser might be another option, it could be less problematic inside and be clearer for users to use?)

[donmccurdy]

loader.parse() will always create a new GLTFParser. In my mind that is the public API, and parser.parse() (which clears the cache) is a private API and should never be externally used. Now that the parser is accessible in more places, maybe we should prefix its private methods with _?

[takahirox]

I agree with clarifying accessibilities (public or private). And making parser.parse() private would sound good to me. Public parser.parse() can make the design inside complex. For example if user calls parser.parse() before completing the previous parsing can have an effect to the previous parsing, like the following example code. Making it private (one parser per one glTF file) is reasonable to me. And _ prefix sounds reasonable to me.

parser.parse(url1).then(gltf => {
   ...
});

// calling .parse() before completing parsing the first one
// can unexpectedly clears the cache.
parser.parse(url2).then(gltf => {
   ...
});

The new extension system will expose parser instance to extension devs. So I'd like to discuss in the detail about what methods should be public and how we make the others private while we will be moving it forward. And I'd like to hear other devs opinions because this PR is already closed.

[takahirox]

This structure looks good to me, and complements the getDependency() API and upcoming plugin API well, I think

Yeah I realized it complements the plugin system well while I was writing a plugin in which I wanted to find a glTF definition associated with a certain already generated Three.js object.

I'm thinking if we can add Mesh associations to the structure. In most cases currently associated gltf.mesh(.primitive) can be found from a Three.js Mesh by first finding a node (Mesh itself or its parents) and an associated glTF node definition, and then getting glTF mesh definition with node.mesh.

But sometimes finding would be difficult. For example a glTF node definition has a mesh property and also an extension which produces another mesh. A Three.js Mesh can't be (easily) detected which it is made from node.mesh or node.extension.mesh.

What do you think of adding Mesh associations to the structure?

[donmccurdy]

That complexity is why it was omitted I think, yes. Or for another example, would a glTF mesh with 4 primitives mean the associations mapping has 4 THREE.Mesh instances mapped to the same glTF mesh? Or something else? It isn't particularly easy. :/

I'm OK with adding that if we have clear use cases to help us design it usefully.

[takahirox]

GLTFLoader generates a Mesh instance per glTF primitive so I'm thinking of adding primitiveIndex for meshes like

{
  types: 'meshes',
  index: meshIndex,
  primitiveIndex; primitiveIndex
}

Then in your example, the four Mesh instances will map to the same glTF mesh but different glTF primitive.

A problem, if we add mesh association, is that Mesh instance can be mapped to both glTF node and glTF mesh. So I would like to suggest to change association data structure to an object whose key is gltf type like

from

{
  type: 'nodes',
  index: nodeIndex
}

to

{
  node: {
    index: nodeIndex
  }
}

The latter structure can store multiple mappings.

Regarding use case, my use case is traverse scenes in afterParse(), find Meshes whose associated glTF mesh.primitive has certain extension (which is "known" so it isn't saved under object.userData.gtfExtensions), and process additional work for such Meshes. I first thought of using loadMesh() hookpoint but it's invokeOne. I didn't want to block applying glTF Mesh extension and didn't want other Mesh extension plugins to block me so I couldn't use.

And I found another problem of finding associated glTF mesh or primitive from Three.js Mesh. Currently finding them relies on scene graph so if user or plugin changes the graph, the associated glTF mesh or primitive is very difficult to be found.