Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

map.setStyle() #9600

Closed
tobiu opened this issue Apr 19, 2020 · 2 comments
Closed

map.setStyle() #9600

tobiu opened this issue Apr 19, 2020 · 2 comments

Comments

@tobiu
Copy link

tobiu commented Apr 19, 2020

Related to #9598

I created 2 styles using mapbox studio. First one extending Monochromatic light, second one using Monochromatic dark. Same layers.

Using the maps load event, i am adding a couple new layers (e.g. heatmaps).

There is a switch theme (style) button in place, to toggle the dark & light styles.

When doing so, the engine failed. It actually failed silently, just using console.warn()

Unable to perform style diff: Unimplemented: setSprite.. Rebuilding the style from scratch.

Since it is not a real error, you can not catch it.

Afterwards, there is a full(!) redraw of the map.

On top of this, the custom layers & sources which i added using the maps load event are lost.

Looking into the map instance more in depth and it turns out that custom layers and sources get saved on the map style instead of on the map instance itself. Feels like an architectural flaw, unless there are good reason which i am missing.

So, i invested time to fix this on my end and would like to share the result.
You could probably use it as a workaround fix, just make sure to double-check first if the amount of layers are the same (not needed for my use case).

    /**
     *
     * @param {Object} data
     * @param {String} data.accessToken
     * @param {String} data.id
     * @param {Object|String} data.style
     */
    setStyle(data) {
        const me = this;

        if (!me.scriptsLoaded || !me.hasMap(data.id)) {
            // todo
        } else {
            if (Neo.isString(data.style)) {
                if (data.style.indexOf('mapbox://styles/') === 0) {
                    data.style = data.style.substring(16);
                }

                if (me.styleMap[data.style]) {
                    me.applyStyleObject(me.maps[data.id], me.styleMap[data.style]);
                } else {
                    fetch(`https://api.mapbox.com/styles/v1/${data.style}?access_token=${data.accessToken}`)
                        .then(response => response.json())
                        .then(styleJson => me.applyStyleObject(me.maps[data.id], styleJson, data.style))
                }
            }

            // map.setStyle breaks with only a console.warn()
            // => causing a full repaint, losing custom sources & layers
            // map.setStyle(data.style);
        }
    }

    /**
     *
     * @param {Object} map
     * @param {Object} styleJson
     * @param {String} [name]
     */
    applyStyleObject(map, styleJson, name) {
        if (name) {
            this.styleMap[name] = styleJson;
        }

        styleJson.layers.forEach(layer => {
            Object.entries(layer.paint).forEach(([key, value]) => {
                map.setPaintProperty(layer.id, key, value);
            });
        });
    }

This works pretty fine, no redraw and using animations. It leads to a question though: is there something like a bulk update mode? applyStyleObject() is calling map.setPaintProperty() around 200 times. If there was something like map.suspendUpdates() & resumeUpdates(), the performance should be faster. Is there something like this in place?

Here is the full example as a live demo:

https://neomjs.github.io/pages/node_modules/neo.mjs/dist/production/apps/covid/index.html#mainview=mapboxglmap

Hit the Theme Dark / Light button multiple times (it will cache the style json, so after toggling 2+ times there are no ajax calls and you can check the pure rendering performance.

Thanks and best regards, Tobias

@andrewharvey
Copy link
Collaborator

Looking into the map instance more in depth and it turns out that custom layers and sources get saved on the map style instead of on the map instance itself. Feels like an architectural flaw, unless there are good reason which i am missing.

Mapbox GL JS doesn't have a concept of a basemap and overlay layers, and I see that as a big architectural advantage as you now have a single map style made up of layers and sources and your map can be designed and crafted together as one.

Unable to perform style diff: Unimplemented: setSprite.. Rebuilding the style from scratch.

Are your two styles using different sprites? If you use the same sprite for both styles, it should mean it doesn't need to do a full reload/refresh.

#2058 is tracking implementing this feature which would remove the warning.

On top of this, the custom layers & sources which i added using the maps load event are lost.

That's expected behaviour, there is some discussion about this at #4006

@asheemmamoowala
Copy link
Contributor

Closing this as a duplicate and to keep the conversation under #2058.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants