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

Downloading an offline pack blocks the visible map from loading #4414

Closed
friedbunny opened this issue Mar 21, 2016 · 8 comments
Closed

Downloading an offline pack blocks the visible map from loading #4414

friedbunny opened this issue Mar 21, 2016 · 8 comments
Assignees
Labels
bug Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS offline

Comments

@friedbunny
Copy link
Contributor

If the active style of a map view is changed while an offline pack is downloading, the new style will fail to load (i.e., it only shows a black screen) while that download is active.

  1. Start downloading an offline pack
  2. Switch the visible style on your map view
  3. The new style will not load until the offline pack finishes (or pauses) downloading

/cc @jfirebaugh @1ec5 @zugaldia

@friedbunny friedbunny added bug iOS Mapbox Maps SDK for iOS offline labels Mar 21, 2016
@1ec5 1ec5 added the macOS Mapbox Maps SDK for macOS label Mar 21, 2016
@friedbunny friedbunny changed the title Switching styles while downloading an offline pack blocks the new style from loading Downloading an offline pack blocks the visible map from loading Mar 31, 2016
@friedbunny
Copy link
Contributor Author

This will also prevent the current map from loading (e.g., viewDidLoad), if you start the offline download before it has a chance to finish loading.

img_4116
Offline download (or pack verification) in progress

img_4117
Offline download finished, map finally loads

@jfirebaugh
Copy link
Contributor

This occurs because OnlineFileSource has a single FIFO queue for requests, which OfflineDownload basically floods with all the tile requests at once. So not only will an offline download prevent a new style from loading until the download is complete, it'll prevent new tiles from loading as well.

The options for fixing this are:

The second option sounds most attractive to me. To reduce memory usage, we know we want to change the OfflineDownload implementation anyway so that it requests tiles in a streaming fashion, rather than all in one shot, and metering would fit into that.

@halset
Copy link
Contributor

halset commented Jul 28, 2016

@jfirebaugh Any tips on how to get going on the second option?

I have tried to use a mutex and a condition_variable to control the number of concurrent requests from the OfflineDownload, but it does not work as the requests are created and executed on the same thread using the RunLoop.

Issuing "util::RunLoop::Get()->runOnce();" after each ensureResource will at least keep the queues small, but it does not improve the interactivity of the map by much.

@friedbunny friedbunny added the Core The cross-platform C++ core, aka mbgl label Aug 3, 2016
@halset
Copy link
Contributor

halset commented Aug 16, 2016

(Sorry for the noise, but I am grateful for any help on this subject, @friedbunny @incanus.)

@jfirebaugh
Copy link
Contributor

At minimum, the loop over tiles and the loop over glyphs need to be made incremental and fully asynchronous. These are the two loops that really flood OnlineFileRequest's own internal queue, delaying requests triggered by user interactions. Therefore rather than calling ensureResource in a loop for every required resource, ensure only a limited number, keep the rest in a queue, and draw from the queue when a prior request completes.

I'm not sure whether it makes sense to fully generalize the request queuing mechanism to the other resource types as well (style, sources, and sprites), or whether that would add too much complexity. You would probably have to track more state explicitly as class members, because it's no longer tracked implicitly via the stack.

As noted, OnlineFileSource has a similar internal queuing mechanism already. On one hand, we should consider if there are any possibilities for code reuse. On the other, OnlineFileSource is suffering from bugs in its state management (#5827), so beware, and try for something simpler.

@Duckbadger
Copy link

Hi @jfirebaugh, any update on this?
I've used both version 3.3.4 and alpha 4 of 3.4.0, and calling suspend still has to wait until the queue has finished checking previously downloaded resources.
It's a huge blocker at this point because the map view will not load any tiles until that task is suspended, which is often a large amount of time.

@jfirebaugh
Copy link
Contributor

It's fixed for the next release (3.4.0 alpha5).

@Duckbadger
Copy link

Okay thanks, we'll check it out when alpha 5 is available.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS offline
Projects
None yet
Development

No branches or pull requests

5 participants