Skip to content
This repository was archived by the owner on Aug 8, 2023. It is now read-only.

Use stale styles - #2721

Merged
kkaefer merged 10 commits into
masterfrom
2721-load-stale-styles
Jan 15, 2016
Merged

kkaefer merged 10 commits into
masterfrom
2721-load-stale-styles

Conversation

@kkaefer

@kkaefer kkaefer commented Jan 4, 2016

Copy link
Copy Markdown
Member

With #2617 in place, we now get stale data from the cache. Currently, stale styles aren't used, but we should change this. This could be trivially implemented by regular style update (= treat it like setting a new style). Currently, this means that we have to reparse everything, and will see a short flicker. However, as soon as we implement style transitions (#2445), we'll get a flicker-free update.

@kkaefer kkaefer added the performance Speed, stability, CPU usage, memory usage, or power usage label Oct 22, 2015
@kkaefer kkaefer self-assigned this Jan 4, 2016
@kkaefer
kkaefer force-pushed the 2721-load-stale-styles branch 3 times, most recently from df6c156 to b10541e Compare January 7, 2016 11:24
@kkaefer kkaefer mentioned this pull request Jan 7, 2016
@kkaefer
kkaefer force-pushed the 2721-load-stale-styles branch 3 times, most recently from b403380 to 5ed4d4e Compare January 7, 2016 18:06
This was referenced Jan 7, 2016
@kkaefer
kkaefer force-pushed the 2721-load-stale-styles branch from 5ed4d4e to 5f13429 Compare January 7, 2016 20:15
@kkaefer

kkaefer commented Jan 7, 2016

Copy link
Copy Markdown
Member Author

👀 @jfirebaugh @tmpsantos

@kkaefer

kkaefer commented Jan 7, 2016

Copy link
Copy Markdown
Member Author

For reference, this patch incorporates #2720, #2722 and #2724 as well.

@jfirebaugh

Copy link
Copy Markdown
Contributor

This PR has a lot of changes and is hard to review comprehensively. Is it possible to split it up into smaller units?

@kkaefer

kkaefer commented Jan 11, 2016

Copy link
Copy Markdown
Member Author

Will roll this again after #3480 is in.

@kkaefer

kkaefer commented Jan 14, 2016

Copy link
Copy Markdown
Member Author

@jfirebaugh pls 👀 (minus the first two commits)

jfirebaugh added a commit that referenced this pull request Jan 15, 2016
Until #2721 lands we still need this.
This adds support for using cached styles that are stale. They're treated like changing styles; when the refreshed style changed compared to the one we've already had, we're swapping out the entire style, which might cause a slight flicker.
We're now supporting using stale TileJSON and GeoJSON data. When we receive a new answer with an updated TileJSON file, we're replacing the Source's metadata with the new one and trigger updates to make sure we're loading the correct tiles. Similarly, GeoJSON data will be reparsed.
Otherwise, this will remove them from the map and overwrite the old data. This could happen e.g. when we have connection trouble and a tile needs to be refreshed, or we're displaying stale information and the connection is down.
@kkaefer
kkaefer force-pushed the 2721-load-stale-styles branch 2 times, most recently from c4f35aa to 88d72fe Compare January 15, 2016 12:33
This is a naïve implementation that essentially merges updated data into existing data. It will *not* remove icons from the stale sprite if they aren't present in the fresh sprite (we aren't tracking the source of a sprite, and the user could have changed it as well). Similarly, it will not update icons that have changed in dimension. This is a rare edge case and probably not worth implementing.
Previously, we could first get a stale response from cache, then immediately a "connection error" response that overwrote the error state variable. In the tileLoadingCompleteCallback, we'd only see the second overwritten error state and abort. This could lead to the map sometimes not being rendered at all until the user interacts with the map.
Updating glyphs is still unsupported, and there's no good use case for doing so. When we're using a stale glyph PBF, and the fresh answer contains changed to that glyph, we will continue to use the old glyph.
@kkaefer
kkaefer force-pushed the 2721-load-stale-styles branch from 88d72fe to a88e58e Compare January 15, 2016 16:07
@kkaefer
kkaefer merged commit a88e58e into master Jan 15, 2016
@kkaefer
kkaefer deleted the 2721-load-stale-styles branch January 15, 2016 16:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

performance Speed, stability, CPU usage, memory usage, or power usage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants