GLTFLoader: add getDependency( type, index ) implementation

[hybridherbst]

Description

@takahirox requested that the getDependency('light') implementation should be a separate PR, so here it is. This is in preparation for supporting the KHR_animation_pointer glTF extension in three.js.

This PR adds a new way for extensions to implement getDependency( type, index ) for custom types; targets for implementation are right now light and audio.

Related:

• #24193
• #24108

[takahirox]

No conlusion yet but let me share what I'm worried about so far. (It would be better than no response for long.)

I'm thinking whether it is good to let getDependency() in the core glTF loader know the light extension handler. Especially, ._loadLight() of GLTFLightsExtension is expected to be a private method. Ideally the core glTF loader doesn't need to know extensions and their handlers for simplicity.

Perhaps the root issue may be the current plugin system doesn't expect that an extension has dependency with other extensions....

[hybridherbst]

Yes, with KHR_animation_pointer an extension certainly has dependencies to other extensions.

I think _loadLight being private (having that underscore) doesn't match how the other extensions do that anyways: the extension methods loadAnimation, loadTexture, loadMaterial etc. all are public.

I'm happy to change _loadLight to loadLight so that these are more consistent to each other.

[robertlong]

I think it might be better to make the glTF plugins expose an optional getDependency(type, index) function which can be used in the default case of the switch statement in getDependency. This would mean any extension could expose its own resources to be used by other extensions or getDependency can be invoked by the user directly.

For example in KHR_audio you could call

getDependency("audioEmitter", 0)

or

getDependency("audioSource", 0)

And the default loader doesn't need to know anything about audioEmitters or audioSources.

[takahirox]

Like this?

// GLTFParser
getDependency( type, index ) {
  const cacheKey = type + ':' + index;
  let dependency = this.cache.get( cacheKey );
  if ( ! dependency ) {
    switch ( type ) {
      ...
      default:
        dependency = this._invokeOne( function ( ext ) {
          return ext !== this && ext.getDependency && ext.getDependency( type, index );
        } );
        if ( ! dependency ) {
          throw new Error( 'Unknown type: ' + type );
        }
        break;
    }
    ...
  }
  ...
}

[robertlong]

Yeah, exactly 👍

[takahirox]

Maybe that sounds good.

Nit: parser.getDependency(type, index) calls .loadXXX(index) that returns the dependency as promise, caches it, and returns it. For consistency, perhaps plugin should have .loadXXX(index) (or .load(type, index)) methods.

// GLTFParser
getDependency( type, index ) {
  const cacheKey = type + ':' + index;
  let dependency = this.cache.get( cacheKey );
  if ( ! dependency ) {
    switch ( type ) {
      ...
      default:
        const functionName = 'load' + type[0].toUpperCase() + type.slice(1); // ugh...
        dependency = this._invokeOne( function ( ext ) {
          return ext[functionName] && ext.[functionName]( index );
          // or return ext.load && ext.load( type, index ); ?
        } );
        if ( ! dependency ) {
          throw new Error( 'Unknown type: ' + type );
        }
        break;
    }
    ...
  }
  ...
}

[hybridherbst]

I rebased this on dev and implemented the suggestions from here – let me know if that works, thanks @takahirox!

Regarding the .loadXXX approach, not sure if the potential added clarity due to that offsets that weird uppercasing-and-splicing...