Skip to content

Commit

Permalink
Add event propagation system to Evented (#3244)
Browse files Browse the repository at this point in the history
* Minor refactoring of Evented

* Refactor Evented tests

* Add Evented#pipe

* Fix flaky map test

* Use Evented#pipe in SourceCache

* Rename "Evented#pipe" to "Evented#forwardEvents"

* Add "passes original 'target' to forwarded listeners" test

* Use event forwarding for source -> style events

* Use event forwarding for layer -> style events

* Use event forwarding for sprite -> style events

* Use event forwarding for all style -> map events

* Get rid of typed error events

* Replace "forwardEvents" with "setEventedParent"

* Minor refactoring

* Make Evented#setEventedParent private

* Fix benchmarks

* Eliminate remaining ImageSprite#load events

* Fix removed bound arguments per PR comment

#3244 (comment)
  • Loading branch information
lucaswoj authored Sep 23, 2016
1 parent f3ad12b commit 24f2135
Show file tree
Hide file tree
Showing 18 changed files with 409 additions and 439 deletions.
2 changes: 1 addition & 1 deletion bench/benchmarks/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ function preloadAssets(stylesheet, callback) {

var style = new Style(stylesheet);

style.on('load', function() {
style.on('style.load', function() {
function getGlyphs(params, callback) {
style['get glyphs'](0, params, function(err, glyphs) {
assets.glyphs[JSON.stringify(params)] = glyphs;
Expand Down
2 changes: 1 addition & 1 deletion bench/benchmarks/style_load.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ module.exports = function() {
.on('error', function(err) {
evented.fire('error', { error: err });
})
.on('load', function() {
.on('style.load', function() {
var time = performance.now() - timeStart;
timeSum += time;
timeCount++;
Expand Down
4 changes: 2 additions & 2 deletions js/source/geojson_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ function GeoJSONSource(id, options, dispatcher) {
this.fire('error', {error: err});
return;
}
this.fire('load');
this.fire('source.load');
}.bind(this));
}

Expand Down Expand Up @@ -119,7 +119,7 @@ GeoJSONSource.prototype = util.inherit(Evented, /** @lends GeoJSONSource.prototy
if (err) {
return this.fire('error', { error: err });
}
this.fire('change');
this.fire('source.change');
}.bind(this));

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

this.image = image;
this._loaded = true;
this.fire('load');
this.fire('source.load');

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

this.fire('change');
this.fire('source.change');
return this;
},

Expand Down
2 changes: 1 addition & 1 deletion js/source/raster_tile_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ function RasterTileSource(id, options, dispatcher) {
return this.fire('error', err);
}
util.extend(this, tileJSON);
this.fire('load');
this.fire('source.load');
}.bind(this));
}

Expand Down
26 changes: 11 additions & 15 deletions js/source/source_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ function SourceCache(id, options, dispatcher) {
this.id = id;
this.dispatcher = dispatcher;

var source = this._source = Source.create(id, options, dispatcher)
.on('load', function () {
var source = this._source = Source.create(id, options, dispatcher);
source.setEventedParent(this);

this.on('source.load', function() {
if (this.map && this._source.onAdd) { this._source.onAdd(this.map); }

this._sourceLoaded = true;
Expand All @@ -42,20 +44,18 @@ function SourceCache(id, options, dispatcher) {
this.attribution = source.attribution;

this.vectorLayerIds = source.vectorLayerIds;
});

this.fire('load');
}.bind(this))
.on('error', function (e) {
this.on('error', function() {
this._sourceErrored = true;
this.fire('error', e);
}.bind(this))
.on('change', function () {
});

this.on('source.change', function() {
this.reload();
if (this.transform) {
this.update(this.transform, this.map && this.map.style.rasterFadeDuration);
}
this.fire('change');
}.bind(this));
});

this._tiles = {};
this._cache = new Cache(0, this.unloadTile.bind(this));
Expand Down Expand Up @@ -160,14 +160,12 @@ SourceCache.prototype = util.inherit(Evented, {
_tileLoaded: function (tile, err) {
if (err) {
tile.state = 'errored';
this.fire('tile.error', {tile: tile, error: err});
this._source.fire('tile.error', {tile: tile, error: err});
this._source.fire('error', {tile: tile, error: err});
return;
}

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

// HACK this is nescessary to fix https://github.com/mapbox/mapbox-gl-js/issues/2986
Expand Down Expand Up @@ -409,7 +407,6 @@ SourceCache.prototype = util.inherit(Evented, {

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

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

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

if (tile.uses > 0)
Expand Down
2 changes: 1 addition & 1 deletion js/source/vector_tile_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function VectorTileSource(id, options, dispatcher) {
return;
}
util.extend(this, tileJSON);
this.fire('load');
this.fire('source.load');
}.bind(this));
}

Expand Down
4 changes: 2 additions & 2 deletions js/source/video_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ function VideoSource(id, options) {
this.setCoordinates(options.coordinates);
}

this.fire('load');
this.fire('source.load');
}.bind(this));
}

Expand Down Expand Up @@ -135,7 +135,7 @@ VideoSource.prototype = util.inherit(Evented, /** @lends VideoSource.prototype *
Math.round((zoomedCoord.row - centerCoord.row) * EXTENT));
});

this.fire('change');
this.fire('source.change');
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('load');
if (this.img) this.fire('style.change');
}.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('load');
if (this.data) this.fire('style.change');
}.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('load', function() {
newSprite.on('style.change', function() {
this.img = newSprite.img;
this.data = newSprite.data;
this.retina = newSprite.retina;
Expand Down
55 changes: 11 additions & 44 deletions js/style/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,7 @@ function Style(stylesheet, animationLoop, options) {
this.sources = {};
this.zoomHistory = {};

util.bindAll([
'_forwardSourceEvent',
'_forwardTileEvent',
'_forwardLayerEvent',
'_redoPlacement'
], this);
util.bindAll(['_redoPlacement'], this);

this._resetUpdates();

Expand Down Expand Up @@ -67,12 +62,12 @@ function Style(stylesheet, animationLoop, options) {

if (stylesheet.sprite) {
this.sprite = new ImageSprite(stylesheet.sprite);
this.sprite.on('load', this.fire.bind(this, 'change'));
this.sprite.setEventedParent(this);
}

this.glyphSource = new GlyphSource(stylesheet.glyphs);
this._resolve();
this.fire('load');
this.fire('style.load');
}.bind(this);

if (typeof stylesheet === 'string') {
Expand Down Expand Up @@ -146,7 +141,7 @@ Style.prototype = util.inherit(Evented, {
if (layerJSON.ref) continue;
layer = StyleLayer.create(layerJSON);
this._layers[layer.id] = layer;
layer.on('error', this._forwardLayerEvent);
layer.setEventedParent(this, {layer: {id: layer.id}});
}

// resolve all layers WITH a ref
Expand All @@ -156,7 +151,7 @@ Style.prototype = util.inherit(Evented, {
var refLayer = this.getLayer(layerJSON.ref);
layer = StyleLayer.create(layerJSON, refLayer);
this._layers[layer.id] = layer;
layer.on('error', this._forwardLayerEvent);
layer.setEventedParent(this, {layer: {id: layer.id}});
}

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

if (this._updates.changed) {
this.fire('change');
this.fire('style.change');
}

this._resetUpdates();
Expand Down Expand Up @@ -340,15 +335,7 @@ Style.prototype = util.inherit(Evented, {
source = new SourceCache(id, source, this.dispatcher);
this.sources[id] = source;
source.style = this;
source
.on('load', this._forwardSourceEvent)
.on('error', this._forwardSourceEvent)
.on('change', this._forwardSourceEvent)
.on('tile.add', this._forwardTileEvent)
.on('tile.load', this._forwardTileEvent)
.on('tile.error', this._forwardTileEvent)
.on('tile.remove', this._forwardTileEvent)
.on('tile.stats', this._forwardTileEvent);
source.setEventedParent(this, {source: source});

this._updates.events.push(['source.add', {source: source}]);
this._updates.changed = true;
Expand All @@ -372,15 +359,7 @@ Style.prototype = util.inherit(Evented, {
var source = this.sources[id];
delete this.sources[id];
delete this._updates.sources[id];
source
.off('load', this._forwardSourceEvent)
.off('error', this._forwardSourceEvent)
.off('change', this._forwardSourceEvent)
.off('tile.add', this._forwardTileEvent)
.off('tile.load', this._forwardTileEvent)
.off('tile.error', this._forwardTileEvent)
.off('tile.remove', this._forwardTileEvent)
.off('tile.stats', this._forwardTileEvent);
source.setEventedParent(null);

this._updates.events.push(['source.remove', {source: source}]);
this._updates.changed = true;
Expand Down Expand Up @@ -420,7 +399,7 @@ Style.prototype = util.inherit(Evented, {
}
this._validateLayer(layer);

layer.on('error', this._forwardLayerEvent);
layer.setEventedParent(this, {layer: {id: layer.id}});

this._layers[layer.id] = layer;
this._order.splice(before ? this._order.indexOf(before) : Infinity, 0, layer.id);
Expand Down Expand Up @@ -454,7 +433,7 @@ Style.prototype = util.inherit(Evented, {
}
}

layer.off('error', this._forwardLayerEvent);
layer.setEventedParent(null);

delete this._layers[id];
delete this._updates.layers[id];
Expand Down Expand Up @@ -730,18 +709,6 @@ Style.prototype = util.inherit(Evented, {
}
},

_forwardSourceEvent: function(e) {
this.fire('source.' + e.type, util.extend({source: e.target.getSource()}, e));
},

_forwardTileEvent: function(e) {
this.fire(e.type, util.extend({source: e.target}, e));
},

_forwardLayerEvent: function(e) {
this.fire('layer.' + e.type, util.extend({layer: {id: e.target.id}}, e));
},

// Callbacks from web workers

'get icons': function(mapId, params, callback) {
Expand All @@ -751,7 +718,7 @@ Style.prototype = util.inherit(Evented, {
spriteAtlas.setSprite(sprite);
spriteAtlas.addIcons(params.icons, callback);
} else {
sprite.on('load', function() {
sprite.on('style.change', function() {
spriteAtlas.setSprite(sprite);
spriteAtlas.addIcons(params.icons, callback);
});
Expand Down
Loading

0 comments on commit 24f2135

Please sign in to comment.