From 0b1a27e21de816c10030643c6ecd1b094acf4531 Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Fri, 4 Aug 2017 20:11:49 -0500 Subject: [PATCH 1/9] change tile loading logic in source cache to load not-previously-loaded parent tiles if a tile and its children are not found. --- src/source/source_cache.js | 92 +++++++++++++++++++++------ src/source/tile.js | 4 ++ src/source/tile_coord.js | 8 +++ test/unit/source/source_cache.test.js | 7 +- 4 files changed, 91 insertions(+), 20 deletions(-) diff --git a/src/source/source_cache.js b/src/source/source_cache.js index 9ff5e99ebd0..bb6e05f84a1 100644 --- a/src/source/source_cache.js +++ b/src/source/source_cache.js @@ -337,6 +337,7 @@ class SourceCache extends Evented { let coord; let tile; let parentTile; + let covered; this.updateCacheSize(transform); @@ -354,13 +355,13 @@ class SourceCache extends Evented { // better, retained tiles. They are not drawn separately. this._coveredTiles = {}; - let visibleCoords; + let idealTileCoords; if (!this.used) { - visibleCoords = []; + idealTileCoords = []; } else if (this._source.coord) { - visibleCoords = transform.getVisibleWrappedCoordinates((this._source.coord: any)); + idealTileCoords = transform.getVisibleWrappedCoordinates((this._source.coord: any)); } else { - visibleCoords = transform.coveringTiles({ + idealTileCoords = transform.coveringTiles({ tileSize: this._source.tileSize, minzoom: this._source.minzoom, maxzoom: this._source.maxzoom, @@ -369,25 +370,79 @@ class SourceCache extends Evented { }); if (this._source.hasTile) { - visibleCoords = visibleCoords.filter((coord) => (this._source.hasTile: any)(coord)); + idealTileCoords = idealTileCoords.filter((coord) => (this._source.hasTile: any)(coord)); } } - for (i = 0; i < visibleCoords.length; i++) { - coord = visibleCoords[i]; - tile = this._addTile(coord); - - retain[coord.id] = true; - if (tile.hasData()) - continue; + const checked = {}; + for (i = 0; i < idealTileCoords.length; i++) { + coord = idealTileCoords[i]; + tile = this._addTile(coord); + let parentIsLoaded = false; + if (tile.hasData()) { + retain[coord.id] = true; + } else { + // The tile we require is not yet loaded or does not exist. + // We are now attempting to load child and parent tiles. + parentIsLoaded = tile.isLoaded(); + + // The tile isn't loaded yet, but retain it anyway because it's an ideal tile. + retain[coord.id] = true; + covered = true; + const overscaledZ = zoom + 1; + if (overscaledZ > this._source.maxzoom) { + // We're looking for an overzoomed child tile. + const childCoord = coord.children(this._source.maxzoom)[0]; + const childTile = this.getTile(childCoord); + if (!!childTile && childTile.hasData()) { + retain[childCoord.id] = true; + } else { + covered = false; + } + } else { + // Check all four actual child tiles. + const children = coord.children(this._source.maxzoom); + for (let j = 0; j < children.length; j++) { + const childCoord = children[i]; + const childTile = childCoord ? this.getTile(childCoord) : null; + + if (!!childTile && childTile.hasData()) { + retain[childCoord.id] = true; + } else { + covered = false; + } + } + } - // The tile we require is not yet loaded. - // Retain child or parent tiles that cover the same area. - if (!this._findLoadedChildren(coord, maxCoveringZoom, retain)) { - parentTile = this.findLoadedParent(coord, minCoveringZoom, retain); - if (parentTile) { - this._addTile(parentTile.coord); + if (!covered) { + // We couldn't find child tiles that entirely cover the ideal tile. + for (let overscaledZ = zoom - 1; overscaledZ >= minCoveringZoom; --overscaledZ) { + const parentId = coord.scaledTo(overscaledZ); + if (checked[parentId.id]) { + // Break parent tile ascent, this route has been previously checked by another child. + break; + } else { + checked[parentId.id] = true; + } + + + tile = this.getTile(parentId); + + if (!tile && parentIsLoaded) { + tile = this._addTile(parentId); + } + + if (tile) { + retain[parentId.id] = true; + // Save the current values, since they're the parent of the next iteration + // of the parent tile ascent loop. + parentIsLoaded = tile.isLoaded(); + if (tile.hasData()) { + break; + } + } + } } } } @@ -428,7 +483,6 @@ class SourceCache extends Evented { for (fadedParent in parentsForFading) { retain[fadedParent] = true; } - // Remove the tiles we don't need anymore. const remove = util.keysDifference(this._tiles, retain); for (i = 0; i < remove.length; i++) { diff --git a/src/source/tile.js b/src/source/tile.js index f75c8d47c51..2dde7ebbcca 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -102,6 +102,10 @@ class Tile { animationLoop.set(this.fadeEndTime - Date.now()); } + isLoaded() { + return this.state === 'errored' || this.state === 'loaded' || this.state === 'reloading'; + } + /** * Given a data object with a 'buffers' property, load it into * this tile's elementGroups and buffers properties and set loaded diff --git a/src/source/tile_coord.js b/src/source/tile_coord.js index 61ab88e894b..330609f7ec5 100644 --- a/src/source/tile_coord.js +++ b/src/source/tile_coord.js @@ -95,6 +95,14 @@ class TileCoord { ]; } + scaledTo(targetZ: number) { + if (targetZ <= this.z) { + return new TileCoord(targetZ, this.x >> (this.z - targetZ), this.y >> (this.z - targetZ), this.w); // parent or same + } else { + return new TileCoord(targetZ, this.x << (targetZ - this.z), this.y << (targetZ - this.z), this.w); // child + } + } + static cover(z: number, bounds: [Coordinate, Coordinate, Coordinate, Coordinate], actualZ: number, renderWorldCopies: boolean | void) { if (renderWorldCopies === undefined) { diff --git a/test/unit/source/source_cache.test.js b/test/unit/source/source_cache.test.js index ac2e9e97500..1edf903c25f 100644 --- a/test/unit/source/source_cache.test.js +++ b/test/unit/source/source_cache.test.js @@ -407,7 +407,12 @@ test('SourceCache#update', (t) => { transform.resize(511, 511); transform.zoom = 0; - const sourceCache = createSourceCache({}); + const sourceCache = createSourceCache({ + loadTile: (tile, callback)=>{ + tile.state = 'loaded'; + callback(null); + } + }); sourceCache.on('data', (e) => { if (e.sourceDataType === 'metadata') { From 6515276d32640c1097b16399e83e58c3ef12a98b Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Tue, 8 Aug 2017 17:25:32 -0700 Subject: [PATCH 2/9] un-ignore raster loading test --- test/ignores.json | 1 - 1 file changed, 1 deletion(-) diff --git a/test/ignores.json b/test/ignores.json index 8d287290795..29841a63987 100644 --- a/test/ignores.json +++ b/test/ignores.json @@ -11,7 +11,6 @@ "render-tests/fill-extrusion-pattern/opacity": "https://github.com/mapbox/mapbox-gl-js/issues/3327", "render-tests/geojson/inline-linestring-fill": "current behavior is arbitrary", "render-tests/line-dasharray/zoom-history": "needs investigation", - "render-tests/raster-loading/missing": "https://github.com/mapbox/mapbox-gl-js/issues/4257", "render-tests/raster-masking/overlapping": "https://github.com/mapbox/mapbox-gl-js/issues/5003", "render-tests/regressions/mapbox-gl-js#3682": "skip - true", "render-tests/runtime-styling/image-add-alpha": "failing on Circle CI 2.0, needs investigation", From 49747ce01645dee113c0a0a1423650659bb4e1fb Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Tue, 5 Sep 2017 13:50:40 -0700 Subject: [PATCH 3/9] rename variables and functions for clarity --- src/source/source_cache.js | 12 ++++++++---- src/source/tile.js | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/source/source_cache.js b/src/source/source_cache.js index bb6e05f84a1..4efcbe77905 100644 --- a/src/source/source_cache.js +++ b/src/source/source_cache.js @@ -379,13 +379,17 @@ class SourceCache extends Evented { for (i = 0; i < idealTileCoords.length; i++) { coord = idealTileCoords[i]; tile = this._addTile(coord); - let parentIsLoaded = false; + let parentWasRequested = false; if (tile.hasData()) { retain[coord.id] = true; } else { // The tile we require is not yet loaded or does not exist. // We are now attempting to load child and parent tiles. - parentIsLoaded = tile.isLoaded(); + + // As we descend up and down the tile pyramid of the ideal tile, we check whether the parent + // tile has been previously requested (and errored in this case due to the previous conditional) + // in order to determine if we need to request its parent. + parentWasRequested = tile.wasRequested(); // The tile isn't loaded yet, but retain it anyway because it's an ideal tile. retain[coord.id] = true; @@ -429,7 +433,7 @@ class SourceCache extends Evented { tile = this.getTile(parentId); - if (!tile && parentIsLoaded) { + if (!tile && parentWasRequested) { tile = this._addTile(parentId); } @@ -437,7 +441,7 @@ class SourceCache extends Evented { retain[parentId.id] = true; // Save the current values, since they're the parent of the next iteration // of the parent tile ascent loop. - parentIsLoaded = tile.isLoaded(); + parentWasRequested = tile.wasRequested(); if (tile.hasData()) { break; } diff --git a/src/source/tile.js b/src/source/tile.js index 2dde7ebbcca..03b0cd0dc0d 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -102,7 +102,7 @@ class Tile { animationLoop.set(this.fadeEndTime - Date.now()); } - isLoaded() { + wasRequested() { return this.state === 'errored' || this.state === 'loaded' || this.state === 'reloading'; } From 9b444fbc83dbfe52a01efab3a7ac90846ab2bb96 Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Tue, 5 Sep 2017 14:02:54 -0700 Subject: [PATCH 4/9] update test suite to handle missing tiles --- src/source/source_cache.js | 2 ++ test/suite_implementation.js | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/source/source_cache.js b/src/source/source_cache.js index 4efcbe77905..d164e5b79f5 100644 --- a/src/source/source_cache.js +++ b/src/source/source_cache.js @@ -212,6 +212,8 @@ class SourceCache extends Evented { if (err) { tile.state = 'errored'; if (err.status !== 404) this._source.fire('error', {tile: tile, error: err}); + // continue to try loading parent/children tiles if a tile doesn't exist (404) + else this.update(this.transform); return; } diff --git a/test/suite_implementation.js b/test/suite_implementation.js index ed25ca02da9..7c5c62b419f 100644 --- a/test/suite_implementation.js +++ b/test/suite_implementation.js @@ -166,7 +166,7 @@ ajax.getImage = function({ url }, callback) { callback(null, png); }); } else { - callback(error || new Error(response.statusCode)); + callback(error || {status: response.statusCode}); } }); }; From 3a34ecaf93e93285bfa961406183732374ab0aae Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Tue, 5 Sep 2017 17:38:14 -0700 Subject: [PATCH 5/9] refactor _updateRetainedTiles into separate function, fix indexing bug --- src/source/source_cache.js | 119 +++++++++++++------------- test/unit/source/source_cache.test.js | 102 ++++++++++++++++++++++ 2 files changed, 161 insertions(+), 60 deletions(-) diff --git a/src/source/source_cache.js b/src/source/source_cache.js index d164e5b79f5..45a5bb2f890 100644 --- a/src/source/source_cache.js +++ b/src/source/source_cache.js @@ -335,25 +335,9 @@ class SourceCache extends Evented { this.transform = transform; if (!this._sourceLoaded || this._paused) { return; } - let i; - let coord; - let tile; - let parentTile; - let covered; - this.updateCacheSize(transform); - // Determine the overzooming/underzooming amounts. - const zoom = (this._source.roundZoom ? Math.round : Math.floor)(this.getZoom(transform)); - const minCoveringZoom = Math.max(zoom - SourceCache.maxOverzooming, this._source.minzoom); - const maxCoveringZoom = Math.max(zoom + SourceCache.maxUnderzooming, this._source.minzoom); - - // Retain is a list of tiles that we shouldn't delete, even if they are not - // the most ideal tile for the current viewport. This may include tiles like - // parent or child tiles that are *already* loaded. - const retain = {}; - - // Covered is a list of retained tiles who's areas are full covered by other, + // Covered is a list of retained tiles who's areas are fully covered by other, // better, retained tiles. They are not drawn separately. this._coveredTiles = {}; @@ -376,7 +360,63 @@ class SourceCache extends Evented { } } + // Determine the overzooming/underzooming amounts. + const zoom = (this._source.roundZoom ? Math.round : Math.floor)(this.getZoom(transform)); + const minCoveringZoom = Math.max(zoom - SourceCache.maxOverzooming, this._source.minzoom); + const maxCoveringZoom = Math.max(zoom + SourceCache.maxUnderzooming, this._source.minzoom); + + // Retain is a list of tiles that we shouldn't delete, even if they are not + // the most ideal tile for the current viewport. This may include tiles like + // parent or child tiles that are *already* loaded. + const retain = this._updateRetainedTiles(idealTileCoords, zoom, minCoveringZoom); + + const parentsForFading = {}; + + if (isRasterType(this._source.type)) { + const ids = Object.keys(retain); + for (let k = 0; k < ids.length; k++) { + const id = ids[k]; + const coord = TileCoord.fromID(+id); + const tile = this._tiles[id]; + if (!tile) continue; + + // If the drawRasterTile has never seen this tile, then + // tile.fadeEndTime may be unset. In that case, or if + // fadeEndTime is in the future, then this tile is still + // fading in. Find tiles to cross-fade with it. + if (typeof tile.fadeEndTime === 'undefined' || tile.fadeEndTime >= Date.now()) { + if (this._findLoadedChildren(coord, maxCoveringZoom, retain)) { + retain[id] = true; + } + const parentTile = this.findLoadedParent(coord, minCoveringZoom, parentsForFading); + if (parentTile) { + this._addTile(parentTile.coord); + } + } + } + } + + let fadedParent; + for (fadedParent in parentsForFading) { + if (!retain[fadedParent]) { + // If a tile is only needed for fading, mark it as covered so that it isn't rendered on it's own. + this._coveredTiles[fadedParent] = true; + } + } + for (fadedParent in parentsForFading) { + retain[fadedParent] = true; + } + // Remove the tiles we don't need anymore. + const remove = util.keysDifference(this._tiles, retain); + for (let i = 0; i < remove.length; i++) { + this._removeTile(remove[i]); + } + } + + _updateRetainedTiles(idealTileCoords: Array, zoom: number, minCoveringZoom: number) { + let i, coord, tile, covered; + const retain = {}; const checked = {}; for (i = 0; i < idealTileCoords.length; i++) { coord = idealTileCoords[i]; @@ -410,7 +450,7 @@ class SourceCache extends Evented { // Check all four actual child tiles. const children = coord.children(this._source.maxzoom); for (let j = 0; j < children.length; j++) { - const childCoord = children[i]; + const childCoord = children[j]; const childTile = childCoord ? this.getTile(childCoord) : null; if (!!childTile && childTile.hasData()) { @@ -434,7 +474,6 @@ class SourceCache extends Evented { tile = this.getTile(parentId); - if (!tile && parentWasRequested) { tile = this._addTile(parentId); } @@ -453,47 +492,7 @@ class SourceCache extends Evented { } } - const parentsForFading = {}; - - if (isRasterType(this._source.type)) { - const ids = Object.keys(retain); - for (let k = 0; k < ids.length; k++) { - const id = ids[k]; - coord = TileCoord.fromID(+id); - tile = this._tiles[id]; - if (!tile) continue; - - // If the drawRasterTile has never seen this tile, then - // tile.fadeEndTime may be unset. In that case, or if - // fadeEndTime is in the future, then this tile is still - // fading in. Find tiles to cross-fade with it. - if (typeof tile.fadeEndTime === 'undefined' || tile.fadeEndTime >= Date.now()) { - if (this._findLoadedChildren(coord, maxCoveringZoom, retain)) { - retain[id] = true; - } - parentTile = this.findLoadedParent(coord, minCoveringZoom, parentsForFading); - if (parentTile) { - this._addTile(parentTile.coord); - } - } - } - } - - let fadedParent; - for (fadedParent in parentsForFading) { - if (!retain[fadedParent]) { - // If a tile is only needed for fading, mark it as covered so that it isn't rendered on it's own. - this._coveredTiles[fadedParent] = true; - } - } - for (fadedParent in parentsForFading) { - retain[fadedParent] = true; - } - // Remove the tiles we don't need anymore. - const remove = util.keysDifference(this._tiles, retain); - for (i = 0; i < remove.length; i++) { - this._removeTile(remove[i]); - } + return retain; } /** diff --git a/test/unit/source/source_cache.test.js b/test/unit/source/source_cache.test.js index 1edf903c25f..4c8c6edbeca 100644 --- a/test/unit/source/source_cache.test.js +++ b/test/unit/source/source_cache.test.js @@ -582,6 +582,7 @@ test('SourceCache#update', (t) => { transform.resize(511, 511); transform.zoom = 1; + const sourceCache = createSourceCache({ loadTile: function(tile, callback) { tile.timeAdded = Date.now(); @@ -687,6 +688,107 @@ test('SourceCache#update', (t) => { t.end(); }); +test('SourceCache#_updateRetainedTiles', (t)=>{ + + t.test('loads ideal tiles if they exist', (t)=>{ + const stateCache = {}; + const sourceCache = createSourceCache({ + loadTile: function(tile, callback) { + tile.state = stateCache[tile.coord.id] || 'errored'; + callback(); + } + }); + + const getTileSpy = t.spy(sourceCache, 'getTile'); + const idealTile = new TileCoord(1, 1, 1); + stateCache[idealTile.id] = 'loaded'; + sourceCache._updateRetainedTiles([idealTile], 1, 0); + t.ok(getTileSpy.notCalled); + t.deepEqual(sourceCache.getIds(), [idealTile.id]); + t.end(); + }); + + t.test('adds parent tile if ideal tile errors and no child tiles are loaded', (t)=>{ + const stateCache = {}; + const sourceCache = createSourceCache({ + loadTile: function(tile, callback) { + tile.state = stateCache[tile.coord.id] || 'errored'; + callback(); + } + }); + + const addTileSpy = t.spy(sourceCache, '_addTile'); + const getTileSpy = t.spy(sourceCache, 'getTile'); + + const idealTiles = [new TileCoord(1, 1, 1), new TileCoord(1, 0, 1)]; + stateCache[idealTiles[0].id] = 'loaded'; + sourceCache._updateRetainedTiles(idealTiles, 1, 0); + + // checks all child tiles to see if they're loaded before loading parent + t.deepEqual(getTileSpy.getCall(0).args[0], new TileCoord(2, 0, 2)); + t.deepEqual(getTileSpy.getCall(1).args[0], new TileCoord(2, 1, 2)); + t.deepEqual(getTileSpy.getCall(2).args[0], new TileCoord(2, 0, 3)); + t.deepEqual(getTileSpy.getCall(3).args[0], new TileCoord(2, 1, 3)); + + // when child tiles aren't found, check and request parent tile + t.deepEqual(getTileSpy.getCall(4).args[0], new TileCoord(0, 0, 0)); + t.deepEqual(addTileSpy.getCall(2).args[0], new TileCoord(0, 0, 0)); + // retained tiles include all ideal tiles and any parents that were loaded to cover + // non-existant tiles + t.deepEqual(sourceCache.getIds(), [0, idealTiles[1].id, idealTiles[0].id]); + + + addTileSpy.restore(); + getTileSpy.restore(); + t.end(); + }); + + t.test('don\'t use wrong parent tile', (t)=> { + const sourceCache = createSourceCache({ + loadTile: function(tile, callback) { + tile.state = 'errored'; + callback(); + } + }); + + const idealTiles = [new TileCoord(2, 0, 0)]; + sourceCache._tiles[idealTiles[0].id] = new Tile(idealTiles[0]); + sourceCache._tiles[idealTiles[0].id].state = 'errored'; + + sourceCache._tiles[new TileCoord(1, 1, 0).id] = new Tile(new TileCoord(1, 1, 0)); + sourceCache._tiles[new TileCoord(1, 1, 0).id].state = 'loaded'; + + + const addTileSpy = t.spy(sourceCache, '_addTile'); + const getTileSpy = t.spy(sourceCache, 'getTile'); + + const retained = sourceCache._updateRetainedTiles(idealTiles, 2, 0); + + const expectedCalls = [ + // all children + new TileCoord(3, 0, 0), // not found + new TileCoord(3, 1, 0), // not found + new TileCoord(3, 0, 1), // not found + new TileCoord(3, 1, 1), // not found + // parents + new TileCoord(1, 0, 0), // not found + new TileCoord(0, 0, 0) // not found + ]; + + getTileSpy.getCalls().forEach((call, i) => { + t.deepEqual(call.args[0], expectedCalls[i]); + }); + + addTileSpy.restore(); + getTileSpy.restore(); + t.end(); + }); + + + + t.end(); +}); + test('SourceCache#clearTiles', (t) => { t.test('unloads tiles', (t) => { const coord = new TileCoord(0, 0, 0); From 9eb352f4baa4b18d12490729a2012bc914e24264 Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Wed, 6 Sep 2017 10:19:13 -0700 Subject: [PATCH 6/9] lint --- test/unit/source/source_cache.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/unit/source/source_cache.test.js b/test/unit/source/source_cache.test.js index 4c8c6edbeca..0bfeb19475c 100644 --- a/test/unit/source/source_cache.test.js +++ b/test/unit/source/source_cache.test.js @@ -762,8 +762,6 @@ test('SourceCache#_updateRetainedTiles', (t)=>{ const addTileSpy = t.spy(sourceCache, '_addTile'); const getTileSpy = t.spy(sourceCache, 'getTile'); - const retained = sourceCache._updateRetainedTiles(idealTiles, 2, 0); - const expectedCalls = [ // all children new TileCoord(3, 0, 0), // not found From b4734db1ff8c2176acbc2231ba2ea168f600d66a Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Wed, 6 Sep 2017 16:44:48 -0700 Subject: [PATCH 7/9] more unit tests --- src/source/source_cache.js | 10 +- test/unit/source/source_cache.test.js | 293 ++++++++++++++++++++++++-- 2 files changed, 277 insertions(+), 26 deletions(-) diff --git a/src/source/source_cache.js b/src/source/source_cache.js index 45a5bb2f890..df523a21640 100644 --- a/src/source/source_cache.js +++ b/src/source/source_cache.js @@ -368,7 +368,7 @@ class SourceCache extends Evented { // Retain is a list of tiles that we shouldn't delete, even if they are not // the most ideal tile for the current viewport. This may include tiles like // parent or child tiles that are *already* loaded. - const retain = this._updateRetainedTiles(idealTileCoords, zoom, minCoveringZoom); + const retain = this._updateRetainedTiles(idealTileCoords, zoom); const parentsForFading = {}; @@ -413,11 +413,14 @@ class SourceCache extends Evented { } } - _updateRetainedTiles(idealTileCoords: Array, zoom: number, minCoveringZoom: number) { + _updateRetainedTiles(idealTileCoords: Array, zoom: number) { let i, coord, tile, covered; const retain = {}; const checked = {}; + const minCoveringZoom = Math.max(zoom - SourceCache.maxOverzooming, this._source.minzoom); + + for (i = 0; i < idealTileCoords.length; i++) { coord = idealTileCoords[i]; tile = this._addTile(coord); @@ -452,7 +455,6 @@ class SourceCache extends Evented { for (let j = 0; j < children.length; j++) { const childCoord = children[j]; const childTile = childCoord ? this.getTile(childCoord) : null; - if (!!childTile && childTile.hasData()) { retain[childCoord.id] = true; } else { @@ -462,8 +464,10 @@ class SourceCache extends Evented { } if (!covered) { + // We couldn't find child tiles that entirely cover the ideal tile. for (let overscaledZ = zoom - 1; overscaledZ >= minCoveringZoom; --overscaledZ) { + const parentId = coord.scaledTo(overscaledZ); if (checked[parentId.id]) { // Break parent tile ascent, this route has been previously checked by another child. diff --git a/test/unit/source/source_cache.test.js b/test/unit/source/source_cache.test.js index 0bfeb19475c..2e1b9a31a51 100644 --- a/test/unit/source/source_cache.test.js +++ b/test/unit/source/source_cache.test.js @@ -688,7 +688,7 @@ test('SourceCache#update', (t) => { t.end(); }); -test('SourceCache#_updateRetainedTiles', (t)=>{ +test('SourceCache#_updateRetainedTiles', (t)=> { t.test('loads ideal tiles if they exist', (t)=>{ const stateCache = {}; @@ -702,7 +702,7 @@ test('SourceCache#_updateRetainedTiles', (t)=>{ const getTileSpy = t.spy(sourceCache, 'getTile'); const idealTile = new TileCoord(1, 1, 1); stateCache[idealTile.id] = 'loaded'; - sourceCache._updateRetainedTiles([idealTile], 1, 0); + sourceCache._updateRetainedTiles([idealTile], 1); t.ok(getTileSpy.notCalled); t.deepEqual(sourceCache.getIds(), [idealTile.id]); t.end(); @@ -722,22 +722,28 @@ test('SourceCache#_updateRetainedTiles', (t)=>{ const idealTiles = [new TileCoord(1, 1, 1), new TileCoord(1, 0, 1)]; stateCache[idealTiles[0].id] = 'loaded'; - sourceCache._updateRetainedTiles(idealTiles, 1, 0); + const retained = sourceCache._updateRetainedTiles(idealTiles, 1); + t.deepEqual(getTileSpy.getCalls().map((c)=>{ return c.args[0]; }), [ + // checks all child tiles to see if they're loaded before loading parent + new TileCoord(2, 0, 2), + new TileCoord(2, 1, 2), + new TileCoord(2, 0, 3), + new TileCoord(2, 1, 3), + + // when child tiles aren't found, check and request parent tile + new TileCoord(0, 0, 0) + ]); - // checks all child tiles to see if they're loaded before loading parent - t.deepEqual(getTileSpy.getCall(0).args[0], new TileCoord(2, 0, 2)); - t.deepEqual(getTileSpy.getCall(1).args[0], new TileCoord(2, 1, 2)); - t.deepEqual(getTileSpy.getCall(2).args[0], new TileCoord(2, 0, 3)); - t.deepEqual(getTileSpy.getCall(3).args[0], new TileCoord(2, 1, 3)); - - // when child tiles aren't found, check and request parent tile - t.deepEqual(getTileSpy.getCall(4).args[0], new TileCoord(0, 0, 0)); - t.deepEqual(addTileSpy.getCall(2).args[0], new TileCoord(0, 0, 0)); // retained tiles include all ideal tiles and any parents that were loaded to cover // non-existant tiles - t.deepEqual(sourceCache.getIds(), [0, idealTiles[1].id, idealTiles[0].id]); - - + t.deepEqual(retained, { + // parent + '0': true, + // 1/0/1 + '65': true, + // 1/1/1 + '97': true + }); addTileSpy.restore(); getTileSpy.restore(); t.end(); @@ -751,18 +757,18 @@ test('SourceCache#_updateRetainedTiles', (t)=>{ } }); - const idealTiles = [new TileCoord(2, 0, 0)]; - sourceCache._tiles[idealTiles[0].id] = new Tile(idealTiles[0]); - sourceCache._tiles[idealTiles[0].id].state = 'errored'; + const idealTile = new TileCoord(2, 0, 0); + sourceCache._tiles[idealTile.id] = new Tile(idealTile); + sourceCache._tiles[idealTile.id].state = 'errored'; sourceCache._tiles[new TileCoord(1, 1, 0).id] = new Tile(new TileCoord(1, 1, 0)); sourceCache._tiles[new TileCoord(1, 1, 0).id].state = 'loaded'; - const addTileSpy = t.spy(sourceCache, '_addTile'); const getTileSpy = t.spy(sourceCache, 'getTile'); - const expectedCalls = [ + sourceCache._updateRetainedTiles([idealTile], 2); + t.deepEqual(getTileSpy.getCalls().map((c)=>{ return c.args[0]; }), [ // all children new TileCoord(3, 0, 0), // not found new TileCoord(3, 1, 0), // not found @@ -771,19 +777,260 @@ test('SourceCache#_updateRetainedTiles', (t)=>{ // parents new TileCoord(1, 0, 0), // not found new TileCoord(0, 0, 0) // not found - ]; + ]); + + t.deepEqual(addTileSpy.getCalls().map((c)=>{ return c.args[0]; }), [ + // ideal tile + new TileCoord(2, 0, 0), + // parents + new TileCoord(1, 0, 0), // not found + new TileCoord(0, 0, 0) // not found + ]); + + addTileSpy.restore(); + getTileSpy.restore(); + t.end(); + }); + + + t.test('use parent tile when ideal tile is not loaded', (t)=>{ + const sourceCache = createSourceCache({ + loadTile: function(tile, callback) { + tile.state = 'loading'; + callback(); + } + }); + const idealTile = new TileCoord(1, 0, 1); + sourceCache._tiles[idealTile.id] = new Tile(idealTile); + sourceCache._tiles[idealTile.id].state = 'loading'; + sourceCache._tiles['0'] = new Tile(0, 0, 0); + sourceCache._tiles['0'].state = 'loaded'; + + const addTileSpy = t.spy(sourceCache, '_addTile'); + const getTileSpy = t.spy(sourceCache, 'getTile'); + + const retained = sourceCache._updateRetainedTiles([idealTile], 1); + + t.deepEqual(getTileSpy.getCalls().map((c)=>{ return c.args[0]; }), [ + // all children + new TileCoord(2, 0, 2), // not found + new TileCoord(2, 1, 2), // not found + new TileCoord(2, 0, 3), // not found + new TileCoord(2, 1, 3), // not found + // parents + new TileCoord(0, 0, 0), // found + ]); + + t.deepEqual(retained, { + // parent of ideal tile + '0' : true, + // ideal tile id + '65' : true + }, 'retain ideal and parent tile when ideal tiles aren\'t loaded'); - getTileSpy.getCalls().forEach((call, i) => { - t.deepEqual(call.args[0], expectedCalls[i]); + addTileSpy.reset(); + getTileSpy.reset(); + + // now make sure we don't retain the parent tile when the ideal tile is loaded + sourceCache._tiles[idealTile.id].state = 'loaded'; + const retainedLoaded = sourceCache._updateRetainedTiles([idealTile], 1); + + t.ok(getTileSpy.notCalled); + t.deepEqual(retainedLoaded, { + // only ideal tile retained + '65' : true + }, 'only retain ideal tiles when they\'re all loaded'); + + addTileSpy.restore(); + getTileSpy.restore(); + + + t.end(); + }); + + t.test('prefer loaded child tiles to parent tiles', (t)=>{ + const sourceCache = createSourceCache({ + loadTile: function(tile, callback) { + tile.state = 'loading'; + callback(); + } }); + const idealTile = new TileCoord(1, 0, 0); + const loadedTiles = [new TileCoord(0, 0, 0), new TileCoord(2, 0, 0)]; + loadedTiles.forEach((t)=>{ + sourceCache._tiles[t.id] = new Tile(t); + sourceCache._tiles[t.id].state = 'loaded'; + }); + + const addTileSpy = t.spy(sourceCache, '_addTile'); + const getTileSpy = t.spy(sourceCache, 'getTile'); + let retained = sourceCache._updateRetainedTiles([idealTile], 1); + t.deepEqual(getTileSpy.getCalls().map((c)=>{ return c.args[0]; }), [ + // all children + new TileCoord(2, 0, 0), + new TileCoord(2, 1, 0), + new TileCoord(2, 0, 1), + new TileCoord(2, 1, 1), + // parent + new TileCoord(0, 0, 0) + ]); + + t.deepEqual(retained, { + // parent of ideal tile (0, 0, 0) (only partially covered by loaded child + // tiles, so we still need to load the parent) + '0' : true, + // ideal tile id (1, 0, 0) + '1' : true, + // loaded child tile (2, 0, 0) + '2': true + }, 'retains children and parent when ideal tile is partially covered by a loaded child tile'); addTileSpy.restore(); + getTileSpy.restore(); + // remove child tile and check that it only uses parent tile + sourceCache._tiles['2'] = null; + retained = sourceCache._updateRetainedTiles([idealTile], 1); + + t.deepEqual(retained, { + // parent of ideal tile (0, 0, 0) (only partially covered by loaded child + // tiles, so we still need to load the parent) + '0' : true, + // ideal tile id (1, 0, 0) + '1' : true + }, 'only retains parent tile if no child tiles are loaded'); + + t.end(); + }); + + t.test('don\'t use tiles below minzoom', (t)=>{ + const sourceCache = createSourceCache({ + loadTile: function(tile, callback) { + tile.state = 'loading'; + callback(); + }, + minzoom: 2 + }); + const idealTile = new TileCoord(2, 0, 0); + const loadedTiles = [new TileCoord(1, 0, 0)]; + loadedTiles.forEach((t)=>{ + sourceCache._tiles[t.id] = new Tile(t); + sourceCache._tiles[t.id].state = 'loaded'; + }); + + const getTileSpy = t.spy(sourceCache, 'getTile'); + const retained = sourceCache._updateRetainedTiles([idealTile], 2); + + t.deepEqual(getTileSpy.getCalls().map((c)=>{ return c.args[0]; }), [ + // all children + new TileCoord(3, 0, 0), + new TileCoord(3, 1, 0), + new TileCoord(3, 0, 1), + new TileCoord(3, 1, 1) + ], 'doesn\'t request parent tiles bc they are lower than minzoom'); + + t.deepEqual(retained, { + // ideal tile id (1, 0, 0) + '2' : true + }, 'doesn\'t retain parent tiles below minzoom'); + + getTileSpy.restore(); + t.end(); + }); + + t.test('use overzoomed tile above maxzoom', (t)=>{ + const sourceCache = createSourceCache({ + loadTile: function(tile, callback) { + tile.state = 'loading'; + callback(); + }, + maxzoom: 2 + }); + const idealTile = new TileCoord(2, 0, 0); + + const getTileSpy = t.spy(sourceCache, 'getTile'); + const retained = sourceCache._updateRetainedTiles([idealTile], 2); + + t.deepEqual(getTileSpy.getCalls().map((c)=>{ return c.args[0]; }), [ + // overzoomed child + new TileCoord(3, 0, 0), + // parents + new TileCoord(1, 0, 0), + new TileCoord(0, 0, 0) + ], 'doesn\'t request childtiles above maxzoom'); + + t.deepEqual(retained, { + // ideal tile id (1, 0, 0) + '2' : true + }, 'doesn\'t retain child tiles above maxzoom'); + getTileSpy.restore(); t.end(); }); + t.test('dont\'t ascend multiple times if a tile is not found', (t)=>{ + const sourceCache = createSourceCache({ + loadTile: function(tile, callback) { + tile.state = 'loading'; + callback(); + } + }); + const idealTiles = [new TileCoord(8, 0, 0), new TileCoord(8, 1, 0)]; + const getTileSpy = t.spy(sourceCache, 'getTile'); + sourceCache._updateRetainedTiles(idealTiles, 8); + t.deepEqual(getTileSpy.getCalls().map((c)=>{ return c.args[0]; }), [ + // child tiles + new TileCoord(9, 0, 0), + new TileCoord(9, 1, 0), + new TileCoord(9, 0, 1), + new TileCoord(9, 1, 1), + // parent tile ascent + new TileCoord(7, 0, 0), + new TileCoord(6, 0, 0), + new TileCoord(5, 0, 0), + new TileCoord(4, 0, 0), + new TileCoord(3, 0, 0), + new TileCoord(2, 0, 0), + new TileCoord(1, 0, 0), + new TileCoord(0, 0, 0), + // second ideal tile children, no parent ascent + new TileCoord(9, 2, 0), + new TileCoord(9, 3, 0), + new TileCoord(9, 2, 1), + new TileCoord(9, 3, 1) + ], 'only ascends up a tile pyramid once'); + + getTileSpy.reset(); + + const loadedTiles = [new TileCoord(4, 0, 0)]; + loadedTiles.forEach((t)=>{ + sourceCache._tiles[t.id] = new Tile(t); + sourceCache._tiles[t.id].state = 'loaded'; + }); + + sourceCache._updateRetainedTiles(idealTiles, 8); + t.deepEqual(getTileSpy.getCalls().map((c)=>{ return c.args[0]; }), [ + // child tiles + new TileCoord(9, 0, 0), + new TileCoord(9, 1, 0), + new TileCoord(9, 0, 1), + new TileCoord(9, 1, 1), + // parent tile ascent + new TileCoord(7, 0, 0), + new TileCoord(6, 0, 0), + new TileCoord(5, 0, 0), + new TileCoord(4, 0, 0), // tile is loaded, stops ascent + + // second ideal tile children, no parent ascent + new TileCoord(9, 2, 0), + new TileCoord(9, 3, 0), + new TileCoord(9, 2, 1), + new TileCoord(9, 3, 1) + ], 'ascent stops if a loaded parent tile is found'); + getTileSpy.restore(); + t.end(); + }); t.end(); }); From bc58f974e11785e50d84b5a5e884af49f64af9da Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Thu, 7 Sep 2017 15:07:36 -0700 Subject: [PATCH 8/9] unignore raster masking test --- test/ignores.json | 1 - .../render-tests/raster-masking/overlapping/style.json | 7 +++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/test/ignores.json b/test/ignores.json index 29841a63987..97c7411877c 100644 --- a/test/ignores.json +++ b/test/ignores.json @@ -11,7 +11,6 @@ "render-tests/fill-extrusion-pattern/opacity": "https://github.com/mapbox/mapbox-gl-js/issues/3327", "render-tests/geojson/inline-linestring-fill": "current behavior is arbitrary", "render-tests/line-dasharray/zoom-history": "needs investigation", - "render-tests/raster-masking/overlapping": "https://github.com/mapbox/mapbox-gl-js/issues/5003", "render-tests/regressions/mapbox-gl-js#3682": "skip - true", "render-tests/runtime-styling/image-add-alpha": "failing on Circle CI 2.0, needs investigation", "render-tests/runtime-styling/image-update-icon": "skip - https://github.com/mapbox/mapbox-gl-js/issues/4804", diff --git a/test/integration/render-tests/raster-masking/overlapping/style.json b/test/integration/render-tests/raster-masking/overlapping/style.json index 430896e4bfc..7bf70d9a603 100644 --- a/test/integration/render-tests/raster-masking/overlapping/style.json +++ b/test/integration/render-tests/raster-masking/overlapping/style.json @@ -31,7 +31,10 @@ { "id": "raster", "type": "raster", - "source": "contour" + "source": "contour", + "paint": { + "raster-fade-duration": 0 + } } ] -} \ No newline at end of file +} From f6d3d50b6d34ddb191ba2f46e3d21931473293dd Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Mon, 11 Sep 2017 11:27:56 -0700 Subject: [PATCH 9/9] type _updateRenderables vars --- src/source/source_cache.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/source/source_cache.js b/src/source/source_cache.js index df523a21640..1dc68cdc77b 100644 --- a/src/source/source_cache.js +++ b/src/source/source_cache.js @@ -413,11 +413,11 @@ class SourceCache extends Evented { } } - _updateRetainedTiles(idealTileCoords: Array, zoom: number) { + _updateRetainedTiles(idealTileCoords: Array, zoom: number): { [string]: boolean} { let i, coord, tile, covered; const retain = {}; - const checked = {}; + const checked: {[number]: boolean } = {}; const minCoveringZoom = Math.max(zoom - SourceCache.maxOverzooming, this._source.minzoom);