Skip to content

Commit

Permalink
replace tiledata with sourcedata, fire on every tile load (#4347)
Browse files Browse the repository at this point in the history
* source sorcery ✨

* fix render tests 🎨

* fix unit tests 🔧

* refactor to make everything dependent on 'data' event with new properties

* change language, updates tests

* use sourceDataType event property 📆

* fixy tests ✅

* 'update' 👉 'content', update tests, docs 📝
  • Loading branch information
mollymerp authored and Anand Thakker committed Mar 8, 2017
1 parent b9f769c commit e2bec14
Show file tree
Hide file tree
Showing 19 changed files with 405 additions and 355 deletions.
1 change: 0 additions & 1 deletion src/source/canvas_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ class CanvasSource extends ImageSource {
this.load();
if (this.canvas) {
if (this.animate) this.play();
this.setCoordinates(this.coordinates);
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/source/geojson_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,9 @@ class GeoJSONSource extends Evented {
this.fire('error', {error: err});
return;
}
this.fire('data', {dataType: 'source'});
this.fire('source.load');
// although GeoJSON sources contain no metadata, we fire this event to let the SourceCache
// know its ok to start requesting tiles.
this.fire('data', {dataType: 'source', sourceDataType: 'metadata'});
});
}

Expand All @@ -126,13 +127,12 @@ class GeoJSONSource extends Evented {
*/
setData(data) {
this._data = data;

this.fire('dataloading', {dataType: 'source'});
this._updateWorkerData((err) => {
if (err) {
return this.fire('error', { error: err });
}
this.fire('data', {dataType: 'source'});
this.fire('data', {dataType: 'source', sourceDataType: 'content'});
});

return this;
Expand Down
5 changes: 2 additions & 3 deletions src/source/image_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,9 @@ class ImageSource extends Evented {
}

_finishLoading() {
this.fire('source.load');

if (this.map) {
this.setCoordinates(this.coordinates);
this.fire('data', {dataType: 'source', sourceDataType: 'metadata'});
}
}

Expand Down Expand Up @@ -124,7 +123,7 @@ class ImageSource extends Evented {
Math.round((zoomedCoord.row - centerCoord.row) * EXTENT));
});

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

Expand Down
9 changes: 7 additions & 2 deletions src/source/raster_tile_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,13 @@ class RasterTileSource extends Evented {
return this.fire('error', err);
}
util.extend(this, tileJSON);
this.fire('data', {dataType: 'source'});
this.fire('source.load');

// `content` is included here to prevent a race condition where `Style#_updateSources` is called
// before the TileJSON arrives. this makes sure the tiles needed are loaded once TileJSON arrives
// ref: https://github.com/mapbox/mapbox-gl-js/pull/4347#discussion_r104418088
this.fire('data', {dataType: 'source', sourceDataType: 'metadata'});
this.fire('data', {dataType: 'source', sourceDataType: 'content'});

});
}

