Skip to content

Commit

Permalink
Add Style#setState, using mapbox-gl-style-spec's diff module
Browse files Browse the repository at this point in the history
  • Loading branch information
Anand Thakker authored and jfirebaugh committed Nov 22, 2016
1 parent 47b017c commit 92956de
Show file tree
Hide file tree
Showing 8 changed files with 18,331 additions and 6 deletions.
8,979 changes: 8,979 additions & 0 deletions debug/dark-v9.js

Large diffs are not rendered by default.

9,131 changes: 9,131 additions & 0 deletions debug/light-v9.js

Large diffs are not rendered by default.

42 changes: 42 additions & 0 deletions debug/setstyle.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<!DOCTYPE html>
<html>
<head>
<title>Mapbox GL JS debug page</title>
<meta charset='utf-8'>
<meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=no">
<link rel='stylesheet' href='/dist/mapbox-gl.css' />
<style>
body { margin: 0; padding: 0; }
html, body, #map { height: 100%; }
</style>
</head>

<body>
<div id='map'></div>

<script src='/dist/mapbox-gl-dev.js'></script>
<script src='/debug/access_token_generated.js'></script>
<script src='/debug/dark-v9.js'></script>
<script src='/debug/light-v9.js'></script>
<script>
// these are set in dark-v9.js and light-v9.js
var dark = window.darkv9;
var light = window.lightv9;

var style = dark;
var map = window.map = new mapboxgl.Map({
container: 'map',
zoom: 2,
center: [-77.01866, 38.888],
style: style,
hash: style
});

setInterval(function () {
style = (style === dark ? light : dark);
map.setStyle(style);
}, 2000);

</script>
</body>
</html>
69 changes: 68 additions & 1 deletion js/style/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,29 @@ const styleSpec = require('./style_spec');
const MapboxGLFunction = require('mapbox-gl-function');
const getWorkerPool = require('../global_worker_pool');
const deref = require('mapbox-gl-style-spec/lib/deref');
const diff = require('mapbox-gl-style-spec/lib/diff');

const supportedDiffOperations = util.pick(diff.operations, [
'addLayer',
'removeLayer',
'setPaintProperty',
'setLayoutProperty',
'setFilter',
'addSource',
'removeSource',
'setLayerZoomRange',
'setLight',
'setTransition'
// 'setGlyphs',
// 'setSprite',
]);

const ignoredDiffOperations = util.pick(diff.operations, [
'setCenter',
'setZoom',
'setBearing',
'setPitch'
]);

/**
* @private
Expand Down Expand Up @@ -291,6 +314,50 @@ class Style extends Evented {
this._updatedAllPaintProps = false;
}

/**
* Update this style's state to match the given style JSON, performing only
* the necessary mutations.
*
* May throw an Error ('Unimplemented: METHOD') if the mapbox-gl-style-spec
* diff algorithm produces an operation that is not supported.
*
* @returns {boolean} true if any changes were made; false otherwise
* @private
*/
setState(nextState) {
this._checkLoaded();

if (validateStyle.emitErrors(this, validateStyle(nextState))) return false;

nextState = util.extend({}, nextState);
nextState.layers = deref(nextState.layers);

const changes = diff(this.serialize(), nextState)
.filter(op => !(op.command in ignoredDiffOperations));

if (changes.length === 0) {
return false;
}

const unimplementedOps = changes.filter(op => !(op.command in supportedDiffOperations));
if (unimplementedOps.length > 0) {
throw new Error(`Unimplemented: ${unimplementedOps.map(op => op.command).join(', ')}.`);
}

changes.forEach((op) => {
if (op.command === 'setTransition') {
// `transition` is always read directly off of
// `this.stylesheet`, which we update below
return;
}
this[op.command].apply(this, op.args);
});

this.stylesheet = nextState;

return true;
}

