Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1ec5 opened this issue Jul 7, 2021 · 2 comments
Open
Labels
archived performance ⚡ Updates to the performance, metrics, or optimization of the SDK

Comments

@1ec5
Copy link
Contributor

1ec5 commented Jul 7, 2021

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.

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 1ec5 added the performance ⚡ Updates to the performance, metrics, or optimization of the SDK label Jul 7, 2021
@1ec5
Copy link
Contributor Author

1ec5 commented Jul 7, 2021

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
Copy link
Contributor

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?

@stale stale bot added the archived label Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archived performance ⚡ Updates to the performance, metrics, or optimization of the SDK
Projects
None yet
Development

No branches or pull requests

2 participants