Skip to content

Commit e5de3a6

Browse files
wayofthefuturewayofthefuture
authored
Fix issue where parent tiles are retained when deeper descendant tiles already cover the missing ideal tile. (#6442)
* Add failing test to show where parent tiles are retained/requested for an ideal tile covered by 2nd generation children * add appropriate tests and modify nth generational retainment logic * change log, build size * rename test, remove throw --------- Co-authored-by: wayofthefuture <[email protected]>
1 parent 4ac8a61 commit e5de3a6

File tree

5 files changed

+125
-73
lines changed

5 files changed

+125
-73
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
- _...Add new stuff here..._
55

66
### 🐞 Bug fixes
7-
- _...Add new stuff here..._
7+
- Fix issue where parent tiles are retained when deeper descendant tiles already cover the missing ideal tile. ([#6442](https://github.com/maplibre/maplibre-gl-js/pull/6442))
88

99
## 5.7.3
1010

‎src/source/source_cache.test.ts‎

Lines changed: 69 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -971,6 +971,31 @@ describe('SourceCache._updateRetainedTiles', () => {
971971
expect(Object.keys(retained).sort()).toEqual(expectedTiles.map(t => t.key).sort());
972972
});
973973

974+
test('_updateRetainedTiles does not retain parents when 2nd generation children are loaded', () => {
975+
const sourceCache = createSourceCache();
976+
sourceCache._source.loadTile = async (tile) => {
977+
tile.state = 'errored';
978+
};
979+
980+
const idealTile = new OverscaledTileID(3, 0, 3, 1, 2);
981+
sourceCache._tiles[idealTile.key] = new Tile(idealTile, undefined);
982+
sourceCache._tiles[idealTile.key].state = 'errored';
983+
984+
const secondGeneration = idealTile
985+
.children(10)
986+
.flatMap(child => child.children(10));
987+
expect(secondGeneration.length).toEqual(16);
988+
989+
for (const id of secondGeneration) {
990+
sourceCache._tiles[id.key] = new Tile(id, undefined);
991+
sourceCache._tiles[id.key].state = 'loaded';
992+
}
993+
const expectedTiles = [...secondGeneration, idealTile];
994+
995+
const retained = sourceCache._updateRetainedTiles([idealTile], 3);
996+
expect(Object.keys(retained).sort()).toEqual(expectedTiles.map(t => t.key).sort());
997+
});
998+
974999
for (const pitch of [0, 20, 40, 65, 75, 85]) {
9751000
test(`retains loaded children for pitch: ${pitch}`, () => {
9761001
const transform = new MercatorTransform();
@@ -1070,6 +1095,38 @@ describe('SourceCache._updateRetainedTiles', () => {
10701095
expect(Object.keys(retained).sort()).toEqual([idealTile].concat(loadedChildren).map(t => t.key).sort());
10711096
});
10721097

1098+
test('_areDescendentsComplete returns true when descendents fully cover a generation', () => {
1099+
const sourceCache = createSourceCache();
1100+
const idealTile = new OverscaledTileID(3, 0, 3, 1, 2);
1101+
1102+
const firstGen = idealTile.children(10);
1103+
expect(sourceCache._areDescendentsComplete(firstGen, 4, 3)).toBe(true);
1104+
1105+
const secondGen = idealTile.children(10).flatMap(c => c.children(10));
1106+
expect(sourceCache._areDescendentsComplete(secondGen, 5, 3)).toBe(true);
1107+
});
1108+
1109+
test('_areDescendentsComplete returns false when descendents are incomplete', () => {
1110+
const sourceCache = createSourceCache();
1111+
const idealTile = new OverscaledTileID(3, 0, 3, 1, 2);
1112+
1113+
const firstGenPartial = idealTile.children(10).slice(0, 3);
1114+
expect(sourceCache._areDescendentsComplete(firstGenPartial, 4, 3)).toBe(false);
1115+
1116+
const secondGenPartial = idealTile.children(10).flatMap(c => c.children(10)).slice(0, 15);
1117+
expect(sourceCache._areDescendentsComplete(secondGenPartial, 5, 3)).toBe(false);
1118+
});
1119+
1120+
test('_areDescendentsComplete properly handles overscaled tiles', () => {
1121+
const sourceCache = createSourceCache();
1122+
1123+
const correct = new OverscaledTileID(4, 0, 3, 1, 2);
1124+
expect(sourceCache._areDescendentsComplete([correct], 4, 3)).toBe(true);
1125+
1126+
const wrong = new OverscaledTileID(5, 0, 3, 1, 2);
1127+
expect(sourceCache._areDescendentsComplete([wrong], 4, 3)).toBe(false);
1128+
});
1129+
10731130
test('adds parent tile if ideal tile errors and no child tiles are loaded', () => {
10741131
const stateCache = {};
10751132
const sourceCache = createSourceCache();
@@ -1272,22 +1329,22 @@ describe('SourceCache._updateRetainedTiles', () => {
12721329
};
12731330
const idealTile = new OverscaledTileID(2, 0, 2, 0, 0);
12741331

1275-
const getTileSpy = vi.spyOn(sourceCache, 'getTile');
1276-
const retained = sourceCache._updateRetainedTiles([idealTile], 2);
1332+
const loadedTiles = [
1333+
new OverscaledTileID(3, 0, 2, 0, 0), // overzoomed child
1334+
new OverscaledTileID(1, 0, 1, 0, 0), // parent
1335+
new OverscaledTileID(0, 0, 0, 0, 0) // parent
1336+
];
1337+
loadedTiles.forEach(t => {
1338+
sourceCache._tiles[t.key] = new Tile(t, undefined);
1339+
sourceCache._tiles[t.key].state = 'loaded';
1340+
});
12771341

1278-
expect(getTileSpy.mock.calls.map((c) => { return c[0]; })).toEqual([
1279-
// overzoomed child
1280-
new OverscaledTileID(3, 0, 2, 0, 0),
1281-
// parents
1282-
new OverscaledTileID(1, 0, 1, 0, 0),
1283-
new OverscaledTileID(0, 0, 0, 0, 0)
1284-
]);
1342+
const retained = sourceCache._updateRetainedTiles([idealTile], 2);
12851343

12861344
expect(retained).toEqual({
1287-
// ideal tile id (2, 0, 0)
1288-
'022': new OverscaledTileID(2, 0, 2, 0, 0)
1345+
'022': new OverscaledTileID(2, 0, 2, 0, 0), // ideal
1346+
'023': new OverscaledTileID(3, 0, 2, 0, 0) // overzoomed
12891347
});
1290-
12911348
});
12921349

12931350
test('don\'t ascend multiple times if a tile is not found', () => {

‎src/source/source_cache.ts‎

Lines changed: 50 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -384,32 +384,38 @@ export class SourceCache extends Evented {
384384
) {
385385
const targetTileIDs = Object.values(targetTiles);
386386
const loadedDescendents: { [_: string]: Tile[] } = this._getLoadedDescendents(targetTileIDs);
387+
const incomplete: { [_: string]: OverscaledTileID } = {};
387388

388389
// retain the uppermost descendents of target tiles
389390
for (const targetID of targetTileIDs) {
390-
const descendentTiles = loadedDescendents[targetID.key];
391-
if (!descendentTiles) continue;
391+
const descendents = loadedDescendents[targetID.key];
392+
if (!descendents?.length) {
393+
incomplete[targetID.key] = targetID;
394+
continue;
395+
}
392396

393-
const targetTileMaxCoveringZoom = targetID.overscaledZ + SourceCache.maxUnderzooming;
397+
// find descendents within the max covering zoom range
398+
const maxCoveringZoom = targetID.overscaledZ + SourceCache.maxUnderzooming;
399+
const candidates = descendents.filter(t => t.tileID.overscaledZ <= maxCoveringZoom);
400+
if (!candidates.length) {
401+
incomplete[targetID.key] = targetID;
402+
continue;
403+
}
394404

395-
// determine the topmost zoom (overscaledZ) in the set of descendent tiles. (i.e. zoom 4 tiles are topmost relative to zoom 5)
396-
let topmostZoom = Infinity;
397-
for (const tile of descendentTiles) {
398-
const zoom = tile.tileID.overscaledZ;
399-
if (zoom <= targetTileMaxCoveringZoom && zoom < topmostZoom) {
400-
topmostZoom = zoom;
401-
}
405+
// retain the uppermost descendents in the topmost zoom below the target tile
406+
const topZoom = Math.min(...candidates.map(t => t.tileID.overscaledZ));
407+
const topIDs = candidates.filter(t => t.tileID.overscaledZ === topZoom).map(t => t.tileID);
408+
for (const tileID of topIDs) {
409+
retain[tileID.key] = tileID;
402410
}
403411

404-
// retain all uppermost descendents (with the same overscaledZ) in the topmost zoom below the target tile
405-
if (topmostZoom !== Infinity) {
406-
for (const tile of descendentTiles) {
407-
if (tile.tileID.overscaledZ === topmostZoom) {
408-
retain[tile.tileID.key] = tile.tileID;
409-
}
410-
}
412+
//determine if the retained generation is fully covered
413+
if (!this._areDescendentsComplete(topIDs, topZoom, targetID.overscaledZ)) {
414+
incomplete[targetID.key] = targetID;
411415
}
412416
}
417+
418+
return incomplete;
413419
}
414420

415421
/**
@@ -434,6 +440,21 @@ export class SourceCache extends Evented {
434440
return loadedDescendents;
435441
}
436442

443+
/**
444+
* Determine if tile ids fully cover the current generation.
445+
* - 1st generation: need 4 children or 1 overscaled child
446+
* - 2nd generation: need 16 children or 1 overscaled child
447+
*/
448+
_areDescendentsComplete(generationIDs: OverscaledTileID[], generationZ: number, ancestorZ: number) {
449+
//if overscaled, seeking 1 tile at generationZ, otherwise seeking a power of 4 for each descending Z
450+
if (generationIDs.length === 1 && generationIDs[0].isOverscaled()) {
451+
return generationIDs[0].overscaledZ === generationZ;
452+
} else {
453+
const expectedTiles = Math.pow(4, generationZ - ancestorZ); //4, 16, 64 (for first 3 gens)
454+
return expectedTiles === generationIDs.length;
455+
}
456+
}
457+
437458
/**
438459
* Find a loaded parent of the given tile (up to minCoveringZoom)
439460
*/
@@ -741,59 +762,29 @@ export class SourceCache extends Evented {
741762
const checked: {[_: string]: boolean} = {};
742763
const minCoveringZoom = Math.max(zoom - SourceCache.maxOverzooming, this._source.minzoom);
743764

744-
const missingTiles = {};
745-
for (const tileID of idealTileIDs) {
746-
const tile = this._addTile(tileID);
765+
let missingIdealTiles = {};
766+
for (const idealID of idealTileIDs) {
767+
const idealTile = this._addTile(idealID);
747768

748769
// retain the tile even if it's not loaded because it's an ideal tile.
749-
retain[tileID.key] = tileID;
750-
751-
if (tile.hasData()) continue;
770+
retain[idealID.key] = idealID;
752771

753-
if (zoom < this._source.maxzoom) {
754-
// save missing tiles that potentially have loaded children
755-
missingTiles[tileID.key] = tileID;
772+
if (!idealTile.hasData()) {
773+
missingIdealTiles[idealID.key] = idealID;
756774
}
757775
}
758776

759-
this._retainLoadedChildren(missingTiles, retain);
760-
761-
for (const tileID of idealTileIDs) {
762-
let tile = this._tiles[tileID.key];
763-
764-
if (tile.hasData()) continue;
765-
766-
// The tile we require is not yet loaded or does not exist;
767-
// Attempt to find children that fully cover it.
768-
769-
if (zoom + 1 > this._source.maxzoom) {
770-
// We're looking for an overzoomed child tile.
771-
const childCoord = tileID.children(this._source.maxzoom)[0];
772-
const childTile = this.getTile(childCoord);
773-
if (!!childTile && childTile.hasData()) {
774-
retain[childCoord.key] = childCoord;
775-
continue; // tile is covered by overzoomed child
776-
}
777-
} else {
778-
// check if all 4 immediate children are loaded (i.e. the missing ideal tile is covered)
779-
const children = tileID.children(this._source.maxzoom);
780-
781-
if (children.length === 4 &&
782-
retain[children[0].key] &&
783-
retain[children[1].key] &&
784-
retain[children[2].key] &&
785-
retain[children[3].key]) continue; // tile is covered by children
786-
787-
if (children.length === 1 &&
788-
retain[children[0].key]) continue; // tile is covered by overscaled child
789-
}
777+
missingIdealTiles = this._retainLoadedChildren(missingIdealTiles, retain);
790778

791-
// We couldn't find child tiles that entirely cover the ideal tile; look for parents now.
779+
// for remaining missing tiles with incomplete child coverage, seek a loaded parent tile
780+
for (const idealKey in missingIdealTiles) {
781+
const tileID = missingIdealTiles[idealKey];
782+
let tile = this._tiles[idealKey];
792783

793784
// As we ascend up the tile pyramid of the ideal tile, we check whether the parent
794785
// tile has been previously requested (and errored because we only loop over tiles with no data)
795786
// in order to determine if we need to request its parent.
796-
let parentWasRequested = tile.wasRequested();
787+
let parentWasRequested = tile?.wasRequested();
797788

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

‎src/source/tile_id.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ export class OverscaledTileID {
123123
}
124124
}
125125

126+
isOverscaled() {
127+
return (this.overscaledZ > this.canonical.z);
128+
}
129+
126130
/*
127131
* calculateScaledKey is an optimization:
128132
* when withWrap == true, implements the same as this.scaledTo(z).key,

‎test/build/min.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ describe('test min build', () => {
3838
const decreaseQuota = 4096;
3939

4040
// feel free to update this value after you've checked that it has changed on purpose :-)
41-
const expectedBytes = 940815;
41+
const expectedBytes = 941940;
4242

4343
expect(actualBytes).toBeLessThan(expectedBytes + increaseQuota);
4444
expect(actualBytes).toBeGreaterThan(expectedBytes - decreaseQuota);

0 commit comments

Comments
 (0)