addSource(id, source, options) {
this._checkLoaded();

Expand Down Expand Up @@ -473,7 +540,7 @@ class Style extends Evented {

const layer = this.getLayer(layerId);

if (filter !== null && this._validate(validateStyle.filter, `layers.${layer.id}.filter`, filter)) return;
if (filter !== null && filter !== undefined && this._validate(validateStyle.filter, `layers.${layer.id}.filter`, filter)) return;

if (util.deepEqual(layer.filter, filter)) return;
layer.filter = util.clone(filter);
Expand Down
22 changes: 19 additions & 3 deletions js/ui/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -645,18 +645,34 @@ class Map extends Camera {
}

/**
* Replaces the map's Mapbox style object with a new value.
* Updates the map's Mapbox style object with a new value. If the given
* value is style JSON object, compares it against the the map's current
* state and perform only the changes necessary to make the map style match
* the desired state.
*
* @param {Object|string} style A JSON object conforming to the schema described in the
* [Mapbox Style Specification](https://mapbox.com/mapbox-gl-style-spec/), or a URL to such JSON.
* @param {boolean} forceNoDiff Force a 'full' update, removing the current style and adding building the given one instead of attempting a diff-based update.
* @returns {Map} `this`
* @see [Change a map's style](https://www.mapbox.com/mapbox-gl-js/example/setstyle/)
*/
setStyle(style) {
setStyle(style, forceNoDiff) {
const shouldTryDiff = !forceNoDiff && this.style && style &&
!(style instanceof Style) && typeof style !== 'string';
if (shouldTryDiff) {
try {
if (this.style.setState(style)) {
this._update(true);
}
return this;
} catch (e) {
util.warnOnce(`Unable to perform style diff: ${e.message || e.error || e}. Rebuilding the style from scratch.`);
}
}

if (this.style) {
this.style.setEventedParent(null);
this.style._remove();

this.off('rotate', this.style._redoPlacement);
this.off('pitch', this.style._redoPlacement);
}
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"grid-index": "^1.0.0",
"mapbox-gl-function": "mapbox/mapbox-gl-function#41c6724e2bbd7bd1eb5991451bbf118b7d02b525",
"mapbox-gl-shaders": "mapbox/mapbox-gl-shaders#597115a1e1bd982944b068f8accde34eada74fc2",
"mapbox-gl-style-spec": "mapbox/mapbox-gl-style-spec#512126c802dbb8f282e9826b181f0d53da00daf2",
"mapbox-gl-style-spec": "mapbox/mapbox-gl-style-spec#77434bd0bdb623809c85afaa3da432ebe01e993f",
"mapbox-gl-supported": "^1.2.0",
"package-json-versionify": "^1.0.2",
"pbf": "^1.3.2",
Expand Down Expand Up @@ -62,7 +62,7 @@
"in-publish": "^2.0.0",
"jsdom": "^9.4.2",
"lodash.template": "^4.4.0",
"mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#5c89044bef42e8b557f60162b515f689e455de75",
"mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#cfa20f8e0de49c48d3bb08c1d4786aedbd082474",
"minifyify": "^7.0.1",
"npm-run-all": "^3.0.0",
"nyc": "^8.3.0",
Expand Down
54 changes: 54 additions & 0 deletions test/js/style/style.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,60 @@ test('Style#_resolve', (t) => {
t.end();
});

test('Style#setState', (t) => {
t.test('throw before loaded', (t) => {
const style = new Style(createStyleJSON());
t.throws(() => {
style.setState(style.serialize());
}, Error, /load/i);
style.on('style.load', () => {
t.end();
});
});

t.test('do nothing if there are no changes', (t) => {
const style = new Style(createStyleJSON());
[
'addLayer',
'removeLayer',
'setPaintProperty',
'setLayoutProperty',
'setFilter',
'addSource',
'removeSource',
'setLayerZoomRange',
'setLight'
].forEach((method) => t.stub(style, method, () => t.fail(`${method} called`)));
style.on('style.load', () => {
const didChange = style.setState(createStyleJSON());
t.notOk(didChange, 'return false');
t.end();
});
});

t.test('return true if there is a change', (t) => {
const initialState = createStyleJSON();
const nextState = createStyleJSON({
sources: {
foo: {
type: 'geojson',
data: { type: 'FeatureCollection', features: [] }
}
}
});

const style = new Style(initialState);
style.on('style.load', () => {
const didChange = style.setState(nextState);
t.ok(didChange);
t.same(style.stylesheet, nextState);
t.end();
});
});

t.end();
});

test('Style#addSource', (t) => {
t.test('throw before loaded', (t) => {
const style = new Style(createStyleJSON()),
Expand Down
36 changes: 36 additions & 0 deletions test/js/ui/map.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,16 @@ test('Map', (t) => {
});
});

t.test('passing null removes style', (t) => {
const map = createMap();
const style = map.style;
t.ok(style);
t.spy(style, '_remove');
map.setStyle(null);
t.equal(style._remove.callCount, 1);
t.end();
});

t.end();
});

Expand Down Expand Up @@ -277,6 +287,32 @@ test('Map', (t) => {
});
});

t.test('creates a new Style if diff fails', (t) => {
const style = createStyle();
const map = createMap({ style: style });
t.stub(map.style, 'setState', () => {
throw new Error('Dummy error');
});

const previousStyle = map.style;
map.setStyle(style);
t.ok(map.style && map.style !== previousStyle);
t.end();
});

t.test('creates a new Style if forceNoDiff parameter is set', (t) => {
const style = createStyle();
const map = createMap({ style: style });
t.stub(map.style, 'setState', () => {
t.fail();
});

const previousStyle = map.style;
map.setStyle(style, true);
t.ok(map.style && map.style !== previousStyle);
t.end();
});

t.end();
});

Expand Down

1 comment on commit 92956de

@lukasmartinelli
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that was an very smart move to do the diffing automatically in #setStyle - while still preserving a simple API for the user. Thanks a lot!! ❤️
Now I no longer need to diff the styles myself.

Please sign in to comment.