Expand Down
5 changes: 3 additions & 2 deletions src/source/source.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ exports.setType = function (name, type) {
* @param {string} options.type The source type, matching the value of `name` used in {@link Style#addSourceType}.
* @param {Dispatcher} dispatcher A {@link Dispatcher} instance, which can be used to send messages to the workers.
*
* @fires load to indicate source data has been loaded, so that it's okay to call `loadTile`
* @fires change to indicate source data has changed, so that any current caches should be flushed
* @fires data with `{dataType: 'source', sourceDataType: 'metadata'}` to indicate that any necessary metadata
* has been loaded so that it's okay to call `loadTile`; and with `{dataType: 'source', sourceDataType: 'content'}`
* to indicate that the source data has changed, so that any current caches should be flushed.
* @property {string} id The id for the source. Must match the id passed to the constructor.
* @property {number} minzoom
* @property {number} maxzoom
Expand Down
36 changes: 19 additions & 17 deletions src/source/source_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,26 @@ class SourceCache extends Evented {
this.id = id;
this.dispatcher = dispatcher;

this.on('source.load', function() {
this._sourceLoaded = true;
});

this.on('error', function() {
this._sourceErrored = true;
});

this.on('data', function(event) {
if (this._sourceLoaded && event.dataType === 'source') {
this.on('data', function(e) {
// this._sourceLoaded signifies that the TileJSON is loaded if applicable.
// if the source type does not come with a TileJSON, the flag signifies the
// source data has loaded (i.e geojson has been tiled on the worker and is ready)
if (e.dataType === 'source' && e.sourceDataType==='metadata') this._sourceLoaded = true;

// for sources with mutable data, this event fires when the underlying data
// to a source is changed. (i.e. GeoJSONSource#setData and ImageSource#serCoordinates)
if (this._sourceLoaded && e.dataType==="source" && e.sourceDataType==='content') {
this.reload();
if (this.transform) {
this.update(this.transform);
}
}
});

this.on('error', function() {
this._sourceErrored = true;
});

this._source = Source.create(id, options, dispatcher, this);

this._tiles = {};
Expand Down Expand Up @@ -110,6 +113,7 @@ class SourceCache extends Evented {
return this._source.serialize();
}


prepare() {
if (this._sourceLoaded && this._source.prepare)
return this._source.prepare();
Expand Down Expand Up @@ -169,7 +173,7 @@ class SourceCache extends Evented {
tile.timeAdded = new Date().getTime();
if (previousState === 'expired') tile.refreshedUponExpiration = true;
this._setTileReloadTimer(id, tile);
this._source.fire('data', {tile: tile, coord: tile.coord, dataType: 'tile'});
this._source.fire('data', {dataType: 'source', tile: tile, coord: tile.coord});

// HACK this is necessary to fix https://github.com/mapbox/mapbox-gl-js/issues/2986
if (this.map) this.map.painter.tileExtentVAO.vao = null;
Expand Down Expand Up @@ -296,6 +300,7 @@ class SourceCache extends Evented {
* @private
*/
update(transform) {
this.transform = transform;
if (!this._sourceLoaded) { return; }
let i;
let coord;
Expand Down Expand Up @@ -394,8 +399,6 @@ class SourceCache extends Evented {
for (i = 0; i < remove.length; i++) {
this.removeTile(+remove[i]);
}

this.transform = transform;
}

/**
Expand Down Expand Up @@ -423,8 +426,8 @@ class SourceCache extends Evented {
}
}
}

if (!tile) {
const cached = Boolean(tile);
if (!cached) {
const zoom = coord.z;
const overscaling = zoom > this._source.maxzoom ? Math.pow(2, zoom - this._source.maxzoom) : 1;
tile = new Tile(wrapped, this._source.tileSize * overscaling, this._source.maxzoom);
Expand All @@ -433,7 +436,7 @@ class SourceCache extends Evented {

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

return tile;
}
Expand Down Expand Up @@ -475,7 +478,6 @@ class SourceCache extends Evented {
clearTimeout(this._timers[id]);
this._timers[id] = undefined;
}
this._source.fire('data', { tile: tile, coord: tile.coord, dataType: 'tile' });

if (tile.uses > 0)
return;
Expand Down
1 change: 0 additions & 1 deletion src/source/tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ class Tile {
showCollisionBoxes: source.map.showCollisionBoxes
}, (_, data) => {
this.reloadSymbolData(data, source.map.style);
source.fire('data', {tile: this, coord: this.coord, 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
8 changes: 6 additions & 2 deletions src/source/vector_tile_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,12 @@ class VectorTileSource extends Evented {
return;
}
util.extend(this, tileJSON);
this.fire('data', {dataType: 'source'});
this.fire('source.load');
// `content` is included here to prevent a race condition where `Style#_updateSources` is called
// before the TileJSON arrives. this makes sure the tiles needed are loaded once TileJSON arrives
// ref: https://github.com/mapbox/mapbox-gl-js/pull/4347#discussion_r104418088
this.fire('data', {dataType: 'source', sourceDataType: 'metadata'});
this.fire('data', {dataType: 'source', sourceDataType: 'content'});

});
}

Expand Down
17 changes: 9 additions & 8 deletions src/style/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,15 @@ class Style extends Evented {
browser.frame(stylesheetLoaded.bind(this, null, stylesheet));
}

this.on('source.load', (event) => {
const source = this.sourceCaches[event.sourceId].getSource();
if (source && source.vectorLayerIds) {
for (const layerId in this._layers) {
const layer = this._layers[layerId];
if (layer.source === source.id) {
this._validateLayer(layer);
this.on('data', (event) => {
if (event.dataType === 'source' && event.sourceDataType === 'metadata') {
const source = this.sourceCaches[event.sourceId].getSource();
if (source && source.vectorLayerIds) {
for (const layerId in this._layers) {
const layer = this._layers[layerId];
if (layer.source === source.id) {
this._validateLayer(layer);
}
}
}
}
Expand All @@ -137,7 +139,6 @@ class Style extends Evented {
if (!layer.sourceLayer) return;
if (!sourceCache) return;
const source = sourceCache.getSource();

if (source.type === 'geojson' || (source.vectorLayerIds &&
source.vectorLayerIds.indexOf(layer.sourceLayer) === -1)) {
this.fire('error', {
Expand Down
16 changes: 7 additions & 9 deletions src/ui/control/attribution_control.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class AttributionControl {
this._updateAttributions();
this._updateEditLink();

this._map.on('data', this._updateData);
this._map.on('sourcedata', this._updateData);
this._map.on('moveend', this._updateEditLink);

if (compact === undefined) {
Expand All @@ -58,7 +58,7 @@ class AttributionControl {
onRemove() {
this._container.parentNode.removeChild(this._container);

this._map.off('data', this._updateData);
this._map.off('sourcedata', this._updateData);
this._map.off('moveend', this._updateEditLink);
this._map.off('resize', this._updateCompact);

Expand All @@ -74,16 +74,14 @@ class AttributionControl {
}
}

_updateData(event) {
if (event.dataType === 'source') {
this._updateAttributions();
this._updateEditLink();
}
_updateData(e) {
this._updateAttributions(e);
this._updateEditLink();
}

_updateAttributions() {
_updateAttributions(e) {
if (!this._map.style) return;

if (e && e.sourceDataType!=='metadata') return;
let attributions = [];

const sourceCaches = this._map.style.sourceCaches;
Expand Down
8 changes: 4 additions & 4 deletions src/ui/control/logo_control.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,22 @@ class LogoControl {
this._map = map;
this._container = DOM.create('div', 'mapboxgl-ctrl');

this._map.on('data', this._updateLogo);
this._map.on('sourcedata', this._updateLogo);
this._updateLogo();
return this._container;
}

onRemove() {
this._container.parentNode.removeChild(this._container);
this._map.off('data', this._updateLogo);
this._map.off('sourcedata', this._updateLogo);
}

getDefaultPosition() {
return 'bottom-left';
}

_updateLogo() {
if (this._logoRequired()) {
_updateLogo(e) {
if (e && e.sourceDataType==='metadata' && this._logoRequired()) {
const anchor = DOM.create('a', 'mapboxgl-ctrl-logo');
anchor.target = "_blank";
anchor.href = "https://www.mapbox.com/";
Expand Down
40 changes: 10 additions & 30 deletions src/ui/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -1672,26 +1672,15 @@ function removeNode(node) {
*/

/**
* Fired when one of the map's sources loads or changes. This event is not fired
* if a tile belonging to a source loads or changes (that is handled by
* `tiledata`). See [`MapDataEvent`](#MapDataEvent) for more information.
* Fired when one of the map's sources loads or changes, including if a tile belonging
* to a source loads or changes. See [`MapDataEvent`](#MapDataEvent) for more information.
*
* @event sourcedata
* @memberof Map
* @instance
* @property {MapDataEvent} data
*/

/**
* Fired when one of the map's sources' tiles loads or changes. See
* [`MapDataEvent`](#MapDataEvent) for more information.
*
* @event tiledata
* @memberof Map
* @instance
* @property {MapDataEvent} data
*/

/**
* Fired when any map data (style, source, tile, etc) begins loading or
* changing asyncronously. All `dataloading` events are followed by a `data`
Expand All @@ -1716,43 +1705,34 @@ function removeNode(node) {

/**
* Fired when one of the map's sources begins loading or changing asyncronously.
* This event is not fired if a tile belonging to a source begins loading or
* changing (that is handled by `tiledataloading`). All `sourcedataloading`
* events are followed by a `sourcedata` or `error` event. See
* [`MapDataEvent`](#MapDataEvent) for more information.
* All `sourcedataloading` events are followed by a `sourcedata` or `error` event.
* See [`MapDataEvent`](#MapDataEvent) for more information.
*
* @event sourcedataloading
* @memberof Map
* @instance
* @property {MapDataEvent} data
*/

/**
* Fired when one of the map's sources' tiles begins loading or changing
* asyncronously. All `tiledataloading` events are followed by a `tiledata`
* or `error` event. See [`MapDataEvent`](#MapDataEvent) for more information.
*
* @event tiledataloading
* @memberof Map
* @instance
* @property {MapDataEvent} data
*/

/**
* A `MapDataEvent` object is emitted with the [`Map#data`](#Map.event:data)
* and [`Map#dataloading`](#Map.event:dataloading) events. Possible values for
* `dataType`s are:
*
* - `'source'`: The non-tile data associated with any source
* - `'style'`: The [style](https://www.mapbox.com/mapbox-gl-style-spec/) used by the map
* - `'tile'`: A vector or raster tile
*
* @typedef {Object} MapDataEvent
* @property {string} type The event type.
* @property {string} dataType The type of data that has changed. One of `'source'`, `'style'`.
* @property {boolean} [isSourceLoaded] True if the event has a `dataType` of `source` and the source has no outstanding network requests.
* @property {Object} [source] The [style spec representation of the source](https://www.mapbox.com/mapbox-gl-style-spec/#sources) if the event has a `dataType` of `source`.
* @property {Coordinate} [coord] The coordinate of the tile if the event has a `dataType` of `tile`.
* @property {string} [sourceDataType] Included if the event has a `dataType` of `source` and the event signals
* that internal data has been received or changed. Possible values are `metadata` and `content`.
* @property {Object} [tile] The tile being loaded or changed, if the event has a `dataType` of `source` and
* the event is related to loading of a tile.
* @property {Coordinate} [coord] The coordinate of the tile if the event has a `dataType` of `source` and
* the event is related to loading of a tile.
*/

/**
Expand Down
Loading

0 comments on commit e2bec14

Please sign in to comment.