mapbox-maps-ios icon indicating copy to clipboard operation
mapbox-maps-ios copied to clipboard

CameraAnimationsManager should avoid easing or flying to the current camera

Open 1ec5 opened this issue 4 years ago • 2 comments

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 avatar Jul 07 '21 17:07 1ec5

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.

1ec5 avatar Jul 07 '21 18:07 1ec5

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?

nishant-karajgikar avatar Jul 08 '21 18:07 nishant-karajgikar