mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
11.15k stars 2.21k forks source link

Option to keep certain styles and layers on setStyle() #4006

Open indus opened 7 years ago

indus commented 7 years ago

Motivation

Injecting custom layers into Mapbox core styles is a common use case and confusing to users, especially when a user wants to swap out the core style and keep the custom layers.

Design Alternatives

3979 current approach

4000 alternative approach (draw-back: need for predefined nested styles)

Design

I would like to suggest an option that would allow the user to copy some layers/sources over to a new style to keep them after setting a new style.

Mock-Up

//layers appended to the end of the new layer array (in order)
setStyle('mapbox://styles/mapbox/' + layerId + '-v9', {
  copySources: ['states'],
  copyLayers: ['states']
})

// layer added before another layer in the new layer array (like addLayer(layer, before))
setStyle('mapbox://styles/mapbox/' + layerId + '-v9', {
  copySources: ['states'],
  copyLayers: [{'states':'waterway-label' }]
})

OR

//layers appended to the end of the new layer array (in order)
setStyle('mapbox://styles/mapbox/' + layerId + '-v9', {
  copy: {
         sources: ['states'],
         layers: ['states']
   }
})

Concepts

It would work like a normal setStyle but after the new style is loaded the copying from the old to the new style would take place before the map gets updated.

Implementation

the copying-code likely could get called at the end of the style-diffing procedure and could look something like that lodash-pseudocode:

var style = _.union( newStyle, _.pick( oldStyle, copyoption ))
anandthakker commented 7 years ago

Thanks for this proposal @indus!

Separate from the copyLayers part and the alternative nested styles proposal in #4000, I think that copySources (or something like it) might fill a current shortcoming of the setStyle API that also wouldn't be addressed by nested styles.

GeoJSONSource in particular may carry a large amount of state if its data is set to a literal GeoJSON value rather than a URL. As @averas describes here, copying this over from the existing style into a new target style when using map.setStyle() may be inconvenient and potentially a performance problem.

While it's true that using a reactive paradigm and immutable objects could mostly the ergonomic and performance issues (respectively), we can't assume that this is necessarily a valid option for all/most users. Thus, I think it makes sense to provide a means for developers to short-circuit the diff for particular sources.

lucaswoj commented 7 years ago

Another design that may address the same problem without requiring the reintroduction of imperative concepts is something akin to React's key attribute -- some developer-provided metadata that hints to GL JS when a source can be copied.

anandthakker commented 7 years ago

As I understand it, the copySources option basically is developer-provided metadata, except perhaps for the word "copy" in its name. It is an option that dictates the interpretation of the main stylesheet parameter being given to setStyle(). @lucaswoj can you elaborate on "reintroduction of imperative concepts"?

lucaswoj commented 7 years ago

A goal of the "smart" setStyle project was to make it possible to use GL JS in a reactive programming paradigm in which each Map#setStyle call need make no assumptions about the current state of the map. copying a source breaks that property, at least conceptually.

Renaming the property to preserveSources would help a little 😄

Having GL JS able to decide whether to preserve or replace a source without the user explicitly having to make this performance decision is even better.

anandthakker commented 7 years ago

Having GL JS able to decide whether to preserve or replace a source without the user explicitly having to make this performance decision is even better.

If we want

{
  sources: {
     myVectorSouce: { ... },
     // there used to be a geojson source here, but now it's gone
  }
}

to sometimes mean "make the style like this -- i.e. if there are any sources that aren't myVectorSource, remove them", and other times to mean "make sure myVectorSource is there, but preserve the existing geojson source", then there will have to be some way for the user to provide explicit information that goes beyond declaring the target style.

Adding something like React's key attribute to the target style is one such means for providing that information--e.g., we could allow an extra hash property on any source, and if the target source's hash matches the existing source with that id, then we skip diffing. It could be argued that this is semantically cleaner than preserveSources, and in the abstract, I'd agree. However, I think that it's practically a bit messy, since it would require GL JS to track a new piece of state (the hash property) on each source.

