GLTFLoader: Introduce plugin system

[takahirox]

From: #11682

This PR adds extensibility to GLTFLoader. It adds puglin API and rewrites the existing extension handlers. I'm not sure if the API and implementation are good yet. So opened as draft PR. Feedback is very welcome.

Benefits

• Simplifying GLTFParser by separating extension handlers from core GLTFParser
• Easiness to add new or custom extension handlers without messing GLTFParser
• Easiness to reuse even custom extension handlers

API

// I call handler plugin so far

class MyExtensionHandler {
  constructor () {
    // If extension is defined, the handler is for an gltfDef.extensions,
    // it's called only for gltfDef.extensions[ extension ] is defined.
    // If extension is empty strings or undefined, the handler is for generic,
    // it's called regardless of gltfDef.extensions.
    this.extension = 'EXT_foo';
  }

  // If onBeforeXXX method is defined,
  // it is called before parser.loadXXX()
  // Currently only GLTF (entire gltf def) supports onBeforeXXX
  // @param {GLTF.definition} def
  // @param {parser} GLTFParser
  // @return {Promise<Object>}
  onBeforeXXX (def, parser) {
    // override def
    return Promise.resolve(def);
  }

  // If onXXX method is defined
  // it is called in parser.loadXXX() to create an XXX instance
  // on behalf of entire or part of parser.loadXXX().
  // Currently only Texture, Material, Geometry and Node,
  // needed for existing extension handlers, support onXXX.
  // We should support all XXX later.
  // @param {GLTF.definition} def
  // @param {parser} GLTFParser
  // @return {Promise<Object>|null}
  onXXX (def, parser) {
    var object = new XXX();
    // set up object
    return Promise.resolve(object);
    // return null if you don't want to create instance because of certain reasons.
    // In such a case, next plugin is called, 
    // or `GLTFParser.loadXXX()` creates if no more plugins registered
  }

  // If onAfterXXX method is defined
  // it is called after parser.loadXXX().
  // In addition to XXX of all .loadXXX(),
  // Map for material.fooTexture.extensions support onAfterXXX.
  // @param {Object} object
  // @param {GLTF.definition} def
  // @param {parser} GLTFParser
  // @return {Promise<Object>}
  onAfterXXX (object, def, parser) {
    // override object properties or 
    // creating a new object
    return Promise.resolve(object);
  }
}

const loader = new GLTFLoader();
loader.registerPlugin(new MyExtensionHandler());
loader.load( ... );

Live Demo

https://raw.githack.com/takahirox/three.js/GLTFLoaderPluginSystem/examples/webgl_loader_gltf_extensions.html

Notes

• I implemented the existing extension handlers into GLTFLoader.js and registered as default because (a.) most of them are popular extensions and I want users to avoid import separated plugin files and (b.) some of them, for example Draco extension, rely on some logics implemented in GLTFLoader.js and I couldn't clearly separate.
• Maybe sounds good to keep adding KHR_ extensions in GLTFLoader.js and adding separated plugin files for other extensions into examples/js(m)/loaders/gltfPlugins/ or somewhere if needed?
• If this PR is too huge, maybe I can break it up into some PRs. For example one PR for one extension handler.
• This PR includes the cleaned up change in GLTFLoader: Remove onBeforeRender dependency #14099, thanks @pailhead. Maybe better to merge this PR after merging GLTFLoader: Remove onBeforeRender dependency #14099 as respect to him if we decide to go with this change?
• (Update) Regarding onXXX, I implemented only Texture, Material, Geometry and Node in this draft PR for now because I first wanted to show the API and how the existing handlers can be rewritten. I'm thinking to add others in this or other PRs later. Regarding onBeforeXXX, I had a comment and I'm not sure yet if we really need. So I implemented only GLTF so far because I only see the request that they need "before" hook point for entire gltf.

[zeux]

From my perspective we would want to keep existing extensions registered by default to aid in usability, the goal would be to make it easier to add support for new extensions without modifying three.js - but I am hoping that we will continue the current practice of adding extensions that are supported “by default” without the user having to opt-in.

