Skip to content

Commit

Permalink
Add "data" event (#3255)
Browse files Browse the repository at this point in the history
* source.change -> data

* source.load -> sourceload & data

* style.change -> data

* style.load -> data, styleload

* tile.add -> dataloading

* tile.load -> data

* tile.remove -> data

* Unrename "source.load" and "style.load"

* source.add & source.remove -> data

* layer.add & layer.remove -> data

* Remove isDataRemoved flag

* Add documentation for "data" event

* Fix setDataPerf benchmark utility

* Minor doc fix

* Add docs for dataloading event

* merge 'sprite' and 'style' into 'style'

* merge all several types into 'source'

* Refactor "set data perf" benchmark utility

* Ensure "Map#_update(true)" is only called after style mutations

* Ensure Attribution#_update is only called for "data[dataType=source]" events

* Attach Source, not SourceCache, to data events
  • Loading branch information
lucaswoj authored Sep 29, 2016
1 parent 62f15aa commit 1d373b0
Show file tree
Hide file tree
Showing 19 changed files with 150 additions and 169 deletions.
3 changes: 1 addition & 2 deletions bench/benchmarks/geojson_setdata_large.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@ module.exports = function() {

map.on('load', function() {
map = setupGeoJSONMap(map);
var source = map.getSource('geojson');

setDataPerf(source, 50, data, function(err, ms) {
setDataPerf(map.style.sources['geojson'], data, function(err, ms) {
if (err) return evented.fire('error', {error: err});
map.remove();
evented.fire('end', {message: formatNumber(ms) + ' ms', score: ms});
Expand Down
4 changes: 1 addition & 3 deletions bench/benchmarks/geojson_setdata_small.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ module.exports = function() {
map.on('load', function() {
map = setupGeoJSONMap(map);

var source = map.getSource('geojson');

setDataPerf(source, 50, featureCollection, function(err, ms) {
setDataPerf(map.style.sources['geojson'], featureCollection, function(err, ms) {
map.remove();
if (err) return evented.fire('error', {error: err});
evented.fire('end', {message: formatNumber(ms) + ' ms', score: ms});
Expand Down
36 changes: 17 additions & 19 deletions bench/lib/set_data_perf.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,29 @@
'use strict';

var NUM_TILES = 6;

module.exports = function(source, numCalls, geojson, cb) {
var tileCount = 0;
module.exports = function(sourceCache, data, callback) {
var sampleCount = 50;
var startTime = null;
var times = [];

source.on('tile.load', function tileCounter() {
tileCount++;
if (tileCount === NUM_TILES) {
tileCount = 0;
times.push(performance.now() - startTime);
var samples = [];

if (times.length < numCalls) {
sourceCache.on('data', function onData() {
if (sourceCache.loaded()) {
samples.push(performance.now() - startTime);
sourceCache.off('data', onData);
if (samples.length < sampleCount) {
startTime = performance.now();
source.setData(geojson);
sourceCache.clearTiles();
sourceCache.on('data', onData);
sourceCache.getSource().setData(data);
} else {
var avgTileTime = times.reduce(function (v, t) {
return v + t;
}, 0) / times.length;
source.off('tile.load', tileCounter);
cb(null, avgTileTime);
callback(null, average(samples));
}
}
});

startTime = performance.now();
source.setData(geojson);
sourceCache.getSource().setData(data);
};

function average(array) {
return array.reduce(function (sum, value) { return sum + value; }, 0) / array.length;
}
1 change: 1 addition & 0 deletions documentation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ toc:
- MapMouseEvent
- MapTouchEvent
- MapBoxZoomEvent
- MapDataEvent
- name: Types
description: |
These are types used in the documentation above to describe arguments,
Expand Down
3 changes: 2 additions & 1 deletion js/source/geojson_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ function GeoJSONSource(id, options, dispatcher) {
this.fire('error', {error: err});
return;
}
this.fire('data', {dataType: 'source'});
this.fire('source.load');
}.bind(this));
}
Expand Down Expand Up @@ -119,7 +120,7 @@ GeoJSONSource.prototype = util.inherit(Evented, /** @lends GeoJSONSource.prototy
if (err) {
return this.fire('error', { error: err });
}
this.fire('source.change');
this.fire('data', {dataType: 'source'});
}.bind(this));

return this;
Expand Down
3 changes: 2 additions & 1 deletion js/source/image_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ function ImageSource(id, options, dispatcher) {

this.image = image;
this._loaded = true;
this.fire('data', {dataType: 'source'});
this.fire('source.load');

if (this.map) {
Expand Down Expand Up @@ -106,7 +107,7 @@ ImageSource.prototype = util.inherit(Evented, /** @lends ImageSource.prototype *
Math.round((zoomedCoord.row - centerCoord.row) * EXTENT));
});

this.fire('source.change');
this.fire('data', {dataType: 'source'});
return this;
},

Expand Down
1 change: 1 addition & 0 deletions js/source/raster_tile_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ function RasterTileSource(id, options, dispatcher) {
return this.fire('error', err);
}
util.extend(this, tileJSON);
this.fire('data', {dataType: 'source'});
this.fire('source.load');
}.bind(this));
}
Expand Down
16 changes: 9 additions & 7 deletions js/source/source_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,12 @@ function SourceCache(id, options, dispatcher) {
this._sourceErrored = true;
});

this.on('source.change', function() {
this.reload();
if (this.transform) {
this.update(this.transform, this.map && this.map.style.rasterFadeDuration);
this.on('data', function(event) {
if (this._sourceLoaded && event.dataType === 'source') {
this.reload();
if (this.transform) {
this.update(this.transform, this.map && this.map.style.rasterFadeDuration);
}
}
});

Expand Down Expand Up @@ -166,7 +168,7 @@ SourceCache.prototype = util.inherit(Evented, {

tile.source = this;
tile.timeAdded = new Date().getTime();
this._source.fire('tile.load', {tile: tile});
this._source.fire('data', {tile: tile, dataType: 'tile'});

// HACK this is nescessary to fix https://github.com/mapbox/mapbox-gl-js/issues/2986
if (this.map) this.map.painter.tileExtentVAO.vao = null;
Expand Down Expand Up @@ -407,7 +409,7 @@ SourceCache.prototype = util.inherit(Evented, {

tile.uses++;
this._tiles[coord.id] = tile;
this._source.fire('tile.add', {tile: tile});
this._source.fire('dataloading', {tile: tile, dataType: 'tile'});

return tile;
},
Expand All @@ -425,7 +427,7 @@ SourceCache.prototype = util.inherit(Evented, {

tile.uses--;
delete this._tiles[id];
this._source.fire('tile.remove', {tile: tile});
this._source.fire('data', {dataType: 'tile'});

This comment has been minimized.

Copy link
@ezheidtmann

ezheidtmann Oct 9, 2016

Contributor

With this change, we no longer have access to the tile coordinate in event handlers when a tile is removed. Was this change intentional? Any reason we can't include the tile here and indicate that it's been removed?


if (tile.uses > 0)
return;
Expand Down
2 changes: 1 addition & 1 deletion js/source/tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ Tile.prototype = {

function done(_, data) {
this.reloadSymbolData(data, source.map.painter, source.map.style);
source.fire('tile.load', {tile: this});
source.fire('data', {tile: this, dataType: 'tile'});

// HACK this is nescessary to fix https://github.com/mapbox/mapbox-gl-js/issues/2986
if (source.map) source.map.painter.tileExtentVAO.vao = null;
Expand Down
1 change: 1 addition & 0 deletions js/source/vector_tile_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ function VectorTileSource(id, options, dispatcher) {
return;
}
util.extend(this, tileJSON);
this.fire('data', {dataType: 'source'});
this.fire('source.load');
}.bind(this));
}
Expand Down
3 changes: 2 additions & 1 deletion js/source/video_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ function VideoSource(id, options) {
this.setCoordinates(options.coordinates);
}

this.fire('data', {dataType: 'source'});
this.fire('source.load');
}.bind(this));
}
Expand Down Expand Up @@ -135,7 +136,7 @@ VideoSource.prototype = util.inherit(Evented, /** @lends VideoSource.prototype *
Math.round((zoomedCoord.row - centerCoord.row) * EXTENT));
});

this.fire('source.change');
this.fire('data', {dataType: 'source'});
return this;
},

Expand Down
6 changes: 3 additions & 3 deletions js/style/image_sprite.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function ImageSprite(base) {
}

this.data = data;
if (this.img) this.fire('style.change');
if (this.img) this.fire('data', {dataType: 'style'});
}.bind(this));

ajax.getImage(normalizeURL(base, format, '.png'), function(err, img) {
Expand All @@ -41,7 +41,7 @@ function ImageSprite(base) {
}

this.img = img;
if (this.data) this.fire('style.change');
if (this.data) this.fire('data', {dataType: 'style'});
}.bind(this));
}

Expand All @@ -58,7 +58,7 @@ ImageSprite.prototype.loaded = function() {
ImageSprite.prototype.resize = function(/*gl*/) {
if (browser.devicePixelRatio > 1 !== this.retina) {
var newSprite = new ImageSprite(this.base);
newSprite.on('style.change', function() {
newSprite.on('data', function() {
this.img = newSprite.img;
this.data = newSprite.data;
this.retina = newSprite.retina;
Expand Down
19 changes: 9 additions & 10 deletions js/style/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ var getWorkerPool = require('../global_worker_pool');

module.exports = Style;

function Style(stylesheet, animationLoop, options) {
this.animationLoop = animationLoop || new AnimationLoop();
function Style(stylesheet, map, options) {
this.map = map;
this.animationLoop = (map && map.animationLoop) || new AnimationLoop();
this.dispatcher = new Dispatcher(getWorkerPool(), this);
this.spriteAtlas = new SpriteAtlas(1024, 1024);
this.lineAtlas = new LineAtlas(256, 512);
Expand Down Expand Up @@ -67,6 +68,7 @@ function Style(stylesheet, animationLoop, options) {

this.glyphSource = new GlyphSource(stylesheet.glyphs);
this._resolve();
this.fire('data', {dataType: 'style'});
this.fire('style.load');
}.bind(this);

Expand Down Expand Up @@ -300,7 +302,7 @@ Style.prototype = util.inherit(Evented, {
this._applyClasses(classes, options);

if (this._updates.changed) {
this.fire('style.change');
this.fire('data', {dataType: 'style'});
}

this._resetUpdates();
Expand Down Expand Up @@ -335,9 +337,9 @@ Style.prototype = util.inherit(Evented, {
source = new SourceCache(id, source, this.dispatcher);
this.sources[id] = source;
source.style = this;
source.setEventedParent(this, {source: source});
source.setEventedParent(this, {source: source.getSource()});

this._updates.events.push(['source.add', {source: source}]);
if (source.onAdd) source.onAdd(this.map);
this._updates.changed = true;

return this;
Expand All @@ -361,7 +363,7 @@ Style.prototype = util.inherit(Evented, {
delete this._updates.sources[id];
source.setEventedParent(null);

this._updates.events.push(['source.remove', {source: source}]);
if (source.onRemove) source.onRemove(this.map);
this._updates.changed = true;

return this;
Expand All @@ -382,7 +384,6 @@ Style.prototype = util.inherit(Evented, {
* ID `before`, or appended if `before` is omitted.
* @param {StyleLayer|Object} layer
* @param {string=} before ID of an existing layer to insert before
* @fires layer.add
* @returns {Style} `this`
* @private
*/
Expand All @@ -408,7 +409,6 @@ Style.prototype = util.inherit(Evented, {
if (layer.source) {
this._updates.sources[layer.source] = true;
}
this._updates.events.push(['layer.add', {layer: layer}]);

return this.updateClasses(layer.id);
},
Expand Down Expand Up @@ -441,7 +441,6 @@ Style.prototype = util.inherit(Evented, {
this._order.splice(this._order.indexOf(id), 1);

this._updates.allLayers = true;
this._updates.events.push(['layer.remove', {layer: layer}]);
this._updates.changed = true;

return this;
Expand Down Expand Up @@ -718,7 +717,7 @@ Style.prototype = util.inherit(Evented, {
spriteAtlas.setSprite(sprite);
spriteAtlas.addIcons(params.icons, callback);
} else {
sprite.on('style.change', function() {
sprite.on('data', function() {
spriteAtlas.setSprite(sprite);
spriteAtlas.addIcons(params.icons, callback);
});
Expand Down
8 changes: 5 additions & 3 deletions js/ui/control/attribution.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,11 @@ Attribution.prototype = util.inherit(Control, {
container = this._container = DOM.create('div', className, map.getContainer());

this._update();
map.on('source.load', this._update.bind(this));
map.on('source.change', this._update.bind(this));
map.on('source.remove', this._update.bind(this));
map.on('data', function(event) {
if (event.dataType === 'source') {
this._update();
}
}.bind(this));
map.on('moveend', this._updateEditLink.bind(this));

return container;
Expand Down
Loading

0 comments on commit 1d373b0

Please sign in to comment.