preserveSources would be less invasive, but (a) is less conceptually clean, and (b) would re-introduce awkwardness for developers using reactive approach.

🤔

kaitok commented 7 years ago

Whats the status now this issue?

songololo commented 6 years ago

Any possible updates on this item? It would be very helpful to have an option to save layer states on style updates within reactive environments.

andrewharvey commented 6 years ago

in case it helps anyone the workaround I'm using at the moment is below. It constructs a new Style object retaining the sources and layers you want copied across to the new style.

import { json } from 'd3-request';
function swapStyle(styleID) {
  var currentStyle = map.getStyle();
  json(`https://api.mapbox.com/styles/v1/mapbox/${styleID}?access_token=${mapboxgl.accessToken}`, (newStyle) => {
    newStyle.sources = Object.assign({}, currentStyle.sources, newStyle.sources); // ensure any sources from the current style are copied across to the new style
    var labelIndex = newStyle.layers.findIndex((el) => { // find the index of where to insert our layers to retain in the new style
      return el.id == 'waterway-label';
    });
    var appLayers = currentStyle.layers.filter((el) => { // app layers are the layers to retain, and these are any layers which have a different source set
      return (el.source && el.source != "mapbox://mapbox.satellite" && el.source != "composite");
    });
    appLayers.reverse(); // reverse to retain the correct layer order
    appLayers.forEach((layer) => {
      newStyle.layers.splice(labelIndex, 0, layer); // inset these layers to retain into the new style
    });
    map.setStyle(newStyle); // now setStyle
  });
}
indus commented 6 years ago

I'm using a factory function to recreate a complete stylesheet everytime the configuration of layers is supposed to change; something like this:

let getStyle = function (map_class) {
    let layers = [];

//...

    if (map_class.basemap)
        layers = [...layers_base]; // from a official stylesheet

    if (map_class.elevation && map_class.hillshade)
        layers.push({
            "id": "elevation_hillshade",
            "source": "elevation_hillshade",
            "type": "raster"
        })
    else {
        map_class.elevation && layers.push({
            "id": "elevation",
            "source": "elevation",
            "type": "raster"
        })
        map_class.hillshade && layers.push({
            "id": "hillshade",
            "source": "hillshade",
            "type": "raster"
        })
    }

// ... 30-50 more if-statements

    let style = {
        version: 8,
        glyphs: "http://localhost:8084/fonts/{fontstack}/{range}.pbf",
        sources: sources, // from a global var
        layers: layers
    };
    return style;
}

this.on("state_changed", function (state) {
    map.setStyle(getStyle(state.map_class))
})

