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

feat: Allow sharing queues between tile layers. #1100

Merged
merged 3 commits into from
Jul 16, 2021
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
15 changes: 8 additions & 7 deletions src/tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ var $ = require('jquery');

/**
* This class defines the raw interface for a "tile" on a map. A tile is
* defined as a rectangular section of a map. The base implementation
* is independent of the actual content of the tile, but assumes that
* the content is loaded asynchronously via a url. The tile object
* has a promise-like interface.
* defined as a quadrilateral section of a map. The base implementation is
* independent of the actual content of the tile, but assumes that the content
* is loaded asynchronously via a url. The tile object has a promise-like
* interface.
* @example
* tile.then(function (data) {...}).catch(function (data) {...});
*
Expand Down Expand Up @@ -48,6 +48,8 @@ var tile = function (spec) {
* @property {object} index The tile index.
* @property {number} index.x The tile x index.
* @property {number} index.y The tile y index.
* @property {number} [index.level] The tile level index.
* @property {number} [index.reference] The tile reference index.
* @name geo.tile#index
*/
Object.defineProperty(this, 'index', {
Expand Down Expand Up @@ -143,13 +145,12 @@ var tile = function (spec) {

/**
* Return a unique string representation of the given tile useble as a hash
* key. Possibly extend later to include url information to make caches
* aware of the tile source.
* key.
*
* @returns {string}
*/
this.toString = function () {
return [this._index.level || 0, this._index.y, this._index.x].join('_');
return [this._index.level || 0, this._index.y, this._index.x, this._index.reference || 0].join('_');
};

/**
Expand Down
62 changes: 56 additions & 6 deletions src/tileLayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ var featureLayer = require('./featureLayer');
* zoom level.
* @property {number} [cacheSize=400] The maximum number of tiles to cache.
* The default is 200 if keepLower is false.
* @property {geo.fetchQueue} [queue] A fetch queue to use. If unspecified, a
* new queue is created.
* @property {number} [queueSize=6] The queue size. Most browsers make at most
* 6 requests to any domain, so this should be no more than 6 times the
* number of subdomains used.
* @property {number} [initialQueueSize=0] The initial queue size. `0` to use
* the queue size. When querying a tile server that needs to load
* information before serving the first time, having an initial queue size of
* information before serving the first tile, having an initial queue size of
* 1 can reduce the load on the tile server. After the initial queue of
* tiles are loaded, the `queueSize` is used for all additional queries
* unless the `initialQueueSize` is set again or the tile cache is reset.
Expand Down Expand Up @@ -209,6 +211,7 @@ var tileLayer = function (arg) {
m_initialQueueSize = arg.initialQueueSize || 0,
m_lastTileSet = [],
m_maxBounds = [],
m_reference,
m_exited,
m_this = this;

Expand All @@ -230,7 +233,7 @@ var tileLayer = function (arg) {
this._cache = tileCache({size: arg.cacheSize});

// initialize the tile fetch queue
this._queue = fetchQueue({
this._queue = arg.queue || fetchQueue({
// this should probably be 6 * subdomains.length if subdomains are used
size: m_queueSize,
initialSize: m_initialQueueSize,
Expand All @@ -239,9 +242,14 @@ var tileLayer = function (arg) {
// smaller values will do needless computations.
track: arg.cacheSize,
needed: function (tile) {
if (this._tileLayers && this._tileLayers.length) {
return this._tileLayers.some((tl) => tile === tl.cache.get(tile.toString(), true));
}
return tile === m_this.cache.get(tile.toString(), true);
}
});
this._queue._tileLayers = this._queue._tileLayers || [];
this._queue._tileLayers.push(m_this);

var m_tileOffsetValues = {};

Expand Down Expand Up @@ -275,6 +283,30 @@ var tileLayer = function (arg) {
return $.extend({}, m_this._activeTiles); // copy on output
}});

/**
* Get/set the queue object.
* @property {geo.fetchQueue} queue The current queue.
* @name geo.tileLayer#queue
*/
Object.defineProperty(this, 'queue', {
get: function () { return m_this._queue; },
set: function (queue) {
/* The queue's needed function determines if a tile is still needed. A
* tile in the queue is needed if it is needed by at least one layer that
* is using it. _tileLayers tracks the layers that share the queue to
* allow walking through the layers and check if any layer needs a tile.
* When the queue is set, maintain the list of joined tile layers. */
if (m_this._queue !== queue) {
if (this._queue && this._queue._tileLayers && this._queue._tileLayers.indexOf(m_this) >= 0) {
Copy link
Member

Choose a reason for hiding this comment

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

I have some idea why this is necessary but wondering if you could provide some explanations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a needed function that determines if a tile is still needed. A tile is needed if it is needed by at least one layer that is using it. The _tileLayers array is just a way to make sure we can enumerate the layers that are sharing the queue, and walk through the layers and ask if any layer needs a tile. Setting the queue (or exiting the layer) needs to maintain the book keeping for this list of layers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can add that as a comment.

Copy link
Member

Choose a reason for hiding this comment

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

yeah that makes sense. So in the next line why are we setting this index to 1? The tileLayer is already has this queue before but the index may have been zero or above zero. Just wondering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're not. We are removing the layer from the old queue's _tileLayers list it was part of. The splice(index, 1) says remove 1 value at its current location. We then push in on the new queue's _tileLayers list at the end of the function.

Copy link
Member

Choose a reason for hiding this comment

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

Ah thanks for reminding the function execution, okay that makes sense.

this._queue._tileLayers.splice(this._queue._tileLayers.indexOf(m_this), 1);
}
m_this._queue = queue;
m_this._queue._tileLayers = m_this._queue._tileLayers || [];
m_this._queue._tileLayers.push(m_this);
}
}
});

/**
* Get/set the queue size.
* @property {number} size The queue size.
Expand Down Expand Up @@ -302,6 +334,20 @@ var tileLayer = function (arg) {
}
});

/**
* Get/set the tile reference value.
* @property {string} reference A reference value to distinguish tiles on
* this layer.
* @name geo.tileLayer#reference
*/
Object.defineProperty(this, 'reference', {
get: function () { return '' + m_this.id() + '_' + (m_reference || 0); },
set: function (reference) {
m_reference = reference;
},
configurable: true
});

/**
* The number of tiles at the given zoom level. The default implementation
* just returns `Math.pow(2, z)`.
Expand Down Expand Up @@ -566,11 +612,12 @@ var tileLayer = function (arg) {
* @param {object} index The tile index
* @param {number} index.x
* @param {number} index.y
* @param {number} index.level
* @param {number} [index.level]
* @param {number} [index.reference]
* @returns {string}
*/
this._tileHash = function (index) {
return [index.level || 0, index.y, index.x].join('_');
return [index.level || 0, index.y, index.x, index.reference || 0].join('_');
};

/**
Expand Down Expand Up @@ -663,8 +710,8 @@ var tileLayer = function (arg) {
// loop over the tile range
for (i = start.x; i <= end.x; i += 1) {
for (j = start.y; j <= end.y; j += 1) {
index = {level: level, x: i, y: j};
source = {level: level, x: i, y: j};
index = {level: level, x: i, y: j, reference: m_this.reference};
source = {level: level, x: i, y: j, reference: m_this.reference};
if (m_this._options.wrapX) {
source.x = modulo(source.x, nTilesLevel.x);
}
Expand Down Expand Up @@ -1622,6 +1669,9 @@ var tileLayer = function (arg) {
// call super method
s_exit.apply(m_this, arguments);
m_exited = true;
if (this._queue && this._queue._tileLayers && this._queue._tileLayers.indexOf(m_this) >= 0) {
this._queue._tileLayers.splice(this._queue._tileLayers.indexOf(m_this), 1);
}
return m_this;
};

Expand Down
4 changes: 2 additions & 2 deletions tests/cases/osmLayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ describe('geo.core.osmLayer', function () {
it('check null tiles and switch to svg', function () {
positions = {};
$.each($('[tile-reference]'), function () {
var ref = $(this).attr('tile-reference');
var ref = $(this).attr('tile-reference').split('_').slice(0, 3).join('_');
positions[ref] = $(this)[0].getBoundingClientRect();
});
map.deleteLayer(layer);
Expand All @@ -268,7 +268,7 @@ describe('geo.core.osmLayer', function () {
});
it('compare tile offsets at angle ' + angle, function () {
$.each($('image[reference]'), function () {
var ref = $(this).attr('reference');
var ref = $(this).attr('reference').split('_').slice(0, 3).join('_');
/* Only check the top level */
if (ref.indexOf('4_') === 0) {
var offset = $(this)[0].getBoundingClientRect();
Expand Down
20 changes: 20 additions & 0 deletions tests/cases/tileLayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,26 @@ describe('geo.tileLayer', function () {
expect(layer.initialQueueSize).toBe(1);
expect(layer._queue.initialSize).toBe(1);
});
it('queue', function () {
var m = map(), layer;
opts.map = m;
layer = geo.tileLayer(opts);
expect(layer.queue._tileLayers.length).toBe(1);
var layer2 = geo.tileLayer(opts);
var origQueue = layer2.queue;
layer2.queue = layer.queue;
expect(layer.queue._tileLayers.length).toBe(2);
layer2.queue = origQueue;
expect(layer.queue._tileLayers.length).toBe(1);
});
it('reference', function () {
var m = map(), layer;
opts.map = m;
layer = geo.tileLayer(opts);
expect(layer.reference).toBe(layer.id() + '_0');
layer.reference = 'A';
expect(layer.reference).toBe(layer.id() + '_A');
});
});
describe('Public utility methods', function () {
describe('isValid', function () {
Expand Down
13 changes: 9 additions & 4 deletions tutorials/multiframe/index.pug
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,13 @@ block mainTutorial
}
updating = true;
// wait until all current tiles are loaded before loading more
map.onIdle(() => {
layerA.onIdle(() => {
// load the new frame in the background layer
layerB._frame = frame;
layerB.url(`${baseUrl}${frame}`);
map.onIdle(() => {
if (frame !== layerB._frame) {
layerB._frame = frame;
layerB.url(`${baseUrl}${frame}`);
}
layerB.onIdle(() => {
// once everything is loaded, check if we still need to swap layers
updating = false;
// the top layer is what we want so we don't have to do anything.
Expand Down Expand Up @@ -122,6 +124,9 @@ block mainTutorial
layerB = map.createLayer('osm', params.layer);
// create a foreground layer for the current frame
params.layer.url = `${baseUrl}0`;
// have the layers share a fetch queue. Since both tile layers are from
// the same server, this prevents requests from getting backlogged.
params.layer.queue = layerB.queue;
layerA = map.createLayer('osm', params.layer);
layerA._frame = 0;
// adjust the frame slider and listen to changes
Expand Down