Skip to content

Commit

Permalink
Un-ignore test-suite runtime-styling tests (#2847)
Browse files Browse the repository at this point in the history
* Don't throw validation error when removing a filter

* Fix "Invalid GL context" test-suite errors

* Fix exception when passing null to "util.deepEqual"

* Unignore test-suite runtime-styling tests

* Converted all jpeg tiles to png

* Track tile state as enum instead of boolean, add reloading state

* Return false from Map#loaded if there are pending source batch updates

* Set "raster-fade-duration" to 0 in raster runtime styling tests

* Refactor TilePyramid

* Prevent dispatcher from firing callbacks after removed
  • Loading branch information
lucaswoj authored Jul 13, 2016
1 parent f8d7d1a commit de61bbf
Show file tree
Hide file tree
Showing 13 changed files with 109 additions and 75 deletions.
4 changes: 2 additions & 2 deletions js/source/raster_tile_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ RasterTileSource.prototype = util.inherit(Evented, {
return;

if (err) {
tile.errored = true;
tile.state = 'errored';
this.fire('tile.error', {tile: tile, error: err});
return;
}
Expand All @@ -90,7 +90,7 @@ RasterTileSource.prototype = util.inherit(Evented, {
this.map.animationLoop.set(this.style.rasterFadeDuration);

tile.source = this;
tile.loaded = true;
tile.state = 'loaded';

this.fire('tile.load', {tile: tile});
}
Expand Down
6 changes: 3 additions & 3 deletions js/source/source.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ exports.redoPlacement = function() {
return;
}

var ids = this._pyramid.orderedIDs();
var ids = this._pyramid.getIds();
for (var i = 0; i < ids.length; i++) {
var tile = this._pyramid.getTile(ids[i]);
this._redoTilePlacement(tile);
Expand All @@ -64,7 +64,7 @@ exports._getTile = function(coord) {

exports._getVisibleCoordinates = function() {
if (!this._pyramid) return [];
else return this._pyramid.renderedIDs().map(TileCoord.fromID);
else return this._pyramid.getRenderableIds().map(TileCoord.fromID);
};

function sortTilesIn(a, b) {
Expand Down Expand Up @@ -124,7 +124,7 @@ exports._querySourceFeatures = function(params) {
}

var pyramid = this._pyramid;
var tiles = pyramid.renderedIDs().map(function(id) {
var tiles = pyramid.getRenderableIds().map(function(id) {
return pyramid.getTile(id);
});

Expand Down
28 changes: 19 additions & 9 deletions js/source/tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,19 @@ module.exports = Tile;
function Tile(coord, size, sourceMaxZoom) {
this.coord = coord;
this.uid = util.uniqueId();
this.loaded = false;
this.isUnloaded = false;
this.uses = 0;
this.tileSize = size;
this.sourceMaxZoom = sourceMaxZoom;
this.buckets = {};

// `this.state` must be one of
//
// - `loading`: Tile data is in the process of loading.
// - `loaded`: Tile data has been loaded. Tile can be rendered.
// - `reloading`: Tile data has been loaded and is being updated. Tile can be rendered.
// - `unloaded`: Tile data has been deleted.
// - `errored`: Tile data was not loaded because of an error.
this.state = 'loading';
}

Tile.prototype = {
Expand All @@ -45,7 +52,7 @@ Tile.prototype = {
* @private
*/
loadVectorData: function(data, style) {
this.loaded = true;
this.state = 'loaded';

// empty GeoJSON tile
if (!data) return;
Expand All @@ -68,7 +75,7 @@ Tile.prototype = {
* @private
*/
reloadSymbolData: function(data, painter, style) {
if (this.isUnloaded) return;
if (this.state === 'unloaded') return;

this.collisionTile = new CollisionTile(data.collisionTile, this.collisionBoxArray);
this.featureIndex.setCollisionTile(this.collisionTile);
Expand Down Expand Up @@ -107,17 +114,16 @@ Tile.prototype = {
this.featureIndex = null;
this.rawTileData = null;
this.buckets = null;
this.loaded = false;
this.isUnloaded = true;
this.state = 'unloaded';
},

redoPlacement: function(source) {
if (!this.loaded || this.redoingPlacement) {
if (this.state !== 'loaded' || this.state === 'reloading') {
this.redoWhenDone = true;
return;
}

this.redoingPlacement = true;
this.state = 'reloading';

source.dispatcher.send('redo placement', {
uid: this.uid,
Expand All @@ -131,7 +137,7 @@ Tile.prototype = {
this.reloadSymbolData(data, source.map.painter, source.map.style);
source.fire('tile.load', {tile: this});

this.redoingPlacement = false;
this.state = 'loaded';
if (this.redoWhenDone) {
this.redoPlacement(source);
this.redoWhenDone = false;
Expand Down Expand Up @@ -165,6 +171,10 @@ Tile.prototype = {
result.push(geojsonFeature);
}
}
},

isRenderable: function() {
return this.state === 'loaded' || this.state === 'reloading';
}
};

Expand Down
46 changes: 29 additions & 17 deletions js/source/tile_pyramid.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function TilePyramid(options) {
this._tiles = {};
this._cache = new Cache(0, function(tile) { return this._unload(tile); }.bind(this));

this._filterRendered = this._filterRendered.bind(this);
this._isIdRenderable = this._isIdRenderable.bind(this);
}


Expand All @@ -47,13 +47,15 @@ TilePyramid.maxUnderzooming = 3;

TilePyramid.prototype = {
/**
* Confirm that every tracked tile is loaded.
* @returns {boolean} whether all tiles are loaded.
* Return true if no tile data is pending, tiles will not change unless
* an additional API call is received.
* @returns {boolean}
* @private
*/
loaded: function() {
for (var t in this._tiles) {
if (!this._tiles[t].loaded && !this._tiles[t].errored)
var tile = this._tiles[t];
if (tile.state !== 'loaded' && tile.state !== 'errored')
return false;
}
return true;
Expand All @@ -64,22 +66,32 @@ TilePyramid.prototype = {
* @returns {Array<number>} ids
* @private
*/
orderedIDs: function() {
getIds: function() {
return Object.keys(this._tiles).map(Number).sort(compareKeyZoom);
},

renderedIDs: function() {
return this.orderedIDs().filter(this._filterRendered);
getRenderableIds: function() {
return this.getIds().filter(this._isIdRenderable);
},

_filterRendered: function(id) {
return this._tiles[id].loaded && !this._coveredTiles[id];
_isIdRenderable: function(id) {
return this._tiles[id].isRenderable() && !this._coveredTiles[id];
},

reload: function() {
this._cache.reset();
for (var i in this._tiles) {
this._load(this._tiles[i]);
var tile = this._tiles[i];

// The difference between "loading" tiles and "reloading" tiles is
// that "reloading" tiles are "renderable". Therefore, a "loading"
// tile cannot become a "reloading" tile without first becoming
// a "loaded" tile.
if (tile.state !== 'loading') {
tile.state = 'reloading';
}

this._load(tile);
}
},

Expand Down Expand Up @@ -157,8 +169,8 @@ TilePyramid.prototype = {
for (var id in this._tiles) {
var tile = this._tiles[id];

// only consider loaded tiles on higher zoom levels (up to maxCoveringZoom)
if (retain[id] || !tile.loaded || tile.coord.z <= coord.z || tile.coord.z > maxCoveringZoom) continue;
// only consider renderable tiles on higher zoom levels (up to maxCoveringZoom)
if (retain[id] || !tile.isRenderable() || tile.coord.z <= coord.z || tile.coord.z > maxCoveringZoom) continue;

// disregard tiles that are not descendants of the given tile coordinate
var z2 = Math.pow(2, Math.min(tile.coord.z, this.maxzoom) - Math.min(coord.z, this.maxzoom));
Expand All @@ -175,7 +187,7 @@ TilePyramid.prototype = {
var parentId = tile.coord.parent(this.maxzoom).id;
tile = this._tiles[parentId];

if (tile && tile.loaded) {
if (tile && tile.isRenderable()) {
delete retain[id];
retain[parentId] = true;
}
Expand All @@ -198,7 +210,7 @@ TilePyramid.prototype = {
for (var z = coord.z - 1; z >= minCoveringZoom; z--) {
coord = coord.parent(this.maxzoom);
var tile = this._tiles[coord.id];
if (tile && tile.loaded) {
if (tile && tile.isRenderable()) {
retain[coord.id] = true;
return tile;
}
Expand Down Expand Up @@ -261,7 +273,7 @@ TilePyramid.prototype = {

retain[coord.id] = true;

if (tile.loaded)
if (tile.isRenderable())
continue;

// The tile we require is not yet loaded.
Expand Down Expand Up @@ -360,7 +372,7 @@ TilePyramid.prototype = {
if (tile.uses > 0)
return;

if (tile.loaded) {
if (tile.isRenderable()) {
this._cache.add(tile.coord.wrapped().id, tile);
} else {
this._abort(tile);
Expand All @@ -387,7 +399,7 @@ TilePyramid.prototype = {
*/
tilesIn: function(queryGeometry) {
var tileResults = {};
var ids = this.orderedIDs();
var ids = this.getIds();

var minX = Infinity;
var minY = Infinity;
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 @@ -85,7 +85,7 @@ VectorTileSource.prototype = util.inherit(Evented, {
return;

if (err) {
tile.errored = true;
tile.state = 'errored';
this.fire('tile.error', {tile: tile, error: err});
return;
}
Expand Down
5 changes: 4 additions & 1 deletion js/style/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ Style.prototype = util.inherit(Evented, {
if (!this._loaded)
return false;

if (Object.keys(this._updates.sources).length)
return false;

for (var id in this.sources)
if (!this.sources[id].loaded())
return false;
Expand Down Expand Up @@ -500,7 +503,7 @@ Style.prototype = util.inherit(Evented, {

var layer = this.getReferentLayer(layerId);

if (this._handleErrors(validateStyle.filter, 'layers.' + layer.id + '.filter', filter)) return this;
if (filter !== null && this._handleErrors(validateStyle.filter, 'layers.' + layer.id + '.filter', filter)) return this;

if (util.deepEqual(layer.filter, filter)) return this;
layer.filter = util.clone(filter);
Expand Down
9 changes: 5 additions & 4 deletions js/util/dispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ function Dispatcher(length, parent) {

this.worker = new Worker(workerBus);
this.actor = new Actor(parentBus, parent);

this.remove = function() {
parentListeners.splice(0, parentListeners.length);
workerListeners.splice(0, workerListeners.length);
};
}

Dispatcher.prototype = {
Expand All @@ -45,9 +50,5 @@ Dispatcher.prototype = {

send: function(type, data, callback, targetID, buffers) {
this.actor.send(type, data, callback, buffers);
},

remove: function() {
// noop
}
};
2 changes: 1 addition & 1 deletion js/util/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ exports.deepEqual = function deepEqual(a, b) {
}
return true;
}
if (typeof a === 'object') {
if (typeof a === 'object' && a !== null && b !== null) {
if (!(typeof b === 'object')) return false;
var keys = Object.keys(a);
if (keys.length !== Object.keys(b).length) return false;
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"highlight.js": "9.3.0",
"istanbul": "^0.4.2",
"lodash": "^4.13.1",
"mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#4868b34cea6bff36a442124e79d5b20b0c60fc9b",
"mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#dd5b5c93e13f8760bad6c6288d18f286f0a752d4",
"minifyify": "^7.0.1",
"nyc": "6.4.0",
"remark": "4.2.2",
Expand Down
2 changes: 1 addition & 1 deletion test/js/source/geojson_source.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ test('GeoJSONSource#update', function(t) {
source.update(transform);

source.once('change', function() {
t.deepEqual(source._pyramid.renderedIDs(), []);
t.deepEqual(source._pyramid.getRenderableIds(), []);
t.end();
});
});
Expand Down
Loading

0 comments on commit de61bbf

Please sign in to comment.