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

Smart setStyle (part 1) #3621

Merged
merged 2 commits into from
Nov 22, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
24 changes: 21 additions & 3 deletions js/ui/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -645,18 +645,36 @@ 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 {Object} [options]
* @param {boolean} [options.diff=true] If false, 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, options) {
const shouldTryDiff = (!options || options.diff !== false) && 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 diff option is false', (t) => {
const style = createStyle();
const map = createMap({ style: style });
t.stub(map.style, 'setState', () => {
t.fail();
});

const previousStyle = map.style;
map.setStyle(style, {diff: false});
t.ok(map.style && map.style !== previousStyle);
t.end();
});

t.end();
});

Expand Down