google / marzipano

A 360° media viewer for the modern web.
http://www.marzipano.net
Apache License 2.0
2k stars 996 forks source link

Stage not in sync with viewer after using moveLayer function #282

Open jordyno opened 4 years ago

jordyno commented 4 years ago

Repro details:

  1. load 2 layers onto a stage, works fine
  2. use stage#moveLayer to swap indexes of these layers, works fine
  3. use scene#switchTo to get "Stage not in sync with viewer" error

any idea what could be the cause?

tjgq commented 4 years ago

If you're using the Viewer/Scene API, the Viewer/Scene are in control of the layers. This check is verifying that the Stage contains the same layers in the same order as the Viewer/Scene implementation left them the last time.

In general this cannot work, because even if the check weren't there, the next time you switch into the original scene the layers will be added in the original order (because that's the correct order as far as the scene knows).

The Scene API wasn't designed with this use case in mind; there's an assumption that the layers don't change after the Scene is created. To support it, we'd need to add layer management methods (moveLayer and maybe others) to the scene, so that it's aware of the changes.

Why do you need to move layers after the scene is created?

jordyno commented 4 years ago

I need to move them because there are several layers expected to be used per each scene and I prefer to load them as needed to save loading times rather than load all of them with 0 opacity and show them as needed. Also, some of the layers are transparent and their order might need to change on the fly. How much work is it to add all the layer management methods? I can try to provide a PR with your advice.

jordyno commented 4 years ago

As a workaround I could add all layers but load only selected ones. Is that possible?

tjgq commented 4 years ago

If you can get away with setting the opacity to 0 on layers you don't need (and avoid reordering), try that first. If you're setting zero opacity on a bunch of them, it might make sense to optimize the viewer for that case (skip rendering). However, I wouldn't assume it's necessary unless you can measure the performance impact.

Otherwise, If you absolutely must reorder the layers (as opposed to setting the opacity to 0 on the ones you want to hide), then the Scene#moveLayer method will be necessary (PR welcome!).

There are two cases to consider for Scene#moveLayer:

