mapbox / mapbox-maps-ios

Interactive, thoroughly customizable maps for iOS powered by vector tiles and Metal
https://www.mapbox.com/mapbox-mobile-sdk
Other
462 stars 149 forks source link

`onMapLoaded` / `onMapLoadingError` not called upon startup #2207

Closed BinaryDennis closed 2 months ago

BinaryDennis commented 2 months ago

Environment

Observed behavior and steps to reproduce

  1. The user has approved location access while the app is being used, including precise location
  2. The user starts up the app
  3. In SwiftUI: Mapbox.Map is being created
  4. onStyleLoaded is called by Mapbox
  5. Possible bug: onMapLoaded nor onMapLoadingError is being called even if the user waits for a long time

Expected behavior

  1. same as above
  2. same as above
  3. same as above
  4. same as above
  5. onMapLoaded or onMapLoadingError is always called after onStyleLoaded

Notes / preliminary analysis

  1. This happens from time to time, not always. But it seems to happen more frequently when the device has recently changed network, eg turning on/off airplane mode. Also, it seems that opening Apple Maps before opening my app, somehow "fixes" the problem, ie onMapLoaded is called.

  2. Ive mostly tested on iPhone 14 Pro device, iPhone 11 Pro device, and on most iOS simulators (although I think the problem occurs more often on iPhone 8 Plus simulator running iOS 16.4)

  3. Ive noticed that even when onMapLoaded is not called, the actual Map/Style, the puck, and the MapContent (PolylineAnnotationGroup) that Ive created (a route) are rendered correctly.

Additional links and references

This is the gist of the SwiftUI View Ive created...

import SwiftUI
@_spi(Experimental) import MapboxMaps

struct MapView: View {

    @ObservedObject var viewModel: MapViewModel

    private let layerIdPuck = "puck" // NOTE: the Puck2D view has a hardcoded layer id of puck

    struct PulseConfig {
        func with(color: Color) -> Puck2DConfiguration.Pulsing {
            var pulsing = Puck2DConfiguration.Pulsing()
            pulsing.isEnabled = true
            pulsing.radius = .accuracy
            pulsing.color = UIColor(color)
            return pulsing
        }
    }

    var body: some View {
        MapReader { proxy in
            Map(viewport: $viewModel.viewport) {
                let puckLayerId = viewModel.map?.allLayerIdentifiers.map(\.id).first { $0 == layerIdPuck } 

                puck //Puck2D
                selectedPOI //MapViewAnnotation
                droppedPin //MapViewAnnotation
                gpxRouteAndMarkers(puckLayerId: puckLayerId)  //PolylineAnnotationGroup, a route

            }
            .presentsWithTransaction(true) // Synchronize Metal and CALayer for better VA performance.
            .ornamentOptions(OrnamentOptions(
                scaleBar: ScaleBarViewOptions(
                    position: .topLeft,
                    visibility: viewModel.scaleBarVisibility),
                compass: CompassViewOptions(
                    position: .topRight,
                    margins: .init(x: 8.0, y: viewModel.compassTopPadding),
                    visibility: viewModel.compassVisibility)
            ))
            .onStyleLoaded(action: { _ in
                viewModel.styleLoaded(proxy)    // <====  THIS IS ALWAYS CALLED
            })
            .onMapLoaded(action: { _ in
                viewModel.mapLoaded(proxy)      // <====  THIS IS NOT ALWAYS CALLED 🐛 (and in those cases where its not called, the `onMapLoadingError` is not called either!)
            })
            .onMapLoadingError(action: { error in
                ...
            })
            .mapStyle(MapStyle.outdoors)
        }
    }

    var puck: MapContent {
        Puck2D(bearing: .course)
            .showsAccuracyRing(true)
            .pulsing(viewModel.showPuckTopImage ? PulseConfig().with(color: themeColors.primaryColor) : .default)
            .topImage(viewModel.showPuckTopImage ? UIImage(named: "dash-puck") : nil)
    }

}

