Raster sources are not diffed properly by setStyle

[tgecho]

mapbox-gl-js version: v0.29.0

Steps to Trigger Behavior

1. Call map.setStyle a multiple times with the same raster source object.

Expected Behavior

setStyle should do nothing if the passed in style object is identical to the last one.

Actual Behavior

The raster sources are removed and then added back every time even though nothing changed, causing an unpleasant flash.

In this fiddle, it calls setStyle with the same style object used to init the map after 3 seconds. Note that the raster tiles flash as they're reloaded. https://jsfiddle.net/e7co8ohd/4/

This is happening because RasterTileSource.serialize returns the same object shape no matter what was passed in.

serialize() {
    return {
        type: 'raster',
        url: this.url,
        tileSize: this.tileSize,
        tiles: this.tiles
    };
}

If you don't specify any of these fields, the serialized version will still return them as undefined, which is unfortunately different from being not defined at all. In addition, if you have a url field you're not allowed to have a tiles field, but the source will include the tiles field it found at the url endpoint.

My current ugly "fixed" serialize function looks like this. I can throw up a PR if it seems acceptable, but I feel like it might make more sense to try to preserve the original passed in spec more explicitly rather than special casing these fields.

serialize() {
    const serialized = {type: 'raster'};
    if (this.url) {
        serialized.url = this.url;
    } else if (this.tiles) {
        serialized.tiles = this.tiles;
    }
    if (this.tileSize) {
        serialized.tileSize = this.tileSize;
    }
    return serialized;
}

[anandthakker]

Thanks for such a clear report and test case @tgecho!

I feel like it might make more sense to try to preserve the original passed in spec more explicitly rather than special casing these fields.

Yep, I agree this is probably the way we want to go. While I think in general we've tended to avoid storing passed-in options after they're used to construct an object, I think this is a case where it makes sense to do so. cc @jfirebaugh

[tgecho]

@anandthakker Just found another similar case. interactive: true on a layer triggers a full style rebuild with Unimplemented: setLayerProperty.. Rebuilding the style from scratch. in the console: https://jsfiddle.net/g24hxLfq/1/

Not sure if it's worth a full new issue. As an aside, we made a new fork of our internal style and this property appeared on all of our layers for some reason...

[anandthakker]

@tgecho looks like your example there is using 0.29 -- could you check to see if this still occurs using the latest version (0.32.1)?

[tgecho]

Whoops. I was running an up-to-date version when I discovered it, but I forgot to update the fiddle when I cloned it. This still shows the issue: https://jsfiddle.net/g24hxLfq/1/

[nextstopsun]

Style spec allows passing additional properties, which are quite handy. They do serialise nicely for vector sources, but unfortunately get lost for raster ones.

[cubodehelio]

I'm experiencing this issue on 0.37.0 when using setStyle on a satellite based theme. Is that possible? I'm trying to follows a reactive approach using Vue.js. With vector based themes like dark everything looks and behave cool, but with a satellite based style every little change (like changing the layout visibility on a single layer) triggers a full reload.

[cubodehelio]

fiddle here: https://jsfiddle.net/5yxgb3bs/3/ It seems like all the custom polygons and such remains intact but not the rasters

[cubodehelio]

any update on this?

[cubodehelio]

HI guys, sorry. I'm still having this issue under some circumstances with v.0.39.1. I updated the previous fiddle: https://jsfiddle.net/5yxgb3bs/7/. In the example I'm changing the layout.visibility on a single layer after a timeout and a full reload is triggered. This happen even if I comment the line that made the change (use setStyle with no style changes at all). Also the console shows the warning:

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

I suspect that the problem arise from this part of the style:

"sources": {
  "composite": {
    "url": "mapbox://mapbox.mapbox-streets-v7",
    "type": "vector"
  },
  "mapbox://mapbox.satellite": {
    "url": "mapbox://mapbox.satellite",
    "type": "raster",
    "tileSize": 256
  }

But i can't be sure though.

[jfirebaugh]

@cubodehelio That looks like a different issue; please open a new issue.