Skip to content

Commit

Permalink
Merge pull request mapbox#2982 from mapbox/fix-add-source-type
Browse files Browse the repository at this point in the history
Revise addSourceType to be independent of Map instance
  • Loading branch information
Anand Thakker committed Sep 23, 2016
2 parents 24f2135 + b22d2ba commit 3933072
Show file tree
Hide file tree
Showing 8 changed files with 219 additions and 90 deletions.
101 changes: 101 additions & 0 deletions debug/custom_source.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
<!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%; }
#checkboxes {
position: absolute;
background: #fff;
top:0;
left:0;
padding:10px;
}
#buffer {
position: absolute;
top:100px;
left:0;
pointer-events: none;
}
#buffer div {
background-color: #fff;
padding: 5px 0;
text-indent: 10px;
white-space: nowrap;
text-shadow:
-1px -1px 0 #fff,
1px -1px 0 #fff,
-1px 1px 0 #fff,
1px 1px 0 #fff;
}
</style>
</head>

<body>
<div id='map'></div>
<div id='checkboxes'>
<input id='show-tile-boundaries-checkbox' name='show-tile-boundaries' type='checkbox'> <label for='show-tile-boundaries'>tile debug</label><br />
<input id='show-symbol-collision-boxes-checkbox' name='show-symbol-collision-boxes' type='checkbox'> <label for='show-symbol-collision-boxes'>collision debug</label><br />
<input id='show-overdraw-checkbox' name='show-overdraw' type='checkbox'> <label for='show-overdraw'>overdraw debug</label><br />
<input id='buffer-checkbox' name='buffer' type='checkbox'> <label for='buffer'>buffer stats</label>
</div>

<div id='buffer' style="display:none">
<em>Waiting for data...</em>
</div>

<script src='/dist/mapbox-gl-dev.js'></script>
<script src='http://devseed.com/mapbox-gl-topojson/dist/mapbox-gl-topojson.js'></script>
<script src='/debug/access-token-generated.js'></script>

<script>
mapboxgl.workerCount = 1
mapboxgl.addSourceType('topojson', mapboxgl.TopoJSONSource, function (err) {
var map = window.map = new mapboxgl.Map({
container: 'map',
zoom: 5.2,
center: [-119.393, 36.883],
style: 'mapbox://styles/mapbox/streets-v8'
})

map.on('load', function () {
map.addSource('counties', {
type: 'topojson',
data: 'http://devseed.com/mapbox-gl-topojson/ca.json',
workerOptions: {
layer: 'counties'
}
})

map.addLayer({
'id': 'county-boundaries',
'type': 'line',
'source': 'counties',
'paint': {
'line-color': '#EC8D8D',
'line-width': {
'base': 1.5,
'stops': [
[
5,
0.75
],
[
18,
32
]
]
}
}
}, 'country-label-lg')
})
})
</script>

</body>
</html>
2 changes: 2 additions & 0 deletions js/mapbox-gl.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ mapboxgl.Marker = require('./ui/marker');

mapboxgl.Style = require('./style/style');

mapboxgl.addSourceType = require('./source/source').addType;

mapboxgl.LngLat = require('./geo/lng_lat');
mapboxgl.LngLatBounds = require('./geo/lng_lat_bounds');
mapboxgl.Point = require('point-geometry');
Expand Down
60 changes: 55 additions & 5 deletions js/source/source.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
'use strict';

var util = require('../util/util');
var Dispatcher = require('../util/dispatcher');
var getWorkerPool = require('../global_worker_pool');

