From 132bfd7f8514eb211afef490567834bf75c6e029 Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Fri, 13 Apr 2018 11:47:09 -0400 Subject: [PATCH] fix based on anand's feedback (fixup) - clarify comment - make wrapDelta more self documenting - move tileid reassigning comment to approprtiate blance - number | void - inline resetting cache reload timers --- src/source/source_cache.js | 49 +++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/src/source/source_cache.js b/src/source/source_cache.js index e1a6320c5b5..36a69738c75 100644 --- a/src/source/source_cache.js +++ b/src/source/source_cache.js @@ -43,7 +43,7 @@ class SourceCache extends Evented { _sourceLoaded: boolean; _sourceErrored: boolean; _tiles: {[any]: Tile}; - _prevLng: number; + _prevLng: number | void; _cache: Cache; _timers: {[any]: TimeoutID}; _cacheTimers: {[any]: TimeoutID}; @@ -394,12 +394,25 @@ class SourceCache extends Evented { } handleWrapJump(lng: number) { - // Wrapping longitude values can cause a jump in `wrap` values. Tiles that - // cover a similar area of the screen can have different wrap values. This - // calculates this difference in wrap values so that we can match tiles - // across frames where the longitude gets wrapped. + // On top of the regular z/x/y values, TileIDs have a `wrap` value that specify + // which cppy of the world the tile belongs to. For example, at `lng: 10` you + // might render z/x/y/0 while at `lng: 370` you would render z/x/y/1. + // + // When lng values get wrapped (going from `lng: 370` to `long: 10`) you expect + // to see the same thing on the screen (370 degrees and 10 degrees is the same + // place in the world) but all the TileIDs will have different wrap values. + // + // In order to make this transition seamless, we calculate the rounded difference of + // "worlds" between the last frame and the current frame. If the map panned by + // a world, then we can assign all the tiles new TileIDs with updated wrap values. + // For example, assign z/x/y/1 a new id: z/x/y/0. It is the same tile, just rendered + // in a different position. + // + // This enables us to reuse the tiles at more ideal locations and prevent flickering. const prevLng = this._prevLng === undefined ? lng : this._prevLng; - const wrapDelta = Math.round((lng - prevLng) / 360); + const lngDifference = lng - prevLng; + const worldDifference = lngDifference / 360; + const wrapDelta = Math.round(worldDifference); this._prevLng = lng; if (wrapDelta) { @@ -410,7 +423,16 @@ class SourceCache extends Evented { tiles[tile.tileID.key] = tile; } this._tiles = tiles; - this._resetTileReloadTimers(); + + // Reset tile reload timers + for (const id in this._timers) { + clearTimeout(this._timers[id]); + delete this._timers[id]; + } + for (const id in this._tiles) { + const tile = this._tiles[id]; + this._setTileReloadTimer(id, tile); + } } } @@ -596,12 +618,12 @@ class SourceCache extends Evented { tile = this._cache.getAndRemove((tileID.wrapped().key: any)); if (tile) { - // set the tileID because the cached tile could have had a different wrap value if (this._cacheTimers[tileID.key]) { clearTimeout(this._cacheTimers[tileID.key]); delete this._cacheTimers[tileID.key]; this._setTileReloadTimer(tileID.key, tile); } + // set the tileID because the cached tile could have had a different wrap value tile.tileID = tileID; } @@ -636,17 +658,6 @@ class SourceCache extends Evented { } } - _resetTileReloadTimers() { - for (const id in this._timers) { - clearTimeout(this._timers[id]); - delete this._timers[id]; - } - for (const id in this._tiles) { - const tile = this._tiles[id]; - this._setTileReloadTimer(id, tile); - } - } - _setCacheInvalidationTimer(id: string | number, tile: Tile) { if (id in this._cacheTimers) { clearTimeout(this._cacheTimers[id]);