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.

Sign up for GitHub

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

Optimize and simplify tile retention logic #6995

Merged
merged 8 commits into from
Jul 31, 2018
Merged

Optimize and simplify tile retention logic #6995

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5b4a9d9
clean up tile retention logic a bit
mourner Jul 19, 2018
2bd0b65
simplify retention logic further
mourner Jul 19, 2018
6a13aeb
simplify raster tile retaining logic
mourner Jul 19, 2018
0737b11
avoid delete when retaining child tiles
mourner Jul 19, 2018
45bbc09
much faster children retention logic
mourner Jul 20, 2018
232b618
simplify parent tile retention
mourner Jul 24, 2018
323aaf0
address PR feedback
mourner Jul 25, 2018
210b686
clear up source cache comments
mourner Jul 31, 2018
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
2 changes: 1 addition & 1 deletion src/render/draw_raster.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ function drawRaster(painter: Painter, sourceCache: SourceCache, layer: RasterSty

gl.uniformMatrix4fv(program.uniforms.u_matrix, false, posMatrix);

const parentTile = sourceCache.findLoadedParent(coord, 0, {}),
const parentTile = sourceCache.findLoadedParent(coord, 0),
fade = getFadeValues(tile, parentTile, sourceCache, layer, painter.transform);

let parentScaleBy, parentTL;
Expand Down
247 changes: 124 additions & 123 deletions src/source/source_cache.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ class SourceCache extends Evented {
}

hasRenderableParent(tileID: OverscaledTileID) {
const parentTile = this.findLoadedParent(tileID, 0, {});
const parentTile = this.findLoadedParent(tileID, 0);
if (parentTile) {
return this._isIdRenderable(parentTile.tileID.key);
}
Expand Down Expand Up @@ -324,59 +324,64 @@ class SourceCache extends Evented {
}

/**
* Recursively find children of the given tile (up to maxCoveringZoom) that are already loaded;
* adds found tiles to retain object; returns true if any child is found.
* Retain children of the given set of tiles (up to maxCoveringZoom) that are already loaded;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this mention the zoom parameter too? It seems to be something like:

Retain children of the given set of tiles that are loaded and have a zoom between zoom (exclusive) and maxCoveringZoom (inclusive).

*/
_findLoadedChildren(tileID: OverscaledTileID, maxCoveringZoom: number, retain: {[any]: OverscaledTileID}): boolean {
let found = false;

_retainLoadedChildren(
idealTiles: {[any]: OverscaledTileID},
zoom: number,
maxCoveringZoom: number,
retain: {[any]: OverscaledTileID}
) {
for (const id in this._tiles) {
let tile = this._tiles[id];

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

// disregard tiles that are not descendants of the given tile coordinate
const z2 = Math.pow(2, tile.tileID.canonical.z - tileID.canonical.z);
if (Math.floor(tile.tileID.canonical.x / z2) !== tileID.canonical.x ||
Math.floor(tile.tileID.canonical.y / z2) !== tileID.canonical.y)
continue;
// only consider renderable tiles up to maxCoveringZoom
if (retain[id] ||
!tile.hasData() ||
tile.tileID.overscaledZ <= zoom ||
tile.tileID.overscaledZ > maxCoveringZoom
) continue;

// found loaded child
retain[id] = tile.tileID;
found = true;
// loop through parents and retain the topmost loaded one if found
let topmostLoadedID = tile.tileID;
while (tile && tile.tileID.overscaledZ > zoom + 1) {
const parentID = tile.tileID.scaledTo(tile.tileID.overscaledZ - 1);

// loop through parents; retain the topmost loaded one if found
while (tile && tile.tileID.overscaledZ - 1 > tileID.overscaledZ) {
const parent = tile.tileID.scaledTo(tile.tileID.overscaledZ - 1);
if (!parent) break;
tile = this._tiles[parentID.key];

tile = this._tiles[parent.key];
if (tile && tile.hasData()) {
delete retain[id];
retain[parent.key] = parent;
topmostLoadedID = parentID;
}
}

// loop through ancestors of the topmost loaded child to see if there's one that needed it
let tileID = topmostLoadedID;
while (tileID.overscaledZ > zoom) {
tileID = tileID.scaledTo(tileID.overscaledZ - 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can you use topmostLoadedID in the conditional and have tileID be a const?

while (topmostLoadedID.overscaledZ > zoom) {
  const tileID = topmostLoadedID.scaledTo(topmostLoadedID.overscaledZ-1);
  ...

Copy link
Member Author

Choose a reason for hiding this comment

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

No, since tileID loops through parents — changing to the above would do a different thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh yeah 🤦‍♀️ its a while loop


if (idealTiles[tileID.key]) {
// found a parent that needed a loaded child; retain that child
retain[topmostLoadedID.key] = topmostLoadedID;
break;
}
}
}
return found;
}

/**
* Find a loaded parent of the given tile (up to minCoveringZoom);
* adds the found tile to retain object and returns the tile if found
Copy link
Contributor

Choose a reason for hiding this comment

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

This clause no longer applies.

*/
findLoadedParent(tileID: OverscaledTileID, minCoveringZoom: number, retain: {[any]: OverscaledTileID}): ?Tile {
findLoadedParent(tileID: OverscaledTileID, minCoveringZoom: number): ?Tile {
for (let z = tileID.overscaledZ - 1; z >= minCoveringZoom; z--) {
const parent = tileID.scaledTo(z);
if (!parent) return;
const id = String(parent.key);
const tile = this._tiles[id];
if (tile && tile.hasData()) {
retain[id] = parent;
return tile;
}
if (this._cache.has(parent)) {
retain[id] = parent;
return this._cache.get(parent);
}
}
Expand Down Expand Up @@ -490,52 +495,48 @@ class SourceCache extends Evented {
// parent or child tiles that are *already* loaded.
const retain = this._updateRetainedTiles(idealTileIDs, zoom);

const parentsForFading = {};

if (isRasterType(this._source.type)) {
const parentsForFading = {};
const fadingTiles = {};
const ids = Object.keys(retain);
for (let k = 0; k < ids.length; k++) {
const id = ids[k];
for (const id of ids) {
const tileID = retain[id];
assert(tileID.key === +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.
Copy link
Contributor

Choose a reason for hiding this comment

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

can we modify and keep this comment to explain the continue condition below?

Copy link
Member Author

@mourner mourner Jul 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I felt this would be redundant due to the comment after that. So one would say "skip tiles not loaded or already faded in", and another "do X with tiles that are loaded but still fading in".

if (typeof tile.fadeEndTime === 'undefined' || tile.fadeEndTime >= browser.now()) {
if (this._findLoadedChildren(tileID, maxCoveringZoom, retain)) {
retain[id] = tileID;
}
const parentTile = this.findLoadedParent(tileID, minCoveringZoom, parentsForFading);
if (parentTile) {
this._addTile(parentTile.tileID);
}
if (!tile || tile.fadeEndTime && tile.fadeEndTime <= browser.now()) continue;

// if the tile is loaded but still fading in, find parents to cross-fade with it
const parentTile = this.findLoadedParent(tileID, minCoveringZoom);
if (parentTile) {
this._addTile(parentTile.tileID);
parentsForFading[parentTile.tileID.key] = parentTile.tileID;
}

fadingTiles[id] = tileID;
}
}

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 tiles that are still fading in, also find children to cross-fade with
this._retainLoadedChildren(fadingTiles, zoom, maxCoveringZoom, retain);

for (const id in parentsForFading) {
if (!retain[id]) {
// If a tile is only needed for fading, mark it as covered so that it isn't rendered on it's own.
this._coveredTiles[id] = true;
retain[id] = parentsForFading[id];
}
}
}
for (fadedParent in parentsForFading) {
retain[fadedParent] = parentsForFading[fadedParent];
}

for (const retainedId in retain) {
// Make sure retained tiles always clear any existing fade holds
// so that if they're removed again their fade timer starts fresh.
this._tiles[retainedId].clearFadeHold();
}

// Remove the tiles we don't need anymore.
const remove = keysDifference(this._tiles, retain);
for (let i = 0; i < remove.length; i++) {
const tileID = remove[i];
for (const tileID of remove) {
const tile = this._tiles[tileID];
if (tile.hasSymbolBuckets && !tile.holdingForFade()) {
tile.setHoldDuration(this.map._fadeDuration);
Expand All @@ -559,74 +560,74 @@ class SourceCache extends Evented {
const minCoveringZoom = Math.max(zoom - SourceCache.maxOverzooming, this._source.minzoom);
const maxCoveringZoom = Math.max(zoom + SourceCache.maxUnderzooming, this._source.minzoom);

for (let i = 0; i < idealTileIDs.length; i++) {
const tileID = idealTileIDs[i];
let tile = this._addTile(tileID);
let parentWasRequested = false;
if (tile.hasData()) {
retain[tileID.key] = tileID;
} else {
// The tile we require is not yet loaded or does not exist.
// We are now attempting to load child and parent tiles.

// 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[tileID.key] = tileID;
let covered = true;
const overscaledZ = zoom + 1;
if (overscaledZ > this._source.maxzoom) {
// We're looking for an overzoomed child tile.
const childCoord = tileID.children(this._source.maxzoom)[0];
const childTile = this.getTile(childCoord);
if (!!childTile && childTile.hasData()) {
retain[childCoord.key] = childCoord;
} else {
covered = false;
}
} else {
this._findLoadedChildren(tileID, maxCoveringZoom, retain);
// check if all 4 immediate children are loaded (i.e. the missing ideal tile is covered)
const children = tileID.children(this._source.maxzoom);
for (let j = 0; j < children.length; j++) {
if (!retain[children[j].key]) {
covered = false;
break;
}
}
const missingTiles = {};
for (const tileID of idealTileIDs) {
const tile = this._addTile(tileID);

// retain the tile even if it's not loaded because it's an ideal tile.
retain[tileID.key] = tileID;

if (tile.hasData()) continue;

if (zoom < this._source.maxzoom) {
// save missing tiles that potentially have loaded children
missingTiles[tileID.key] = tileID;
}
}

// retain any loaded children of ideal tiles up to maxCoveringZoom
this._retainLoadedChildren(missingTiles, zoom, maxCoveringZoom, retain);

for (const tileID of idealTileIDs) {
let tile = this._tiles[tileID.key];

if (tile.hasData()) continue;

// The tile we require is not yet loaded or does not exist;
// Attempt to find children that fully cover it.

if (zoom + 1 > this._source.maxzoom) {
// We're looking for an overzoomed child tile.
const childCoord = tileID.children(this._source.maxzoom)[0];
const childTile = this.getTile(childCoord);
if (!!childTile && childTile.hasData()) {
retain[childCoord.key] = childCoord;
continue; // tile is covered by overzoomed child
}
} else {
// check if all 4 immediate children are loaded (i.e. the missing ideal tile is covered)
const children = tileID.children(this._source.maxzoom);

if (retain[children[0].key] &&
retain[children[1].key] &&
retain[children[2].key] &&
retain[children[3].key]) continue; // tile is covered by children
}

// We couldn't find child tiles that entirely cover the ideal tile; look for parents now.

if (!covered) {

// We couldn't find child tiles that entirely cover the ideal tile.
for (let overscaledZ = tileID.overscaledZ - 1; overscaledZ >= minCoveringZoom; --overscaledZ) {

const parentId = tileID.scaledTo(overscaledZ);
if (checked[parentId.key]) {
// Break parent tile ascent, this route has been previously checked by another child.
break;
} else {
checked[parentId.key] = true;
}

tile = this.getTile(parentId);
if (!tile && parentWasRequested) {
tile = this._addTile(parentId);
}

if (tile) {
retain[parentId.key] = parentId;
// Save the current values, since they're the parent of the next iteration
// of the parent tile ascent loop.
parentWasRequested = tile.wasRequested();
if (tile.hasData()) {
break;
}
}
}
// As we ascend up the tile pyramid of the ideal tile, we check whether the parent
// tile has been previously requested (and errored because we only loop over tiles with no data)
// in order to determine if we need to request its parent.
let parentWasRequested = tile.wasRequested();

for (let overscaledZ = tileID.overscaledZ - 1; overscaledZ >= minCoveringZoom; --overscaledZ) {
const parentId = tileID.scaledTo(overscaledZ);

// Break parent tile ascent if this route has been previously checked by another child.
if (checked[parentId.key]) break;
checked[parentId.key] = true;

tile = this.getTile(parentId);
if (!tile && parentWasRequested) {
tile = this._addTile(parentId);
}
if (tile) {
retain[parentId.key] = parentId;
// Save the current values, since they're the parent of the next iteration
// of the parent tile ascent loop.
parentWasRequested = tile.wasRequested();
if (tile.hasData()) break;
}
}
}
Expand Down
18 changes: 4 additions & 14 deletions test/unit/source/source_cache.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1443,13 +1443,8 @@ test('SourceCache#findLoadedParent', (t) => {

sourceCache._tiles[tile.tileID.key] = tile;

const retain = {};
const expectedRetain = {};
expectedRetain[tile.tileID.key] = tile.tileID;

t.equal(sourceCache.findLoadedParent(new OverscaledTileID(2, 0, 2, 3, 3), 0, retain), undefined);
t.deepEqual(sourceCache.findLoadedParent(new OverscaledTileID(2, 0, 2, 0, 0), 0, retain), tile);
t.deepEqual(retain, expectedRetain);
t.equal(sourceCache.findLoadedParent(new OverscaledTileID(2, 0, 2, 3, 3), 0), undefined);
t.deepEqual(sourceCache.findLoadedParent(new OverscaledTileID(2, 0, 2, 0, 0), 0), tile);
t.end();
});

Expand All @@ -1464,13 +1459,8 @@ test('SourceCache#findLoadedParent', (t) => {
const tile = new Tile(new OverscaledTileID(1, 0, 1, 0, 0), 512, 22);
sourceCache._cache.add(tile.tileID, tile);

const retain = {};
const expectedRetain = {};
expectedRetain[tile.tileID.key] = tile.tileID;

t.equal(sourceCache.findLoadedParent(new OverscaledTileID(2, 0, 2, 3, 3), 0, retain), undefined);
t.equal(sourceCache.findLoadedParent(new OverscaledTileID(2, 0, 2, 0, 0), 0, retain), tile);
t.deepEqual(retain, expectedRetain);
t.equal(sourceCache.findLoadedParent(new OverscaledTileID(2, 0, 2, 3, 3), 0), undefined);
t.equal(sourceCache.findLoadedParent(new OverscaledTileID(2, 0, 2, 0, 0), 0), tile);
t.equal(sourceCache._cache.order.length, 1);

t.end();
Expand Down