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.

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

[Maps] Keep track of tile loading state #90970

Closed

Conversation

thomasneirynck
Copy link
Contributor

@thomasneirynck thomasneirynck commented Feb 10, 2021

WIP - POC

Closes #76237


[UPDATE] Second approach: 5f37ebc

This keeps explicit track of the life-cycle of each tile in a separate data-structure (this._tileCache)

  • sourcedataloading: a tile load event is started (this is internal to mapbox, doesn't even correspond to actual network request) -> add to the cache
  • error: a tile load failed (e.g. due to 503 from server) -> remove from the cache
  • sourcedata: a tile load event succeeded -> remove from the cache

This also needs to explicitly erase aborted requests, when tile-loading-state is determined. An aborted request does not have an associated event associated with it (aborted state can be acccessed through internal property).


First approach: fb5d418

This listens to sourcedata events, and checks source tile status with MbMap#isSourceLoaded(sourceId)

MbMap#isSourceLoaded(sourceId) seems buggy, or at least, does not entirely behave as expected. Most notably, it will return true when there are pending requests. This starts to happen after the layer has had many failed tile-request (e.g. because server does not support zoom level).

Can test with http://c.tile.stamen.com/watercolor/{z}/{x}/{y}.jpg

  1. Add TMS layer: http://c.tile.stamen.com/watercolor/{z}/{x}/{y}.jpg
  2. Zoom in/out a bit between 0 and 10: -> loader behaves as expected
  3. zoom in to level 20: -> tiles start to error out -> spinner keeps showing (fair enough, although not ideal)
  4. zoom back out to level 10, pan a bit -> good tiles are being requested -> spinner never shows anymore (this is not what we'd like)

Will need to see if there's a better approach (e.g. hooking into lifecycle of individual tiles, keeping track of requested tiles and success/errors on per layer basis).

use tile lifecycle
@thomasneirynck thomasneirynck added discuss release_note:enhancement [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation WIP Work in progress labels Feb 10, 2021
@kibanamachine
Copy link
Contributor

kibanamachine commented Feb 10, 2021

💔 Build Failed

Failed CI Steps

Metrics [docs]

‼️ ERROR: metrics for 5f37ebc were not reported

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

setAreTilesLoaded: (layerId: string, areTilesLoaded: boolean) => void;
}

interface Tile {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about moving all of the tile loading logic into a separate component? For example, TooltipControl, isolates all of the listeners and logic for tooltip events. I think it would be nice to do the same for tile events.

@thomasneirynck
Copy link
Contributor Author

closing in favor of #91585, going with approach 2, but moving unrelated changes to different PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation discuss release_note:enhancement WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Maps] Feature Request: have maps tile layers show loading status
3 participants