mapbox / mapbox/mapbox-gl-js

Don't load tiles that will be shown in a very small number of frames while animating

Open
#5,482 5 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

feature :green_apple:
Dominant language
TypeScript
Stars
12.4k
Forks
2.4k
PR merge metrics
No merged PRs in 30d

Description

## Motivation

When animating the map, it's possible to create a scenario where tiles are repeatedly requested but never cached:

For example, when repeatedly animating back and forth between z7.5 and z8.5, all necessary z7 tiles for the resting z7.5 state will be retained, and all necessary z8 tiles for the resting z8.5 state will be retained, but there may be (likely, but depends on exact viewport dimensions) rows and/or columns of z8 tiles that will be needed only in a few frames in the middle of the animation (they'll be outside of the viewport when the map is a bit further zoomed in to 8.5). We request those tiles during the animation, but then when the SourceCache goes to remove it, it's not [added to the cache](https://github.com/mapbox/mapbox-gl-js/blob/c173ce8d4034dd9305c10a14587b6e1c0d3166f1/src/source/source_cache.js#L585-L588) because its state is still `'loading'`, so it fails [a `hasData()` check](https://github.com/mapbox/mapbox-gl-js/blob/c173ce8d4034dd9305c10a14587b6e1c0d3166f1/src/source/tile.js#L432-L434). The tile is aborted, never finishes loading and is never cached, so it'll be repeatedly requested when zooming back and forth between these fractional zooms. (A similar case may happen in any animation — `flyTo`, etc — or with rapid pan/zoom scrubbing.)

## Design Alternatives

* Make tile loading aware of ongoing animations, calculate the duration that a new tile will be visible based on the animation, and decide on some heuristic that determines that _n_ frames isn't worth loading a tile (especially if we have another tile available that we could overzoom perhaps).

* Circumstantially loosen rules about tile aborting. Maybe tiles in certain situations (tiles requested during programmatic animations? Raster tiles, since they don't require bucket parsing? Any tiles, when a map/source `allowTilesToFinishProcessing` param is set?) could be allowed to finish downloading and immediately moved to the cache.

* Provide developers with a way to preload tiles for an animation (https://github.com/mapbox/mapbox-gl-js/issues/2470).

* Do nothing.

## Design

I don't have a strong conviction on what's the right/best answer here, but option 1 seems reasonable and practical. It's possible we may need to make this a configurable heuristic.

cc @mapbox/gl-core

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the linked SourceCache and Tile code around the hasData() and cache-removal behavior described in the issue. Compare the listed animation and tile-aborting alternatives, then define a concrete approach whose completion can be checked by repeated fractional-zoom animation no longer repeatedly requesting tiles that are briefly visible.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend, performance
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.