When I compare it to the class based system we had I still think its ugly because the stylesheet is no longer a plain JSON object but a function (and you wouldn't be able to transfer it easily to a native library). It also may be less performant (?), but at least it works for me after the deprecation and until there is some progress towards an incremental/partial setStyle

anandthakker commented 6 years ago

Alternative proposal at #6701

ggerber commented 4 years ago
import { json } from 'd3-request';
function swapStyle(styleID) {
  var currentStyle = map.getStyle();
  json(`https://api.mapbox.com/styles/v1/mapbox/${styleID}?access_token=${mapboxgl.accessToken}`, (newStyle) => {
    newStyle.sources = Object.assign({}, currentStyle.sources, newStyle.sources); // ensure any sources from the current style are copied across to the new style
    var labelIndex = newStyle.layers.findIndex((el) => { // find the index of where to insert our layers to retain in the new style
      return el.id == 'waterway-label';
    });
    var appLayers = currentStyle.layers.filter((el) => { // app layers are the layers to retain, and these are any layers which have a different source set
      return (el.source && el.source != "mapbox://mapbox.satellite" && el.source != "composite");
    });
    appLayers.reverse(); // reverse to retain the correct layer order
    appLayers.forEach((layer) => {
      newStyle.layers.splice(labelIndex, 0, layer); // inset these layers to retain into the new style
    });
    map.setStyle(newStyle); // now setStyle
  });
}

the style object for Mapbox Streets has a 'sprite' property with value "mapbox://sprites/mapbox/streets-v11" if one changes the baselayer to Mapbox Satellite by doing a http request, the returned style object has a sprite property with value "mapbox://sprites/mapbox/satellite-v9"

My understanding is that these two different sprite values will force setStyle() to rebuild the style from scratch, since it cannot perform a diff as the underlying setSprite() is not implemented.

Should one simply accept that when switching from streets to satellite a complete rebuild of the style is required when using setStyle() or is there a way to reconcile the two different sprite values?

andrewharvey commented 4 years ago

Should one simply accept that when switching from streets to satellite a complete rebuild of the style is required when using setStyle() or is there a way to reconcile the two different sprite values?

@ggerber One approach (out of a few) that I use is in Mapbox Studio I'd download a Streets style, and download a Satellite style, then manually concatenate the layers list and combine the sources from the style JSON, then upload that as a Style to Mapbox Studio and then upload both sets of icons to that style, so you end up with a style containing both streets and satellite but in one style with one spritesheet, then just toggle layers on/off when switching "styles".

anthony-bernardo commented 4 years ago

Unfortunatly this workaround do not work for me.

Is there an other way ?

ericbrumfield commented 4 years ago

I was able to get a work around from stack overflow working here

In addition, if you've loaded any custom images such as marker/symbol images you'll have to add them back as well. It took me a few, but I figured out that you'd have to do this in the 'style.load' event, making sure you try/catch and gulp the exception for map.removeImage followed by map.addImage. I don't prefer try/catch gulping like that, but there wasn't a method like getImage that I could find to check if it exists or not in the map already.

stevage commented 3 years ago

@andrewharvey in case it helps anyone the workaround I'm using at the moment is below. It constructs a new Style object retaining the sources and layers you want copied across to the new style.

I had to tweak this slightly to get it to work - looks like the name of the Satellite source has changed to mapbox.

I also tweaked a few other things for readability.

// styleID should be in the form "mapbox/satellite-v9"
async function switchBasemap(map, styleID) {
    const { data: newStyle } = await axios.get(
        `https://api.mapbox.com/styles/v1/${styleID}?access_token=${mapboxgl.accessToken}`
    );
    const currentStyle = map.getStyle();
    // ensure any sources from the current style are copied across to the new style
    newStyle.sources = Object.assign(
        {},
        currentStyle.sources,
        newStyle.sources
    );

    // find the index of where to insert our layers to retain in the new style
    let labelIndex = newStyle.layers.findIndex((el) => {
        return el.id == 'waterway-label';
    });

    // default to on top
    if (labelIndex === -1) {
        labelIndex = newStyle.layers.length;
    }
    const appLayers = currentStyle.layers.filter((el) => {
        // app layers are the layers to retain, and these are any layers which have a different source set
        return (
            el.source &&
            el.source != 'mapbox://mapbox.satellite' &&
            el.source != 'mapbox' &&
            el.source != 'composite'
        );
    });
    newStyle.layers = [
        ...newStyle.layers.slice(0, labelIndex),
        ...appLayers,
        ...newStyle.layers.slice(labelIndex, -1),
    ];
    map.setStyle(newStyle);
}
sirmspencer commented 3 years ago

@stevage Do you know if your method maintains cached layers? I have tried the method were I save the source/layers from the old source and then add them to the map after the style is updated. Im wondering if by keeping the existing soruce/layers in the new style if mapbox is able to keep the layer cache.

stevage commented 3 years ago

It seems to - the preserved layers stay visible on the map, they don't even flicker or disappear.

sirmspencer commented 3 years ago

@andrewharvey (instead of posting a large quote reply, see two posts up)

Thanks for this code sample. It inspired me to get a closer to what I need where the layers keep their cache when the base map is changed. The piece you are missing is that if the sprite value is modified, mapbox will do a hard refresh on all the layers. In all my layers I dont have custom sprites, I just grab one to set as the default. Then each time I make the fetch call, I set newStyle.sprite = to the default sprite url. Im in clojure and have some helper functions but heres the gist:

(go
    (let [cur-style (js->clj (.getStyle @the-map))
          new-style (js->clj (<! (u/fetch-and-process source {} (fn [res] (.json res)))))
          sources   (->> (get cur-style "sources")
                         (u/filterm (fn [[k _]] (str/starts-with? (name k) "fire"))))
          layers    (->> (get cur-style "layers")
                         (filter (fn [l] (str/starts-with? (get l "id") "fire"))))]
      (-> @the-map (.setStyle (clj->js (-> new-style
                                           (assoc "sprite" "mapbox://sprites/user/id1/id2")
                                           (update "sources" merge sources)
                                           (update "layers" concat layers)))))))
stevage commented 3 years ago

Ah, in my particular case I have hacked the two style sheets to use the same spritesheet, using mbsprite

WGrobler commented 3 years ago

Thanks @stevage for the workaround.

I would suggest moving the const currentStyle = map.getStyle(); after the axios.get function. I had an issue where one of my external functions were adding a layer while the axios function were waiting for data from mapbox. This resulted in the new layer getting lost during the basemap change.

Because axios get is awaited, there is some delay (however small) due to the external request, this introduces the possibility that the output of map.getStyle() changes before the function is finished.

There is still a (extremely) small chance that an external function would modify map while the switchBasemap function is running, but for this to happen the external function needs to be called in the micro/nano seconds between the end of the axios await and the final setStyle. I don't think that's likely to happen.

stevage commented 3 years ago

That's a great point, thanks - updated the code. (Since I suspect people come here from Google and use that code...)

rowanwins commented 2 years ago

Another thing I discovered when doing style changes is that feature-state isn't retained using the approach outlined by @stevage and @andrewharvey . So I came up with the following addition which works reasonably well, although admittedly it's a bit hacky reaching into private properties but it works.

async function switchBasemap (map, styleID) {
   ....
  const tempState = {}
  appLayers.forEach(lyr => {
    tempState[lyr.source] = Object.assign({}, map.style.sourceCaches[lyr.source]._state)
  })
   ....
  map.setStyle(newStyle)
  map.once('styledata', () => {
    reapplyState(appLayers, tempState)
  })
}

function reapplyState (appLayers, tempState) {
    appLayers.forEach(lyr => {
      const source = tempState[lyr.source]
      const vtLyrs = Object.keys(source.state)
      if (vtLyrs.length > 0) {
        vtLyrs.forEach((sourceLayer) => {
          const ids = Object.keys(source.state[sourceLayer])
          ids.forEach(id => {
            map.setFeatureState({
              source: lyr.source,
              sourceLayer,
              id
            }, source.state[sourceLayer][id])
          })
        })
      }
    })
}
markhicken commented 2 years ago

Using fetch instead of axios...

// styleID should be in the form "mapbox/satellite-v9"
async function switchBaseMap(map, styleID) {
  const response = await fetch(
    `https://api.mapbox.com/styles/v1/${styleID}?access_token=${mapboxgl.accessToken}`
  );
  const responseJson = await response.json();
  const newStyle = responseJson;

  const currentStyle = map.getStyle();
  // ensure any sources from the current style are copied across to the new style
  newStyle.sources = Object.assign({},
    currentStyle.sources,
    newStyle.sources
  );

  // find the index of where to insert our layers to retain in the new style
  let labelIndex = newStyle.layers.findIndex((el) => {
    return el.id == 'waterway-label';
  });

  // default to on top
  if (labelIndex === -1) {
    labelIndex = newStyle.layers.length;
  }
  const appLayers = currentStyle.layers.filter((el) => {
    // app layers are the layers to retain, and these are any layers which have a different source set
    return (
      el.source &&
      el.source != 'mapbox://mapbox.satellite' &&
      el.source != 'mapbox' &&
      el.source != 'composite'
    );
  });
  newStyle.layers = [
    ...newStyle.layers.slice(0, labelIndex),
    ...appLayers,
    ...newStyle.layers.slice(labelIndex, -1),
  ];
  map.setStyle(newStyle);
}
chriszrc commented 1 year ago

I recently extended this plugin that adds a base style switcher to automatically preserve user-added layers. It does so by keeping a dictionary of the layers that mapbox adds by default for any base style (roads, satellite, etc), and also tracking the layers/sources that are present in a style before switching, and re-added any styles that mapbox didn't add. Not sure it's the best strategy, but it really highlights the pain of user-added layers not being preserved when switching the map style-

https://github.com/chriszrc/style-switcher

SourceCipher commented 1 year ago

I really dont understand how is that not by design? This is ridiculous to handle the layers you added to the map since there could be hundreds of them and in some complicated projects its not easy to do that. At least they could provide the flag to keep the layers or discard.

SourceCipher commented 1 year ago

So after doing some research I came up with this solution. It works well but obviously its a workaround.

Now for each getSource and addLayer methods you will add your flag to identify which layers have been added by you (lets say myLayer)

map.getSource('sourceName).setData({
      type: 'FeatureCollection',
      features: [],
      myLayer: true
})

map.addLayer({
      id: id,
      type: type,
      source: source,
      metadata: { myLayer: true },
      // Rest of the properties
})
public mapStyleSwitcher = () => {
    // This will get all the layers and sources of your current map
    const layers = map.getStyle().layers
    const sources = map.getStyle().sources

    // Filter them all out to keep only your added layers
    const filteredLayers = layers.filter((l: any) => l?.metadata?.myLayer)
    const filteredSources = {}

    // Filter them all out to keep only your added sources
    Object.keys(sources).forEach((key: any) => {
      if (sources[key]?.data?.myLayer) {
        filteredSources[key] = sources[key]
      }
    })

    // Custom style you can find in mapbox studio smth like this mapbox://styles/username/styleCode
    axios
      .get(`https://api.mapbox.com/styles/v1/username/styleCode?access_token=yourMapboxToken`)
      .then((result: any) => {
        const newStyle = result.data

        // Merge newly fetched layers and sources with your filtered layers and sources
        newStyle.layers = [...newStyle.layers, ...filteredLayers]
        newStyle.sources = { ...newStyle.sources, ...filteredSources }

        // Append the new style to your map and  - vuola!
        map.setStyle(newStyle)
      })
      .catch(error => {
        console.log(error)
      })
  }

If someone is interested in seeing it a working demo, let me know so I can add it to the codepen. Also if anyone thinks its a stupid/dangerous workaround also let me know 😄

SourceCipher commented 1 year ago

@niZaliznyak You know you can comment here ant tag people? You dont need to make new issues on any repos 😄

niZaliznyak commented 1 year ago

Sorry. I didn't know :smiling_face_with_tear:

niZaliznyak commented 1 year ago

Here is the solution in the official documentation: https://docs.mapbox.com/mapbox-gl-js/example/style-switch/

SourceCipher commented 1 year ago

The example is not a proper way to do it because they just give you an option to redraw the same layer. That not really keeping the layers created.

In many cases you will have many layers with various states which might have changed during the usage and redrawing them is not an option.

Also for the sake of performance is better to keep the layers you have and snap them in place instead of redrawing them all, as for example, if you have 100k markers, 10k polygons, lines etc, to redraw them it will take long time.

MaxHammermann commented 1 year ago

@stevage What are your CORS settings? I'm getting CORS errors despite it being a public API responding with "*" when called by mapbox itself?

ricardohsweiss commented 2 weeks ago

Any updates on this one? Im having a hard time to make the symbol layers appear after switching the style, Im using the following code snippet, and my map style is a custom one, made with mapbox studio if that matters

Using fetch instead of axios...

// styleID should be in the form "mapbox/satellite-v9"
async function switchBaseMap(map, styleID) {
  const response = await fetch(
    `https://api.mapbox.com/styles/v1/${styleID}?access_token=${mapboxgl.accessToken}`
  );
  const responseJson = await response.json();
  const newStyle = responseJson;

  const currentStyle = map.getStyle();
  // ensure any sources from the current style are copied across to the new style
  newStyle.sources = Object.assign({},
    currentStyle.sources,
    newStyle.sources
  );

  // find the index of where to insert our layers to retain in the new style
  let labelIndex = newStyle.layers.findIndex((el) => {
    return el.id == 'waterway-label';
  });

  // default to on top
  if (labelIndex === -1) {
    labelIndex = newStyle.layers.length;
  }
  const appLayers = currentStyle.layers.filter((el) => {
    // app layers are the layers to retain, and these are any layers which have a different source set
    return (
      el.source &&
      el.source != 'mapbox://mapbox.satellite' &&
      el.source != 'mapbox' &&
      el.source != 'composite'
    );
  });
  newStyle.layers = [
    ...newStyle.layers.slice(0, labelIndex),
    ...appLayers,
    ...newStyle.layers.slice(labelIndex, -1),
  ];
  map.setStyle(newStyle);
}
niZaliznyak commented 2 weeks ago

