Merge pull request #4805 from bghgary/gltf-bug-fixes

Fix a bug with MSFT_lod extension throwing exception

loaders/src/glTF/2.0/Extensions/EXT_lights_imageBased.ts

@@ -28,6 +28,16 @@ module BABYLON.GLTF2.Extensions {
     export class EXT_lights_imageBased extends GLTFLoaderExtension {
         public readonly name = NAME;
 
+        private _lights?: ILight[];
+
+        protected _onLoading(): void {
+            const extensions = this._loader._gltf.extensions;
+            if (extensions && extensions[this.name]) {
+                const extension = extensions[this.name] as ILights;
+                this._lights = extension.lights;
+            }
+        }
+
         protected _loadSceneAsync(context: string, scene: _ILoaderScene): Nullable<Promise<void>> { 
             return this._loadExtensionAsync<ILightReference>(context, scene, (extensionContext, extension) => {
                 const promises = new Array<Promise<void>>();
@@ -108,16 +118,6 @@ module BABYLON.GLTF2.Extensions {
                 return light._babylonTexture!;
             });
         }
-
-        private get _lights(): Array<ILight> {
-            const extensions = this._loader._gltf.extensions;
-            if (!extensions || !extensions[this.name]) {
-                throw new Error(`#/extensions: '${this.name}' not found`);
-            }
-
-            const extension = extensions[this.name] as ILights;
-            return extension.lights;
-        }
     }
 
     GLTFLoader._Register(NAME, loader => new EXT_lights_imageBased(loader));

loaders/src/glTF/2.0/Extensions/KHR_lights.ts

@@ -34,6 +34,16 @@ module BABYLON.GLTF2.Extensions {
     export class KHR_lights extends GLTFLoaderExtension {
         public readonly name = NAME;
 
+        private _lights?: ILight[];
+
+        protected _onLoading(): void {
+            const extensions = this._loader._gltf.extensions;
+            if (extensions && extensions[this.name]) {
+                const extension = extensions[this.name] as ILights;
+                this._lights = extension.lights;
+            }
+        }
+
         protected _loadSceneAsync(context: string, scene: _ILoaderScene): Nullable<Promise<void>> { 
             return this._loadExtensionAsync<ILightReference>(context, scene, (extensionContext, extension) => {
                 const promise = this._loader._loadSceneAsync(extensionContext, scene);
@@ -88,16 +98,6 @@ module BABYLON.GLTF2.Extensions {
                 return promise;
             });
         }
-
-        private get _lights(): Array<ILight> {
-            const extensions = this._loader._gltf.extensions;
-            if (!extensions || !extensions[this.name]) {
-                throw new Error(`#/extensions: '${this.name}' not found`);
-            }
-
-            const extension = extensions[this.name] as ILights;
-            return extension.lights;
-        }
     }
 
     GLTFLoader._Register(NAME, loader => new KHR_lights(loader));

loaders/src/glTF/2.0/Extensions/MSFT_lod.ts

@@ -40,48 +40,6 @@ module BABYLON.GLTF2.Extensions {
         private _materialSignalLODs = new Array<Deferred<void>>();
         private _materialPromiseLODs = new Array<Array<Promise<void>>>();
 
-        constructor(loader: GLTFLoader) {
-            super(loader);
-
-            this._loader._readyPromise.then(() => {
-                for (let indexLOD = 0; indexLOD < this._nodePromiseLODs.length; indexLOD++) {
-                    const promise = Promise.all(this._nodePromiseLODs[indexLOD]).then(() => {
-                        if (indexLOD !== 0) {
-                            this._loader._parent._endPerformanceCounter(`Node LOD ${indexLOD}`);
-                        }
-
-                        this._loader._parent._log(`Loaded node LOD ${indexLOD}`);
-                        this.onNodeLODsLoadedObservable.notifyObservers(indexLOD);
-
-                        if (indexLOD !== this._nodePromiseLODs.length - 1) {
-                            this._loader._parent._startPerformanceCounter(`Node LOD ${indexLOD + 1}`);
-                            this._nodeSignalLODs[indexLOD].resolve();
-                        }
-                    });
-
-                    this._loader._completePromises.push(promise);
-                }
-
-                for (let indexLOD = 0; indexLOD < this._materialPromiseLODs.length; indexLOD++) {
-                    const promise = Promise.all(this._materialPromiseLODs[indexLOD]).then(() => {
-                        if (indexLOD !== 0) {
-                            this._loader._parent._endPerformanceCounter(`Material LOD ${indexLOD}`);
-                        }
-
-                        this._loader._parent._log(`Loaded material LOD ${indexLOD}`);
-                        this.onMaterialLODsLoadedObservable.notifyObservers(indexLOD);
-
-                        if (indexLOD !== this._materialPromiseLODs.length - 1) {
-                            this._loader._parent._startPerformanceCounter(`Material LOD ${indexLOD + 1}`);
-                            this._materialSignalLODs[indexLOD].resolve();
-                        }
-                    });
-
-                    this._loader._completePromises.push(promise);
-                }
-            });
-        }
-
         public dispose() {
             super.dispose();
 
@@ -97,6 +55,48 @@ module BABYLON.GLTF2.Extensions {
             this.onNodeLODsLoadedObservable.clear();
         }
 
+        protected _onReady(): void {
+            for (let indexLOD = 0; indexLOD < this._nodePromiseLODs.length; indexLOD++) {
+                const promise = Promise.all(this._nodePromiseLODs[indexLOD]).then(() => {
+                    if (indexLOD !== 0) {
+                        this._loader._parent._endPerformanceCounter(`Node LOD ${indexLOD}`);
+                    }
+
+                    this._loader._parent._log(`Loaded node LOD ${indexLOD}`);
+                    this.onNodeLODsLoadedObservable.notifyObservers(indexLOD);
+
+                    if (indexLOD !== this._nodePromiseLODs.length - 1) {
+                        this._loader._parent._startPerformanceCounter(`Node LOD ${indexLOD + 1}`);
+                        if (this._nodeSignalLODs[indexLOD]) {
+                            this._nodeSignalLODs[indexLOD].resolve();
+                        }
+                    }
+                });
+
+                this._loader._completePromises.push(promise);
+            }
+
+            for (let indexLOD = 0; indexLOD < this._materialPromiseLODs.length; indexLOD++) {
+                const promise = Promise.all(this._materialPromiseLODs[indexLOD]).then(() => {
+                    if (indexLOD !== 0) {
+                        this._loader._parent._endPerformanceCounter(`Material LOD ${indexLOD}`);
+                    }
+
+                    this._loader._parent._log(`Loaded material LOD ${indexLOD}`);
+                    this.onMaterialLODsLoadedObservable.notifyObservers(indexLOD);
+
+                    if (indexLOD !== this._materialPromiseLODs.length - 1) {
+                        this._loader._parent._startPerformanceCounter(`Material LOD ${indexLOD + 1}`);
+                        if (this._materialSignalLODs[indexLOD]) {
+                            this._materialSignalLODs[indexLOD].resolve();
+                        }
+                    }
+                });
+
+                this._loader._completePromises.push(promise);
+            }
+        }
+
         protected _loadNodeAsync(context: string, node: _ILoaderNode): Nullable<Promise<void>> {
             return this._loadExtensionAsync<IMSFTLOD>(context, node, (extensionContext, extension) => {
                 let firstPromise: Promise<void>;

loaders/src/glTF/2.0/Extensions/MSFT_sRGBFactors.ts

@@ -7,25 +7,27 @@ module BABYLON.GLTF2.Extensions {
     export class MSFT_sRGBFactors extends GLTFLoaderExtension {
         public readonly name = NAME;
 
-        protected _loadMaterialAsync(context: string, material: _ILoaderMaterial, mesh: _ILoaderMesh, babylonMesh: Mesh, babylonDrawMode: number, assign: (babylonMaterial: Material) => void): Nullable<Promise<void>> {
+        protected _loadMaterialPropertiesAsync(context: string, material: _ILoaderMaterial, babylonMaterial: Material): Nullable<Promise<void>> {
             return this._loadExtrasValueAsync<boolean>(context, material, (extensionContext, value) => {
                 if (value) {
-                    return this._loader._loadMaterialAsync(context, material, mesh, babylonMesh, babylonDrawMode, (babylonMaterial: PBRMaterial) => {
-                        if (!babylonMaterial.albedoTexture) {
-                            babylonMaterial.albedoColor.toLinearSpaceToRef(babylonMaterial.albedoColor);
-                        }
-
-                        if (!babylonMaterial.reflectivityTexture) {
-                            babylonMaterial.reflectivityColor.toLinearSpaceToRef(babylonMaterial.reflectivityColor);
-                        }
-
-                        assign(babylonMaterial);
-                    });
+                    const promise = this._loader._loadMaterialPropertiesAsync(context, material, babylonMaterial);
+                    this._convertColorsToLinear(babylonMaterial as PBRMaterial);
+                    return promise;
                 }
 
                 return null;
             });
         }
+
+        private _convertColorsToLinear(babylonMaterial: PBRMaterial): void {
+            if (!babylonMaterial.albedoTexture) {
+                babylonMaterial.albedoColor.toLinearSpaceToRef(babylonMaterial.albedoColor);
+            }
+
+            if (!babylonMaterial.reflectivityTexture) {
+                babylonMaterial.reflectivityColor.toLinearSpaceToRef(babylonMaterial.reflectivityColor);
+            }
+        }
     }
 
     GLTFLoader._Register(NAME, loader => new MSFT_sRGBFactors(loader));

loaders/src/glTF/2.0/babylon.glTFLoader.ts

@@ -26,7 +26,6 @@ module BABYLON.GLTF2 {
         public _parent: GLTFFileLoader;
         public _gltf: _ILoaderGLTF;
         public _babylonScene: Scene;
-        public _readyPromise: Promise<void>;
         public _completePromises = new Array<Promise<void>>();
 
         private _disposed = false;
@@ -54,9 +53,6 @@ module BABYLON.GLTF2 {
             GLTFLoader._ExtensionNames.push(name);
         }
 
-        /**
-         * Loader state or null if the loader is not active.
-         */
         public get state(): Nullable<GLTFLoaderState> {
             return this._state;
         }
@@ -80,7 +76,6 @@ module BABYLON.GLTF2 {
 
             delete this._gltf;
             delete this._babylonScene;
-            delete this._readyPromise;
             this._completePromises.length = 0;
 
             for (const name in this._extensions) {
@@ -148,17 +143,17 @@ module BABYLON.GLTF2 {
 
         private _loadAsync(nodes: Nullable<Array<number>>): Promise<void> {
             return Promise.resolve().then(() => {
-                this._parent._startPerformanceCounter("Loading => Ready");
-                this._parent._startPerformanceCounter("Loading => Complete");
+                this._loadExtensions();
+                this._checkExtensions();
 
-                this._state = GLTFLoaderState.LOADING;
-                this._parent._log("Loading");
+                const loadingToReadyCounterName = `${GLTFLoaderState[GLTFLoaderState.LOADING]} => ${GLTFLoaderState[GLTFLoaderState.READY]}`;
+                const loadingToCompleteCounterName = `${GLTFLoaderState[GLTFLoaderState.LOADING]} => ${GLTFLoaderState[GLTFLoaderState.COMPLETE]}`;
 
-                const readyDeferred = new Deferred<void>();
-                this._readyPromise = readyDeferred.promise;
+                this._parent._startPerformanceCounter(loadingToReadyCounterName);
+                this._parent._startPerformanceCounter(loadingToCompleteCounterName);
 
-                this._loadExtensions();
-                this._checkExtensions();
+                this._setState(GLTFLoaderState.LOADING);
+                GLTFLoaderExtension._OnLoading(this);
 
                 const promises = new Array<Promise<void>>();
 
@@ -179,30 +174,30 @@ module BABYLON.GLTF2 {
                 }
 
                 const resultPromise = Promise.all(promises).then(() => {
-                    this._state = GLTFLoaderState.READY;
-                    this._parent._log("Ready");
-
-                    readyDeferred.resolve();
+                    this._setState(GLTFLoaderState.READY);
+                    GLTFLoaderExtension._OnReady(this);
 
                     this._startAnimations();
                 });
 
                 resultPromise.then(() => {
-                    this._parent._endPerformanceCounter("Loading => Ready");
+                    this._parent._endPerformanceCounter(loadingToReadyCounterName);
 
                     Tools.SetImmediate(() => {
                         if (!this._disposed) {
                             Promise.all(this._completePromises).then(() => {
-                                this._parent._endPerformanceCounter("Loading => Complete");
+                                this._parent._endPerformanceCounter(loadingToCompleteCounterName);
 
-                                this._state = GLTFLoaderState.COMPLETE;
-                                this._parent._log("Complete");
+                                this._setState(GLTFLoaderState.COMPLETE);
 
                                 this._parent.onCompleteObservable.notifyObservers(undefined);
                                 this._parent.onCompleteObservable.clear();
+
                                 this.dispose();
-                            }).catch(error => {
-                                Tools.Error(`glTF Loader: ${error.message}`);
+                            }, error => {
+                                this._parent.onErrorObservable.notifyObservers(error);
+                                this._parent.onErrorObservable.clear();
+
                                 this.dispose();
                             });
                         }
@@ -210,10 +205,13 @@ module BABYLON.GLTF2 {
                 });
 
                 return resultPromise;
-            }).catch(error => {
+            }, error => {
                 if (!this._disposed) {
-                    Tools.Error(`glTF Loader: ${error.message}`);
+                    this._parent.onErrorObservable.notifyObservers(error);
+                    this._parent.onErrorObservable.clear();
+
                     this.dispose();
+
                     throw error;
                 }
             });
@@ -294,6 +292,11 @@ module BABYLON.GLTF2 {
             }
         }
 
+        private _setState(state: GLTFLoaderState): void {
+            this._state = state;
+            this._parent._log(GLTFLoaderState[this._state]);
+        }
+
         private _createRootNode(): _ILoaderNode {
             this._rootBabylonMesh = new Mesh("__root__", this._babylonScene);
 
@@ -1753,6 +1756,15 @@ module BABYLON.GLTF2 {
 
             return null;
         }
+
+        public _forEachExtensions(action: (extension: GLTFLoaderExtension) => void): void {
+            for (const name of GLTFLoader._ExtensionNames) {
+                const extension = this._extensions[name];
+                if (extension.enabled) {
+                    action(extension);
+                }
+            }
+        }
     }
 
     GLTFFileLoader._CreateGLTFLoaderV2 = parent => new GLTFLoader(parent);

loaders/src/glTF/2.0/babylon.glTFLoaderExtension.ts

@@ -35,6 +35,16 @@ module BABYLON.GLTF2 {
         // #region Overridable Methods
 
         /**
+         * Override this method to do work after the state changes to LOADING.
+         */
+        protected _onLoading(): void {}
+
+        /**
+         * Override this method to do work after the state changes to READY.
+         */
+        protected _onReady(): void {}
+
+        /**
          * Override this method to modify the default behavior for loading scenes.
          * @hidden
          */
@@ -141,6 +151,22 @@ module BABYLON.GLTF2 {
         }
 
         /**
+         * Helper method called by the loader after the state changes to LOADING.
+         * @hidden
+         */
+        public static _OnLoading(loader: GLTFLoader): void {
+            loader._forEachExtensions(extension => extension._onLoading());
+        }
+
+        /**
+         * Helper method called by the loader after the state changes to READY.
+         * @hidden
+         */
+        public static _OnReady(loader: GLTFLoader): void {
+            loader._forEachExtensions(extension => extension._onReady());
+        }
+
+        /**
          * Helper method called by the loader to allow extensions to override loading scenes.
          * @hidden
          */

loaders/src/glTF/babylon.glTFFileLoader.ts

@@ -264,6 +264,8 @@ module BABYLON {
 
         /**
          * Callback raised when the asset is completely loaded, immediately before the loader is disposed.
+         * For assets with LODs, raised when all of the LODs are complete.
+         * For assets without LODs, raised when the model is complete, immediately after the loader resolves the returned promise.
          */
         public set onComplete(callback: () => void) {
             if (this._onCompleteObserver) {
@@ -273,6 +275,23 @@ module BABYLON {
         }
 
         /**
+         * Observable raised when an error occurs.
+         */
+        public readonly onErrorObservable = new Observable<any>();
+
+        private _onErrorObserver: Nullable<Observer<any>>;
+
+        /**
+         * Callback raised when an error occurs.
+         */
+        public set onError(callback: (reason: any) => void) {
+            if (this._onErrorObserver) {
+                this.onErrorObservable.remove(this._onErrorObserver);
+            }
+            this._onErrorObserver = this.onErrorObservable.add(callback);
+        }
+
+        /**
          * Observable raised after the loader is disposed.
          */
         public readonly onDisposeObservable = new Observable<void>();
@@ -312,10 +331,13 @@ module BABYLON {
          * @returns a promise that resolves when the asset is completely loaded.
          */
         public whenCompleteAsync(): Promise<void> {
-            return new Promise(resolve => {
+            return new Promise((resolve, reject) => {
                 this.onCompleteObservable.addOnce(() => {
                     resolve();
                 });
+                this.onErrorObservable.addOnce(reason => {
+                    reject(reason);
+                });
             });
         }
 

tests/unit/babylon/src/Loading/babylon.sceneLoader.tests.ts

@@ -74,7 +74,7 @@ describe('Babylon Scene Loader', function () {
             });
         });
 
-        it('Load TwoQuads with ImporMesh and two node names', () => {
+        it('Load TwoQuads with ImportMesh and two node names', () => {
             const scene = new BABYLON.Scene(subject);
             return BABYLON.SceneLoader.ImportMeshAsync(["node0", "node1"], "http://models.babylonjs.com/Tests/TwoQuads/", "TwoQuads.gltf", scene).then(() => {
                 expect(scene.getMeshByName("node0"), "node0").to.exist;
@@ -397,6 +397,20 @@ describe('Babylon Scene Loader', function () {
             return Promise.all(promises);
         });
 
+        it('Load TwoQuadsNoTextures with LODs', () => {
+            const scene = new BABYLON.Scene(subject);
+
+            const promises = new Array<Promise<any>>();
+
+            BABYLON.SceneLoader.OnPluginActivatedObservable.addOnce((loader: BABYLON.GLTFFileLoader) => {
+                promises.push(loader.whenCompleteAsync());
+            });
+
+            promises.push(BABYLON.SceneLoader.AppendAsync("http://models.babylonjs.com/Tests/TwoQuads/", "TwoQuadsNoTextures.gltf", scene));
+
+            return Promise.all(promises);
+        });
+
         it('Load MultiPrimitive', () => {
             const scene = new BABYLON.Scene(subject);
             return BABYLON.SceneLoader.ImportMeshAsync(null, "http://models.babylonjs.com/Tests/MultiPrimitive/", "MultiPrimitive.gltf", scene).then(result => {