Fix #611 Fix #608 Fix loading cancellations

mistic100 · Dec 23, 2021 · 3c3ea82 · 3c3ea82
1 parent fcb5a4f
commit 3c3ea82
Show file tree

Hide file tree

Showing 10 changed files with 96 additions and 62 deletions.
diff --git a/src/Viewer.js b/src/Viewer.js
@@ -20,8 +20,10 @@ import { TooltipRenderer } from './services/TooltipRenderer';
 import {
   each,
   exitFullscreen,
+  getAbortError,
   getAngle,
   getShortestArc,
+  isAbortError,
   isExtendedPosition,
   isFullscreenEnabled,
   logWarn,
@@ -440,7 +442,7 @@ export class Viewer extends EventEmitter {
    * If another loading is already in progress it will be aborted.
    * @param {*} path - URL of the new panorama file
    * @param {PSV.PanoramaOptions} [options]
-   * @returns {Promise}
+   * @returns {Promise<boolean>} resolves false if the loading was aborted by another call
    */
   setPanorama(path, options = {}) {
     if (this.prop.loadingPromise !== null) {
@@ -476,33 +478,40 @@ export class Viewer extends EventEmitter {
     this.config.panorama = path;
 
     const done = (err) => {
-      if (err && err.type === 'abort') {
-        console.warn(err);
-      }
-      else if (err) {
-        this.showError(this.config.lang.loadError);
-        console.error(err);
-      }
-
       this.loader.hide();
       this.renderer.show();
 
       this.prop.loadingPromise = null;
 
-      if (err) {
+      if (isAbortError(err)) {
+        console.warn(err);
+        return false;
+      }
+      else if (err) {
+        this.showError(this.config.lang.loadError);
+        console.error(err);
         return Promise.reject(err);
       }
       else {
         return true;
       }
     };
 
-    if (!options.transition || !this.prop.ready || !this.adapter.constructor.supportsTransition) {
-      if (options.showLoader || !this.prop.ready) {
-        this.loader.show();
-      }
+    if (options.showLoader || !this.prop.ready) {
+      this.loader.show();
+    }
+
+    const loadingPromise = this.adapter.loadTexture(this.config.panorama, options.panoData)
+      .then((textureData) => {
+        // check if another panorama was requested
+        if (textureData.panorama !== this.config.panorama) {
+          return Promise.reject(getAbortError());
+        }
+        return textureData;
+      });
 
-      this.prop.loadingPromise = this.adapter.loadTexture(this.config.panorama, options.panoData)
+    if (!options.transition || !this.prop.ready || !this.adapter.constructor.supportsTransition) {
+      this.prop.loadingPromise = loadingPromise
         .then((textureData) => {
           this.renderer.setTexture(textureData);
           this.renderer.setPanoramaPose(textureData.panoData);
@@ -518,11 +527,7 @@ export class Viewer extends EventEmitter {
         .then(done, done);
     }
     else {
-      if (options.showLoader) {
-        this.loader.show();
-      }
-
-      this.prop.loadingPromise = this.adapter.loadTexture(this.config.panorama, options.panoData)
+      this.prop.loadingPromise = loadingPromise
         .then((textureData) => {
           this.loader.hide();
 

diff --git a/src/adapters/cubemap/index.js b/src/adapters/cubemap/index.js
@@ -78,7 +78,7 @@ export class CubemapAdapter extends AbstractAdapter {
     }
 
     return Promise.all(promises)
-      .then(texture => ({ texture }));
+      .then(texture => ({ panorama, texture }));
   }
 
   /**

diff --git a/src/adapters/equirectangular-tiles/index.js b/src/adapters/equirectangular-tiles/index.js
@@ -192,10 +192,8 @@ export class EquirectangularTilesAdapter extends AbstractAdapter {
       return Promise.reject(new PSVError('Panorama cols and rows must be powers of 2.'));
     }
 
-    panorama.height = panorama.width / 2;
-
     this.prop.colSize = panorama.width / panorama.cols;
-    this.prop.rowSize = panorama.height / panorama.rows;
+    this.prop.rowSize = panorama.width / 2 / panorama.rows;
     this.prop.facesByCol = SPHERE_SEGMENTS / panorama.cols;
     this.prop.facesByRow = SPHERE_SEGMENTS / 2 / panorama.rows;
 
@@ -207,27 +205,22 @@ export class EquirectangularTilesAdapter extends AbstractAdapter {
 
     const panoData = {
       fullWidth    : panorama.width,
-      fullHeight   : panorama.height,
+      fullHeight   : panorama.width / 2,
       croppedWidth : panorama.width,
-      croppedHeight: panorama.height,
+      croppedHeight: panorama.width / 2,
       croppedX     : 0,
       croppedY     : 0,
     };
 
     if (panorama.baseUrl) {
       return this.psv.textureLoader.loadImage(panorama.baseUrl, p => this.psv.loader.setProgress(p))
         .then((img) => {
-          return {
-            texture : this.__createBaseTexture(img),
-            panoData: panoData,
-          };
+          const texture = this.__createBaseTexture(img);
+          return { panorama, texture, panoData };
         });
     }
     else {
-      return Promise.resolve({
-        texture : null,
-        panoData: panoData,
-      });
+      return Promise.resolve({ panorama, panoData });
     }
   }
 

diff --git a/src/adapters/equirectangular/index.js b/src/adapters/equirectangular/index.js
@@ -66,7 +66,7 @@ export class EquirectangularAdapter extends AbstractAdapter {
 
         const texture = this.__createEquirectangularTexture(img, panoData);
 
-        return { texture, panoData };
+        return { panorama, texture, panoData };
       });
   }
 

diff --git a/src/index.js b/src/index.js
@@ -115,6 +115,7 @@ export {
 /**
  * @typedef {Object} PSV.TextureData
  * @summary Result of the {@link PSV.adapters.AbstractAdapter#loadTexture} method
+ * @property {*} panorama
  * @property {external:THREE.Texture|external:THREE.Texture[]|Record<string, external:THREE.Texture[]>} texture
  * @property {PSV.PanoData} [panoData]
  */

diff --git a/src/plugins/virtual-tour/index.js b/src/plugins/virtual-tour/index.js
@@ -122,12 +122,14 @@ export class VirtualTourPlugin extends AbstractPlugin {
      * @property {PSV.plugins.VirtualTourPlugin.Node} currentNode
      * @property {external:THREE.Mesh} currentArrow
      * @property {PSV.Tooltip} currentTooltip
+     * @property {string} loadingNode
      * @private
      */
     this.prop = {
       currentNode   : null,
       currentArrow  : null,
       currentTooltip: null,
+      loadingNode   : null,
     };
 
     /**
@@ -336,18 +338,33 @@ export class VirtualTourPlugin extends AbstractPlugin {
   /**
    * @summary Changes the current node
    * @param {string} nodeId
+   * @returns {Promise<boolean>} resolves false if the loading was aborted by another call
    */
   setCurrentNode(nodeId) {
+    if (nodeId === this.prop.currentNode?.id) {
+      return Promise.resolve(true);
+    }
+
     this.psv.loader.show();
     this.psv.hideError();
 
+    this.prop.loadingNode = nodeId;
+
     // if this node is already preloading, wait for it
     return Promise.resolve(this.preload[nodeId])
       .then(() => {
+        if (this.prop.loadingNode !== nodeId) {
+          return Promise.reject(utils.getAbortError());
+        }
+
         this.psv.textureLoader.abortLoading();
         return this.datasource.loadNode(nodeId);
       })
       .then((node) => {
+        if (this.prop.loadingNode !== nodeId) {
+          return Promise.reject(utils.getAbortError());
+        }
+
         this.psv.navbar.setCaption(`<em>${this.psv.config.lang.loading}</em>`);
 
         this.prop.currentNode = node;
@@ -374,12 +391,18 @@ export class VirtualTourPlugin extends AbstractPlugin {
             panoData        : node.panoData,
             sphereCorrection: node.sphereCorrection,
           })
-            // eslint-disable-next-line prefer-promise-reject-errors
-            .catch(() => Promise.reject(null)), // the error is already displayed by the core
+            .catch((err) => {
+              // the error is already displayed by the core
+              return Promise.reject(utils.isAbortError(err) ? err : null);
+            }),
           this.datasource.loadLinkedNodes(nodeId),
         ]);
       })
       .then(() => {
+        if (this.prop.loadingNode !== nodeId) {
+          return Promise.reject(utils.getAbortError());
+        }
+
         const node = this.prop.currentNode;
 
         if (node.markers) {
@@ -403,14 +426,23 @@ export class VirtualTourPlugin extends AbstractPlugin {
          * @param {string} nodeId
          */
         this.trigger(EVENTS.NODE_CHANGED, nodeId);
+
+        this.prop.loadingNode = null;
+
+        return true;
       })
       .catch((err) => {
+        if (utils.isAbortError(err)) {
+          return Promise.resolve(false);
+        }
+        else if (err) {
+          this.psv.showError(this.psv.config.lang.loadError);
+        }
+
         this.psv.loader.hide();
         this.psv.navbar.setCaption('');
 
-        if (err) {
-          this.psv.showError(this.psv.config.lang.loadError);
-        }
+        this.prop.loadingNode = null;
 
         return Promise.reject(err);
       });

diff --git a/src/services/TextureLoader.js b/src/services/TextureLoader.js
@@ -14,13 +14,6 @@ export class TextureLoader extends AbstractService {
   constructor(psv) {
     super(psv);
 
-    /**
-     * @summary Current HTTP requests
-     * @type {XMLHttpRequest[]}
-     * @private
-     */
-    this.requests = [];
-
     /**
      * @summary THREE file loader
      * @type {external:THREE:FileLoader}
@@ -62,7 +55,7 @@ export class TextureLoader extends AbstractService {
    * @package
    */
   abortLoading() {
-    [...this.requests].forEach(r => r.abort());
+    // noop implementation waiting for https://github.com/mrdoob/three.js/pull/23070
   }
 
   /**
@@ -80,12 +73,9 @@ export class TextureLoader extends AbstractService {
       let progress = 0;
       onProgress && onProgress(progress);
 
-      const request = this.loader.load(
+      this.loader.load(
         url,
         (result) => {
-          const rIdx = this.requests.indexOf(request);
-          if (rIdx !== -1) this.requests.splice(rIdx, 1);
-
           progress = 100;
           onProgress && onProgress(progress);
           resolve(result);
@@ -100,17 +90,9 @@ export class TextureLoader extends AbstractService {
           }
         },
         (err) => {
-          const rIdx = this.requests.indexOf(request);
-          if (rIdx !== -1) this.requests.splice(rIdx, 1);
-
           reject(err);
         }
       );
-
-      // when we hit the cache, the result is the cache value
-      if (request instanceof XMLHttpRequest) {
-        this.requests.push(request);
-      }
     });
   }
 

diff --git a/src/utils/psv.js b/src/utils/psv.js
@@ -19,6 +19,25 @@ export function pluginInterop(plugin, target) {
   return null;
 }
 
+/**
+ * @summary Builds an Error with name 'AbortError'
+ * @return {Error}
+ */
+export function getAbortError() {
+  const error = new Error('Loading was aborted.');
+  error.name = 'AbortError';
+  return error;
+}
+
+/**
+ * @summary Tests if an Error has name 'AbortError'
+ * @param {Error} err
+ * @return {boolean}
+ */
+export function isAbortError(err) {
+  return err?.name === 'AbortError';
+}
+
 /**
  * @summary Displays a warning in the console
  * @memberOf PSV.utils

diff --git a/types/Viewer.d.ts b/types/Viewer.d.ts
@@ -188,8 +188,9 @@ export class Viewer extends EventEmitter {
    * @description Loads a new panorama file, optionally changing the camera position/zoom and activating the transition animation.<br>
    * If the "options" parameter is not defined, the camera will not move and the ongoing animation will continue.<br>
    * If another loading is already in progress it will be aborted.
+   * @returns resolves false if the loading was aborted by another call
    */
-  setPanorama(panorama: any, options?: PanoramaOptions): Promise<void>;
+  setPanorama(panorama: any, options?: PanoramaOptions): Promise<boolean>;
 
   /**
    * @summary Update options

diff --git a/types/plugins/virtual-tour/index.d.ts b/types/plugins/virtual-tour/index.d.ts
@@ -70,7 +70,8 @@ export class VirtualTourPlugin extends AbstractPlugin {
 
   /**
    * @summary Changes the current node
+   * @returns resolves false if the loading was aborted by another call
    */
-  setCurrentNode(nodeId: string);
+  setCurrentNode(nodeId: string): Promise<boolean>;
 
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -78,7 +78,7 @@ export class CubemapAdapter extends AbstractAdapter { @@
         }
         return Promise.all(promises)
-          .then(texture => ({ texture }));
+          .then(texture => ({ panorama, texture }));
       }
       /**
@@ Expand Down @@