mapbox / mapbox-navigation-ios

Turn-by-turn navigation logic and UI in Swift on iOS
https://docs.mapbox.com/ios/navigation/
Other
860 stars 310 forks source link

Performance issues on older iPads #4003

Open RRuais opened 2 years ago

RRuais commented 2 years ago

Mapbox Navigation SDK version

2.5.1

Steps to reproduce

On older iPads (iPad 6th generation or older) when displaying a static map with user location the CPU will ramp to 60+ percent on device and remain at that level without directly interacting with or physically moving the iPad. This seems to be attributed to consistently updating the heading causing a redraw of the map.

If we add a distance filter to the mapView location options the CPU returns to an expected level.

navigationMapView.mapView.location.options.distanceFilter = 10

Newer iPads do not show the same CPU peak as older iPads.

OfflineMaps.zip

Screen Shot 2022-07-12 at 14 10 03

Expected behavior

Using a static map should not overload the iPad's CPU.

Actual behavior

Using a static map will overload the iPad's CPU for older generation iPads causing poor performance and lack of responsiveness to the iPad itself. A large portion of the iPads using our app are 5th to 6th generation.

Is this a one-time issue or a repeatable issue?

repeatable

DavidKasprzyk commented 2 years ago

Here's a sample of cpu and energy impact from a run on one of our iPads (6th Generation). I've only opened the attached sample app and the iPad is sitting on the desk in front of me.

Screen Shot 2022-07-12 at 19 29 42
ShanMa1991 commented 2 years ago

Hi @DavidKasprzyk and @RRuais , thanks for the feedback, this issue could be reproduced with the single MapView from the following steps:

  1. Open the Examples App of https://github.com/mapbox/mapbox-maps-ios, build it on simulator iPad (6th generation)
  2. Change the Feature-Location to City Run on the simulator
  3. Choose the Display the user's location from Location Examples.

We could see the cpu is running around 56% - 65%. It's related with the mapView rendering with the location update. Because the NavigationMapView is built with the MapView from Maps SDK for iOS . Do you have any suggestions on this issue? @mapbox/maps-ios

DavidKasprzyk commented 2 years ago

I'm wondering if there's a minimum delta that can be set for heading changes before Mapbox initials a full redraw. I'm not sure the best minimum value to use, but unless these older iPads are drastically varying their heading when simply being held static, then perhaps only updating if the delta heading is greater than that set minimum would you actually want to trigger a full map redraw. This may take a little experimentation to get that value set to an acceptable constant, but I would think 1º as a initial value for the delta would be a good starting point.

If it's not heading, but the actual location that triggering the redraw, then again maybe there's a specific minimum delta position that must be reached before Mapbox tries to update the view.

ShanMa1991 commented 2 years ago

@DavidKasprzyk In fact we could apply the MapView.location.options.distanceFilter to reduce the redraw frequency manually. Because if we display user location, we allow the MapView to be updated with the location. The ticket has also been created in Map side in https://github.com/mapbox/mapbox-maps-ios/issues/1471 to track.

evil159 commented 2 years ago

@DavidKasprzyk If you use the default AppleLocationProvider - then heading updates are already being filtered by 1º. Meaning that CLLocationManager notifies about heading update only when heading delta > 1º from the previous update.

It could be that 1º is still too low(especially for older hardware), I tested it with a phone and it seems that even slightest movements trigger heading updates. In my testing 10º threshold produced pretty good results visually(almost no delay for the puck bearing indicator) while keeping the update frequency reasonable. I think for slower devices it could be bumped a bit higher.

DavidKasprzyk commented 2 years ago

Is there a way for us to configure the heading updates as an option, like we are able to with the distanceFilter? Would it require us to not just use the default configurations? If so we can try this on our end as well with the older iPads and report back.

evil159 commented 2 years ago

Currently we don't expose a way to configure heading filter, modifying AppleLocationProvider would be the fastest way to try it out(insert locationProvider.headingFilter = myThreshold at AppleLocationProvider.swift:8 or create a custom LocationProvider(based on CLLocationManager still). We have put this issue into our board and will be investigating from our end as well.

evil159 commented 1 year ago

@ShanMa1991 @DavidKasprzyk @RRuais We've discovered the cause of excessive map redraws when idling https://github.com/mapbox/mapbox-maps-ios/pull/1789

DavidKasprzyk commented 1 year ago

@evil159 thanks so much; we will look to validate on our end with the new release.