[pailhead]

It’d be nice if I could clean up #14099. Three day weekend coming up so could be in the next couple of days.

[zeux]

As an experiment I've decided to use this to try to implement MESHOPT_compression (draft extension spec: KhronosGroup/glTF#1702).

To implement this extension, I need to:

1. Intercept attempts to load bufferView objects
2. If the bufferView object contains the extension MESHOPT_compression, I need to load the buffer using extensions["MESHOPT_compression"].buffer as an index
3. Once that is finished, I need to process the buffer and return a typed array buffer.

I was able to successfully implement this like this:

function GLTFMeshoptCompressionExtension(decoder) {
	this.extension = "MESHOPT_compression";
	this.decoder = decoder;
}

GLTFMeshoptCompressionExtension.prototype = {

	constructor: GLTFMeshoptCompressionExtension,

	onAfterBufferView: function ( bufferOriginal, bufferViewDef, parser ) {

		if ( bufferViewDef.extensions === undefined || bufferViewDef.extensions[this.extension] === undefined ) {
			return bufferOriginal;
		}

		var extensionDef = bufferViewDef.extensions[this.extension];
		var decoder = this.decoder;
		var buffer = parser.loadBuffer(extensionDef.buffer);

		return Promise.all([buffer, decoder.ready]).then( function (results) {

			var buffer = results[0];

			var byteOffset = extensionDef.byteOffset || 0;
			var byteLength = extensionDef.byteLength || 0;

			var count = extensionDef.count;
			var stride = extensionDef.byteStride;

			var result = new ArrayBuffer(count * stride);
			var source = new Uint8Array(buffer, byteOffset, byteLength);

			switch ( extensionDef.mode ) {

				case 0:
				decoder.decodeVertexBuffer(new Uint8Array(result), count, stride, source);
				break;

				case 1:
				decoder.decodeIndexBuffer(new Uint8Array(result), count, stride, source);
				break;

				default:
				throw new Error( 'THREE.GLTFLoader: Unrecognized meshopt compression mode.' );

			}

			return result;

		} );

	},
};

One issue that I am running into however is that I am able to add a function call after bufferView gets loaded by overriding onAfterBufferView, but not before. This is not great because this means that the original fallback buffer view is loaded which is suboptimal (may result in pulling in additional resources). Ideally I would like to be able to replace buffer view loading instead of running code after the fact. I tried overriding onBeforeBufferView and onBufferView but that doesn't seem to work.

[takahirox]

Thanks for the comments and trial. And sorry, I didn't write clearly.

Regarding onXXX, I implemented only Texture, Material, Geometry and Node in this draft PR for now because I first wanted to show the API and how the existing handlers can be rewritten. I'm thinking to add others in this or other PRs later. In your case, perhaps the handler works with onBufferView as you want when it's added.

Regarding onBeforeXXX, I had a comment and I'm not sure yet if we really need. So I implemented only GLTF so far because I only see the request that they need "before" hook point for entire gltf.

[zeux]

@takahirox Thanks! I've added support for onBufferView which was just a couple of lines in loadBufferView and this made it possible for me to add a complete and correct implementation of the extension I mentioned. 👍

[donmccurdy]

From my perspective we would want to keep existing extensions registered by default to aid in usability...