var sourceTypes = {
'vector': require('../source/vector_tile_source'),
Expand All @@ -10,6 +12,8 @@ var sourceTypes = {
'image': require('../source/image_source')
};

var Source = module.exports = {};

/*
* Creates a tiled data source instance given an options object.
*
Expand All @@ -19,7 +23,7 @@ var sourceTypes = {
* @param {Dispatcher} dispatcher
* @returns {Source}
*/
exports.create = function(id, source, dispatcher) {
Source.create = function(id, source, dispatcher) {
source = new sourceTypes[source.type](id, source, dispatcher);

if (source.id !== id) {
Expand All @@ -30,14 +34,60 @@ exports.create = function(id, source, dispatcher) {
return source;
};

exports.getType = function (name) {
Source.getType = function (name) {
return sourceTypes[name];
};

exports.setType = function (name, type) {
Source.setType = function (name, type) {
sourceTypes[name] = type;
};

/**
* Adds a [custom source type](#Custom Sources), making it available for use with
* {@link Map#addSource}.
*
* @param {string} name The name of the source type; source definition objects use this name in the `{type: ...}` field.
* @param {Function} SourceType A {@link Source} constructor.
* @param {Function} callback called after SourceType has been added and, if relevant, its worker code has been sent to the workers.
* @private
*/
Source.addType = function (name, SourceType, callback) {
if (Source.getType(name)) {
throw new Error('A source type named ' + name + ' already exists.');
}

Source.setType(name, SourceType);

if (SourceType.workerSourceURL) {
getDispatcher().broadcast('load worker source', {
name: name,
url: SourceType.workerSourceURL
}, function (err) {
callback(err);
});
} else {
callback();
}
};

/*
* A Dispatcher instance for use in registering custom WorkerSources.
*
* Note that it is created on demand, and once created, this dispatcher
* instance prevents the Workers in the global pool from being destroyed even
* when the the last map instance is destroyed. This is intended behavior, as
* it ensures that any custom WorkerSources will be registered on the workers
* used by future map instances.
*/
var dispatcher;
function getDispatcher () {
if (!dispatcher) {
dispatcher = new Dispatcher(getWorkerPool(), {});
}
return dispatcher;
}


/**
* The `Source` interface must be implemented by each source type, including "core" types (`vector`, `raster`, `video`, etc.) and all custom, third-party types.
*
Expand All @@ -46,7 +96,7 @@ exports.setType = function (name, type) {
*
* @param {string} id The id for the source. Must not be used by any existing source.
* @param {Object} options Source options, specific to the source type (except for `options.type`, which is always required).
* @param {string} options.type The source type, matching the value of `name` used in {@link Style#addSourceType}.
* @param {string} options.type The source type, matching the value of `name` used in {@link Source.addType}.
* @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`
Expand Down Expand Up @@ -116,7 +166,7 @@ exports.setType = function (name, type) {
* implementation may also be targeted by the {@link Source} via
* `dispatcher.send('source-type.methodname', params, callback)`.
*
* @see {@link Map#addSourceType}
* @see {@link Source.addType}
* @private
*
* @class WorkerSource
Expand Down
2 changes: 1 addition & 1 deletion js/source/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function Worker(self) {

this.self.registerWorkerSource = function (name, WorkerSource) {
if (this.workerSourceTypes[name]) {
throw new Error('Worker source with name "' + name + '" already registered.');
util.warnOnce('Worker source named "' + name + '" already registered.');
}
this.workerSourceTypes[name] = WorkerSource;
}.bind(this);
Expand Down
18 changes: 0 additions & 18 deletions js/style/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ var browser = require('../util/browser');
var Dispatcher = require('../util/dispatcher');
var AnimationLoop = require('./animation_loop');
var validateStyle = require('./validate_style');
var Source = require('../source/source');
var QueryFeatures = require('../source/query_features');
var SourceCache = require('../source/source_cache');
var styleSpec = require('./style_spec');
Expand Down Expand Up @@ -660,23 +659,6 @@ Style.prototype = util.inherit(Evented, {
return source ? QueryFeatures.source(source, params) : [];
},

addSourceType: function (name, SourceType, callback) {
if (Source.getType(name)) {
return callback(new Error('A source type called "' + name + '" already exists.'));
}

Source.setType(name, SourceType);

if (!SourceType.workerSourceURL) {
return callback(null, null);
}

this.dispatcher.broadcast('load worker source', {
name: name,
url: SourceType.workerSourceURL
}, callback);
},

_validate: function(validate, key, value, props, options) {
if (options && options.validate === false) {
return false;
Expand Down
14 changes: 1 addition & 13 deletions js/ui/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ util.extend(Map.prototype, /** @lends Map.prototype */{
* @param {string} id The ID of the source to add. Must not conflict with existing sources.
* @param {Object} source The source object, conforming to the
* Mapbox Style Specification's [source definition](https://www.mapbox.com/mapbox-gl-style-spec/#sources).
* @param {string} source.type The source type, which must be either one of the core Mapbox GL source types defined in the style specification or a custom type that has been added to the map with {@link Map#addSourceType}.
* @param {string} source.type The source type, which must be one of the core Mapbox GL source types defined in the style specification.
* @fires source.add
* @returns {Map} `this`
*/
Expand All @@ -666,18 +666,6 @@ util.extend(Map.prototype, /** @lends Map.prototype */{
return this;
},

/**
* Adds a [custom source type](#Custom Sources), making it available for use with
* {@link Map#addSource}.
* @private
* @param {string} name The name of the source type; source definition objects use this name in the `{type: ...}` field.
* @param {Function} SourceType A {@link Source} constructor.
* @param {Function} callback Called when the source type is ready or with an error argument if there is an error.
*/
addSourceType: function (name, SourceType, callback) {
return this.style.addSourceType(name, SourceType, callback);
},

/**
* Removes a source from the map's style.
*
Expand Down
59 changes: 59 additions & 0 deletions test/js/source/source.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
'use strict';

var test = require('tap').test;
var Source = require('../../../js/source/source');
var proxyquire = require('proxyquire');

test('Source#addType', function (t) {
t.test('adds source type', function (t) {
// expect no call to load worker source
var Source = proxyquire('../../../js/source/source', {
'../util/dispatcher': function () {
t.fail();
}
});

var SourceType = function () {};

Source.addType('foo', SourceType, function (err) {
t.error(err);
t.equal(Source.getType('foo'), SourceType);
t.end();
});
});

t.test('triggers workers to load worker source code', function (t) {
var SourceType = function () {};
SourceType.workerSourceURL = 'worker-source.js';

var Source = proxyquire('../../../js/source/source', {
'../util/dispatcher': function () {
this.broadcast = function (type, params) {
if (type === 'load worker source') {
t.equal(Source.getType('bar'), SourceType);
t.equal(params.name, 'bar');
t.equal(params.url, 'worker-source.js');
t.end();
}
};
}
});

Source.addType('bar', SourceType, function (err) { t.error(err); });
});

t.test('throws for duplicate source type', function (t) {
Source.addType('source.test.type-3', function () {}, function (err) {
t.error(err);
t.throws(function () {
Source.addType('source.test.type-3', function () {}, function (err) {
t.error(err);
t.fail();
});
});
});
t.end();
});

t.end();
});
Loading

0 comments on commit 3933072

Please sign in to comment.