I expect some of the code will have to reside in Viewer (in the same way that e.g. Scene#switchTo delegates to Viewer#switchScene) because Scene itself isn't aware of the transitions.

campmdabt commented 4 years ago

I have a similar setup where layers are created on the fly as necessary, and then are kept in memory and transitioned between using the opacity method (which has logic to figure out how to transition based on its position in the stack).

The transition function looks like this:

/**
 * Switches from the current layer (the one with effects.hide = false) to the target layer over the tweenTime.
 * @param {Scene} mSceneObj Marzipano scene where targetMLayer exists
 * @param {Layer} targetMLayer Marzipano layer to transition to
 * @param {number} tweenTime Transition length in ms
 */
export function transitionToNewLayer(mSceneObj, targetMLayer, tweenTime = 100) {
    let curLayerIndex = mSceneObj.listLayers().findIndex((layer) => {
        return !layer.effects().hide; // current layer is the one not hidden
    });
    curLayerIndex = curLayerIndex || 0;

    const newLayerIndex = mSceneObj.listLayers().findIndex((layer) => {
        return layer == targetMLayer; // find the target layer index
    });

    if (curLayerIndex === newLayerIndex) { return; }

    mSceneObj.listLayers().forEach((layer, index) => {
        // If not the current or new layer, hide it and move on
        if (index !== curLayerIndex && index !== newLayerIndex) {
            layer.mergeEffects({ opacity: 0, hide: true });
            return;
        }

        if (tweenTime) {
            Marzipano.util.tween(tweenTime, tweenUpdate, tweenDone);
        } else {
            tweenDone();
        }

        function tweenUpdate(alpha) {
            if (newLayerIndex > curLayerIndex) { // if the layer we want to show is on top, just increase the opacity there
                if (layer === targetMLayer) { // if this is the layer we want to show
                    layer.mergeEffects({ opacity: alpha, hide: false });
                } else {
                    layer.mergeEffects({ opacity: 1, hide: false }); // keep this shown until we completely fade in the new one
                }
            } else {
                if (layer === targetMLayer) { // if this is the layer we want to show
                    layer.mergeEffects({ opacity: 1, hide: false });
                } else {
                    layer.mergeEffects({ opacity: 1 - alpha, hide: false }); // fade this one out since it's in front of the new one
                }
            }
        }

        function tweenDone() {
            // When done, make sure everything is hidden but the new one
            if (layer === targetMLayer) { // if this is the layer we want to show
                layer.mergeEffects({ opacity: 1, hide: false });
            } else {
                layer.mergeEffects({ opacity: 0, hide: true });
            }
        }
    });
}

There is also logic that needs to be added when changing to a new scene if you want to fade in.

In order to create new layers on the fly, I have a data object that holds the created layers (sceneRenderObj in the following function) so I can check against it to see if the target layer already exists:

export async function changeSceneRendering(sceneRenderObj, shouldPushState = true) {
    const mScene = data.cur.scene.getMScene();
    if (!sceneRenderObj.getMLayer()) { // if we haven't already created the layer for this rendering, do it now
        const baseDataLocation = data.cur.scene.getAbsDataLocation() || data.cur.tourRevision.getAbsDataLocation() || data.cur.tourRevision.dataLocation;
        const panoTileFolderUrl = render.getBasePanoLocation(baseDataLocation, sceneRenderObj);
        sceneRenderObj.setMLayer(await pano.createMarzipanoSceneLayer(mScene, panoTileFolderUrl));
    }

    pano.transitionToNewLayer(mScene, sceneRenderObj.getMLayer(), 150);
    data.cur.rendering = sceneRenderObj;

    shouldPushState && historyInterface.addHistoryState({
        revisionId: data.cur.tourRevision.revisionId,
        sceneId: data.cur.scene.id,
        viewParams: pano.getCurrentView(),
        renderId: data.cur.rendering.renderId,
    });

    historyInterface.updateUrl();
    analytics.sendVirtualPageView();
    analytics.sendRenderOptionChangeEvent();

    // Let all listeners know that we've changed the pano
    const changeEvent = new CustomEvent('panoRenderChange', {
        bubbles: true,
        detail: {
            newSceneObj: data.cur.scene,
            tourRevObj: data.cur.tourRevision,
            renderingObj: sceneRenderObj,
        },
    });

    events.getListeningElements('panoRenderChange').forEach((element) => {
        if (!element) {
            console.error(`Cannot dispatch 'panoRenderChange' event on null element!`);
            return;
        }
        element.dispatchEvent(changeEvent);
    });
}

The pano.createMarzipanoSceneLayer function looks like this:

/**
 * Creates a new layer for the given scene and returns a promise that will be resolved when all data for that layer has finished downloading all the layer tiles.
 * In order for the promise to resolve, the scene we're adding the layer to MUST BE THE CURRENT SCENE. This is because the layers need to be part of the stage for the 
 * 'renderComplete' event to fire, and they're only added to the stage when that scene is switched to. The layer has opacity = 0 and hide = true when created.
 * @param {Scene} mScene Marzipano scene to create the new layer in
 * @param {string} newPanoBaseUrl Base URL for the pano tiles. Basically the folder where the tiles are stored.
 */
export async function createMarzipanoSceneLayer(mScene, newPanoBaseUrl) {
    const origLayer = mScene.listLayers()[0];
    const newLayer = mScene.createLayer({
        source: createImageSource(newPanoBaseUrl),
        geometry: origLayer.geometry(), // keep the geometry the same as it was
        pinFirstLevel: true,
        layerOpts: {
            effects: {
                opacity: 0, // start out invisible until everything is loaded since this new layer is created on top
                hide: true
            }
        }
    });

    return new Promise((resolve) => {
        mScene.viewer().stage().addEventListener('renderComplete', renderComplete); // NOTE: THIS WILL NOT FIRE WITH 'TRUE' UNLESS WE'RE ACTUALLY ON THAT SCENE ALREADY!

        function renderComplete(renderingFinished) {
            if (!renderingFinished) { return; } // if this event provides a false value, we're not done yet

            // If we make it here, all visible tile textures must be loaded
            mScene.viewer().stage().removeEventListener('renderComplete', renderComplete); // remove the listener so it's no longer called
            resolve(newLayer);
        }
    });
}

Functionally, it looks like this: https://sizzle.atlasbayvr.com/viewer/?companyId=atlas-bay-vr&projectId=marzipano-layer-demo&sceneId=unit-a1-bath&view=%7B%22yaw%22%3A180%2C%22pitch%22%3A14.08%2C%22roll%22%3A0%2C%22fov%22%3A84.09%7D&renderId=finish-scheme_scheme-1%2Bbath-options_shower%2Bfurniture-visibility_visible

Use the render option buttons to change layers - turn on developer tools and change the network speed to see the affect of loading a layer once which takes a long time, then switching to it again is instant.

PS Please forgive any sloppy or poorly architected code, I've only started with web development in February.

jordyno commented 4 years ago

@campmdabt thanks for your advice! Unfortunately your solution might not work too good in my case, as I have got a high resolution panoramas and loading all of them onto the stage at once would cause major resources draw. I would still like to take a look how it works in your implementation though, but your link did not work - login was required.

Thank you @tjgq for your insight! Maybe I can get away with just setting the opacity to 0, but I would still need to skip rendering somehow so that I do not jam the bandwidth and the memory. Using the 0 opacity workaround would mean adding about 15-30 layers per each scene and that is a lot. I have tested a scene with 12 layers, 11 of them transparent with only a few tiles each (I tuned Marzipano to skip rendering empty tiles) and could see a performance drop on mid-end devices. I assume I would need to avoid the loadAsset on layers which are currently unneeded. I wonder how much work that would be and whether it's not easier to create Scene#moveLayer method instead.

jordyno commented 4 years ago

@tjgq I am just looking at the scene and we actually do have the Scene#moveLayer already! Sorry for not noticing it in the first place! Now when I use this method directly instead of stage all seems to be working! Thank you!

campmdabt commented 4 years ago

@jordyno Sorry about the login issue - I forgot to set the project to public. Fixed now. Hoping the functions may help someone else.

tjgq commented 4 years ago

@jordyno Are you sure you're looking at the right file? I don't see anything named moveLayer in src/Scene.js.

The zero opacity optimization I was thinking of was for Stage#render to decide that zero tiles are visible when the opacity is zero (look for the call to this._collectTiles). Since no tiles are marked as visible in the TextureStore, they're never loaded, and loadAsset/createTexture are never called. If they've been loaded previously, they'll just stay in the TextureStore cache; you can reduce memory cosumption further (in your own code) by calling TextureStore#clear or TextureStore#clearNotPinned to evict them after you set opacity to zero.

jordyno commented 4 years ago

You are absolutely right @tjgq , I have added it some time ago into my own repo, that is why you did not see it:

Scene.prototype.moveLayer = function(layer, i) {
  var index = this._layers.indexOf(layer);
  if (index < 0) {
    throw new Error('No such layer in scene');
  }

  if (i < 0 || i >= this._layers.length) {
    throw new Error('Invalid layer position');
  }

  layer = this._layers.splice(index, 1)[0];
  this._layers.splice(i, 0, layer);
  this._viewer._stage.moveLayer(layer, i);
};

While it has fixed the issue partially, the problem remains when I need moveLayer of scene that is about to get switched. With Scene#moveLayer I can reliably change indexes of any layer after the scene is switched and it works fine. Now if I switch to another scene, there are no more errors, but: If I need to moveLayer inside target scene right before the switchScene method, I get the "No such layer in stage" error, which is exactly as described by you Tiago - further changes need to be made to the Viewer.

But now as you have pointed out, the 0 opacity optimization seems sufficient and smarter than messing with layer indexes. There are only 2 issues:

  1. How would I handle preload of such layer? Would listening to textureStore start working right after changing the opacity? The problem is I am already using opacity to wait for the previewFaces to load.
  2. After such layer is shown and then opacity changed back to 0, will Viewer unload the tiles as needed to regain memory? Is this by methods you mentioned? By calling TextureStore#clear or TextureStore#clearNotPinned ?

Many thanks for your help!

jordyno commented 4 years ago

@tjgq What do you think about adding something like layer#visible property to Marzipano? Layers set to not visible would return zero visible tiles in layer#visibleTiles. Would that work?

jordyno commented 4 years ago

@campmdabt thanks for the demo! It demonstrates nicely the multilayer functions in real world!

tjgq commented 4 years ago

Preloading would still work if you implement the optimization as I suggested above. Even when the Stage tells the TextureStore that no tiles are visible, pinned tiles will remain cached.

Once the optimization kicks in, the TextureStore will hold the pinned tiles + up to N previously visible tiles (LRU). Call TextureStore#clearNotPinned after setting the opacity to zero if you don't want the previously visible tiles to remain loaded.

jordyno commented 4 years ago

Thanks @tjgq I will try your optimization and report. It will be necessary anyway. Even with just 2 layers now and the other layer set to zero opacity, I am getting: [Violation] 'requestAnimationFrame' handler took 65ms

jordyno commented 4 years ago

@tjgq getting back to this optimization now. It does help, but even at 0 opacity, the preview is still loaded, which I would like to control. That is, when in 0 opacity, I'd like to load preview of specific layers only. What would be the easiest way?

richjava commented 3 years ago

@campmdabt I'm interested in how you get a layer source from a URL (to the tiles directory). In your code you have:

const newLayer = mScene.createLayer({
        source: createImageSource(newPanoBaseUrl),
        geometry: origLayer.geometry(), // keep the geometry the same as it was
        pinFirstLevel: true,
        layerOpts: {
            effects: {
                opacity: 0, // start out invisible until everything is loaded since this new layer is created on top
                hide: true
            }
        }
    });

Could you please share what your createImageSource(newPanoBaseUrl) function looks like (or explain how it works)?

campmdabt commented 3 years ago

Could you please share what your createImageSource(newPanoBaseUrl) function looks like (or explain how it works)?

Sorry for the late response, unsure why I didn't get a notification?

Anyway, see below. I use Krpano to create the tiles for use here, right now they're all full directional tiles, 1 to a side, 6 in total.


 * Creates a source to use with a new Marzipano scene/layer
 * @param {string} panoBaseUrl Base URL for the render tiles. Basically the folder where the pano tiles are stored.
 */
function createImageSource(panoBaseUrl) {
    return Marzipano.ImageUrlSource.fromString(
        `${panoBaseUrl}/pano_{f}.jpg`, { concurrency: 10, retryDelay: 1000, cubeMapPreviewUrl: `${panoBaseUrl}/preview.jpg`, cubeMapPreviewFaceOrder: 'lfrbud' }
    );
}