Agreed (although I'd like to remove MSFT_texture_dds at some point...).

But I am hoping that we will continue the current practice of adding extensions that are supported “by default” without the user having to opt-in.

Yes, for KHR_ extensions at least, others on a case-by-case basis.

It’d be nice if I could clean up #14099. Three day weekend coming up so could be in the next couple of days.

Seems like more work for you if it's already incorporated in this PR? Maybe @takahirox should just submit it with extra indentation, and then you can fix that to claim the git blame. 😉

[donmccurdy]

Just curious — is it possible to put all the hooks into onDependency? E.g...

case 'node':
  dependency = this._onBefore( type, index )
    .then(( def ) => this._on( type, index, def ))
    .then(( result, def ) => this.load( type, index, def ))
    .then(( result ) => this._onAfter( type, index, def, result ));
  break;

That would be simpler to maintain, but I don't know if it's compatible with all the cases we need.

We may also want to measure parsing time before/after here. I'm a little worried that adding a chain of promises for every accessor might create a lot of overhead.

[takahirox]

is it possible to put all the hooks into onDependency

I haven't looked into the all asset types yet and perhaps we need a bit change in .loadXXX(), but probably it's possible. Maybe like this?

// .getDependency()
case 'XXX':
  dependency = this._onBefore(XXX, index (or json.XXXs[index])).then(function (def) {
    return parser._on(XXX, def).then(function (customXXX) {
      return parser.loadXXX(index, def, customXXX);
    }).then(function (result) {
      return parser._onAfter(XXX, result, def);
    });
  });
  break;

// .loadXXX()
/**
 * @param {number} xxxIndex
 * @param {GLTF.definition} xxxDef
 * @param {Object|null} customXXX (optional)
 * @return {Promise<Object>}
 */
GLTFParser.prototype.loadXXX = function (xxxIndex, xxxDef, customXXX) {
  var pending;
  if (customXXX) {
    pending = Promise.resolve(customXXX);
  } else {
    pending = create core spec promise object from xxxDef (and xxxIndex);
  }
  pending.then(function (xxx) {
    // setting up common properties here
    // for example name, position/quaternion/scale/matrix, and so on for Node
    return xxx;
  } );
};

We may also want to measure parsing time before/after here.

Yes, that's the plan. I first wanted to share the rough sketch API idea.

[takahirox]

In the above my comment, I thought to pass customXXX to loadXXX() to let it set up common properties (name and so on) for example the ones in loadMaterial() but coming to think that it can be a bit messy and some extension handlers may not want the common properties to be overridden because they set the properties something special.

So considering to call .loadXXX() only if ._onXXX() doesn't create an object. Pros: .loadXXX() doesn't need to change other than taking from index to def (and index), we can keep the code clean. And the parser don't override anything extension handler made. Cons: Extension handlers may have duplicated codes with the ones in the parser because they need to even set up the common properties by themselves.

Regarding assignExtrasToUserData() and addUnknownExtensionsToUserData(), we may move out from .loadXXX() and call in getDependency() instead if we don't want to expose them?

case 'XXX':
  dependency = this._onBefore(XXX, index (or json.XXXs[index])).then(function (def) {
    return parser._on(XXX, def).then(function (customXXX) {
      return customXXX || parser.loadXXX(index, def);
    }).then(function (result) {
      assignExtrasToUserData(result, def);
      if (def.extensions) addUnknownExtensionsToUserData(extensions, result, def);
      return parser._onAfter(XXX, result, def);
    });
  });
  break;

[takahirox]

With the API above (onBeforeXXX, onXXX, and onAfterXXX for all asset types + Map, Geometry, and GLTF) I measured the loading response time (from .load() to onLoad) of some models in gltf extension example on my Windows Chrome. The numbers are average of five time trials. I haven't looked into the detail yet but Promise chain seems to have non-negligible response time impact. But I guess (and hope) main thread blocking time may not be so different? If so, I hope this impact can be acceptable.

(I'm not sure yet why the Monster glTF-Draco numbers between dev and plugin are close... Five time trial is not may enough? Or draco uncompression processing can be dominant?)

	dev [ms]	plugin [ms]	plugin / dev [%]
Boombox glTF	42.81	51.13	+19%
Boombox glTF-dds	205.70	225.52	+10%
Monster glTF	49.06	74.62	+52%
Monster glTF-Draco	167.96	164.90	-2%

[donmccurdy]

I'm having a bit of trouble reviewing all of this at once. Maybe consider my comment above (since it'd be easier to answer that question with all of the extensions included). And if that's OK, do you mind introducing the new plugin API with just a single extension first?

[takahirox]

do you mind introducing the new plugin API with just a single extension first?

Sent PR #18484

[takahirox]

Closing in favor of #19144