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

fire sourcedata when source metadata and tiles are loaded #4347

Merged
merged 8 commits into from
Mar 8, 2017
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
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
Copy link
Contributor

Choose a reason for hiding this comment

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

Small addition: "@fires data with sourceDataType: 'metadata' to indicate..." -- since that's what source cache now requires of the event fired by Source.

* 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 @@ -1676,26 +1676,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 @@ -1720,43 +1709,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