From 3c3ea8249549dc052433d3733cb245dfc53046b4 Mon Sep 17 00:00:00 2001 From: mistic100 Date: Thu, 23 Dec 2021 20:06:23 +0100 Subject: [PATCH] Fix #611 Fix #608 Fix loading cancellations --- src/Viewer.js | 45 ++++++++++++--------- src/adapters/cubemap/index.js | 2 +- src/adapters/equirectangular-tiles/index.js | 19 +++------ src/adapters/equirectangular/index.js | 2 +- src/index.js | 1 + src/plugins/virtual-tour/index.js | 42 ++++++++++++++++--- src/services/TextureLoader.js | 22 +--------- src/utils/psv.js | 19 +++++++++ types/Viewer.d.ts | 3 +- types/plugins/virtual-tour/index.d.ts | 3 +- 10 files changed, 96 insertions(+), 62 deletions(-) diff --git a/src/Viewer.js b/src/Viewer.js index 1737cb186..647cdf636 100644 --- 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} resolves false if the loading was aborted by another call */ setPanorama(path, options = {}) { if (this.prop.loadingPromise !== null) { @@ -476,20 +478,18 @@ 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 { @@ -497,12 +497,21 @@ export class Viewer extends EventEmitter { } }; - 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 index f00a3eda5..fbdfe203e 100644 --- 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 index dbc5907bd..3427cc4e5 100644 --- 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,9 +205,9 @@ 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, }; @@ -217,17 +215,12 @@ export class EquirectangularTilesAdapter extends AbstractAdapter { 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 index fb0ef9e4c..a4719ddf7 100644 --- 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 index 6dcb6befc..237fa0d3a 100644 --- 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} texture * @property {PSV.PanoData} [panoData] */ diff --git a/src/plugins/virtual-tour/index.js b/src/plugins/virtual-tour/index.js index 530e9c612..081cb5ed6 100644 --- 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} 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(`${this.psv.config.lang.loading}`); 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 index 5c3b985b2..b34abee05 100644 --- 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 index caa85f949..c9ad5cc90 100644 --- 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 index 2645abf2a..270e46a8f 100644 --- 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.
* If the "options" parameter is not defined, the camera will not move and the ongoing animation will continue.
* 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; + setPanorama(panorama: any, options?: PanoramaOptions): Promise; /** * @summary Update options diff --git a/types/plugins/virtual-tour/index.d.ts b/types/plugins/virtual-tour/index.d.ts index 01d594ba5..f785a5f12 100644 --- 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; }