@ricardohsweiss You have to add listener on style download. My solution was, add sources and layers again after the style.load event:

map.on("style.load", () => {
        this.wmsLayers?.forEach((layer) => {
          addWMSLayer(this.map, layer);
        });
        this.showSourceAndLayers = true;

        addScannersSourceAndLayers(
          this.map,
          this.scannersMapData,
          this.onScannerClick,
          this.openScannerContext,
          isFirstLaunch
        );

        addCommentsSourceAndLayers(
          this.map,
          this.commentsMapData,
          this.onCommentClick,
          isFirstLaunch
        );

        isFirstLaunch = false;
      });
    },
ricardohsweiss commented 2 weeks ago

@niZaliznyak I just found out that actually the issue is regarding the image of the symbols, not the layers itself. When we set the new style mapbox complains about missing images (that i previously loaded with loadImage and addImage functions).

Anyone has an idea of how can I readd the images from one style to the other without having to add them again manually? I have images that are in the local bundle, but also some that are loaded through url. This is scattered through several places in the application, would be really helpful to just "migrate" them to the new style

ricardohsweiss commented 1 week ago

So after some research I realized that was not possible/straightforward to keep the images in the map sprite after changing the map style via setStyle, so my solution was to store all imgs in cdn, and keep track of an "imageCollection" global object containing the id and url of the images in the map.

Also made some changes to the function of adding the sources and layers again in the map, as I use custom mapbox styles created in mapbox studio, I was not happy by doing a fetch or axios call to get the map style, i rather let mapbox resolve the url and fetch for me as those can change in the future.

Here is my updated code with typescript if anyone is interested:

const layersToIgnore = [
  'place-mapbox-default-layers-here'
]

async function switchBaseMap(
  map: mapboxgl.Map,
  styleUri: string,
  mapStyle: MapStyle
) {
  const currentStyle = map.getStyle()
  const imageCollection = getImageCollectionFromGlobalStorage()

  const allCurrentSources = Object.keys(currentStyle.sources).reduce(
    (acc: { id: string; source: AnySourceData }[], el: string) => {
      if (el !== 'composite') {
        acc.push({
          id: el,
          source: { ...currentStyle.sources[el] }
        })
      }
      return acc
    },
    []
  )

  const allCurrentLayers = currentStyle.layers
    .filter(
      el =>
        !layersToIgnore.includes(el.id) ||
        ((el as SymbolLayer).source &&
          (el as SymbolLayer).source != 'mapbox://mapbox.satellite' &&
          (el as SymbolLayer).source != 'mapbox' &&
          (el as SymbolLayer).source != 'composite')
    )

  map.setStyle(styleUri)

  map.once('style.load', async () => {
    await addImagesAsync(map, imageCollection)

    allCurrentSources.forEach(source => {
      map.addSource(source.id, source.source)
    })

    allCurrentLayers.forEach(layer => {
      map.addLayer(layer)
    })
  })
}

Hope this saves some time for those who had the same issue 😃