Iv also tried a work-around that unfortunately did not fix the problem, before the View is being rendered I called this function in the view model to "prepare" the location

 func workAroundForMapBoxNotAlwaysCalllingMapLoaded() {
        let manager = CLLocationManager()
        let authorizedStatuses: [CLAuthorizationStatus] = [.authorizedAlways, .authorizedWhenInUse]
        let isAccepted = authorizedStatuses.contains(manager.authorizationStatus)
        guard isAccepted else { return }
        manager.startUpdatingLocation()
        _ = manager.location
 }
persidskiy commented 2 months ago

@BinaryDennis Hi, thank you for the report.

This situation may happen when the root StyleJSON is loaded, but some resources like sprites, or tiles aren't loaded yet. Then the engine will try reload them with some backoff.

Most of the time using the onStyleLoaded is safe and may be a better alternative. What's your use-case?

BinaryDennis commented 2 months ago

Thanks for looking into this issue.

I was assuming that onMapLoaded indicates that the MapView is ready to be used, ie animating viewport changes, start observing events such as location/camera updates etc etc. Therefore I have a spinner as an overlay on the map view until onMapLoaded is called. Thus users cannot interact with the map until such time.

What is the maximum time it can take until this operation is considered fail, and thus onMapLoaded or onMapLoadingError will be called?

The reason Im asking, is because I have waited more than 30 seconds for the callback onMapLoaded but still not received it. And in my experience both as an end-user and app-developer, I dont believe end users will wait longer than that on any UI to be loaded in a mobile context.

So one could also ask, if its good-enough to wait for onStyleLoaded before starting to "work" with the map, then what is the added value that onMapLoaded will give to a dev-user of your API given that it might take quite a long time before that callback is called?

In what scenarios would one need to use onMapLoaded then? If there are no practial scenarios, perhaps it should be removed from the API in order to decrease confusion?

Best Regards Dennis

persidskiy commented 2 months ago

@BinaryDennis Thank you for the details. I'll try to address your questions.

The map loading process is progressive, meaning the map can be rendered on the screen much sooner than it is "completely loaded" (onMapLoaded event). This way, the end user will see the map as soon as possible, like a web page.

In the worst case scenario, in slow and inconsistent networks, the full load may take minutes. When the network is available, the engine retries loads. This is done because there is no "refresh" button in maps, unlike browsers and our ultimate goal is to render the map.

The loading process itself is quite complicated because the style can have subresources such as images, sources, and other imported styles. Also, the cache may be used. The general idea of the loading flow is demonstrated here https://docs.mapbox.com/ios/maps/api/11.5.0/documentation/mapboxmaps/mapboxmap

At the style loaded stage (onStyleLoaded event), the StyleJSON is loaded and applied, so the engine starts to render it while some parts are still being loaded. The style model is known, so 99.9% of the API is safe to use. In some rare cases when you want to modify the source loaded from the style of something like that, some other event may be used.

I'll comment some of your statements below

I was assuming that onMapLoaded indicates that the MapView is ready to be used, ie animating viewport changes, start observing events such as location/camera updates etc etc.

It's safe to do at any time, even before the style is loaded.

I have a spinner as an overlay on the map view until onMapLoaded is called

It's better to avoid this, or use onStyleLoaded at least.

In general, the API (especially SwiftUI) is designed in a way to allow the developer to just tell the map what to render and not care much about the loading process. For example, the Annotations/Layers/Sources that you put into Map just render as soon as possible, observing the loading process internally. The Viewport API observes the style loading process internally to work with the camera correctly.

In what scenarios would one need to use onMapLoaded then? If there are no practial scenarios, perhaps it should be removed from the API in order to decrease confusion?

One can use it for logging purposes or something else. But we probably will revisit it in the major versions.

BinaryDennis commented 2 months ago

Thank you for that elaborate answer @persidskiy

And thanks for linking to depiction of the general flow of loading.

I will rewrite my code not to wait for onMapLoaded based on your answer then.

👍