search

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

CameraAnimationsManager should avoid easing or flying to the current camera #520

Open 1ec5 opened 3 years ago

1ec5 commented 3 years ago

As of v10.0.0-rc.3, CameraAnimationsManager.fly(to:duration:completion:) and ease(to:duration:curve:completion:) unconditionally create and start animators, even if the target camera is literally or practically identical to the current camera. An application may chain many of these calls together, for example to follow the user’s location. But if the user doesn’t move, these chained animation calls end up wasting power to animate an otherwise still map.

These methods should compare the original and target cameras and avoid starting the animator if there isn’t an appreciable difference between the two cameras. At a glance, I don’t think it would be necessary to perform the same check for the raw makeAnimator(…) methods, because the client code probably expects to have to do more coordination around the animation and may have more sophisticated logic for debouncing animations, especially concurrent animations.

https://github.com/mapbox/mapbox-maps-ios/blob/cde8041e1cdadcabd61d3ee9fc964f1493fa2571/Sources/MapboxMaps/Foundation/Camera/CameraAnimationsManager.swift#L107 https://github.com/mapbox/mapbox-maps-ios/blob/cde8041e1cdadcabd61d3ee9fc964f1493fa2571/Sources/MapboxMaps/Foundation/Camera/CameraAnimationsManager.swift#L145

For reference, mapbox/mapbox-gl-native#7125 was the first of a few PRs that implemented camera debouncing in the previous iOS/macOS map SDK.

/cc @mapbox/maps-ios @MaximAlien

1ec5 commented 3 years ago

At a glance, I don’t think it would be necessary to perform the same check for the raw makeAnimator(…) methods, because the client code probably expects to have to do more coordination around the animation and may have more sophisticated logic for debouncing animations, especially concurrent animations.

As a case in point, the navigation SDK makes animators directly, so mapbox/mapbox-navigation-ios#3152 tracks adding debouncing logic there.

nishant-karajgikar commented 3 years ago

These methods should compare the original and target cameras and avoid starting the animator if there isn’t an appreciable difference between the two cameras.

@1ec5, what do you think an "appreciable difference" constitutes?