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

[Camera Animations] Introduce short-lived camera view #282

Merged
merged 40 commits into from
Apr 27, 2021

Conversation

nishant-karajgikar
Copy link
Contributor

@nishant-karajgikar nishant-karajgikar commented Apr 20, 2021

PRs must be submitted under the terms of our Contributor License Agreement CLA.

Pull request checklist:

  • Briefly describe the changes in this PR.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality. If tests were not written, please explain why.
  • Add example if relevant.
  • Document any changes to public APIs.
  • Apply changelog label ('breaking change', 'bug 🪲', 'build', 'docs', 'feature 🍏', 'performance ⚡', 'testing 💯') or use the label 'skip changelog'
  • Add an entry inside this element for inclusion in the mapbox-maps-ios changelog: <changelog></changelog>.
  • Update the migration guide, API Docs, Markdown files - Readme, Developing, etc

Summary of changes

CameraView

  • The role of the CameraView has been significantly curtailed with this PR.
  • The CameraView will only exist (i.e. be part of the view hierarchy) for the duration of an animated transition.
  • All business logic has been pulled out of the cameraView.

CameraManager

  • The makeCameraAnimator* methods have been updated to accept CameraAnimations as required arguments.
  • The makeCameraAnimator* methods have been renamed to makeAnimator* methods

CameraTransition (new)

  • The CameraTransition is a new construct that allows better control on the "from" and "to" values of a camera animation
  • An instance of this type is passed into each animation block and is designed to be mutated by the developer.
  • The mutated value is then used to construct the interpolation

FlyToCameraAnimator (new)

  • flyTo has been re-implemented to rely solely on the FlyToInterpolator. There is no interaction with keyframe animations any more.

BasicCameraAnimator (renamed from CameraAnimator)

  • Apart from a rename, this construct now represents a singular transition.

CameraAnimator (new)

  • Both FlyToCameraAnimator and BasicCameraAnimator conform to CameraAnimator
  • The protocol is designed as a way for SDK consumers to interrupt animations and check on the state of those animations.

CameraAnimatorInterface(new)

  • Internal facing protocol that both FlyToCameraAnimator and BasicCameraAnimator conform to
  • Used to update the camera on each tick of the display link

Gestures

  • Gestures now directly call __map.setCamera() instead of going via CoreAnimation
  • This results in a nice bump in gesture performance due to the greater frequency of touch events as opposed to screen refresh rate.

User Impact

  1. The call site for camera animators now looks like:

    mapView.camera.makeAnimator(duration: 3, curve: .easeOut) { (transition) in 
        transition.center.toValue = CLLocationCoordinate2D(latitude: 10, longitude: 10)
        transition.zoom.toValue = 14
        transition.bearing.toValue = -35
    }
  2. Animations can only be added to an animator at init-time. They must be passed into one of the makeAnimator* functions.

  3. The addAnimation(_ animations: () -> Void, delayFactor: CGFloat) method has been removed in favor of constructor injecting animations.

Videos:

FlyTo

RPReplay_Final1618957459.MP4

Animations

RPReplay_Final1618957647.MP4

Animated Padding

padding.mov

@nishant-karajgikar nishant-karajgikar added the breaking change ⚠️ If your pull request introduces a breaking change and updates are required when version is published label Apr 20, 2021
@macdrevx macdrevx closed this Apr 21, 2021
@macdrevx macdrevx deleted the nishantk/short-lived-camera-view-2 branch April 21, 2021 18:32
@macdrevx macdrevx restored the nishantk/short-lived-camera-view-2 branch April 21, 2021 18:33
@macdrevx
Copy link
Contributor

My mistake 😬

@macdrevx macdrevx reopened this Apr 21, 2021
@nishant-karajgikar nishant-karajgikar force-pushed the nishantk/short-lived-camera-view-2 branch 3 times, most recently from 78284b1 to 7f90949 Compare April 21, 2021 19:44
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@nishant-karajgikar nishant-karajgikar force-pushed the nishantk/short-lived-camera-view-2 branch 2 times, most recently from 6bb9c4d to 9be58e0 Compare April 22, 2021 15:31
*/
func schedulePendingCompletion(forAnimator animator: CameraAnimator,
/// Ask the map to set camera to new camera
func jumpTo(camera: CameraOptions)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should consider splitting up the responsibilities of this protocol. Some of them could be fulfilled by injecting each animator with the MapboxMap. The rest (the ones that are UIView-related could be contained in a more focused protocol.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with this. Could we perhaps do it as part of subsequent PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nishant-karajgikar nishant-karajgikar force-pushed the nishantk/short-lived-camera-view-2 branch from c434462 to c7959d1 Compare April 27, 2021 14:42
@nishant-karajgikar nishant-karajgikar force-pushed the nishantk/short-lived-camera-view-2 branch 2 times, most recently from 17c3f85 to 370ece1 Compare April 27, 2021 15:53
@nishant-karajgikar nishant-karajgikar force-pushed the nishantk/short-lived-camera-view-2 branch from 370ece1 to 62183b9 Compare April 27, 2021 15:54
@nishant-karajgikar nishant-karajgikar force-pushed the nishantk/short-lived-camera-view-2 branch 3 times, most recently from 13e79cb to be3f0a0 Compare April 27, 2021 17:32
@nishant-karajgikar nishant-karajgikar force-pushed the nishantk/short-lived-camera-view-2 branch from be3f0a0 to ef5a8ef Compare April 27, 2021 17:36
@nishant-karajgikar nishant-karajgikar force-pushed the nishantk/short-lived-camera-view-2 branch from 6fd2914 to 73aaef2 Compare April 27, 2021 18:28
@nishant-karajgikar nishant-karajgikar merged commit 905c069 into main Apr 27, 2021
@nishant-karajgikar nishant-karajgikar deleted the nishantk/short-lived-camera-view-2 branch April 27, 2021 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change ⚠️ If your pull request introduces a breaking change and updates are required when version is published
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants